On 06/24/2011 12:42 AM, Lai Jiangshan wrote:
> Custom readline generator will help for some usecase.
Indeed, this could be very nice as extended to more commands.
>
> Also add a custom readline generator for the "help" command.
>
> Signed-off-by: Lai Jiangshan <laijs@xxxxxxxxxxxxxx>
> ---
Hmm, this is a new virsh feature, and we've missed the RC1 deadline,
even though your v1 was posted before then. On one hand, it seems like
this would be self-contained enough that I would be okay with including
a v2 in 0.9.3, unless anyone else objects in the next day or so.
However, there are some cleanups to do first, and in the process of
identifying those, I have a suggestion for a complete revamp of the
patch idea which would certainly put us into post-release approval:
> diff --git a/tools/virsh.c b/tools/virsh.c
> index fcd254d..51e43c1 100644
> --- a/tools/virsh.c
> +++ b/tools/virsh.c
> @@ -13575,7 +13575,7 @@ vshCloseLogFile(vshControl *ctl)
> * (i.e. STATE == 0), then we start at the top of the list.
> */
> static char *
> -vshReadlineCommandGenerator(const char *text, int state)
> +vshReadlineCmdAndGrpGenerator(const char *text, int state, int grpname)
You are using grpname as a bool, so s/int/bool/
> static char *
> +vshReadlineCommandGenerator(const char *text, int state)
> +{
> + return vshReadlineCmdAndGrpGenerator(text, state, 0);
> +}
> +
> +static char *
> +vshReadlineHelpOptionGenerator(const char *text, int state)
> +{
> + return vshReadlineCmdAndGrpGenerator(text, state, 1);
and pass false/true rather than 0/1.
> +}
> +
> +struct vshCustomReadLine {
> + const char *name;
> + char *(*CustomReadLineOptionGenerator)(const char *text, int state);
> +};
> +
> +struct vshCustomReadLine customeReadLine[] = {
> + { "help", vshReadlineHelpOptionGenerator },
> + { NULL, NULL }
Rather than maintaining yet another array of mappings, along with the
corresponding cost of name lookups within that array, what if we instead
change vshCmdDef to instead have a new function pointer field, which is
NULL if the default command completion is good enough, but which is the
particular command's custom completer when we want a smarter function.
> +static struct vshCustomReadLine *vshCustomReadLineSearch(const char *name)
> +{
> + struct vshCustomReadLine *ret = customeReadLine;
s/custome/custom/, if we even keep this function. But I don't think we
need it, because...
> @@ -13632,6 +13672,7 @@ vshReadlineOptionsGenerator(const char *text, int state)
> memcpy(cmdname, rl_line_buffer, p - rl_line_buffer);
>
> cmd = vshCmddefSearch(cmdname);
> + rl = vshCustomReadLineSearch(cmdname);
...here, you could just get the custom generator as a member of the
enlarged vshCmdDef struct.
One other idea would be to add new enums to vshCmdOptType. For example,
VSH_OT_CMD would be identical to VSH_OT_STRING, except that during
tab-completion, it completes on command and group names (for that
matter, while VSH_OT_STRING and VSH_OT_DATA used to have separate
purposes, I no longer see any difference between them, and we could
probably consolidate those two types). But I don't know if that makes
tab completion any easier or harder. In terms of your help example, the
two ideas play out roughly like:
1. 'help TAB' -> completion generator parses first word, 'help', and
sees that it is a command. The help command has a custom generator, so
invoke that. The custom generator for help then knows that the only
possibility for a next word is a command or group name, and returns the
full list of such names.
2. 'help TAB' -> completion generator parses first word, and learns that
'help' command has a VSH_OT_CMD second argument. It then calls the CMD
generator, which knows how to generate the full list of such names.
But when you get to other commands, you can start to see the benefit of
having strongly-typed VSH_OT arguments:
1. 'vol-key --pool TAB' -> completion generator parses first word and
sees that it is the 'vol-key' command. It then calls the vol-key
command generator, which must parse the second word '--pool' to see that
completion wants a name of a pool, rather than a volume id. But there
are quite a few vol-* commands, all of which would have to repeat this
--pool parsing.
2. 'vol-key --pool TAB' -> completion generator parses '--pool' and sees
that it requires an argument, and that it is of type VSH_OT_POOL. It is
then able to provide a list of all pool names, merged also with the list
of all other possible options (in this case, --vol).
That is, if you have command-based custom generators, then each command
has to repeat parsing functionality, then call back to common list
generators; whereas if you have option-based custom generators, then you
have fewer callbacks because all the smarts are tied to well-typed
options, and after all, it is the type of each option that determines
which list to generate, more than the type of the command that includes
the option.
--
Eric Blake eblake@xxxxxxxxxx +1-801-349-2682
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list