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

Christian Ebert blacktrash at gmx.net
Wed Jul 8 12:17:33 CDT 2009


* Matt Mackall on Wednesday, July 08, 2009 at 10:57:01 -0500
> On Wed, 2009-07-08 at 00:43 +0200, Christian Ebert wrote:
>> * Martin Geisler on Tuesday, July 07, 2009 at 22:50:37 +0200
>>> Matt Mackall <mpm at selenic.com> writes:
>>>> How do we work around this? Good question. In the case of kwrepo,
>>>> where we're defining a new class already, the easiest thing to do is
>>>> simply to superclass:
>>>> 
>>>> class baz(foo):
>>>> def bar(self):
>>>>   # add our baz-specific behavior
>>>>   ...
>>>>   # fall through to base behavior where appropriate
>>>>   super(baz, self).bar()
>>>> 
>>>> Then no reassignment is necessary and we never need to deal with bound
>>>> methods.
>>> 
>>> Right, that makes good sense. And so I would say that the part of the
>>> patch that adds wrappers like this is unnecessary (since we don't add
>>> any kwrepo-specific behavior to sopener):
>>> 
>>>   class kwrepo(repo.__class__):
>>>       def sopener(self, *args):
>>>           return super(kwrepo, self).sopener(*args)
>> 
>> In the code should it be:
>> 
>>    class kwrepo(repo.__class__):
>>        def file(self, f):
>>            if f[0] == '/':
>>                f = f[1:]
>>            return kwfilelog(self.sopener, kwt, f)
>> 
>> or:
>> 
>>    class kwrepo(repo.__class__):
>>        def file(self, f):
>>            if f[0] == '/':
>>                f = f[1:]
>>            return kwfilelog(repo.sopener, kwt, f)
>>                             ^^^^
>> or doesn't it matter at all?
> 
> Never use repo. That's the second issue. It's effectively the same as:
> 
> a = {}
> a['a'] = a # put a reference to a inside a
> print a
> {'a': {...}}

Well, then to me it looks like my patch makes sense. The only way
to shorten it a little bit that I can see if the current

            return kwfilelog(self.sopener, kwt, f)

is allowed.

Sorry for being dense (I really do make an effort, hehe) and
thank you for taking the time to explain.

c
-- 
\black\trash movie    _C O W B O Y_  _C A N O E_  _C O M A_
Ein deutscher Western/A German Western
-->> http://www.blacktrash.org/underdogma/ccc.html
-->> http://www.blacktrash.org/underdogma/ccc-en.html


More information about the Mercurial-devel mailing list