12:52 a.m.
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
12:52 a.m.
New subject: [libvirt] [PATCH 1/3] virsh: Introduce usage of option completers to auto-complete arguments
Call option completers if argument completion is requested using the
corresponding option completer, if it is defined.
Signed-off-by: Nishith Shah <nishithshah.2211(a)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
12:52 a.m.
New subject: [libvirt] [PATCH 2/3] virsh: Allow data or argument options to be completed as well
Signed-off-by: Nishith Shah <nishithshah.2211(a)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
12:52 a.m.
New subject: [libvirt] [PATCH 3/3] virsh: Complete multiple options when any one option requires data
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(a)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
11:24 a.m.
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
1:01 p.m.
On Wed, Aug 24, 2016 at 9:54 PM, Michal Privoznik <mprivozn(a)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
3012
days inactive
3016
days old
5 comments
2 participants
participants (2)
-
Michal Privoznik -
Nishith Shah