Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes:
> On Thu, 25 Jun 2020, Junio C Hamano wrote:
>
>> "Johannes Schindelin via GitGitGadget" <gitgitgadget@xxxxxxxxx>
>> writes:
>>
>> > diff --git a/diff-lib.c b/diff-lib.c
>> > index 61812f48c2..25fd2dee19 100644
>> > --- a/diff-lib.c
>> > +++ b/diff-lib.c
>> > @@ -220,8 +220,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
>> > } else if (revs->diffopt.ita_invisible_in_index &&
>> > ce_intent_to_add(ce)) {
>> > diff_addremove(&revs->diffopt, '+', ce->ce_mode,
>> > - the_hash_algo->empty_tree, 0,
>> > - ce->name, 0);
>> > + &null_oid, 0, ce->name, 0);
>>
>> This (even if the preimage were correctly using empty_blob) is *so*
>> simple a change that it is very surprising that the new test in
>> [2/2] passes without any other code change.
>>
>> It means that difftool was written correctly in the first place,
>> right?
>
> Well, it means that difftool does not even consume the OID. Sure, it
> parses it, but then it ignores it. All it _really_ is interested in is
> that status flag (`A`),
Ah, that's an ultimately defensive code. No matter what is on the
right hand side of the raw output, as long as it knows that the
right hand side is a file on the working tree, it can safely ignore
the object name and directly go to the working tree file.
Nice ;-)
> so technically, my fix in 1/2 is not even needed
> after `sk/diff-files-show-i-t-a-as-new` to let 2/2 pass.
Sure, but I think it is the right thing to do nevertheless. Giving
the object name for an empty blob would be wrong unless the working
tree file is known to be empty (and empty tree? what were we even
thinking, gee...).
