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

Strip trailing whitespace from all files #1757

Closed
fijha opened this issue Mar 19, 2016 · 5 comments
Closed

Strip trailing whitespace from all files #1757

fijha opened this issue Mar 19, 2016 · 5 comments

Comments

@fijha
Copy link

fijha commented Mar 19, 2016

There is unnecessary trailing whitespace in almost every file… Needs to be cleaned up…

@ryancramerdesign
Copy link
Owner

Whitespace at the end of lines and/or between lines is inconsequential per our style guide. It's the whitespace at the beginning of lines that we care about. See here: http://processwire.com/api/coding-style-guide/#2.3-indentation-tabs-spaces

@fijha fijha changed the title Remove trailing spaces from all files Remove trailing whitespace from all files Apr 5, 2016
@fijha
Copy link
Author

fijha commented Apr 5, 2016

Hi Ryan, sorry, I was thinking about unnecessary whitespace at the and of lines (after semicolon etc.) which is just noise… You can see in the screenshot that it looks like there are a lot of a changes made, but just spaces was removed… also file size can be a little smaller…

screen shot 2016-04-05 at 23 42 16

… or there is added whitespace (1ea08e6)

screen shot 2016-04-05 at 23 58 10

I use TextWrangler on a mac which has an option "Strip trailing whitespace" before save… It would be nice if git has that option too, after commit, or just making sure trailing whitespace was removed before commit so only real useful changes will be committed…

@fijha fijha changed the title Remove trailing whitespace from all files Strip trailing whitespace from all files Apr 5, 2016
@teppokoivula
Copy link

While I agree that consistency is a good thing, even when it comes to small things, I can't say that I see much real value in this. Hopefully I don't sound too rude, but in my opinion going through all files just to remove trailing whitespace would be micro-optimization at best, and for the most part just waste of time.

Additionally I think you might've missed the part of the ProcessWire coding style guide Ryan was pointing to, as it clearly states that whitespace at the end of lines is perfectly fine:

Additional whitespace MAY be used between or at the end of lines (whitespace is disregarded in diffs).

If it's GitHub diff you're worried about, try appending "?w=1" at the end of the URL and it won't display space changes anymore. If you're using git client locally, you can add the -w flag to get the same result there.

@fijha
Copy link
Author

fijha commented Apr 6, 2016

Thanks @teppokoivula, I'm new to git and didn't know about that option… I still can't see the point of having whitespace at the end of lines, but as you mentioned, is just micro-optimization… not a big deal… It wouldn't be hard to clean that up… even trough some PHP script (rtrim() on every line)… I just think it would be nice :), but of course it is not that important.

@ryancramerdesign
Copy link
Owner

Admittedly I find a space at the end of lines helpful in my editing
environment (which is either VIM or PhpStorm running in VIM mode). Though
I'm not suggesting that anyone adopt trailing whitespace as a standard
(which would be silly). But rtrim()'ing lines is just one of those things
that doesn't seem worthwhile at this point in time, especially when it's
actually kind of handy (for me anyway). We've got better things to focus
time on, for now at least. :) I know there are also projects that desire to
maintain no whitespace at the end of lines, which I think is primarily for
the purpose of diffs that don't ignore whitespace. So I understand where
you are coming from .The reasons are of course understandable, but so far
those don't translate to PW. If at some point it surfaces as a
consequential issue we'll definitely reconsider it though.

On Wed, Apr 6, 2016 at 5:43 AM, Jany notifications@github.com wrote:

Thanks @teppokoivula https://github.com/teppokoivula, I'm new to git
and didn't know about that option… I still can't see the point of having
whitespace at the end of lines, but as you mentioned, is just
micro-optimization… not a big deal… It wouldn't be hard to clean that up…
even trough some PHP script (rtrim() on every line)… I just think it
would be nice :), but of course it is not that important.


You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#1757 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants