Fixed Parser problem for SELECT (((3))) as ....#448
Merged
guilhermeblanco merged 4 commits intodoctrine:masterfrom Nov 3, 2012
Merged
Fixed Parser problem for SELECT (((3))) as ....#448guilhermeblanco merged 4 commits intodoctrine:masterfrom
guilhermeblanco merged 4 commits intodoctrine:masterfrom
Conversation
lib/Doctrine/ORM/Query/Parser.php
Outdated
Member
There was a problem hiding this comment.
break is useless here. It is a dead statement as you return before it
Contributor
Author
|
Hi, no problem to clean up the things you mentioned. I'd also like to add some comments like in SelectExpression. But is the general style ok? |
Member
|
It is |
Contributor
Author
|
I think it done now. I removed the check for SELECT in _IsFunction. The unittests pass, and I believe it's cleaner to check for aggregates outside of _isFunction (as it is done already). I'm on vacation for 2 weeks now, so I won't be able to comment. Cheers Stefan |
Member
|
Can you rebase this PR? fabio changed the CS of the classes and made this unmergable. Sorry for the invonenience. |
_isFunction doesn't exclude subselects anymore
Contributor
Author
|
I rebased the PR to the current master. Regards Stefan |
guilhermeblanco
added a commit
that referenced
this pull request
Nov 3, 2012
Fixed Parser problem for SELECT (((3))) as ....
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hi this is the old PR #438 ported to master.
I had to modify more than I expected to get it to work properly.
I had to apply some modifications to Parser::ScalarExpression and would like to discuss the desired coding style. After my modifications Parser::ScalarExpression looks a bit more like Parser::SelectExpression, but I don't know your preferred style.
I think there is some more cleanup we could apply to ScalarExpression, but I'd like to get some feedback first.
Stefan