[RFAv3,0/4] More flexible user-defined commands prefixing and naming.

Message ID 20191128200901.27511-1-philippe.waroquiers@skynet.be
State New, archived
Headers

Commit Message

Philippe Waroquiers Nov. 28, 2019, 8:08 p.m. UTC
  More flexible user-defined commands prefixing and naming.

This is the third version of the patch series.
The only modification is to ensure that GDB does not ask the question:
   Keeping subcommands of prefix command "xxxx".
   Redefine command "xxxx"? (y or n)
if xxxx was only defined as a prefix.
Test and doc updated accordingly.

To ease the review, the diff between v2 and v3 is given below.

This patch series improves the way a user can define user commands
using CLI sequences of commands.

Currently, a user can define a command after an existing command prefix
e.g.  define target mycommand
but cannot define a prefix to define command such as:
      define mytargetprefix mycommand.

This patch series adds the command 'define-prefix to allow
user commands to be prefix commands.

Also, this patch series adds . as an allowed character for user
defined commands.
This can e.g. be used to define a set of Valgrind specific user command
corresponding to the Valgrind monitor commands (such as check_memory,
v.info, v.set, ...).

This then allows to use GDB completion and expression evaluation
for sending monitor commands e.g. to Valgrind;

For example, for the Valgrind monitor 'check_memory' command:
  check_memory [addressable|defined] <addr> [<len>]
        check that <len> (or 1) bytes at <addr> have the given accessibility
            and outputs a description of <addr>
we can now define some new GDB commands such as:
(gdb) define-prefix Vmonitor
(gdb) define-prefix Vmonitor check_memory
(gdb) define Vmonitor check_memory addressable
eval "monitor check_memory addressable %#lx %d", $arg0, $arg1
end
(gdb) define Vmonitor check_memory defined
eval "monitor check_memory defined %#lx %d", $arg0, $arg1
end
(gdb)

Compared to the 'raw' monitor command, the new GDB commands provide completion
and evaluation of expressions.

Second version of the patch series was:
Compared to the first version, this handles the comments of Simon.
2 changes worth mentionning here:
  * The command name was changed to define-prefix.
  * In case the user redefines a command that is also a prefix command,
    the confirmation message is now:
       (gdb) define-prefix xxxx
       (gdb) define xxxx
       Keeping subcommands of prefix command "xxxx".
       Redefine command "xxxx"? (y or n) 
    This reminds the user that the subcommands of the prefix command xxxx
    are kept even when xxxx is redefined.
  

Comments

