whitespace-only changes

whitespace-only changes

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

Larsi wrote:

  > Thanks for the patch, but we don't normally apply whitespace-only fixes,
  > because that makes it harder to investigate, later (with git blame and
  > friends), what previous code changes did.

I am sure he knows what he's talking about, but it is unfortunate that
we cannot fix bad indentation.  Can't we correct this problem somehow?

For instance, label whitespace changes clearly in the commit log, and
change some tools to ignore them?

-- 
Dr Richard Stallman
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

Re: whitespace-only changes

[Mattias Engdegård]

20 aug. 2020 kl. 05.06 skrev Richard Stallman <rms@gnu.org>:

> I am sure he knows what he's talking about, but it is unfortunate that
> we cannot fix bad indentation.  Can't we correct this problem somehow?

At the very least we should always accept correction of misleading indentation. That is not noise, but fully equivalent to fixing wrong comments or bad naming, neither of which change the program as seen by the machine but do alter it in human eyes.

At the other end of the scale, I definitely share Lars's dim view of invisible changes (trimming end-of-line whitespace or transmuting tabs into spaces).

Re: whitespace-only changes

[Eli Zaretskii]

> From: Richard Stallman <rms@gnu.org>
> cc: emacs-devel@gnu.org
> Date: Wed, 19 Aug 2020 23:06:55 -0400
> 
> For instance, label whitespace changes clearly in the commit log, and
> change some tools to ignore them?

The tool is Git, and it will treat any change as significant.  I'm not
aware of a Git option to ignore some changes based on the log
messages.

Re: whitespace-only changes

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> The tool is Git, and it will treat any change as significant.  I'm not
> aware of a Git option to ignore some changes based on the log
> messages.

Me neither, but it sure would be nice to be able to tell git "this is
not a functional change" (just think of the yearly copyright notice
update).

git has no way of verifying that it's a non-functional change, of
course, so it would just be a hint to the UI.  That could then be used
by nefarious people to sneak in functional changes but mark them as
non-functional.

So it's probably not a workable idea.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: whitespace-only changes

[David Engster]

> Eli Zaretskii <eliz@gnu.org> writes:
>
>> The tool is Git, and it will treat any change as significant.  I'm not
>> aware of a Git option to ignore some changes based on the log
>> messages.
>
> Me neither, but it sure would be nice to be able to tell git "this is
> not a functional change" (just think of the yearly copyright notice
> update).
>
> git has no way of verifying that it's a non-functional change, of
> course, so it would just be a hint to the UI.  That could then be used
> by nefarious people to sneak in functional changes but mark them as
> non-functional.
>
> So it's probably not a workable idea.

I agree, but just to have mentioned it, at least 'git blame' does
support ignoring commits. These can be given on the commandline or in a
file (see --ignore-rev and --ignore-revs-file since version v2.23). This
file could of course be generated by looking at commit messages.

-David

Re: whitespace-only changes

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

If git itself doesn't have all the facilities for ignoring trivial
commits that we would want, we can add those facilities in various ways,
with patches to git or scripts.  We can offer the patches upstream.

It is useful for humans to make the commit log say "whitespace change."
But scripts can detect a trivial change by looking at the actual diff.

-- 
Dr Richard Stallman
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

Re: whitespace-only changes

[Sergey Organov]

Richard Stallman <rms@gnu.org> writes:

> [[[ To any NSA and FBI agents reading my email: please consider    ]]]
> [[[ whether defending the US Constitution against all enemies,     ]]]
> [[[ foreign or domestic, requires you to follow Snowden's example. ]]]
>
> If git itself doesn't have all the facilities for ignoring trivial
> commits that we would want, we can add those facilities in various ways,
> with patches to git or scripts.  We can offer the patches upstream.

Git has various options to ignore whitespace changes during merge:

   ignore-space-change, ignore-all-space, ignore-space-at-eol,
   ignore-cr-at-eol
       Treats lines with the indicated type of whitespace change as
       unchanged for the sake of a three-way merge. Whitespace changes
       mixed with other changes to a line are not ignored. See also
       git-diff(1) -b, -w, --ignore-space-at-eol, and
       --ignore-cr-at-eol.

       •   If their version only introduces whitespace changes to a
           line, our version is used;

       •   If our version introduces whitespace changes but their
           version includes a substantial change, their version is
           used;

       •   Otherwise, the merge proceeds in the usual way.


OTOH, when merging, by design Git never looks at individual commits, it
rather performs 3-way merge with respect to common ancestor, so no
marking of individual commits would help.

-- Sergey

Re: whitespace-only changes

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > > If git itself doesn't have all the facilities for ignoring trivial
  > > commits that we would want, we can add those facilities in various ways,
  > > with patches to git or scripts.  We can offer the patches upstream.

  > Git has various options to ignore whitespace changes during merge:

People did not spell out precisely which "tools" they thought would
get confused by whitespace-only changes.  I got the impression
they were not talking specifically and only about merging,
but I don't actually know.  Perhaps someone should say.

-- 
Dr Richard Stallman
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

Re: whitespace-only changes

[Daniel Martín]

Richard Stallman <rms@gnu.org> writes:

> [[[ To any NSA and FBI agents reading my email: please consider    ]]]
> [[[ whether defending the US Constitution against all enemies,     ]]]
> [[[ foreign or domestic, requires you to follow Snowden's example. ]]]
>
> Larsi wrote:
>
>   > Thanks for the patch, but we don't normally apply whitespace-only fixes,
>   > because that makes it harder to investigate, later (with git blame and
>   > friends), what previous code changes did.
>
> I am sure he knows what he's talking about, but it is unfortunate that
> we cannot fix bad indentation.  Can't we correct this problem somehow?
>
> For instance, label whitespace changes clearly in the commit log, and
> change some tools to ignore them?

Git 2.23 introduced a feature to make git blame ignore these kind of
commits that only contain refactoring changes. The steps to make it work
are as follows:

First, add a .git-blame-ignore-revs to the root of the repository. This
file should contain the list of commit hashes to ignore (one per line).

Then, each developer should configure their local environment to take
the .git-blame-ignore-revs file into account by running the following
command from a shell:

$ git config blame.ignoreRevsFile .git-blame-ignore-revs

It's not an ideal solution, though. It depends on having a recent
version of git installed, only works for git blame, and it won't work if
someone is using https://git.savannah.gnu.org to check the history of
code changes, for example.

--
Daniel Martín

Re: whitespace-only changes

[Lars Ingebrigtsen]

Daniel Martín <mardani29@yahoo.es> writes:

> First, add a .git-blame-ignore-revs to the root of the repository. This
> file should contain the list of commit hashes to ignore (one per line).

How do revs end up in that file?  Is it a manual process?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: whitespace-only changes

[Daniel Martín]

Lars Ingebrigtsen <larsi@gnus.org> writes:
>
> How do revs end up in that file?  Is it a manual process?

Yes, the developer needs to add the revs manually.

--
Daniel Martín

Re: whitespace-only changes

[Stefan Monnier]

> I am sure he knows what he's talking about, but it is unfortunate that
> we cannot fix bad indentation.  Can't we correct this problem somehow?

FWIW, I think it's OK to commit whitespace-only changes when they fix
a "serious" problem, e.g. when the indentation is confusing.

AFAIK in the sample suggested commit, none of the changes were "serious"
(there was even one reindentation where I happen to prefer the current
indentation (even though it doesn't match what our auto-indent code
does)).

> For instance, label whitespace changes clearly in the commit log, and
> change some tools to ignore them?

I don't think whitespace-only changes are justified often enough to
warrant all that work (there are many tools that would be affected).
More useful would be to make the tools aware of file where whitespace
changes are (mostly) not significant so they can auto-resolve
some conflicts.


        Stefan

Re: whitespace-only changes

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > More useful would be to make the tools aware of file where whitespace
  > changes are (mostly) not significant so they can auto-resolve
  > some conflicts.

Can you say more concretely what that would mean?  What is the job
you suggest they do?


-- 
Dr Richard Stallman
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)