On Fri, Dec 4, 2020 at 11:23 AM Sergey Organov <sorganov@xxxxxxxxx> wrote:
>
> Elijah Newren <newren@xxxxxxxxx> writes:
>
> > On Thu, Dec 3, 2020 at 11:48 AM Sergey Organov <sorganov@xxxxxxxxx> wrote:
> >>
> >> Elijah Newren <newren@xxxxxxxxx> writes:
> >>
> >> > On Sun, Nov 8, 2020 at 1:43 PM Sergey Organov <sorganov@xxxxxxxxx> 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 see what you're saying, and I think there's some value in it. But I
think there's something still missing. For example, you suggest
getting the commit limiting I mention with
--remerge-diff-only --diff-merges=off
But --diff-merges is only supposed to control _merge_ commits, which I
flagged as the big impedance mismatch for my new option. Why would it
turn off diffs for non-merge commits like cherry-picks and rebases?
> >
> > (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.
Since you want things to have orthogonal subcomponents that can be
built up, let's assume we did make --remerge-diff-only be solely about
commit limiting. In that case, --evils could be gotten by specifying
a combination of flags, and --evils would just be a shorthand. What
are the flags that you would need to specify, though? In particular,
you've only named two options above and they don't cover the necessary
behavior; a third is needed:
--remerge-diff-only --diff-merges=remerge
--${DIFF_OPTION_NAME_FOR_CHERRY_PICKS_AND_REVERTS}=remerge
The first two aren't enough because --diff_merges only changes how
diffs for _merge_ commits are shown, and we need a flag for changing
how the single-parent cherry-pick and reverts are shown.
