[PATCH 2 of 2] keyword: eliminate potential reference cycles from kwrepo

[Christian Ebert]

Hi Martin,

* Martin Geisler on Tuesday, July 07, 2009 at 16:15:06 +0200
> Christian Ebert <blacktrash at gmx.net> writes:
> 
>> # HG changeset patch
>> # User Christian Ebert <blacktrash at gmx.net>
>> # Date 1246790211 -7200
>> # Node ID c26ebe8f48b8c4c2b504d9c467d313f30be76fba
>> # Parent  fbe2c25a36131fac923b25334a4e576991e64557
>> keyword: eliminate potential reference cycles from kwrepo
>> 
>> Reference member functions of repo.__class__ consistently in
>> super(kwrepo, self) at class level.
>> Therefore we also need dumb wrappers around sopener and commitctx.
>> 
>> diff --git a/hgext/keyword.py b/hgext/keyword.py
>> --- a/hgext/keyword.py
>> +++ b/hgext/keyword.py
>> @@ -450,6 +450,12 @@
>>     kwtools['templater'] = kwt = kwtemplater(ui, repo)
>> 
>>     class kwrepo(repo.__class__):
>> +        def sopener(self, *args):
>> +            return super(kwrepo, self).sopener(*args)
>> +
>> +        def commitctx(self, *args):
>> +            return super(kwrepo, self).commitctx(*args)
> 
> I don't get it -- are those methods not just no-ops?

To say I get it would be exaggerated ;-) See below:

>>         def file(self, f):
>>             if f[0] == '/':
>>                 f = f[1:]

Next line is:
            return kwfilelog(self.sopener, kwt, f)

It would also _work_ (pass the tests) with repo.sopener

ATM there is a mixture of self.repomemberfunc and
repo.repomemberfunc, and when I tried to clean this up Matt
chimed in like so:

http://marc.info/?l=mercurial-devel&m=124654567201924&w=2

snip

>>         def kwcommitctx(self, ctx, error=False):
>>             wlock = lock = None
>>             try:
>> -                wlock = self.wlock()
>> -                lock = self.lock()
>> +                wlock = super(kwrepo, self).wlock()
>> +                lock = super(kwrepo, self).lock()

Third working alternative:
                 wlock = repo.wlock()
                 lock = repo.wlock()

Basically I thought it would be good to use 1 of the 3
alternatives throughout.

> Why is this necessary? The kwrepo class does not override the wlock and
> lock methods, so I don't see why you would explicitly call the
> superclass' methods?

I'm not sure if this is necessary either. My layman's
understanding is: it doesn't break, it works, fine ;-)
However, this attitude is difficult to reconcile with my desire
for consistency. And after Matt's reply I /believe/ calling the
superclass method explicitly is safer.

c
-- 
_B A U S T E L L E N_ lesen! --->> <http://www.blacktrash.org/baustellen/>