Re: [PATCH 00/18] Implement full completer limiting

[Doug Evans <dje at google dot com>]

On Mon, Apr 13, 2015 at 12:23 PM, Keith Seitz <keiths@redhat.com> wrote:
> This series of patches is essentially a rewrite of the completion API
> to enable better/more consistent completion-limiting.
>
> Currently completer functions are not required to implement completion-
> limiting.  These functions will compute all possible completions and then
> rely on complete_line to limit the result.
>
> The main goal of this patchset is to require completer functions to
> implement proper completion-limiting using maybe_add_completion.  This
> actually cleans up the completer API significantly and fixes at least one
> serious bug (an assertion failure, gdb/17960).
>
> The new API requires all completions to be added to the completion
> list using maybe_add_completion:
>
> void
> my_completer_function (struct completer_data *cdata,
>                        struct cmd_list_element *cmd,
>                        const char *text, const char *prefix)
> {
>     while (/* there are more completions to look for */)
>     {
>       char *match = xstrdup (a_completion_match);
>       enum maybe_add_completion_enum add_status;
>
>       add_status = maybe_add_completion (cdata, match);
>       switch (add_status)
>       {
>         case MAYBE_ADD_COMPLETION_OK:
>           /* Completion was added -- keep looking for more.  */
>           break;
>         case MAYBE_ADD_COMPLETION_OK_MAX_REACHED:
>           /* Completion was added, but now at maximum permitted completions.
>              Stop looking for more matches.  */
>           return;
>         case MAYBE_ADD_COMPLETION_MAX_REACHED:
>           /* Completion was not added;  maximum permitted completions
>              already reached.  Stop looking for more matches.  */
>           xfree (match);
>           return;
>         case MAYBE_ADD_COMPLETION_DUPLICATE:
>           /* This completion is already in the list.  Keep looking for
>              more matches.  */
>           xfree (match);
>           break;
>       }
>     }
> }
>
> Each patch of the set has been tested regression-free against x86_64
> linux, native and native-gdbserver.

Hi.

I've gone over the entire patch set.  A few things I like, but there's
at least one thing I'm concerned about.  Replicating the above switch
in each completer: IWBN to avoid such duplication.
We should still be able to remove the global state and fix 17960.