From: Keith Seitz <keiths@redhat.com>
To: Andrew Burgess <andrew.burgess@embecosm.com>
Cc: gdb-patches@sourceware.org
Subject: Re: [PATCH v3 00/19] New completer API
Date: Sat, 08 Aug 2015 16:25:00 -0000 [thread overview]
Message-ID: <55C62D6D.1000903@redhat.com> (raw)
In-Reply-To: <20150808064442.GB2986@embecosm.com>
On 08/07/2015 11:44 PM, Andrew Burgess wrote:
>> 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.
I'll have to try and sneak something like that past Doug on some other
series. :-)
>> 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.
That's a valid point. Thank you for reminding me of that. With large
patches like this (with multiple revisions), I can sometimes not see the
proverbial forest, or I forget it's even there.
> 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.
I appreciate your comments. Keep 'em coming!
Keith
next prev parent reply other threads:[~2015-08-08 16:25 UTC|newest]
Thread overview: 45+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-08-07 2:37 Keith Seitz
2015-08-06 19:18 ` [PATCH v3 09/19] Implement completion limiting for interpreter_completer Keith Seitz
2015-08-23 4:03 ` Doug Evans
2015-08-06 19:20 ` [PATCH v3 12/19] Implement completion limiting for sim_command_completer Keith Seitz
2015-08-23 4:11 ` Doug Evans
2015-08-06 19:58 ` [PATCH v3 18/19] Use the hashtable to accumulate completion results Keith Seitz
2015-08-23 17:53 ` Doug Evans
2015-08-06 19:58 ` [PATCH v3 06/19] Implement completion limiting for condition_completer Keith Seitz
2015-08-23 3:53 ` Doug Evans
2015-08-06 19:58 ` [PATCH v3 04/19] Implement completion limiting for add_filename_to_list Keith Seitz
2015-08-23 1:07 ` Doug Evans
2015-08-06 19:58 ` [PATCH v3 19/19] Remove the vector return result from the completion API Keith Seitz
2015-08-23 18:03 ` Doug Evans
2015-08-06 19:58 ` [PATCH v3 02/19] Remove completion_tracker_t from the public " Keith Seitz
2015-08-23 1:02 ` Doug Evans
2015-08-24 16:06 ` Doug Evans
2015-08-06 20:03 ` [PATCH v3 01/19] Add struct completer_data to the " Keith Seitz
2015-08-23 0:29 ` Doug Evans
2015-08-06 21:06 ` [PATCH v3 17/19] Make the completion API completely opaque Keith Seitz
2015-08-23 15:14 ` Doug Evans
2015-08-06 21:06 ` [PATCH v3 05/19] Implement completion limiting for ada_make_symbol_completion_list Keith Seitz
2015-08-23 3:47 ` Doug Evans
2015-08-06 22:03 ` [PATCH v3 07/19] Implement completion limiting for filename_completer Keith Seitz
2015-08-23 3:58 ` Doug Evans
2015-08-06 22:03 ` [PATCH v3 16/19] Implement completion limiting for tui_reggroup_completer Keith Seitz
2015-08-23 4:25 ` Doug Evans
2015-08-06 22:03 ` [PATCH v3 11/19] Implement completion limiting for reg_or_group_completer Keith Seitz
2015-08-23 4:09 ` Doug Evans
2015-08-06 22:03 ` [PATCH v3 08/19] Implement completion limiting for signal_completer Keith Seitz
2015-08-23 3:59 ` Doug Evans
2015-08-06 22:03 ` [PATCH v3 13/19] Implement completion limiting for complete_on_enum Keith Seitz
2015-08-23 4:19 ` Doug Evans
2015-08-06 22:12 ` [PATCH v3 10/19] Implement completion limiting for cmdpy_completer Keith Seitz
2015-08-23 4:07 ` Doug Evans
2015-08-06 22:12 ` [PATCH v3 14/19] Implement completion limiting in add_struct_fields Keith Seitz
2015-08-23 4:23 ` Doug Evans
2015-08-06 22:36 ` [PATCH v3 03/19] Implement completion-limiting for complete_on_cmdlist Keith Seitz
2015-08-23 1:05 ` Doug Evans
2015-08-07 2:37 ` [PATCH v3 15/19] Implement completion limiting for scmcmd_add_completion Keith Seitz
2015-08-23 4:24 ` Doug Evans
2015-08-07 22:57 ` [PATCH v3 00/19] New completer API Andrew Burgess
2015-08-08 0:04 ` Keith Seitz
2015-08-08 6:44 ` Andrew Burgess
2015-08-08 16:25 ` Keith Seitz [this message]
2015-08-22 22:25 ` Doug Evans
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=55C62D6D.1000903@redhat.com \
--to=keiths@redhat.com \
--cc=andrew.burgess@embecosm.com \
--cc=gdb-patches@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox