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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Christian Holtje <docwhat@xxxxxxxxx> 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.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux