Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Yaml] parse inlined tags without values#23757
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
xabbuh commentedAug 2, 2017
| Q | A |
|---|---|
| Branch? | master |
| Bug fix? | yes |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | |
| License | MIT |
| Doc PR |
GuilhemN commentedAug 2, 2017
Tag values are always strings as they are not evaluated, I don't think this change makes sense. |
| } | ||
| $tagLength =strcspn($value,"\t\n",$i +1); | ||
| $tagLength =strcspn($value,"\t\n".$stopOn,$i +1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
In fact indicators must always be forbidden in tags handle (seehttp://www.yaml.org/spec/1.2/spec.html#ns-tag-char).
It should be done in 3.3 as a bug fix imo.
xabbuh commentedAug 2, 2017
I didn't find anything in the specs that forbids having a tag without a value. Did I miss anything? |
nicolas-grekas commentedAug 2, 2017
at leasthttp://yaml-online-parser.appspot.com/ forbids it |
GuilhemN commentedAug 2, 2017 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
http://yaml-online-parser.appspot.com/ is buggy then :)
Having a tag without a value is allowed, I'm just saying the value is not Edit: the only thing we should change is forbidding indicators in a tag name. |
nicolas-grekas commentedAug 3, 2017
OK, it's not buggy, it's that a space is required after the tag, and then it resolves to the empty string yes. |
GuilhemN commentedAug 3, 2017
@nicolas-grekas the reference ishttp://ben-kiki.org/ypaste/data/497/index.html :)
|
xabbuh commentedAug 3, 2017 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
tags without values are now getting parsed as empty strings (their values are parsed as empty strings) |
| } | ||
| $tagLength =strcspn($value,"\t\n",$i +1); | ||
| $tagLength =strcspn($value,"\t\n".$stopOn,$i +1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Imo this function should just be updated to:
privatestaticfunction parseTag($value, &$i,$flags) {if ('!' !==$value[$i]) {return; }$tagLength =strcspn($value,"\t\n,[]{}",$i +1);// ...
xabbuh commentedAug 4, 2017
Let's wait here until we have merged#22913 so we can indeed forbid indicator characters in tags (as written in#23757 (comment)). Currently, we cannot do that because we would then break parsing the |
GuilhemN commentedAug 6, 2017
xabbuh commentedAug 7, 2017
rebased and ready |
This PR was merged into the 4.0-dev branch.Discussion----------[Yaml] parse inlined tags without values| Q | A| ------------- | ---| Branch? | master| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets || License | MIT| Doc PR |Commits-------3bb0a3f [Yaml] parse inlined tags without values