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] chomp newlines only at the end of YAML documents#18828
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 commentedMay 21, 2016
| Q | A |
|---|---|
| Branch? | 2.3 |
| Bug fix? | yes |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | #18784 |
| License | MIT |
| Doc PR |
theofidry commentedMay 21, 2016 • 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.
It does fix the issue 👍 |
| return'' !==$ltrimmedLine &&$ltrimmedLine[0] ==='#'; | ||
| } | ||
| privatefunctionisCurrentLineLastLineInDocument() |
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.
Let's "nitpick" a bit: wouldisLastDocumentLine() be a better name forisCurrentLineLastLineInDocument()?
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.
Not sure, we don't pass any argument so it's not really clear to what the method name refers to imo. Note that theisCurrentLine prefix is a common pattern in the parser (e.g.isCurrentLineComment(),isCurrentLineBlank() and so on).
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.
OK then.
javiereguiluz commentedMay 23, 2016
👍 Status: reviewed |
fabpot commentedMay 23, 2016
Thank you@xabbuh. |
…abbuh)This PR was merged into the 2.3 branch.Discussion----------[Yaml] chomp newlines only at the end of YAML documents| Q | A| ------------- | ---| Branch? | 2.3| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#18784| License | MIT| Doc PR |Commits-------a4b1fa6 chomp newlines only at the end of YAML documents