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] fix parsing multi-line mapping values#19304
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 commentedJul 6, 2016
| Q | A |
|---|---|
| Branch? | 2.7 |
| Bug fix? | yes |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | #11109 |
| License | MIT |
| Doc PR |
fabpot commentedJul 8, 2016
The question is whether we want to support this. Having a string on multiple lines is already supported via '|' for instance, so do we really need another way? Wouldn't it be good to re-read the spec and write a doc that explicitly list what we do support and what we don't want to support? |
xabbuh commentedJul 9, 2016
I could also live with not fixing this issue. Though in that case I suggest to provide a more meaningful exception message (something like what I suggested in#11911). |
xabbuh commentedJul 15, 2016
ping @symfony/deciders Any opinion on this? |
Tobion commentedJul 15, 2016
If that is according to the yaml spec, we should support it as well. We removed support for other stuff like non-quoted strings starting with |
fabpot commentedJul 18, 2016
We don't want to support the full YAML spec but a subset of it. So, removing things that were not compatible with the YAML spec was needed but this is different from supporting features that are currently not supported. |
javiereguiluz commentedJul 25, 2016
👍 because this YAML feature is not that "edgy" and because the implementation is reasonably small and simple. Status: reviewed |
fabpot commentedAug 15, 2016
ok, let's do it. But that's a new feature then, which should be done on master.@xabbuh I let you merge this on master (there are some conflicts). |
smarcet commentedSep 19, 2016
is there is any update of this fix ? |
…inkine)This PR was merged into the 3.4 branch.Discussion----------[Yaml] Fix escaped quotes in quoted multi-line string| Q | A| ------------- | ---| Branch? | 3.4| Bug fix? | yes| New feature? | no| Deprecations? | no| License | MITThis PR continues#19304This PR fixes incorrect parsing quoted multi-line string which contain escaped quotes, see testsCommits-------2e99caa [Yaml] Fix escaped quotes in quoted multi-line string
…inkine)This PR was merged into the 3.4 branch.Discussion----------[Yaml] Fix escaped quotes in quoted multi-line string| Q | A| ------------- | ---| Branch? | 3.4| Bug fix? | yes| New feature? | no| Deprecations? | no| License | MITThis PR continuessymfony/symfony#19304This PR fixes incorrect parsing quoted multi-line string which contain escaped quotes, see testsCommits-------2e99caacaf [Yaml] Fix escaped quotes in quoted multi-line string