Re: [libvirt] [PATCH 2/4] virsh: Extend virsh commands definition with completer
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(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 619 bytes)