| Submitter | Pierre-Yves David |
|---|---|
| Date | Dec. 16, 2016, 7:31 p.m. |
| Message ID | <f3479f74af0d2b21c89e.1481916689@nodosa.octopoid.net> |
| Download | mbox | patch |
| Permalink | /patch/17937/ |
| State | Accepted |
| Headers | show |
Comments
On Fri, Dec 16, 2016 at 08:31:29PM +0100, Pierre-Yves David wrote: > # HG changeset patch > # User Pierre-Yves David <pierre-yves.david@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 Queued, thanks > > 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) > self._table[name] = func > self._extrasetup(name, func, *args, **kwargs) > > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
On Fri, 16 Dec 2016 20:31:29 +0100, Pierre-Yves David wrote: > # HG changeset patch > # User Pierre-Yves David <pierre-yves.david@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 slightly prefer raising the error before modifying the func object, but that would make little difference since ProgrammingError isn't a recoverable error. > self._table[name] = func > self._extrasetup(name, func, *args, **kwargs)
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@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 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 >> self._table[name] = func >> self._extrasetup(name, func, *args, **kwargs)
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@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/,/%/. > > 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.
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@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!
Patch
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) self._table[name] = func self._extrasetup(name, func, *args, **kwargs)