From 22904b5702d52b1407f7c259c41cfa046049c06a Mon Sep 17 00:00:00 2001 From: Michal Privoznik Date: Tue, 26 Jan 2021 09:51:27 +0100 Subject: [PATCH] vsh: Rework how option to complete is found MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 was pressed. For instance, for the following input: virsh # command --arg1 hello --arg2 world 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 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 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 Reviewed-by: Ján Tomko --- tools/virsh.c | 4 ++-- tools/virt-admin.c | 4 ++-- tools/vsh.c | 58 ++++++++++++++++++++++++++++++++-------------- tools/vsh.h | 5 +++- 4 files changed, 49 insertions(+), 22 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index a6bfbbbb87..732655a894 100644 --- 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 72084a78e9..1108a07896 100644 --- 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 eb29bbbeb8..be16d1db89 100644 --- 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 0c5584891d..39f70913fe 100644 --- 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);