Patchwork [4,of,7] revert: group action into a single dictionary

login
register
mail settings
Submitter Pierre-Yves David
Date May 19, 2014, 3:58 p.m.
Message ID <eaff5c46777081a26791.1400515083@marginatus.alto.octopoid.net>
Download mbox | patch
Permalink /patch/4799/
State Accepted
Commit 33395a7e55276ce7caaeb214455e5c9130e4ff02
Headers show

Comments

Pierre-Yves David - May 19, 2014, 3:58 p.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@fb.com>
# Date 1400024551 25200
#      Tue May 13 16:42:31 2014 -0700
# Node ID eaff5c46777081a267917ab8631e74f7f51780d3
# Parent  2343dafe2e301dce55499c4c64a909f415f3bc17
revert: group action into a single dictionary

We had 4 different variables to old the list of the 4 possibles actions. I'm
grouping them in a single dictionary for a few reasons. First it make is clearer
they are all related and meant to be the final actions performed by revert.
Second this simplify the parameter of the _performrevert function.  Finally the
two elements in each entry (list and message) have a different consumer in a
different function, this change will make it easier to split them in a later
commit.

Patch

diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -2306,26 +2306,26 @@  def revert(ui, repo, ctx, parents, *pats
                 return _('forgetting %s\n')
             return _('removing %s\n')
 
         # action to be actually performed by revert
         # (<list of file>, message>) tuple
-        revert = ([], _('reverting %s\n'))
-        add = ([], _('adding %s\n'))
-        remove = ([], removeforget)
-        undelete = ([], _('undeleting %s\n'))
+        actions = {'revert': ([], _('reverting %s\n')),
+                   'add': ([], _('adding %s\n')),
+                   'remove': ([], removeforget),
+                   'undelete': ([], _('undeleting %s\n'))}
 
         disptable = (
             # dispatch table:
             #   file state
             #   action if in target manifest
             #   action if not in target manifest
             #   make backup if in target manifest
             #   make backup if not in target manifest
-            (modified, revert,   remove, True,  True),
-            (added,    revert,   remove, True,  False),
-            (removed,  undelete, None,   True,  False),
-            (deleted,  revert,   remove, False, False),
+            (modified, actions['revert'],   actions['remove'], True,  True),
+            (added,    actions['revert'],   actions['remove'], True,  False),
+            (removed,  actions['undelete'], None,              True,  False),
+            (deleted,  actions['revert'],   actions['remove'], False, False),
             )
 
         for abs, (rel, exact) in sorted(names.items()):
             # hash on file in target manifest (or None if missing from target)
             mfentry = mf.get(abs)
@@ -2362,11 +2362,11 @@  def revert(ui, repo, ctx, parents, *pats
                 # Not touched in current dirstate.
 
                 # file is unknown in parent, restore older version or ignore.
                 if abs not in repo.dirstate:
                     if mfentry:
-                        handle(add, True)
+                        handle(actions['add'], True)
                     elif exact:
                         ui.warn(_('file not managed: %s\n') % rel)
                     continue
 
                 # parent is target, no changes mean no changes
@@ -2382,25 +2382,26 @@  def revert(ui, repo, ctx, parents, *pats
                 if abs in pmf and mfentry:
                     # if version of file is same in parent and target
                     # manifests, do nothing
                     if (pmf[abs] != mfentry or
                         pmf.flags(abs) != mf.flags(abs)):
-                        handle(revert, False)
+                        handle(actions['revert'], False)
                 else:
-                    handle(remove, False)
+                    handle(actions['remove'], False)
+
         if not opts.get('dry_run'):
-            _performrevert(repo, parents, ctx, revert, add, remove, undelete)
+            _performrevert(repo, parents, ctx, actions)
 
             if targetsubs:
                 # Revert the subrepos on the revert list
                 for sub in targetsubs:
                     ctx.sub(sub).revert(ui, ctx.substate[sub], *pats, **opts)
     finally:
         wlock.release()
 
-def _performrevert(repo, parents, ctx, revert, add, remove, undelete):
-    """function that actually perform all the action computed for revert
+def _performrevert(repo, parents, ctx, actions):
+    """function that actually perform all the actions computed for revert
 
     This is an independent function to let extension to plug in and react to
     the imminent revert.
 
     Make sure you have the working directory locked when calling this function.
@@ -2410,11 +2411,11 @@  def _performrevert(repo, parents, ctx, r
     def checkout(f):
         fc = ctx[f]
         repo.wwrite(f, fc.data(), fc.flags())
 
     audit_path = pathutil.pathauditor(repo.root)
-    for f in remove[0]:
+    for f in actions['remove'][0]:
         if repo.dirstate[f] == 'a':
             repo.dirstate.drop(f)
             continue
         audit_path(f)
         try:
@@ -2430,29 +2431,29 @@  def _performrevert(repo, parents, ctx, r
         # merges to avoid losing information about merged/dirty files.
         if p2 != nullid:
             normal = repo.dirstate.normallookup
         else:
             normal = repo.dirstate.normal
-    for f in revert[0]:
+    for f in actions['revert'][0]:
         checkout(f)
         if normal:
             normal(f)
 
-    for f in add[0]:
+    for f in actions['add'][0]:
         checkout(f)
         repo.dirstate.add(f)
 
     normal = repo.dirstate.normallookup
     if node == parent and p2 == nullid:
         normal = repo.dirstate.normal
-    for f in undelete[0]:
+    for f in actions['undelete'][0]:
         checkout(f)
         normal(f)
 
     copied = copies.pathcopies(repo[parent], ctx)
 
-    for f in add[0] + undelete[0] + revert[0]:
+    for f in actions['add'][0] + actions['undelete'][0] + actions['revert'][0]:
         if f in copied:
             repo.dirstate.copy(copied[f], f)
 
 def command(table):
     '''returns a function object bound to table which can be used as