Skip to content

ReturnStatements - rewrite to use Tokens and fix for #167, #341, #487#488

Merged
keradus merged 1 commit intoPHP-CS-Fixer:masterfrom
keradus:return-on-tokens
Aug 31, 2014
Merged

ReturnStatements - rewrite to use Tokens and fix for #167, #341, #487#488
keradus merged 1 commit intoPHP-CS-Fixer:masterfrom
keradus:return-on-tokens

Conversation

@keradus
Copy link
Copy Markdown
Member

@keradus keradus commented Aug 28, 2014

ReturnStatements - rewrite to use Tokens and fix for #167, #341, #487

@keradus
Copy link
Copy Markdown
Member Author

keradus commented Aug 28, 2014

Because all fixers deserve to use Tokens ;)

@keradus keradus changed the title ReturnStatements - rewrite to use Tokens and fix #487 [bug] ReturnStatements - rewrite to use Tokens and fix #341, #487 Aug 28, 2014
@stof
Copy link
Copy Markdown
Contributor

stof commented Aug 28, 2014

@keradus when mentionning the fixed issues, it is better to put them in the PR description rather than the title. This way, they would get linked automatically

@keradus
Copy link
Copy Markdown
Member Author

keradus commented Aug 28, 2014

Actually they were linked by commit ;)
But very well, desc filled

@stof
Copy link
Copy Markdown
Contributor

stof commented Aug 28, 2014

Can you add a testcase for https://github.com/fabpot/PHP-CS-Fixer/issues/167 as well ?

@keradus
Copy link
Copy Markdown
Member Author

keradus commented Aug 28, 2014

yes

@keradus keradus changed the title [bug] ReturnStatements - rewrite to use Tokens and fix #341, #487 [bug] ReturnStatements - rewrite to use Tokens and fix for #167, #341, #487 Aug 28, 2014
@keradus keradus changed the title [bug] ReturnStatements - rewrite to use Tokens and fix for #167, #341, #487 ReturnStatements - rewrite to use Tokens and fix for #167, #341, #487 Aug 30, 2014
@keradus keradus mentioned this pull request Aug 30, 2014
@keradus
Copy link
Copy Markdown
Member Author

keradus commented Aug 31, 2014

any opinion ?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

shouldn't you use a backward iteration to avoid the need of bumping the limit ?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Backward iteration help not bumping the limit when you add a Token at index greater than current iteration index.

Here we will add a Token at lesser index, one we do not meet yet, so we wiil need to increase limit as well. No gain.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

hmm, right

@stof
Copy link
Copy Markdown
Contributor

stof commented Aug 31, 2014

👍

@keradus
Copy link
Copy Markdown
Member Author

keradus commented Aug 31, 2014

merged

@keradus keradus merged commit 8897fce into PHP-CS-Fixer:master Aug 31, 2014
keradus added a commit that referenced this pull request Aug 31, 2014
, #487 (keradus)

This PR was merged into the 1.0.x-dev branch.

Discussion
----------

ReturnStatements - rewrite to use Tokens and fix for #167, #341, #487

ReturnStatements - rewrite to use Tokens and fix for #167, #341, #487

Commits
-------

8897fce ReturnStatements - rewrite to use Tokens and fix for #167, #341, #487
@keradus keradus deleted the return-on-tokens branch August 31, 2014 13:38
keradus added a commit that referenced this pull request Sep 9, 2014
This PR was squashed before being merged into the 1.0.x-dev branch (closes #472).

Discussion
----------

BracesFixer

Braces for structure blocks, fix #98

- [x] add missing braces
- [x] correct single-level intend
- [X] correct multi-level intend
- [X] fix `T_FOR` token syntax
- [X] ignore T_END* syntax
- [X] make Travis pass
- [X] fix #168 -> fixed in PR #488
- [X] fix #374
- [X] remove new line between `}` and T_ELSE, E_ELSEIF, T_CATCH
- [X] detect duplicated functionality in CurlyBracketsNewlineFixer
- [X] detect duplicated functionality in ControlSpacesFixer

Commits
-------

5f3a1fd BracesFixer
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants