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

From: Torsten Bögershausen <tboegi@web.de>
Date: May 15, 2016, 06:39 UTC
Message-ID: <55965a71-7078-60ad-0746-6f677dbb3f06@web.de>
In-Reply-To: <xmqqoa887a25.fsf@gitster.mtv.corp.google.com>

On 14.05.16 20:45, Junio C Hamano wrote:

Show 23 quoted lines

> Torsten Bögershausen <tboegi@web.de> writes:
> 
>> Do we need to run diff_populate_filespec() twice when src==dst ?
> 
> Of course we do.
> 
> src and dst may have the same path, but are coming from different
> places (src may be an indexed blob while dst may be a file in the
> working tree).
> 
>> If yes, we may need to introduce a flag besides
>> #define CHECK_SIZE_ONLY 1
>> #define CHECK_BINARY    2
>> to suppress the conversion warning ??
> 
> I do not think that belongs to diff_populate_filespec() at all.
> 
> Why should conversion routine give this warning when called by
> diff_populate_filespec() in the first place?  Shouldn't it be silent
> by default, and is allowed to talk _ONLY_ when we are attempting to
> actually replace the data in the index, e.g. "git add" and "git
> commit -a"?
>

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 ?