Re: [PATCH] pre-commit hook should ignore carriage returns at EOL

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 24, 2008, 22:31 UTC
Message-ID: <7v3an2bh3b.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <39C2861E-F800-40AE-8C15-4FC3BB51EF16@gmail.com>

Christian Holtje <docwhat@gmail.com> writes:

> The code is checking for \r$ and then doing a different space check
> depending on that, not one after another.
>
> Thanks for the feedback. I'll put up v2 in a second.

Please don't.

It's an ancient sample hook that is not be enabled by default. I do not want people to be wasting too much time on the relic.

However, if this sample is to be changed at all, please do it right.

If somebody suddenly adds CR at the end of an existing file that ought to have LF line endings, we _DO_ want to catch that as a breakage. So the title of the commit "should ignore carriage returns at EOL" is WRONG. It shouldn't, in general.

One thing the hook could and probably should do these days is if the file type says you _ought to_ have CRLF line endings, actively make sure your lines do end with CRLF (this is a much stronger and better check than blindly ignoring CR before LF for such files). And on the other hand, if the file should end with LF, do make sure it does not have CR before it.

The person who did the sample hook you are looking at couldn't do so because there weren't autocrlf nor gitattributes(5) facility back then. But you can use them now to rewrite this properly.

I wonder if "git diff --check" can be used for most if not all of the checking, without the big Perl script you are touching in your patch. That facility did not exist when the current sample hook was written, either.