Re: [PATCH] PR python/16699: GDB Python command completion with overriden complete vs. completer class

[Tom Tromey <tromey@redhat.com>]

>>>>> "Sergio" == Sergio Durigan Junior <sergiodj@redhat.com> writes:

Sergio> Unfortunately I couldn't come up with a good way to do that.  readline
Sergio> messes with the text/word being parsed while it is parsing them, so it
Sergio> is hard to restart the parsing because we may not have the full context
Sergio> in all situations.  Or at least that's what I found.

I looked through the readline docs here.
Ok, I see now, the completion API is irredeemably wacky.

Sergio> But I fixed my patch according to your comment above, and I think now it
Sergio> is right.  What I did is simple: instead of providing dummy arguments to
Sergio> the completer, I am now passing the real arguments.  As far as I have
Sergio> tested, it works.

Cute.

Sergio> What do you think?  Is it too hacky?

Someday we will invent a hackiness metric to let us know for sure.
"M(h) is greater than 0.98!  Patch rejected by the robot."


Seriously, at first I thought this was probably a bad idea.
And it is a little weird, since first it word-breaks some random way,
then redoes the breaking later.

Is there a way to call the Python function just once and store the
results in the non-enum-return case?  Since otherwise it seems that
every completion request requires two calls to a
possibly-expensive-though-we-hope-not completer.

Anyway I'm ok-enough with it I suppose.

Sergio> +void
Sergio> +set_cmd_completer_handle_brkchars (struct cmd_list_element *cmd,
Sergio> +	  void (*completer_handle_brkchars) (struct cmd_list_element *self,
Sergio> +					     const char *text,
Sergio> +					     const char *word))
Sergio> +{
Sergio> +  cmd->completer_handle_brkchars = completer_handle_brkchars; /* Ok.  */

I think the "Ok" comment usually is there as a note to the ARI.
However, does ARI actually check this line?
If not -> no comment needed.

Sergio> +/* Set the completer_handle_brkchars callback.  */
Sergio> +
Sergio> +extern void set_cmd_completer_handle_brkchars (struct cmd_list_element *,
Sergio> +					       void (*f)
Sergio> +					       (struct cmd_list_element *,
Sergio> +						const char *,
Sergio> +						const char *));

I think the "f" argument should have type "completer_ftype *" rather
than being spelled out.

Sergio> +static void
Sergio> +cmdpy_completer_handle_brkchars (struct cmd_list_element *command,
Sergio> +				 const char *text, const char *word)
Sergio> +{
Sergio> +  cmdpy_object *obj = (cmdpy_object *) get_cmd_context (command);
Sergio> +  PyObject *textobj, *wordobj, *resultobj = NULL;
Sergio> +  /*  const char dummy_text[] = "dummytext";
Sergio> +      const char dummy_word[] = "dummyword"; */

No need for the commented-out bits.

Sergio> +# This one should always pass.
Sergio> +send_gdb "completefileinit ${testdir_complete}\t"
Sergio> +gdb_test_multiple "" "completefileinit completion" {
Sergio> +    -re "^completefileinit ${testdir_regex}$" {
Sergio> +        pass "completefileinit completion"
Sergio> +    }

FWIW I generally find it simpler to test the "complete" command rather
than the send_gdb dance.

Either way is ok though.

Tom