[PATCH 0/2] diff-merges: introduce '-d' option

[PATCH 0/2] diff-merges: introduce '-d' option

[Sergey Organov]

This new convenience option requests full diff with respect to first
parent, so that

  git log -d

will output diff with respect to first parent for every commit,
universally, no matter how many parents the commit turns out to have.

It's implemented as pure synonym for

  --diff-merges=first-parent --patch

The first commit in the series tweaks diff-merges documentation a bit,
and is valuable by itself. It's put here as '-d' implementation commit
depends on it in its documentation part.

Note: the need for this new convenience option mostly emerged from
denial by the community of patches that modify '-m' behavior to imply
'-p' as the rest of similar options (such as --cc) do.

Sergey Organov (2):
  diff-merges: improve --diff-merges documentation
  diff-merges: introduce '-d' option

 Documentation/diff-options.txt | 101 +++++++++++++++++++--------------
 Documentation/git-log.txt      |   4 +-
 diff-merges.c                  |   3 +
 t/t4013-diff-various.sh        |   8 +++
 4 files changed, 71 insertions(+), 45 deletions(-)

-- 
2.25.1

[PATCH 1/2] diff-merges: improve --diff-merges documentation

[Sergey Organov]

* Put descriptions of convenience shortcuts first, so they are the
  first things reader observes, not lengthy stuff.

* Add explanation note on '-m' not implying '-p' unlike similar
  options.

* 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 | 97 +++++++++++++++++++---------------
 Documentation/git-log.txt      |  2 +-
 2 files changed, 55 insertions(+), 44 deletions(-)

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 9f33f887711d..f93aa3e46a52 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -43,66 +43,77 @@ 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.
++
+Note: This option not implying `-p` is legacy feature that is
+preserved for the sake of backward compatibility.
+
+-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.
+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`.
+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.
 +
---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`.
+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.
+--
 
 --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`
-- 
2.25.1

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

[Junio C Hamano]

Sergey Organov <sorganov@gmail.com> writes:

>  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.
> ++
> +Note: This option not implying `-p` is legacy feature that is
> +preserved for the sake of backward compatibility.

It is more like that `-p` does not imply `-m` (which used to mean
"consider showing the comparison between parent(s) and the child,
even for merge commits"), even though newer options like `-c`,
`--cc` and others do imply `-m` (simply because they do not make
much sense if they are not allowed to work on merges) that may make
new people confused.  If `-p` implied `-m` (or if `-m` implied
`-p`), it would also have been utterly confusing and useless for
human consumption.

In either case, unless the reason why `-p` does not imply `-m`
unlike others is explained, I do not think the note adds that much
value.  I'd suggest dropping it.

>  --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.
>  +
> +The following formats are supported:
> ++
> +--
> +off, none::
>  	Disable output of diffs for merge commits. Useful to override
>  	implied value.
>  +
> +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`.
>  +
> +first-parent, 1::
> +	Show full diff with respect to first parent. This is the same
> +	format as `--patch` produces for non-merge commits.
>  +
> +separate::
> +	Show full diff with respect to each of parents.
> +	Separate log entry and diff is generated for each parent.
>  +
> +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).
>  +
> +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.
> +--

Looks reasonable, even though I didn't quite see much problem with
the original.  If we were shuffling the sections like this patch, I
wonder if moving combined/dense-combined a bit higher (perhaps
before the "remerge") may make more sense, though (the ordering
would simply become "simpler to more involved").

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

[Sergey Organov]

Junio C Hamano <gitster@pobox.com> writes:

