1015779 - Remove trailing white space across mail/

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Comment 1

[Josiah Bruner [:jsbruner]]

So while I was working on a personal project I came up with this command to remove trailing whitespace from my project files, but realized we could probably use it to cleanup around mail/ as well.

Here's that command: find . -name "*.css" | xargs gsed -i 's/[ \t]*$//'

gsed == GNU sed

Comment 2

[Josiah Bruner [:jsbruner]]

Attachment: Remove trailing whitespace - C++

This is the cleanup for C++.

Joshua, for the review really just take a look at the command, unless of course want to go through every line. :)

C++ actually did the best in regards to not having much trailing white space.

Comment 3

[Josiah Bruner [:jsbruner]]

Attachment: Remove trailing whitespace - JS

Mike, same deal as Joshua, just take a peak at the patch and focus on the command instead for the review. This should be pretty easy review-wise.

JS did the worst white space wise.

Comment 4

[Josiah Bruner [:jsbruner]]

Attachment: Remove trailing whitespace - XUL

UL version.

Comment 5

[Josiah Bruner [:jsbruner]]

Attachment: Remove trailing whitespace - XML

ML.

Comment 6

[Josiah Bruner [:jsbruner]]

Attachment: Remove trailing whitespace - C-type headers.

C headers.

Comment 7

[Josiah Bruner [:jsbruner]]

I just CC'd people I know may be affected by this change. Just as a heads up that you may have to un-bitrot your patches.

Comment 8

[Josiah Bruner [:jsbruner]]

I'll probably have to unbitrot these patches before landing, so don't r- for such a reason.

Comment 9

[:aceman]

I welcome such cleanup but I am sure some reviewers may frown upon such unnecessary noise ;)

Comment 10

[Magnus Melin [:mkmelin]]

Hm, yes, as you loose history, or make it hard to get to at least. Some history is more valuable than other of course...

Comment 11

[Josiah Bruner [:jsbruner]]

You don't really loose a lot of history though. One commit per file isn't really that big a deal I think. Unless there's some other history you're taking about besides a file's hg commit log.

I imagine quite a bit of this is a matter of preference. Personally I'd prefer to not have to deal with the white space (I frequently either have to remove white space when writing my own patches, or they are mentioned during review, even if I didn't actually add it. And I do the same when reviewing others)

So I myself would rather not have to worry about that anymore, and just take the little bit of hg history cloudiness. But I suppose Joshua and Mike can make the call.

Comment 12

[Joshua Cranmer [:jcranmer]]

You don't loose history, you lose history. :-)

English grammar aside, both hg and git let you follow blame while ignoring whitespace [CVS didn't,  but it's in the minority of VCSen]. The standard web interfaces for hg and git don't give you this option, but we fully control DXR and it's not unfeasible to add this kind of option to DXR at some point in the future. As one of the relatively few people on this bug that regularly does deep-dives on history of source code [to CVS 1.1 era], I'm not horribly bothered by whitespace-only changes, particularly if the commit message clearly indicates that it is whitespace-only.

(In reply to Josiah Bruner [:JosiahOne] from comment #1)
> Here's that command: find . -name "*.css" | xargs gsed -i 's/[ \t]*$//'

A few things to keep in mind:
1. Not all file formats are whitespace-agnostic. Makefiles are extremely bad offenders in this regard, but email files also have very meaningful CRLF and trailing whitespace that should not be obliterated. Arguably some of our textual resources for tests fall under the same category.
2. Different files have different coding styles. Sometimes even different files with the same extensions (yay for our "follow prevailing style" exception :-/ ).

I personally kind of prefer doing a single pass with a smart formatting tool (e.g., clang-format) to make all of our code conform to style guidelines, but that option never seems to get off the ground.

Comment 13

[Magnus Melin [:mkmelin]]

Yes I was talking about hg blame. I didn't know about the hg annotate -w option, seems silly that's not the default in the web ui.

Comment 14

[Josiah Bruner [:jsbruner]]

Attachment: Remove trailing whitespace - JS

Updated for trunk.

Comment 15

[Josiah Bruner [:jsbruner]]

https://hg.mozilla.org/comm-central/rev/9f37dabe70cb
https://hg.mozilla.org/comm-central/rev/f5bcaef84129
https://hg.mozilla.org/comm-central/rev/7e23a0acc5e8
https://hg.mozilla.org/comm-central/rev/09f077f965ef
https://hg.mozilla.org/comm-central/rev/9477e9a40a03