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

LinefeedFixer - Fix in a safe way #2209

Merged
merged 1 commit into from Oct 15, 2016
Merged

LinefeedFixer - Fix in a safe way #2209

merged 1 commit into from Oct 15, 2016

Conversation

SpacePossum
Copy link
Contributor

The fixer should not touch all line breaks.

@keradus
Copy link
Member

keradus commented Sep 27, 2016

then we need a second fixer (risky one) that will fix all the rest...

@SpacePossum
Copy link
Contributor Author

then we need a second fixer (risky one) that will fix all the rest...

I think this is a bug fix, so do not need such a fixer for BC reasons. Do you think someone really wants it?

@gharlan
Copy link
Contributor

gharlan commented Sep 27, 2016

Do you think someone really wants it?

I know at least one: me!

@keradus
Copy link
Member

keradus commented Sep 27, 2016

me too .actually most of community. the only thing is that it's risky.

we have exactly the same problem with trailing spaces. PSR requires to not have them in non-empty lines - even if it's part of string var. but then it modifies a content, so it's risky. but still, most of us wanna follow that.

@keradus
Copy link
Member

keradus commented Sep 27, 2016

It's bugfix but also shrinking functionality. Now one cannot fix linefeed inside var content. could you prepare the new fixer as well please? it's really important for us ! :)

@SpacePossum
Copy link
Contributor Author

alrighty, would a config flag work as well or you really want a diff. fixer?

@keradus
Copy link
Member

keradus commented Sep 27, 2016

I was thinking about the config as well, but then you need to dynamically say if the fixer is risky or not depends on config.

@SpacePossum
Copy link
Contributor Author

I'm kind of happy with this PR as is, but if we need another fixer please provide names for both fixers and I'll update this PR :)

@keradus
Copy link
Member

keradus commented Sep 28, 2016

as discussed it could be an option. the only thing to keep in mind is risky vs not risky... depends on used options

@keradus
Copy link
Member

keradus commented Oct 15, 2016

OK, I treat it as bugfix after thinking about it. Missing functionality is raised here: #2224

@keradus keradus added this to the v1.12.3 milestone Oct 15, 2016
@keradus
Copy link
Member

keradus commented Oct 15, 2016

👍

@keradus
Copy link
Member

keradus commented Oct 15, 2016

Thank you @SpacePossum.

@keradus keradus merged commit 4c2da37 into PHP-CS-Fixer:1.12 Oct 15, 2016
keradus added a commit that referenced this pull request Oct 15, 2016
This PR was merged into the 1.12 branch.

Discussion
----------

LinefeedFixer - Fix in a safe way

The fixer should not touch all line breaks.

Commits
-------

4c2da37 Make LinefeedFixer a safe fixer.
@keradus keradus deleted the 1_12_LinefeedFixer_safe branch October 15, 2016 19:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants