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

[Martin Geisler]

Simon Heimberg <simohe at besonet.ch> writes:

> # HG changeset patch
> # User Simon Heimberg <simohe at besonet.ch>
> # Date 1241209449 -7200
> # Node ID ff1d180722e90fb6908b6cb29c067ea1889793cc
> # Parent  cc8cecb78ddab5a5cd06df68a2e8e334a0100ddd
> mq: use util.propertycache, introduce method invalidate for mq and repo
>
> diff -r cc8cecb78dda -r ff1d180722e9 hgext/mq.py
> --- a/hgext/mq.py	Die Apr 28 09:04:26 2009 +0200
> +++ b/hgext/mq.py	Fre Mai 01 22:24:09 2009 +0200
> @@ -123,8 +123,6 @@
>          self.path = patchdir or os.path.join(path, "patches")
>          self.opener = util.opener(self.path)
>          self.ui = ui
> -        self.applied = []
> -        self.full_series = []
>          self.applied_dirty = 0
>          self.series_dirty = 0
>          self.series_path = "series"
> @@ -134,13 +132,33 @@
>          self.guards_dirty = False
>          self._diffopts = None
>  
> -        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?

> +            if a in self.__dict__:
> +                del self.__dict__[a]

Would it not be nicer to use hasattr and delattr?

> +    @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).

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.

>  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.

-- 
Martin Geisler

VIFF (Virtual Ideal Functionality Framework) brings easy and efficient
SMPC (Secure Multiparty Computation) to Python. See: http://viff.dk/.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 196 bytes
Desc: not available
Url : http://selenic.com/pipermail/mercurial-devel/attachments/20090503/e72bc0b5/attachment.pgp