Elijah Newren <newren@xxxxxxxxx> writes:
> On Fri, Dec 18, 2020 at 8:05 AM Sergey Organov <sorganov@xxxxxxxxx> wrote:
>>
>> Elijah Newren <newren@xxxxxxxxx> writes:
>>
>> > On Wed, Dec 16, 2020 at 10:50 AM Sergey Organov <sorganov@xxxxxxxxx> 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@xxxxxxxxx>
>> >> ---
>> >> 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?
>
> The documentation on -m was vague enough that it made me wonder.
> Fixing it would probably prevent me from having asked this question.
OK, I see, so let's try to fix the cause, please see below.
>
>> >> ++
>> >> +--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.
>
> That wasn't at all clear to me as the intent of the last sentence.
Well, actually, the right answer to your question is: -m is different
from *everything* ;-)
Maybe the confusion you got is not caused by the documentation, but
rather by some of your expectations? In particular, I wonder, how comes
--first-parent appeared in your question, that is entirely unrelated?
I mean I'm still unsure how to make this description more clear, maybe
this will do:
--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` doesn't produce any output 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.
>
> Sometimes non-native speakers use the language more accurately. This
> might be such a case, but that sentence did make me think you might be
> attempting to document past behavior as a way of helping people adjust
> to current/new behavior.
>
>> > (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.
>
> It doesn't seem like illogical behavior to me. Perhaps the view of
> -c as a diff-merges option, reinforced by placing -c right next to
> --diff-merges in the documentation, is what causes you to think so?
-c, described as selecting specific format of representation of merge
commits, suddenly enabling diff output of non-merge commits, is logical?
No. Useful? Maybe. Logical? Not to me. Just saying, see below.
> If so, maybe we should document -c (and --cc) separately after all the
> --diff-merges options, using something like the following:
>
> -c:::
> Implies both --patch and --diff-merges=combined, i.e. turn on
> patches for normal commits and show a combined diff format for merges.
The problem with this is that it'd then be preferable to describe -m
separately as well, and that's not that simple.
Overall, I'd rather change that in place, like this:
--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
`--patch`.
Thanks,
-- Sergey
