El dom, 08-03-2009 a las 09:15 -0700, Adam Winer escribió: > 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.) >
Lots of times it is also created by people inadvertently deleting or adding invisible spaces, or having files with mix of tabs/spaces processed automatically by editors. > > 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". > Most if not all diff tools are currently flagging "useless" whitespace in red everywhere in the context shown, so I think you will need a lot of evangelism to get this "should only" working. > > 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. > I think whitespace cleanup + tight specs for whitespace is useful as a whole. Quite often it is a nice byproduct of efforts to understand code or other refactorings. We clearly disagree here. But I'm about to stop being a mentor for Shindig, due to lack of time + lack of professional incentive for gadgets, so my opinion should not be weighted too much here. Regards Santiago > -- 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 > >> >>> > >> >> > >> > > > > >
