patch: rewrite reversehunks (issue5337)
authorJun Wu <quark@fb.com>
Tue, 20 Jun 2017 23:22:38 -0700
changeset 32997 66117dae87f9
parent 32996 41b081ac2145
child 32998 8dc62c97a665
patch: rewrite reversehunks (issue5337) The old reversehunks code accesses "crecord.uihunk._hunk", which is the raw recordhunk without crecord selection information, therefore "revert -i" cannot revert individual lines, aka. issue5337. The patch rewrites related logic to return the right reverse hunk for revert. Namely, 1. "fromline" and "toline" are correctly swapped [1] 2. crecord.uihunk generates a correct reverse hunk [2] Besides, reversehunks(hunks) will no longer modify its input "hunks", which is more expected. [1]: To explain why "fromline" and "toline" need to be swapped, take the following example: $ cat > a <<EOF > 1 > 2 > 3 > 4 > EOF $ cat > b <<EOF > 2 > 3 > 5 > EOF $ diff a b 1d0 <---- "1" is "fromline" and "0" is "toline" < 1 and they are swapped if diff from the reversed direction 4c3 | < 4 | --- | > 5 | | $ diff b a | 0a1 <---------+ > 1 3c4 <---- also "4c3" gets swapped to "3c4" < 5 --- > 4 [2]: This is a bit tricky. For example, given a file which is empty in working parent but has 3 lines in working copy, and the user selection: select hunk to discard [x] +1 [ ] +2 [x] +3 The user intent is to drop "1" and "3" in working copy but keep "2", so the reverse patch would be something like: -1 2 (2 is a "context line") -3 We cannot just take all selected lines and swap "-" and "+", which will be: -1 -3 That patch won't apply because of "2". So the correct way is to insert "2" as a "context line" by inserting it first then deleting it: -2 +2 Therefore, the correct revert patch is: -1 -2 +2 -3 It could be reordered to look more like a common diff hunk: -1 -2 -3 +2 Note: It's possible to return multiple hunks so there won't be lines like "-2", "+2". But the current implementation is much simpler. For deletions, like the working parent has "1\n2\n3\n" and it was changed to empty in working copy: select hunk to discard [x] -1 [ ] -2 [x] -3 The user intent is to drop the deletion of 1 and 3 (in other words, keep those lines), but still delete "2". The reverse patch is meant to be applied to working copy which is empty. So the patch would be: +1 +3 That is to say, there is no need to special handle the unselected "2" like the above insertion case.
mercurial/crecord.py
mercurial/patch.py
--- a/mercurial/crecord.py	Wed Jun 21 10:46:18 2017 +0200
+++ b/mercurial/crecord.py	Tue Jun 20 23:22:38 2017 -0700
@@ -427,6 +427,54 @@
         self.pretty(x)
         return x.getvalue()
 
+    def reversehunk(self):
+        """return a recordhunk which is the reverse of the hunk
+
+        Assuming the displayed patch is diff(A, B) result. The returned hunk is
+        intended to be applied to B, instead of A.
+
+        For example, when A is "0\n1\n2\n6\n" and B is "0\n3\n4\n5\n6\n", and
+        the user made the following selection:
+
+                 0
+            [x] -1           [x]: selected
+            [ ] -2           [ ]: not selected
+            [x] +3
+            [ ] +4
+            [x] +5
+                 6
+
+        This function returns a hunk like:
+
+                 0
+                -3
+                -4
+                -5
+                +1
+                +4
+                 6
+
+        Note "4" was first deleted then added. That's because "4" exists in B
+        side and "-4" must exist between "-3" and "-5" to make the patch
+        applicable to B.
+        """
+        dels = []
+        adds = []
+        for line in self.changedlines:
+            text = line.linetext
+            if line.applied:
+                if text[0] == '+':
+                    dels.append(text[1:])
+                elif text[0] == '-':
+                    adds.append(text[1:])
+            elif text[0] == '+':
+                dels.append(text[1:])
+                adds.append(text[1:])
+        hunk = ['-%s' % l for l in dels] + ['+%s' % l for l in adds]
+        h = self._hunk
+        return patchmod.recordhunk(h.header, h.toline, h.fromline, h.proc,
+                                   h.before, hunk, h.after)
+
     def __getattr__(self, name):
         return getattr(self._hunk, name)
 
--- a/mercurial/patch.py	Wed Jun 21 10:46:18 2017 +0200
+++ b/mercurial/patch.py	Tue Jun 20 23:22:38 2017 -0700
@@ -959,6 +959,18 @@
         rem = len([h for h in hunk if h[0] == '-'])
         return add, rem
 
+    def reversehunk(self):
+        """return another recordhunk which is the reverse of the hunk
+
+        If this hunk is diff(A, B), the returned hunk is diff(B, A). To do
+        that, swap fromline/toline and +/- signs while keep other things
+        unchanged.
+        """
+        m = {'+': '-', '-': '+'}
+        hunk = ['%s%s' % (m[l[0]], l[1:]) for l in self.hunk]
+        return recordhunk(self.header, self.toline, self.fromline, self.proc,
+                          self.before, hunk, self.after)
+
     def write(self, fp):
         delta = len(self.before) + len(self.after)
         if self.after and self.after[-1] == '\\ No newline at end of file\n':
@@ -1493,7 +1505,7 @@
      c
      1
      2
-    @@ -1,6 +2,6 @@
+    @@ -2,6 +1,6 @@
      c
      1
      2
@@ -1501,26 +1513,17 @@
     +4
      5
      d
-    @@ -5,3 +6,2 @@
+    @@ -6,3 +5,2 @@
      5
      d
     -lastline
 
     '''
 
-    from . import crecord as crecordmod
     newhunks = []
     for c in hunks:
-        if isinstance(c, crecordmod.uihunk):
-            # curses hunks encapsulate the record hunk in _hunk
-            c = c._hunk
-        if isinstance(c, recordhunk):
-            for j, line in enumerate(c.hunk):
-                if line.startswith("-"):
-                    c.hunk[j] = "+" + c.hunk[j][1:]
-                elif line.startswith("+"):
-                    c.hunk[j] = "-" + c.hunk[j][1:]
-            c.added, c.removed = c.removed, c.added
+        if util.safehasattr(c, 'reversehunk'):
+            c = c.reversehunk()
         newhunks.append(c)
     return newhunks