Add autocompletion for convenience vars in print and set

Message ID CAF5HaEUosXVSvxJQkEp3DgBYMMKGNXnJ-r4LOQV3PDeqfYnptA@mail.gmail.com
State New, archived
Headers

Commit Message

Daniel Gutson May 22, 2014, 3:17 p.m. UTC
  Second version.
Comments below:

On Tue, May 20, 2014 at 1:36 PM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Daniel" == Daniel Gutson <daniel.gutson@tallertechnologies.com> writes:
>
> Daniel> I could not find any testsuite where to add tests for this; if
> Daniel> there are, please let me know.
>
> See testsuite/gdb.base/completion.exp

Thanks, I ran all the tests and passed. I did not add a new test
case though since I didn't check how to add a new convenience var
and undefine it later from the test framework. (Should I try harder?)

>
> Daniel>  exp    :       VARIABLE
> Daniel>                         {
> Daniel> -                         write_dollar_variable (pstate, $1);
> Daniel> +                         if (!parse_completion)
> Daniel> +                           write_dollar_variable (pstate, $1);
> Daniel>                         }
>
> I think this isn't correct.  I think it won't work if you try to
> complete on a field name where the "LHS" has a convenience variable.
> That is something like:
>
>    set $var = (struct x *) malloc (...)
>    complete print $var.somethin

You were right, that was broken.

>
> Instead I think you need a new production, like "exp : VARIABLE COMPLETE".

I found another solution without touching the yacc file: adding a check
inside write_dollar_variable

>
> Daniel> +  if (p != NULL && *p == '$')
> Daniel> +    return complete_internalvar (p + 1);
>
> I'm not sure this is correct either, but offhand I don't know.
> Should it not look at "word"?

