[PATCH] Add autocompletion for convenience vars in print and set

[PATCH] Add autocompletion for convenience vars in print and set

[Daniel Gutson]

[-- Attachment #1: Type: text/plain, Size: 569 bytes --]

Hi,

  this small patch adds autocompletion for convenience variables
for 'print' and 'set' commands. I guess other commands using the
same completers will be benefited as well.

I could not find any testsuite where to add tests for this; if there are,
please let me know.


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

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

[-- Attachment #2: completer.patch --]
[-- Type: text/x-patch, Size: 2241 bytes --]

diff --git a/gdb/c-exp.y b/gdb/c-exp.y
index 0191067..3ae969f 100644
--- a/gdb/c-exp.y
+++ b/gdb/c-exp.y
@@ -777,7 +777,8 @@ exp	:	variable
 
 exp	:	VARIABLE
 			{
-			  write_dollar_variable (pstate, $1);
+			  if (!parse_completion)
+			    write_dollar_variable (pstate, $1);
 			}
 	;
 
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/symtab.c b/gdb/symtab.c
index 15ac3d1..5ceaf46 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

Re: [PATCH] Add autocompletion for convenience vars in print and set

[Daniel Gutson]

I forgot to mention that if OK, please commit it for me since I don't have write
access.

Thanks,

   Daniel.

On Tue, May 20, 2014 at 12:51 PM, Daniel Gutson
<daniel.gutson@tallertechnologies.com> wrote:
> Hi,
>
>   this small patch adds autocompletion for convenience variables
> for 'print' and 'set' commands. I guess other commands using the
> same completers will be benefited as well.
>
> I could not find any testsuite where to add tests for this; if there are,
> please let me know.
>
>
> 2014-05-20  Daniel Gutson  <daniel.gutson@tallertechnologies.com>
>
> gdb/
>         * c-exp.y (exp): Do not create an internal var when completing.
>         * completer.c (expression_completer): Call complete_internalvar.
>         * symtab.c (make_symbol_completion_list): Call complete_internalvar.



-- 

Daniel F. Gutson
Chief Engineering Officer, SPD


San Lorenzo 47, 3rd Floor, Office 5

Córdoba, Argentina


Phone: +54 351 4217888 / +54 351 4218211

Skype: dgutson

Re: [PATCH] Add autocompletion for convenience vars in print and set

[Tom Tromey]

>>>>> "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

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

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

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"?

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

Re: [PATCH] Add autocompletion for convenience vars in print and set

[Daniel Gutson]

[-- Attachment #1: Type: text/plain, Size: 2464 bytes --]

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.

[-- Attachment #2: completer2.patch --]
[-- Type: text/x-patch, Size: 2620 bytes --]

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

Re: [PATCH] Add autocompletion for convenience vars in print and set

[Daniel Gutson]

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.



-- 

Daniel F. Gutson
Chief Engineering Officer, SPD


San Lorenzo 47, 3rd Floor, Office 5

Córdoba, Argentina


Phone: +54 351 4217888 / +54 351 4218211

Skype: dgutson

Re: [PATCH] Add autocompletion for convenience vars in print and set

[Andrew Burgess]

On 22/05/2014 4:17 PM, Daniel Gutson 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?)

I'm not a maintainer, but you'll probably be asked for some tests of 
this (really useful) feature.

I don't think you need to worry about removing any convenience 
variables you create, each .exp test file starts gdb afresh, 
and sometime (look for clean_restart) gdb is restarted within 
a single .exp file.

For this feature you can probably get away with just adding some 
new tests to the end of the completion.exp file, I've included an
example test in this mail, but you'll probably want some more.

thanks,
Andrew


diff --git a/gdb/testsuite/gdb.base/completion.exp b/gdb/testsuite/gdb.base/completion.exp
index 2608309..c028548 100644
--- a/gdb/testsuite/gdb.base/completion.exp
+++ b/gdb/testsuite/gdb.base/completion.exp
@@ -747,6 +747,27 @@ gdb_test_multiple "" "$test" {
     }
 }
 
+set test "complete '\$cv_'"
+gdb_test_no_output "set \$cv_aaa = 4" \
+    "Create convenience variable \$cv_aaa"
+send_gdb "p \$cv_"
+gdb_test_multiple "" "$test" {
+    -re "^p \\\$cv_" {
+	send_gdb "\t"
+	gdb_test_multiple "" "$test" {
+	    -re "aaa $" {
+		send_gdb "\n"
+		gdb_test_multiple "" "$test" {
+		    -re "\\\$\[0-9\]+ = 4.*$gdb_prompt $" {
+			pass "$test"
+		    }
+		}
+	    }
+	}
+    }
+}
+
+
 # Restore globals modified in this test...
 set timeout $oldtimeout1
 

Re: [PATCH] Add autocompletion for convenience vars in print and set

[Daniel Gutson]

On Tue, May 27, 2014 at 1:45 PM, Andrew Burgess <aburgess@broadcom.com> wrote:
> On 22/05/2014 4:17 PM, Daniel Gutson 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?)
>
> I'm not a maintainer, but you'll probably be asked for some tests of
> this (really useful) feature.
>
> I don't think you need to worry about removing any convenience
> variables you create, each .exp test file starts gdb afresh,
> and sometime (look for clean_restart) gdb is restarted within
> a single .exp file.
>
> For this feature you can probably get away with just adding some
> new tests to the end of the completion.exp file, I've included an
> example test in this mail, but you'll probably want some more.
>
> thanks,
> Andrew
>
>
> diff --git a/gdb/testsuite/gdb.base/completion.exp b/gdb/testsuite/gdb.base/completion.exp
> index 2608309..c028548 100644
> --- a/gdb/testsuite/gdb.base/completion.exp
> +++ b/gdb/testsuite/gdb.base/completion.exp
> @@ -747,6 +747,27 @@ gdb_test_multiple "" "$test" {
>      }
>  }
>
> +set test "complete '\$cv_'"
> +gdb_test_no_output "set \$cv_aaa = 4" \
> +    "Create convenience variable \$cv_aaa"
> +send_gdb "p \$cv_"
> +gdb_test_multiple "" "$test" {
> +    -re "^p \\\$cv_" {
> +       send_gdb "\t"
> +       gdb_test_multiple "" "$test" {
> +           -re "aaa $" {
> +               send_gdb "\n"
> +               gdb_test_multiple "" "$test" {
> +                   -re "\\\$\[0-9\]+ = 4.*$gdb_prompt $" {
> +                       pass "$test"
> +                   }
> +               }
> +           }
> +       }
> +    }
> +}
> +
> +
>  # Restore globals modified in this test...
>  set timeout $oldtimeout1

Thanks Andrew!



>
>
>
>



-- 

Daniel F. Gutson
Chief Engineering Officer, SPD


San Lorenzo 47, 3rd Floor, Office 5

Córdoba, Argentina


Phone: +54 351 4217888 / +54 351 4218211

Skype: dgutson

Re: [PATCH] Add autocompletion for convenience vars in print and set

[Daniel Gutson]

(sorry top posting)

Has this patch been applied?

Thanks,

   Daniel.

On Tue, May 27, 2014 at 5:24 PM, Daniel Gutson
<daniel.gutson@tallertechnologies.com> wrote:
> On Tue, May 27, 2014 at 1:45 PM, Andrew Burgess <aburgess@broadcom.com> wrote:
>> On 22/05/2014 4:17 PM, Daniel Gutson 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?)
>>
>> I'm not a maintainer, but you'll probably be asked for some tests of
>> this (really useful) feature.
>>
>> I don't think you need to worry about removing any convenience
>> variables you create, each .exp test file starts gdb afresh,
>> and sometime (look for clean_restart) gdb is restarted within
>> a single .exp file.
>>
>> For this feature you can probably get away with just adding some
>> new tests to the end of the completion.exp file, I've included an
>> example test in this mail, but you'll probably want some more.
>>
>> thanks,
>> Andrew
>>
>>
>> diff --git a/gdb/testsuite/gdb.base/completion.exp b/gdb/testsuite/gdb.base/completion.exp
>> index 2608309..c028548 100644
>> --- a/gdb/testsuite/gdb.base/completion.exp
>> +++ b/gdb/testsuite/gdb.base/completion.exp
>> @@ -747,6 +747,27 @@ gdb_test_multiple "" "$test" {
>>      }
>>  }
>>
>> +set test "complete '\$cv_'"
>> +gdb_test_no_output "set \$cv_aaa = 4" \
>> +    "Create convenience variable \$cv_aaa"
>> +send_gdb "p \$cv_"
>> +gdb_test_multiple "" "$test" {
>> +    -re "^p \\\$cv_" {
>> +       send_gdb "\t"
>> +       gdb_test_multiple "" "$test" {
>> +           -re "aaa $" {
>> +               send_gdb "\n"
>> +               gdb_test_multiple "" "$test" {
>> +                   -re "\\\$\[0-9\]+ = 4.*$gdb_prompt $" {
>> +                       pass "$test"
>> +                   }
>> +               }
>> +           }
>> +       }
>> +    }
>> +}
>> +
>> +
>>  # Restore globals modified in this test...
>>  set timeout $oldtimeout1
>
> Thanks Andrew!
>
>
>
>>
>>
>>
>>
>
>
>
> --
>
> Daniel F. Gutson
> Chief Engineering Officer, SPD
>
>
> San Lorenzo 47, 3rd Floor, Office 5
>
> Córdoba, Argentina
>
>
> Phone: +54 351 4217888 / +54 351 4218211
>
> Skype: dgutson



-- 

Daniel F. Gutson
Chief Engineering Officer, SPD

San Lorenzo 47, 3rd Floor, Office 5
Córdoba, Argentina

Phone:   +54 351 4217888 / +54 351 4218211
Skype:    dgutson
LinkedIn: http://ar.linkedin.com/in/danielgutson