git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Elijah Newren <newren@gmail.com>
Cc: Sergey Organov <sorganov@gmail.com>, git@vger.kernel.org
Subject: Re: [PATCH v3 1/3] diff-merges: improve --diff-merges documentation
Date: Fri, 06 Oct 2023 11:01:39 -0700	[thread overview]
Message-ID: <xmqq7cnzaav0.fsf@gitster.g> (raw)
In-Reply-To: <CABPp-BFsrt0zS3NHsVAyOSW6vGioe8Z-iN2M3_JNBpP2fWVq9g@mail.gmail.com> (Elijah Newren's message of "Fri, 6 Oct 2023 07:41:51 -0700")

Elijah Newren <newren@gmail.com> writes:

>> > +--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.

>     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.

>> 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.



  parent reply	other threads:[~2023-10-06 18:01 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-09 12:54 [PATCH 0/2] diff-merges: introduce '-d' option Sergey Organov
2023-09-09 12:54 ` [PATCH 1/2] diff-merges: improve --diff-merges documentation Sergey Organov
2023-09-11 21:12   ` Junio C Hamano
2023-09-12  7:37     ` Sergey Organov
2023-09-13  0:22       ` Junio C Hamano
2023-09-18 16:20         ` Sergey Organov
2023-09-19 16:38           ` Junio C Hamano
2023-09-19 19:52             ` Sergey Organov
2023-09-09 12:54 ` [PATCH 2/2] diff-merges: introduce '-d' option Sergey Organov
2023-09-11 21:01   ` Junio C Hamano
2023-09-12  7:59     ` Sergey Organov
2023-09-14 22:17       ` Junio C Hamano
2023-09-14 23:56         ` Sergey Organov
2023-09-15 17:24           ` Junio C Hamano
2023-09-16 18:37             ` Sergey Organov
2023-09-26  2:50               ` Junio C Hamano
2023-09-26  9:04                 ` Sergey Organov
2023-09-26 17:08                   ` Junio C Hamano
2023-09-26 20:05                     ` Sergey Organov
2023-09-20 15:02 ` [PATCH v2 0/2] " Sergey Organov
2023-09-20 15:02   ` [PATCH v2 1/2] diff-merges: improve --diff-merges documentation Sergey Organov
2023-09-20 15:02   ` [PATCH v2 2/2] diff-merges: introduce '-d' option Sergey Organov
2023-10-04 21:45 ` [PATCH v3 0/3] diff-merges: introduce '--dd' option Sergey Organov
2023-10-04 21:45   ` [PATCH v3 1/3] diff-merges: improve --diff-merges documentation Sergey Organov
2023-10-04 22:02     ` Eric Sunshine
2023-10-04 22:13       ` Sergey Organov
2023-10-05 21:11       ` Junio C Hamano
2023-10-06 17:02         ` Sergey Organov
2023-10-05 21:24     ` Junio C Hamano
2023-10-06 14:41       ` Elijah Newren
2023-10-06 17:03         ` Sergey Organov
2023-10-06 17:07         ` Sergey Organov
2023-10-06 18:01         ` Junio C Hamano [this message]
2023-10-06 18:36           ` Sergey Organov
2023-10-06 23:19             ` Junio C Hamano
2023-10-07  1:31           ` Elijah Newren
2023-10-07  1:50             ` Junio C Hamano
2023-10-07  6:49               ` Junio C Hamano
2023-10-09 17:04                 ` Elijah Newren
2023-10-10  0:24                   ` Junio C Hamano
2023-10-10  2:44         ` [silly] worldview documents? Junio C Hamano
2023-10-10 14:58           ` Emily Shaffer
2023-10-06 17:18       ` [PATCH v3 1/3] diff-merges: improve --diff-merges documentation Sergey Organov
2023-10-06 18:42       ` Sergey Organov
2023-10-04 21:45   ` [PATCH v3 2/3] diff-merges: introduce '--dd' option Sergey Organov
2023-10-05 21:45     ` Junio C Hamano
2023-10-06 17:05       ` Sergey Organov
2023-10-04 21:45   ` [PATCH v3 3/3] completion: complete '--dd' Sergey Organov
2023-10-05 21:45     ` Junio C Hamano
2023-10-06 18:53       ` Sergey Organov
2023-10-09 16:05 ` [PATCH v4 0/3] diff-merges: introduce '--dd' option Sergey Organov
2023-10-09 16:05   ` [PATCH v4 1/3] diff-merges: improve --diff-merges documentation Sergey Organov
2023-10-09 16:05   ` [PATCH v4 2/3] diff-merges: introduce '--dd' option Sergey Organov
2023-10-09 16:05   ` [PATCH v4 3/3] completion: complete '--dd' Sergey Organov
2023-10-09 20:02   ` [PATCH v4 0/3] diff-merges: introduce '--dd' option Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqq7cnzaav0.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=newren@gmail.com \
    --cc=sorganov@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).