public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Sergey Organov <sorganov@gmail•com>
To: Junio C Hamano <gitster@pobox•com>
Cc: git@vger•kernel.org
Subject: Re: [PATCH 1/2] diff-merges: improve --diff-merges documentation
Date: Tue, 12 Sep 2023 10:37:09 +0300	[thread overview]
Message-ID: <87ttrzhmfu.fsf@osv.gnss.ru> (raw)
In-Reply-To: <xmqqfs3ktnvo.fsf@gitster.g> (Junio C. Hamano's message of "Mon, 11 Sep 2023 14:12:59 -0700")

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

  reply	other threads:[~2023-09-12  7:37 UTC|newest]

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

Reply instructions:

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

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

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

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

  git send-email \
    --in-reply-to=87ttrzhmfu.fsf@osv.gnss.ru \
    --to=sorganov@gmail$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=gitster@pobox$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox