[PATCH 08/12] vsh: Deduplicate filtering in vshReadlineOptionsGenerator()

[Jonathon Jongsma]

On Thu,  4 Feb 2021 15:13:33 +0100
Michal Privoznik <mprivozn at redhat.com> wrote:

> This is what we do for completer callbacks: we let them generate
> all possible outputs ignoring any partial input (e.g. prefix of a
> domain name) and then use vshCompleterFilter() to filter out
> those strings which don't fit the partial input (prefix).
> 
> The same algorithm is implemented in
> vshReadlineOptionsGenerator() even though a bit differently.
> There is no need to have the same code twice.

I think this might be clearer stated a bit differently. For example, if
I'm understanding correctly, a suggested alternate commit message:

    Completer callbacks generate all possible outputs ignoring any partial
    input (e.g. prefix of a domain name) and then use vshCompleterFilter() to
    filter out those strings which don't fit the partial input (prefix).

    In contrast, vshReadlineOptionsGenerator() does some internal filtering and
    only generates completions that match a given prefix. Rather than treating
    these scenarios differently, simply generate all possible options and
    filter them all at the end.

Reviewed-by: Jonathon Jongsma <jjongsma at redhat.com>

> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  tools/vsh.c | 26 ++++++++------------------
>  1 file changed, 8 insertions(+), 18 deletions(-)
> 
> diff --git a/tools/vsh.c b/tools/vsh.c
> index f83b2a95a9..5f082f1a35 100644
> --- a/tools/vsh.c
> +++ b/tools/vsh.c
> @@ -2607,12 +2607,11 @@ vshReadlineCommandGenerator(const char *text)
>  }
>  
>  static char **
> -vshReadlineOptionsGenerator(const char *text,
> +vshReadlineOptionsGenerator(const char *text G_GNUC_UNUSED,
>                              const vshCmdDef *cmd,
>                              vshCmd *last)
>  {
>      size_t list_index = 0;
> -    size_t len = strlen(text);
>      size_t ret_size = 0;
>      g_auto(GStrv) ret = NULL;
>  
> @@ -2631,16 +2630,6 @@ vshReadlineOptionsGenerator(const char *text,
>          if (cmd->opts[list_index].type == VSH_OT_ALIAS)
>              continue;
>  
> -        if (len > 2) {
> -            /* provide auto-complete only when the text starts with
> -- */
> -            if (STRNEQLEN(text, "--", 2))
> -                return NULL;
> -            if (STRNEQLEN(name, text + 2, len - 2))
> -                continue;
> -        } else if (STRNEQLEN(text, "--", len)) {
> -            return NULL;
> -        }
> -
>          while (opt) {
>              if (STREQ(opt->def->name, name) && opt->def->type !=
> VSH_OT_ARGV) { exists = true;
> @@ -2790,14 +2779,15 @@ vshReadlineParse(const char *text, int state)
>                      }
>                  }
>  
> -                /* For string list returned by completer we have to
> do
> -                 * filtering based on @text because completer
> returns all
> -                 * possible strings. */
> -                if (vshCompleterFilter(&completer_list, text) < 0 ||
> -                    virStringListMerge(&list, &completer_list) < 0) {
> +                if (virStringListMerge(&list, &completer_list) < 0)
>                      goto cleanup;
> -                }
>              }
> +
> +            /* For string list returned by completers we have to do
> +             * filtering based on @text because completers returns
> all
> +             * possible strings. */
> +            if (vshCompleterFilter(&list, text) < 0)
> +                goto cleanup;
>          }
>      }
>