On Tue, Feb 1, 2022 at 1:35 AM Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> wrote:
>
> On Fri, Jan 21 2022, Elijah Newren via GitGitGadget wrote:
>
> > From: Elijah Newren <newren@xxxxxxxxx>
> > [...]
> > ifdef::git-log[]
> > ---diff-merges=(off|none|on|first-parent|1|separate|m|combined|c|dense-combined|cc)::
> > +--diff-merges=(off|none|on|first-parent|1|separate|m|combined|c|dense-combined|cc|remerge|r)::
> > --no-diff-merges::
> > Specify diff format to be used for merge commits. Default is
> > {diff-merges-default} unless `--first-parent` is in use, in which case
> > @@ -64,6 +64,14 @@ ifdef::git-log[]
> > each of the parents. Separate log entry and diff is generated
> > for each parent.
> > +
> > +--diff-merges=remerge:::
> > +--diff-merges=r:::
> > +--remerge-diff:::
> > + With this option, two-parent merge commits are remerged to
> > + create a temporary tree object -- potentially containing files
> > + with conflict markers and such. A diff is then shown between
> > + that temporary tree and the actual merge commit.
> > ++
>
> Re some previous discussion. I really think we should add something like
> this paragraph to this:
>
> The output emitted when this option is used is subject to change, and so
> is its interaction with other options (unless explicitly
> documented). I.e. many of the same caveats as the "OUTPUT STABILITY" in
> the linkgit:git-range-diff[1] documentation describes apply here. In
> particular other diff filtering options, pathspec limitations etc. may
> not produce the expected results, as some of those may apply to the
> "real" diff of the merge, and not on the generated "remerge-diff".
>
> I think that would nicely give us permission to develop this further
> without having to think about all the option interaction etc.
>
> This is really useful right now, but I'd hate for it to get merged with
> some bug/behavior that's not obvious to us now, and it being hard to fix
> that because we'd have to consider the implicitly promised backwards
> compatibility.
Sure I can add something. I think the first sentence should be
sufficient though.
> > int saved_dcctc = 0;
> > + struct tmp_objdir *remerge_objdir = NULL;
> > +
> > + if (rev->remerge_diff) {
> > + remerge_objdir = tmp_objdir_create("remerge-diff");
> > + if (!remerge_objdir)
> > + die(_("unable to create temporary object directory"));
>
> I guess the s/die_errno/die/ here is better for now as we won't report
> the wrong errno, but also lose the common case of errno being right. But
> that can be fixed up with some other series to the tmp-objdir API.
>
> > [...]
> > +# This test is ort-specific
> > +test "${GIT_TEST_MERGE_ALGORITHM:-ort}" = ort || {
> > + skip_all="GIT_TEST_MERGE_ALGORITHM != ort"
> > + test_done
> > +}
>
> FWIW this is still on a more complex pattern that it needs to be, see
> this v1 discussion (which you seemed to ack):
>
> https://lore.kernel.org/git/CABPp-BE+4rZNP-5mT2MNOWR6y6BgEG6mt1r_qcrZtarom6aGsw@xxxxxxxxxxxxxx/
Um, I thought I made this change. How did I lose it?
Thanks for catching; will fix.
