search

kubkon / zig-yaml

YAML parser for Zig
MIT License
139 stars 37 forks source link

[feature]: support parsing comments in documents #5

Closed ssteinbach closed 2 years ago

ssteinbach commented 2 years ago

I'm trying to open a yaml document like:

test_string: "hello!"
# this is a comment
other_thing: 12

If I remove the comment, the parse is successful, but if it is present, the parser errors with an unexpected token. Looking through the code it definitely seems to have references to comments, but I didn't see any in the yaml files in the repo, so I thought I would ask.

The error:

zig-yaml/src/parse.zig:339:21: 0x103042b55 in .yaml.parse.Parser.doc (visualizer)
                    return error.UnexpectedToken;

Thanks!

kubkon commented 2 years ago

Hi @ssteinbach, thanks for submitting an issue! Yes, support for comments is not really there yet. Is that something you'd like to add to the parser perhaps? I'll be more than happy to help you out ofc if you choose to work on this! :-)

ssteinbach commented 2 years ago

Hi @kubkon, I ended up falling back to the built in JSON parser. Unfortunately I don't have bandwidth to take that on at the moment. Sorry about that!

kubkon commented 2 years ago

Not at all! That's also my problem, hahaha. I'll try to commit some more time to this though, and will keep you posted if something interesting develops!

ssteinbach commented 2 years ago

awesome! Will check out the new parser!

kubkon commented 2 years ago

awesome! Will check out the new parser!

Please do, and if you run into bugs (which you most likely will), please do flood me with issues! I will be doing more development on this parser in the coming weeks as I've neglected it long enough :-)

ssteinbach commented 2 years ago

yay definitely works now! Only thing I noticed is that simple.yaml is missing a comment (for testing purposes) if you want to catch this kind of regression in the future. Thanks again for fixing this!

kubkon commented 2 years ago

yay definitely works now! Only thing I noticed is that simple.yaml is missing a comment (for testing purposes) if you want to catch this kind of regression in the future. Thanks again for fixing this!

We test for comments in the main test suite https://github.com/kubkon/zig-yaml/blob/f3e25993d6a1801aa9d098867694a17531f223fe/src/yaml/test.zig#L353 so I don't think there is any value in adding comments into simple.yaml unless I'm missing something?