diff mbox

[v4,2/2] Provide completer for "info registers"

Message ID 877fxwnbd6.fsf@br87z6lw.de.ibm.com
State New
Headers show

Commit Message

Andreas Arnez Dec. 12, 2014, 3:20 p.m. UTC
On Fri, Dec 12 2014, Pedro Alves wrote:

> On 12/11/2014 11:49 AM, Andreas Arnez wrote:
>
>> +set regs_output [capture_command_output "mt print registers" \
>> +		     ".*Name.*Nr.*Rel.*Offset.*Size.*Type.\[^\n\]*\n"]
>> +append regs_output "\n"
>> +append regs_output [capture_command_output "mt print reggroups" \
>> +			".*Group.*Type\[^\n]*\n"]
>> +set all_regs {}
>> +foreach {-> reg} [regexp -all -inline -line {^\s+(\w+\S*)} $regs_output] {
>
> This "->" here confused me a little.  AFAIK, $- is a more common "don't care"
> variable (and what foreach's documentation suggests).  Any reason to
> pick -> instead?

I think I've borrowed this from an example in the regexp's
documentation:

  regexp {\mfoo(?!bar\M)(\w*)} $string -> restOfWord

Admittedly, in the foreach case this does not look as nicely.  So I will
change it to "-", as suggested.

> Also, why do we need the "\S*" ?  I'd assume {^\s+(\w+)} works just as well.

OK, all existing register and reggroup names consist wholly of
alphanumeric and underscore characters.  So I will change the pattern as
suggested.

>
>> +    lappend all_regs $reg
>> +}
>> +
>> +set regs_output [capture_command_output "mt print user-registers" \
>> +		     ".*Nr.*Name\[^\n]*\n"]
>> +foreach {-> reg} [regexp -all -inline -line {\d+\s+(\w+\S*)} $regs_output] {
>
> Likewise.

Yes.  And looking at it again, I think that this pattern should be more
consistent with the one above and match the whole beginning of a line.
Thus I will rephrase it to {^\s+\d+\s+(\w+)}.  In this way the command
can add more columns in the future without having to adjust the pattern
here.

> Otherwise this looks good to me.

Thanks.  Will push this then with changes below.

--

Comments

Pedro Alves Dec. 12, 2014, 3:30 p.m. UTC | #1
On 12/12/2014 03:20 PM, Andreas Arnez wrote:

> Thanks.  Will push this then with changes below.

Great, that looks good.

Thanks again for doing this.
Doug Evans Dec. 12, 2014, 5:45 p.m. UTC | #2
On Fri, Dec 12, 2014 at 7:20 AM, Andreas Arnez <arnez@linux.vnet.ibm.com> wrote:
> On Fri, Dec 12 2014, Pedro Alves wrote:
>
>> On 12/11/2014 11:49 AM, Andreas Arnez wrote:
>>
>>> +set regs_output [capture_command_output "mt print registers" \
>>> +                 ".*Name.*Nr.*Rel.*Offset.*Size.*Type.\[^\n\]*\n"]
>>> +append regs_output "\n"
>>> +append regs_output [capture_command_output "mt print reggroups" \
>>> +                    ".*Group.*Type\[^\n]*\n"]
>>> +set all_regs {}
>>> +foreach {-> reg} [regexp -all -inline -line {^\s+(\w+\S*)} $regs_output] {
>>
>> This "->" here confused me a little.  AFAIK, $- is a more common "don't care"
>> variable (and what foreach's documentation suggests).  Any reason to
>> pick -> instead?
>
> I think I've borrowed this from an example in the regexp's
> documentation:
>
>   regexp {\mfoo(?!bar\M)(\w*)} $string -> restOfWord
>
> Admittedly, in the foreach case this does not look as nicely.  So I will
> change it to "-", as suggested.
>
>> Also, why do we need the "\S*" ?  I'd assume {^\s+(\w+)} works just as well.
>
> OK, all existing register and reggroup names consist wholly of
> alphanumeric and underscore characters.  So I will change the pattern as
> suggested.
>
>>
>>> +    lappend all_regs $reg
>>> +}
>>> +
>>> +set regs_output [capture_command_output "mt print user-registers" \
>>> +                 ".*Nr.*Name\[^\n]*\n"]
>>> +foreach {-> reg} [regexp -all -inline -line {\d+\s+(\w+\S*)} $regs_output] {
>>
>> Likewise.
>
> Yes.  And looking at it again, I think that this pattern should be more
> consistent with the one above and match the whole beginning of a line.
> Thus I will rephrase it to {^\s+\d+\s+(\w+)}.  In this way the command
> can add more columns in the future without having to adjust the pattern
> here.
>
>> Otherwise this looks good to me.
>
> Thanks.  Will push this then with changes below.
>
> --
>
> --- a/gdb/testsuite/gdb.base/completion.exp
> +++ b/gdb/testsuite/gdb.base/completion.exp
> @@ -146,13 +146,13 @@ append regs_output "\n"
>  append regs_output [capture_command_output "mt print reggroups" \
>                         ".*Group.*Type\[^\n]*\n"]
>  set all_regs {}
> -foreach {-> reg} [regexp -all -inline -line {^\s+(\w+\S*)} $regs_output] {
> +foreach {- reg} [regexp -all -inline -line {^\s+(\w+)} $regs_output] {
>      lappend all_regs $reg
>  }
>
>  set regs_output [capture_command_output "mt print user-registers" \
>                      ".*Nr.*Name\[^\n]*\n"]
> -foreach {-> reg} [regexp -all -inline -line {\d+\s+(\w+\S*)} $regs_output] {
> +foreach {- reg} [regexp -all -inline -line {^\s+\d+\s+(\w+)} $regs_output] {
>      lappend all_regs $reg
>  }
>


Hi.
I'm seeing the following new failure on amd64-linux:

(gdb) complete info registers ^M
info registers ah^M
info registers al^M
...
info registers ymm9^M
info registers ymm9h^M
(gdb) FAIL: gdb.base/completion.exp: complete 'info registers '
diff mbox

Patch

--- a/gdb/testsuite/gdb.base/completion.exp
+++ b/gdb/testsuite/gdb.base/completion.exp
@@ -146,13 +146,13 @@  append regs_output "\n"
 append regs_output [capture_command_output "mt print reggroups" \
 			".*Group.*Type\[^\n]*\n"]
 set all_regs {}
-foreach {-> reg} [regexp -all -inline -line {^\s+(\w+\S*)} $regs_output] {
+foreach {- reg} [regexp -all -inline -line {^\s+(\w+)} $regs_output] {
     lappend all_regs $reg
 }
 
 set regs_output [capture_command_output "mt print user-registers" \
 		     ".*Nr.*Name\[^\n]*\n"]
-foreach {-> reg} [regexp -all -inline -line {\d+\s+(\w+\S*)} $regs_output] {
+foreach {- reg} [regexp -all -inline -line {^\s+\d+\s+(\w+)} $regs_output] {
     lappend all_regs $reg
 }