Skip to content
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

function join() and list($content, ) should not be check... #930

Closed
samsonasik opened this issue Dec 28, 2014 · 9 comments
Closed

function join() and list($content, ) should not be check... #930

samsonasik opened this issue Dec 28, 2014 · 9 comments
Labels

Comments

@samsonasik
Copy link

I think when using 'querybuilder' like :

public function join()
{
   // ...
}

the join() function should not be check and changed to implode. see my comment at : zendframework/zendframework#7079 (diff)
and about :

list($content, ) = ...

should not be check too, see my comment at zendframework/zendframework#7079 (diff) that space not removed, then "parenthesis" cs, but when it removed, it makes another error : "method_argument_space".

@GrahamCampbell
Copy link
Contributor

This is fixed already.

@GrahamCampbell
Copy link
Contributor

830e64a

@GrahamCampbell
Copy link
Contributor

Will be included in the 1.4 release.

@stof
Copy link
Contributor

stof commented Dec 31, 2014

@GrahamCampbell @keradus please close this issue as it is already fixed

@GrahamCampbell
Copy link
Contributor

Yeh, I would have closed it, but I'm not a collaborator. :P

@stof
Copy link
Contributor

stof commented Dec 31, 2014

oh, I thought you were

@samsonasik
Copy link
Author

i'm closing it. thanks ;)

@keradus
Copy link
Member

keradus commented Jan 3, 2015

Yeah, that is why issue reports shouldn't be closed so willingly.
This issue contains in facts 2 issues. First one is fixed already, but not the second one.

Please see solution at #952

//cc @samsonasik @GrahamCampbell @stof

@samsonasik
Copy link
Author

@keradus thank you ;)

keradus added a commit that referenced this issue Jan 4, 2015
…ma (keradus)

This PR was merged into the 2.0-dev branch.

Discussion
----------

ParenthesisFixer - fix case with list call with trailing comma

Fix #930
Note: Should be merged into 1.4 branch as setted in milestone, not master as setted in PR.

Consider following code:
`<?php list($path, $mode,) = foo();`

If we run MethodArgumentSpaceFixer on it, it will add a space after comma:
`<?php list($path, $mode, ) = foo();`

Then if we run ParenthesisFixer it will remove that space:
`<?php list($path, $mode,) = foo();`

This PR change behavior of ParenthesisFixer - stop removing space if there is trailing comma.
Also added a tests for it in both fixers.

Commits
-------

b02b097 ParenthesisFixer - fix case with list call with trailing comma
keradus added a commit that referenced this issue Jan 4, 2015
…ma (keradus)

This PR was merged into the 1.4 branch.

Discussion
----------

ParenthesisFixer - fix case with list call with trailing comma

Fix #930
Clone #952

Consider following code:
`<?php list($path, $mode,) = foo();`

If we run MethodArgumentSpaceFixer on it, it will add a space after comma:
`<?php list($path, $mode, ) = foo();`

Then if we run ParenthesisFixer it will remove that space:
`<?php list($path, $mode,) = foo();`

This PR change behavior of ParenthesisFixer - stop removing space if there is trailing comma.
Also added a tests for it in both fixers.

Commits
-------

0db8e07 ParenthesisFixer - fix case with list call with trailing comma
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants