On 05/08/2018 04:20 PM, Lin Ma wrote:
> Currently the VSH_OT_ARGV options don't support complete, But some of
> VSH_OT_ARGV options are gonna support complete in upcoming patches.
>
> Once applied the upcoming completion patches for VSH_OT_ARGV options, If
> we don't ignore VSH_OT_ARGV here, The vshReadlineOptionsGenerator will
> be called, Hence complete output will consist of the result by command
> completer + the result by option completer, It's confusing.
> e.g.
> $ virsh domstats --domain <TAB><TAB>
> --backing --interface --list-paused --perf --vcpu
> --balloon leap42.3 --list-persistent --raw win10
> --block --list-active --list-running sles12sp3
> --cpu-total --list-inactive --list-shutoff sles15
> --enforce --list-other --list-transient --state
>
> After this patch and the upcoming completion patches:
> $ virsh domstats --domain <TAB><TAB>
> leap42.3 sles12sp3 sles15 win10
>
> Signed-off-by: Lin Ma <lma@xxxxxxxx>
> ---
> tools/vsh.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/tools/vsh.c b/tools/vsh.c
> index 57f7589b53..e45bb0d825 100644
> --- a/tools/vsh.c
> +++ b/tools/vsh.c
> @@ -2824,7 +2824,9 @@ vshReadlineParse(const char *text, int state)
> if (!cmd) {
> list = vshReadlineCommandGenerator(text);
> } else {
> - if (!opt || (opt->type != VSH_OT_DATA && opt->type != VSH_OT_STRING))
> + if (!opt || (opt->type != VSH_OT_DATA &&
> + opt->type != VSH_OT_STRING &&
> + opt->type != VSH_OT_ARGV))
> list = vshReadlineOptionsGenerator(text, cmd, partial);
>
> if (opt && opt->completer) {
>
I don't think this is right. The VSH_OT_ARGV means that there can be
more than one argument. And these are usually specified at the end like
this:
# virsh domstats dom1 dom2 dom3
This patch allows to specify just one domain. I guess we need a specific
completer for this command.
After going further with the review I had revisit this patch again. Now
it makes perfect sense, because after patch 09/12 you allow multiple
--domain arguments. Therefore I think a small reorder is in place. 09/12
should be placed before this patch. I'll fix that when pushing.
ACK
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
