Skip to content

Conversation

msteffen
Copy link
Contributor

@msteffen msteffen commented Mar 4, 2021

Hello @goccy, I found your yaml parser as an alternative to go-yaml, and it seems to handle several cases that go-yaml/yaml doesn't: not just anchors and aliases, but also repeated flow documents ({ ... }{ ... }) and tabs in flow documents, which I think would allow us to replace our use of encoding/json with it.

I created this pull request to add one piece of error handling, for unterminated flow documents—please let me know what you think, or if there's more I can or should do to improve it. Also please let me know if other pull requests would be welcome. There are two other changes I'd be interested in making, if you're open to them:

  • Error on unterminated strings ("abc...<EOF>)
  • Acceptance tests for repeated flow documents and tabs in flow documents (an important use case for us)

I'd be happy to get in touch if you want to discuss these further.

Thanks for creating such a great library!

@codecov-io
Copy link

codecov-io commented Mar 4, 2021

Codecov Report

Merging #213 (f6baeac) into master (2ca66a4) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #213   +/-   ##
=======================================
  Coverage   76.71%   76.71%           
=======================================
  Files          12       12           
  Lines        2946     2946           
=======================================
  Hits         2260     2260           
  Misses        461      461           
  Partials      225      225           

@goccy
Copy link
Owner

goccy commented Mar 13, 2021

Sorry for the late reply.
Thank you for your great PR !

LGTM :) I'll merge it .

@goccy goccy merged commit 8a30050 into goccy:master Mar 13, 2021
@goccy
Copy link
Owner

goccy commented Mar 13, 2021

I also currently working on fixing a unterminated string issue .

Thanks !

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.

3 participants