From patchwork Wed Jun 21 06:23:39 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [V2] patch: rewrite reversehunks (issue5337) From: Jun Wu X-Patchwork-Id: 21580 Message-Id: To: Date: Tue, 20 Jun 2017 23:23:39 -0700 # HG changeset patch # User Jun Wu # Date 1498026158 25200 # Tue Jun 20 23:22:38 2017 -0700 # Node ID f54272510c4ec915bc13b1b9732d148cd1decdd0 # Parent 0ce2cbebd74964ffe61e79de8941461bccc9371b # Available At https://bitbucket.org/quark-zju/hg-draft # hg pull https://bitbucket.org/quark-zju/hg-draft -r f54272510c4e 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 < 1 > 2 > 3 > 4 > EOF $ cat > b < 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. diff --git a/mercurial/crecord.py b/mercurial/crecord.py --- a/mercurial/crecord.py +++ b/mercurial/crecord.py @@ -428,4 +428,52 @@ class uihunk(patchnode): 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) diff --git a/mercurial/patch.py b/mercurial/patch.py --- a/mercurial/patch.py +++ b/mercurial/patch.py @@ -960,4 +960,16 @@ class recordhunk(object): 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) @@ -1494,5 +1506,5 @@ def reversehunks(hunks): 1 2 - @@ -1,6 +2,6 @@ + @@ -2,6 +1,6 @@ c 1 @@ -1502,5 +1514,5 @@ def reversehunks(hunks): 5 d - @@ -5,3 +6,2 @@ + @@ -6,3 +5,2 @@ 5 d @@ -1509,17 +1521,8 @@ def reversehunks(hunks): ''' - 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