| Submitter | Yuya Nishihara |
|---|---|
| Date | July 11, 2016, 2:28 p.m. |
| Message ID | <292f0845baa57737bc9b.1468247299@mimosa> |
| Download | mbox | patch |
| Permalink | /patch/15795/ |
| State | Accepted |
| Headers | show |
Comments
These 2 patches look good to me. Once they are accepted I will introduce MercurialException and replace Exception -> MercurialException here. My goal is not related to dispatch.runcommand(), but more to unshelve’s shelvedstate.load exception-catching, but I think it’s generally a good thing to do. On 7/11/16, 3:28 PM, "Mercurial-devel on behalf of Yuya Nishihara" <mercurial-devel-bounces@mercurial-scm.org on behalf of yuya@tcha.org> wrote: ># HG changeset patch ># User Yuya Nishihara <yuya@tcha.org> ># Date 1468240802 -32400 ># Mon Jul 11 21:40:02 2016 +0900 ># Node ID 292f0845baa57737bc9b8ce9d21d124dba157cc2 ># Parent 5a15266e4b16eb9dba59fa561a2c3b6d3214db4b >error: make hintable exceptions reject unknown keyword arguments (API) > >Previously they would accept any typos of the hint keyword. > >diff --git a/mercurial/error.py b/mercurial/error.py >--- a/mercurial/error.py >+++ b/mercurial/error.py >@@ -18,12 +18,12 @@ from __future__ import absolute_import > class Hint(object): > """Mix-in to provide a hint of an error > >- This should come first in the inheritance list to consume **kw and pass >- only *args to the exception class. >+ This should come first in the inheritance list to consume a hint and >+ pass remaining arguments to the exception class. > """ > def __init__(self, *args, **kw): >- super(Hint, self).__init__(*args) >- self.hint = kw.get('hint') >+ self.hint = kw.pop('hint', None) >+ super(Hint, self).__init__(*args, **kw) > > class RevlogError(Hint, Exception): > pass >diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py >--- a/mercurial/subrepo.py >+++ b/mercurial/subrepo.py >@@ -56,9 +56,9 @@ def _getstorehashcachename(remotepath): > class SubrepoAbort(error.Abort): > """Exception class used to avoid handling a subrepo error more than once""" > def __init__(self, *args, **kw): >+ self.subrepo = kw.pop('subrepo', None) >+ self.cause = kw.pop('cause', None) > error.Abort.__init__(self, *args, **kw) >- self.subrepo = kw.get('subrepo') >- self.cause = kw.get('cause') > > def annotatesubrepoerror(func): > def decoratedmethod(self, *args, **kargs): >_______________________________________________ >Mercurial-devel mailing list >Mercurial-devel@mercurial-scm.org >https://urldefense.proofpoint.com/v2/url?u=https-3A__www.mercurial-2Dscm.org_mailman_listinfo_mercurial-2Ddevel&d=CwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=Pp-gQYFgs4tKlSFPF5kfCw&m=u1Nss-4AcSzhEtnaLykilc2S5_VePcD0L3v978GuwAY&s=WL4gb7tLHGfce7tTDtoJJ-ERGlP8-5tBIJS5xa4eSWE&e=
On Tue, 12 Jul 2016 09:39:25 +0000, Kostia Balytskyi wrote: > These 2 patches look good to me. Once they are accepted I will introduce > MercurialException and replace Exception -> MercurialException here. My goal > is not related to dispatch.runcommand(), but more to unshelve’s > shelvedstate.load exception-catching, but I think it’s generally a good > thing to do. I don't get the point why you prefer catching everything except MercurialException over catching exceptions that can be raised by int() and nodemod.bin(). IMHO, it's bad practice to suppress unknown exceptions.
On 7/12/16, 2:11 PM, "Yuya Nishihara" <youjah@gmail.com on behalf of yuya@tcha.org> wrote: >On Tue, 12 Jul 2016 09:39:25 +0000, Kostia Balytskyi wrote: >> These 2 patches look good to me. Once they are accepted I will introduce >> MercurialException and replace Exception -> MercurialException here. My goal >> is not related to dispatch.runcommand(), but more to unshelve’s >> shelvedstate.load exception-catching, but I think it’s generally a good >> thing to do. > >I don't get the point why you prefer catching everything except >MercurialException over catching exceptions that can be raised by >int() and nodemod.bin(). IMHO, it's bad practice to suppress unknown >exceptions. I do want to catch exceptions that can be raised by int() and nodemod.bin(), but I want to also be able to catch things like “unknown node” or “filtered node”. I also think that this might be useful beyond unshelve usecase.
On Tue, 12 Jul 2016 14:02:27 +0000, Kostia Balytskyi wrote: > On 7/12/16, 2:11 PM, "Yuya Nishihara" <youjah@gmail.com on behalf of yuya@tcha.org> wrote: > > >On Tue, 12 Jul 2016 09:39:25 +0000, Kostia Balytskyi wrote: > >> These 2 patches look good to me. Once they are accepted I will introduce > >> MercurialException and replace Exception -> MercurialException here. My goal > >> is not related to dispatch.runcommand(), but more to unshelve’s > >> shelvedstate.load exception-catching, but I think it’s generally a good > >> thing to do. > > > >I don't get the point why you prefer catching everything except > >MercurialException over catching exceptions that can be raised by > >int() and nodemod.bin(). IMHO, it's bad practice to suppress unknown > >exceptions. > > I do want to catch exceptions that can be raised by int() and nodemod.bin(), but I want > to also be able to catch things like “unknown node” or “filtered node”. These two are RepoLookupError. > I also think that this > might be useful beyond unshelve usecase. Yeah, I agree with that.
On Tue, Jul 12, 2016 at 09:39:25AM +0000, Kostia Balytskyi wrote: > These 2 patches look good to me. Once they are accepted I will introduce MercurialException and replace Exception -> MercurialException here. My goal is not related to dispatch.runcommand(), but more to unshelve’s shelvedstate.load exception-catching, but I think it’s generally a good thing to do. Queued these, thanks > > On 7/11/16, 3:28 PM, "Mercurial-devel on behalf of Yuya Nishihara" <mercurial-devel-bounces@mercurial-scm.org on behalf of yuya@tcha.org> wrote: > > ># HG changeset patch > ># User Yuya Nishihara <yuya@tcha.org> > ># Date 1468240802 -32400 > ># Mon Jul 11 21:40:02 2016 +0900 > ># Node ID 292f0845baa57737bc9b8ce9d21d124dba157cc2 > ># Parent 5a15266e4b16eb9dba59fa561a2c3b6d3214db4b > >error: make hintable exceptions reject unknown keyword arguments (API) > > > >Previously they would accept any typos of the hint keyword. > > > >diff --git a/mercurial/error.py b/mercurial/error.py > >--- a/mercurial/error.py > >+++ b/mercurial/error.py > >@@ -18,12 +18,12 @@ from __future__ import absolute_import > > class Hint(object): > > """Mix-in to provide a hint of an error > > > >- This should come first in the inheritance list to consume **kw and pass > >- only *args to the exception class. > >+ This should come first in the inheritance list to consume a hint and > >+ pass remaining arguments to the exception class. > > """ > > def __init__(self, *args, **kw): > >- super(Hint, self).__init__(*args) > >- self.hint = kw.get('hint') > >+ self.hint = kw.pop('hint', None) > >+ super(Hint, self).__init__(*args, **kw) > > > > class RevlogError(Hint, Exception): > > pass > >diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py > >--- a/mercurial/subrepo.py > >+++ b/mercurial/subrepo.py > >@@ -56,9 +56,9 @@ def _getstorehashcachename(remotepath): > > class SubrepoAbort(error.Abort): > > """Exception class used to avoid handling a subrepo error more than once""" > > def __init__(self, *args, **kw): > >+ self.subrepo = kw.pop('subrepo', None) > >+ self.cause = kw.pop('cause', None) > > error.Abort.__init__(self, *args, **kw) > >- self.subrepo = kw.get('subrepo') > >- self.cause = kw.get('cause') > > > > def annotatesubrepoerror(func): > > def decoratedmethod(self, *args, **kargs): > >_______________________________________________ > >Mercurial-devel mailing list > >Mercurial-devel@mercurial-scm.org > >https://urldefense.proofpoint.com/v2/url?u=https-3A__www.mercurial-2Dscm.org_mailman_listinfo_mercurial-2Ddevel&d=CwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=Pp-gQYFgs4tKlSFPF5kfCw&m=u1Nss-4AcSzhEtnaLykilc2S5_VePcD0L3v978GuwAY&s=WL4gb7tLHGfce7tTDtoJJ-ERGlP8-5tBIJS5xa4eSWE&e= > > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Patch
diff --git a/mercurial/error.py b/mercurial/error.py --- a/mercurial/error.py +++ b/mercurial/error.py @@ -18,12 +18,12 @@ from __future__ import absolute_import class Hint(object): """Mix-in to provide a hint of an error - This should come first in the inheritance list to consume **kw and pass - only *args to the exception class. + This should come first in the inheritance list to consume a hint and + pass remaining arguments to the exception class. """ def __init__(self, *args, **kw): - super(Hint, self).__init__(*args) - self.hint = kw.get('hint') + self.hint = kw.pop('hint', None) + super(Hint, self).__init__(*args, **kw) class RevlogError(Hint, Exception): pass diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py --- a/mercurial/subrepo.py +++ b/mercurial/subrepo.py @@ -56,9 +56,9 @@ def _getstorehashcachename(remotepath): class SubrepoAbort(error.Abort): """Exception class used to avoid handling a subrepo error more than once""" def __init__(self, *args, **kw): + self.subrepo = kw.pop('subrepo', None) + self.cause = kw.pop('cause', None) error.Abort.__init__(self, *args, **kw) - self.subrepo = kw.get('subrepo') - self.cause = kw.get('cause') def annotatesubrepoerror(func): def decoratedmethod(self, *args, **kargs):