Message ID | 1408515134-31165-1-git-send-email-gabriel@krisman.be |
---|---|
State | New |
Headers | show |
Hi Gabriel, Thanks for the patch! On 08/20/2014 07:12 AM, Gabriel Krisman Bertazi wrote: > c = lookup_cmd (&comname, cmdlist, "", 0, 1); > - /* c->user_commands would be NULL if it's a python/scheme command. */ > - if (c->class != class_user || !c->user_commands) > - error (_("Not a user command.")); > + if (c->class != class_user) > + error (_("Not a user command.")); Doesn't this mean this reverts part of 7d74f2446, and thus now we'd show python/scheme commands? IIUC 7d74f2446, it looks like gdb.python/py-cmd.exp is missing a test that makes sure "show user" doesn't list the user-defined python command. (hmm, this "show user" vs "help user-defined" difference isn't very intuitive) > --- a/gdb/testsuite/gdb.base/default.exp > +++ b/gdb/testsuite/gdb.base/default.exp > @@ -693,7 +693,7 @@ gdb_test "show prompt" "Gdb's prompt is \"$gdb_prompt \".*" "show prompt" > #test show radix > gdb_test "show radix" "Input and output radices set to decimal 10, hex a, octal 12." "show radix" > #test show user > -gdb_test_no_output "show user" "show user" > +gdb_test "show user" "User command \"user-defined\".*" "show user" > #test show values What is this printing now ? Thanks, Pedro Alves
On Wednesday, August 20 2014, Gabriel Krisman Bertazi wrote: > diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c > index b415267..4c42ff1 100644 > --- a/gdb/cli/cli-cmds.c > +++ b/gdb/cli/cli-cmds.c > @@ -1245,9 +1245,8 @@ show_user (char *args, int from_tty) > const char *comname = args; > > c = lookup_cmd (&comname, cmdlist, "", 0, 1); > - /* c->user_commands would be NULL if it's a python/scheme command. */ > - if (c->class != class_user || !c->user_commands) > - error (_("Not a user command.")); > + if (c->class != class_user) > + error (_("Not a user command.")); ^^^^^^^^ Missing TAB. Minor-tiny nit. Cheers,
Pedro Alves <palves@redhat.com> writes: >> c = lookup_cmd (&comname, cmdlist, "", 0, 1); >> - /* c->user_commands would be NULL if it's a python/scheme command. */ >> - if (c->class != class_user || !c->user_commands) >> - error (_("Not a user command.")); >> + if (c->class != class_user) >> + error (_("Not a user command.")); > > Doesn't this mean this reverts part of 7d74f2446, and thus now we'd > show python/scheme commands? > > IIUC 7d74f2446, it looks like gdb.python/py-cmd.exp is missing > a test that makes sure "show user" doesn't list the user-defined python command. > > (hmm, this "show user" vs "help user-defined" difference isn't very > intuitive) Hello Pedro, Thanks for your review. Indeed, this means we'd start to show python/scheme commands, but I wasn't aware of commit 7d74f2446. I don't see that as a problem, because python/scheme are also user commands (or, at least, are part of class_user class). Considering that "help user-defined" also shows python/scheme commands, I believe it would be more practical to just list them on "show user" (should we update the doc string, as well). Either way, the "Not a user command" error message is quite misleading for python/scheme commands, though. If you disagree, I'd be happy to update the patch to rely on another condition to avoid printing python/scheme commands. c->function.cfunc/sfunc == NULL seems to work pretty well to avoid printing python commands. What do you think? > What is this printing now ? Now, it prints: (gdb) show user User command "emptycmd": User command "cmd1": print "Hello" User command "python-hello": User command "user-defined": Using the verification for cfunc/sfunc above, also avoids printing the "user-defined" line. Thanks,
Hi Gabriel, On 08/23/2014 03:27 AM, Gabriel Krisman Bertazi wrote: > Pedro Alves <palves@redhat.com> writes: > >>> c = lookup_cmd (&comname, cmdlist, "", 0, 1); >>> - /* c->user_commands would be NULL if it's a python/scheme command. */ >>> - if (c->class != class_user || !c->user_commands) >>> - error (_("Not a user command.")); >>> + if (c->class != class_user) >>> + error (_("Not a user command.")); >> >> Doesn't this mean this reverts part of 7d74f2446, and thus now we'd >> show python/scheme commands? >> >> IIUC 7d74f2446, it looks like gdb.python/py-cmd.exp is missing >> a test that makes sure "show user" doesn't list the user-defined python command. >> >> (hmm, this "show user" vs "help user-defined" difference isn't very >> intuitive) > > Hello Pedro, > > Thanks for your review. > > Indeed, this means we'd start to show python/scheme commands, but I > wasn't aware of commit 7d74f2446. I found it by noticing you were removing the "c->user_commands would be NULL if it's a python/scheme command" comment, and using "git blame" to find out why that comment was there in the first place. > > I don't see that as a problem, because python/scheme are also user > commands (or, at least, are part of class_user class). Considering that > "help user-defined" also shows python/scheme commands, I believe it > would be more practical to just list them on "show user" (should we > update the doc string, as well). I think we'll need to track down the original discussion, and ask whoever was involved. Otherwise, we'd just be going in circles. :-) E.g., that commit changed the online help and the manual to try to make that clear. (gdb) help show user Show definitions of non-python/scheme user defined commands. Argument is the name of the user defined command. With no argument, show definitions of all user defined commands. > Either way, the "Not a user command" error message is quite misleading > for python/scheme commands, though. > > If you disagree, I'd be happy to update the patch to rely on another > condition to avoid printing python/scheme commands. > c->function.cfunc/sfunc == NULL seems to work pretty well to avoid > printing python commands. > > What do you think? > >> What is this printing now ? > > Now, it prints: > > (gdb) show user > User command "emptycmd": > User command "cmd1": > print "Hello" > > User command "python-hello": > User command "user-defined": > The question was actually in context of the gdb.base/default.exp change: > --- a/gdb/testsuite/gdb.base/default.exp > +++ b/gdb/testsuite/gdb.base/default.exp > @@ -693,7 +693,7 @@ gdb_test "show prompt" "Gdb's prompt is \"$gdb_prompt \".*" "show prompt" > #test show radix > gdb_test "show radix" "Input and output radices set to decimal 10, hex a, octal 12." "show radix" > #test show user > -gdb_test_no_output "show user" "show user" > +gdb_test "show user" "User command \"user-defined\".*" "show user" What are we showing now by default? > Using the verification for cfunc/sfunc above, also avoids > printing the "user-defined" line. Hmm, but what is that "user-defined" command ? Thanks, Pedro Alves
diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c index b415267..4c42ff1 100644 --- a/gdb/cli/cli-cmds.c +++ b/gdb/cli/cli-cmds.c @@ -1245,9 +1245,8 @@ show_user (char *args, int from_tty) const char *comname = args; c = lookup_cmd (&comname, cmdlist, "", 0, 1); - /* c->user_commands would be NULL if it's a python/scheme command. */ - if (c->class != class_user || !c->user_commands) - error (_("Not a user command.")); + if (c->class != class_user) + error (_("Not a user command.")); show_user_1 (c, "", args, gdb_stdout); } else diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c index 0f0a97e..37cb82a 100644 --- a/gdb/cli/cli-script.c +++ b/gdb/cli/cli-script.c @@ -1717,10 +1717,10 @@ show_user_1 (struct cmd_list_element *c, const char *prefix, const char *name, } cmdlines = c->user_commands; - if (!cmdlines) - return; fprintf_filtered (stream, "User command \"%s%s\":\n", prefix, name); + if (!cmdlines) + return; print_command_lines (current_uiout, cmdlines, 1); fputs_filtered ("\n", stream); } diff --git a/gdb/testsuite/gdb.base/commands.exp b/gdb/testsuite/gdb.base/commands.exp index 7363420..b2d1c76 100644 --- a/gdb/testsuite/gdb.base/commands.exp +++ b/gdb/testsuite/gdb.base/commands.exp @@ -243,6 +243,28 @@ proc user_defined_command_test {} { gdb_test "show user mycommand" \ " while \\\$arg0.*set.* if \\\(\\\$arg0.*p/x.* else\[^\n\].*p/x.* end\[^\n\].* end\[^\n\].*" \ "display user command in user_defined_command_test" + + # Create and test an user-defined command with an empty body. + gdb_test_multiple "define myemptycommand" \ + "define myemptycommand in user_defined_command_test" { + -re "End with" { + pass "define myemptycommand in user_defined_command_test" + } + } + gdb_test "end" \ + "" \ + "End definition of user-defined command with empty body." + + gdb_test_no_output "myemptycommand" \ + "execute user-defined empty command in user_defined_command_test" + + gdb_test "show user" \ + "User command \"myemptycommand.*" \ + "display empty command in command list in user_defined_command_test" + + gdb_test "show user myemptycommand" \ + "User command \"myemptycommand.*" \ + "display user-defined emtpy command in user_defined_command_test" } proc watchpoint_command_test {} { diff --git a/gdb/testsuite/gdb.base/default.exp b/gdb/testsuite/gdb.base/default.exp index 6674df3..c9fbedd 100644 --- a/gdb/testsuite/gdb.base/default.exp +++ b/gdb/testsuite/gdb.base/default.exp @@ -693,7 +693,7 @@ gdb_test "show prompt" "Gdb's prompt is \"$gdb_prompt \".*" "show prompt" #test show radix gdb_test "show radix" "Input and output radices set to decimal 10, hex a, octal 12." "show radix" #test show user -gdb_test_no_output "show user" "show user" +gdb_test "show user" "User command \"user-defined\".*" "show user" #test show values gdb_test_no_output "show values" "show values" #test show verbose diff --git a/gdb/testsuite/gdb.base/setshow.exp b/gdb/testsuite/gdb.base/setshow.exp index 639ca72..302039f 100644 --- a/gdb/testsuite/gdb.base/setshow.exp +++ b/gdb/testsuite/gdb.base/setshow.exp @@ -275,7 +275,7 @@ gdb_test_no_output "set write on" "set write on" # This is only supported on targets which use exec.o. gdb_test "show write" "Writing into executable and core files is on..*" "show write (on)" #test show user -gdb_test_no_output "show user" "show user" +gdb_test "show user" "User command \"user-defined\".*" "show user" #test set verbose on gdb_test_no_output "set verbose on" "set verbose on" #test show verbose on