public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Eric Gallager <egall@gwmail.gwu.edu>
To: Jakub Jelinek <jakub@redhat.com>
Cc: David Malcolm <dmalcolm@redhat.com>,
	Guillaume Gomez <guillaume1.gomez@gmail.com>,
	 gcc-patches@gcc.gnu.org
Subject: Re: [PATH] [CLEANUP] Remove trailing whitespace characters
Date: Mon, 11 Sep 2023 10:43:59 -0400	[thread overview]
Message-ID: <CAMfHzOum7XwWAK4Oo76vE8znXZk4CE9asqU5Aex60Lx=s4FWyA@mail.gmail.com> (raw)
In-Reply-To: <ZP8ZbVF5x4y5R2f8@tucnak>

On Mon, Sep 11, 2023 at 9:43 AM Jakub Jelinek via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> On Mon, Sep 11, 2023 at 09:27:48AM -0400, David Malcolm via Gcc-patches wrote:
> > On Sun, 2023-09-10 at 16:36 +0200, Guillaume Gomez wrote:
> > > When going through the code, I saw a lot of trailing whitespace
> > > characters so I decided to write a small script that would remove
> > > them. I didn't expect there would be so many though... Not sure if
> > > patch with so many changes are accepted like this or if I should send
> > > more focused one.
> >
> > I'm not sure either.
> >
> > Some notes on the patch:
>
> IMHO testsuite shouldn't be touched at all, there are certainly tests
> which test whether such sources are handled correctly.
>
> Non-C/C++ sources shouldn't be changed this way either.
>
> The ^L stuff should be preserved, not removed.
>
> And even with that, I'm not sure it is a good idea to change it because
> it will be a nightmare for git blame.
>

Some git hosting services have added support for special files to
ignore revisions like this in git blame, for example, on GitHub, it's
called .git-blame-ignore-revs:
https://github.blog/changelog/2022-03-24-ignore-commits-in-the-blame-view-beta/
See for example:
https://github.com/cooljeanius/highlight.js/blob/main/.git-blame-ignore-revs

> The usual way of fixing up formatting if it was committed in a broken way
> is only when one is touching with real code changes something, fixing up
> formatting on it or around it is fine.
>
> If we decide to fix formatting in bulk, I think we should have a flag day
> and change also other formatting mistakes at the same time (say 8 spaces
> instead of tabs for start of line indentation (before first non-blank
> character), = at the end of line except for static var initializers, etc.
> But to make that worthwhile, it would be better to then have a pre-commit
> hook that would enforce formatting.  And, we haven't managed to come up with
> something like that yet.
>
>         Jakub
>

  reply	other threads:[~2023-09-11 14:44 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-10 14:36 Guillaume Gomez
2023-09-11 13:27 ` David Malcolm
2023-09-11 13:30   ` Guillaume Gomez
2023-09-11 13:43   ` Jakub Jelinek
2023-09-11 14:43     ` Eric Gallager [this message]
2023-09-11 14:49       ` Arthur Cohen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAMfHzOum7XwWAK4Oo76vE8znXZk4CE9asqU5Aex60Lx=s4FWyA@mail.gmail.com' \
    --to=egall@gwmail.gwu.edu \
    --cc=dmalcolm@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=guillaume1.gomez@gmail.com \
    --cc=jakub@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).