Re: What's cooking extra

From: Clemens Buchacher <drizzd@aon.at>
Date: May 22, 2010, 22:27 UTC
Message-ID: <20100522222746.GA2694@localhost>
In-Reply-To: <50199F1F-3513-43A6-8990-957F3D0AF58C@gmail.com>

Hi Eyvind,

Thanks for the extended summary. I still have several doubts, as detailed below. But I understand that this has been heavily discussed and if the discussion has indeed come to a conclusion, then I will not complain about it now.

On Sat, May 22, 2010 at 09:42:14PM +0200, Eyvind Bernhardsen wrote:

Show 5 quoted lines

> On 22. mai 2010, at 15.09, Clemens Buchacher wrote:
> 
> > As soon as the existing crlf attribute is given priority over
> > core.autocrlf, all the problems discussed originally go away.
> > So what exactly are the new attributes supposed to do?

For all my comments below I am assuming that the behavior of autocrlf will be changed to respect the crlf/text attribute by default.

> There is one new attribute, "eol", that is used for files which
> need a specific line ending.  Being able to "force" LF or CRLF
> line endings has been requested several times on the list, and is
> already sort of provided for by "crlf=input".

[...]

Show 7 quoted lines

> Any file with the "text" attribute set will have its line endings
> normalized to LF in the repository.  If "text" is set to the
> special value "auto", git will only convert the file if it looks
> like a text file.
> 
> The "eol" attribute is used for files that need a specific line
> ending.  Setting it also sets "text".

If a file needs specific line endings, why enable conversion for this file at all? Just make sure the repository contains the correct version and unset the crlf attribute.

> core.eol controls which line endings to use for normalized files
> that don't have the "eol" attribute set, and defaults to the
> platform native line ending.

That makes sense to me, except for the part where I need a per-file attribute.

Show 9 quoted lines

> When core.autocrlf is set, the default value of the "text"
> attribute is set to "auto" but with an extra safety feature: if a
> file contains CRs in the index, it won't be normalized.  The
> extra feature comes from Finn Arne's "safe autocrlf" patch.
> 
> There is a backwards compatibility wrinkle in that core.autocrlf
> will override core.eol if the latter isn't explicitly set, so
> that "core.autocrlf=true" still results in CRLFs in the working
> directory on Linux.

This also makes sense. I just fear that making this frequently misunderstood feature even more complex will only confuse users further.

I do see the value of a global core.eol option, however, since it allows me to convert to LF instead of CRLF, which AFAIK is not currently possible.

On the other hand, this will cause users to stop caring whether or not a file in the repository has LF or CRLF line endings. I am not sure if that is a good thing, but I suppose it is better than what we have now.

Show 8 quoted lines

> > And, renaming the crlf attribute to text? Where did Linus suggest that? If
> > we do that, we don't even have to talk about backwards compatibility any
> > more.
> 
> In <alpine.LFD.2.00.1005121824260.3711@i5.linux-foundation.org>:
> > So if you rename these things, keep them separate.  Make the "am I a
> > text-file" boolean be a boolean (plus "auto"), and just call it "text". 
> > And make the "what end of line to use" be just "eol" then.

I see. Well, if we rename the "crlf" attribute then we will have a macro attribute "binary" and an attribute "text", which are not the opposite of each other. That is a bit strange.

> The "crlf" attribute will be used if it is present so backwards
> compatibility is preserved to a degree.

Ah, ok. That is fine then.

Show 5 quoted lines

> Scripts that test for
> the "crlf" attribute explicitly (such as git-cvsserver, which I
> fixed) will break.  I don't know how big a problem that is going
> to be in practice, but nobody raised it as an issue during the
> discussion.

I agree that should not be a big issue.

Regards, Clemens