[PATCH] fancyopts: allow all callable as default parameter value
Jordi GutiƩrrez Hermoso
jordigh at octave.org
Thu Jun 11 10:18:32 CDT 2015
Hi, looks like you figured out how to send patches to hg. Great!
On Thu, 2015-06-11 at 09:04 -0400, shiyao.ma wrote:
> # HG changeset patch
> # User introom <i at introo.me>
> # Date 1434027264 14400
> # Thu Jun 11 08:54:24 2015 -0400
> # Node ID 54fb12cb448edd26a5334940af7b33e4b4414615
> # Parent 142fb8767743ff6b1c02a6ba01c5935f8dcf422d
> fancyopts: allow all callable as default parameter value
>
> The current fancyopts allows function as default parameter value
> but not other callables.
> By supporting other callables, we can have the benefits of e.g.,
> custom __str__ method, which will be printed by 'hg help' as
> the default value.
Okay, but...
>
> diff --git a/mercurial/fancyopts.py b/mercurial/fancyopts.py
> --- a/mercurial/fancyopts.py
> +++ b/mercurial/fancyopts.py
> @@ -104,19 +104,20 @@
> for opt, val in opts:
> name = argmap[opt]
> obj = defmap[name]
> + t = type(obj)
> if callable(obj):
> state[name] = defmap[name](val)
> - elif isinstance(obj, int):
> + elif t is type(1):
I don't see what these changes are doing or what do they have to do
with your goal as stated in the commit message. Under what
circumstance will `isintance(obj, int)` return true but `type(obj) is
int` return false?
Also, why are you using `type(1)` instead of `int`?
The same question applies to all subsequent changes. I don't
understand how your patch changes anything at all. If you are sure
that your patch is indeed correct, can you please resend with a longer
explanation in your commit message? Use `--flag V2` in `hg email` to
resend.
> try:
> state[name] = int(val)
> except ValueError:
> raise util.Abort(_('invalid value %r for option %s, '
> 'expected int') % (val, opt))
> - elif isinstance(obj, basestring):
> + elif t is type(''):
> state[name] = val
> - elif isinstance(obj, list):
> + elif t is type([]):
> state[name].append(val)
> - elif obj is None or isinstance(obj, bool):
> + elif t is type(None) or t is type(False):
> state[name] = True
>
> # return unparsed args
>
More information about the Mercurial-devel
mailing list