Re: [PATCH 6/6] diff-merges: let -m imply -p

[Sergey Organov <sorganov@gmail.com>]

Junio C Hamano <gitster@pobox.com> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> Sergey Organov <sorganov@gmail.com> writes:
>>
>>> Fix long standing inconsistency between -c/--cc that do imply -p, on
>>> one side, and -m that did not imply -p, on the other side.
>>>
>>> After this patch
>>>
>>>   git log -m
>>>
>>> will start to produce diffs without need to provide -p as well, that
>>> improves both consistency and usability. It gets even more useful if
>>> one sets "log.diffMerges" configuration variable to "first-parent" to
>>> force -m produce usual diff with respect to first parent only.
>>
>> Please make sure that you clearly state that you do not blindly
>> force --patch output in the proposed log message.  Explicitly
>> mentioning that "git log --stat -m" would not give a patch but just
>> diffstat would be assuring.
>
> Also, avoid "-p" in the title.  "let -m imply diff generation" might
> be a good compromise.
>
> What --cc/-c implies is to show some kind of diff for merges
> (dense_combined_merges, combine_merges and !ignore_merges are the
> members of the revs field that controls how merge commits) and they
> ask for specific kind of diff is shown.  So "-c/--cc imply -p" is
> quite wrong (you never get straight --patch output for merges when
> you give -c/--cc---you get combined diff).  In a sense, you could
> say -c/--cc implies -m (i.e. do show some kind of diff for merges).
>
> Taken together, perhaps:
>
>     Subject: diff: let -m imply diff generation
>
>     The "-c/--cc" options to "git log" asks for merges to be shown
>     with patch-like output, implicitly enabling the "-m" option
>     (which is used to tell "do not ignore merge commits when showing
>     patches).  However, the opposite is not true; giving "-m" alone
>     does not tell "git log" that the user wants some form of patches.
>     
>     Make "-m" imply "we want some form of diff output", so that "git
>     log -m" would behave identically to "git log -m -p".  When the
>     user explicitly asks for what kind of diff output is desired,
>     e.g. "git log -m --stat", there is no need to imply anything,
>     specifically, do NOT blindly turn on the "-p: option to turn it
>     into "git log -m --stat -p:.
>
> or something like that.

While working on this, I've added more tests and explanations to the
next re-roll as I've promised, but I didn't change the subject nor did I
try to explain that much, as my original subject is consistent with
current Git documentation, concise, and straight to the point.

"<opt> implies -p" -- that's how current Git manual describes the
behavior, and that's how I described it in these patches. Whatever
"imply" actually means for -c/--cc, it now means for -m as well, and
that's the essence of the patches, so I stand for my original subject.

I think that if we realize that "<opt> implies -p" is not precise enough
description of actual behavior, it should be a separate topic of
documentation improvements, and doesn't belong to these series anyway.

Thanks,
-- Sergey Organov