nickel icon indicating copy to clipboard operation
nickel copied to clipboard

Evaluate yaml-rust2

Open jneem opened this issue 1 year ago • 3 comments

I had a go at adding positions to yaml, using the yaml-rust2 crate. It sort of works (without any error handling yet), but I also have some concerns:

  • the rust yaml ecosystem is kind of in flux right now. yaml-rust2 seems like the most active yaml library at the moment, but for how long?
  • only the low-level API supports positions, so I had to implement my own MarkedEventReceiver to construct the objects from the parser events. There's a marked_yaml crate, but it's pretty new and not much used yet.
  • the marks aren't always in quite the right position. See here; also, I haven't figured out how to easily and correctly find the position of a string unless it's in the "plain" style.

One alternative, as we already discussed, would be to wrap a mature C library.

jneem avatar Jul 18 '24 08:07 jneem

Argh, we should have synced before. In fact I asked about this feature on an issue thread (https://github.com/Ethiraric/yaml-rust2/issues/27), and this was implemented in https://github.com/saphyr-rs/saphyr, which is from the same author but supposed to get more innovations (and potentially breaking changes) while yaml-rust2 would just get maintenance. So I think something very similar to what you implemented can already be done with saphyr, I just haven't had the time to look into it.

At the end of the corresponding PR, the author mentions that indeed the markers sent by their parser are off (see https://github.com/saphyr-rs/saphyr/pull/6#issuecomment-2208620304), and it's a bug of the parser.

We can either use saphyr already, if the imprecision is small, it's already better than what existed before. Or we can even contribute upstream to fix the parser; I suppose the saphyr maintainer now already knows about us and would be eager to merge this change, given they were eager to merge the marker change.

yannham avatar Jul 18 '24 08:07 yannham

No worries, I would have checked first but I found myself with some unexpected free time today...

I will have a look at saphyr and see if I can figure out the position bug.

jneem avatar Jul 18 '24 14:07 jneem

I had a quick look at libfyaml today. Their scanner already has support for recording the end of a token (which is basically the thing I'm trying to add to saphyr-parser). But their high-level API doesn't keep track of spans, so in addition to wrapping the C API we'd need to write the thing that consumes events and produces a tree (i.e. basically what's in this PR).

jneem avatar Jul 29 '24 05:07 jneem

Closing in favor of #2332

jneem avatar Aug 30 '25 03:08 jneem