[libvirt] [PATCH 2/4] virsh: Extend virsh commands definition with completer

[Eric Blake]

On 07/04/2011 02:41 AM, Michal Privoznik wrote:
> This commit introduces command-based completer. Any command can now have
> a completer, which will generate additional command arguments. Completer
> is called repeatedly until it returns NULL. On the first call, @state is
> non-zero.

Don't you mean that on the first call, state is 0, and on subsequent
calls, it is 1?

> ---
>  tools/virsh.c |  364 ++++++++++++++++++++++++++++++---------------------------
>  1 files changed, 193 insertions(+), 171 deletions(-)
> 
> diff --git a/tools/virsh.c b/tools/virsh.c
> index eeacec3..3dabb10 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -202,6 +202,7 @@ typedef struct {
>      const vshCmdOptDef *opts;   /* definition of command options */
>      const vshCmdInfo *info;     /* details about command */
>      int flags;                  /* bitwise OR of VSH_CMD_FLAG */
> +    char * (*completer) (const char *, int);    /* command completer */
>  } vshCmdDef;

I'm still not convinced that this is the best approach.  Let's look at
patch 3/4, where one of the commands you changed was virsh
attach-device, to use the listAllDomains completer:

>      {"attach-device", cmdAttachDevice, opts_attach_device,
> -     info_attach_device, 0, NULL},
> +     info_attach_device, 0, complt_listAllDomains},

But attach-device has this synopsis:

    attach-device <domain> <file> [--persistent]

That means, if I type:

attach-device d<TAB>

I get a list of all domains starting with d.  But if I type:

attach-device domain f<TAB>

I would _still_ get a list of all domains starting with f, when what I
_really_ want at that point is a list of a files in the current
directory starting with f.

Furthermore, if I type:

attach-device -<TAB>

I do _not_ get any indication that --domain, --file, or --persistent are
also valid things to type at that point in the command, rather, it tries
to give me a list of all domains starting with '-', even though to
actually use such a domain, I would have to type an extra '--':

attach-device -- -odd-domain

Whereas if you go with my suggestion of having typed parameters, then
attach-device does _not_ need a custom completer.  Rather, it marks that
the 'domain' option is a VSH_OT_DOMAIN_ALL, and the 'file' option is a
VSH_OT_FILE.  Then,

attach-device d<TAB>

says that since the argument does not start with '-', it must be the
first data argument - and that data argument is typed VSH_OT_DOMAIN_ALL,
so call complt_listDomainsFlags to get that list. Likewise,

attach-device domain f<TAB>

says that since we have already parsed an argument, the next argument
must belong to the 'file' option, and we can use the normal readline
file completion hook.  Meanwhile,

attach-device -<TAB>

can see that the only valid thing here is an option name, and can thus
cycle through the valid remaining options, to return the list
'--domain', '--file', '--persistent'.  And putting it all together,

attach-device <TAB>

can output the merged list of '--domain', '--file', '--persistent', as
well as all domain names except for those that begin with '-'.

At this point, I'm not seeing any utility in a per-function completer,
when compared to a more generic utility in a per-option completer.

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20110705/8f261792/attachment-0001.sig>