Subject: Re: [PATCH v1 24/27] doc/git-log: describe new --diff-merges options

From: Sergey Organov <[email-masked]>
Date: Thu, 03 Dec 2020 22:34:08 +0300

Elijah Newren <[email-masked]> writes:

> On Sun, Nov 8, 2020 at 1:46 PM Sergey Organov <[email-masked]> wrote:
>>
>> Describe all the new --diff-merges options in the git-log.txt
>>
>> Signed-off-by: Sergey Organov <[email-masked]>
>> ---
>>  Documentation/git-log.txt | 79 +++++++++++++++++++++++----------------
>>  1 file changed, 46 insertions(+), 33 deletions(-)
>>
>> diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
>> index 2b8ac5ff882a..de498a189646 100644
>> --- a/Documentation/git-log.txt
>> +++ b/Documentation/git-log.txt
>> @@ -120,45 +120,58 @@ DIFF FORMATTING
>>  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 `-c`, `--cc`, or `-m` is given, merge commits
>> -will never 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 merges are treated like normal
>> -single-parent commits (this can be overridden by providing a
>> -combined-diff option or with `--no-diff-merges`).
>> +Note that unless one of `--diff-merges` variants (including short
>> +`-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
>> +`--diff-merges=first-parent` is implied.
>>
>> --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.
>> -
>> ---cc::
>> -       This flag implies the `-c` option and further compresses the
>> -       patch output by omitting uninteresting hunks whose contents in
>> -       the parents have only two variants and the merge result picks
>> -       one of them without modification.
>> +--diff-merges=(off|none|first-parent|separate|combined|dense-combined)::
>> +--no-diff-merges::
>> +       Specify diff format to be used for merge commits. This has no
>> +       effect unless diff output is enabled in the first place (e.g.,
>> +       with `--patch` option.)
>
> This seems inconsistent with c7eaf8b4c3 ("log: when --cc is given,
> default to -p unless told otherwise", 2015-08-20); shouldn't these
> imply -p?

Looks like it is indeed, and worse yet, the patch series don't handle
this optimally due to my ignorance of this behavior. I think I need to
make changes to imply -p whenever some of diff-merge options (except
--diff-merges=none) are explicitly given, cause it's now, after these
patches, is unclear why only combined diffs imply -p, and not, say,
--diff-merges=first-parent.

Nice catch, thanks!

BTW, in the original, why only "git log" is affected? Ah, probably
because "git show" has -p by default?

While we are at it, why do we have "git show" at all? How is it
essentially different from "git log -n1 --first-parent ..."? I mean,
shouldn't it effectively be just an alias?

>
>> ++
>> +--diff-merges=(off|none):::
>> +--no-diff-merges:::
>> +       (default) Disable output of diffs for merge commits. Useful to
>> +       override implied value.
>> ++
>> +--diff-merges=first-parent:::
>> +       This option makes merge commits show the full diff with
>> +       respect to the first parent only, exactly like  regular
>> +       commits.
>
> Not sure that "exactly like regular commits" is helpful here; I'd
> personally rather cut those four words out.  I'm worried it'll be
> taken not as an implementation explanation, but as a "this treats
> merge commits in the natural way that regular commits are" which users
> may mistakenly translate to "it shows what changes the user manually
> made as part of the commit" which is not at all the correct mapping.

Dunno. Don't have strict preference here. Git has no idea how the
changes in a commit have been made in the first place. Changes are just
changes.

To my excuse, I took this from git:5fbb4bc191 that has:

+Note that unless one of `-c`, `--cc`, or `-m` is given, merge commits
+will never 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 merges are treated like normal
+single-parent commits (this can be overridden by providing a
+combined-diff option or with `--no-diff-merges`).

Thanks,
-- Sergey Organov