Patchwork [1,of,4] revert: move manifest membership condition outside of the loop

login
register
mail settings
Submitter Pierre-Yves David
Date Aug. 13, 2014, 8:13 p.m.
Message ID <da0fc935abb80656cfb7.1407960831@marginatus.alto.octopoid.net>
Download mbox | patch
Permalink /patch/5370/
State Accepted
Headers show

Comments

Pierre-Yves David - Aug. 13, 2014, 8:13 p.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@fb.com>
# Date 1403620102 -3600
#      Tue Jun 24 15:28:22 2014 +0100
# Node ID da0fc935abb80656cfb7e315589c05bd57a3e9d2
# Parent  88d54549740874519609f79c2c064033726644c2
revert: move manifest membership condition outside of the loop

Currently, revset is using information from dirstate status and alter its
behavior whenever the file exist in the target manifest or not. This tests are
done a big for loop. We move this member ship testing outside of the loop and
simplifies associates data structure.

This is a step toward a cleaner implementation of revert based on status.
Sean Farley - Aug. 13, 2014, 8:19 p.m.
Pierre-Yves David writes:

> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@fb.com>
> # Date 1403620102 -3600
> #      Tue Jun 24 15:28:22 2014 +0100
> # Node ID da0fc935abb80656cfb7e315589c05bd57a3e9d2
> # Parent  88d54549740874519609f79c2c064033726644c2
> revert: move manifest membership condition outside of the loop
>
> Currently, revset is using information from dirstate status and alter its
> behavior whenever the file exist in the target manifest or not. This tests are
> done a big for loop. We move this member ship testing outside of the loop and
> simplifies associates data structure.
>
> This is a step toward a cleaner implementation of revert based on status.
>
> diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
> --- a/mercurial/cmdutil.py
> +++ b/mercurial/cmdutil.py
> @@ -2384,32 +2384,42 @@ def revert(ui, repo, ctx, parents, *pats
>          def removeforget(abs):
>              if repo.dirstate[abs] == 'a':
>                  return _('forgetting %s\n')
>              return _('removing %s\n')
>  
> +        # split between files known in target manifest and the others
> +        smf = set(mf)

Sweet variable name, bro.

Patch

diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -2384,32 +2384,42 @@  def revert(ui, repo, ctx, parents, *pats
         def removeforget(abs):
             if repo.dirstate[abs] == 'a':
                 return _('forgetting %s\n')
             return _('removing %s\n')
 
+        # split between files known in target manifest and the others
+        smf = set(mf)
+
+        missingmodified = modified - smf
+        modified -= missingmodified
+        missingadded = added - smf
+        added -= missingadded
+        missingremoved = removed - smf
+        removed -= missingremoved
+        missingdeleted = deleted - smf
+        deleted -= missingdeleted
+
         # action to be actually performed by revert
         # (<list of file>, message>) tuple
         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, (actions['revert'],   True),
-                       (actions['remove'],   True)),
-            (added,    (actions['revert'],   True),
-                       (actions['remove'],   False)),
-            (removed,  (actions['undelete'], True),
-                       (None,                False)),
-            (deleted,  (actions['revert'], False),
-                       (actions['remove'], False)),
+            #   action
+            #   make backup
+            (modified,         (actions['revert'],   True)),
+            (missingmodified,  (actions['remove'],   True)),
+            (added,            (actions['revert'],   True)),
+            (missingadded,     (actions['remove'],   False)),
+            (removed,          (actions['undelete'], True)),
+            (missingremoved,   (None,                False)),
+            (deleted,          (actions['revert'],   False)),
+            (missingdeleted,   (actions['remove'],   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)
@@ -2431,18 +2441,15 @@  def revert(ui, repo, ctx, parents, *pats
                         msg = msg(abs)
                     ui.status(msg % rel)
             # search the entry in the dispatch table.
             # if the file is in any of this sets, it was touched in the working
             # directory parent and we are sure it needs to be reverted.
-            for table, hit, miss in disptable:
+            for table, (action, backup) in disptable:
                 if abs not in table:
                     continue
-                # file has changed in dirstate
-                if mfentry:
-                    handle(*hit)
-                elif miss[0] is not None:
-                    handle(*miss)
+                if action is not None:
+                    handle(action, backup)
                 break
             else:
                 # Not touched in current dirstate.
 
                 # file is unknown in parent, restore older version or ignore.