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

[Yaml] fix parse error when unindented collections contain a comment #36683

Merged
merged 1 commit into from May 4, 2020

Conversation

wdiesveld
Copy link
Contributor

Q A
Branch? 5.0
Bug fix? yes
New feature? no
Deprecations? no
Tickets Fix #36558
License MIT

Problem

The method Parser::getNextEmbedBlock did not determine the yaml-block correctly when there was a comment before the first unindented collection item. This was caused by the fact that the check for unindented collection items was done for the first line of the block only. So in case this first line is a comment, this check will result in false, while in fact the parser is in an unindented collection.

Solution

In the solution I implemented the parser will check for comment lines as well. As long as the loop encounters a comment line, it will check (in the next iteration) whether the line is an unindented collection item. So this check will be done until all comments before the first uncommented item are parsed.

@wdiesveld wdiesveld requested a review from xabbuh as a code owner May 4, 2020 10:15
@wdiesveld wdiesveld changed the title Ticket 36558 Fix for Ticket 36558 May 4, 2020
@xabbuh xabbuh added the Yaml label May 4, 2020
@xabbuh xabbuh added this to the 3.4 milestone May 4, 2020
@xabbuh
Copy link
Member

xabbuh commented May 4, 2020

For 3.4?

@wdiesveld
Copy link
Contributor Author

@xabbuh - I could not test it for 3.4 and 4.0, only for 5.0 - I get an out-of-mem error when running composer update for 3.4 & 4.0 (Windows 10). So the PR is for 5.0

@nicolas-grekas nicolas-grekas changed the title Fix for Ticket 36558 [Yaml] fix parse error when unindented collections contain a comment May 4, 2020
@xabbuh xabbuh changed the base branch from 5.0 to 3.4 May 4, 2020 11:18
@xabbuh
Copy link
Member

xabbuh commented May 4, 2020

@wdiesveld No problem, I have changed the base branch and rebased your changes. Thank you for working on this issue.

@fabpot
Copy link
Member

fabpot commented May 4, 2020

Thank you @wdiesveld.

@fabpot fabpot merged commit bb77914 into symfony:3.4 May 4, 2020
@Nyholm
Copy link
Member

Nyholm commented May 4, 2020

Awesome. Congratulations to your first contribution!

This was referenced May 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants