[PATCH 1 of 4] histedit: make verification configurable

Mateusz Kwapich mitrandir at fb.com
Fri Nov 13 01:45:56 UTC 2015


# HG changeset patch
# User Mateusz Kwapich <mitrandir at fb.com>
# Date 1447375340 28800
#      Thu Nov 12 16:42:20 2015 -0800
# Node ID 659c8c265d8516a5685ac226abc91aaa79028202
# Parent  f9984f76fd90e439221425d751e29bae17bec995
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

diff --git a/hgext/histedit.py b/hgext/histedit.py
--- a/hgext/histedit.py
+++ b/hgext/histedit.py
@@ -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."""
@@ -807,7 +825,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
@@ -888,7 +906,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()
 
@@ -1038,7 +1056,7 @@
 
     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,
@@ -1050,22 +1068,25 @@
     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()
+        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') %
diff --git a/tests/test-histedit-arguments.t b/tests/test-histedit-arguments.t
--- a/tests/test-histedit-arguments.t
+++ b/tests/test-histedit-arguments.t
@@ -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
diff --git a/tests/test-histedit-commute.t b/tests/test-histedit-commute.t
--- a/tests/test-histedit-commute.t
+++ b/tests/test-histedit-commute.t
@@ -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


More information about the Mercurial-devel mailing list