PR python/16699: GDB Python command completion with overriden complete vs. completer class
Commit Message
Sergio Durigan Junior <sergiodj@redhat.com> writes:
> On Tuesday, August 19 2014, I wrote:
>
>> On Tuesday, July 08 2014, I wrote:
>>
>>> Thank you for the catches. I will update my local version.
>>
>> Ping.
>
> Ping^2.
>
>> --
>> Sergio
>> GPG key ID: 0x65FC5E36
>> Please send encrypted e-mail if possible
>> http://sergiodj.net/
>>
>> gdb/
>> 2014-08-19 Sergio Durigan Junior <sergiodj@redhat.com>
>>
>> PR python/16699
>> * cli/cli-decode.c (set_cmd_completer_handle_brkchars): New
>> function.
>> (add_cmd): Set "completer_handle_brkchars" to NULL.
>> * cli/cli-decode.h (struct cmd_list_element)
>> <completer_handle_brkchars>: New field.
>> * command.h (completer_ftype_void): New typedef.
>> (set_cmd_completer_handle_brkchars): New prototype.
>> * completer.c (set_gdb_completion_word_break_characters): New
>> function.
>> (complete_line_internal): Call "completer_handle_brkchars"
>> callback from command.
>> * completer.h: Include "command.h".
>> (set_gdb_completion_word_break_characters): New prototype.
>> * python/py-cmd.c (cmdpy_completer_helper): New function.
>> (cmdpy_completer_handle_brkchars): New function.
>> (cmdpy_completer): Adjust to use cmdpy_completer_helper.
>> (cmdpy_init): Set completer_handle_brkchars to
>> cmdpy_completer_handle_brkchars.
>>
>> gdb/testsuite/
>> 2014-08-19 Sergio Durigan Junior <sergiodj@redhat.com>
>>
>> PR python/16699
>> * gdb.python/py-completion.exp: New file.
>> * gdb.python/py-completion.py: Likewise.
Hi.
I've spent some more time looking at the patch.
I have one style nit and one request.
Style nit: I see lots of places that do "if (!ptr)".
Convention is to write this as "if (ptr == NULL)".
Request:
The "why" of things is explained sufficiently well in your
original email:
https://sourceware.org/ml/gdb-patches/2014-03/msg00301.html
And while I can appreciate this text being added to the commit message,
I don't want to have to read commit logs to have a basic understanding
of the "why" of the code. [I'm all for more deeper understanding
being deferred to commit logs as appropriate, but I should be able
to get a basic understanding from the code itself.]
Can you add something descriptive to the code?
For example, how about something like this?
I have no more comments so LGTM with those changes.
@@ -219,6 +219,14 @@ cmdpy_function (struct cmd_list_element
returns that variable, without actually calling the Python method
again. This saves us one Python method call.
+ The reason for this two step dance is that we need to know the set
+ of "brkchars" to use early on, before we actually try to perform the
+ completion. But if a Python command supplies a "complete" method then
+ we have to call that method first: it may return as its result the kind
+ of completion to perform and that will in turn specify which brkchars
+ to use. IOW, we need the result of the "complete" method before we
+ actually perform the completion.
+
It is important to mention that this function is built on the
assumption that the calls to cmdpy_completer_handle_brkchars and
cmdpy_completer will be subsequent with nothing intervening. This