* RFA: fix two field name completion bugs
@ 2008-06-08 18:41 Tom Tromey
2008-06-09 19:15 ` Daniel Jacobowitz
0 siblings, 1 reply; 5+ messages in thread
From: Tom Tromey @ 2008-06-08 18:41 UTC (permalink / raw)
To: gdb-patches
After committing the field name completion patch I found a couple
bugs.
One is a huge goof -- I must not have checked the '->' case, since it
didn't work. How embarrassing.
The other is that spaces in the expression could throw off the
completer. The fix is to make sure that the entire expression is
passed to expression_completer, then duplicate some logic there in the
case where location_completer is called.
I added test cases for both of these.
Ok?
Tom
ChangeLog:
2008-06-08 Tom Tromey <tromey@redhat.com>
* completer.c (complete_line): Don't special-case
expression_completer.
(expression_completer): Only pass last word to
location_completer.
* c-exp.y (yylex): Check 'token', not 'operator'.
testsuite/ChangeLog:
2008-06-08 Tom Tromey <tromey@redhat.com>
* gdb.base/completion.exp: New tests for field name completion
with spaces, and field name completion with '->'.
Index: c-exp.y
===================================================================
RCS file: /cvs/src/src/gdb/c-exp.y,v
retrieving revision 1.44
diff -u -r1.44 c-exp.y
--- c-exp.y 6 Jun 2008 20:58:08 -0000 1.44
+++ c-exp.y 8 Jun 2008 18:38:48 -0000
@@ -1433,7 +1433,7 @@
{
lexptr += 2;
yylval.opcode = tokentab2[i].opcode;
- if (in_parse_field && tokentab2[i].opcode == ARROW)
+ if (in_parse_field && tokentab2[i].token == ARROW)
last_was_structop = 1;
return tokentab2[i].token;
}
Index: completer.c
===================================================================
RCS file: /cvs/src/src/gdb/completer.c,v
retrieving revision 1.25
diff -u -r1.25 completer.c
--- completer.c 6 Jun 2008 20:58:08 -0000 1.25
+++ completer.c 8 Jun 2008 18:38:48 -0000
@@ -387,7 +387,7 @@
expression_completer (char *text, char *word)
{
struct type *type;
- char *fieldname;
+ char *fieldname, *p;
/* Perform a tentative parse of the expression, to see whether a
field completion is required. */
@@ -418,8 +418,15 @@
}
}
+ /* Commands which complete on locations want to see the entire
+ argument. */
+ for (p = word;
+ p > text && p[-1] != ' ' && p[-1] != '\t';
+ p--)
+ ;
+
/* Not ideal but it is what we used to do before... */
- return location_completer (text, word);
+ return location_completer (p, word);
}
/* Complete on command names. Used by "help". */
@@ -604,8 +611,7 @@
rl_completer_word_break_characters =
gdb_completer_file_name_break_characters;
}
- else if (c->completer == location_completer
- || c->completer == expression_completer)
+ else if (c->completer == location_completer)
{
/* Commands which complete on locations want to
see the entire argument. */
@@ -673,8 +679,7 @@
rl_completer_word_break_characters =
gdb_completer_file_name_break_characters;
}
- else if (c->completer == location_completer
- || c->completer == expression_completer)
+ else if (c->completer == location_completer)
{
for (p = word;
p > tmp_command
Index: testsuite/gdb.base/completion.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/completion.exp,v
retrieving revision 1.30
diff -u -r1.30 completion.exp
--- testsuite/gdb.base/completion.exp 6 Jun 2008 20:58:08 -0000 1.30
+++ testsuite/gdb.base/completion.exp 8 Jun 2008 18:38:50 -0000
@@ -654,6 +654,40 @@
timeout { fail "(timeout) complete 'p values\[0\].a' 2" }
}
+send_gdb "p values\[0\] . a\t"
+sleep 3
+gdb_expect {
+ -re "^p values.0. . a_field $"\
+ { send_gdb "\n"
+ sleep 1
+ gdb_expect {
+ -re "^.* = 0.*$gdb_prompt $"\
+ { pass "complete 'p values\[0\] . a'"}
+ -re ".*$gdb_prompt $" { fail "complete 'p values\[0\] . a'"}
+ timeout {fail "(timeout) complete 'p values\[0\] . a'"}
+ }
+ }
+ -re ".*$gdb_prompt $" { fail "complete 'p values\[0\] . a'" }
+ timeout { fail "(timeout) complete 'p values\[0\] . a' 2" }
+ }
+
+send_gdb "p &values\[0\] -> a\t"
+sleep 3
+gdb_expect {
+ -re "^p &values.0. -> a_field $"\
+ { send_gdb "\n"
+ sleep 1
+ gdb_expect {
+ -re "^.* = .*0x\[0-9a-fA-F\]*.*$gdb_prompt $"\
+ { pass "complete 'p &values\[0\] -> a'"}
+ -re ".*$gdb_prompt $" { fail "complete 'p &values\[0\] -> a'"}
+ timeout {fail "(timeout) complete 'p &values\[0\] -> a'"}
+ }
+ }
+ -re ".*$gdb_prompt $" { fail "complete 'p &values\[0\] -> a'" }
+ timeout { fail "(timeout) complete 'p &values\[0\] -> a' 2" }
+ }
+
# The following tests used to simply try to complete `${objdir}/file',
# and so on. The problem is that ${objdir} can be very long; the
# completed filename may be more than eighty characters wide. When
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: RFA: fix two field name completion bugs
2008-06-08 18:41 RFA: fix two field name completion bugs Tom Tromey
@ 2008-06-09 19:15 ` Daniel Jacobowitz
2008-06-09 19:26 ` Tom Tromey
0 siblings, 1 reply; 5+ messages in thread
From: Daniel Jacobowitz @ 2008-06-09 19:15 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
On Sun, Jun 08, 2008 at 12:41:28PM -0600, Tom Tromey wrote:
> ChangeLog:
> 2008-06-08 Tom Tromey <tromey@redhat.com>
>
> * completer.c (complete_line): Don't special-case
> expression_completer.
> (expression_completer): Only pass last word to
> location_completer.
> * c-exp.y (yylex): Check 'token', not 'operator'.
This part is OK.
> +send_gdb "p values\[0\] . a\t"
> +sleep 3
> +gdb_expect {
I know there's other tests in completion.exp that do this but are the
sleeps really necessary? Anywhere that we sleep, the testsuite
becomes timing sensitive - not to mention slow.
--
Daniel Jacobowitz
CodeSourcery
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: RFA: fix two field name completion bugs
2008-06-09 19:15 ` Daniel Jacobowitz
@ 2008-06-09 19:26 ` Tom Tromey
2008-06-09 22:40 ` Daniel Jacobowitz
0 siblings, 1 reply; 5+ messages in thread
From: Tom Tromey @ 2008-06-09 19:26 UTC (permalink / raw)
To: gdb-patches
>>>>> "Daniel" == Daniel Jacobowitz <drow@false.org> writes:
>> +send_gdb "p values\[0\] . a\t"
>> +sleep 3
>> +gdb_expect {
Daniel> I know there's other tests in completion.exp that do this but are the
Daniel> sleeps really necessary? Anywhere that we sleep, the testsuite
Daniel> becomes timing sensitive - not to mention slow.
I just copied and pasted this code. FWIW completion.exp seems ripe
for refactoring into some procs ... but I don't plan to do that.
I looked at cvs annotate but it did not tell me much. Many of the
sleeps in this file date to the initial revision.
I removed the new sleeps here, and it still worked. This doesn't tell
us anything directly -- it could still fail on a slower machine. But
if nobody complains after a while I suppose we can assume it was ok :)
I can commit that version if you prefer. Just let me know. I've
appended the new completion.exp patch.
Tom
Index: testsuite/gdb.base/completion.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/completion.exp,v
retrieving revision 1.30
diff -u -r1.30 completion.exp
--- testsuite/gdb.base/completion.exp 6 Jun 2008 20:58:08 -0000 1.30
+++ testsuite/gdb.base/completion.exp 9 Jun 2008 19:11:42 -0000
@@ -654,6 +654,36 @@
timeout { fail "(timeout) complete 'p values\[0\].a' 2" }
}
+send_gdb "p values\[0\] . a\t"
+gdb_expect {
+ -re "^p values.0. . a_field $"\
+ { send_gdb "\n"
+ gdb_expect {
+ -re "^.* = 0.*$gdb_prompt $"\
+ { pass "complete 'p values\[0\] . a'"}
+ -re ".*$gdb_prompt $" { fail "complete 'p values\[0\] . a'"}
+ timeout {fail "(timeout) complete 'p values\[0\] . a'"}
+ }
+ }
+ -re ".*$gdb_prompt $" { fail "complete 'p values\[0\] . a'" }
+ timeout { fail "(timeout) complete 'p values\[0\] . a' 2" }
+ }
+
+send_gdb "p &values\[0\] -> a\t"
+gdb_expect {
+ -re "^p &values.0. -> a_field $"\
+ { send_gdb "\n"
+ gdb_expect {
+ -re "^.* = .*0x\[0-9a-fA-F\]*.*$gdb_prompt $"\
+ { pass "complete 'p &values\[0\] -> a'"}
+ -re ".*$gdb_prompt $" { fail "complete 'p &values\[0\] -> a'"}
+ timeout {fail "(timeout) complete 'p &values\[0\] -> a'"}
+ }
+ }
+ -re ".*$gdb_prompt $" { fail "complete 'p &values\[0\] -> a'" }
+ timeout { fail "(timeout) complete 'p &values\[0\] -> a' 2" }
+ }
+
# The following tests used to simply try to complete `${objdir}/file',
# and so on. The problem is that ${objdir} can be very long; the
# completed filename may be more than eighty characters wide. When
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: RFA: fix two field name completion bugs
2008-06-09 19:26 ` Tom Tromey
@ 2008-06-09 22:40 ` Daniel Jacobowitz
2008-06-09 22:50 ` Tom Tromey
0 siblings, 1 reply; 5+ messages in thread
From: Daniel Jacobowitz @ 2008-06-09 22:40 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
On Mon, Jun 09, 2008 at 01:11:59PM -0600, Tom Tromey wrote:
> I can commit that version if you prefer. Just let me know. I've
> appended the new completion.exp patch.
Please do. If it survives, perhaps we should try removing them all
:-)
--
Daniel Jacobowitz
CodeSourcery
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: RFA: fix two field name completion bugs
2008-06-09 22:40 ` Daniel Jacobowitz
@ 2008-06-09 22:50 ` Tom Tromey
0 siblings, 0 replies; 5+ messages in thread
From: Tom Tromey @ 2008-06-09 22:50 UTC (permalink / raw)
To: gdb-patches
>>>>> "Daniel" == Daniel Jacobowitz <drow@false.org> writes:
Daniel> On Mon, Jun 09, 2008 at 01:11:59PM -0600, Tom Tromey wrote:
>> I can commit that version if you prefer. Just let me know. I've
>> appended the new completion.exp patch.
Daniel> Please do. If it survives, perhaps we should try removing them all
Daniel> :-)
Ok, I've checked it in. Thanks for the review.
Tom
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2008-06-09 19:26 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-06-08 18:41 RFA: fix two field name completion bugs Tom Tromey
2008-06-09 19:15 ` Daniel Jacobowitz
2008-06-09 19:26 ` Tom Tromey
2008-06-09 22:40 ` Daniel Jacobowitz
2008-06-09 22:50 ` Tom Tromey
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox