r338291 - Remove trailing space

Fāng-ruì Sòng via cfe-commits cfe-commits at lists.llvm.org
Mon Jul 30 15:17:38 PDT 2018


On 2018-07-30, Aaron Ballman wrote:
>On Mon, Jul 30, 2018 at 4:43 PM, Fāng-ruì Sòng <maskray at google.com> wrote:
>> Does this apply to only public headers (include/llvm include/llvm-c
>> include/clang ...) or everything? (lib/**/*.{cpp,h})?
>
>I've understood it applies to "everything" in that you should not
>commit large-scale NFC changes that don't have considerable benefit
>(for some definition of considerable benefit).

The benefits I can think of are:

* Some editors are configured to highlight trailing whitespace. Before
  the two cleanup commits, they will interfere reading.
* Some editors are configured to remove whitespace (as Michael pointed
  out). The removal will show up in diffs where revision authors have to undo
  manually. For some out-of-tree users, if they have local patches but do not
  strip trailing whitespace, related to settings of their code review system,
  this could turn to whitespace errors.

> e.g., if you're fixing
>a typo in an API and it hits a lot of files, that's fine because a
>typo in an API is pretty egregious, but don't clang-format a bunch of
>files and commit that because formatting isn't super critical and
>instead we migrate it more slowly as the code gets touched.

Thank you for raising these. I learned from your examples☺

>On Mon, Jul 30, 2018 at 4:49 PM, Fāng-ruì Sòng <maskray at google.com> wrote:
>> Maybe not too terrible for out-of-tree projects. If they use git
>> mirror, `git rebase` is smart enough to ignore changing lines with
>> trailing whitespace (if not, there is git rebase
>> -Xignore-space-at-eol). Some editors are configured with highlighting
>> trailing spaces and these spaces will turn to eyesore...
>
>Not everyone uses git; svn is still the official repository for the project.

I am not familiar with svn but stackoverflow tells me there is svn diff -x "--ignore-eol-style".
This should also be available to other svn functionality.


More information about the cfe-commits mailing list