On 12/10/2011 06:00, Matthew Brush wrote:
Remove extra whitespace at end of lines in all source files.
- Processed with rstrip-whitespace.py script added to scripts/ directory.
- Script run on all .c and .h files in src/ and plugins/ directories.
- Also remove more than one newline at the end of files.
We've mostly done this before, but I'm not sure this is a good idea. It means more maintenance burden on receiving code. What's so bad about trailing whitespace?
On 10/12/2011 04:14 PM, Nick Treleaven wrote:
We've mostly done this before, but I'm not sure this is a good idea. It means more maintenance burden on receiving code. What's so bad about trailing whitespace?
Well, it turns into a problem as soon as people strip it.
Apparently, one can use git hooks (http://progit.org/book/ch7-3.html) to remove trailing whitespace, see http://snipplr.com/view/28523/git-precommit-hook-to-fix-trailing-whitespace/
Take care
Alex Eberspächer
On 11-10-12 07:14 AM, Nick Treleaven wrote:
On 12/10/2011 06:00, Matthew Brush wrote:
Remove extra whitespace at end of lines in all source files.
- Processed with rstrip-whitespace.py script added to scripts/ directory.
- Script run on all .c and .h files in src/ and plugins/ directories.
- Also remove more than one newline at the end of files.
We've mostly done this before, but I'm not sure this is a good idea. It means more maintenance burden on receiving code. What's so bad about trailing whitespace?
The main reasons are to avoid noise in patches, prevent patch corruption when sent over email, and IMO it's just good practice (ie. what people contributing code would expect). If we don't care about this, then why care about spaces vs. tabs, spaces before function parameter braces, indentation, and so on?
Also, I'm not suggesting that we reject code solely because of trailing whitespace, just that it become something the developers do; I would guess that the vast majority of what I removed in that commit was caused by Geany developers themselves.
It takes two seconds to turn the stripping feature on in Geany or Git and it automatically handles the "bug" in Geany's auto-indent where it adds the indentation when you press enter but doesn't remove it when you leave the line blank. This is where the bulk of the trailing whitespace came from. As of for the extra newlines at the end of files, I think those are caused by a "bug" in one of the processing scripts used on Geany's source code (fix-alignment.pl:91 maybe).
If you really feel strongly against this, I honestly don't mind if you revert the commit and we can just forget about it altogeher. FWIW, I did check with Colomban about this, confirming that there shouldn't be trailing whitespace and that it was ok to commit this, before doing so.
Sorry for such a long response to something so trivial :)
Cheers, Matthew Brush
Le 13/10/2011 00:50, Matthew Brush a écrit :
On 11-10-12 07:14 AM, Nick Treleaven wrote:
On 12/10/2011 06:00, Matthew Brush wrote:
Remove extra whitespace at end of lines in all source files.
- Processed with rstrip-whitespace.py script added to scripts/
directory.
- Script run on all .c and .h files in src/ and plugins/ directories.
- Also remove more than one newline at the end of files.
We've mostly done this before, but I'm not sure this is a good idea. It means more maintenance burden on receiving code. What's so bad about trailing whitespace?
The main reasons are to avoid noise in patches, prevent patch corruption when sent over email, and IMO it's just good practice (ie. what people contributing code would expect). If we don't care about this, then why care about spaces vs. tabs, spaces before function parameter braces, indentation, and so on?
Also, I'm not suggesting that we reject code solely because of trailing whitespace, just that it become something the developers do; I would guess that the vast majority of what I removed in that commit was caused by Geany developers themselves.
It takes two seconds to turn the stripping feature on in Geany or Git and it automatically handles the "bug" in Geany's auto-indent where it adds the indentation when you press enter but doesn't remove it when you leave the line blank. This is where the bulk of the trailing whitespace came from. As of for the extra newlines at the end of files, I think those are caused by a "bug" in one of the processing scripts used on Geany's source code (fix-alignment.pl:91 maybe).
If you really feel strongly against this, I honestly don't mind if you revert the commit and we can just forget about it altogeher. FWIW, I did check with Colomban about this, confirming that there shouldn't be trailing whitespace and that it was ok to commit this, before doing so.
Actually I don't mind much whether we leave or not whitespaces (in most of my personal projects I keep them everywhere BTW), but since it was Geany's coding style 'til now, I'd say we should stick to it. This isn't so important IMHO, but I also find it better to have it consistent over files -- though it's not as important as most indentation rules.
Ah, and I doubt this would cause much trouble with incoming patches. (I think) many people already strips spaces, and those who submit patches are likely to have had a shot at HACKING. And finally, we can either choose not to mind if there are some, or we could quite easily drop them anyway.
So to conclude, basically I'd say we should stick to the rule, but I don't mind if we decide not to bother about it [1]. I just don't want to see those ugly commits that removes tons of whitespaces additionally to the real change. So... just do as you want.
Regards, Colomban
[1] And it's just whitespaces after all, we probably better concentrate on the real code, not need to check pro and cons forever I think.
On 12/10/2011 23:50, Matthew Brush wrote:
On 11-10-12 07:14 AM, Nick Treleaven wrote:
On 12/10/2011 06:00, Matthew Brush wrote:
Remove extra whitespace at end of lines in all source files.
- Processed with rstrip-whitespace.py script added to scripts/
directory.
- Script run on all .c and .h files in src/ and plugins/ directories.
- Also remove more than one newline at the end of files.
We've mostly done this before, but I'm not sure this is a good idea. It means more maintenance burden on receiving code. What's so bad about trailing whitespace?
The main reasons are to avoid noise in patches, prevent patch corruption
Unfortunately it can cause noise also.
when sent over email, and IMO it's just good practice (ie. what people contributing code would expect). If we don't care about this, then why care about spaces vs. tabs, spaces before function parameter braces, indentation, and so on?
Those things affect readability.
Also, I'm not suggesting that we reject code solely because of trailing whitespace, just that it become something the developers do; I would guess that the vast majority of what I removed in that commit was caused by Geany developers themselves.
It takes two seconds to turn the stripping feature on in Geany or Git
How do I turn it on for Git?
The stripping feature in Geany should be made a project preference, I don't want to cause noise on non-Geany files.
and it automatically handles the "bug" in Geany's auto-indent where it adds the indentation when you press enter but doesn't remove it when you leave the line blank. This is where the bulk of the trailing whitespace came from. As of for the extra newlines at the end of files, I think those are caused by a "bug" in one of the processing scripts used on Geany's source code (fix-alignment.pl:91 maybe).
The extra newlines were probably added by my function snippet that includes 2 trailing newlines.
If you really feel strongly against this, I honestly don't mind if you revert the commit and we can just forget about it altogeher. FWIW, I did check with Colomban about this, confirming that there shouldn't be trailing whitespace and that it was ok to commit this, before doing so.
Sorry for such a long response to something so trivial :)
I just thought I'd raise it. If we have Git support for this then it's not much of an issue.
On 10/13/2011 04:42 AM, Nick Treleaven wrote:
The stripping feature in Geany should be made a project preference, I don't want to cause noise on non-Geany files.
This is actually on my TODO list, unless anyone feels like doing it before me :)
Cheers, Matthew Brush
On 13/10/2011 22:24, Matthew Brush wrote:
On 10/13/2011 04:42 AM, Nick Treleaven wrote:
The stripping feature in Geany should be made a project preference, I don't want to cause noise on non-Geany files.
This is actually on my TODO list, unless anyone feels like doing it before me :)
Great. BTW the long line marker project setting has an option 'Use global settings'. I don't think we should repeat this for all new settings as it doesn't scale well. Perhaps there could be one 'Override global settings' per tab, but this isn't important.
Also, presumably we should avoid making any glade changes on master until after merging the gtkbuilder branch?