8000 [Yaml] Allow comments after tab by superdav42 · Pull Request #15747 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[Yaml] Allow comments after tab #15747

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

Closed
wants to merge 10 commits into from

Conversation

superdav42
Copy link
Contributor
Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets
License MIT

If a yml file has a tab character before a line ending comment the comment will be included in the parsed value. Yaml spec allows tab or space as whitespace characters so we need to check for tab as well. See included test.
Recently caused an odd and hard to find bug in our project.

@@ -209,7 +209,8 @@ public static function parseScalar($scalar, $delimiters = null, $stringDelimiter
$i += strlen($output);

// remove comments
if (false !== $strpos = strpos($output, ' #')) {
if ((false !== $strpos = strpos($output, '#')) &&
($output[$strpos-1] == ' ' || $output[$strpos-1] == "\t")) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be on one line

@superdav42
Copy link
Contributor Author

@fabpot Fixed the code style.

@@ -209,7 +209,7 @@ public static function parseScalar($scalar, $delimiters = null, $stringDelimiter
$i += strlen($output);

// remove comments
if (false !== $strpos = strpos($output, ' #')) {
if ((false !== $strpos = strpos($output, '#')) && ($output[$strpos - 1] == ' ' || $output[$strpos - 1] == "\t")) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you have to use === and the yoda style (' ' === $output[$strpos - 1])

@fabpot
Copy link
Member
fabpot commented Sep 14, 2015

@superdav42 Can you also rebase (instead of merge) to get rid of the merge commits?

@fabpot
Copy link
Member
fabpot commented Sep 14, 2015

Can you also paste the part of the YAML configuration that talks about this? Thanks.

@superdav42
Copy link
Contributor Author

@fabpot Rebasing doesn't seem to work so well after publishing, Created new PR #15793

@stof
Copy link
Member
stof commented Sep 14, 2015

@superdav42 you just need to force the push to your feature branch (as explained in our documentation btw). For branches used for PRs, it is fine

fabpot added a commit that referenced this pull request Oct 7, 2015
…perdav42)

This PR was merged into the 2.3 branch.

Discussion
----------

[Yaml] Allow tabs before comments at the end of a line

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets |
| License       | MIT

If a yml file has a tab character before a line ending comment the comment will be included in the parsed value. Yaml spec allows tab or space as whitespace characters so we need to check for tab as well. See included test.
Recently caused an odd and hard to find bug in our project.

See spec:
http://www.yaml.org/spec/1.2/spec.html#s-b-comment
http://www.yaml.org/spec/1.2/spec.html#s-separate-in-line
http://www.yaml.org/spec/1.2/spec.html#s-white

This is a new PR replacing #15747

@fabpot

Commits
-------

d040be7 [Yaml] Allow tabs before comments at the end of a line
fabpot added a commit to symfony/yaml that referenced this pull request Oct 7, 2015
…perdav42)

This PR was merged into the 2.3 branch.

Discussion
----------

[Yaml] Allow tabs before comments at the end of a line

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets |
| License       | MIT

If a yml file has a tab character before a line ending comment the comment will be included in the parsed value. Yaml spec allows tab or space as whitespace characters so we need to check for tab as well. See included test.
Recently caused an odd and hard to find bug in our project.

See spec:
http://www.yaml.org/spec/1.2/spec.html#s-b-comment
http://www.yaml.org/spec/1.2/spec.html#s-separate-in-line
http://www.yaml.org/spec/1.2/spec.html#s-white

This is a new PR replacing symfony/symfony#15747

@fabpot

Commits
-------

d040be7 [Yaml] Allow tabs before comments at the end of a line
fabpot added a commit to symfony/yaml that referenced this pull request Oct 10, 2015
…perdav42)

This PR was merged into the 2.3 branch.

Discussion
----------

[Yaml] Allow tabs before comments at the end of a line

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets |
| License       | MIT

If a yml file has a tab character before a line ending comment the comment will be included in the parsed value. Yaml spec allows tab or space as whitespace characters so we need to check for tab as well. See included test.
Recently caused an odd and hard to find bug in our project.

See spec:
http://www.yaml.org/spec/1.2/spec.html#s-b-comment
http://www.yaml.org/spec/1.2/spec.html#s-separate-in-line
http://www.yaml.org/spec/1.2/spec.html#s-white

This is a new PR replacing symfony/symfony#15747

@fabpot

Commits
-------

d040be7 [Yaml] Allow tabs before comments at the end of a line
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.

5 participants
0