Subject: Re: [PATCH v1 00/27] git-log: implement new --diff-merge options

From: Sergey Organov <[email-masked]>
Date: Fri, 04 Dec 2020 22:23:00 +0300

Elijah Newren <[email-masked]> writes:

> On Thu, Dec 3, 2020 at 11:48 AM Sergey Organov <[email-masked]> wrote:
>>
>> Elijah Newren <[email-masked]> writes:
>>
>> > On Sun, Nov 8, 2020 at 1:43 PM Sergey Organov <[email-masked]> wrote:
>> >>
>> >> These patch series implement new set of options governing the diff output
>> >> of merge commits, all under the umbrella of the single
>> >> --diff-merges=<mode>
>> >> option. Most of the new options being synonyms for -m/-c/--cc options,
>> >> there is also additional functionality provided, allowing to get
>> >> the format
>> >> of "-p --first-parent" without change in history traversal that
>> >> --first-parent option causes.
>> >>
>> >> The net result of these series are the following new options:
>> >>
>> >> --diff-merges=   |  old equivalent
>> >> -----------------+----------------
>> >> first-parent     | --first-parent (only format implications)
>> >> separate         | -m
>> >> combined         | -c
>> >> dense-combined   | --cc
>> >
>> > Interesting.  I have some local patches implementing another choice,
>> > with the new flag --remerge-diff.  This flag will cause `git show` or
>> > `git log` to automatically remerge the two parents in a 2-parent merge
>> > commit, and then diff the merge commit against that automatic merge,
>> > showing the result.  Thus, the diff for a merge commit is likely to be
>> > empty if the merge was clean, and is likely to show the removal of
>> > conflict markers if the merge was not clean.
>> >
>> > I'm curious how it'd interact with this new option.  Would it also get
>> > a name, e.g. --diff-merges=remerge-diff?  Feels like a mouthful, but I
>> > can't come up with anything better.
>>
>> Maybe, --diff-merges=remerge?
>>
>> >
>> > Also, I'm curious how it'd interact with another option I added, named
>> > --remerge-diff-only.  This latter option modifies revision traversal
>> > in that it skips octopus merges, root commits, and single parent
>> > commits IF no cherry-pick or revert information can be found.  If it
>> > finds a 2-parent merge commit, it behaves like --remerge-diff.  If it
>> > finds a 1-parent commit with cherry-pick or revert information, it'll
>> > do an in memory repeat of that cherry-pick (or revert) and then diff
>> > the actual commit against what the automatic cherry-pick would
>> > perform.  Again, that likely means an empty diff if the automatic
>> > cherry-pick was clean, and showing any changes made by the user to
>> > complete the original cherry-pick (such as deleting conflict markers
>> > and picking which chunks from which side to keep) if the automatic
>> > cherry-pick was not clean.  (I suspect --remerge-diff-only is much
>> > more likely to be used with `git show` than with `git log`.)  Anyway,
>> > your changes seem to suggest that anything relating to how diffs for
>> > merges are handled should be documented in the same section, but
>> > --remerge-diff-only doesn't fit.  And it'd seem odd to have
>> > --remerge-diff and --remerge-diff-only not show up in adjacently but
>> > be split into separate sections.  Any ideas?
>>
>> Sounds like commits limiting option to me. I think it could be named by
>> its limiting behavior only, say, --remerges. Then it will imply
>> --diff-merges=remerge, that'd allow user to re-define diff format if she
>> needs to.
>
> It is commit limiting, but the focus is more on the behavioral change
> in how diffs are shown:
>   * for 2-parent merges
>   * for single-parent commits with cherry-pick or revert information
> and acknowledging that since it has _altered_ the normal way of
> showing diffs for a number of single-parent commits, that it'd be
> confusing to show normal diffs of unaffected commits (how would you be
> able to tell what type of diff you're looking at if both appear in the
> log?).  Thus, it does commit limiting to only select commits which
> will make use of the new diff type.

That's how you currently look at it.

For me it looks like pure commit limiting with these criteria might be
useful by itself, and with my suggestion one could then achieve it
using, say:

--remerge-diff-only --diff-merges=off

>
> (I suspect it will be more common for folks to use the
> --remerge-diff-only option, or whatever we end up calling it, with
> `git show` where the commit limiting doesn't matter -- but I have used
> it with log to go looking for "evil" reverts/cherry-picks that might
> have occurred in history.)

What you describe is complex enough to doubt it could be entirely
described by option name, so shorter --evils might be better choice
in this case.

Overall, if you add --diff-merges=remerge as a new diff format, and then
--evils that implies the former, then it seems like all possible
use-cases will be covered, and you have short option name for the most
useful case.

-- Sergey Organov