Skip to content
This repository was archived by the owner on Jul 12, 2020. It is now read-only.

Conversation

@tomphp
Copy link
Contributor

@tomphp tomphp commented Dec 8, 2013

Extract method was messing up the extracted method if the code being extracted was indented more than 4 spaces.

Also the method call to the extracted method was not indented correctly.

This PR fixes both these issues.

Unit tests & acceptance tests are included.

PS: For some reason 2 scenarios are failing the the Fix Class Names feature but there were failing before I started working so I'm assuming that they either are currently broken in the repo or something in my environment?

Copy link
Contributor

Choose a reason for hiding this comment

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

Is the reset necessary? It leaks technical data into the domain.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can replace it with $line[0] if you prefer? I suppose under these circumstances the first index isn't going to be anything other than 0

@tomphp
Copy link
Contributor Author

tomphp commented Dec 9, 2013

@beberlei I have replaced the reset with $lines[0]. On further thinking this could cause a problem if the first line is a blank line, however I think first line is better than minWhitespace() since that's the initial indentation of the block.

In reality I think all the whitespace detection should be refactored into a WhitespaceDetector class, if you are happy for me to do that I'll have a go at that a bit later on.

@tomphp
Copy link
Contributor Author

tomphp commented Jan 4, 2014

Rebased, fixed to work with new patch generation and squashed :)

beberlei added a commit that referenced this pull request Apr 14, 2015
Extract method works properly with code indented more than one level
@beberlei beberlei merged commit 1086616 into QafooLabs:master Apr 14, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants