Re: [PATCH v3 1/3] diff-merges: improve --diff-merges documentation

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 6, 2023, 18:01 UTC
Message-ID: <xmqq7cnzaav0.fsf@gitster.g>
In-Reply-To: <CABPp-BFsrt0zS3NHsVAyOSW6vGioe8Z-iN2M3_JNBpP2fWVq9g@mail.gmail.com>

Elijah Newren <newren@gmail.com> writes:

Show 14 quoted lines

>> > +--cc::
>> > +     Produce dense combined diff output for merge commits.
>> > +     Shortcut for '--diff-merges=dense-combined -p'.
>>
>> Good.
>>
>> > +--remerge-diff::
>> > +     Produce diff against re-merge.
>> > +     Shortcut for '--diff-merges=remerge -p'.
>> ...
> Perhaps:
>
> Produce remerge-diff output for merge commits, in order to show how
> conflicts were resolved.

I do not mind it, but then I'd prefer to see ", in order to show how" also in the description of "--cc" and "-c" for consistency.

A succinct way to say what they do may be hard to come by, but I think of them showing places that did not have obvious natural resolution.

Show 7 quoted lines

>     For a two-parent merge commit, a merge of these two commits is
>     retried to create a temporary tree object, potentially containing
>     files with conflict markers.  A diff is then shown between that
>     temporary tree and the actual merge commit.  This has the effect
>     of showing whether and how both semantic and textual conflicts
>     were resolved by the user (i.e. what changes the user made after
>     running 'git merge' and before finally committing).

Yes, and because we do not have a back reference from here to the description for "--remerge-diff" we saw earlier, we'd need the "in order to" you suggested earlier there, too.

Show 13 quoted lines

>> Either way, it makes readers wonder what happens to merges with more
>> than 2 parents (octopus merges).  It is not a new problem and this
>> topic should not attempt to fix it.
>
> We could add:
>
> For octopus merges (merges with more than two parents), currently
> only shows a warning about skipping such commits.
>
> if wanted.
>
> But perhaps I've distracted too much from Sergey's topic, and I should
> submit these wording tweaks as a patch on top?  I'm fine either way.

The primary purpose of polishing during a review cycle should be to help the original contributor to express what they wanted to express better, so talking about octopus behaviour, which wasn't covered in the original nor the patch under review, can be left out to avoid extending the scope of the topic further.

But everything else you said in the message I am responding to falls into the scope of the "improving existing documentation for various merge presentation modes" topic, I would think, and they are more or less usable verbatim, so it would not be too much of a burden to make sure they are used in the next iteration.

Thanks for a review, and thanks Sergey for streamlining the documentation around here.