Page MenuHomePhabricator

diff: add experimental support for "merge diffs"
Needs RevisionPublic

Authored by durin42 on May 7 2020, 5:26 PM.

Details

Reviewers
marmoute
Group Reviewers
hg-reviewers
Summary

This does not currently pass tests, because in-memory merge can't
represent conflicts, which this requires. I intend to shave that yak
next.

Publishing the patch now as motivation for the preceding patches, and
in case someone has feedback on the approach.

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

durin42 created this revision.May 7 2020, 5:26 PM

Looking forward to using this!

mercurial/commands.py
2414

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

2503–2518

Seems like it would be cleaner to combine these two.

2506–2507

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

2512

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

tests/test-diff-change.t
187

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

durin42 marked 3 inline comments as done.May 11 2020, 5:34 PM
durin42 updated this revision to Diff 21341.
durin42 added inline comments.May 11 2020, 5:46 PM
mercurial/commands.py
2414

Wow, that feels obvious in hindsight. And then we can also default-true when tweakdefaults is on.

tests/test-diff-change.t
187

Not sure what you mean. Can you elaborate?

durin42 edited the summary of this revision. (Show Details)May 11 2020, 6:21 PM
durin42 updated this revision to Diff 21342.

For anyone curious what's left, the test currently fails like this:

--- tests/test-diff-change.t
+++ tests/test-diff-change.t.err
@@ -241,9 +241,15 @@
   ->>>>>>> other: d9e7de69eac3 - test: new file
    9
    10
+  diff -r 5010caab09f6 new-file-p2.txt
+  --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+  +++ b/new-file-p2.txt	Thu Jan 01 00:00:00 1970 +0000
+  @@ -0,0 +1,1 @@
+  +this file is new in p2 of the merge
 
 There must _NOT_ be a .hg/merge directory leftover.
   $ test ! -e .hg/merge
-
-
-  $ cd ..
+  [1]
+
+
+  $ cd ..
martinvonz added inline comments.May 11 2020, 10:57 PM
mercurial/commands.py
2502

How about calling these "pctx1" and "pctx2" instead so the same name doesn't change meaning and so you don't have to reassign "ctx2"?

tests/test-diff-change.t
187

Like the test case you added below. There, base is "8", left is "z", right is "y", and resolved is "z", if I'm reading that right.

durin42 updated this revision to Diff 21449.May 18 2020, 6:08 PM
durin42 updated this revision to Diff 21515.May 28 2020, 4:27 PM

I like the idea a lot :-)

durin42 updated this revision to Diff 21596.Jun 9 2020, 4:52 PM
marmoute requested changes to this revision.Fri, Jul 31, 12:03 PM

According to IRC discussion, this series is not ready for review, so moving it out of yadda.

This revision now requires changes to proceed.Fri, Jul 31, 12:03 PM