> Sergey Organov <sorganov@gmail.com> writes:
>
>>  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.
>> ++
>> +Note: This option not implying `-p` is legacy feature that is
>> +preserved for the sake of backward compatibility.
>
> It is more like that `-p` does not imply `-m` (which used to mean
> "consider showing the comparison between parent(s) and the child,
> even for merge commits"), even though newer options like `-c`,
> `--cc` and others do imply `-m` (simply because they do not make
> much sense if they are not allowed to work on merges) that may make
> new people confused.

No, neither --cc nor -c imply -m.

-m is documented to produce very specific output that is neither -c nor
--cc, and it's indeed how it works.

-c and --cc imply -p, not -m, and it has been documented for ages
already, and it's indeed how it works, and that's what corresponding
commits that added the features claim.

Overall, --cc, -c, and --remerge-diff all imply -p, whereas -m does not.
This is simple fact.

So I feel we need to document why -m doesn't imply -p as other similar
options do.

> If `-p` implied `-m` (or if `-m` implied
> `-p`), it would also have been utterly confusing and useless for
> human consumption.

Fortunately, -p does not imply -m, but if -m implied -p, similar to --cc
and -c, it'd be rather very natural, and thus people keep asking why
it's not the case.

> In either case, unless the reason why `-p` does not imply `-m`
> unlike others is explained, I do not think the note adds that much
> value.  I'd suggest dropping it.

-p does not imply others. It's others (--cc, etc.) that imply -p.

The problem being solved is that we periodically get (valid) questions
why -m does not behave similar to -c and --cc, and now --remerge-diff.

>
>>  --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.
>>  +
>> +The following formats are supported:
>> ++
>> +--
>> +off, none::
>>  	Disable output of diffs for merge commits. Useful to override
>>  	implied value.
>>  +
>> +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`.
>>  +
>> +first-parent, 1::
>> +	Show full diff with respect to first parent. This is the same
>> +	format as `--patch` produces for non-merge commits.
>>  +
>> +separate::
>> +	Show full diff with respect to each of parents.
>> +	Separate log entry and diff is generated for each parent.
>>  +
>> +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).
>>  +
>> +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.
>> +--
>
> Looks reasonable, even though I didn't quite see much problem with
> the original.

The original --diff-merge=... line was so long it didn't fit, especially
after "remerge" has been added, and also was hard to grok.

> If we were shuffling the sections like this patch, I
> wonder if moving combined/dense-combined a bit higher (perhaps
> before the "remerge") may make more sense, though (the ordering
> would simply become "simpler to more involved").

I kept original order, but I agree combined/dense-combined fit better
above remerge.

I'll change the order in re-roll.

Thanks,
-- Sergey Organov

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

[Junio C Hamano]

Sergey Organov <sorganov@gmail.com> writes:

>> It is more like that `-p` does not imply `-m` (which used to mean
>> "consider showing the comparison between parent(s) and the child,
>> even for merge commits"), even though newer options like `-c`,
>> `--cc` and others do imply `-m` (simply because they do not make
>> much sense if they are not allowed to work on merges) that may make
>> new people confused.
>
> No, neither --cc nor -c imply -m.

I was only trying to help you polish the text you added to explain
what you called the "legacy feature" to reflect the reason behind
that legacy.  As you obviously were not there back then when I made
"--cc" imply "-m" while keeping "-p" not to imply "-m".

Our "git log [--diff-output-options]" logic was (and still is) not
to show the comparison between parents and the child by default for
merge commits even with -p/--raw/--stat (these were what existed and
were common back then) and "git log --raw/--stat/-p" showed the
raw/diffstat/patch after the log message for one-parent commits but
only the log message for merges.

The reason behind that design choice is that Linus (and I and
others) did not find that the patches for merges are as useful as
patches for regular commits.  We made "git log -p" to omit
patches for merges that tend to become large.

	Side note: the first-parent patch is sort of readable, but
	if you are not doing the "--first-parent" traversal (which
	is a much later invention, so it wasn't even an option), you
	are showing individual commits and their patches while
	traversing the side branch, then the first-parent patch of a
	merge amounts to the squash of individual changes on the
	side branch that got merged.  It was deemed redundant
	presentation that is just wasteful and harder to grok than
	reading individual commits.  Worse, the patch against second
	and later parent(s) have no real value (it shows how behind
	the fork point of the side branch was relative to the tip of
	the trunk, which is rarely useful).

But we also wanted to have a mode of "git log -p" that spews
everything to the output that could be used to reconstruct the
history, hence we added "-m" to tell "git log":

	By default, you are designed not to show comparison between
	parents and the child for merge commit.  But when "-m" is
	given, do show the comparison for merge commit in the format
	that other options given to you, like --raw, --patch,
	specifies.

We however didn't have a good idea how to represent such a
comparison between parents and the child, so we chose the most
redundant, verbose, and obvious, which is N pairwise patches with
each of N parents to the child (for a N-parent patch).

Later "--cc" and "-c" came as an alternative way to represent
comparison between parents and the child.

Given that I, together with Linus, invented "--cc" and "-c", taking
inspiration from how Paul Mackerras showed a merge in his 'gitk'
tool, and made the design decision not to require "-m" to get the
output in the format they specify when the "git log" traversal shows
merge commits, I do not know what to say when you repeat that "--cc"
does not imply "-m".  It simply is not true.

I think this is the second time you claimed the above after I
explained the same to you, if I am not mistaken.  If you do not want
to be corrected, that is fine, and I'll stop wasting my time trying
to correct you.

But I still have to make sure that you (or anybody else) do not
spread misinformation to other users by writing incorrect statements
in documentation patches.

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

[Sergey Organov]

Junio C Hamano <gitster@pobox.com> writes:

> Sergey Organov <sorganov@gmail.com> writes:
>
>>> It is more like that `-p` does not imply `-m` (which used to mean
>>> "consider showing the comparison between parent(s) and the child,
>>> even for merge commits"), even though newer options like `-c`,
>>> `--cc` and others do imply `-m` (simply because they do not make
>>> much sense if they are not allowed to work on merges) that may make
>>> new people confused.
>>
>> No, neither --cc nor -c imply -m.
>
> I was only trying to help you polish the text you added to explain
> what you called the "legacy feature" to reflect the reason behind
> that legacy.  As you obviously were not there back then when I made
> "--cc" imply "-m" while keeping "-p" not to imply "-m".

Your help is appreciated, yet unfortunately I still can't figure how to
improve the text based on your advice.

Your "I made --cc imply -m" does not explain why later, when you made
--cc imply -p (did you, or was it somebody else?), you didn't make -m
imply -p at the same time, and then "while keeping -p not to imply -m"
sounds out of place as we rather try to figure why "-m not implies -p".

The "--c imply -m" part of the help raises yet another question: if --cc
implied -m, why it was not -m that was made to imply -p instead of --cc
(and -c)? Then both --cc and -c would imply -p automatically as a
side-effect of implication of -p by -m (do not confuse with agreed
non-implication of -m by -p), and then all the relevant options were
consistent. This consideration renders current situation more surprising
instead of clarifying it, I'm afraid.

"-p does not imply -m" fact is fine with me and is not the cause of user
confusion I'm trying to address. How does it help us to explain why "-m
does not imply -p" though?

[...]

>
> Given that I, together with Linus, invented "--cc" and "-c", taking
> inspiration from how Paul Mackerras showed a merge in his 'gitk'
> tool, and made the design decision not to require "-m" to get the
> output in the format they specify when the "git log" traversal shows
> merge commits, I do not know what to say when you repeat that "--cc"
> does not imply "-m".  It simply is not true.

I keep saying "--cc does not imply -m" because it does not seem to,
unless you either use some vague meaning of "imply", or mean some other
"-m", not the one used in "git log". Please check:

$ cd src/git
$ git --version
git version 2.42.0.111.gd814540bb75b
$ git describe
v2.42.0-111-gd814540bb75b
$ git log 74a2e88700efc -n1 -p --cc > diff.actual
$ git log 74a2e88700efc -n1 -p --cc -m > diff.expected
$ cmp diff.expected diff.actual
diff.expected diff.actual differ: byte 706, line 18
$

This test tells us that "--c" is not the same as "--cc -m", that for me
in turn reads "--cc does not imply -m", and that's what I continue to
say.

>
> I think this is the second time you claimed the above after I
> explained the same to you, if I am not mistaken.  If you do not want
> to be corrected, that is fine, and I'll stop wasting my time trying
> to correct you.

I'd love to be corrected, but I think I carefully checked my grounds
before saying that --cc does not imply -m, please consider:

1. "--cc implies -m" is not documented. Please point to the
   documentation in case I missed it.

2. Git does not behave as if "--cc implied -m", see the test-case above.

If it's neither documented nor matches actual behavior, it's not there,
at least from the POV of random user, to whom my original clarification
of "why -m does not imply -p?" has been addressed.

On top of that, I even can't figure why we argue about it in the first
place, as it seems to be irrelevant to the issue at hand: explain why -m
does not imply -p?

>
> But I still have to make sure that you (or anybody else) do not
> spread misinformation to other users by writing incorrect statements
> in documentation patches.

I'm all against spreading misinformation, and try my best to avoid it
myself. I still fail to see what misinformation, exactly, you find in
this particular explanation by me:

" Note: This option [`-m`] not implying `-p` is legacy feature that is
  preserved for the sake of backward compatibility. "

That's exactly what I figured out from a lot of discussions over my
multiple attempts to make `-m` behave more usefully. Is it that "legacy
feature" somehow sounds offensive, or what?

As, despite your help, I fail to come up with better edition of the
note, please, if you feel like it, suggest your own variant of
explanation to the user why `-m` is left inconsistent with the rest of
diff for merges options, provided current matching documentation reads
roughly like this (from more recent options to oldest):

   --remrege-diff: produces "remerge" output. Implies -p.
   --cc: produces dense combined output. Implies -p.
   -c: produces combined output. Implies -p.
   -m: produces separate output, provided -p is given as well (?!).

and so why

  git log -m

surprisingly has no visible effect, and then the user needs to
type:

   git log -m -p


That's all I wanted to explain to the user in a few words with the note
you argue against.

Thanks,
-- Sergey Organov

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

[Junio C Hamano]

Sergey Organov <sorganov@gmail.com> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> I was only trying to help you polish the text you added to explain
>> what you called the "legacy feature" to reflect the reason behind
>> that legacy.  As you obviously were not there back then when I made
>> "--cc" imply "-m" while keeping "-p" not to imply "-m".
>
> Your help is appreciated, yet unfortunately I still can't figure how to
> improve the text based on your advice.

If I were doing this patch, I would start from something like this:

-m::
	By default, comparisons between parent commits and the child
	commit are not shown for merge commits, but with the `-m`
	option, `git log` can be told to show comparisons for merges
	in chosen formats (e.g. `--raw`, `-p`, `--stat`).  When
	output formats (e.g. `--cc`) that are specifically designed
	to show better comparisons for merges are given, this option
	is implied; in other words, you do not have to say e.g. `git
	log -m --cc`.  `git log --cc` suffices.


The rest is a tangent that is not related to the above.  I suspect
that this also applies to newer `--remerge-diff`, as it also targets
to show merges better than the original "pairwise patches" that were
largely useless, but the right way to view what `--cc` and other
formats do for non-merge commits is *not* to think that they "imply"
`-p`.  It is more like that the output from these formats on
non-merge commits happen to be identical to what `-p` would produce.
You could say that the "magic" these options know to show merge
commits better degenerates to what `-p` gives when applied to
non-merge commits.

Another way to look at it is that `--cc` and friends, even though
they are meant as improvements for showing merges over "-m -p" that
gives human-unreadable pair-wise diffs, do not imply "--merges"
(i.e. show only merge commits)---hence they have to show something
for non-merge commits.  Because output formats for all of them were
modeled loosely [*] after "-p" output, we happened to pick it as the
format they fall back to when they are not showing comparisons for
merge commits.


[Footnote]

 * Here, `-p` roughly means "what GNU patch and `git apply` take".
   Output from `-c` and `--cc` on merge commits do not qualify, but
   they are loosely modeled after it.

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

[Sergey Organov]

Junio C Hamano <gitster@pobox.com> writes:

> Sergey Organov <sorganov@gmail.com> writes:
>
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>> I was only trying to help you polish the text you added to explain
>>> what you called the "legacy feature" to reflect the reason behind
>>> that legacy.  As you obviously were not there back then when I made
>>> "--cc" imply "-m" while keeping "-p" not to imply "-m".
>>
>> Your help is appreciated, yet unfortunately I still can't figure how to
>> improve the text based on your advice.
>
> If I were doing this patch, I would start from something like this:
>
> -m::
> 	By default, comparisons between parent commits and the child
> 	commit are not shown for merge commits, but with the `-m`
> 	option, `git log` can be told to show comparisons for merges
> 	in chosen formats (e.g. `--raw`, `-p`, `--stat`).  When
> 	output formats (e.g. `--cc`) that are specifically designed
> 	to show better comparisons for merges are given, this option
> 	is implied; in other words, you do not have to say e.g. `git
> 	log -m --cc`.  `git log --cc` suffices.

Well, to me this piece looks much harder to understand than current Git
documentation, and then seemingly contradicts current Git behavior and
implementation, as "log --cc -m" is not the same as "log --cc" in the
current Git (so we can't say that --cc implies -m), and "log -m --cc" is
the same as "log --cc" due to absolutely different reason: -m and --cc
are mutually exclusive options, so the last one simply takes precedence.

In the current Git, as documented, -m just produces separate diff with
respect to every parent. Simple and straightforward. Users don't need to
learn about --cc, -c, --raw, --stat... to figure what -m does and if
it's what they need. Unfortunately they still need to learn about -p,
but I'm already done trying to promote this simple change.

>
> The rest is a tangent that is not related to the above.  I suspect
> that this also applies to newer `--remerge-diff`, as it also targets
> to show merges better than the original "pairwise patches" that were
> largely useless, but the right way to view what `--cc` and other
> formats do for non-merge commits is *not* to think that they "imply"
> `-p`.  It is more like that the output from these formats on
> non-merge commits happen to be identical to what `-p` would produce.
> You could say that the "magic" these options know to show merge
> commits better degenerates to what `-p` gives when applied to
> non-merge commits.
>
> Another way to look at it is that `--cc` and friends, even though
> they are meant as improvements for showing merges over "-m -p" that
> gives human-unreadable pair-wise diffs, do not imply "--merges"
> (i.e. show only merge commits)---hence they have to show something
> for non-merge commits.  Because output formats for all of them were
> modeled loosely [*] after "-p" output, we happened to pick it as the
> format they fall back to when they are not showing comparisons for
> merge commits.

I admit you are very creative producing these views,, but currently
these options just imply -p. Simple to understand, useful, works.

Overall, as you don't like my simple clarification, and I don't like the
direction(s) you propose, I figure I rather withdraw the part of patch
causing contention in the re-roll.

Thanks,
-- Sergey Organov

[PATCH 2/2] diff-merges: introduce '-d' option

[Sergey Organov]

This option provides a shortcut to request diff with respect to first
parent for any kind of commit, universally. It's implemented as pure
synonym for "--diff-merges=first-parent --patch".

Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
 Documentation/diff-options.txt | 4 ++++
 Documentation/git-log.txt      | 2 +-
 diff-merges.c                  | 3 +++
 t/t4013-diff-various.sh        | 8 ++++++++
 4 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index f93aa3e46a52..d773dafcb10a 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -51,6 +51,10 @@ ifdef::git-log[]
 Note: This option not implying `-p` is legacy feature that is
 preserved for the sake of backward compatibility.
 
+-d::
+	Produce diff with respect to first parent.
+	Shortcut for '--diff-merges=first-parent -p'.
+
 -c::
 	Produce combined diff output for merge commits.
 	Shortcut for '--diff-merges=combined -p'.
diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 9b7ec96e767a..59bd74a1a596 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -120,7 +120,7 @@ By default, `git log` does not generate any diff output. The options
 below can be used to show the changes made by each commit.
 
 Note that unless one of `--diff-merges` variants (including short
-`-m`, `-c`, and `--cc` options) is explicitly given, merge commits
+`-d`, `-m`, `-c`, and `--cc` options) is explicitly given, merge commits
 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
diff --git a/diff-merges.c b/diff-merges.c
index ec97616db1df..6eb72e6fc28a 100644
--- a/diff-merges.c
+++ b/diff-merges.c
@@ -125,6 +125,9 @@ int diff_merges_parse_opts(struct rev_info *revs, const char **argv)
 	if (!suppress_m_parsing && !strcmp(arg, "-m")) {
 		set_to_default(revs);
 		revs->merges_need_diff = 0;
+	} else if (!strcmp(arg, "-d")) {
+		set_first_parent(revs);
+		revs->merges_imply_patch = 1;
 	} else if (!strcmp(arg, "-c")) {
 		set_combined(revs);
 		revs->merges_imply_patch = 1;
diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index 5de1d190759f..a07d6eb6dd97 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -473,6 +473,14 @@ test_expect_success 'log --diff-merges=on matches --diff-merges=separate' '
 	test_cmp expected actual
 '
 
+test_expect_success 'log -d matches --diff-merges=1 -p' '
+	git log --diff-merges=1 -p master >result &&
+	process_diffs result >expected &&
+	git log -d master >result &&
+	process_diffs result >actual &&
+	test_cmp expected actual
+'
+
 test_expect_success 'deny wrong log.diffMerges config' '
 	test_config log.diffMerges wrong-value &&
 	test_expect_code 128 git log
-- 
2.25.1

Re: [PATCH 2/2] diff-merges: introduce '-d' option

[Junio C Hamano]

Sergey Organov <sorganov@gmail.com> writes:

> This option provides a shortcut to request diff with respect to first
> parent for any kind of commit, universally. It's implemented as pure
> synonym for "--diff-merges=first-parent --patch".
>
> Signed-off-by: Sergey Organov <sorganov@gmail.com>
> ---

Sounds very straight-forward.

Given that "--first-parent" in "git log --first-parent -p" already
defeats "-m" and shows the diff against the first parent only,
people may find it confusing if "git log -d" does not act as a
shorthand for that.  From the above and also from the documentation
update, it is hard to tell if that is what you implemented, or it
only affects the "diff-merges" part.

Other than that, the patch looks quite small and to the point.

Thanks.

>  Documentation/diff-options.txt | 4 ++++
>  Documentation/git-log.txt      | 2 +-
>  diff-merges.c                  | 3 +++
>  t/t4013-diff-various.sh        | 8 ++++++++
>  4 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
> index f93aa3e46a52..d773dafcb10a 100644
> --- a/Documentation/diff-options.txt
> +++ b/Documentation/diff-options.txt
> @@ -51,6 +51,10 @@ ifdef::git-log[]
>  Note: This option not implying `-p` is legacy feature that is
>  preserved for the sake of backward compatibility.
>  
> +-d::
> +	Produce diff with respect to first parent.
> +	Shortcut for '--diff-merges=first-parent -p'.
> +
>  -c::
>  	Produce combined diff output for merge commits.
>  	Shortcut for '--diff-merges=combined -p'.
> diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
> index 9b7ec96e767a..59bd74a1a596 100644
> --- a/Documentation/git-log.txt
> +++ b/Documentation/git-log.txt
> @@ -120,7 +120,7 @@ By default, `git log` does not generate any diff output. The options
>  below can be used to show the changes made by each commit.
>  
>  Note that unless one of `--diff-merges` variants (including short
> -`-m`, `-c`, and `--cc` options) is explicitly given, merge commits
> +`-d`, `-m`, `-c`, and `--cc` options) is explicitly given, merge commits
>  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
> diff --git a/diff-merges.c b/diff-merges.c
> index ec97616db1df..6eb72e6fc28a 100644
> --- a/diff-merges.c
> +++ b/diff-merges.c
> @@ -125,6 +125,9 @@ int diff_merges_parse_opts(struct rev_info *revs, const char **argv)
>  	if (!suppress_m_parsing && !strcmp(arg, "-m")) {
>  		set_to_default(revs);
>  		revs->merges_need_diff = 0;
> +	} else if (!strcmp(arg, "-d")) {
> +		set_first_parent(revs);
> +		revs->merges_imply_patch = 1;
>  	} else if (!strcmp(arg, "-c")) {
>  		set_combined(revs);
>  		revs->merges_imply_patch = 1;
> diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
> index 5de1d190759f..a07d6eb6dd97 100755
> --- a/t/t4013-diff-various.sh
> +++ b/t/t4013-diff-various.sh
> @@ -473,6 +473,14 @@ test_expect_success 'log --diff-merges=on matches --diff-merges=separate' '
>  	test_cmp expected actual
>  '
>  
> +test_expect_success 'log -d matches --diff-merges=1 -p' '
> +	git log --diff-merges=1 -p master >result &&
> +	process_diffs result >expected &&
> +	git log -d master >result &&
> +	process_diffs result >actual &&
> +	test_cmp expected actual
> +'
> +
>  test_expect_success 'deny wrong log.diffMerges config' '
>  	test_config log.diffMerges wrong-value &&
>  	test_expect_code 128 git log

Re: [PATCH 2/2] diff-merges: introduce '-d' option

[Sergey Organov]

Junio C Hamano <gitster@pobox.com> writes:

> Sergey Organov <sorganov@gmail.com> writes:
>
>> This option provides a shortcut to request diff with respect to first
>> parent for any kind of commit, universally. It's implemented as pure
>> synonym for "--diff-merges=first-parent --patch".
>>
>> Signed-off-by: Sergey Organov <sorganov@gmail.com>
>> ---
>
> Sounds very straight-forward.
>
> Given that "--first-parent" in "git log --first-parent -p" already
> defeats "-m" and shows the diff against the first parent only,
> people may find it confusing if "git log -d" does not act as a
> shorthand for that.

It doesn't, and I believe it's a good thing, as primary function of
--first-parent is to change history traversal rules, and if -d did that,
it would be extremely confusing.

Also, --first-parent is correctly documented as implying
--diff-merges=first-parent, not as defeating -m.

> From the above and also from the documentation update, it is hard to
> tell if that is what you implemented, or it only affects the
> "diff-merges" part.

If we read resulting documentation with a fresh eye, -d is similar to
--cc, and -c, just producing yet another kind of output, so I think all
this fits together quite nicely and shouldn't cause confusion.

>
> Other than that, the patch looks quite small and to the point.

Thanks,
-- Sergey Organov

Re: [PATCH 2/2] diff-merges: introduce '-d' option

[Junio C Hamano]

Sergey Organov <sorganov@gmail.com> writes:

>> Sounds very straight-forward.
>>
>> Given that "--first-parent" in "git log --first-parent -p" already
>> defeats "-m" and shows the diff against the first parent only,
>> people may find it confusing if "git log -d" does not act as a
>> shorthand for that.
>
> It doesn't, and I believe it's a good thing, as primary function of
> --first-parent is to change history traversal rules, and if -d did that,
> it would be extremely confusing.

I am not sure about that.

> Also, --first-parent is correctly documented as implying
> --diff-merges=first-parent, not as defeating -m.

Yes, exactly.  That makes me even more convinced that the intuitive
behaviour, when we say "we have this great short-hand option that
lets your 'git log' to do the first-parent thing with patch output",
is to do the first-parent traversal _and_ show first-parent patches.

"-d" is documented as a short-hand for "--diff-merges=first-parent
--patch" and not for "--first-parent --patch", so the behaviour may
correctly match documentation, but that does not make the documented
behaviour an intuitive one.  And a behaviour that is not intuitive
is confusing.

> If we read resulting documentation with a fresh eye, -d is similar to
> --cc, and -c, just producing yet another kind of output, so I think all
> this fits together quite nicely and shouldn't cause confusion.

Another thing is that showing first-parent patch for merges while
letting the traversal also visit the second-parent chain is not as
useful an option as it could be, even though it is not so bad as the
original "-m -p" that also showed second-parent patch for merges as
well.  People would have to say "log --first-parent -p" to get the
first-parent traversal with first-parent patch output, and they
would not behefit from having "-d".

Re: [PATCH 2/2] diff-merges: introduce '-d' option

[Sergey Organov]

Junio C Hamano <gitster@pobox.com> writes:

> Sergey Organov <sorganov@gmail.com> writes:
>
>>> Sounds very straight-forward.
>>>
>>> Given that "--first-parent" in "git log --first-parent -p" already
>>> defeats "-m" and shows the diff against the first parent only,
>>> people may find it confusing if "git log -d" does not act as a
>>> shorthand for that.
>>
>> It doesn't, and I believe it's a good thing, as primary function of
>> --first-parent is to change history traversal rules, and if -d did that,
>> it would be extremely confusing.
>
> I am not sure about that.
>
>> Also, --first-parent is correctly documented as implying
>> --diff-merges=first-parent, not as defeating -m.
>
> Yes, exactly.  That makes me even more convinced that the intuitive
> behaviour, when we say "we have this great short-hand option that
> lets your 'git log' to do the first-parent thing with patch output",
> is to do the first-parent traversal _and_ show first-parent patches.
>
> "-d" is documented as a short-hand for "--diff-merges=first-parent
> --patch" and not for "--first-parent --patch", so the behaviour may
> correctly match documentation, but that does not make the documented
> behaviour an intuitive one.  And a behaviour that is not intuitive
> is confusing.

I think both behaviors make sense, provided they are correctly
documented. I just prefer the one that is more basic, yet allows to
achieve things that another one does not.

>
>> If we read resulting documentation with a fresh eye, -d is similar to
>> --cc, and -c, just producing yet another kind of output, so I think all
>> this fits together quite nicely and shouldn't cause confusion.
>
> Another thing is that showing first-parent patch for merges while
> letting the traversal also visit the second-parent chain is not as
> useful an option as it could be, even though it is not so bad as the
> original "-m -p" that also showed second-parent patch for merges as
> well.

I don't see why desire to look at diff-to-first-parent on "side"
branches is any different from desire to look at them on "primary"
branch, sorry, so I still don't want "-d" to affect traversal or other
commit filtering rules. We do have --first-parent as well as a few
others for that.

> People would have to say "log --first-parent -p" to get the
> first-parent traversal with first-parent patch output, and they
> would not behefit from having "-d".

Well, at least they can now say "log --first-parent -d" as well ;)

Honestly, the "log --first-parent -p" (without "-m") suddenly producing
diffs for merge commits is already unnatural, needs yet another
special-casing in documentation, and then, finally, this relatively new
behavior was introduced exactly because there were no "-d" at that time,
to save typing "-m". The latter is yet another example of why "-d" in
its current form is a good idea.

That said, if you feel like there is place for a short-cut for this
particular use-case, it'd be fine with me, say:

--fpd:
  short-cut for "--first-parent -d"

would fit quite nicely into the picture, I think.

Thanks,
-- Sergey Organov

Re: [PATCH 2/2] diff-merges: introduce '-d' option

[Junio C Hamano]

Sergey Organov <sorganov@gmail.com> writes:

> I don't see why desire to look at diff-to-first-parent on "side"
> branches is any different from desire to look at them on "primary"
> branch

Yeah, but that is not what I meant.  The above argues for why
"--diff-merges=first-parent" should exist independently from the
"--first-parent" traversal *and* display option.  I am not saying
it should not exist.

But I view that the desire to look at any commits and its changes on
the "side" branch at all *is* at odds with the wish to look at
first-parent change for merge commits.  Once you decide to look at
first-parent change for a merge commit, then every change you see
for each commit on the "side" branch, whether it is shown as
first-parent diff or N pairwise diffs, is what you have already seen
in the change in the merge commit, because "git log" goes newer to
older, and the commits on the side branches appear after the merge
that brings them to the mainline.

Making "log -d" mean "log --diff-merges=first-parent --patch" lets
that less useful combination ("show first-parent patches but
traverse side branches as well") squat on the short and sweet "-d"
that could be used for more useful "log --first-parent --patch",
which would also be more common and intuitive to users, and that is
what I suspect will become problematic in the longer run.

Thanks.

Re: [PATCH 2/2] diff-merges: introduce '-d' option

[Sergey Organov]

Junio C Hamano <gitster@pobox.com> writes:

> Sergey Organov <sorganov@gmail.com> writes:
>
>> I don't see why desire to look at diff-to-first-parent on "side"
>> branches is any different from desire to look at them on "primary"
>> branch
>
> Yeah, but that is not what I meant.  The above argues for why
> "--diff-merges=first-parent" should exist independently from the
> "--first-parent" traversal *and* display option.  I am not saying
> it should not exist.

I was not assuming you were saying this, as it has been discussed and
agreed upon when --diff-merges=first-parent was introduced, though I
think I now see your point more clearly.

>
> But I view that the desire to look at any commits and its changes on
> the "side" branch at all *is* at odds with the wish to look at
> first-parent change for merge commits.

I think I do now understand what you mean, yet I have alternative view
on the issue.

> Once you decide to look at first-parent change for a merge commit,
> then every change you see for each commit on the "side" branch,
> whether it is shown as first-parent diff or N pairwise diffs, is what
> you have already seen in the change in the merge commit,

Actually, this happens to be exactly one of intended use-cases for "-d".
It's useful to see how some change introduced by the merge looked in the
context of the original commit, or to figure where the change came from.

> because "git log" goes newer to older, and the commits on the side
> branches appear after the merge that brings them to the mainline.

The exact order is orthogonal to the issue at hands, I think.

> Making "log -d" mean "log --diff-merges=first-parent --patch" lets
> that less useful combination ("show first-parent patches but
> traverse side branches as well") squat on the short and sweet "-d"
> that could be used for more useful "log --first-parent --patch",
> which would also be more common and intuitive to users, and that is
> what I suspect will become problematic in the longer run.

Sorry, "-d ≡ --first-parent --patch" you suggest contradicts my view on
the whole scheme of things, for several reasons:

* I still find it problematic if -d, intended to fit nicely among --cc,
-c, -d, -m, -p, --remerge-diff options, suddenly implies --first-parent.
This would bring yet another inconsistency, and I don't want to be the
one who introduced it.

* In its current state -d conveniently means: "gimme simple diff output
for everything", where --first-parent you suggest doesn't fit at all.

* Current -d implementation is semantically as close to -p as possible,
tweaking exactly one thing compared to -p: the format of output for
merge commits, so is simpler than what you suggest from all angles, as
--first-parent tweaks more than one thing.

* To me what you argue for looks mostly like a desire to have a
short-cut for "--first-parent --patch", and my patch in question does
not seem to contradict this desire, as it'd be very surprising if
somebody came up with the name "-d" for such a short-cut. Definitely not
me.

* Finally, if -d becomes "--patch --first-parent", how do I get back
useful "--patch --diff-merges=first-parent" part of it, provided
--first-parent is unreversable? And even if it were reversable, then

   git log -d --no-first-parent =
   git log --patch --first-parent --no-first-parent =
   git log --patch

is definitely not what is needed, nor frequent demand to revert implied
things indicates optimal design. Compare this to

   git log -d --first-parent

that current -d provides for you to get what you need, and that
unambiguously reads: "gimme *d*iff for all commits while following
*first parent* through the history" (while, unlike, -p not requiring
--first-parent to implicitly tweak diff for merges output).

Overall, after considering your concern, I'd still prefer to leave "-d"
semantics as implemented, consistent with the rest of similar options,
and let somebody else define more shortcuts for their frequent use-cases
if they feel like it.

Thanks,
-- Sergey Organov

P.S. I also figure that maybe our divergence comes from the fact that I
consider merge commits to be primarily commits (introducing particular
set of changes, and then having reference to the source of the changes),
whereas you consider them primarily merges (joining two histories, and
then maybe some artificial changes that make merges "evil"). That's why
we often end up agreeing to disagree, as both these points of view seem
pretty valid.

Re: [PATCH 2/2] diff-merges: introduce '-d' option

[Junio C Hamano]

Sergey Organov <sorganov@gmail.com> writes:

> P.S. I also figure that maybe our divergence comes from the fact that I
> consider merge commits to be primarily commits (introducing particular
> set of changes, and then having reference to the source of the changes),
> whereas you consider them primarily merges (joining two histories, and
> then maybe some artificial changes that make merges "evil"). That's why
> we often end up agreeing to disagree, as both these points of view seem
> pretty valid.

It rarely is the case that two opposing world views are equally
valid, though.

If there were an option that forbids any comparison output from a
single parent commit (say --ndfnm "no-diff-for-non-merge"), then
those with "merges are the primary thing, single-parent commits on
the merged branches are implementation details" worldview would be
commonly using "--diff-merges=first-parent --patch --ndfnm" and (1)
viewing only the combined effect of merging side branches without
seeing noise from individual commits whose effects are already shown
in these merges, and (2) traversing the side branches as well, so
that merges from side-side branches into the side branches are
viewable the same way as merges into the mainline.

But because no such option exists and nobody asked for such an
option during the whole lifetime of the project, I highly doubt
that it is a valid world view with wide backing from the users.

Even if it were a valid world view with wide backing, the
combination "--diff-merges=first-parent --patch" would be less than
ideal presentation for them (due to lack of "--ndfnm").  And as I
already said, it would not be useful without --first-parent
traversal for the worldview git has supported.

That is why I find it of dubious value to let short-and-sweet '-d'
be squatted by less than ideal "--diff-merges=first-parent --patch"
combination.  Shorthands are scarse resources, and we want to be
careful before handing them out.

Thanks.

Re: [PATCH 2/2] diff-merges: introduce '-d' option

[Sergey Organov]

Junio C Hamano <gitster@pobox.com> writes:

> Sergey Organov <sorganov@gmail.com> writes:
>
>> P.S. I also figure that maybe our divergence comes from the fact that I
>> consider merge commits to be primarily commits (introducing particular
>> set of changes, and then having reference to the source of the changes),
>> whereas you consider them primarily merges (joining two histories, and
>> then maybe some artificial changes that make merges "evil"). That's why
>> we often end up agreeing to disagree, as both these points of view seem
>> pretty valid.
>
> It rarely is the case that two opposing world views are equally
> valid, though.

Yes. In this particular case the two are not opposing though, rather
orthogonal, as they reflect the intrinsic dualism of the concept of
"merge commit". Merge commit is both a new state, and history junction,
neither of which is more or less valid or essential, and I use both
views myself, depending on situation.

An electron is both a particle and a wave, and one just uses its side
that is more convenient for explanation of the case in hand.

I promote features that I routinely need in my workflows, yet respecting
the other side of the coin as well, even though I may rarely find this
other side useful. I mean, for me, this -c/--cc (let alone -m) output is
only confusing, yet I won't be saying that it's somehow less valid than
proposed -d.

> If there were an option that forbids any comparison output from a
> single parent commit (say --ndfnm "no-diff-for-non-merge"),
> then those with "merges are the primary thing, single-parent commits
> on the merged branches are implementation details" worldview would be
> commonly using "--diff-merges=first-parent --patch --ndfnm" and (1)
> viewing only the combined effect of merging side branches without
> seeing noise from individual commits whose effects are already shown
> in these merges, and (2) traversing the side branches as well, so that
> merges from side-side branches into the side branches are viewable the
> same way as merges into the mainline.

No need to ask for a new option, as the behavior you describe is already
there, and is spelled "git log --diff-merges=first-parent"
(--diff-merges=1 for short).

> But because no such option exists and nobody asked for such an
> option during the whole lifetime of the project, I highly doubt
> that it is a valid world view with wide backing from the users.

Your concern above seems to be void, so this doesn't hold either.

As a side-note though, something like this has been asked recently, as
what you describe by --ndfnm should in fact have been what --no-patch
does, but surprisingly does not (please recall recent discussion of this
issue).

> Even if it were a valid world view with wide backing,

Apparently it is valid, otherwise there would be no need for diff to
first parent at all, let alone "git log --first-parent -p" have used it
by default.

> the combination "--diff-merges=first-parent --patch" would be less
> than ideal presentation for them (due to lack of "--ndfnm").

First, as we figured above, --ndfnm is not needed, and second, me, being
one of "them", tries hard to convince you it is the best presentation
"them" can get, while "ideal" simply never exists.

> And as I already said, it would not be useful without --first-parent
> traversal for the worldview git has supported.

Yes, you said it earlier in this thread, and as well I already explained
how it is useful without --first-parent.

> That is why I find it of dubious value to let short-and-sweet '-d'
> be squatted by less than ideal "--diff-merges=first-parent --patch"
> combination.

Hopefully I do understand your concerns, yet I believe
"--diff-merges=first-parent --patch" is way better for "-d" shortcut
than "--first-parent --patch", for the reasons I already explained
earlier in this thread.

> Shorthands are scarse resources, and we want to be careful before
> handing them out.

Yep, agreed.

I believe I carefully thought it over though, weighing all pros and
cons, and thus -d fits nicely among -c and --cc, being yet another
frequently desired format for merges, plays nice with -p as well, and is
very mnemonic, giving us convenient, user-friendly, and consistent user
interface overall.

Thanks,
-- Sergey Organov

Re: [PATCH 2/2] diff-merges: introduce '-d' option

[Junio C Hamano]

Sergey Organov <sorganov@gmail.com> writes:

> No need to ask for a new option, as the behavior you describe is already
> there, and is spelled "git log --diff-merges=first-parent"
> (--diff-merges=1 for short).

Ah, that changes things.  

Making "--diff-merges=<how>" only about the presentation of merge
commits, requiring a separate "-p" for single-parent commits [*],
does make the life for those in the "merges are the only interesting
things" camp a lot easier, exactly because the lack of "-p" can be
used to say "I am not interested in chanages by single-parent
commits".

	Side note: I personally think it is a design mistake of
	--diff-merges=<how> (e.g., --cc and --diff-merges=cc do not
	behave the same way) but that is a different story, and it
	is way too late now anyway to "fix" or change.

So "-d" that stands for "--diff-merges=first-parent -p" makes the
more useful (to those who think "merges are the only interesting
things", which I do not belong to) "--diff-merges=first-parent"
(without "-p") less useful.  And the combination is not useful for
those of us who find individual patches plus tweaks by merges
(either --cc or --remerge-diff) are the way to look at the history.

I still do not think that we want to give a short-and-sweet single
letter option for such a combination.

Thanks for clarification.

Re: [PATCH 2/2] diff-merges: introduce '-d' option

[Sergey Organov]

Junio C Hamano <gitster@pobox.com> writes:

> Sergey Organov <sorganov@gmail.com> writes:
>
>> No need to ask for a new option, as the behavior you describe is already
>> there, and is spelled "git log --diff-merges=first-parent"
>> (--diff-merges=1 for short).
>
> Ah, that changes things.

Only a tiny bit, unfortunately, as I'm still struggling to finally
convince you (((

>
> Making "--diff-merges=<how>" only about the presentation of merge
> commits, requiring a separate "-p" for single-parent commits [*],
> does make the life for those in the "merges are the only interesting
> things" camp a lot easier, exactly because the lack of "-p" can be
> used to say "I am not interested in chanages by single-parent
> commits".
>
> 	Side note: I personally think it is a design mistake of
> 	--diff-merges=<how> (e.g., --cc and --diff-merges=cc do not
> 	behave the same way) but that is a different story, and it
> 	is way too late now anyway to "fix" or change.

        Side note: This has been considered and agreed upon when
        --diff-merges= options were introduced, and as far as I recall,
        at that time you explicitly agreed it might be useful to be able
        to get output only for merge commits.

        --cc is a simple alias for "--diff-merges=cc --patch" nowadays,
        so yes, they do behave differently, and that's by design. Dunno
        see any design mistake here, as we get all useful variations of
        behavior with a straightforward design, more frequent use-cases
        served by shorter options. Looks fine.

>
> So "-d" that stands for "--diff-merges=first-parent -p" makes the
> more useful (to those who think "merges are the only interesting
> things", which I do not belong to) "--diff-merges=first-parent"
> (without "-p") less useful.  And the combination is not useful for
> those of us who find individual patches plus tweaks by merges
> (either --cc or --remerge-diff) are the way to look at the history.

Yes, you have your --cc, -c, and --remerge-diff (that I'd call something
like --rd probably, but anyway). Could I please have my simple,
straightforward, mnemonic, and terribly useful "-d" as well?

In other words, will I finally be faced with "if you need it, do it
yourself" argument? ;)

> I still do not think that we want to give a short-and-sweet single
> letter option for such a combination.

I have very simple desire: convenient way to tell Git to show me diff to
the first parent for merge commits, as that's the thing I need 99% of
times when I do request diff output at all. That's exactly what I'd have
seen as changes when I was about to commit the merge as well, similar to
any other commit. It's so natural that I can't figure why it looks so
damn rare or unusual to you, and that it makes you argue so hard against
-d, especially when -p, -c, --cc, or even -m, are already there?

I do sympathize your desire to be careful about short options, but what
reservation for "-d" do you still have in mind? It seems that it was
just waiting for me to come and finally bring it to life with the best
meaning possible. How long should I wait for it to remain unused to
finally be able to make use of it?

Thanks,
-- Sergey Organov

[PATCH v2 0/2] diff-merges: introduce '-d' option

[Sergey Organov]

This new convenience option requests full diff with respect to first
parent, so that

  git log -d

will output diff with respect to first parent for every commit,
universally, no matter how many parents the commit turns out to have.

It's implemented as pure synonym for

  --diff-merges=first-parent --patch

The first commit in the series tweaks diff-merges documentation a bit,
and is valuable by itself. It's put here as '-d' implementation commit
depends on it in its documentation part.

Note: the need for this new convenience option mostly emerged from
denial by the community of patches that modify '-m' behavior to imply
'-p' as the rest of similar options (such as --cc) do.

Updates in v2:

  * Reordered documentation for diff-merges formats in accordance with
    Junio recommendation.

  * Removed clarification of surprising -m behavior due to controversy
    with Junio on how exactly it should look like.

Sergey Organov (2):
  diff-merges: improve --diff-merges documentation
  diff-merges: introduce '-d' option

 Documentation/diff-options.txt | 102 ++++++++++++++++++---------------
 Documentation/git-log.txt      |   4 +-
 diff-merges.c                  |   3 +
 t/t4013-diff-various.sh        |   8 +++
 4 files changed, 70 insertions(+), 47 deletions(-)

Interdiff against v1:
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index d773dafcb10a..19bb78ff6652 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -47,9 +47,6 @@ ifdef::git-log[]
 	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.
-+
-Note: This option not implying `-p` is legacy feature that is
-preserved for the sake of backward compatibility.
 
 -d::
 	Produce diff with respect to first parent.
@@ -96,16 +93,6 @@ separate::
 	Show full diff with respect to each of parents.
 	Separate log entry and diff is generated for each parent.
 +
-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).
-+
 combined, c::
 	Show differences from each of the parents to the merge
 	result simultaneously instead of showing pairwise diff between
@@ -117,6 +104,16 @@ dense-combined, cc::
 	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).
 --
 
 --combined-all-paths::
-- 
2.25.1

[PATCH v2 1/2] diff-merges: improve --diff-merges documentation

[Sergey Organov]

* 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(-)

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`
-- 
2.25.1

[PATCH v2 2/2] diff-merges: introduce '-d' option

[Sergey Organov]

This option provides a shortcut to request diff with respect to first
parent for any kind of commit, universally. It's implemented as pure
synonym for "--diff-merges=first-parent --patch".

Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
 Documentation/diff-options.txt | 4 ++++
 Documentation/git-log.txt      | 2 +-
 diff-merges.c                  | 3 +++
 t/t4013-diff-various.sh        | 8 ++++++++
 4 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 8035210c1418..19bb78ff6652 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -48,6 +48,10 @@ ifdef::git-log[]
 	similar to '--diff-merges=on' (which see) except `-m` will
 	produce no output unless `-p` is given as well.
 
+-d::
+	Produce diff with respect to first parent.
+	Shortcut for '--diff-merges=first-parent -p'.
+
 -c::
 	Produce combined diff output for merge commits.
 	Shortcut for '--diff-merges=combined -p'.
diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 9b7ec96e767a..59bd74a1a596 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -120,7 +120,7 @@ By default, `git log` does not generate any diff output. The options
 below can be used to show the changes made by each commit.
 
 Note that unless one of `--diff-merges` variants (including short
-`-m`, `-c`, and `--cc` options) is explicitly given, merge commits
+`-d`, `-m`, `-c`, and `--cc` options) is explicitly given, merge commits
 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
