Skip to content
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

Support merge keys #31

Open
travisbrown opened this issue Jun 8, 2017 · 3 comments
Open

Support merge keys #31

travisbrown opened this issue Jun 8, 2017 · 3 comments

Comments

@travisbrown
Copy link
Member

SnakeYAML supports YAML's "merge key" extension, which makes the last four elements in this sequence resolve to the same thing:

scala> val doc = """
     | ---
     | - &CENTER { x: 1, y: 2 }
     | - &LEFT { x: 0, y: 2 }
     | - &BIG { r: 10 }
     | - &SMALL { r: 1 }
     | 
     | # All the following maps are equal:
     | 
     | - # Explicit keys
     |   x: 1
     |   y: 2
     |   r: 10
     |   label: center/big
     | 
     | - # Merge one map
     |   << : *CENTER
     |   r: 10
     |   label: center/big
     | 
     | - # Merge multiple maps
     |   << : [ *CENTER, *BIG ]
     |   label: center/big
     | 
     | - # Override
     |   << : [ *BIG, *LEFT, *SMALL ]
     |   x: 1
     |   label: center/big
     | """

This functionality isn't currently available in circe-yaml, which does resolve the references but returns JSON objects with "<<" keys in them.

I need this functionality, and while I could hack it with cursors or something I'd rather take advantage of the fact that SnakeYAML can do it for me. This seems pretty easy—you can just subclass org.yaml.snakeyaml.constructor.SafeConstructor and call its protected flattenMapping on your mapping nodes.

I'm assuming we don't want this to be the default behavior, so I think we'll want some way to set options either in the parser itself or in the yamlToJson method. I'm happy to take a stab at this as soon as possible but if anyone has ideas about how it should look, please let me know.

/cc @jeremyrsmith @jeffmay @jonas @denisrosset

@jeremyrsmith
Copy link
Collaborator

jeremyrsmith commented Jun 8, 2017

I'm +1 for making this the default behavior, and also +1 on making it the only behavior (in order to avoid having to add configuration cruft)

Edit: I anticipate a 0% chance that anyone using the key << does not intend to use it this way. But if that does happen, we can always add a way to configure it at that point.

@travisbrown
Copy link
Member Author

Okay @jeremyrsmith, I'm convinced—I'll PR this now and then publish it in a quick 0.6.2, and if anyone complains we can revisit in a future release.

This was referenced Jun 8, 2017
jeremyrsmith added a commit that referenced this issue Jun 10, 2017
This approach moves the node handling to an object algebra. This allows
for relatively easily adding configurable stuff that's able to change
how various YAML types are handled as the need arises. One can even plug
in their own algebra if they want to get nitty-gritty.

It also allows adding optional error accumulation, which is something
that's always felt missing.

A nice thing here is by default we don't have to even check any
configuration - only when a parser instance is configured do we change
to `ConfiguredAlg`.

Also should be binary compatible (and `yaml.parser.configure(...)` is a
nice way to get a configured parser!).

TODO: Add tests around error accumulation and such (coverage currently
lacking)
@jeffmay
Copy link
Contributor

jeffmay commented Jun 14, 2017

I agree with @jeremyrsmith as well about making this the default behavior.

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 a pull request may close this issue.

3 participants