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

Doug Evans
2015-04-20T03:21:49Z

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.