diff --git a/diff-merges.c b/diff-merges.c
index ec97616db1df..6eb72e6fc28a 100644
--- a/diff-merges.c
+++ b/diff-merges.c
@@ -125,6 +125,9 @@ int diff_merges_parse_opts(struct rev_info *revs, const char **argv)
 	if (!suppress_m_parsing && !strcmp(arg, "-m")) {
 		set_to_default(revs);
 		revs->merges_need_diff = 0;
+	} else if (!strcmp(arg, "-d")) {
+		set_first_parent(revs);
+		revs->merges_imply_patch = 1;
 	} else if (!strcmp(arg, "-c")) {
 		set_combined(revs);
 		revs->merges_imply_patch = 1;
diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index 5de1d190759f..a07d6eb6dd97 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -473,6 +473,14 @@ test_expect_success 'log --diff-merges=on matches --diff-merges=separate' '
 	test_cmp expected actual
 '
 
+test_expect_success 'log -d matches --diff-merges=1 -p' '
+	git log --diff-merges=1 -p master >result &&
+	process_diffs result >expected &&
+	git log -d master >result &&
+	process_diffs result >actual &&
+	test_cmp expected actual
+'
+
 test_expect_success 'deny wrong log.diffMerges config' '
 	test_config log.diffMerges wrong-value &&
 	test_expect_code 128 git log
-- 
2.25.1

[PATCH v3 0/3] diff-merges: introduce '--dd' option

[Sergey Organov]

This new convenience option requests full diff with respect to first
parent, so that

  git log --dd

will output diff with respect to first parent for every commit,
universally, no matter how many parents the commit turns out to have.

'--dd' is implemented as pure synonym for "--diff-merges=first-parent
--patch".

The first commit in the series tweaks diff-merges documentation a bit,
and is valuable by itself. It's put here as '--dd' implementation
commit depends on it in its documentation part.

Note: the need for this new convenience option mostly emerged from
denial by the community of patches that modify '-m' behavior to imply
'-p' as the rest of similar options (such as --cc) do. So, basically,
'--dd' is what '-m' should have been to be more useful.

Updates in v3:

  * Option renamed from '-d' to '--dd' due to Junio overpowering
    request to keep short-and-sweet '-d' reserved for another (yet
    unspecified) use.

  * Added completion of '--dd' to git-completion.bash.

Updates in v2:

  * Reordered documentation for diff-merges formats in accordance with
    Junio recommendation.

  * Removed clarification of surprising -m behavior due to controversy
    with Junio on how exactly it should look like.

Sergey Organov (3):
  diff-merges: improve --diff-merges documentation
  diff-merges: introduce '--dd' option
  completion: complete '--dd'

 Documentation/diff-options.txt         | 103 ++++++++++++++-----------
 Documentation/git-log.txt              |   4 +-
 contrib/completion/git-completion.bash |   2 +-
 diff-merges.c                          |   3 +
 t/t4013-diff-various.sh                |   8 ++
 5 files changed, 72 insertions(+), 48 deletions(-)

Interdiff against v2:
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 19bb78ff6652..f80d493dd4c8 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -48,10 +48,6 @@ ifdef::git-log[]
 	similar to '--diff-merges=on' (which see) except `-m` will
 	produce no output unless `-p` is given as well.
 
--d::
-	Produce diff with respect to first parent.
-	Shortcut for '--diff-merges=first-parent -p'.
-
 -c::
 	Produce combined diff output for merge commits.
 	Shortcut for '--diff-merges=combined -p'.
@@ -60,6 +56,11 @@ ifdef::git-log[]
 	Produce dense combined diff output for merge commits.
 	Shortcut for '--diff-merges=dense-combined -p'.
 
+--dd::
+	Produce diff with respect to first parent for both merge and
+	regular commits.
+	Shortcut for '--diff-merges=first-parent -p'.
+
 --remerge-diff::
 	Produce diff against re-merge.
 	Shortcut for '--diff-merges=remerge -p'.
diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 59bd74a1a596..579682172fe4 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -120,7 +120,7 @@ By default, `git log` does not generate any diff output. The options
 below can be used to show the changes made by each commit.
 
 Note that unless one of `--diff-merges` variants (including short
-`-d`, `-m`, `-c`, and `--cc` options) is explicitly given, merge commits
+`-m`, `-c`, `--cc`, and `--dd` options) is explicitly given, merge commits
 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
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 133ec92bfae7..ca4fa39f3ff8 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2042,7 +2042,7 @@ __git_log_shortlog_options="
 "
 # Options accepted by log and show
 __git_log_show_options="
