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

[Mads Kiilerich]

On 07/22/2009 12:31 PM, Gilles Moris wrote:
> 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.
>    

Yes, the array must have the right size because it always gets indexed, 
but the values are not used. You could start with an empty list and 
.append to it. IMHO that would be slightly simpler (IMHO) and slightly 
slower - but no so much that I can measure it.

http://stackoverflow.com/questions/537086/reserve-memory-for-list-in-python

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

I have second thoughts: The parent order doesn't depend on the revision 
order, so the tag order will always be the same and welldefined, and 
there is thus no reason to sort them.

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

AFAIK: Changesets without any parents will have the null rev (-1) as 
parent, so there will always be 1 or two parents.

So it could be similar to this - and even simpler if there is a simple 
way to loop over all revisions:

     @util.propertycache
     def lasttagmap(self):
         ltmap = [(0, 0, 'null')] # rev r is at index r+1
         for ctx in (self.repo[r] for r in range(len(self.repo))):
             tags = [t for t in ctx.tags() if self.repo.tagtype(t) == 
'global']
             if tags:
                 ltmap.append((ctx.date(), 0, '='.join(tags)))
             else:
                 ldate, ldist, ltag = max(ltmap[p.rev() + 1] for p in 
ctx.parents())
                 ltmap.append((ldate, ldist + 1, ltag))
         return ltmap

But now we are discussing so minor details that bikeshed coloring is 
more important. Someone, please apply the patch and stop us ;-)

/Mads