Re: [PATCH v2 29/33] doc/git-log: describe new --diff-merges options

[Sergey Organov <sorganov@gmail.com>]

Elijah Newren <newren@gmail.com> writes:

> On Wed, Dec 16, 2020 at 10:50 AM Sergey Organov <sorganov@gmail.com> wrote:
>>
>> Describe all the new --diff-merges options in the git-log.txt and
>> adopt description of originals accordingly.
>
> You also took care to explain interactions of options with -p that
> were previously undocumented, which is a nice bonus.  That wording
> could still be improved a bit, though, as noted below.
>
>> Signed-off-by: Sergey Organov <sorganov@gmail.com>
>> ---
>>  Documentation/git-log.txt | 85 ++++++++++++++++++++++++---------------
>>  1 file changed, 52 insertions(+), 33 deletions(-)
>>
>> diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
>> index 2b8ac5ff882a..27bc619490c6 100644
>> --- a/Documentation/git-log.txt
>> +++ b/Documentation/git-log.txt
>> @@ -120,45 +120,64 @@ 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 `first-parent` is
>> +the default format.
>
> Thanks for fixing this up.  :-)

Don't mention it :-)

>
>>
>> --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|1|separate|m|combined|c|dense-combined|cc)::
>> +--no-diff-merges::
>> +       Specify diff format to be used for merge commits. Default is
>> +       `off` unless `--first-parent` is in use, in which case
>> +       `first-parent` is the default.
>> ++
>> +--diff-merges=(off|none):::
>> +--no-diff-merges:::
>> +       Disable output of diffs for merge commits. Useful to override
>> +       implied value.
>> ++
>> +--diff-merges=first-parent:::
>> +--diff-merges=1:::
>> +       This option makes merge commits show the full diff with
>> +       respect to the first parent only.
>
> Does it imply -p?

No, none of --diff-merges options do. This one is not any special. Why
the question?

>> ++
>> +--diff-merges=separate:::
>> +--diff-merges=m:::
>> +-m:::
>> +       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. `-m` is different in that it doesn't produce
>> +       any output without `-p`.
>
> Different from what?  From --first-parent?  From flags that haven't
> been covered yet?  (-c and --cc show up below)

Well, from --diff-merges=m and --diff-merges=separate, that, as any
other --diff-merge option, do produce output (for merge commits) even
without -p.

>
>> ++
>> +--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. Historically,
>> +       `-c` enables diff output for non-merge commits as well.
>
> "Historically"?  Does that mean it doesn't anymore?

Eh, I don't think "historically" means that, but I'm not sure, being
non-native English speaker.

> (Maybe, "The short form, `-c`, also enables diff output for non-merge
> commits as well." or something like that?)

... and then try to explain why this otherwise illogical behavior is
there? I thought "historically" would cover that.

>
>> ++
>> +--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.  Historically, `--c` enables diff output for
>> +       non-merge commits as well.
>
> Same note as above.

Yep.

>
>>  --combined-all-paths::
>>         This flag causes combined diffs (used for merge commits) to
>>         list the name of the file from all parents.  It thus only has
>> -       effect when -c or --cc are specified, and is likely only
>> -       useful if filename changes are detected (i.e. when either
>> -       rename or copy detection have been requested).
>> +       effect when `--diff-merges=[dense-]combined` is in use, and
>> +       is likely only useful if filename changes are detected (i.e.
>> +       when either rename or copy detection have been requested).
>>
>> --m::
>> -       This flag makes the merge commits show the full diff like
>> -       regular commits; for each merge parent, a separate log entry
>> -       and diff is generated. An exception is that only diff against
>> -       the first parent is shown when `--first-parent` option is given;
>> -       in that case, the output represents the changes the merge
>> -       brought _into_ the then-current branch.
>> -
>> ---diff-merges=off::
>> ---no-diff-merges::
>> -       Disable output of diffs for merge commits (default). Useful to
>> -       override `-m`, `-c`, or `--cc`.
>>
>>  :git-log: 1
>>  include::diff-options.txt[]
>> --
>> 2.25.1
>
> The rest looks good.

Thanks,
-- Sergey