[PATCH]: Optional explanation for options

[John A Meinel]

Robert Collins wrote:
> On Wed, 2005-09-07 at 09:04 +0100, Magnus Therning wrote:
>
>>The following patch makes it possible to add explanations to options for
>>commands. E.g.
>>
>>class cmd_my_command(Command):
>>    ...
>>    takes_options = [('revision', 'Retrieve particular revision')]
>>    ...
>>

> This is somewhat ugly: an api where type inspection is needed is prone
> to mistakes, and likely to have headaches. I think a better approach
> would be to done something along the lines of this - which is wrong, but
> demonstrates the concept:

What about something like this instead:

class cmd_my_command(Command):
    takes_options = ['verbose'
        , Option('custom', parser=XYZ, help='foo')]

And then the parser would try something like:
opts = {}
for opt in cmd.takes_options:
    try:
        opt = OPTIONS[opt]
    except KeyError:
	pass
    opts[opt.name] = opt

That gets around something like "isinstance" since you don't seem to
like it.

I personally would prefer:

for opt in cmd.takes_options:
    if isinstance(opt, Option):
      opts[opt.name] = opt
    else:
      opts[opt] = OPTION[opt]

Or you could use "isinstance(opt, basestring)" to invert which was the
default action.

I think your method is rather verbose, and pretty ugly. Versus just
declaring the option name so that it grabs the default version of that
option.

Or maybe we can do something like:

takes_options = [Option('revision'), Option('overloaded', parser=XYZ,
help='foo')]

And then the Option function/class can handle pulling any items that
aren't overloaded from the default OPTIONS object.

In fact, you can write a generic one as:

def Option(name, **kwargs):
   if OPTIONS.has_key(name):
       opt_stuff = OPTIONS[name]
   else:
       opt_stuff = {}
   opt_stuff.update(kwargs)
   return opt_stuff

John
=:->


>
> Make the entries in OPTIONS a list of tuples with default explanations.
> Then we get
> class cmd_my_command(Command):
>     ...
>     takes_options = [OPTIONS[option] for option in ['revision', ...]]
>     ...
>
> For commands that want to override:
> class cmd_my_command(Command):
>     ...
>     takes_options = [(OPTIONS['revision'][0], "foo bar does abc")]
>     ...
>
> The things that are wrong is that this is overly verbose and unclear.
>
>
> Rob

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 253 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20050910/c6bc4b11/attachment.pgp