Thread (1 message) 1 message, 1 author, 2023-09-12

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

From: Sergey Organov <hidden>
Date: 2023-09-12 07:37:17

Junio C Hamano [off-list ref] writes:
Sergey Organov [off-list ref] writes:
quoted
 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.
quoted
 --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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help