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

Merge keys appear not to be supported #168

Open
martingkelly opened this issue Mar 17, 2020 · 11 comments
Open

Merge keys appear not to be supported #168

martingkelly opened this issue Mar 17, 2020 · 11 comments

Comments

@martingkelly
Copy link

I found this in my own project when attempting to use merge keys ("<<:", https://yaml.org/type/merge.html). Specifically, the key gets parsed literally as a scalar with value "<<". To see this, try the following:

tests/example-deconstructor < examples/anchors.yaml

You will see a node like this in the output:

  - type: SCALAR
    value: "<<"
    implicit: {plain: true, non-plain: false}
    style: plain
@i-ky
Copy link

i-ky commented Mar 17, 2020

Apparently this "Working Draft" did not make it into "Final Draft" YAML 1.1 specification. There are no mentions of "<<" or "merge".

@martingkelly
Copy link
Author

Yes, I noticed that. It seems to be supported in many other tools (e.g. pyyaml and most online YAML validators), thus more of a de-facto than official standard.

If we decide not to support this for now, I think it would be good to at least:

  • Change or remove the example in the repository, which suggests that it is supported.
  • Throw an error when someone uses "<<" as a key value, as this YAML won't work as intended (it will appear to succeed but not parse correctly). This would break some users, but such users were already broken.

@perlpunk
Copy link
Member

perlpunk commented Mar 17, 2020

For a parser, the scalar << is just a scalar node like foo or bar.
Only the composer/constructor resolves scalars and creates booleans, numbers, null, and, if implemented/enabled, merge keys.

At the parsing level nothing special should be done with a scalar <<.

Btw, as you mentioned pyyaml - yes, pyyaml supports merge keys, and even when it's used with libyaml as the backend.

Since libyaml doesn't resolve boolans, numbers etc. as it only does the parsing, there is nothing to implement.

@martingkelly
Copy link
Author

I see. So to clarify, it's intended that C code that wants to use merge keys needs to implement that itself? This seems pretty non-trivial to implement on top of libyaml. Do you have an example of how this work? It seems like a lot of code. Is there a higher-level C library on top of libyaml that I could use to implement without writing a lot of custom code?

@perlpunk
Copy link
Member

A higher level library like pyyaml processes merge keys when it already has constructed a partial data structure.
With a merge key you usually merge an existing alias. This is not trivial by itself if you look at the spec for merge keys, but the given example covers all use cases I believe.

So if you want to add this on top of libyaml, you need to have the previous anchors stored in a mapping-like data structure, and additionally the currently processed mapping has to be constructed already before you can add the data from the merge keys.

You could try out https://github.com/pantoniou/libfyaml, its documentation says it supports merge keys on request.

@i-ky
Copy link

i-ky commented Mar 17, 2020

I don't think it will be too much code... If you are interested I can probably provide an example.

@martingkelly
Copy link
Author

Yes, an example would be helpful.

I'm a little confused about something: anchors seem to be working just fine. For instance, the following parses without issue:

a: &node_a
  a
b: *node_a

b will get the same value as a after parsing with libyaml, with no extra work necessary. I had expected this is how merge keys with <<: would be treated, as they're related to anchors. Are anchors part of parsing but merge keys are not part of parsing?

I guess I'm trying to understand if this is a question of "we haven't implemented this but would like to", or is it "we plan never to implement this, by design."

@perlpunk
Copy link
Member

An alias is part of the syntax. Aliases will be parsed as an alias_event. The value, however, can not be resolved until the data is constructed.
How do you process the YAML? If you just use the parsing event API from libyaml, then you shouldn't get a scalar event, but an alias event.
Or are you using the document API? When constructing the document, an alias will be loaded as a pointer, so you would see the same value.

It's hard to help if we don't know what you are actually doing.

@martingkelly
Copy link
Author

I'm using document parsing. This is the code if you want to look: https://github.com/XevoInc/yobd/blob/master/src/parser.c

@perlpunk
Copy link
Member

perlpunk commented Apr 14, 2020

@martingkelly ok, like I said, resolving of booleans and other types does not happen in the document loader, so also merge keys are simple scalar nodes like true, false, 23 or something. I also don't see such code in your link.

I'm not saying it would be impossible to implement that in the document loader. But post processing would probably a better idea than putting it directly into the existing loader code.

@martingkelly
Copy link
Author

In my link, I parse directly into my own data structures; I don't do intermediate processing or translation of the YAML. e.g. in https://github.com/XevoInc/yobd/blob/master/src/parser.c#L320, we convert a scalar to a number and stick it into a data structure.

It sounds like this issue can be resolved as "by design", as libyaml is intended to do only parsing and not composing/construction, and alias resolution is considered part of composing/construction. Is this correct?

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

No branches or pull requests

3 participants