[cfe-dev] r338291 - Remove trailing space

[Fāng-ruì Sòng via cfe-dev]

I apologize that my two patches "Remove trailing space" (r338291 in clang, r338293 in llvm)
are committed without a discussion happening within the community.

Forward to llvm-dev and cfe-dev to see if they are what the community would like to see committed or not.

The original discussion is at this thread
http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20180730/236802.html

On 2018-07-30, Aaron Ballman wrote:
>On Mon, Jul 30, 2018 at 6:17 PM, Fāng-ruì Sòng <maskray at google.com> wrote:
>
>    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.
>
>I don't disagree that there are pros and cons in the discussion and that we
>should consider them carefully, but I think there should be a discussion that
>happens with the community *before* landing this patch, even though it's a NFC
>patch. Please revert these patches and start a discussion thread on whether
>this is something the community would like to see committed or not.

I want to collect some responses before doing the revert, in case the
revert actually makes things worse (people have to rebuild since these
files have changed again).