-	--diff-merges --diff-merges= --no-diff-merges --remerge-diff
+	--diff-merges --diff-merges= --no-diff-merges --dd --remerge-diff
 "
 
 __git_diff_merges_opts="off none on first-parent 1 separate m combined c dense-combined cc remerge r"
diff --git a/diff-merges.c b/diff-merges.c
index 6eb72e6fc28a..45507588a279 100644
--- a/diff-merges.c
+++ b/diff-merges.c
@@ -125,15 +125,15 @@ int diff_merges_parse_opts(struct rev_info *revs, const char **argv)
 	if (!suppress_m_parsing && !strcmp(arg, "-m")) {
 		set_to_default(revs);
 		revs->merges_need_diff = 0;
-	} else if (!strcmp(arg, "-d")) {
-		set_first_parent(revs);
-		revs->merges_imply_patch = 1;
 	} else if (!strcmp(arg, "-c")) {
 		set_combined(revs);
 		revs->merges_imply_patch = 1;
 	} else if (!strcmp(arg, "--cc")) {
 		set_dense_combined(revs);
 		revs->merges_imply_patch = 1;
+	} else if (!strcmp(arg, "--dd")) {
+		set_first_parent(revs);
+		revs->merges_imply_patch = 1;
 	} else if (!strcmp(arg, "--remerge-diff")) {
 		set_remerge_diff(revs);
 		revs->merges_imply_patch = 1;
diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index a07d6eb6dd97..4b474808311e 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -473,10 +473,10 @@ test_expect_success 'log --diff-merges=on matches --diff-merges=separate' '
 	test_cmp expected actual
 '
 
-test_expect_success 'log -d matches --diff-merges=1 -p' '
+test_expect_success 'log --dd matches --diff-merges=1 -p' '
 	git log --diff-merges=1 -p master >result &&
 	process_diffs result >expected &&
-	git log -d master >result &&
+	git log --dd master >result &&
 	process_diffs result >actual &&
 	test_cmp expected actual
 '
-- 
2.25.1

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

[Sergey Organov]

* 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(-)

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`
-- 
2.25.1

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

[Eric Sunshine]

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.

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

[Sergey Organov]

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

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

[Junio C Hamano]

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.

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

[Sergey Organov]

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

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

[Junio C Hamano]

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.

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

[Elijah Newren]

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.

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

[Sergey Organov]

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

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

[Sergey Organov]

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.

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

[Junio C Hamano]

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.

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

[Sergey Organov]

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

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

[Junio C Hamano]

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.

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

[Elijah Newren]

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

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

[Junio C Hamano]

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

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

[Junio C Hamano]

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.

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

[Elijah Newren]

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

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

[Junio C Hamano]

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.

[silly] worldview documents?

[Junio C Hamano]

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.

Re: [silly] worldview documents?

[Emily Shaffer]

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.

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

[Sergey Organov]

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

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

[Sergey Organov]

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

[PATCH v3 2/3] diff-merges: introduce '--dd' option

[Sergey Organov]

This option provides a shortcut to request diff with respect to first
parent for any kind of commit, universally. It's implemented as pure
synonym for "--diff-merges=first-parent --patch".

NOTE: originally proposed as '-d', and renamed to '--dd' due to Junio
request to keep "short-and-sweet" '-d' reserved for other uses.

Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
 Documentation/diff-options.txt | 5 +++++
 Documentation/git-log.txt      | 2 +-
 diff-merges.c                  | 3 +++
 t/t4013-diff-various.sh        | 8 ++++++++
 4 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 8035210c1418..f80d493dd4c8 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -56,6 +56,11 @@ ifdef::git-log[]
 	Produce dense combined diff output for merge commits.
 	Shortcut for '--diff-merges=dense-combined -p'.
 
+--dd::
+	Produce diff with respect to first parent for both merge and
+	regular commits.
+	Shortcut for '--diff-merges=first-parent -p'.
+
 --remerge-diff::
 	Produce diff against re-merge.
 	Shortcut for '--diff-merges=remerge -p'.
diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 9b7ec96e767a..579682172fe4 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -120,7 +120,7 @@ By default, `git log` does not generate any diff output. The options
 below can be used to show the changes made by each commit.
 
 Note that unless one of `--diff-merges` variants (including short
-`-m`, `-c`, and `--cc` options) is explicitly given, merge commits
+`-m`, `-c`, `--cc`, and `--dd` options) is explicitly given, merge commits
 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
diff --git a/diff-merges.c b/diff-merges.c
index ec97616db1df..45507588a279 100644
--- a/diff-merges.c
+++ b/diff-merges.c
@@ -131,6 +131,9 @@ int diff_merges_parse_opts(struct rev_info *revs, const char **argv)
 	} else if (!strcmp(arg, "--cc")) {
 		set_dense_combined(revs);
 		revs->merges_imply_patch = 1;
+	} else if (!strcmp(arg, "--dd")) {
+		set_first_parent(revs);
+		revs->merges_imply_patch = 1;
 	} else if (!strcmp(arg, "--remerge-diff")) {
 		set_remerge_diff(revs);
 		revs->merges_imply_patch = 1;
diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index 5de1d190759f..4b474808311e 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -473,6 +473,14 @@ test_expect_success 'log --diff-merges=on matches --diff-merges=separate' '
 	test_cmp expected actual
 '
 
+test_expect_success 'log --dd matches --diff-merges=1 -p' '
+	git log --diff-merges=1 -p master >result &&
+	process_diffs result >expected &&
+	git log --dd master >result &&
+	process_diffs result >actual &&
+	test_cmp expected actual
+'
+
 test_expect_success 'deny wrong log.diffMerges config' '
 	test_config log.diffMerges wrong-value &&
 	test_expect_code 128 git log
-- 
2.25.1

Re: [PATCH v3 2/3] diff-merges: introduce '--dd' option

[Junio C Hamano]

Sergey Organov <sorganov@gmail.com> writes:

> This option provides a shortcut to request diff with respect to first
> parent for any kind of commit, universally. It's implemented as pure
> synonym for "--diff-merges=first-parent --patch".

That explains what the patch does, but it does not tell us why it is
useful [*].

> NOTE: originally proposed as '-d', and renamed to '--dd' due to Junio
> request to keep "short-and-sweet" '-d' reserved for other uses.

The note is not grammatical, and more importantly, readers of "git
log" 6 months down the road would not care.  I'd rather not see it
in the proposed log message.  It is suitable material to place after
the three-dash line, or in the cover letter for the iteration.

> diff --git a/diff-merges.c b/diff-merges.c
> index ec97616db1df..45507588a279 100644
> --- a/diff-merges.c
> +++ b/diff-merges.c
> @@ -131,6 +131,9 @@ int diff_merges_parse_opts(struct rev_info *revs, const char **argv)
>  	} else if (!strcmp(arg, "--cc")) {
>  		set_dense_combined(revs);
>  		revs->merges_imply_patch = 1;
> +	} else if (!strcmp(arg, "--dd")) {
> +		set_first_parent(revs);
> +		revs->merges_imply_patch = 1;

Quite straight-forward as expected.  I do not think "--dd" clicks
for many people as "first parent diffs all over", though.

>  	} else if (!strcmp(arg, "--remerge-diff")) {
>  		set_remerge_diff(revs);
>  		revs->merges_imply_patch = 1;
> diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
> index 5de1d190759f..4b474808311e 100755
> --- a/t/t4013-diff-various.sh
> +++ b/t/t4013-diff-various.sh
> @@ -473,6 +473,14 @@ test_expect_success 'log --diff-merges=on matches --diff-merges=separate' '
>  	test_cmp expected actual
>  '
>  
> +test_expect_success 'log --dd matches --diff-merges=1 -p' '
> +	git log --diff-merges=1 -p master >result &&
> +	process_diffs result >expected &&
> +	git log --dd master >result &&
> +	process_diffs result >actual &&
> +	test_cmp expected actual
> +'
> +
>  test_expect_success 'deny wrong log.diffMerges config' '
>  	test_config log.diffMerges wrong-value &&
>  	test_expect_code 128 git log

Looking good.

Thanks.


[Footnote]

* As I said elsewhere, I do not think it is a good idea to encourage
  users' to adopt a screwed-up worldview in which first parent is
  special but not special, and does the wrong thing for reverse
  merges.  If the option were short-hand for "--first-parent -p",
  at least I would be more sympathetic.

Re: [PATCH v3 2/3] diff-merges: introduce '--dd' option

[Sergey Organov]

Junio C Hamano <gitster@pobox.com> writes:

> Sergey Organov <sorganov@gmail.com> writes:
>
>> This option provides a shortcut to request diff with respect to first
>> parent for any kind of commit, universally. It's implemented as pure
>> synonym for "--diff-merges=first-parent --patch".
>
> That explains what the patch does, but it does not tell us why it is
> useful [*].
>
>> NOTE: originally proposed as '-d', and renamed to '--dd' due to Junio
>> request to keep "short-and-sweet" '-d' reserved for other uses.
>
> The note is not grammatical, and more importantly, readers of "git
> log" 6 months down the road would not care.  I'd rather not see it
> in the proposed log message.  It is suitable material to place after
> the three-dash line, or in the cover letter for the iteration.

OK, will get rid of it.

Thanks,
-- Sergey Organov

[PATCH v3 3/3] completion: complete '--dd'

[Sergey Organov]

'--dd' only makes sense for 'git log' and 'git show', so add it to
__git_log_show_options which is referenced in the completion for these
two commands.

Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
 contrib/completion/git-completion.bash | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 133ec92bfae7..ca4fa39f3ff8 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2042,7 +2042,7 @@ __git_log_shortlog_options="
 "
 # Options accepted by log and show
 __git_log_show_options="
-	--diff-merges --diff-merges= --no-diff-merges --remerge-diff
+	--diff-merges --diff-merges= --no-diff-merges --dd --remerge-diff
 "
 
 __git_diff_merges_opts="off none on first-parent 1 separate m combined c dense-combined cc remerge r"
-- 
2.25.1

Re: [PATCH v3 3/3] completion: complete '--dd'

[Junio C Hamano]

Sergey Organov <sorganov@gmail.com> writes:

> '--dd' only makes sense for 'git log' and 'git show', so add it to
> __git_log_show_options which is referenced in the completion for these
> two commands.
>
> Signed-off-by: Sergey Organov <sorganov@gmail.com>
> ---
>  contrib/completion/git-completion.bash | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 133ec92bfae7..ca4fa39f3ff8 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -2042,7 +2042,7 @@ __git_log_shortlog_options="
>  "
>  # Options accepted by log and show
>  __git_log_show_options="
> -	--diff-merges --diff-merges= --no-diff-merges --remerge-diff
> +	--diff-merges --diff-merges= --no-diff-merges --dd --remerge-diff
>  "
>  
>  __git_diff_merges_opts="off none on first-parent 1 separate m combined c dense-combined cc remerge r"

Quite straight-forward.  I am kind of surprised that we do not have
to list "--cc" here.  Perhaps it is so short and common that people
do not need completion help?

But that is not a new problem caused by this series, so it is OK.

Unless "--cc" gets completed without being listed here, using some
automation like the "--git-completion-helper" option, in which case
we may want to see if we can remove all of the above and complete
them the same way as "--cc" gets completed.  I didn't check.

Thanks.

Re: [PATCH v3 3/3] completion: complete '--dd'

[Sergey Organov]

Junio C Hamano <gitster@pobox.com> writes:

> Sergey Organov <sorganov@gmail.com> writes:
>
>> '--dd' only makes sense for 'git log' and 'git show', so add it to
>> __git_log_show_options which is referenced in the completion for these
>> two commands.
>>
>> Signed-off-by: Sergey Organov <sorganov@gmail.com>
>> ---
>>  contrib/completion/git-completion.bash | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
>> index 133ec92bfae7..ca4fa39f3ff8 100644
>> --- a/contrib/completion/git-completion.bash
>> +++ b/contrib/completion/git-completion.bash
>> @@ -2042,7 +2042,7 @@ __git_log_shortlog_options="
>>  "
>>  # Options accepted by log and show
>>  __git_log_show_options="
>> -	--diff-merges --diff-merges= --no-diff-merges --remerge-diff
>> +	--diff-merges --diff-merges= --no-diff-merges --dd --remerge-diff
>>  "
>>  
>>  __git_diff_merges_opts="off none on first-parent 1 separate m combined c dense-combined cc remerge r"
>
> Quite straight-forward.  I am kind of surprised that we do not have
> to list "--cc" here.  Perhaps it is so short and common that people
> do not need completion help?
>
> But that is not a new problem caused by this series, so it is OK.
>
> Unless "--cc" gets completed without being listed here, using some
> automation like the "--git-completion-helper" option, in which case
> we may want to see if we can remove all of the above and complete
> them the same way as "--cc" gets completed.  I didn't check.

I checked, though with rather old 2.25.1 running on my Ubuntu, and it
is not completed.

I think that it's still a good idea to add --cc to completions, so that
it's there in the suggested completion list, for the sake of
discoverability. That's why I bothered to add --dd to the completions.

Thanks,
-- Sergey Organov


>
> Thanks.

[PATCH v4 0/3] diff-merges: introduce '--dd' option

[Sergey Organov]

This new convenience option requests full diff with respect to first
parent, so that

  git log --dd

will output diff with respect to first parent for every commit,
universally, no matter how many parents the commit turns out to have.

Gives user quick and universal way to see what changes, exactly, were
brought to a branch by merges as well as by regular commits.

'--dd' is implemented as pure synonym for "--diff-merges=first-parent
--patch".

The first commit in the series tweaks diff-merges documentation a bit,
and is valuable by itself. It's put here as '--dd' implementation
commit depends on it in its documentation part.

Note: the need for this new convenience option mostly emerged from
denial by the community of patches that modify '-m' behavior to imply
'-p' as the rest of similar options (such as --cc) do. So, basically,
'--dd' is what '-m' should have been to be more useful.

Updates in v4:

  * Removed "(which see)" reference from documentation that caused
    confusion.

  * Removed explanation why it's --dd and not simply -d from commit
    message.

  * Refined --remerge-diff short description according to Junio and
    Elijah comments.

  * Added explanation of --dd purpose.

  * Fixed style and syntax of "on,m::" description.

Updates in v3:

  * Option renamed from '-d' to '--dd' due to Junio overpowering
    request to keep short-and-sweet '-d' reserved for another (yet
    unspecified) use.

  * Added completion of '--dd' to git-completion.bash.

Updates in v2:

  * Reordered documentation for diff-merges formats in accordance with
    Junio recommendation.

  * Removed clarification of surprising -m behavior due to controversy
    with Junio on how exactly it should look like.

Sergey Organov (3):
  diff-merges: improve --diff-merges documentation
  diff-merges: introduce '--dd' option
  completion: complete '--dd'

 Documentation/diff-options.txt         | 105 ++++++++++++++-----------
 Documentation/git-log.txt              |   4 +-
 contrib/completion/git-completion.bash |   2 +-
 diff-merges.c                          |   3 +
 t/t4013-diff-various.sh                |   8 ++
 5 files changed, 73 insertions(+), 49 deletions(-)

Interdiff against v3:
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index f80d493dd4c8..23f95e6172b9 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -45,7 +45,7 @@ endif::git-format-patch[]
 ifdef::git-log[]
 -m::
 	Show diffs for merge commits in the default format. This is
-	similar to '--diff-merges=on' (which see) except `-m` will
+	similar to '--diff-merges=on', except `-m` will
 	produce no output unless `-p` is given as well.
 
 -c::
@@ -62,7 +62,7 @@ ifdef::git-log[]
 	Shortcut for '--diff-merges=first-parent -p'.
 
 --remerge-diff::
-	Produce diff against re-merge.
+	Produce remerge-diff output for merge commits.
 	Shortcut for '--diff-merges=remerge -p'.
 
 --no-diff-merges::
@@ -83,7 +83,7 @@ off, none::
 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
+	`log.diffMerges` configuration variable, whose default value
 	is `separate`.
 +
 first-parent, 1::
-- 
2.25.1

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

[Sergey Organov]

* 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 | 100 ++++++++++++++++++---------------
 Documentation/git-log.txt      |   2 +-
 2 files changed, 55 insertions(+), 47 deletions(-)

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 9f33f887711d..69065c0e90a8 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', 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 remerge-diff output for merge commits.
+	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
-	`log.diffMerges` configuration parameter, which default value
+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 variable, whose 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`
-- 
2.25.1

[PATCH v4 2/3] diff-merges: introduce '--dd' option

[Sergey Organov]

This option provides a shortcut to request diff with respect to first
parent for any kind of commit, universally. It's implemented as pure
synonym for "--diff-merges=first-parent --patch".

Gives user quick and universal way to see what changes, exactly, were
brought to a branch by merges as well as by regular commits.

Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
 Documentation/diff-options.txt | 5 +++++
 Documentation/git-log.txt      | 2 +-
 diff-merges.c                  | 3 +++
 t/t4013-diff-various.sh        | 8 ++++++++
 4 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 69065c0e90a8..23f95e6172b9 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -56,6 +56,11 @@ ifdef::git-log[]
 	Produce dense combined diff output for merge commits.
 	Shortcut for '--diff-merges=dense-combined -p'.
 
+--dd::
+	Produce diff with respect to first parent for both merge and
+	regular commits.
+	Shortcut for '--diff-merges=first-parent -p'.
+
 --remerge-diff::
 	Produce remerge-diff output for merge commits.
 	Shortcut for '--diff-merges=remerge -p'.
diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 9b7ec96e767a..579682172fe4 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -120,7 +120,7 @@ By default, `git log` does not generate any diff output. The options
 below can be used to show the changes made by each commit.
 
 Note that unless one of `--diff-merges` variants (including short
-`-m`, `-c`, and `--cc` options) is explicitly given, merge commits
+`-m`, `-c`, `--cc`, and `--dd` options) is explicitly given, merge commits
 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
