vsh: Rework how option to complete is found

The way that auto completion works currently is that user's input
is parsed, and then we try to find the first --option (in the
parsed structure) that has the same value as user's input around
where <TAB> was pressed. For instance, for the following input:

  virsh # command --arg1 hello --arg2 world<TAB>

we will see "world" as text that user is trying to autocomplete
(this is affected by rl_basic_word_break_characters which
readline uses internally to break user's input into individual
words) and find that it is --arg2 that user is trying to
autocomplete. So far so good, for this naive approach. But
consider the following example:

  virsh # command --arg1 world --arg2 world<TAB>

Here, both arguments have the same value and because we see
"world" as text that user is trying to autocomplete we would
think that it is --arg1 that user wants to autocomplete. This is
obviously wrong.

Fortunately, readline stores the current position of cursor (into
rl_point) and we can use that when parsing user's input: whenever
we reach a position that matches the cursor then we know that
that is the place where <TAB> was pressed and hence that is the
--option that user wants to autocomplete. Readline stores the
cursor position as offset (numbered from 1) from the beginning of
user's input. We store this input into @parser->pos initially,
but then advance it as we tokenize it. Therefore, what we need is
to store the original position too.

Thanks to Martin who helped me with this.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

diff --git a/tools/virsh.c b/tools/virsh.c
index a6bfbbbb87ea00450b770e47e88631e568676b33..732655a89474290823817ede08322c635e4a8f39 100644 (file)
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -777,7 +777,7 @@ virshParseArgv(vshControl *ctl, int argc, char **argv)
         ctl->imode = false;
         if (argc - optind == 1) {
             vshDebug(ctl, VSH_ERR_INFO, "commands: \"%s\"\n", argv[optind]);
-            return vshCommandStringParse(ctl, argv[optind], NULL);
+            return vshCommandStringParse(ctl, argv[optind], NULL, 0);
         } else {
             return vshCommandArgvParse(ctl, argc - optind, argv + optind);
         }
@@ -915,7 +915,7 @@ main(int argc, char **argv)
             if (*ctl->cmdstr) {
                 vshReadlineHistoryAdd(ctl->cmdstr);
 
-                if (vshCommandStringParse(ctl, ctl->cmdstr, NULL))
+                if (vshCommandStringParse(ctl, ctl->cmdstr, NULL, 0))
                     vshCommandRun(ctl, ctl->cmd);
             }
             VIR_FREE(ctl->cmdstr);

diff --git a/tools/virt-admin.c b/tools/virt-admin.c
index 72084a78e906a53040173f3ce376cf4890e98f08..1108a07896ba4a3b0d703f651868241442faeb84 100644 (file)
--- a/tools/virt-admin.c
+++ b/tools/virt-admin.c
@@ -1364,7 +1364,7 @@ vshAdmParseArgv(vshControl *ctl, int argc, char **argv)
         ctl->imode = false;
         if (argc - optind == 1) {
             vshDebug(ctl, VSH_ERR_INFO, "commands: \"%s\"\n", argv[optind]);
-            return vshCommandStringParse(ctl, argv[optind], NULL);
+            return vshCommandStringParse(ctl, argv[optind], NULL, 0);
         } else {
             return vshCommandArgvParse(ctl, argc - optind, argv + optind);
         }
@@ -1594,7 +1594,7 @@ main(int argc, char **argv)
             if (*ctl->cmdstr) {
                 vshReadlineHistoryAdd(ctl->cmdstr);
 
-                if (vshCommandStringParse(ctl, ctl->cmdstr, NULL))
+                if (vshCommandStringParse(ctl, ctl->cmdstr, NULL, 0))
                     vshCommandRun(ctl, ctl->cmd);
             }
             VIR_FREE(ctl->cmdstr);

diff --git a/tools/vsh.c b/tools/vsh.c
index eb29bbbeb8f6c0e2ddd59093898b771845d5c44f..be16d1db8909e5fd4cb8990ccd5e1fd24938837a 100644 (file)
--- a/tools/vsh.c
+++ b/tools/vsh.c
@@ -1318,6 +1318,8 @@ struct _vshCommandParser {
                                  char **, bool);
     /* vshCommandStringGetArg() */
     char *pos;
+    const char *originalLine;
+    size_t point;
     /* vshCommandArgvGetArg() */
     char **arg_pos;
     char **arg_end;
@@ -1426,6 +1428,10 @@ vshCommandParse(vshControl *ctl, vshCommandParser *parser, vshCmd **partial)
                             arg->data = tkdata;
                             tkdata = NULL;
                             arg->next = NULL;
+
+                            if (parser->pos - parser->originalLine == parser->point - 1)
+                                arg->completeThis = true;
+
                             if (!first)
                                 first = arg;
                             if (last)
@@ -1477,6 +1483,9 @@ vshCommandParse(vshControl *ctl, vshCommandParser *parser, vshCmd **partial)
                 arg->next = NULL;
                 tkdata = NULL;
 
