[PATCH] registrar: raise a programming error on duplicated registering

Pierre-Yves David pierre-yves.david at ens-lyon.org
Mon Dec 19 09:49:55 EST 2016



On 12/19/2016 03:09 PM, Yuya Nishihara wrote:
> On Mon, 19 Dec 2016 08:51:49 +0100, Pierre-Yves David wrote:
>> On 12/18/2016 08:24 AM, Yuya Nishihara wrote:
>>> On Fri, 16 Dec 2016 20:31:29 +0100, Pierre-Yves David wrote:
>>>> # HG changeset patch
>>>> # User Pierre-Yves David <pierre-yves.david at ens-lyon.org>
>>>> # Date 1481545965 -3600
>>>> #      Mon Dec 12 13:32:45 2016 +0100
>>>> # Node ID f3479f74af0d2b21c89e906c503f4cddf4b3c39b
>>>> # Parent  182cacaa4c32330c0466b7111a75d060830783e8
>>>> registrar: raise a programming error on duplicated registering
>>>>
>>>> Previous, registering different object with the same name would silently
>>>> overwrite the first value with the second one. We now detect the situation and
>>>> raise an error. No extension in test or core had the issues.
>>>>
>>>> diff --git a/mercurial/registrar.py b/mercurial/registrar.py
>>>> --- a/mercurial/registrar.py
>>>> +++ b/mercurial/registrar.py
>>>> @@ -8,6 +8,7 @@
>>>>  from __future__ import absolute_import
>>>>
>>>>  from . import (
>>>> +    error,
>>>>      pycompat,
>>>>      util,
>>>>  )
>>>> @@ -55,6 +56,9 @@ class _funcregistrarbase(object):
>>>>              func._origdoc = doc
>>>>              func.__doc__ = self._formatdoc(decl, doc)
>>>>
>>>> +        if name in self._table:
>>>> +            raise error.ProgrammingError('duplicate registration for name %s',
>>>> +                                         name)
>>>
>>> It should be '% name'.
>>
>> I'm not sure? We usually have the offending entry at the end because it
>> has variable length. What do you think?
>> (I made a minor change in the new version,
>> 'duplicate registration for name: "%s"')
>
> I meant s/,/%/.

Ha right, I've apparently fixed this without paying attention in my 
rework :-)

>>> I slightly prefer raising the error before modifying the func object, but that
>>> would make little difference since ProgrammingError isn't a recoverable error.
>>
>> Ha!, very good catch. I pushed an updated version here, if you want to
>> look at/take it.
>>
>> https://www.mercurial-scm.org/repo/users/marmoute/mercurial/rev/b52e8a4f4c0f
>
> I'll take this, thanks.

Thanks!

-- 
Pierre-Yves David


More information about the Mercurial-devel mailing list