Re: [PATCH] crlf: Add test showing double warning on commit

From: Junio C Hamano <gitster@pobox.com>
Date: May 15, 2016, 18:55 UTC
Message-ID: <xmqqshxj5exx.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <55965a71-7078-60ad-0746-6f677dbb3f06@web.de>

Torsten Bögershausen <tboegi@web.de> writes:

Show 7 quoted lines

> Nja, (Or Nyes in English), the old handling tried to be "nice" to the user:
> $ git add text # gave warning
> #User forgets, does other things, git reset HEAD....
> $ git commit # Gave the warning one more time, to remind the user, 
>              # what he did, and what is really commited.
>
> But it may be, that diff_populate_filespec() is the wrong place for speaches ?

It is *not* necessarily a wrong place for speeches, but certainly it is wrong place to talk about whatever convert_to_git() does.

Let's step back and think what the warning is about.

I personally feel that "CRLF will be turned into LF" is pointless when the user expressed her preference "I want to have LF in the repository and CRLF in the working tree, when it makes sense". It is not even a warning but merely reports "we did what we were told to do." But for those who set safecrlf=warn to get that warning, we should give the warning when we actually convert the the working tree contents and strip CR from CRLF before recording it as an object.

An as-is "git commit" (nothing on command line affects the index with working tree contents, e.g. "-a" or pathspec) does not do any such conversion. The "damage" the user requested to be warned has long been done before the command is run.

So the warning from "git add" is good, but it shouldn't appear from anything that does "diff".