RE: [PATCH v5 12/10] remote-bzr: support the new 'force' option

[Felipe Contreras <felipe.contreras@gmail.com>]

Richard Hansen wrote:
> Signed-off-by: Richard Hansen <rhansen@bbn.com>
> ---
>  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