[PATCH] templates: add {lasttag} and {lasttagdistance} keywords

Gilles Moris gilles.moris at free.fr
Wed Jul 22 05:31:39 CDT 2009


On Wed July 22 2009 01:52:15 Mads Kiilerich wrote:
> > +    @util.propertycache
> > +    def lasttagmap(self):
> > +        '''get an array[rev] of latest global tag tuple, each tuple being
> > +        (date of the latest tag, distance to that tag, the tag(s)).
> > +        '''
> > +        defvalue = 0, 0, 'null'
> > +        ltmap = [defvalue] * len(self.repo)
> >    
> 
> Using defvalue here is a bit misleading. These values are never used. So 
> using None would be slightly more readable, IMHO, and perhaps a bit faster.
> 

I agree that this value is not used after the array initialization, but it is
used in the loop below.
I feel it is clearer to show that it will contain finally a tuple, but that's
a matter of taste may be.
I don't think anyone is faster, as this is just reference to an object.

> > +
> > +        for r in range(len(self.repo)):
> > +            ctx = self.repo[r]
> > +
> > +            tags = [t for t in ctx.tags() if self.repo.tagtype(t) == 'global']
> > +            if tags:
> > +                ltmap[r] = ctx.date(), 0, '='.join(sorted(tags))
> > +            else:
> > +                # the last tag of this rev comes from the ancestor with the
> > +                # latest tag (secondarily the longest distance to the tag),
> > +                # achieved by comparing the tuples.
> > +                ldate, ldist, ltag = max(
> > +                    p.rev()>= 0 and ltmap[p.rev()] or defvalue
> > +                    for p in ctx.parents())

I am here a little concerned if ctx.parents() returns an empty list.
That does not seem possible, but I wonder if we should fix it anyway.
reduce() was able to handle that gracefully with the default value argument.

> > +                ltmap[r] = ldate, ldist + 1, ltag
> > +
> > +        return ltmap
> > +
[...]
> > +    - lasttag: String. Most recent global tag in the ancestors of this
> > +          changeset.
> >    
> 
> "most recent global tag" is probably correct, but it is slightly 
> confusing that a different word than "latest tag" is used. Perhaps it 
> would be an idea to stick to one of them.
> 

This is an explanation of what "lasttag" means.

> > +    - lasttagdistance: Integer. Longest path to the last tag.
> >    
> 
> I assume it should be "latest" not "last".
> 
> But that is related to that the template variable names IMHO could be 
> improved too...
> 

Either way. "last" is shorter, "latest" more explicit.
Should we wait for other propositions ?

Regards.
Gilles.



More information about the Mercurial-devel mailing list