[Commented On] D8504: diff: add experimental support for "merge diffs"

[martinvonz (Martin von Zweigbergk)]

martinvonz added a comment.


  Looking forward to using this!

INLINE COMMENTS

> commands.py:2413
> +            b'merge',
> +            b'',
> +            _(

How about making this boolean-valued instead? I'd expect `hg diff --merge -c <some merge commit>` to work. That will also make it easy to provide a config option to always enable it. (I don't ever want the current behavior for `-c` with merge commits.)

> commands.py:2503-2518
> +    elif mergerev:
> +        repo = scmutil.unhidehashlikerevs(repo, [change], b'nowarn')
> +        mctx = scmutil.revsingle(repo, mergerev, None)
> +        if mctx.p2().node() == nullid:
> +            raise error.Abort(_(b'--merge requires a merge commit'))
> +        ctx1 = mctx.p1()
> +        ctx2 = mctx.p2()

Seems like it would be cleaner to combine these two.

> commands.py:2506-2507
> +        mctx = scmutil.revsingle(repo, mergerev, None)
> +        if mctx.p2().node() == nullid:
> +            raise error.Abort(_(b'--merge requires a merge commit'))
> +        ctx1 = mctx.p1()

Or should we allow non-merge commits so one can always pass `--merge` without thinking (e.g. in `[defaults]`)?

> commands.py:2512
> +        wctx.setbase(ctx1)
> +        stats = mergemod.merge(ctx2, wc=wctx)
> +        ctx1 = wctx

Want to set a specific merge tool here while running this?

> test-diff-change.t:187
> +  +11
> +
>    $ cd ..

We should have at least one test showing a "four-way diff" with base, left, right, and resolved states.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8504/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8504

To: durin42, #hg-reviewers
Cc: martinvonz, mercurial-patches
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mercurial-scm.org/pipermail/mercurial-patches/attachments/20200508/85e5b7af/attachment-0002.html>