[PATCH 1 of 2 STABLE V2] context: add 'changectx.dirs()' to recognize directory matching pattern

FUJIWARA Katsunori foozy at lares.dti.ne.jp
Tue Feb 21 07:40:15 UTC 2012


At Mon, 20 Feb 2012 14:12:20 -0600,
Matt Mackall wrote:
> 
> On Sun, 2012-02-19 at 19:25 +0900, FUJIWARA Katsunori wrote:
> > # HG changeset patch
> > # User FUJIWARA Katsunori <foozy at lares.dti.ne.jp>
> > # Date 1329646846 -32400
> > # Branch stable
> > # Node ID 59430cdb845b58c916547dd5623ee438abcb2814
> > # Parent  0e0060bf2f440d5cc33e5f36d99868a5380debd4
> > context: add 'changectx.dirs()' to recognize directory matching pattern
> 
> I don't want util.adddirs (and if I did want it, I would want it in its
> own patch!). It's way too special-purpose for util and indeed only has
> one user.
> 
> > +    @propertycache
> > +    def _dirs(self):
> > +        dirs = set()
> > +        for f in self._manifest:
> > +            util.adddirs(f, dirs)
> > +        return dirs
> 
> You've added a scope lookup and a function call (to a single use
> function) to the inner loop. Both unfortunate things for a language with
> effectively no optimization. Much better to just inline adddirs here.
> 
> I still think we should have a workingctx version of this method that
> reuses the dirstate directory data rather than building another copy.

Thank you for your comments !

I want to confirm about 'workingctx' version of this.

For this bug fix, 'match.bad()' is invoked only in 'working' case, in
which 'ctx1' should be 'changectx', not 'workingctx'.

In my plan to use 'chagectx.dirs()' for largefiles bug fix, I'll use
'dirstate' directly for working context, because 'changectx.dirs()' is
not enough for some reasons.

So, I plan to define 'dirs()' only in 'changectx' now.

But should I define it also in 'workingctx' to reduce building cost
for latent purposes ? If so, I'll define it like:

    def dirs(self):
        return self._repo.dirstate.dirs()

# this requires 'dirstate.dirs()' as public interface of 'dirstate._dirs'

> >          if not parentworking:
> >              def bad(f, msg):
> > -                if f not in ctx1:
> > +                # 'f' may be a directory pattern from 'match.files()',
> > +                # so 'f not in ctx1' is not enough
> > +                if (f not in ctx1) and (f not in ctx1.dirs()):
> >                      self.ui.warn('%s: %s\n' % (self.dirstate.pathto(f), msg))
> >              match.bad = bad
> 
> Separate patch, please. Unnecessary parens.

----------------------------------------------------------------------
[FUJIWARA Katsunori]                             foozy at lares.dti.ne.jp



More information about the Mercurial-devel mailing list