diff --git a/diff-merges.c b/diff-merges.c
index ec97616db1df..45507588a279 100644
--- a/diff-merges.c
+++ b/diff-merges.c
@@ -131,6 +131,9 @@ int diff_merges_parse_opts(struct rev_info *revs, const char **argv)
 	} else if (!strcmp(arg, "--cc")) {
 		set_dense_combined(revs);
 		revs->merges_imply_patch = 1;
+	} else if (!strcmp(arg, "--dd")) {
+		set_first_parent(revs);
+		revs->merges_imply_patch = 1;
 	} else if (!strcmp(arg, "--remerge-diff")) {
 		set_remerge_diff(revs);
 		revs->merges_imply_patch = 1;
diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index 5de1d190759f..4b474808311e 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -473,6 +473,14 @@ test_expect_success 'log --diff-merges=on matches --diff-merges=separate' '
 	test_cmp expected actual
 '
 
+test_expect_success 'log --dd matches --diff-merges=1 -p' '
+	git log --diff-merges=1 -p master >result &&
+	process_diffs result >expected &&
+	git log --dd master >result &&
+	process_diffs result >actual &&
+	test_cmp expected actual
+'
+
 test_expect_success 'deny wrong log.diffMerges config' '
 	test_config log.diffMerges wrong-value &&
 	test_expect_code 128 git log
-- 
2.25.1

[PATCH v4 3/3] completion: complete '--dd'

[Sergey Organov]

'--dd' only makes sense for 'git log' and 'git show', so add it to
__git_log_show_options which is referenced in the completion for these
two commands.

Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
 contrib/completion/git-completion.bash | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 133ec92bfae7..ca4fa39f3ff8 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2042,7 +2042,7 @@ __git_log_shortlog_options="
 "
 # Options accepted by log and show
 __git_log_show_options="
-	--diff-merges --diff-merges= --no-diff-merges --remerge-diff
+	--diff-merges --diff-merges= --no-diff-merges --dd --remerge-diff
 "
 
 __git_diff_merges_opts="off none on first-parent 1 separate m combined c dense-combined cc remerge r"
-- 
2.25.1

Re: [PATCH v4 0/3] diff-merges: introduce '--dd' option

[Junio C Hamano]

Sergey Organov <sorganov@gmail.com> writes:

> Updates in v4:
>
>   * Removed "(which see)" reference from documentation that caused
>     confusion.
>
>   * Removed explanation why it's --dd and not simply -d from commit
>     message.
>
>   * Refined --remerge-diff short description according to Junio and
>     Elijah comments.
>
>   * Added explanation of --dd purpose.
>
>   * Fixed style and syntax of "on,m::" description.

Will replace.  Thanks.