+                if (parser->pos - parser->originalLine == parser->point)
+                    arg->completeThis = true;
+
                 if (!first)
                     first = arg;
                 if (last)
@@ -1588,7 +1597,7 @@ vshCommandArgvGetArg(vshControl *ctl G_GNUC_UNUSED,
 bool
 vshCommandArgvParse(vshControl *ctl, int nargs, char **argv)
 {
-    vshCommandParser parser;
+    vshCommandParser parser = { 0 };
 
     if (nargs <= 0)
         return false;
@@ -1675,15 +1684,38 @@ vshCommandStringGetArg(vshControl *ctl, vshCommandParser *parser, char **res,
     return VSH_TK_ARG;
 }
 
+
+/**
+ * vshCommandStringParse:
+ * @ctl virsh control structure
+ * @cmdstr: string to parse
+ * @partial: store partially parsed command here
+ * @point: position of cursor (rl_point)
+ *
+ * Parse given string @cmdstr as a command and store it under
+ * @ctl->cmd. For readline completion, if @partial is not NULL on
+ * the input then errors in parsing are ignored (because user is
+ * still in progress of writing the command string) and partially
+ * parsed command is stored at *@partial (caller has to free it
+ * afterwards). Among with @partial, caller must set @point which
+ * is the position of cursor in @cmdstr (offset, numbered from 1).
+ * Parser will then set @completeThis attribute to true for the
+ * vshCmdOpt that appeared under the cursor.
+ */
 bool
-vshCommandStringParse(vshControl *ctl, char *cmdstr, vshCmd **partial)
+vshCommandStringParse(vshControl *ctl,
+                      char *cmdstr,
+                      vshCmd **partial,
+                      size_t point)
 {
-    vshCommandParser parser;
+    vshCommandParser parser = { 0 };
 
     if (cmdstr == NULL || *cmdstr == '\0')
         return false;
 
     parser.pos = cmdstr;
+    parser.originalLine = cmdstr;
+    parser.point = point;
     parser.getNextArg = vshCommandStringGetArg;
     return vshCommandParse(ctl, &parser, partial);
 }
@@ -2634,28 +2666,20 @@ vshReadlineOptionsGenerator(const char *text,
 
 
 static const vshCmdOptDef *
-vshReadlineCommandFindOpt(const vshCmd *partial,
-                          const char *text)
+vshReadlineCommandFindOpt(const vshCmd *partial)
 {
     const vshCmd *tmp = partial;
 
-    while (tmp && tmp->next) {
-        if (tmp->def == tmp->next->def &&
-            !tmp->next->opts)
-            break;
-        tmp = tmp->next;
-    }
-
-    if (tmp && tmp->opts) {
+    while (tmp) {
         const vshCmdOpt *opt = tmp->opts;
 
         while (opt) {
-            if (STREQ_NULLABLE(opt->data, text) ||
-                STREQ_NULLABLE(opt->data, " "))
+            if (opt->completeThis)
                 return opt->def;
 
             opt = opt->next;
         }
+        tmp = tmp->next;
     }
 
     return NULL;
@@ -2717,7 +2741,7 @@ vshReadlineParse(const char *text, int state)
 
         *(line + rl_point) = '\0';
 
-        vshCommandStringParse(NULL, line, &partial);
+        vshCommandStringParse(NULL, line, &partial, rl_point);
 
         if (partial) {
             cmd = partial->def;
@@ -2735,7 +2759,7 @@ vshReadlineParse(const char *text, int state)
             cmd = NULL;
         }
 
-        opt = vshReadlineCommandFindOpt(partial, text);
+        opt = vshReadlineCommandFindOpt(partial);
 
         if (!cmd) {
             list = vshReadlineCommandGenerator(text);

diff --git a/tools/vsh.h b/tools/vsh.h
index 0c5584891d7011f7d316373dfe87c66a68f948ab..39f70913fe7b2965381bb0e606da62a4720a340e 100644 (file)
--- a/tools/vsh.h
+++ b/tools/vsh.h
@@ -152,6 +152,8 @@ struct _vshCmdOptDef {
 struct _vshCmdOpt {
     const vshCmdOptDef *def;    /* non-NULL pointer to option definition */
     char *data;                 /* allocated data, or NULL for bool option */
+    bool completeThis;          /* true if this is the option user's wishing to
+                                   autocomplete */
     vshCmdOpt *next;
 };
 
@@ -292,7 +294,8 @@ int vshBlockJobOptionBandwidth(vshControl *ctl,
                                unsigned long *bandwidth);
 bool vshCommandOptBool(const vshCmd *cmd, const char *name);
 bool vshCommandRun(vshControl *ctl, const vshCmd *cmd);
-bool vshCommandStringParse(vshControl *ctl, char *cmdstr, vshCmd **partial);
+bool vshCommandStringParse(vshControl *ctl, char *cmdstr,
+                           vshCmd **partial, size_t point);
 
 const vshCmdOpt *vshCommandOptArgv(vshControl *ctl, const vshCmd *cmd,
                                    const vshCmdOpt *opt);