Annoying API change in dirstate.py

Greg Ward greg at gerg.ca
Mon Feb 15 09:03:14 CST 2010


On Mon, Feb 15, 2010 at 9:24 AM, Dirkjan Ochtman <dirkjan at ochtman.nl> wrote:
> You should dig up the emails leading to that changeset, there might be
> reasoning from Matt. Also, "a little extra work" better happens
> outside the hot loop, otherwise it kind of sucks.

I've currently implemented it like this:

--- a/mercurial/dirstate.py
+++ b/mercurial/dirstate.py
@@ -430,7 +430,7 @@
                 return True
         return False

-    def walk(self, match, subrepos, unknown, ignored):
+    def walk(self, match, unknown, ignored, subrepos=None):
         '''
         Walk recursively through the directory tree, finding all files
         matched by match.
@@ -496,8 +496,9 @@
         files = set(match.files())
         if not files or '.' in files:
             files = ['']
-        results = dict.fromkeys(subrepos)
-        results['.hg'] = None
+        results = {'.hg': None}
+        if subrepos is not None:
+            results.update(dict.fromkeys(subrepos))

         # step 1: find all explicit files
         for ff in sorted(files):
@@ -575,12 +576,13 @@
                 if not st is None and not getkind(st.st_mode) in
(regkind, lnkkind):
                     st = None
                 results[nf] = st
-        for s in subrepos:
-            del results[s]
+        if subrepos is not None:
+            for s in subrepos:
+                del results[s]
         del results['.hg']
         return results

Both checks are outside the inner loop, i.e. they only happen once per
call to walk().  But it is kind of annoying to have to do the same
thing twice.  I might take Sune's advice and just do

  if subrepos is None:
      subrepos = []

at the top of the method.  I know that's the cleaner way to do it, but
that idiom has always irked me because of the extra malloc().  ;-)
Guess that just gives me a way as an old-fashioned C programmer.

Greg


More information about the Mercurial-devel mailing list