It actually works ("word" didn't have the $).

However, I noticed that 'set' cmd autocomplete doesn't work for
members of structures (neither with convenience vars nor regular
symbols). I'll address that in the next patch once this gets approved
and committed.

>
> Daniel> +  VEC (char_ptr) * ret = current_language->la_make_symbol_completion_list (
> Daniel> +                                                      text, word,
> Daniel> +                                                      TYPE_CODE_UNDEF);
>
> No space before "ret".
> The line breaks look odd, I would break before the "=" and not after the "(".
>
> thanks,
> Tom

2014-05-22  Daniel Gutson  <daniel.gutson@tallertechnologies.com>

gdb/
        * parse.c (write_dollar_variable): Do not create an internal
var when completing.
        * completer.c (expression_completer): Call complete_internalvar.
        * symtab.c (make_symbol_completion_list): Call complete_internalvar.
  

Comments

Daniel Gutson May 27, 2014, 3:18 p.m. UTC | #1
ping for maintainer.

Thanks.

On Thu, May 22, 2014 at 12:17 PM, Daniel Gutson
<daniel.gutson@tallertechnologies.com> wrote:
> Second version.
> Comments below:
>
> On Tue, May 20, 2014 at 1:36 PM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>>> "Daniel" == Daniel Gutson <daniel.gutson@tallertechnologies.com> writes:
>>
>> Daniel> I could not find any testsuite where to add tests for this; if
>> Daniel> there are, please let me know.
>>
>> See testsuite/gdb.base/completion.exp
>
> Thanks, I ran all the tests and passed. I did not add a new test
> case though since I didn't check how to add a new convenience var
> and undefine it later from the test framework. (Should I try harder?)
>
>>
>> Daniel>  exp    :       VARIABLE
>> Daniel>                         {
>> Daniel> -                         write_dollar_variable (pstate, $1);
>> Daniel> +                         if (!parse_completion)
>> Daniel> +                           write_dollar_variable (pstate, $1);
>> Daniel>                         }
>>
>> I think this isn't correct.  I think it won't work if you try to
>> complete on a field name where the "LHS" has a convenience variable.
>> That is something like:
>>
>>    set $var = (struct x *) malloc (...)
>>    complete print $var.somethin
>
> You were right, that was broken.
>
>>
>> Instead I think you need a new production, like "exp : VARIABLE COMPLETE".
>
> I found another solution without touching the yacc file: adding a check
> inside write_dollar_variable
>
>>
>> Daniel> +  if (p != NULL && *p == '$')
>> Daniel> +    return complete_internalvar (p + 1);
>>
>> I'm not sure this is correct either, but offhand I don't know.
>> Should it not look at "word"?
>
> It actually works ("word" didn't have the $).
>
> However, I noticed that 'set' cmd autocomplete doesn't work for
> members of structures (neither with convenience vars nor regular
> symbols). I'll address that in the next patch once this gets approved
> and committed.
>
>>
>> Daniel> +  VEC (char_ptr) * ret = current_language->la_make_symbol_completion_list (
>> Daniel> +                                                      text, word,
>> Daniel> +                                                      TYPE_CODE_UNDEF);
>>
>> No space before "ret".
>> The line breaks look odd, I would break before the "=" and not after the "(".
>>
>> thanks,
>> Tom
>
> 2014-05-22  Daniel Gutson  <daniel.gutson@tallertechnologies.com>
>
> gdb/
>         * parse.c (write_dollar_variable): Do not create an internal
> var when completing.
>         * completer.c (expression_completer): Call complete_internalvar.
>         * symtab.c (make_symbol_completion_list): Call complete_internalvar.
  

Patch

diff --git a/gdb/completer.c b/gdb/completer.c
index 94f70a9..6c5cdf8 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -20,6 +20,7 @@ 
 #include "symtab.h"
 #include "gdbtypes.h"
 #include "expression.h"
+#include "value.h"
 #include "filenames.h"		/* For DOSish file names.  */
 #include "language.h"
 #include "gdb_assert.h"
@@ -446,8 +447,11 @@  expression_completer (struct cmd_list_element *ignore,
        p--)
     ;
 
-  /* Not ideal but it is what we used to do before...  */
-  return location_completer (ignore, p, word);
+  if (p != NULL && *p == '$')
+    return complete_internalvar (p + 1);
+  else
+    /* Not ideal but it is what we used to do before...  */
+    return location_completer (ignore, p, word);
 }
 
 /* Here are some useful test cases for completion.  FIXME: These
diff --git a/gdb/parse.c b/gdb/parse.c
index 105d0cd..218be13 100644
--- a/gdb/parse.c
+++ b/gdb/parse.c
@@ -692,10 +692,12 @@  write_dollar_variable (struct parser_state *ps, struct stoken str)
     }
 
   /* Any other names are assumed to be debugger internal variables.  */
-
-  write_exp_elt_opcode (ps, OP_INTERNALVAR);
-  write_exp_elt_intern (ps, create_internalvar (copy_name (str) + 1));
-  write_exp_elt_opcode (ps, OP_INTERNALVAR);
+  if (!parse_completion)
+    {
+      write_exp_elt_opcode (ps, OP_INTERNALVAR);
+      write_exp_elt_intern (ps, create_internalvar (copy_name (str) + 1));
+      write_exp_elt_opcode (ps, OP_INTERNALVAR);
+    }
   return;
 handle_last:
   write_exp_elt_opcode (ps, OP_LAST);
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 15ac3d1..52eb64c 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -4542,14 +4542,21 @@  default_make_symbol_completion_list (const char *text, const char *word,
 }
 
 /* Return a vector of all symbols (regardless of class) which begin by
-   matching TEXT.  If the answer is no symbols, then the return value
-   is NULL.  */
+   matching TEXT.  If the answer is no symbols, then check whether the
+   text is an internal var ($foo), if so, return what complete_internalvar
+   returns; otherwise the return value is NULL.  */
 
 VEC (char_ptr) *
 make_symbol_completion_list (const char *text, const char *word)
 {
-  return current_language->la_make_symbol_completion_list (text, word,
-							   TYPE_CODE_UNDEF);
+  VEC (char_ptr) *ret
+    = current_language->la_make_symbol_completion_list (text, word,
+                                                      TYPE_CODE_UNDEF);
+
+  if (ret == NULL && *text == '$')
+    ret = complete_internalvar (text + 1);
+
+  return ret;
 }
 
 /* Like make_symbol_completion_list, but only return STRUCT_DOMAIN