Re: [libvirt] [PATCHv2 1/4] Improve virsh autocompletion (extract parser)

This commit extracts the parsing logic from vshCommandParse so that it can be used by other methods. The vshCommandParse method is designed to parse full commands and error out on unknown information, so it is not suitable to simply use it for autocompletion. Instead, the logic has been extracted. The new method essentially performs one pass of the loop previously done by vshCommandParse. Depending on what happens, instead of erroring or continuing the loop, it returns a value from an enum indicating the current state. It also modifies several arguments as appropriate. Then, a caller can choose to deal with the state, or may simply ignore the state when convenient (vshCommandParse deals with the state, so as to continue providing its previous functionality, while a completer could choose to ignore states involving unknown options, for example). --- tools/virsh.c | 338 +++++++++++++++++++++++++++++++++++++++------------------- 1 file changed, 228 insertions(+), 110 deletions(-)

+ VSH_LINE_STATE_CMD_DONE, + VSH_LINE_STATE_LINE_DONE, +} vshLineExtractionState; + +static vshLineExtractionState

+vshExtractLinePart(vshControl *ctl, vshCommandParser *parser, + uint32_t *opts_seen, uint32_t *opts_need_arg, + char **tok_out, const vshCmdDef **cmd, + const vshCmdOptDef **opt, bool raise_err, int state) +{ + static bool data_only = false; + static uint32_t opts_required = 0;

+ vshCommandToken tok_type; + char *tok = NULL; + vshLineExtractionState ret = VSH_LINE_STATE_IN_PROGRESS; + + if (!state) { + data_only = false; + opts_required = 0; + } + + *opt = NULL; + + tok_type = parser->getNextArg(ctl, parser, &tok, raise_err); + + if (tok_type == VSH_TK_ERROR) { + ret = VSH_LINE_STATE_TOK_ERR; + *opt = NULL;

+ goto cleanup; + } else if (tok_type == VSH_TK_END) { + ret = VSH_LINE_STATE_LINE_DONE; + *opt = NULL;

+ goto cleanup; + } else if (tok_type == VSH_TK_SUBCMD_END) { + *opt = NULL;

+ //*cmd = NULL;

+ ret = VSH_LINE_STATE_CMD_DONE; + goto cleanup; + } + + if (*cmd == NULL) { + *cmd = vshCmddefSearch(tok); + if (!*cmd) { + ret = VSH_LINE_STATE_UNKNOWN_CMD; + *tok_out = vshStrdup(ctl, tok); + goto cleanup; + } + + if (vshCmddefOptParse(*cmd, opts_need_arg, &opts_required) < 0) + ret = VSH_LINE_STATE_BAD_OPTS; + else + ret = VSH_LINE_STATE_IN_PROGRESS; + } else if (data_only) { + goto get_data; + } else if (tok[0] == '-' && tok[1] == '-' && + c_isalnum(tok[2])) { + char *optstr = strchr(tok + 2, '='); + int opt_index = 0; + + if (optstr) { + *optstr = '\0'; /* convert the '=' to '\0' */ + optstr = vshStrdup(ctl, optstr + 1); + } + /* Special case 'help' to ignore all spurious options */

+ *opt = vshCmddefGetOption(ctl, *cmd, tok + 2, + opts_seen, &opt_index, + &optstr, raise_err); + if (!*opt) { + VIR_FREE(optstr); + *tok_out = vshStrdup(ctl, tok); + ret = VSH_LINE_STATE_UNKNOWN_OPT; + goto cleanup; + } + + VIR_FREE(tok); + + if ((*opt)->type != VSH_OT_BOOL) { + if (optstr) + tok = optstr; + else + tok_type = parser->getNextArg(ctl, parser, &tok, raise_err); + + if (tok_type == VSH_TK_ERROR) { + ret = VSH_LINE_STATE_TOK_ERR; + *tok_out = vshStrdup(ctl, tok); + } else if (tok_type == VSH_TK_END) { + ret = VSH_LINE_STATE_LINE_DONE; + } else if (tok_type == VSH_TK_SUBCMD_END) { + ret = VSH_LINE_STATE_CMD_DONE; + } else { + ret = VSH_LINE_STATE_IN_PROGRESS; + *tok_out = vshStrdup(ctl, tok); + } + + if ((*opt)->type != VSH_OT_ARGV) + *opts_need_arg &= ~(1 << opt_index); + } else { + if (optstr) { + ret = VSH_LINE_STATE_INVALID_EQUALS; + VIR_FREE(optstr); + } else { + ret = VSH_LINE_STATE_IN_PROGRESS; + } + } + } else if (tok[0] == '-' && tok[1] == '-' && !tok[2]) { + ret = VSH_LINE_STATE_DATA_ONLY; + data_only = true; + } else { + get_data: + *opt = vshCmddefGetData(*cmd, opts_need_arg, opts_seen); + if (!*opt) + ret = VSH_LINE_STATE_UNEXPECTED_DATA; + else + ret = VSH_LINE_STATE_IN_PROGRESS; + + *tok_out = vshStrdup(ctl, tok); + } + + cleanup: + VIR_FREE(tok); + return ret; +} + + static bool vshCommandParse(vshControl *ctl, vshCommandParser *parser) { char *tkdata = NULL; vshCmd *clast = NULL; vshCmdOpt *first = NULL; + int state = 0; if (ctl->cmd) { vshCommandFree(ctl->cmd); @@ -1906,11 +2044,10 @@ vshCommandParse(vshControl *ctl, vshCommandParser *parser) while (1) { vshCmdOpt *last = NULL; const vshCmdDef *cmd = NULL; - vshCommandToken tk; - bool data_only = false; uint32_t opts_need_arg = 0; uint32_t opts_required = 0; uint32_t opts_seen = 0; + vshLineExtractionState line_state; first = NULL; @@ -1918,112 +2055,85 @@ vshCommandParse(vshControl *ctl, vshCommandParser *parser) const vshCmdOptDef *opt = NULL; tkdata = NULL; - tk = parser->getNextArg(ctl, parser, &tkdata); - if (tk == VSH_TK_ERROR) - goto syntaxError; - if (tk != VSH_TK_ARG) { - VIR_FREE(tkdata); - break; - } + line_state = vshExtractLinePart(ctl, parser, &opts_seen, + &opts_need_arg, &tkdata, + &cmd, &opt, true, state); + state++;

- if (cmd == NULL) { - /* first token must be command name */ - if (!(cmd = vshCmddefSearch(tkdata))) { - vshError(ctl, _("unknown command: '%s'"), tkdata); - goto syntaxError; /* ... or ignore this command only? */ + if (line_state == VSH_LINE_STATE_TOK_ERR) { + if (opt) { /* we got some funky syntax in an option */ + vshError(ctl, + _("expected syntax: --%s <%s>"), + opt->name, + opt->type == + VSH_OT_INT ? _("number") : _("string")); } - if (vshCmddefOptParse(cmd, &opts_need_arg, - &opts_required) < 0) { + /* otherwise, we're here because the tokenizer couldn't + * even start reading */ + goto syntaxError; + } else if (line_state == VSH_LINE_STATE_LINE_DONE || + line_state == VSH_LINE_STATE_CMD_DONE) {

+ if (!opt) { /* we're actually at the end of the line */ + VIR_FREE(tkdata); + break; + } else { /* we have an flag without an arg */

vshError(ctl, - _("internal error: bad options in command: '%s'"), - tkdata); + _("expected syntax: --%s <%s>"), + opt->name, + opt->type == + VSH_OT_INT ? _("number") : _("string")); goto syntaxError; } - VIR_FREE(tkdata); - } else if (data_only) { - goto get_data; - } else if (tkdata[0] == '-' && tkdata[1] == '-' && - c_isalnum(tkdata[2])) { - char *optstr = strchr(tkdata + 2, '='); - int opt_index = 0; - - if (optstr) { - *optstr = '\0'; /* convert the '=' to '\0' */ - optstr = vshStrdup(ctl, optstr + 1); - } - /* Special case 'help' to ignore all spurious options */ - if (!(opt = vshCmddefGetOption(ctl, cmd, tkdata + 2, - &opts_seen, &opt_index, - &optstr))) { - VIR_FREE(optstr); - if (STREQ(cmd->name, "help")) - continue; + } else if (line_state == VSH_LINE_STATE_UNEXPECTED_DATA) { + if (STRNEQ(cmd->name, "help")) { + /* anything's find after help, but

+ * otherwise we got some unexpected + * positional arguments */ + vshError(ctl, _("unexpected data '%s'"), tkdata); goto syntaxError;