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@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org