Skip to content

feat: Add --merge option to CLI tool #611

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
May 15, 2025
Merged

Conversation

danielbayley
Copy link
Contributor

# test.yaml
a: &a
  a: 2
b:
  <<: *a
yaml --single --merge --json < test.yaml
{
  "a": {
    "a": 2
  },
  "b": {
    "a": 2
  }
}

@eemeli
Copy link
Owner

eemeli commented Mar 20, 2025

Do you have a use case for this that isn't solved by --yaml 1.1? As in, are you actually working with documents that extend the YAML 1.2 schema with support for << merge keys, or are you working with YAML 1.1 documents?

@danielbayley
Copy link
Contributor Author

are you working with YAML 1.1 documents?

Well I'm not currently using old documents which I have no control over, if that's what you mean?

But I don't want to be stuck on the 1.1 spec to be able to retain the use of << merge keys going forward…

This PR simply enables the existing merge option. Is the rationale different between the underlying API, and the CLI?

Also, out of curiosity, why was the << merge key removed in the 1.2 spec? @eemeli

@eemeli
Copy link
Owner

eemeli commented Apr 2, 2025

This PR simply enables the existing merge option. Is the rationale different between the underlying API, and the CLI?

Yeah, a bit. I'd prefer to keep the CLI pretty minimal, rather than presuming that all the options it could have would have actual use.

Regarding --merge my concern is that in other tools supporting << tends to correlate with presuming YAML 1.1 values in general, where e.g. infamously the plain sclarar no parses as a boolean. So if you're using the yaml CLI to emit YAML without --yaml 1.1 but still reading the output elsewhere with a YAML 1.1 parser, you could get surprised by the results.

Are there other parses than yaml that you're using to work with your documents?

Also, out of curiosity, why was the << merge key removed in the 1.2 spec?

I'm afraid I wasn't around during that transition, so can't speak authoritatively. But as an implementer, it works very differently from anything else in the language and isn't really well defined.

@danielbayley
Copy link
Contributor Author

danielbayley commented Apr 3, 2025

I'd prefer to keep the CLI pretty minimal

Not an ureasonable sentiment, but this is a single option, for a substantial feature—substantial in it's ability to cut down on a lot of duplicate config at least…

infamously the plain scalar no parses as a boolean.

Which is exactly the kind of thing I'm getting at… I definitely don't want plain scalar yes/no to ever be parsed as boolean, but do want to retain << merge ability… So if the CLI is arbitrarily restricted to --yaml 1.1 as the only way to enable the latter, I'm kind of screwed.

Are there other parses than yaml that you're using to work with your documents?

YAMLScript, and sometimes js-yaml, but I am trying to migrate everything possible over to this superior package, including replacing js-yaml in pnpm with this.

Copy link
Owner

@eemeli eemeli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I definitely don't want plain scalar yes/no to ever be parsed as boolean, but do want to retain << merge ability… So if the CLI is arbitrarily restricted to --yaml 1.1 as the only way to enable the latter, I'm kind of screwed.

Ok, that makes sense. I added a couple more tests just to make sure the option was working as intended, but the code itself looks good.

@eemeli eemeli merged commit 0f29ce6 into eemeli:main May 15, 2025
21 checks passed
@danielbayley danielbayley deleted the cli--merge branch May 15, 2025 09:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants