diff mbox series

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

Message ID 20231004214558.210339-2-sorganov@gmail.com (mailing list archive)
State Superseded
Headers show
Series [v3,1/3] diff-merges: improve --diff-merges documentation | expand

Commit Message

Sergey Organov Oct. 4, 2023, 9:45 p.m. UTC
* Put descriptions of convenience shortcuts first, so they are the
  first things reader observes rather than lengthy detailed stuff.

* Get rid of very long line containing all the --diff-merges formats
  by replacing them with <format>, and putting each supported format
  on its own line.

Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
 Documentation/diff-options.txt | 98 ++++++++++++++++++----------------
 Documentation/git-log.txt      |  2 +-
 2 files changed, 54 insertions(+), 46 deletions(-)

Comments

Eric Sunshine Oct. 4, 2023, 10:02 p.m. UTC | #1
On Wed, Oct 4, 2023 at 5:51 PM Sergey Organov <sorganov@gmail.com> wrote:
> * Put descriptions of convenience shortcuts first, so they are the
>   first things reader observes rather than lengthy detailed stuff.
>
> * Get rid of very long line containing all the --diff-merges formats
>   by replacing them with <format>, and putting each supported format
>   on its own line.
>
> Signed-off-by: Sergey Organov <sorganov@gmail.com>
> ---
> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
> @@ -43,66 +43,74 @@ endif::git-diff[]
> +-m::
> +       Show diffs for merge commits in the default format. This is
> +       similar to '--diff-merges=on' (which see) except `-m` will
> +       produce no output unless `-p` is given as well.

I'm having difficulty grasping the parenthetical "(which see)" comment.
Sergey Organov Oct. 4, 2023, 10:13 p.m. UTC | #2
Eric Sunshine <sunshine@sunshineco.com> writes:

> On Wed, Oct 4, 2023 at 5:51 PM Sergey Organov <sorganov@gmail.com> wrote:
>> * Put descriptions of convenience shortcuts first, so they are the
>>   first things reader observes rather than lengthy detailed stuff.
>>
>> * Get rid of very long line containing all the --diff-merges formats
>>   by replacing them with <format>, and putting each supported format
>>   on its own line.
>>
>> Signed-off-by: Sergey Organov <sorganov@gmail.com>
>> ---
>> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
>> @@ -43,66 +43,74 @@ endif::git-diff[]
>> +-m::
>> +       Show diffs for merge commits in the default format. This is
>> +       similar to '--diff-merges=on' (which see) except `-m` will
>> +       produce no output unless `-p` is given as well.
>
> I'm having difficulty grasping the parenthetical "(which see)" comment.

I believe it's translated full form of q.v., see:

https://en.wikipedia.org/wiki/List_of_Latin_abbreviations

"q.v.
 quod vide
 "which see"

Imperative, used after a term or phrase that should be looked up
elsewhere in the current document or book."

HTH,
-- Sergey Organov
Junio C Hamano Oct. 5, 2023, 9:11 p.m. UTC | #3
Eric Sunshine <sunshine@sunshineco.com> writes:

>> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
>> @@ -43,66 +43,74 @@ endif::git-diff[]
>> +-m::
>> +       Show diffs for merge commits in the default format. This is
>> +       similar to '--diff-merges=on' (which see) except `-m` will
>> +       produce no output unless `-p` is given as well.
>
> I'm having difficulty grasping the parenthetical "(which see)" comment.

I am, too.  I know what it means when written in the more common
Latin abbreviation (q.v.), but I suspect it may be rare to spell it
in English like this.  I found

https://writingcenter.unc.edu/tips-and-tools/latin-terms-and-abbreviations/

that starts its explanation with this:

     The abbreviation q.v. stands for quod vide, which translates
     literally as “which see,” although in practice it mea
     something more like “for which see elsewhere.

and it goes on to say:

     The reader is expected to know how to locate this information
     without further assistance. Since there is always the
     possibility that the reader won’t be able to find the
     information cited by q.v., it’s better to use a simple English
     phrase such as “for more on this topic, see pages 72-3” or
     “a detailed definition appears on page 16.” Such phrases are
     immediately comprehensible to the reader (who may not even know
     what q.v. means) and remove any ambiguity about where
     additional information is located.

which only applies halfway to this example, as with the text before
it makes it very clear for readers that they need to learn about
"--diff-merges=on".  It is so clear to the point that the only
effect "(which see)" here has is to waste bytes and confuses
readers, I am afraid.
Junio C Hamano Oct. 5, 2023, 9:24 p.m. UTC | #4
Sergey Organov <sorganov@gmail.com> writes:

> ---diff-merges=(off|none|on|first-parent|1|separate|m|combined|c|dense-combined|cc|remerge|r)::
> +-m::
> +	Show diffs for merge commits in the default format. This is
> +	similar to '--diff-merges=on' (which see) except `-m` will
> +	produce no output unless `-p` is given as well.

I think the sentence reads better without the translated (q.v.) that
confused Eric.

> +-c::
> +	Produce combined diff output for merge commits.
> +	Shortcut for '--diff-merges=combined -p'.
> +
> +--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'.

I suspect that many people do not get what "re-merge" in "against
re-merge" really is.  As "combined diff" and "dense combined diff"
are not explained in the previous two entries either, and expect the
readers to read the real description (which more or less matches
what the original description for "-c" and "--cc" had, which is
good), it would be better to say "Produce remerge-diff output for
merge commits."  here, too.  It makes it consistent, and "for merge
commits" makes it clear the "magic" does not apply to regular
commits (which the above entries for "-c" and "--cc" do, which is
very good).

>  --no-diff-merges::
> +	Synonym for '--diff-merges=off'.
> +
> +--diff-merges=<format>::
>  	Specify diff format to be used for merge commits. Default is
> -	{diff-merges-default} unless `--first-parent` is in use, in which case
> -	`first-parent` is the default.
> +	{diff-merges-default} unless `--first-parent` is in use, in
> +	which case `first-parent` is the default.

This reads well.

In the longer term, "--diff-merge=first-parent" that is used without
first-parent traversal should be discouraged and be deprecated, I
think, but that is a separate story [*].

> ---diff-merges=(off|none):::
> ---no-diff-merges:::
> +The following formats are supported:
> ++
> +--
> +off, none::
>  	Disable output of diffs for merge commits. Useful to override
>  	implied value.
>  +
> ---diff-merges=on:::
> ---diff-merges=m:::
> --m:::
> -	This option makes diff output for merge commits to be shown in
> -	the default format. `-m` will produce the output only if `-p`
> -	is given as well. The default format could be changed using
> +on, m::
> +	Make diff output for merge commits to be shown in the default
> +	format. The default format could be changed using
>  	`log.diffMerges` configuration parameter, which default value
>  	is `separate`.

The original is already wrong so these are not problems this patch
introduces, but

 - "configuration variable" is how we refer to these entities.
 - "which default value" -> "whose default value".

> ---diff-merges=first-parent:::
> ---diff-merges=1:::
> -	This option makes merge commits show the full diff with
> -	respect to the first parent only.
> +first-parent, 1::
> +	Show full diff with respect to first parent. This is the same
> +	format as `--patch` produces for non-merge commits.
>  +

Yes, this is the same output as `-p`, as if parents other than the
first parent of the merge commit did not exist.

This was inherited from the original elsewhere, but it makes it
unnecessary confusing to say "full diff" here and in the next one.

    Show `--patch` output with respoect to the first parent for a
    merge commit, as if the other parents did not exist.

perhaps?

> +separate::
> +	Show full diff with respect to each of parents.
> +	Separate log entry and diff is generated for each parent.

In the early days of Git before -c/--cc were invented, we explained
this mode as "pairwise comparison", and the phrase "pairwise" still
may be the best one to describe the behaviour here.  In fact, we see
in the updated description of combined below the exact phrase is used
to refer to this oldest output format.

    Show the `--patch` output pairwise, together with the commit
    header, repeated for each parent for a merge commit.

or something, perhaps.  I added "repeated" here to make the contrast
with "simultaneously" stand out.

> +combined, c::
> +	Show differences from each of the parents to the merge
> +	result simultaneously instead of showing pairwise diff between
> +	a parent and the result one at a time. Furthermore, it lists
> +	only files which were modified from all parents.
> ++
> +dense-combined, cc::
> +	Further compress output produced by `--diff-merges=combined`
> +	by omitting uninteresting hunks whose contents in the parents
> +	have only two variants and the merge result picks one of them
> +	without modification.
> ++
> +remerge, r::
> +	Remerge two-parent merge commits to create a temporary tree
> +	object--potentially containing files with conflict markers
> +	and such.  A diff is then shown between that temporary tree
> +	and the actual merge commit.

The original says "two-parent merge comimts are remerged" so it is
not a failure of this patch, but the first verb "Remerge" sounds
unnecessarily unfriendly to the readers.

	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 `--patch` output
	then is shown between ...

would be easier to follow and more faithful to the original
description added by db757e8b (show, log: provide a --remerge-diff
capability, 2022-02-02).

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.

Looks very good otherwise.  Let me read on.

Thanks.


[Footnote]

* When a project allows fast-forward merges, something like this can
  happen (and Git was _designed_ to allow and even encourage it)

  - Linus pulls from Sergey and sees merge conflicts that are very
    messy.  Sergey is asked to resolve the conflict, as Linus knows
    Sergey understands the changes he is asking Linus to pull much
    better than Linus does.

  - Sergey does "git pull origin" that would give the same set of
    conflicts Linus saw, perhaps ours/theirs sides swapped, resolves
    the conflicts, and comits the merge result.  He may even add a
    few other improvements on top (or may not).  He tells Linus that
    his tree is ready to be pulled again.

  - Linus pulls from Sergey again.  This time it is fast-forward,
    without an extra merge commit that records the Linus's previous
    tip as the first parent and Sergey's work as the second parent.

  - Linus continues working from here.

  In such a workflow, merges are nothing more than "combining
  multiple histories together" and the first parenthood is NOT
  inherently special among parents at all.  The original "-m -p"
  (aka "pairwise diff") output reflects this world view and ensures
  that all parents are shown more or less as equals (yes, the first
  parent diff is shown first before the other parents, but you
  cannot avoid it when outputting to a single dimension medium).

  This world view was the only world view Git supported, until I
  added the "--first-parent" traversal in 0053e902 (git-log
  --first-parent: show only the first parent log, 2007-03-13).

  With the "--first-parent", with "--no-ff" option to "git merge", a
  different world view becomes possible.  A merge is not merely
  combining multiple histories, which are equals.  It is bringing
  work done on a side branch into the trunk.  To see the overview of
  the history, "git log --first-parent" would give the outline,
  which would be full of merges from side branches, each of which
  can be seen as summarizing the work done on the side branch that
  was merged, and it may occasionally have single-parent commits
  that are hotfixes or trivial clean-ups or project administrivia
  commits.  With "-p", "git log" would show the changes the work
  done on a side branch as a single unit for a merge, and individual
  commits if they are single-parent.  The life is good.

  It all breaks down if the "diff against the first parent" is done
  on a merge that is not bringing the work on a side branch in to
  the trunk.  The merge done in the second step Sergey did for Linus
  in the above example will have his work on the history leading to
  its first parent, and from the overall project's point of view,
  the second parent is the tip of the history of the trunk.  Showing
  first-parent diff for a merge that was *not* discovered via the
  first-parent traversal would show such a meaningless patch.  This
  is an illustration of the fallout from mixing two incompatible
  world views together, "--diff-merges=first-parent" wants to work
  in a world where the first-parent is special among parents, but
  traversal without "--first-parent" wants to treat all the branches
  equally.

  All the other <format>s accepted by the "--diff-merges=<format>"
  option are symmetrical and they work equally well when in a
  history of a project that considers the first-parenthood special
  (i.e. work on a side branch is brought into the trunk history) or
  in a history with merges whose parent order should not matter, so
  unlike "--diff-merges=first-parent", it makes sense to apply them
  with or without first-parent traversal.  It however is not true
  for the "--diff-merges=first-parent" variant, which is asymmetric.

  And that is why I think use of "--diff-merges=first-parent"
  without "--first-parent" traversal is a bad thing to teach users
  to use.
Elijah Newren Oct. 6, 2023, 2:41 p.m. UTC | #5
Hi,

On Thu, Oct 5, 2023 at 2:24 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Sergey Organov <sorganov@gmail.com> writes:
>
> > ---diff-merges=(off|none|on|first-parent|1|separate|m|combined|c|dense-combined|cc|remerge|r)::
> > +-m::
> > +     Show diffs for merge commits in the default format. This is
> > +     similar to '--diff-merges=on' (which see) except `-m` will
> > +     produce no output unless `-p` is given as well.
>
> I think the sentence reads better without the translated (q.v.) that
> confused Eric.

Agreed; confused me too.

> > +-c::
> > +     Produce combined diff output for merge commits.
> > +     Shortcut for '--diff-merges=combined -p'.
> > +
> > +--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'.
>
> I suspect that many people do not get what "re-merge" in "against
> re-merge" really is.  As "combined diff" and "dense combined diff"
> are not explained in the previous two entries either, and expect the
> readers to read the real description (which more or less matches
> what the original description for "-c" and "--cc" had, which is
> good), it would be better to say "Produce remerge-diff output for
> merge commits."  here, too.  It makes it consistent, and "for merge
> commits" makes it clear the "magic" does not apply to regular
> commits (which the above entries for "-c" and "--cc" do, which is
> very good).

Perhaps:

Produce remerge-diff output for merge commits, in order to show how
conflicts were resolved.

> > +separate::
> > +     Show full diff with respect to each of parents.
> > +     Separate log entry and diff is generated for each parent.
>
> In the early days of Git before -c/--cc were invented, we explained
> this mode as "pairwise comparison", and the phrase "pairwise" still
> may be the best one to describe the behaviour here.  In fact, we see
> in the updated description of combined below the exact phrase is used
> to refer to this oldest output format.
>
>     Show the `--patch` output pairwise, together with the commit
>     header, repeated for each parent for a merge commit.

I like this.

> or something, perhaps.  I added "repeated" here to make the contrast
> with "simultaneously" stand out.
>
> > +combined, c::
> > +     Show differences from each of the parents to the merge
> > +     result simultaneously instead of showing pairwise diff between
> > +     a parent and the result one at a time. Furthermore, it lists
> > +     only files which were modified from all parents.
> > ++
> > +dense-combined, cc::
> > +     Further compress output produced by `--diff-merges=combined`
> > +     by omitting uninteresting hunks whose contents in the parents
> > +     have only two variants and the merge result picks one of them
> > +     without modification.
> > ++
> > +remerge, r::
> > +     Remerge two-parent merge commits to create a temporary tree
> > +     object--potentially containing files with conflict markers
> > +     and such.  A diff is then shown between that temporary tree
> > +     and the actual merge commit.
>
> The original says "two-parent merge comimts are remerged" so it is
> not a failure of this patch, but the first verb "Remerge" sounds
> unnecessarily unfriendly to the readers.
>
>         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 `--patch` output
>         then is shown between ...
>
> would be easier to follow and more faithful to the original
> description added by db757e8b (show, log: provide a --remerge-diff
> capability, 2022-02-02).

I like it.  Perhaps it may also benefit from explaining why this mode
is useful as well:

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

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

> [Footnote]
>
> * When a project allows fast-forward merges, something like this can
>   happen (and Git was _designed_ to allow and even encourage it)
>
>   - Linus pulls from Sergey and sees merge conflicts that are very
>     messy.  Sergey is asked to resolve the conflict, as Linus knows
>     Sergey understands the changes he is asking Linus to pull much
>     better than Linus does.
>
>   - Sergey does "git pull origin" that would give the same set of
>     conflicts Linus saw, perhaps ours/theirs sides swapped, resolves
>     the conflicts, and comits the merge result.  He may even add a
>     few other improvements on top (or may not).  He tells Linus that
>     his tree is ready to be pulled again.
>
>   - Linus pulls from Sergey again.  This time it is fast-forward,
>     without an extra merge commit that records the Linus's previous
>     tip as the first parent and Sergey's work as the second parent.
>
>   - Linus continues working from here.
>
>   In such a workflow, merges are nothing more than "combining
>   multiple histories together" and the first parenthood is NOT
>   inherently special among parents at all.  The original "-m -p"
>   (aka "pairwise diff") output reflects this world view and ensures
>   that all parents are shown more or less as equals (yes, the first
>   parent diff is shown first before the other parents, but you
>   cannot avoid it when outputting to a single dimension medium).
>
>   This world view was the only world view Git supported, until I
>   added the "--first-parent" traversal in 0053e902 (git-log
>   --first-parent: show only the first parent log, 2007-03-13).
>
>   With the "--first-parent", with "--no-ff" option to "git merge", a
>   different world view becomes possible.  A merge is not merely
>   combining multiple histories, which are equals.  It is bringing
>   work done on a side branch into the trunk.  To see the overview of
>   the history, "git log --first-parent" would give the outline,
>   which would be full of merges from side branches, each of which
>   can be seen as summarizing the work done on the side branch that
>   was merged, and it may occasionally have single-parent commits
>   that are hotfixes or trivial clean-ups or project administrivia
>   commits.  With "-p", "git log" would show the changes the work
>   done on a side branch as a single unit for a merge, and individual
>   commits if they are single-parent.  The life is good.
>
>   It all breaks down if the "diff against the first parent" is done
>   on a merge that is not bringing the work on a side branch in to
>   the trunk.  The merge done in the second step Sergey did for Linus
>   in the above example will have his work on the history leading to
>   its first parent, and from the overall project's point of view,
>   the second parent is the tip of the history of the trunk.  Showing
>   first-parent diff for a merge that was *not* discovered via the
>   first-parent traversal would show such a meaningless patch.  This
>   is an illustration of the fallout from mixing two incompatible
>   world views together, "--diff-merges=first-parent" wants to work
>   in a world where the first-parent is special among parents, but
>   traversal without "--first-parent" wants to treat all the branches
>   equally.
>
>   All the other <format>s accepted by the "--diff-merges=<format>"
>   option are symmetrical and they work equally well when in a
>   history of a project that considers the first-parenthood special
>   (i.e. work on a side branch is brought into the trunk history) or
>   in a history with merges whose parent order should not matter, so
>   unlike "--diff-merges=first-parent", it makes sense to apply them
>   with or without first-parent traversal.  It however is not true
>   for the "--diff-merges=first-parent" variant, which is asymmetric.
>
>   And that is why I think use of "--diff-merges=first-parent"
>   without "--first-parent" traversal is a bad thing to teach users
>   to use.

Thanks for writing this up.  In the past, I didn't know how to put
into words why I didn't particularly care for this mode.  You explain
it rather well.
Sergey Organov Oct. 6, 2023, 5:02 p.m. UTC | #6
Junio C Hamano <gitster@pobox.com> writes:

> Eric Sunshine <sunshine@sunshineco.com> writes:
>
>>> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
>>> @@ -43,66 +43,74 @@ endif::git-diff[]
>>> +-m::
>>> +       Show diffs for merge commits in the default format. This is
>>> +       similar to '--diff-merges=on' (which see) except `-m` will
>>> +       produce no output unless `-p` is given as well.
>>
>> I'm having difficulty grasping the parenthetical "(which see)" comment.
>
> I am, too.  I know what it means when written in the more common
> Latin abbreviation (q.v.), but I suspect it may be rare to spell it
> in English like this.  I found

Well, I didn't invent it, and didn't lookup it until asked, it just
popped-up out of my head somehow.

I'll remove it as causes confusion.

Thanks,
-- Sergey Organov
Sergey Organov Oct. 6, 2023, 5:03 p.m. UTC | #7
Elijah Newren <newren@gmail.com> writes:

> Hi,
>
> On Thu, Oct 5, 2023 at 2:24 PM Junio C Hamano <gitster@pobox.com> wrote:
>>
>> Sergey Organov <sorganov@gmail.com> writes:
>>
>> > ---diff-merges=(off|none|on|first-parent|1|separate|m|combined|c|dense-combined|cc|remerge|r)::
>> > +-m::
>> > +     Show diffs for merge commits in the default format. This is
>> > +     similar to '--diff-merges=on' (which see) except `-m` will
>> > +     produce no output unless `-p` is given as well.
>>
>> I think the sentence reads better without the translated (q.v.) that
>> confused Eric.
>
> Agreed; confused me too.

Will remove, no problem.

Thanks,
-- Sergey Organov
Sergey Organov Oct. 6, 2023, 5:07 p.m. UTC | #8
Elijah Newren <newren@gmail.com> writes:

> Hi,
>
> On Thu, Oct 5, 2023 at 2:24 PM Junio C Hamano <gitster@pobox.com> wrote:
>>
>> Sergey Organov <sorganov@gmail.com> writes:
>> > +--remerge-diff::
>> > +     Produce diff against re-merge.
>> > +     Shortcut for '--diff-merges=remerge -p'.
>>
>> I suspect that many people do not get what "re-merge" in "against
>> re-merge" really is.  As "combined diff" and "dense combined diff"
>> are not explained in the previous two entries either, and expect the
>> readers to read the real description (which more or less matches
>> what the original description for "-c" and "--cc" had, which is
>> good), it would be better to say "Produce remerge-diff output for
>> merge commits."  here, too.  It makes it consistent, and "for merge
>> commits" makes it clear the "magic" does not apply to regular
>> commits (which the above entries for "-c" and "--cc" do, which is
>> very good).
>
> Perhaps:
>
> Produce remerge-diff output for merge commits, in order to show how
> conflicts were resolved.

Will use this description in re-roll.

>
>> > +separate::
>> > +     Show full diff with respect to each of parents.
>> > +     Separate log entry and diff is generated for each parent.
>>
>> In the early days of Git before -c/--cc were invented, we explained
>> this mode as "pairwise comparison", and the phrase "pairwise" still
>> may be the best one to describe the behaviour here.  In fact, we see
>> in the updated description of combined below the exact phrase is used
>> to refer to this oldest output format.
>>
>>     Show the `--patch` output pairwise, together with the commit
>>     header, repeated for each parent for a merge commit.
>
> I like this.
>
>> or something, perhaps.  I added "repeated" here to make the contrast
>> with "simultaneously" stand out.

Please let's left it for some follow-up, as this patch does not rephrase
original, just changes the presentation.

>>
>> > +combined, c::
>> > +     Show differences from each of the parents to the merge
>> > +     result simultaneously instead of showing pairwise diff between
>> > +     a parent and the result one at a time. Furthermore, it lists
>> > +     only files which were modified from all parents.
>> > ++
>> > +dense-combined, cc::
>> > +     Further compress output produced by `--diff-merges=combined`
>> > +     by omitting uninteresting hunks whose contents in the parents
>> > +     have only two variants and the merge result picks one of them
>> > +     without modification.
>> > ++
>> > +remerge, r::
>> > +     Remerge two-parent merge commits to create a temporary tree
>> > +     object--potentially containing files with conflict markers
>> > +     and such.  A diff is then shown between that temporary tree
>> > +     and the actual merge commit.
>>
>> The original says "two-parent merge comimts are remerged" so it is
>> not a failure of this patch, but the first verb "Remerge" sounds
>> unnecessarily unfriendly to the readers.
>>
>>         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 `--patch` output
>>         then is shown between ...
>>
>> would be easier to follow and more faithful to the original
>> description added by db757e8b (show, log: provide a --remerge-diff
>> capability, 2022-02-02).
>
> I like it.  Perhaps it may also benefit from explaining why this mode
> is useful as well:
>
>     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).
>
>> 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.
>
>> [Footnote]
>>
>> * When a project allows fast-forward merges, something like this can
>>   happen (and Git was _designed_ to allow and even encourage it)
>>
>>   - Linus pulls from Sergey and sees merge conflicts that are very
>>     messy.  Sergey is asked to resolve the conflict, as Linus knows
>>     Sergey understands the changes he is asking Linus to pull much
>>     better than Linus does.
>>
>>   - Sergey does "git pull origin" that would give the same set of
>>     conflicts Linus saw, perhaps ours/theirs sides swapped, resolves
>>     the conflicts, and comits the merge result.  He may even add a
>>     few other improvements on top (or may not).  He tells Linus that
>>     his tree is ready to be pulled again.
>>
>>   - Linus pulls from Sergey again.  This time it is fast-forward,
>>     without an extra merge commit that records the Linus's previous
>>     tip as the first parent and Sergey's work as the second parent.
>>
>>   - Linus continues working from here.
>>
>>   In such a workflow, merges are nothing more than "combining
>>   multiple histories together" and the first parenthood is NOT
>>   inherently special among parents at all.  The original "-m -p"
>>   (aka "pairwise diff") output reflects this world view and ensures
>>   that all parents are shown more or less as equals (yes, the first
>>   parent diff is shown first before the other parents, but you
>>   cannot avoid it when outputting to a single dimension medium).
>>
>>   This world view was the only world view Git supported, until I
>>   added the "--first-parent" traversal in 0053e902 (git-log
>>   --first-parent: show only the first parent log, 2007-03-13).
>>
>>   With the "--first-parent", with "--no-ff" option to "git merge", a
>>   different world view becomes possible.  A merge is not merely
>>   combining multiple histories, which are equals.  It is bringing
>>   work done on a side branch into the trunk.  To see the overview of
>>   the history, "git log --first-parent" would give the outline,
>>   which would be full of merges from side branches, each of which
>>   can be seen as summarizing the work done on the side branch that
>>   was merged, and it may occasionally have single-parent commits
>>   that are hotfixes or trivial clean-ups or project administrivia
>>   commits.  With "-p", "git log" would show the changes the work
>>   done on a side branch as a single unit for a merge, and individual
>>   commits if they are single-parent.  The life is good.
>>
>>   It all breaks down if the "diff against the first parent" is done
>>   on a merge that is not bringing the work on a side branch in to
>>   the trunk.  The merge done in the second step Sergey did for Linus
>>   in the above example will have his work on the history leading to
>>   its first parent, and from the overall project's point of view,
>>   the second parent is the tip of the history of the trunk.  Showing
>>   first-parent diff for a merge that was *not* discovered via the
>>   first-parent traversal would show such a meaningless patch.  This
>>   is an illustration of the fallout from mixing two incompatible
>>   world views together, "--diff-merges=first-parent" wants to work
>>   in a world where the first-parent is special among parents, but
>>   traversal without "--first-parent" wants to treat all the branches
>>   equally.
>>
>>   All the other <format>s accepted by the "--diff-merges=<format>"
>>   option are symmetrical and they work equally well when in a
>>   history of a project that considers the first-parenthood special
>>   (i.e. work on a side branch is brought into the trunk history) or
>>   in a history with merges whose parent order should not matter, so
>>   unlike "--diff-merges=first-parent", it makes sense to apply them
>>   with or without first-parent traversal.  It however is not true
>>   for the "--diff-merges=first-parent" variant, which is asymmetric.
>>
>>   And that is why I think use of "--diff-merges=first-parent"
>>   without "--first-parent" traversal is a bad thing to teach users
>>   to use.
>
> Thanks for writing this up.  In the past, I didn't know how to put
> into words why I didn't particularly care for this mode.  You explain
> it rather well.
Sergey Organov Oct. 6, 2023, 5:18 p.m. UTC | #9
Junio C Hamano <gitster@pobox.com> writes:

