Re: [PATCH 1/2] doc: fixup/squash: clarify use of <oid-hash> in subject line

> Philip Oakley <philipoakley@iee.email> writes:
>
>> The use of ellision `...` isn't great, as it gives no hint or clue,
>> leaving the subsequent test with a difficult explanation.
> True.  If you are planning to correct it in 2/2, then I think it
> makes more sense to squash that in to have a single patch.

>> Clarify if a full oid has is required, or a unique abbreviation within
>> the respository, or just uniques within the rebase instruction?
> Puzzled.  You must know the answer to "do we need a full object
> name, or is it sufficient to have anything that gives us a unique
> commit object name?" so why not write it in the patch instead of
> asking the question here?  Or do you not know the answer and this is
> a RFC/WIP patch????

>
>> This is a minimal change that sidesteps the chance to rewrite/clarify
>> the potential wider confusions over specifying the <commit> being
>> referred to in the fixup/squash process.
> Hmph.  So this step cannot be reviewed to judge if it is a good
> change by itself?

>
> Let me locally recreate a squashed single patch and review _that_
> instead.
>
>>  Documentation/git-rebase.txt | 18 ++++++++++--------
>>  1 file changed, 10 insertions(+), 8 deletions(-)
>>
>> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
>> index 4624cfd288..462cb4c52c 100644
>> --- a/Documentation/git-rebase.txt
>> +++ b/Documentation/git-rebase.txt
>> @@ -571,16 +571,18 @@ See also INCOMPATIBLE OPTIONS below.
>>  
>>  --autosquash::
>>  --no-autosquash::
>> -	When the commit log message begins with "squash! ..." (or
>> -	"fixup! ..."), and there is already a commit in the todo list that
>> -	matches the same `...`, automatically modify the todo list of rebase
>> +	When the commit log message begins with "squash! <line>" (or
>> +	"fixup! <line>"), and there is already a commit in the todo list that
>> +	matches the same `<line>`, automatically modify the todo list of rebase
>>  	-i so that the commit marked for squashing comes right after the
>>  	commit to be modified, and change the action of the moved commit
>> +	from `pick` to `squash` (or `fixup`).
>> ++
>> +A commit matches the `<line>` if
>> +the commit subject matches, or if the `<line>` refers to the commit's
>> +hash. As a fall-back, partial matches of the commit subject work,
>> +too.  The recommended way to create fixup/squash commits is by using
>> +the `--fixup`/`--squash` options of linkgit:git-commit[1].
>>  +
> Overall it looks much better than the original.
>
> The original did not even attempt to define what is a "match" for
> the purpose of this option, so the ellipses may have been OK, but
> once we need to refer to what is there, we need a name to refer to
> it and ellipses no longer are sufficient, and using the step 1/2
> alone would not make any sense.  We definitely should take the step
> 2/2 together with it.

>
> "A commit matches the <line> if the commit subject matches" is not a
> great definition of what a "match" is, though.  The readers are left
> in the same darkness about what constitutes a "match" of <line>
> against "the commit subject".  If you define this "subject matches"
> as a substring match, for example, you do not even have to say "as a
> fall-back"---it is by (the updated version of your) definition that
> how the commit subject and <line> matches so there is no need to
> allow any fall-back involved.