On Sat, Mar 7, 2009 at 5:56 AM, Santiago Gala <[email protected]> wrote:
> El vie, 06-03-2009 a las 08:13 -0800, Adam Winer escribió:
>> What is the advantage of removing trailing whitespace?  I can't see a
>> reason to do it at all.  Unlike removing unnecessary imports (say), it
>> doesn't improve the codebase in any objective manner.
>>
>
> The typical reason for removing whitespace is that whitespace changes
> cause puzzling spurious diffs, where nothing changes but some whitespace
> change gets committed. So, while it does not improve the codebase viewed
> statically, useless whitespace are prone to change in time, and those
> changes pollute the sequence of changesets that make the history of the
> codebase. Hence the reason why Ian asked for a separate commit for
> whitespace cleanup.

What creates spurious diffs isn't the presence of trailing whitespace,
it's tools that remove trailing whitespace on lines you're not
directly editing.  Eclipse, IntelliJ, and Emacs can all be configured
not to do so.  (As best I can tell, they're all configured that way by
default, too.)

> As for benefits of not having them, the simplest one is that codereview
> and other diff tools won't flag trailing whitespace and whitespace
> between tabs in red in ulterior diffs involving lines with them.

Code review tools should only flag diffs on lines that have been
modified, so again we're back to "don't remove trailing whitespace on
lines you're not otherwise touching".

> But
> whitespace cleanups are only worthwhile if new commits are disciplined
> to not bring them in again.

Indeed, and a lot of the changes in these whitespace-removal commits
are just removing "trailing" whitespace which is really just entirely
blank lines between code.  That's much harder to stop, and IMO not
worth trying to stop.

I really don't think we need any whitespace cleanup effort.

-- Adam


>
> Regards
> Santiago
>
>> Cheers,
>> Adam
>>
>> On Fri, Mar 6, 2009 at 3:11 AM, Vincent Siveton <[email protected]> wrote:
>> > The majority of the source code had already removed trailing
>> > whitespace. Some statistics:
>> > - around 600 java files
>> > - only around 140 java files don't removed trailing whitespace
>> >
>> > So, IMHO removing trailing whitespace is the way to go.
>> >
>> > WDYT?
>> >
>> > Cheers,
>> >
>> > Vincent
>> >
>> > 2009/3/5 Adam Winer <[email protected]>:
>> >> Instead of 2 commits, could you change it to not remove trailing
>> >> whitespaces?  It unnecessarily obfuscates history, especially when
>> >> there's trailing whitespace on non-blank lines.  This is typically an
>> >> option that can be adjusted in an IDE.
>> >>
>> >> -- Adam
>> >>
>> >>
>> >> On Thu, Mar 5, 2009 at 3:21 PM, Vincent Siveton <[email protected]> 
>> >> wrote:
>> >>> Hi Adam,
>> >>>
>> >>> 2009/3/5 Adam Winer <[email protected]>:
>> >>>> I see a lot of whitespace changes with each submission, apparently
>> >>>> around blank lines.  Any way you could avoid making these changes?
>> >>>> It's much harder to read diffs like this.
>> >>>
>> >>> Yeah, my IDE removes trailing whitespaces. I will do 2 commits next time.
>> >>>
>> >>> Cheers,
>> >>>
>> >>> Vincent
>> >>>
>> >>
>> >
>
>

Reply via email to