> Sergey Organov <sorganov@gmail.com> writes:

[...]

>>  --no-diff-merges::
>> +	Synonym for '--diff-merges=off'.
>> +
>> +--diff-merges=<format>::
>>  	Specify diff format to be used for merge commits. Default is
>> -	{diff-merges-default} unless `--first-parent` is in use, in which case
>> -	`first-parent` is the default.
>> +	{diff-merges-default} unless `--first-parent` is in use, in
>> +	which case `first-parent` is the default.
>
> This reads well.
>
> In the longer term, "--diff-merge=first-parent" that is used without
> first-parent traversal should be discouraged and be deprecated, I
> think, but that is a separate story [*].

I fail to see why useful harmless feature is to be deprecated. I believe
users are pretty capable to decide if they need it by themselves,
without our guidance.

Thanks,
-- Sergey Organov
Junio C Hamano Oct. 6, 2023, 6:01 p.m. UTC | #10
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.
Sergey Organov Oct. 6, 2023, 6:36 p.m. UTC | #11
Junio C Hamano <gitster@pobox.com> writes:

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

So, is it OK with both of you if I leave it as:

"Produce remerge-diff output for merge commits."

for now, and let you tweak the descriptions later on, if needed?

Thanks,
-- Sergey Organov
Sergey Organov Oct. 6, 2023, 6:42 p.m. UTC | #12
Junio C Hamano <gitster@pobox.com> writes:

> Sergey Organov <sorganov@gmail.com> writes:

[...]

>> +on, m::
>> +	Make diff output for merge commits to be shown in the default
>> +	format. The default format could be changed using
>>  	`log.diffMerges` configuration parameter, which default value
>>  	is `separate`.
>
> The original is already wrong so these are not problems this patch
> introduces, but
>
>  - "configuration variable" is how we refer to these entities.
>  - "which default value" -> "whose default value".

Added this amendment to the patch.

Thanks,
-- Sergey Organov
Junio C Hamano Oct. 6, 2023, 11:19 p.m. UTC | #13
Sergey Organov <sorganov@gmail.com> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> 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.
>
> So, is it OK with both of you if I leave it as:
>
> "Produce remerge-diff output for merge commits."
>
> for now, and let you tweak the descriptions later on, if needed?

I do not know what Elijah would say, but in one of iterations of my
draft response to him indeed suggested that "in order to" here is
not necessary if it is described for the "--diff-merges=remerge"
option, because those who know enough to skip referring to the other
entry are expected to know why it exists.  So I think I am OK with
that.

Thanks.
Elijah Newren Oct. 7, 2023, 1:31 a.m. UTC | #14
On Fri, Oct 6, 2023 at 11:01 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> 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.

The problem is it's really hard for me to come up with an answer to
that, in part because...

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

In my opinion, --remerge-diff does this better; wouldn't we want a
rationale where these particular modes shine?  Is that a non-empty
set?  (It may well be, but to me, --cc was never worse than -c while
often being better, and likewise, --remerge-diff is never worse than
--cc while often being better, at least on anything I had thought to
use any of these for.  Maybe there are other usecases for -c and --cc
I'm just not thinking of?)
Junio C Hamano Oct. 7, 2023, 1:50 a.m. UTC | #15
Elijah Newren <newren@gmail.com> writes:

> In my opinion, --remerge-diff does this better; wouldn't we want a
> rationale where these particular modes shine?  Is that a non-empty
> set?  (It may well be, but to me, --cc was never worse than -c while
> often being better, and likewise, --remerge-diff is never worse than
> --cc while often being better, at least on anything I had thought to
> use any of these for.  Maybe there are other usecases for -c and --cc
> I'm just not thinking of?)

Between -c and --cc, I do not think there is anything that makes us
favor -c over --cc.  While the algorithm to decide which hunks out
of -c's output to omit was being polished, comparison with -c served
a good way to give baseline, but once --cc has become solid, I do
not think I've used -c myself.

I personally find that a very trivial merge resolution is far easier
to read with --cc than --remerge-diff, the latter being way too
verbose.

Also, --cc and -c should work inside a read-only repository where
you only have read access to.  If remerge needs to write some
objects to the repository, then you'd need some hack to give a
writable object store overlay via the alternate odb mechanism, or
something, right?


$ git show --oneline --cc -U1 9fde277c338
9fde277c33 Merge branch 'cc/git-replay' into seen

diff --cc Makefile
index cf60c16deb,05a504dc28..c581c1ddba
--- a/Makefile
+++ b/Makefile
@@@ -803,4 -801,2 +803,3 @@@ TEST_BUILTINS_OBJS += test-env-helper.
  TEST_BUILTINS_OBJS += test-example-decorate.o
- TEST_BUILTINS_OBJS += test-fast-rebase.o
 +TEST_BUILTINS_OBJS += test-find-pack.o
  TEST_BUILTINS_OBJS += test-fsmonitor-client.o
diff --cc t/helper/test-tool.c
index 9010ac6de7,9ca1586de7..77b1d7c15d
--- a/t/helper/test-tool.c
+++ b/t/helper/test-tool.c
@@@ -32,4 -32,2 +32,3 @@@ static struct test_cmd cmds[] = 
  	{ "example-decorate", cmd__example_decorate },
- 	{ "fast-rebase", cmd__fast_rebase },
 +	{ "find-pack", cmd__find_pack },
  	{ "fsmonitor-client", cmd__fsmonitor_client },
diff --cc t/helper/test-tool.h
index f134f96b97,a03bbfc6b2..5deeca66fe
--- a/t/helper/test-tool.h
+++ b/t/helper/test-tool.h
@@@ -26,4 -26,2 +26,3 @@@ int cmd__env_helper(int argc, const cha
  int cmd__example_decorate(int argc, const char **argv);
- int cmd__fast_rebase(int argc, const char **argv);
 +int cmd__find_pack(int argc, const char **argv);
  int cmd__fsmonitor_client(int argc, const char **argv);
$ git show --oneline --remerge-diff -U1 9fde277c338
9fde277c33 Merge branch 'cc/git-replay' into seen
diff --git a/Makefile b/Makefile
remerge CONFLICT (content): Merge conflict in Makefile
index 987c8e3569..c581c1ddba 100644
--- a/Makefile
+++ b/Makefile
@@ -803,9 +803,3 @@ TEST_BUILTINS_OBJS += test-env-helper.o
 TEST_BUILTINS_OBJS += test-example-decorate.o
-<<<<<<< 0fd7a144c5 (Merge branch 'js/doc-unit-tests-with-cmake' into seen)
-TEST_BUILTINS_OBJS += test-fast-rebase.o
 TEST_BUILTINS_OBJS += test-find-pack.o
-||||||| 1fc548b2d6
-TEST_BUILTINS_OBJS += test-fast-rebase.o
-=======
->>>>>>> 0b853ad4db (replay: stop assuming replayed branches do not diverge)
 TEST_BUILTINS_OBJS += test-fsmonitor-client.o
diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
remerge CONFLICT (content): Merge conflict in t/helper/test-tool.c
index 87a9794564..77b1d7c15d 100644
--- a/t/helper/test-tool.c
+++ b/t/helper/test-tool.c
@@ -32,9 +32,3 @@ static struct test_cmd cmds[] = {
 	{ "example-decorate", cmd__example_decorate },
-<<<<<<< 0fd7a144c5 (Merge branch 'js/doc-unit-tests-with-cmake' into seen)
-	{ "fast-rebase", cmd__fast_rebase },
 	{ "find-pack", cmd__find_pack },
-||||||| 1fc548b2d6
-	{ "fast-rebase", cmd__fast_rebase },
-=======
->>>>>>> 0b853ad4db (replay: stop assuming replayed branches do not diverge)
 	{ "fsmonitor-client", cmd__fsmonitor_client },
diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h
remerge CONFLICT (content): Merge conflict in t/helper/test-tool.h
index e8abf4c42f..5deeca66fe 100644
--- a/t/helper/test-tool.h
+++ b/t/helper/test-tool.h
@@ -26,9 +26,3 @@ int cmd__env_helper(int argc, const char **argv);
 int cmd__example_decorate(int argc, const char **argv);
-<<<<<<< 0fd7a144c5 (Merge branch 'js/doc-unit-tests-with-cmake' into seen)
-int cmd__fast_rebase(int argc, const char **argv);
 int cmd__find_pack(int argc, const char **argv);
-||||||| 1fc548b2d6
-int cmd__fast_rebase(int argc, const char **argv);
-=======
->>>>>>> 0b853ad4db (replay: stop assuming replayed branches do not diverge)
 int cmd__fsmonitor_client(int argc, const char **argv);
Junio C Hamano Oct. 7, 2023, 6:49 a.m. UTC | #16
Junio C Hamano <gitster@pobox.com> writes:

> Elijah Newren <newren@gmail.com> writes:
>
>> In my opinion, --remerge-diff does this better; wouldn't we want a
>> ...
> I personally find that a very trivial merge resolution is far easier
> to read with --cc than --remerge-diff, the latter being way too
> verbose.
>
> Also, --cc and -c should work inside a read-only repository where
> you only have read access to.  If remerge needs to write some
> objects to the repository, then you'd need some hack to give a
> writable object store overlay via the alternate odb mechanism, or
> something, right?

Well, the above did not come out as well as I intended, as I forgot
to prefix it with something I thought was obvious from what I said
in the recent discussion in the earlier iteration of this topic,
where I said that it would be "--remerge-diff", if I were to pick an
option that is so useful that it deserves short and sweet single
letter.  Narutally, it came after we gained experience with "--cc",
so it would be surprising if it did worse.  Just like it is natural
to expect that "--cc" would give more useful output than "-m -p"
that predates everybody else.

In short, I would say "--remerge-diff" would give output that is the
easiest to grok among the three modern variants to show the changes
a merge introduces.

The above two cases, where I said cc does better than remerge-diff,
were meant as _exceptions_ for that general sentiment.
Elijah Newren Oct. 9, 2023, 5:04 p.m. UTC | #17
On Fri, Oct 6, 2023 at 11:49 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Junio C Hamano <gitster@pobox.com> writes:
>
> > Elijah Newren <newren@gmail.com> writes:
> >
> >> In my opinion, --remerge-diff does this better; wouldn't we want a
> >> ...
> > Between -c and --cc, I do not think there is anything that makes us
> > favor -c over --cc.  While the algorithm to decide which hunks out
> > of -c's output to omit was being polished, comparison with -c served
> > a good way to give baseline, but once --cc has become solid, I do
> > not think I've used -c myself.

Perhaps, then, the user manual should either omit -c, or recommend
users use --cc instead?

> > I personally find that a very trivial merge resolution is far easier
> > to read with --cc than --remerge-diff, the latter being way too
> > verbose.

Ah, indeed, for those that know the --cc output format well (it takes
a bit to figure out for newcomers), your example demonstrates this
nicely.  Thanks.

> > Also, --cc and -c should work inside a read-only repository where
> > you only have read access to.  If remerge needs to write some
> > objects to the repository, then you'd need some hack to give a
> > writable object store overlay via the alternate odb mechanism, or
> > something, right?

Well, it does use a temporary object store with the alternate odb
mechanism already, but I don't think there's any code to allow the
user to input the location for the temporary store, and thus we'd
probably attempt to write it underneath the same read-only directory.
So, yes, read-only repositories would likely be problematic for
--remerge-diff.

However, are read-only repositories worth mentioning in the documentation here?

> Well, the above did not come out as well as I intended, as I forgot
> to prefix it with something I thought was obvious from what I said
> in the recent discussion in the earlier iteration of this topic,
> where I said that it would be "--remerge-diff", if I were to pick an
> option that is so useful that it deserves short and sweet single
> letter.  Narutally, it came after we gained experience with "--cc",
> so it would be surprising if it did worse.  Just like it is natural
> to expect that "--cc" would give more useful output than "-m -p"
> that predates everybody else.
>
> In short, I would say "--remerge-diff" would give output that is the
> easiest to grok among the three modern variants to show the changes
> a merge introduces.
>
> The above two cases, where I said cc does better than remerge-diff,
> were meant as _exceptions_ for that general sentiment.

Thanks, this is useful.  This does make me wonder, though: Should we
perhaps guide users as to what we recommend (and recommend against) in
this documentation?

If we have lots of options and they all shine on different usecases,
it makes sense to just provide a long list of possibilities for users.
But if we generally feel that one is entirely supplanted by another
(e.g. -c by --cc) it seems beneficial to mention that, and if we
generally feel that one will often be clearer or more useful than the
others (e.g. --remerge-diff), it seems beneficial to recommend it.
Thoughts?

Also, perhaps this would be best to include in a follow-up series (as
it appears from Sergey's latest iteration that we are leaving other
tweaks for a later series anyway), if we do decide we want to do it...
Junio C Hamano Oct. 10, 2023, 12:24 a.m. UTC | #18
Elijah Newren <newren@gmail.com> writes:

>> >> In my opinion, --remerge-diff does this better; wouldn't we want a
>> >> ...
>> > Between -c and --cc, I do not think there is anything that makes us
>> > favor -c over --cc.  While the algorithm to decide which hunks out
>> > of -c's output to omit was being polished, comparison with -c served
>> > a good way to give baseline, but once --cc has become solid, I do
>> > not think I've used -c myself.
>
> Perhaps, then, the user manual should either omit -c, or recommend
> users use --cc instead?

I do not think I'd miss "-c", but I do not know about others.

>> > I personally find that a very trivial merge resolution is far easier
>> > to read with --cc than --remerge-diff, the latter being way too
>> > verbose.
>
> Ah, indeed, for those that know the --cc output format well (it takes
> a bit to figure out for newcomers), your example demonstrates this
> nicely.  Thanks.

Yup.  And newcomers would take a bit to figure out remerge-diff
output, too, so my answers were written from the "nobody will stay
newcomer forever.  now once they get proficient enough, which ones
are good for them" viewpoint.
Junio C Hamano Oct. 10, 2023, 2:44 a.m. UTC | #19
Elijah Newren <newren@gmail.com> writes:

>> [Footnote]
>> ...
>
> Thanks for writing this up.  In the past, I didn't know how to put
> into words why I didn't particularly care for this mode.  You explain
> it rather well.

I am glad it helped non-zero number of people.

It probably owes big to my failing, but because we strongly view it
a virtue not to be opinionated, we have many discrete tools and
features that can be used in combination to support a workflow well,
even when some of these tools and features are not useful for a
different workflow.  It indeed is a good thing to be flexible and to
support different workflows well, and we tend not to single out a
workflow among many and advocate it, but because our documentation
lacks description of major possible workflows, what their underlying
philosophies and their strengths are, how some of our tools and
features support them, and why some others are not good fit.  Being
given a toolbox with too many tools without being taught how they
are to be used together and for what purpose may be a fun puzzle to
figure out for tinkerers, but when you have a problem to solve and
tinkering is not your main focus, which is true for most people, it
is not fun.  

In short, in pursuit of not to be opinionated, we fail to give the
readers best current practices.  The first place to start rectifying
it might be to have some write-ups for various major workflows and
the worldview behind them.  The importance given to first-parenthood
offers two quite different worldviews that affects the choice of
tools (e.g. "merge --no-ff", "checkout origin/master && merge mine
&& branch -f mine" aka "reverse merge").

I suspect that this also relates to your "would --cc be totally
unnecessary now we have --remerge-diff?" as well.  What kind of
conflicts are interesting highly depends on what you are looking
for, which in turn is influenced by the workflow employed by the
project and what role you are playing in it.
Emily Shaffer Oct. 10, 2023, 2:58 p.m. UTC | #20
On Mon, Oct 9, 2023 at 7:44 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Elijah Newren <newren@gmail.com> writes:
>
> >> [Footnote]
> >> ...
> >
> > Thanks for writing this up.  In the past, I didn't know how to put
> > into words why I didn't particularly care for this mode.  You explain
> > it rather well.
>
> I am glad it helped non-zero number of people.
>
> It probably owes big to my failing, but because we strongly view it
> a virtue not to be opinionated, we have many discrete tools and
> features that can be used in combination to support a workflow well,
> even when some of these tools and features are not useful for a
> different workflow.  It indeed is a good thing to be flexible and to
> support different workflows well, and we tend not to single out a
> workflow among many and advocate it, but because our documentation
> lacks description of major possible workflows, what their underlying
> philosophies and their strengths are, how some of our tools and
> features support them, and why some others are not good fit.  Being
> given a toolbox with too many tools without being taught how they
> are to be used together and for what purpose may be a fun puzzle to
> figure out for tinkerers, but when you have a problem to solve and
> tinkering is not your main focus, which is true for most people, it
> is not fun.
>
> In short, in pursuit of not to be opinionated, we fail to give the
> readers best current practices.  The first place to start rectifying
> it might be to have some write-ups for various major workflows and
> the worldview behind them.

I completely agree with you. The #1 conversation I have with friends
who are new to Git is figuring out which of the major workflows they
should use, and what are the drawbacks and benefits. And how to
diagnose based on the existing commit history, review tool in use, etc
which one is used by their peers. Having this documented somewhere -
maybe in Pro Git book, maybe in manpage - would be hugely useful. I
could envision a diagram of a sample commit history, and "if it
already looks like this or you want it to look like this, your
workflow should be blah" and finally a list of some drawbacks,
benefits, and warnings about what to avoid in that workflow. Plus,
perhaps, many shout-outs to `git reflog` for fixing when you
misunderstood and made a mess. (that's the #2 conversation I have :) )

> The importance given to first-parenthood
> offers two quite different worldviews that affects the choice of
> tools (e.g. "merge --no-ff", "checkout origin/master && merge mine
> && branch -f mine" aka "reverse merge").
>
> I suspect that this also relates to your "would --cc be totally
> unnecessary now we have --remerge-diff?" as well.  What kind of
> conflicts are interesting highly depends on what you are looking
> for, which in turn is influenced by the workflow employed by the
> project and what role you are playing in it.
diff mbox series

Patch

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 9f33f887711d..8035210c1418 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -43,66 +43,74 @@  endif::git-diff[]
 endif::git-format-patch[]
 
 ifdef::git-log[]
---diff-merges=(off|none|on|first-parent|1|separate|m|combined|c|dense-combined|cc|remerge|r)::
+-m::
+	Show diffs for merge commits in the default format. This is
+	similar to '--diff-merges=on' (which see) except `-m` will
+	produce no output unless `-p` is given as well.
+
+-c::
+	Produce combined diff output for merge commits.
+	Shortcut for '--diff-merges=combined -p'.
+
+--cc::
+	Produce dense combined diff output for merge commits.
+	Shortcut for '--diff-merges=dense-combined -p'.
+
+--remerge-diff::
+	Produce diff against re-merge.
+	Shortcut for '--diff-merges=remerge -p'.
+
 --no-diff-merges::
+	Synonym for '--diff-merges=off'.
+
+--diff-merges=<format>::
 	Specify diff format to be used for merge commits. Default is
-	{diff-merges-default} unless `--first-parent` is in use, in which case
-	`first-parent` is the default.
+	{diff-merges-default} unless `--first-parent` is in use, in
+	which case `first-parent` is the default.
 +
---diff-merges=(off|none):::
---no-diff-merges:::
+The following formats are supported:
++
+--
+off, none::
 	Disable output of diffs for merge commits. Useful to override
 	implied value.
 +
---diff-merges=on:::
---diff-merges=m:::
--m:::
-	This option makes diff output for merge commits to be shown in
-	the default format. `-m` will produce the output only if `-p`
-	is given as well. The default format could be changed using
+on, m::
+	Make diff output for merge commits to be shown in the default
+	format. The default format could be changed using
 	`log.diffMerges` configuration parameter, which default value
 	is `separate`.
 +
---diff-merges=first-parent:::
---diff-merges=1:::
-	This option makes merge commits show the full diff with
-	respect to the first parent only.
+first-parent, 1::
+	Show full diff with respect to first parent. This is the same
+	format as `--patch` produces for non-merge commits.
 +
---diff-merges=separate:::
-	This makes merge commits show the full diff with respect to
-	each of the parents. Separate log entry and diff is generated
-	for each parent.
+separate::
+	Show full diff with respect to each of parents.
+	Separate log entry and diff is generated for each parent.
 +
---diff-merges=remerge:::
---diff-merges=r:::
---remerge-diff:::
-	With this option, two-parent merge commits are remerged to
-	create a temporary tree object -- potentially containing files
-	with conflict markers and such.  A diff is then shown between
-	that temporary tree and the actual merge commit.
+combined, c::
+	Show differences from each of the parents to the merge
+	result simultaneously instead of showing pairwise diff between
+	a parent and the result one at a time. Furthermore, it lists
+	only files which were modified from all parents.
++
+dense-combined, cc::
+	Further compress output produced by `--diff-merges=combined`
+	by omitting uninteresting hunks whose contents in the parents
+	have only two variants and the merge result picks one of them
+	without modification.
++
+remerge, r::
+	Remerge two-parent merge commits to create a temporary tree
+	object--potentially containing files with conflict markers
+	and such.  A diff is then shown between that temporary tree
+	and the actual merge commit.
 +
 The output emitted when this option is used is subject to change, and
 so is its interaction with other options (unless explicitly
 documented).
-+
---diff-merges=combined:::
---diff-merges=c:::
--c:::
-	With this option, diff output for a merge commit shows the
-	differences from each of the parents to the merge result
-	simultaneously instead of showing pairwise diff between a
-	parent and the result one at a time. Furthermore, it lists
-	only files which were modified from all parents. `-c` implies
-	`-p`.
-+
---diff-merges=dense-combined:::
---diff-merges=cc:::
---cc:::
-	With this option the output produced by
-	`--diff-merges=combined` is further compressed by omitting
-	uninteresting hunks whose contents in the parents have only
-	two variants and the merge result picks one of them without
-	modification.  `--cc` implies `-p`.
+--
 
 --combined-all-paths::
 	This flag causes combined diffs (used for merge commits) to
diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 2a66cf888074..9b7ec96e767a 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -124,7 +124,7 @@  Note that unless one of `--diff-merges` variants (including short
 will not show a diff, even if a diff format like `--patch` is
 selected, nor will they match search options like `-S`. The exception
 is when `--first-parent` is in use, in which case `first-parent` is
-the default format.
+the default format for merge commits.
 
 :git-log: 1
 :diff-merges-default: `off`