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

From: Christian Holtje <docwhat@gmail.com>
Date: Jun 26, 2008, 02:41 UTC
Message-ID: <635350D7-2CC0-4FA6-BB74-CE9ED930ECE8@gmail.com>
In-Reply-To: <7vhcbh1g4e.fsf@gitster.siamese.dyndns.org>

On Jun 25, 2008, at 3:14 PM, Junio C Hamano wrote:

Show 38 quoted lines

> Christian Holtje <docwhat@gmail.com> writes:
>>
>> Junio, do you want me to make this change anyway?  It does make  
>> sense.
>> The unittests for the pre-commit hook may or may not still be   
>> useful.
>
> "disable" is not an issue.  The intention has always been that these  
> are
> samples, and it was an accident that some packaging shipped them  
> enabled
> by mistake.  The patch was to make that mistake harder to make.
>
> The issue now is about keeping the example hooks _relevant_.  The  
> one we
> have does not work well with projects that want to check in files with
> CRLF line endings (iow, without using autocrlf to attempt to make the
> project files cross-platform), so it is irrelevant for such projects  
> with
> Windows origin.
>
> The "solution" you are proposing to strip out \r makes the check less
> useful for projects that want to keep files with LF line endings in  
> the
> commited history, because your patch would stop catching such a  
> mistake of
> adding an CR before LF.  It is robbing from Peter to pay Paul, and I  
> am
> afraid it would make the sample even more irrelevant in the end.  I  
> do not
> think we would want to go there.
>
> I suggested using "diff --check" (and possibly teaching "diff --check"
> other things the scripted example checks, such as conflict markers),
> which would know to honor the line endings specified per path via
> gitattributes(5), instead of building on top of the big Perl script,  
> and I
> had an impression that you agreed to the approach.

I'm completely confused how gitattributes and core.autocrlf interact, etc.

I'm expecting the default behavior is that git will leave my files alone. This seems to be the case.

In order for the crlf stuff to work, does it need to have core.autocrlf set to true? If so, that seems wrong. gitattributes(5) is supposed to let you have fine grain control over files. In addition, .git/config isn't passed around via clone so .gitattributes works differently depending on who clones it and their settings.

Furthermore, how would 'git diff --check' know what the line endings are for a file? I may have a mostly unix repository but I may have a few crlf text files I need to have checked out as crlf on windows (but not unix).

Shouldn't the crlf stuff be something like:
.gitattributes:
   eol=crlf -- dos files. no conversion, but diff --check will know  
what is what.
   eol=lf -- unix (otherwise same)
   eol=convert -- stores internally as lf, converts on the fly to lf  
or crlf on filesystem based on system preference. diff --check won't  
worry about new lines matching system-eol if they don't match the eol  
in the file.
   eol=binary -- binary

core.eol=crlf -- ony files with all crlf endings are text. core.eol=lf -- only files with all lf (no crlf) endings are text. core.eol=convert -- crlf changes will be converted to lf as per system- eol core.eol=false -- do nothing (default) (binary)

core.system-eol=crlf -- pretend the system is windows core.system-eol=lf -- pretend the system is unix core.system-eol=auto -- determine from the system (default)

Otherwise, I have no clue what's going on. Once someone writes this in english, I'll try to write unittests and I'll submit them. Then someone who knows what he/she is doing can write the actual code to make the tests pass. Or maybe I'll try, who knows.

Ciao!