Re: [PATCH] Document check option to git diff.

From: Junio C Hamano <junkio@cox.net>
Date: Jan 27, 2007, 03:05 UTC
Message-ID: <7vps91p3ss.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <17850.45971.918929.314082@lisa.zopyra.com>

Bill Lear <rael@zopyra.com> writes:

Show 11 quoted lines

> I believe that an accurate and concise statement would be:
>
>     Warn if changes introduce trailing whitespace
>     or an indent that uses a space before a tab.
>
> I think it should be explicitly limited to "space" and not
> "whitespace" before the tab, as "whitespace" really includes tab.
>
> Do I really need to say "trailing whitespace at the end of the line"?
> That seems overly verbose: trailing whitespace is, I think, understood
> to trail at the end of the line.

All true.  Thanks for rewording.

Show 6 quoted lines

> Also: I suppose I am wondering about the motivation for this switch.
> It seems to reflect the aesthetics of the git project.  Whitespace at
> the end of a line is meaningless and wasteful, so I understand and
> sympathize with the judgment that this is undesirable.  Spaces
> preceding tabs are somewhat murkier: two tabs, a space, and text pass
> the check, but a tab, space, tab and text do not.  Why is this bad?

The trailing space removal comes from the kernel project aesthetics:

	http://www.zip.com.au/~akpm/linux/patches/stuff/tpp.txt
	linux-2.6/Documentation/CodingStyle (end of Chapter 1)

"TAB SP TAB blah" visually is the same as "TAB TAB blah". It is bad for the same reason as "fofofofo SP TAB EOL" (which is visually the same as "fofofofo EOL") is bad. This is not from the kernel project, so if the above reasoning is flawed, it is my fault. I once considered saying 8 spaces are bad and should be replaced with a tab, but I refrained from going that far ;-).

> I'm sure there is a better way to categorize these violations other
> than "funny".  Should we not say "wasteful and inelegant, and
> therefore anathema to any decent, self-respecting person"?

"Wasteful" is probably better than "funny"; I do not think of a good wording.

Sometimes we do want to keep the trailing whitespaces (the patch that came over e-mail to produce commit c7c24889, for example), so getting warnings from "git-diff --check" (or its counterpart, "git-apply --whitespace=warn") is not a crime. But most of the time, they only waste space and bandwidth and many people who are conscious about hygiene of their sources seem to avoid them like the plague.