Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Daniel Jacobowitz <drow@false.org>
To: Tom Tromey <tromey@redhat.com>
Cc: gdb-patches@sources.redhat.com
Subject: Re: [RFA] Patch to limit field name completion candidates
Date: Thu, 05 Jun 2008 17:10:00 -0000	[thread overview]
Message-ID: <20080605170952.GJ29085@caradoc.them.org> (raw)
In-Reply-To: <m38wyf6py1.fsf@fleche.redhat.com>

On Mon, May 12, 2008 at 05:52:06PM -0600, Tom Tromey wrote:
> How this works:
> 
> I added a new expression_completer and set this as the completion
> function for various things, like "p".  This tries to complete a field
> expression, but falls back to the old location_completer (which is
> what all these commands currently use).
> 
> The expression completer sets a global flag, in_parse_field, and then
> calls the expression parser.  The parsers are expected to look at this
> flag and call mark_struct_expression at the right time.  Then, if this
> was called, the generic code extracts the left-hand-side
> subexpression, gets its type, and then does field completion as you'd
> expect.
> 
> I only updated the C parser.  This code works by modifying the lexer
> to return a special COMPLETE token in the important cases.  Note that
> it completes both "p foo.TAB" and "p foo.somethingTAB" correctly --
> the former by making an expression to a field with an empty name.

Hi Tom,

Here's a few things I noticed.

We should complete after pointers and references, not just structs.
Especially true for arrow, as that is not especially useful on
structs.

I am concerned that relying on the parser to parse incomplete
expressions will not work out.  The bodies won't be run until the rule
is reduced, and there may not be enough context in what the user has
typed to reduce the field completion.  I'm not sure that's a real
problem, but I worry that it will be fragile.  Still - clearly better
than nothing.

I tried "p (*gdb_stdout).<tab>" and got no completions.  On the
testcase, "p (values[0]).<tab>" works so I am not sure what the
problem is.  Could you take a look at it?

Oh, and at least a NEWS entry would be good - and probably a manual
change too.

>     into OUTEXPR, starting at index OUTBEG.
>     In the process, convert it from suffix to prefix form.  */
>  
> -static void
> +static int
>  prefixify_subexp (struct expression *inexpr,
>  		  struct expression *outexpr, int inend, int outbeg)
>  {

Please document the return value.

Otherwise, the code looks fine.

-- 
Daniel Jacobowitz
CodeSourcery


  parent reply	other threads:[~2008-06-05 17:10 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-05-13 17:01 Tom Tromey
2008-05-13 18:11 ` Eli Zaretskii
2008-05-13 18:20   ` Tom Tromey
2008-05-13 18:32     ` Daniel Jacobowitz
2008-05-13 18:36       ` Tom Tromey
2008-05-13 21:00       ` Eli Zaretskii
2008-05-13 20:52     ` Eli Zaretskii
2008-06-05 17:10 ` Daniel Jacobowitz [this message]
2008-06-05 19:21   ` Tom Tromey
2008-06-05 19:30     ` Eli Zaretskii
2008-06-05 20:02       ` Tom Tromey
2008-06-06  9:47         ` Eli Zaretskii
2008-06-05 19:46     ` Daniel Jacobowitz
2008-06-05 20:03       ` Tom Tromey
2008-06-05 20:08         ` Daniel Jacobowitz
2008-06-06  0:49           ` Tom Tromey
2008-06-06  2:32             ` Daniel Jacobowitz
2008-06-06  2:41               ` Tom Tromey
2008-06-06 10:28                 ` Eli Zaretskii
2008-06-06 17:50                   ` Tom Tromey
2008-06-06 19:46                     ` Eli Zaretskii
2008-06-06 19:54                       ` Daniel Jacobowitz
2008-06-05 20:50       ` Tom Tromey
2008-06-06 10:13         ` Eli Zaretskii
2008-06-06 10:53           ` Eli Zaretskii

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=20080605170952.GJ29085@caradoc.them.org \
    --to=drow@false.org \
    --cc=gdb-patches@sources.redhat.com \
    --cc=tromey@redhat.com \
    /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