Re: [PATCH v5 12/10] remote-bzr: support the new 'force' option
On Mon, Nov 11, 2013 at 12:12 PM, Richard Hansen wrote: > On 2013-11-11 06:51, Felipe Contreras wrote: >> def do_option(parser): >> global force >> _, key, value = parser.line.split(' ') > > I'm surprised you prefer this over 'key, val = parser[1:3]' or even > '_, key, val = parser[:]'. Are you intending to eventually remove > Parser.__getitem__()? I don't, actually. I'm fine with either way. Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 12/10] remote-bzr: support the new 'force' option
On 2013-11-11 06:51, Felipe Contreras wrote: > Richard Hansen wrote: >> Signed-off-by: Richard Hansen >> --- >> contrib/remote-helpers/git-remote-bzr | 34 >> +- >> contrib/remote-helpers/test-bzr.sh| 22 +- >> 2 files changed, 54 insertions(+), 2 deletions(-) >> >> diff --git a/contrib/remote-helpers/git-remote-bzr >> b/contrib/remote-helpers/git-remote-bzr >> index 7e34532..ba693d1 100755 >> --- a/contrib/remote-helpers/git-remote-bzr >> +++ b/contrib/remote-helpers/git-remote-bzr >> @@ -42,6 +42,7 @@ import json >> import re >> import StringIO >> import atexit, shutil, hashlib, urlparse, subprocess >> +import types >> >> NAME_RE = re.compile('^([^<>]+)') >> AUTHOR_RE = re.compile('^([^<>]+?)? ?[<>]([^<>]*)(?:$|>)') >> @@ -684,7 +685,8 @@ def do_export(parser): >> peer = bzrlib.branch.Branch.open(peers[name], >> >> possible_transports=transports) >> try: >> -peer.bzrdir.push_branch(branch, revision_id=revid) >> +peer.bzrdir.push_branch(branch, revision_id=revid, >> +overwrite=force) >> except bzrlib.errors.DivergedBranches: >> print "error %s non-fast forward" % ref >> continue >> @@ -718,8 +720,34 @@ def do_capabilities(parser): >> print "*import-marks %s" % path >> print "*export-marks %s" % path >> >> +print "option" >> print >> >> +class InvalidOptionValue(Exception): >> +pass >> + >> +def do_option(parser): >> +(opt, val) = parser[1:3] >> +handler = globals().get('do_option_' + opt) >> +if handler and type(handler) == types.FunctionType: >> +try: >> +handler(val) >> +except InvalidOptionValue: >> +print "error '%s' is not a valid value for option '%s'" % (val, >> opt) >> +else: >> +print "unsupported" >> + >> +def do_bool_option(val): >> +if val == 'true': ret = True >> +elif val == 'false': ret = False >> +else: raise InvalidOptionValue() >> +print "ok" >> +return ret >> + >> +def do_option_force(val): >> +global force >> +force = do_bool_option(val) >> + > > While this organization has merit, I think it's overkill for a single option, > or just a couple of them. If in the future we add more, we might revisit this, > for the moment something like this would suffice: OK, I'll reroll. > > class InvalidOptionValue(Exception): > pass > > def get_bool_option(val): > if val == 'true': > return True > elif val == 'false': > return False > else: > raise InvalidOptionValue() > > def do_option(parser): > global force > _, key, value = parser.line.split(' ') I'm surprised you prefer this over 'key, val = parser[1:3]' or even '_, key, val = parser[:]'. Are you intending to eventually remove Parser.__getitem__()? Thanks, Richard > try: > if key == 'force': > force = get_bool_option(value) > print 'ok' > else: > print 'unsupported' > except InvalidOptionValue: > print "error '%s' is not a valid value for option '%s'" % (value, > key) > > Cheers. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v5 12/10] remote-bzr: support the new 'force' option
Richard Hansen wrote: > Signed-off-by: Richard Hansen > --- > contrib/remote-helpers/git-remote-bzr | 34 +- > contrib/remote-helpers/test-bzr.sh| 22 +- > 2 files changed, 54 insertions(+), 2 deletions(-) > > diff --git a/contrib/remote-helpers/git-remote-bzr > b/contrib/remote-helpers/git-remote-bzr > index 7e34532..ba693d1 100755 > --- a/contrib/remote-helpers/git-remote-bzr > +++ b/contrib/remote-helpers/git-remote-bzr > @@ -42,6 +42,7 @@ import json > import re > import StringIO > import atexit, shutil, hashlib, urlparse, subprocess > +import types > > NAME_RE = re.compile('^([^<>]+)') > AUTHOR_RE = re.compile('^([^<>]+?)? ?[<>]([^<>]*)(?:$|>)') > @@ -684,7 +685,8 @@ def do_export(parser): > peer = bzrlib.branch.Branch.open(peers[name], > > possible_transports=transports) > try: > -peer.bzrdir.push_branch(branch, revision_id=revid) > +peer.bzrdir.push_branch(branch, revision_id=revid, > +overwrite=force) > except bzrlib.errors.DivergedBranches: > print "error %s non-fast forward" % ref > continue > @@ -718,8 +720,34 @@ def do_capabilities(parser): > print "*import-marks %s" % path > print "*export-marks %s" % path > > +print "option" > print > > +class InvalidOptionValue(Exception): > +pass > + > +def do_option(parser): > +(opt, val) = parser[1:3] > +handler = globals().get('do_option_' + opt) > +if handler and type(handler) == types.FunctionType: > +try: > +handler(val) > +except InvalidOptionValue: > +print "error '%s' is not a valid value for option '%s'" % (val, > opt) > +else: > +print "unsupported" > + > +def do_bool_option(val): > +if val == 'true': ret = True > +elif val == 'false': ret = False > +else: raise InvalidOptionValue() > +print "ok" > +return ret > + > +def do_option_force(val): > +global force > +force = do_bool_option(val) > + While this organization has merit, I think it's overkill for a single option, or just a couple of them. If in the future we add more, we might revisit this, for the moment something like this would suffice: class InvalidOptionValue(Exception): pass def get_bool_option(val): if val == 'true': return True elif val == 'false': return False else: raise InvalidOptionValue() def do_option(parser): global force _, key, value = parser.line.split(' ') try: if key == 'force': force = get_bool_option(value) print 'ok' else: print 'unsupported' except InvalidOptionValue: print "error '%s' is not a valid value for option '%s'" % (value, key) Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html