Re: [PATCH] Respect crlf attribute even if core.autocrlf has not been set

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Jul 23, 2008, 01:10 UTC
Message-ID: <alpine.DEB.1.00.0807230203350.8986@racer>
In-Reply-To: <7vy73tihl6.fsf@gitster.siamese.dyndns.org>

Hi,

On Tue, 22 Jul 2008, Junio C Hamano wrote:

Show 16 quoted lines

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > When a file's crlf attribute is explicitely set, it does not make 
> > sense to ignore it, just because the config variable core.autocrlf has 
> > not been set.
> 
> I am not sure if I agree with that reasoning.
> 
> Attribute defines what each path is.  Is it a text file, is it a binary? 
> The nature of the contents does not change between people on POSIX and 
> Windows, and that is why it is described in .gitattributes and cloned 
> across repositories.
> 
> On the other hand, the configuration defines what to do with contents with
> various attributes in this particular repository.  Do I want to see a text
> file checked out with CRLF endings, or LF?

Actually, I now see that I expressed myself badly.  Extremely badly.

The whole issue is about _check in_, as can be seen from the test case I provided.

And I think it is even a bug in crlf handling, as gitattributes.txt has this to say about crlf=input:

        This is similar to setting the attribute to `true`, but
        also forces git to act as if `core.autocrlf` is set to
        `input` for the path.

It suggests to this coder that core.autocrlf is not even looked at when crlf=input.

Show 8 quoted lines

> So it is perfectly valid and normal for a cross-platform minded project 
> to use the crlf atttribute to say "These files are text" and expect them 
> to be checked out with LF endings on POSIX while making sure they are 
> checked out with CRLF on Windows.  Adding CR at the end of line for such 
> files on POSIX systems is positively a wrong thing to do in such a case.
> 
> Projects like the kernel that originate from LF side of the world may not
> bother marking things as such, though.

I fully agree.

However, if you want to avoid CRs to _enter_ the repository, when you have a lot of binary files tracked, you _do_ want to force all repositories to crlf=input.

Now, if you look at the patch, you will see that it _only_ touches crlf_to_git(), but not only for crlf=input, but also for crlf=true.

I maintain that this respects the law of least surprise, namely that if you set the attribute crlf=true, and some person forgets to set core.autocrlf=true, at _check in_ CRs are stripped, but at _check out_, no CR is added (as the person did not ask for core.autocrlf, but that should not punish all the others who do not want to have CRs in the repository).

But yes, the commit message, and the oneline in particular are severely lacking.

Tomorrow, Dscho