[libvirt] [PATCH 0/3] virsh: Option completers and small improvements/fixes for autocomplete
This series introduces option completers and adds some minor improvements and fixes(not bugs per se, just better/sane behavior) in vshReadlineParse. The first patch introduces the usage of option completers to auto-complete arguments for a particular option. The second and third patches provide small improvements like completing the options of type VSH_OT_ARGV or VSH_OT_DATA, and to complete multiple options as well, if a previous option requires an argument, and that argument has been provided. Nishith Shah (3): virsh: Introduce usage of option completers to auto-complete arguments virsh: Allow data or argument options to be completed as well virsh: Complete multiple options when any one option requires data tools/vsh.c | 75 ++++++++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 49 insertions(+), 26 deletions(-) -- 2.1.4
Call option completers if argument completion is requested using the corresponding option completer, if it is defined. Signed-off-by: Nishith Shah <nishithshah.2211@gmail.com> --- tools/vsh.c | 66 ++++++++++++++++++++++++++++++++++++++++++------------------- 1 file changed, 46 insertions(+), 20 deletions(-) diff --git a/tools/vsh.c b/tools/vsh.c index 6823ab5..79656ed 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -2636,14 +2636,16 @@ vshReadlineParse(const char *text, int state) vshCommandToken tk; static const vshCmdDef *cmd; const vshCmdOptDef *opt; - char *tkdata, *optstr, *const_tkdata; + char *tkdata, *optstr, *const_tkdata, *completed_name; char *res = NULL; static char *ctext, *sanitized_text; + static char **completed_list; + static unsigned int completed_list_index; static uint64_t const_opts_need_arg, const_opts_required, const_opts_seen; uint64_t opts_need_arg, opts_seen; size_t opt_index; static bool cmd_exists, opts_filled, opt_exists; - static bool non_bool_opt_exists; + static bool non_bool_opt_exists, data_complete; if (!state) { parser.pos = rl_line_buffer; @@ -2658,6 +2660,9 @@ vshReadlineParse(const char *text, int state) sanitized_text = NULL; optstr = NULL; + completed_list = NULL; + completed_list_index = 0; + /* Sanitize/de-quote the autocomplete text */ tk = sanitizer.getNextArg(NULL, &sanitizer, &sanitized_text, false); @@ -2687,6 +2692,7 @@ vshReadlineParse(const char *text, int state) cmd_exists = false; opts_filled = false; non_bool_opt_exists = false; + data_complete = false; const_opts_need_arg = 0; const_opts_required = 0; @@ -2707,12 +2713,12 @@ vshReadlineParse(const char *text, int state) if (vshCmddefOptFill(cmd, &const_opts_need_arg, &const_opts_required) < 0) goto error; + opts_need_arg = const_opts_need_arg; + opts_seen = const_opts_seen; opts_filled = true; } else if (tkdata[0] == '-' && tkdata[1] == '-' && c_isalnum(tkdata[2])) { /* Command retrieved successfully, move to options */ - opts_need_arg = const_opts_need_arg; - opts_seen = const_opts_seen; optstr = strchr(tkdata + 2, '='); opt_index = 0; @@ -2728,6 +2734,7 @@ vshReadlineParse(const char *text, int state) VIR_FREE(optstr); goto error; } + opt_exists = true; VIR_FREE(const_tkdata); if (opt->type != VSH_OT_BOOL) { @@ -2745,10 +2752,11 @@ vshReadlineParse(const char *text, int state) goto error; tkdata = const_tkdata; - if (STREQ(tkdata, sanitized_text)) { - /* auto-complete non-bool option */ - break; - } + } + if (STREQ(tkdata, sanitized_text)) { + /* auto-complete non-bool option arg */ + data_complete = true; + break; } if (opt->type != VSH_OT_ARGV) opts_need_arg &= ~(1ULL << opt_index); @@ -2760,17 +2768,19 @@ vshReadlineParse(const char *text, int state) goto error; } } - } else { - /* No -- option provided and some other token given */ - if (!opt_exists) { + } else if (!opt_exists) { + /* No -- option provided and some other token given + * Try to find the default option. + */ + if (!(opt = vshCmddefGetData(cmd, &opts_need_arg, &opts_seen)) + && opt->type == VSH_OT_BOOL) goto error; - } else if (non_bool_opt_exists) { - /* TODO - * -- non bool option present, so parse the next arg - * or call completer on it if it is to be completed - */ - return NULL; - } + opt_exists = true; + opts_need_arg = const_opts_need_arg; + opts_seen = const_opts_seen; + } else { + /* In every other case, return NULL */ + goto error; } VIR_FREE(const_tkdata); @@ -2795,10 +2805,26 @@ vshReadlineParse(const char *text, int state) VIR_FREE(const_tkdata); } - if (!cmd_exists) + if (!cmd_exists) { res = vshReadlineCommandGenerator(sanitized_text, state); - else if (opts_filled && !non_bool_opt_exists) + } else if (opts_filled && !non_bool_opt_exists) { res = vshReadlineOptionsGenerator(sanitized_text, state, cmd); + } else if (non_bool_opt_exists && data_complete && opt->completer) { + if (!completed_list) + completed_list = opt->completer(opt->completer_flags); + if (completed_list) { + while ((completed_name = completed_list[completed_list_index])) { + if (!STRPREFIX(completed_name, sanitized_text)) + continue; + res = vshStrdup(NULL, completed_name); + completed_list_index++; + return res; + } + res = NULL; + virStringFreeList(completed_list); + completed_list_index = 0; + } + } if (!res) { VIR_FREE(sanitized_text); -- 2.1.4
Signed-off-by: Nishith Shah <nishithshah.2211@gmail.com> --- tools/vsh.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tools/vsh.c b/tools/vsh.c index 79656ed..e8aa7b3 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -2607,10 +2607,6 @@ vshReadlineOptionsGenerator(const char *text, int state, const vshCmdDef *cmd_pa list_index++; - if (opt->type == VSH_OT_DATA || opt->type == VSH_OT_ARGV) - /* ignore non --option */ - continue; - if (len > 2) { /* provide auto-complete only when the text starts with -- */ if (STRNEQLEN(text, "--", 2)) -- 2.1.4
Before this patch: virsh # start --domain dom1 [TAB][TAB] <- offers filename completion virsh # start --domain [TAB][TAB] <- offers filename completion After this patch: virsh # start --domain dom1 [TAB][TAB] <- offers command completion virsh # start --domain [TAB][TAB] <- calls domain completer if defined, otherwise falls back to filename completion Signed-off-by: Nishith Shah <nishithshah.2211@gmail.com> --- tools/vsh.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tools/vsh.c b/tools/vsh.c index e8aa7b3..2f1962b 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -2731,6 +2731,7 @@ vshReadlineParse(const char *text, int state) goto error; } + opts_seen = const_opts_seen; opt_exists = true; VIR_FREE(const_tkdata); if (opt->type != VSH_OT_BOOL) { @@ -2748,14 +2749,14 @@ vshReadlineParse(const char *text, int state) goto error; tkdata = const_tkdata; + virSkipSpaces((const char **)&tkdata); } if (STREQ(tkdata, sanitized_text)) { /* auto-complete non-bool option arg */ data_complete = true; break; } - if (opt->type != VSH_OT_ARGV) - opts_need_arg &= ~(1ULL << opt_index); + non_bool_opt_exists = false; } else { tkdata = NULL; /* opt type is BOOL */ -- 2.1.4
On 20.08.2016 07:52, Nishith Shah wrote:
This series introduces option completers and adds some minor improvements and fixes(not bugs per se, just better/sane behavior) in vshReadlineParse.
The first patch introduces the usage of option completers to auto-complete arguments for a particular option.
The second and third patches provide small improvements like completing the options of type VSH_OT_ARGV or VSH_OT_DATA, and to complete multiple options as well, if a previous option requires an argument, and that argument has been provided.
Nishith Shah (3): virsh: Introduce usage of option completers to auto-complete arguments virsh: Allow data or argument options to be completed as well virsh: Complete multiple options when any one option requires data
tools/vsh.c | 75 ++++++++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 49 insertions(+), 26 deletions(-)
Okay, the patches look good to me. And I would merge them right away, but there is a slight change in behaviour in virsh after your patch 2/3. And you even describe it in the commit message: Previously we had: virsh # domstate<TAB><TAB> just "--reason" was offered. Now, after the change "--domain" is offered for completion too. This is important so that we know which completer should we call for the following input for instance: virsh # domstate --domain<TAB><TAB> In this case we want to offer list of domains. I'd really value output from others whether this might work or is this change something we don't wanna do? Michal
On Wed, Aug 24, 2016 at 9:54 PM, Michal Privoznik <mprivozn@redhat.com> wrote:
On 20.08.2016 07:52, Nishith Shah wrote:
This series introduces option completers and adds some minor improvements and fixes(not bugs per se, just better/sane behavior) in vshReadlineParse.
The first patch introduces the usage of option completers to auto-complete arguments for a particular option.
The second and third patches provide small improvements like completing the options of type VSH_OT_ARGV or VSH_OT_DATA, and to complete multiple options as well, if a previous option requires an argument, and that argument has been provided.
Nishith Shah (3): virsh: Introduce usage of option completers to auto-complete arguments virsh: Allow data or argument options to be completed as well virsh: Complete multiple options when any one option requires data
tools/vsh.c | 75 ++++++++++++++++++++++++++++++ ++++++++++--------------------- 1 file changed, 49 insertions(+), 26 deletions(-)
Okay, the patches look good to me. And I would merge them right away, but there is a slight change in behaviour in virsh after your patch 2/3. And you even describe it in the commit message:
Previously we had:
virsh # domstate<TAB><TAB>
just "--reason" was offered. Now, after the change "--domain" is offered for completion too. This is important so that we know which completer should we call for the following input for instance:
virsh # domstate --domain<TAB><TAB>
In this case we want to offer list of domains.
I'd really value output from others whether this might work or is this change something we don't wanna do?
Well, I couldn't really think of any reason why we were not showing the options of type VSH_OT_ARGV and VSH_OT_DATA except maybe that we didn't have support for further required completion and also the correctness of auto-complete in such a case. I think now vshReadlineParse handles such cases fairly well. Nishith
participants (2)
-
Michal Privoznik -
Nishith Shah