Simon Marchi Nov. 30, 2019, 3:36 a.m. UTC | #1
On 2019-11-28 3:08 p.m., Philippe Waroquiers wrote:
> More flexible user-defined commands prefixing and naming.
> 
> This is the third version of the patch series.
> The only modification is to ensure that GDB does not ask the question:
>    Keeping subcommands of prefix command "xxxx".
>    Redefine command "xxxx"? (y or n)
> if xxxx was only defined as a prefix.
> Test and doc updated accordingly.
> 
> To ease the review, the diff between v2 and v3 is given below.
> 
> This patch series improves the way a user can define user commands
> using CLI sequences of commands.
> 
> Currently, a user can define a command after an existing command prefix
> e.g.  define target mycommand
> but cannot define a prefix to define command such as:
>       define mytargetprefix mycommand.
> 
> This patch series adds the command 'define-prefix to allow
> user commands to be prefix commands.
> 
> Also, this patch series adds . as an allowed character for user
> defined commands.
> This can e.g. be used to define a set of Valgrind specific user command
> corresponding to the Valgrind monitor commands (such as check_memory,
> v.info, v.set, ...).
> 
> This then allows to use GDB completion and expression evaluation
> for sending monitor commands e.g. to Valgrind;
> 
> For example, for the Valgrind monitor 'check_memory' command:
>   check_memory [addressable|defined] <addr> [<len>]
>         check that <len> (or 1) bytes at <addr> have the given accessibility
>             and outputs a description of <addr>
> we can now define some new GDB commands such as:
> (gdb) define-prefix Vmonitor
> (gdb) define-prefix Vmonitor check_memory
> (gdb) define Vmonitor check_memory addressable
> eval "monitor check_memory addressable %#lx %d", $arg0, $arg1
> end
> (gdb) define Vmonitor check_memory defined
> eval "monitor check_memory defined %#lx %d", $arg0, $arg1
> end
> (gdb)
> 
> Compared to the 'raw' monitor command, the new GDB commands provide completion
> and evaluation of expressions.
> 
> Second version of the patch series was:
> Compared to the first version, this handles the comments of Simon.
> 2 changes worth mentionning here:
>   * The command name was changed to define-prefix.
>   * In case the user redefines a command that is also a prefix command,
>     the confirmation message is now:
>        (gdb) define-prefix xxxx
>        (gdb) define xxxx
>        Keeping subcommands of prefix command "xxxx".
>        Redefine command "xxxx"? (y or n) 
>     This reminds the user that the subcommands of the prefix command xxxx
>     are kept even when xxxx is redefined.
> 
> diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c
> index 79c499a4c2..6900f5c9ee 100644
> --- a/gdb/cli/cli-script.c
> +++ b/gdb/cli/cli-script.c
> @@ -1397,11 +1397,13 @@ do_define_command (const char *comname, int from_tty,
>  
>        if (c->theclass == class_user || c->theclass == class_alias)
>  	{
> -	  /* if C is a prefix command, tell the user its subcommands will
> -	     be kept, and ask if ok to redefine the command.  */
> +	  /* if C is a prefix command that was previously defined,
> +	     tell the user its subcommands will be kept, and ask
> +	     if ok to redefine the command.  */
>  	  if (c->prefixlist != nullptr)
> -	    q = query (_("Keeping subcommands of prefix command \"%s\".\n"
> -			 "Redefine command \"%s\"? "), c->name, c->name);
> +	    q = c->user_commands.get () == nullptr
> +	      || query (_("Keeping subcommands of prefix command \"%s\".\n"
> +			  "Redefine command \"%s\"? "), c->name, c->name);
>  	  else
>  	    q = query (_("Redefine command \"%s\"? "), c->name);
>  	}
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 17f357832c..2c30ea657e 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -26564,6 +26564,11 @@ Example:
>  @example
>  (gdb) define-prefix abc
>  (gdb) define-prefix abc def
> +(gdb) define abc def
> +Type commands for definition of "abc def".
> +End with a line saying just "end".
> +>echo command initial def\n
> +>end
>  (gdb) define abc def ghi
>  Type commands for definition of "abc def ghi".
>  End with a line saying just "end".
> diff --git a/gdb/testsuite/gdb.base/define-prefix.exp b/gdb/testsuite/gdb.base/define-prefix.exp
> index e8f4c3c4e4..b04c5959c0 100644
> --- a/gdb/testsuite/gdb.base/define-prefix.exp
> +++ b/gdb/testsuite/gdb.base/define-prefix.exp
> @@ -98,27 +98,45 @@ gdb_test "abc-prefix def-prefix ghi-prefix-cmd alternate-jkl-cmd" \
>  
>  
>  
> -# Now redefine ghi-prefix-cmd as a real command, and check it is working.
> +# Now define ghi-prefix-cmd as a real command, and check it is working.
> +send_gdb "define abc-prefix def-prefix ghi-prefix-cmd\n"
> +gdb_expect {
> +    -re "Type commands for definition of \"abc-prefix def-prefix ghi-prefix-cmd\".\r\nEnd with a line saying just \"end\".\r\n>$" {
> +	send_gdb "echo defined command ghi-prefix-cmd\\n\nend\n"
> +	gdb_expect {
> +	    -re "$gdb_prompt $" {pass "define user command: ghi-prefix-cmd"}
> +	    timeout {fail "(timeout) define user command: ghi-prefix-cmd"}
> +	}
> +    }
> +    timeout {fail "(timeout) define user command: ghi-prefix-cmd"}
> +}
> +
> +# Verify ghi-prefix-cmd command works.
> +gdb_test "abc-prefix def-prefix ghi-prefix-cmd" \
> +    "defined command ghi-prefix-cmd" \
> +    "use defined user command: ghi-prefix-cmd"
> +
> +# Now redefine ghi-prefix-cmd, and check it is working.
>  send_gdb "define abc-prefix def-prefix ghi-prefix-cmd\n"
>  gdb_expect {
>      -re "Keeping subcommands of prefix command \"ghi-prefix-cmd\"\.\r\nRedefine command \"ghi-prefix-cmd\".*y or n. $" {
>  	send_gdb "y\n"
> -	    gdb_expect {
> -		-re "Type commands for definition of \"abc-prefix def-prefix ghi-prefix-cmd\".\r\nEnd with a line saying just \"end\".\r\n>$" {
> -		    send_gdb "echo redefined command ghi-prefix-cmd\\n\nend\n"
> -		    gdb_expect {
> -			-re "$gdb_prompt $" {pass "redefine user command: ghi-prefix-cmd"}
> -			timeout {fail "(timeout) redefine user command: ghi-prefix-cmd"}
> -		    }
> +	gdb_expect {
> +	    -re "Type commands for definition of \"abc-prefix def-prefix ghi-prefix-cmd\".\r\nEnd with a line saying just \"end\".\r\n>$" {
> +		send_gdb "echo redefined command ghi-prefix-cmd\\n\nend\n"
> +		gdb_expect {
> +		    -re "$gdb_prompt $" {pass "redefine user command: ghi-prefix-cmd"}
> +		    timeout {fail "(timeout) redefine user command: ghi-prefix-cmd"}
>  		}
> -		timeout {fail "(timeout) redefine user command: ghi-prefix-cmd"}
>  	    }
> +	    timeout {fail "(timeout) redefine user command: ghi-prefix-cmd"}
> +	}
>      }
> -    -re "$gdb_prompt $"	{fail "redefine user command: ghi-prefix-cmd"}
> +    -re "$gdb_prompt $" {fail "redefine user command: ghi-prefix-cmd"}
>      timeout {fail "(timeout) redefine user command: ghi-prefix-cmd"}
>  }
>  
> -# Verify ghi-prefix-cmd command works.
> +# Verify redefined ghi-prefix-cmd command works.
>  gdb_test "abc-prefix def-prefix ghi-prefix-cmd" \
>      "redefined command ghi-prefix-cmd" \
>      "use redefined user command: ghi-prefix-cmd"
> 
> 
> 

Thanks, this version of the series LGTM.  You can push it, if others have
comments about the naming or behavior of the command, there's still time
to complain before the release.

Simon
  
Philippe Waroquiers Nov. 30, 2019, 8:55 a.m. UTC | #2
On Fri, 2019-11-29 at 22:36 -0500, Simon Marchi wrote:
> Thanks, this version of the series LGTM.  You can push it, if others have
> comments about the naming or behavior of the command, there's still time
> to complain before the release.
Pushed after fixing the formatting comment and retest.
Thanks for the reviews
Philippe
  

Patch

diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c
index 79c499a4c2..6900f5c9ee 100644
--- a/gdb/cli/cli-script.c
+++ b/gdb/cli/cli-script.c
@@ -1397,11 +1397,13 @@  do_define_command (const char *comname, int from_tty,
 
       if (c->theclass == class_user || c->theclass == class_alias)
 	{
-	  /* if C is a prefix command, tell the user its subcommands will
-	     be kept, and ask if ok to redefine the command.  */
+	  /* if C is a prefix command that was previously defined,
+	     tell the user its subcommands will be kept, and ask
+	     if ok to redefine the command.  */
 	  if (c->prefixlist != nullptr)
-	    q = query (_("Keeping subcommands of prefix command \"%s\".\n"
-			 "Redefine command \"%s\"? "), c->name, c->name);
+	    q = c->user_commands.get () == nullptr
+	      || query (_("Keeping subcommands of prefix command \"%s\".\n"
+			  "Redefine command \"%s\"? "), c->name, c->name);
 	  else
 	    q = query (_("Redefine command \"%s\"? "), c->name);
 	}
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 17f357832c..2c30ea657e 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -26564,6 +26564,11 @@  Example:
 @example
 (gdb) define-prefix abc
 (gdb) define-prefix abc def
+(gdb) define abc def
+Type commands for definition of "abc def".
+End with a line saying just "end".
+>echo command initial def\n
+>end
 (gdb) define abc def ghi
 Type commands for definition of "abc def ghi".
 End with a line saying just "end".
diff --git a/gdb/testsuite/gdb.base/define-prefix.exp b/gdb/testsuite/gdb.base/define-prefix.exp
index e8f4c3c4e4..b04c5959c0 100644
--- a/gdb/testsuite/gdb.base/define-prefix.exp
+++ b/gdb/testsuite/gdb.base/define-prefix.exp
@@ -98,27 +98,45 @@  gdb_test "abc-prefix def-prefix ghi-prefix-cmd alternate-jkl-cmd" \
 
 
 
-# Now redefine ghi-prefix-cmd as a real command, and check it is working.
+# Now define ghi-prefix-cmd as a real command, and check it is working.
+send_gdb "define abc-prefix def-prefix ghi-prefix-cmd\n"
+gdb_expect {
+    -re "Type commands for definition of \"abc-prefix def-prefix ghi-prefix-cmd\".\r\nEnd with a line saying just \"end\".\r\n>$" {
+	send_gdb "echo defined command ghi-prefix-cmd\\n\nend\n"
+	gdb_expect {
+	    -re "$gdb_prompt $" {pass "define user command: ghi-prefix-cmd"}
+	    timeout {fail "(timeout) define user command: ghi-prefix-cmd"}
+	}
+    }
+    timeout {fail "(timeout) define user command: ghi-prefix-cmd"}
+}
+
+# Verify ghi-prefix-cmd command works.
+gdb_test "abc-prefix def-prefix ghi-prefix-cmd" \
+    "defined command ghi-prefix-cmd" \
+    "use defined user command: ghi-prefix-cmd"
+
+# Now redefine ghi-prefix-cmd, and check it is working.
 send_gdb "define abc-prefix def-prefix ghi-prefix-cmd\n"
 gdb_expect {
     -re "Keeping subcommands of prefix command \"ghi-prefix-cmd\"\.\r\nRedefine command \"ghi-prefix-cmd\".*y or n. $" {
 	send_gdb "y\n"
-	    gdb_expect {
-		-re "Type commands for definition of \"abc-prefix def-prefix ghi-prefix-cmd\".\r\nEnd with a line saying just \"end\".\r\n>$" {
-		    send_gdb "echo redefined command ghi-prefix-cmd\\n\nend\n"
-		    gdb_expect {
-			-re "$gdb_prompt $" {pass "redefine user command: ghi-prefix-cmd"}
-			timeout {fail "(timeout) redefine user command: ghi-prefix-cmd"}
-		    }
+	gdb_expect {
+	    -re "Type commands for definition of \"abc-prefix def-prefix ghi-prefix-cmd\".\r\nEnd with a line saying just \"end\".\r\n>$" {
+		send_gdb "echo redefined command ghi-prefix-cmd\\n\nend\n"
+		gdb_expect {
+		    -re "$gdb_prompt $" {pass "redefine user command: ghi-prefix-cmd"}
+		    timeout {fail "(timeout) redefine user command: ghi-prefix-cmd"}
 		}
-		timeout {fail "(timeout) redefine user command: ghi-prefix-cmd"}
 	    }
+	    timeout {fail "(timeout) redefine user command: ghi-prefix-cmd"}
+	}
     }
-    -re "$gdb_prompt $"	{fail "redefine user command: ghi-prefix-cmd"}
+    -re "$gdb_prompt $" {fail "redefine user command: ghi-prefix-cmd"}
     timeout {fail "(timeout) redefine user command: ghi-prefix-cmd"}
 }
 
-# Verify ghi-prefix-cmd command works.
+# Verify redefined ghi-prefix-cmd command works.
 gdb_test "abc-prefix def-prefix ghi-prefix-cmd" \
     "redefined command ghi-prefix-cmd" \
     "use redefined user command: ghi-prefix-cmd"