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.
> 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/
