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

Fix quoted map key #328

Merged
merged 3 commits into from Dec 2, 2022
Merged

Fix quoted map key #328

merged 3 commits into from Dec 2, 2022

Conversation

goccy
Copy link
Owner

@goccy goccy commented Dec 1, 2022

fix #301

ref #327

Copy link
Contributor

@braydonk braydonk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does look easier, however it looks like this will break on the following valid yaml:

a:
  "b" : 1
  "c": 2

It would be good to add this as a test case, I should have added it in mine.

My proposal in #327 had something to check if the buffer is currently all whitespace, maybe including this would fix it?

@goccy
Copy link
Owner Author

goccy commented Dec 2, 2022

@braydonk Thank you for your review ! You are right, I will try to make a fix that will take care of whitespace.

@codecov-commenter
Copy link

Codecov Report

Merging #328 (6a9ddb5) into master (7b77440) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #328   +/-   ##
=======================================
  Coverage   75.63%   75.63%           
=======================================
  Files          13       13           
  Lines        4359     4359           
=======================================
  Hits         3297     3297           
  Misses        819      819           
  Partials      243      243           

@goccy
Copy link
Owner Author

goccy commented Dec 2, 2022

I will try to make a fix that will take care of whitespace.

DONE.

Copy link
Contributor

@braydonk braydonk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good strategy! Looks good to me.

@goccy goccy merged commit 2cd47e3 into master Dec 2, 2022
@goccy goccy deleted the fix-quoted-map-key branch December 2, 2022 14:58
@goccy
Copy link
Owner Author

goccy commented Dec 2, 2022

@braydonk Thank you for your review !! Merged.

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.

Unmarshalling a list of maps fails with "unexpected key name"
3 participants