Re: [PATCH 2/3] diff-format.txt: correct misleading wording

[Philippe Blain <levraiphilippeblain@gmail.com>]

Le 2022-06-13 à 14:56, Junio C Hamano a écrit :
> "Philippe Blain via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> From: Philippe Blain <levraiphilippeblain@gmail.com>
>>
>> Near the end of the "Raw output format" section, an example shows the
>> output of 'git diff-files' for a tracked file modified on disk but not
>> yet added to the index. However the wording is:
>>
>>     <sha1> is shown as all 0's if a file is new on the filesystem
>>     and it is out of sync with the index.
>>
>> which is confusing since it can be understood to mean that 'file' is a
>> new, yet untracked file, in which case 'git diff-files' does not care
>> about it at all.
> 
> I do not think such an understanding is sensible, as "untracked
> file" cannot be "out of sync with the index", because even its stale
> version wouldn't be in the index if it is untracked.
> 
> But I agree that not all people are sensible, and it would be nicer
> if the documentation helped them, too ;-)

Well, call me not sensible then ;) When I read that, I had trouble understanding
what was meant precisely because I had what you wrote in mind.

> 
>> When this example was introduced all the way back in c64b9b8860
>> (Reference documentation for the core git commands., 2005-05-05), 'old'
>> and 'new' referred to the two entities being compared, depending on the
>> command being used (diff-index, diff-tree or diff-files - which at the
>> time were diff-cache, diff-tree and show-diff). The wording used at the
>> time was:
>>
>>     <new-sha1> is shown as all 0's if new is a file on the
>>     filesystem and it is out of sync with the cache.
> 
> Yeah, I remember this version of wording.
> 
>> Rework the introductory sentence of the example to instead refer to
>> 'sha1 for "dst"', which is what the text description above it uses, and
>> fix the wording so that we do not mention a "new file".
> 
> That's good.  We may need to upgrade them to 'object name' to wean
> ourselves away from SHA-1 but that should be a separate topic.
> 
>> While at it, also tweak the wording used in the description of the raw
>> format to explicitely state that all 0's are used for the destination
>> hash if the working tree is out of sync with the index, instead of the
>> more vague "look at worktree".
> 
> I am not sure if that is a good idea.  Those who understand what the
> "work tree out of sync with" phrase mean would understand "look at
> work tree" but the reverse would not be true.

I'm not sure I agree. Even looking at it from a grammatical perspective,
"the hash is all zero if a path is unmerged, is a deletion or is out of sync
with the index" makes more sense to me than "the hash is all zero if a path is 
unmerged, is a deletion or "look at worktree""...

> 
> The other hunk about "new" -> "dst" is a good change regardless, but
> even there, "out of sync with" may need to be rewritten to make it
> easier to understand.  Is it different from "the index does not know
> the exact value (hence you need to look in the working tree if you
> really cared to find it out, perhaps with hash-object)"?

I think it's clear what is meant, I don't think we need to change it.

Thanks,

Philippe.