Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
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


  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