Skip to content

[Yaml] look for colon in parsed inline string #16745

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

Merged
merged 1 commit into from
Nov 29, 2015
Merged

Conversation

xabbuh
Copy link
Member

@xabbuh xabbuh commented Nov 29, 2015

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #16562
License MIT
Doc PR

Looking for a colon in an unquoted mapping value can lead to falsely
reported parse errors (e.g. when a comment after the mapping value
contains a colon).

Looking for a colon in an unquoted mapping value can lead to falsely
reported parse errors (e.g. when a comment after the mapping value
contains a colon).
@peterrehm
Copy link
Contributor

I stumbled about those errors, I think it would be helpful to mention the file, otherwise you just know the line number and not even the value. Upgrading it is not very easy.

@fabpot
Copy link
Member

fabpot commented Nov 29, 2015

@peterrehm Adding the file in exceptions is done by the main Yaml class, which is the only one to know the file. The Parser class only deals with strings. So, adding the file to the deprecation notices would be very difficult.

@fabpot
Copy link
Member

fabpot commented Nov 29, 2015

Thank you @xabbuh.

@peterrehm
Copy link
Contributor

@fabpot To upgrade locally I added the value to the message and then I knew where to upgrade, otherwise it was just impossible for me to find out what to upgrade. Mabe this is an option.

@fabpot fabpot merged commit 2127058 into symfony:2.8 Nov 29, 2015
fabpot added a commit that referenced this pull request Nov 29, 2015
This PR was merged into the 2.8 branch.

Discussion
----------

[Yaml] look for colon in parsed inline string

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

Looking for a colon in an unquoted mapping value can lead to falsely
reported parse errors (e.g. when a comment after the mapping value
contains a colon).

Commits
-------

2127058 [Yaml] look for colon in parsed inline string
@peterrehm
Copy link
Contributor

I submitted a PR for the improvements, let me know your thoughts.

@xabbuh xabbuh deleted the issue-16562 branch November 29, 2015 21:41
This was referenced Nov 30, 2015
@ghost ghost mentioned this pull request Dec 1, 2015
stof added a commit to stof/symfony that referenced this pull request Dec 1, 2015
stof added a commit that referenced this pull request Dec 2, 2015
This PR was merged into the 3.0 branch.

Discussion
----------

Reapply the Yaml bugfix of #16745

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #16784
| License       | MIT
| Doc PR        | n/a

The fix done in #16745 was missed when resolving conflicts during the merge to 3.0

Commits
-------

d9393d8 Reapply the Yaml bugfix of #16745
stof added a commit that referenced this pull request Dec 5, 2015
* 3.0:
  Set the redraw frequency at least to 1. Setting it to 0 would otherwise produce an error.
  [Form] document changes to form type interfaces
  [Process] Unset callback after stop to free memory
  Improve error message for undefined DIC aliases
  [VarDumper] fixed .sf-dump z-index
  [Validator] Updated Luxembourgish translations for 2.8
  Disallow http-kernel 3.x, fixes #16837.
  [Config] Throw an exception when using cannotBeEmpty() on numeric or boolean nodes
  [DependencyInjection] Validate class names and factory methods
  ampq → amqp
  Fix typo
  Refactoring EntityUserProvider::__construct() to not do work, cause cache warm error
  [Form] Add context to FormFactory deprecations
  Reapply the Yaml bugfix of #16745
  CS: remove unneeded parentheses around control statements
  [DomCrawler] add upgrade hint on interface changes
  [TwigBridge] Clean deps now that 2.8.0 is tagged
stof added a commit that referenced this pull request Dec 5, 2015
This PR was merged into the 2.8 branch.

Discussion
----------

[DX][YAML] Improved Deprecation Notices

Show the actual invalid string for the Yaml deprecations to ease the upgrade as you can grep the strings.

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

Related #16745

Commits
-------

3179407 Improved error messages for Yaml Deprecations
@fabpot fabpot mentioned this pull request Dec 26, 2015
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.

4 participants