histedit: make verification configurable
authorMateusz Kwapich <mitrandir@fb.com>
Tue, 17 Nov 2015 16:37:26 -0800
changeset 27082 4898e442f392
parent 27081 37290f2f2c3b
child 27083 6d5d7ac41ef4
histedit: make verification configurable Before we can add a 'base' action to histedit need to change verification so that action can specify which steps of verification should run for it. Also it's everything we need for the exec and stop actions implementation. I thought about baking verification into each histedit action (so each of them is responsible for verifying its constraints) but it felt wrong because: - every action would need to know its context (eg. the list of all other actions) - a lot of duplicated work will be added - each action will iterate through all others - the steps of the verification would need to be extracted and named anyway in order to be reused The verifyrules function grows too big now. I plan to refator it in one of the next series.
hgext/histedit.py
tests/test-histedit-arguments.t
tests/test-histedit-commute.t
--- a/hgext/histedit.py	Fri Nov 13 18:31:58 2015 +0800
+++ b/hgext/histedit.py	Tue Nov 17 16:37:26 2015 -0800
@@ -334,6 +334,24 @@
             raise error.Abort(_('unknown changeset %s listed') % rulehash[:12])
         return cls(state, node)
 
+    def constraints(self):
+        """Return a set of constrains that this action should be verified for
+
+           Available constraints:
+               noduplicates - aborts if there are multiple rules for one node
+               noother - abort if the node doesn't belong to edited stack
+        """
+
+        return set(['noduplicates', 'noother'])
+
+    def nodetoverify(self):
+        """Returns a node associated with the action that will be used for
+        verification purposes.
+
+        If the action doesn't correspond to node it should return None
+        """
+        return self.node
+
     def run(self):
         """Runs the action. The default behavior is simply apply the action's
         rulectx onto the current parentctx."""
@@ -810,7 +828,7 @@
             f.close()
         rules = [l for l in (r.strip() for r in rules.splitlines())
                  if l and not l.startswith('#')]
-        rules = verifyrules(rules, repo, [repo[c] for [_a, c] in state.rules])
+        rules = verifyrules(rules, state, [repo[c] for [_a, c] in state.rules])
         state.rules = rules
         state.write()
         return
@@ -891,7 +909,7 @@
             f.close()
         rules = [l for l in (r.strip() for r in rules.splitlines())
                  if l and not l.startswith('#')]
-        rules = verifyrules(rules, repo, ctxs)
+        rules = verifyrules(rules, state, ctxs)
 
         parentctxnode = repo[root].parents()[0].node()
 
@@ -1039,34 +1057,42 @@
 
     return rules
 
-def verifyrules(rules, repo, ctxs):
+def verifyrules(rules, state, ctxs):
     """Verify that there exists exactly one edit rule per given changeset.
 
     Will abort if there are to many or too few rules, a malformed rule,
     or a rule on a changeset outside of the user-given range.
     """
+    known_constraints = ['noother', 'noduplicates']
     parsed = []
     expected = set(c.hex() for c in ctxs)
     seen = set()
     for r in rules:
         if ' ' not in r:
             raise error.Abort(_('malformed line "%s"') % r)
-        action, rest = r.split(' ', 1)
-        ha = rest.strip().split(' ', 1)[0]
-        try:
-            ha = repo[ha].hex()
-        except error.RepoError:
-            raise error.Abort(_('unknown changeset %s listed') % ha[:12])
-        if ha not in expected:
-            raise error.Abort(
-                _('may not use changesets other than the ones listed'))
-        if ha in seen:
-            raise error.Abort(_('duplicated command for changeset %s') %
-                    ha[:12])
-        seen.add(ha)
-        if action not in actiontable or action.startswith('_'):
-            raise error.Abort(_('unknown action "%s"') % action)
-        parsed.append([action, ha])
+        verb, rest = r.split(' ', 1)
+
+        if verb not in actiontable or verb.startswith('_'):
+            raise error.Abort(_('unknown action "%s"') % verb)
+        action = actiontable[verb].fromrule(state, rest)
+        constraints = action.constraints()
+        for constraint in constraints:
+            if constraint not in known_constraints:
+                error.Abort(_('unknown constraint "%s"') % constraint)
+
+        nodetoverify = action.nodetoverify()
+        if nodetoverify is not None:
+            ha = node.hex(nodetoverify)
+            if 'noother' in constraints and ha not in expected:
+                raise error.Abort(
+                    _('may not use "%s" with changesets '
+                      'other than the ones listed') % verb)
+            if 'noduplicates' in constraints and ha in seen:
+                raise error.Abort(_('duplicated command for changeset %s') %
+                        ha[:12])
+            seen.add(ha)
+            rest = ha
+        parsed.append([verb, rest])
     missing = sorted(expected - seen)  # sort to stabilize output
     if missing:
         raise error.Abort(_('missing rules for changeset %s') %
--- a/tests/test-histedit-arguments.t	Fri Nov 13 18:31:58 2015 +0800
+++ b/tests/test-histedit-arguments.t	Tue Nov 17 16:37:26 2015 -0800
@@ -175,7 +175,7 @@
   > pick c8e68270e35a 3 four
   > pick 08d98a8350f3 4 five
   > EOF
-  abort: may not use changesets other than the ones listed
+  abort: may not use "pick" with changesets other than the ones listed
   [255]
 
 Test malformed line
--- a/tests/test-histedit-commute.t	Fri Nov 13 18:31:58 2015 +0800
+++ b/tests/test-histedit-commute.t	Tue Nov 17 16:37:26 2015 -0800
@@ -281,7 +281,7 @@
   > pick de71b079d9ce e
   > pick 38b92f448761 c
   > EOF
-  abort: may not use changesets other than the ones listed
+  abort: may not use "pick" with changesets other than the ones listed
   $ hg log --graph
   @  changeset:   7:803ef1c6fcfd
   |  tag:         tip