[PATCH 7 of 8] histedit: move all arguments check at the beginning of the command

Pierre-Yves David pierre-yves.david at ens-lyon.org
Tue Apr 16 14:20:36 CDT 2013


# HG changeset patch
# User Pierre-Yves David <pierre-yves.david at logilab.fr>
# Date 1366139833 -7200
# Node ID 1315e6aa4ba9eeaf17125269d6218c2b1a21d2e9
# Parent  1f9f0a2c3f39564814fb977303328f148521e2c3
histedit: move all arguments check at the beginning of the command

This changeset move all checks and raise related to arguments validation at the
top of the file. This gather all the logic in one place and clarify the code
doing actual works. This pave the way to splitting this gigantic function in
separated function.

A `goal` variable is introduced in the process. It hold the action to be done by
this invocation (new, continue or abort).

An invalid invocation is found in the process (the new code is a bit stricter).

diff --git a/hgext/histedit.py b/hgext/histedit.py
--- a/hgext/histedit.py
+++ b/hgext/histedit.py
@@ -426,26 +426,56 @@ actiontable = {'p': pick,
      ('o', 'outgoing', False, _('changesets not found in destination')),
      ('f', 'force', False,
       _('force outgoing even for unrelated repositories')),
      ('r', 'rev', [], _('first revision to be edited'))],
      _("[PARENT]"))
-def histedit(ui, repo, *parent, **opts):
+def histedit(ui, repo, *freeargs, **opts):
     """interactively edit changeset history
     """
     # TODO only abort if we try and histedit mq patches, not just
     # blanket if mq patches are applied somewhere
     mq = getattr(repo, 'mq', None)
     if mq and mq.applied:
         raise util.Abort(_('source has mq patches applied'))
 
-    parent = list(parent) + opts.get('rev', [])
+    # basic argument incompatibility processing
+    outg = opts.get('outgoing')
+    cont = opts.get('continue')
+    abort = opts.get('abort')
+    force = opts.get('force')
+    rules = opts.get('commands', '')
+    revs = opts.get('rev', [])
+    goal = 'new' # This invocation goal, in new, continue, abort
+    if force and not outg:
+        raise util.Abort(_('--force only allowed with --outgoing'))
+    if cont:
+        if util.any((outg, abort, revs, freeargs, rules)):
+            raise util.Abort(_('no arguments allowed with --continue'))
+        goal = 'continue'
+    elif abort:
+        if util.any((outg, revs, freeargs, rules)):
+            raise util.Abort(_('no arguments allowed with --abort'))
+        goal = 'abort'
+    else:
+        if os.path.exists(os.path.join(repo.path, 'histedit-state')):
+            raise util.Abort(_('history edit already in progress, try '
+                               '--continue or --abort'))
+        if outg:
+            if revs:
+                raise util.Abort(_('no revisions allowed with --outgoing'))
+            if len(freeargs) > 1:
+                raise util.Abort(
+                    _('only one repo argument allowed with --outgoing'))
+        else:
+            parent = list(freeargs) + opts.get('rev', [])
+            if len(parent) != 1:
+                raise util.Abort(
+                    _('histedit requires exactly one parent revision'))
+
     if opts.get('outgoing'):
-        if len(parent) > 1:
-            raise util.Abort(
-                _('only one repo argument allowed with --outgoing'))
-        elif parent:
-            parent = parent[0]
+        if freeargs:
+            parent = freeargs[0]
 
         dest = ui.expandpath(parent or 'default-push', parent or 'default')
         dest, revs = hg.parseurl(dest, None)[:2]
         ui.status(_('comparing with %s\n') % util.hidepassword(dest))
 
@@ -458,54 +488,43 @@ def histedit(ui, repo, *parent, **opts):
         # hexlify nodes from outgoing, because we're going to parse
         # parent[0] using revsingle below, and if the binary hash
         # contains special revset characters like ":" the revset
         # parser can choke.
         parent = [node.hex(n) for n in discovery.findcommonoutgoing(
-            repo, other, [], force=opts.get('force')).missing[0:1]]
-    else:
-        if opts.get('force'):
-            raise util.Abort(_('--force only allowed with --outgoing'))
+            repo, other, [], force=force).missing[0:1]]
+        if not parent:
+            raise util.Abort(_('no outgoing ancestors'))
 
-    if opts.get('continue', False):
-        if len(parent) != 0:
-            raise util.Abort(_('no arguments allowed with --continue'))
+    if goal == 'continue':
         (parentctxnode, rules, keep, topmost, replacements) = readstate(repo)
         currentparent, wantnull = repo.dirstate.parents()
         parentctx = repo[parentctxnode]
         parentctx, repl = bootstrapcontinue(ui, repo, parentctx, rules, opts)
         replacements.extend(repl)
-    elif opts.get('abort', False):
-        if len(parent) != 0:
-            raise util.Abort(_('no arguments allowed with --abort'))
+    elif goal == 'abort':
         (parentctxnode, rules, keep, topmost, replacements) = readstate(repo)
         mapping, tmpnodes, leafs, _ntm = processreplacement(repo, replacements)
         ui.debug('restore wc to old parent %s\n' % node.short(topmost))
         hg.clean(repo, topmost)
         cleanupnode(ui, repo, 'created', tmpnodes)
         cleanupnode(ui, repo, 'temp', leafs)
         os.unlink(os.path.join(repo.path, 'histedit-state'))
         return
     else:
         cmdutil.bailifchanged(repo)
-        if os.path.exists(os.path.join(repo.path, 'histedit-state')):
-            raise util.Abort(_('history edit already in progress, try '
-                               '--continue or --abort'))
 
         topmost, empty = repo.dirstate.parents()
 
-        if len(parent) != 1:
-            raise util.Abort(_('histedit requires exactly one parent revision'))
         parent = scmutil.revsingle(repo, parent[0]).node()
 
         keep = opts.get('keep', False)
         revs = between(repo, parent, topmost, keep)
         if not revs:
             raise util.Abort(_('%s is not an ancestor of working directory') %
                              node.short(parent))
 
         ctxs = [repo[r] for r in revs]
-        rules = opts.get('commands', '')
         if not rules:
             rules = '\n'.join([makedesc(c) for c in ctxs])
             rules += '\n\n'
             rules += editcomment % (node.short(parent), node.short(topmost))
             rules = ui.edit(rules, ui.username())
diff --git a/tests/test-histedit-non-commute.t b/tests/test-histedit-non-commute.t
--- a/tests/test-histedit-non-commute.t
+++ b/tests/test-histedit-non-commute.t
@@ -238,11 +238,11 @@ edit the history, this time with a fold 
   merging e incomplete! (edit conflicts, then use 'hg resolve --mark')
   Fix up the change and run hg histedit --continue
 
   $ echo 'I can haz no commute' > e
   $ hg resolve --mark e
-  $ hg histedit --commands $EDITED --continue 2>&1 | fixbundle
+  $ hg histedit --continue 2>&1 | fixbundle
   0 files updated, 0 files merged, 0 files removed, 0 files unresolved
   merging e
   warning: conflicts during merge.
   merging e incomplete! (edit conflicts, then use 'hg resolve --mark')
   Fix up the change and run hg histedit --continue


More information about the Mercurial-devel mailing list