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

[Andrew Burgess <andrew.burgess@embecosm.com>]

* Keith Seitz <keiths@redhat.com> [2015-08-07 17:04:08 -0700]:

> 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,

Sorry, I should have said "users of the API".

> 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?

No. But you could rename to
add_completion_then_should_more_completions_be_added (he jokes)
(though maybe add_completion_and_continue would work?).  But
I would have just assumed a good comment was enough.

> 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.

I agree with that point.

> 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?

My response was certainly _not_ on any technical grounds, simply a
matter of style.  As a general rule when I see every use of a
function be:

   function_call (args) == SAME_VALUE

I tend to think, can't we just push the comparison (effectively) up
into the function_call.

Anyway, it was just a passing thought based on a mild interest in this
code, having recently touched it, I don't think it's a big issue.

Thanks
Andrew