[YAMLParser] Improve plain scalar spec compliance #68946
Conversation
The `YAMLParser.h` header file claims support for YAML 1.2 with a few deviations, but our plain scalar parsing failed to parse some valid YAML according to the spec. This change puts us more in compliance with the YAML spec, now letting us parse plain scalars containing additional special characters in cases where they are not ambiguous.
akirchhoff-modular
force-pushed
the
akirchhoff/yaml-parser-plain-scalar-fixes
branch
from
4b8c1aa to
d99c12b
Compare
|
This LGTM, but I'll let others more familiar with the code to accept it. |
There was a problem hiding this comment.
Thank you for the patch!
I am sorry for the delay in review, I was out most of last week.
The change looks good overall, but there are a couple test changes/additions I'd like, and I am a bit confused around some of the more subtle conditions and hoped you could just help me understand.
There was a problem hiding this comment.
Thanks @slinder1 for your detailed feedback. I have made some changes in response to your feedback, and also provided responses to your questions below. Please take another look when you can. Thanks!
|
@akirchhoff-modular as discussed offline, I'll merge this on your behalf. Thanks for the thorough review, @slinder1! |
The
YAMLParser.hheader file claims support for YAML 1.2 with a few deviations, but our plain scalar parsing failed to parse some valid YAML according to the spec. This change puts us more in compliance with the YAML spec, now letting us parse plain scalars containing additional special characters in cases where they are not ambiguous.