[PATCH] mq: use util.propertycache, introduce method invalidate for mq and repo

[Simon Heimberg]

Am Sonntag, den 03.05.2009, 21:21 +0200 schrieb Martin Geisler:
> Simon Heimberg <simohe at besonet.ch> writes:
> 
[snip]
> >  
> > -        if os.path.exists(self.join(self.series_path)):
> > -            self.full_series = self.opener(self.series_path).read().splitlines()
> > -        self.parse_series()
> > +    def invalidate(self):
> > +        for a in 'applied full_series series series_guards':
> 
> This doesn't look right, I suspect you wanted to add .split() at the end?
You are right of course.
> 
> > +            if a in self.__dict__:
> > +                del self.__dict__[a]
> 
> Would it not be nicer to use hasattr and delattr?
hasattr does not work. It would find the method in the class and would
create the attribute. (See answer of mpm too.)
I agree, delattr is nicer than del.
> 
> > +    @util.propertycache
> > +    def applied(self):
> >          if os.path.exists(self.join(self.status_path)):
> >              lines = self.opener(self.status_path).read().splitlines()
> > -            self.applied = [statusentry(l) for l in lines]
> > +            return [statusentry(l) for l in lines]
> > +        return []
> > +
> > +    @util.propertycache
> > +    def full_series(self):
> > +        if os.path.exists(self.join(self.series_path)):
> > +            return self.opener(self.series_path).read().splitlines()
> > +        return []
> > +
> > +    @util.propertycache
> > +    def series(self):
> > +        self.parse_series()
> > +        return self.series
> 
> IMHO, having a method return "itself" looks strange :-) What really
> happens is that the call to parse_series changes self.series (and
> self.series_guards) from a method to a list(!), which you return. The
> util.propertycache decorator then takes the returned list and assigns it
> to self.series (again).
This is how it is done in dirstate for _map and _copymap. I do not know
how to do it different.
> 
> So I can see that it works, but it feels a bit complicated.
> 
> >      def diffopts(self):
> >          if self._diffopts is None:
> > @@ -1622,6 +1640,7 @@
> >          if qrepo:
> >              qrepo.add(added)
> >  
> > +
> 
> Spurious whitespace change.
deleted
> 
> >  def delete(ui, repo, *patches, **opts):
> >      """remove patches from queue
> >  
> > @@ -2462,6 +2481,10 @@
> >  
> >              return partial
> >  
> > +        def invalidate(self):
> > +            tagscache = super(mqrepo, self).invalidate()
> > +            self.mq.invalidate()
> > +
> 
> What is invalidate used for -- is this change related to the use of
> util.propertycache? If not, then please separate the two changes into
> two changesets.
> 
I separate it.

The new version was sent some minutes ago.

Thanks for your feedback, Martin.