Skip to content

[ExpressionLanguage] fixed a BC break #20015

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Sep 21, 2016
Merged

[ExpressionLanguage] fixed a BC break #20015

merged 1 commit into from
Sep 21, 2016

Conversation

fabpot
Copy link
Member

@fabpot fabpot commented Sep 21, 2016

Q A
Branch? master
Bug fix? yes
New feature? no
BC breaks? fixes a BC break :)
Deprecations? no
Tests pass? yes
Fixed tickets see #19060 (comment)
License MIT
Doc PR n/a

@fabpot
Copy link
Member Author

fabpot commented Sep 21, 2016

ping @nicolas-grekas @lyrixx

@nicolas-grekas
Copy link
Member

For reference, this was done to ease with dumping AST. With NameNode, $foo->foo() is dumped as is.
With this change, it will be dumped as $foo->"foo"().
But BC is of paramount importance, so 👍
Maybe adjust Node::dump() to deal with this case thought?

@fabpot
Copy link
Member Author

fabpot commented Sep 21, 2016

@nicolas-grekas Indeed, tweaking dump() to take care of that would be ideal. Can you prepare a patch?

@nicolas-grekas
Copy link
Member

Would it be OK to add a new node type that extends ConstantNode, to be used in this case? Because unescaping strings while dumping has some code smell to me.

@fabpot
Copy link
Member Author

fabpot commented Sep 21, 2016

Not sure :)

@fabpot fabpot merged commit b00930f into symfony:master Sep 21, 2016
fabpot added a commit that referenced this pull request Sep 21, 2016
@fabpot
bug #20015 [ExpressionLanguage] fixed a BC break (fabpot)
287f7c8 
This PR was merged into the 3.2-dev branch.

Discussion
----------

[ExpressionLanguage] fixed a BC break

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | fixes a BC break :)
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | see #19060 (comment)
| License       | MIT
| Doc PR        | n/a

Commits
-------

b00930f [ExpressionLanguage] fixed a BC break
@fabpot fabpot deleted the fix-bc-break branch September 28, 2016 14:38
@fabpot fabpot mentioned this pull request Oct 27, 2016
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Bug ExpressionLanguage Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@fabpot @nicolas-grekas @carsonbot