Thread (54 messages) flat view 54 messages, 9 authors, 2016-06-15

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

From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:44:59

Hi,

On Tue, 22 Jul 2008, Junio C Hamano wrote:
Johannes Schindelin [off-list ref] writes:
quoted
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.
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help