Re: [PATCH v3 00/19] New completer API

[Keith Seitz <keiths@redhat.com>]

On 08/07/2015 03:56 PM, Andrew Burgess wrote:
> I think that the above text is out of date w.r.t. the example below,
> in the above you talk about maybe_add_completion, but in the example
> below you use the add_completion wrapper.  A small detail and
> unimportant detail; unless I'm missing something, in which case
> ... I'm confused!
> 

Yup, you're right. s/maybe_add_completion/add_completion/g.

> I wonder, looking at add_completion, do users _care_ about the reason?

Users? No. Developers? Yes. add_completion either returns ..._OK,
meaning that more completions may be added. ..._MAX_REACHED meaning that
no more completions should be added (but will just waste cycles).

The return value does not indicate whether the addition was successful.
Just whether more may be added.

> Of all the possible return codes from add_completion, I think we can
> classify them as either "stop trying to add completions please", or
> "feel free to add more completions".

Yup.

> Could the add_completion interface be simplified to simply return a
> bool, true if more completions can be added, false if not?

I don't like boolean return values in this case. Forget knowing what
happens under the covers (or now that you've read the proposed API).
Just by reading "bool add_completion (struct completer_data *, const
char *);" can you tell what the boolean return value means?

My first reading of that would be "the addition was successfully
completed," but that's not what it means in this specific case, because
the return value indicates something entirely different.

But now I wonder if I am missing something. Is defining an enum going to
choke some compiler? Does it violate, or is it ambiguous in, the
language? Or is it simply a matter of style?

Keith