public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [RFC] Add ada-exception-catchpoints to -list-features command output.
@ 2013-11-10 17:16 Joel Brobecker
  2013-11-10 22:16 ` Eli Zaretskii
  2013-11-11 15:22 ` Tom Tromey
  0 siblings, 2 replies; 42+ messages in thread
From: Joel Brobecker @ 2013-11-10 17:16 UTC (permalink / raw)
  To: gdb-patches

Hello,

The -list-features GDB/MI command is extremely useful to GDB frontends
who would like to know whether the underlying GDB supports a given
feature or not. Recently, I added new coommands for Ada exception
catching, but forgot about -list-features.

This patch adds an entry meant to help the frontend for those features.
But looking at the way the -list-features command is designed, I am
wondering whether this approach is going to scale well. As new commands
and other new features or major bug fixes get in, it seems like the
list is going to grow maybe a little beyond what's reasonable.

Without trying to redesign this feature entirely, since we're far from
being in that difficult situation, I am thinking it would be OK
to aggregate both fields related to Ada exceptions, namely...

  - "info-ada-exceptions" (being introduced as we speak, see
    http://www.sourceware.org/ml/gdb-patches/2013-11/msg00232.html)

  - this new field (I chose "ada-exceptions-catchpoints")

... into a single field. For instance, we could chose "ada-exceptions",
meant to cover both the commands already in (for catching Ada exceptions),
and the new command being introduced (for listing all Ada exceptions).

So, although this patch proposes a new field (this is the straightforward
approach), given that all this GDB/MI work was done within the same
release cycle, and withing a reasonable amount of time, I think it
would be fine for everyone to use one single field in -list-features.

Thoughts?

In practical terms, I would drop the part in the patch quoted above
that adds the new -list-features field, and would propose another
one, distinct from the "-info-ada-exceptions" patch, which would
then introduce the new "ada-exceptions" -list-features field instead.

Thank you!

gdb/ChangeLog:

        * mi/mi-main.c (mi_cmd_list_features): Add
        "ada-exception-catchpoints" to -list-features output.

gdb/doc/ChangeLog:

        * gdb.texinfo (GDB/MI Miscellaneous Commands): Document new
        field "ada-exception-catchpoints" in -list-features output.
---
 gdb/doc/gdb.texinfo | 3 +++
 gdb/mi/mi-main.c    | 1 +
 2 files changed, 4 insertions(+)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 9127f94..9731bbf 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -34971,6 +34971,9 @@ CLI will be announced via async records.
 indicates support for the @code{-ada-task-info} command.
 @item info-ada-exceptions
 indicates support for the @code{-info-ada-exceptions} command.
+@item ada-exception-catchpoints
+indicates support for the @code{-catch-assert} and @code{-catch-exception}
+commands.
 @end table
 
 @subheading The @code{-list-target-features} Command
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index bf0fce3..8ff7f27 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1816,6 +1816,7 @@ mi_cmd_list_features (char *command, char **argv, int argc)
       ui_out_field_string (uiout, NULL, "breakpoint-notifications");
       ui_out_field_string (uiout, NULL, "ada-task-info");
       ui_out_field_string (uiout, NULL, "info-ada-exceptions");
+      ui_out_field_string (uiout, NULL, "ada-exception-catchpoints");
       
 #if HAVE_PYTHON
       if (gdb_python_initialized)
-- 
1.8.1.2

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [RFC] Add ada-exception-catchpoints to -list-features command output.
  2013-11-10 17:16 [RFC] Add ada-exception-catchpoints to -list-features command output Joel Brobecker
@ 2013-11-10 22:16 ` Eli Zaretskii
  2013-11-12 11:25   ` Joel Brobecker
  2013-11-11 15:22 ` Tom Tromey
  1 sibling, 1 reply; 42+ messages in thread
From: Eli Zaretskii @ 2013-11-10 22:16 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

> From: Joel Brobecker <brobecker@adacore.com>
> Date: Sun, 10 Nov 2013 13:49:20 +0400
> 
> The -list-features GDB/MI command is extremely useful to GDB frontends
> who would like to know whether the underlying GDB supports a given
> feature or not. Recently, I added new coommands for Ada exception
> catching, but forgot about -list-features.
> 
> This patch adds an entry meant to help the frontend for those features.
> But looking at the way the -list-features command is designed, I am
> wondering whether this approach is going to scale well. As new commands
> and other new features or major bug fixes get in, it seems like the
> list is going to grow maybe a little beyond what's reasonable.

Thanks, the documentation part is OK.

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [RFC] Add ada-exception-catchpoints to -list-features command output.
  2013-11-10 17:16 [RFC] Add ada-exception-catchpoints to -list-features command output Joel Brobecker
  2013-11-10 22:16 ` Eli Zaretskii
@ 2013-11-11 15:22 ` Tom Tromey
  2013-11-12  9:18   ` Joel Brobecker
  2013-11-12 12:11   ` [RFC] New GDB/MI command "-info-gdb-mi-command" Joel Brobecker
  1 sibling, 2 replies; 42+ messages in thread
From: Tom Tromey @ 2013-11-11 15:22 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

Joel> This patch adds an entry meant to help the frontend for those features.
Joel> But looking at the way the -list-features command is designed, I am
Joel> wondering whether this approach is going to scale well. As new commands
Joel> and other new features or major bug fixes get in, it seems like the
Joel> list is going to grow maybe a little beyond what's reasonable.

We could add a way to check for specific commands.  Then new commands
would never need to be added to the feature list.

Joel> So, although this patch proposes a new field (this is the straightforward
Joel> approach), given that all this GDB/MI work was done within the same
Joel> release cycle, and withing a reasonable amount of time, I think it
Joel> would be fine for everyone to use one single field in -list-features.

I think it is reasonable, too, provided that the MI docs note the
details of what the feature means.

Tom

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [RFC] Add ada-exception-catchpoints to -list-features command output.
  2013-11-11 15:22 ` Tom Tromey
@ 2013-11-12  9:18   ` Joel Brobecker
  2013-11-12 12:11   ` [RFC] New GDB/MI command "-info-gdb-mi-command" Joel Brobecker
  1 sibling, 0 replies; 42+ messages in thread
From: Joel Brobecker @ 2013-11-12  9:18 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

> Joel> This patch adds an entry meant to help the frontend for those features.
> Joel> But looking at the way the -list-features command is designed, I am
> Joel> wondering whether this approach is going to scale well. As new commands
> Joel> and other new features or major bug fixes get in, it seems like the
> Joel> list is going to grow maybe a little beyond what's reasonable.
> 
> We could add a way to check for specific commands.  Then new commands
> would never need to be added to the feature list.

I like the idea. I need to think a bit about the actual API.
I was thinking something along the lines of:

    -info-gdb-mi-command MI_COMMAND

The first implementation would just return one field, which would
say yay or nay. But eventually, it might be nice to add other
properties, such as maybe some kind of versioning number to help
track evolution of the command, or maybe a command-specific feature
list. The latter might be less abstract/complex and extensible enough
to fit all needs.

So, for instance (if the following is valid GDB/MI syntax):

    -info-gdb-mi-command catch-load
    -^done,info={exists="true",features=['-c']}

which would tell us that the -catch-load command exists, and that
in this version of GDB, the "-c" features is supported, for instance
telling us that it's possible to add a condition to the catchpoint.

> 
> Joel> So, although this patch proposes a new field (this is the straightforward
> Joel> approach), given that all this GDB/MI work was done within the same
> Joel> release cycle, and withing a reasonable amount of time, I think it
> Joel> would be fine for everyone to use one single field in -list-features.
> 
> I think it is reasonable, too, provided that the MI docs note the
> details of what the feature means.

Of course. I will send a patch as soon as I have a moment.

-- 
Joel

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [RFC] Add ada-exception-catchpoints to -list-features command output.
  2013-11-10 22:16 ` Eli Zaretskii
@ 2013-11-12 11:25   ` Joel Brobecker
  2013-11-12 16:39     ` Eli Zaretskii
  0 siblings, 1 reply; 42+ messages in thread
From: Joel Brobecker @ 2013-11-12 11:25 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

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

Hi Eli,

> Thanks, the documentation part is OK.

Thanks for the review. As discussed with Tom, I've decided to
merge the two entries into one. Attached is a patch that does that,
together with a revised manual update.  Can you tell me what you think?
In particular, I am wondering whether the "all related to Ada
exceptions" really brings anything in this context other than noise.

gdb/ChangeLog:

        * mi/mi-main.c (mi_cmd_list_features): Replace "info-ada-exceptions"
        entry with "ada-exceptions".

gdb/doc/ChangeLog:

        * gdb.texinfo (GDB/MI Miscellaneous Commands): Delete
        the documentation of "info-ada-exceptions" in the output
        of the "-list-features" command.  Add the documentation
        of the "ada-exception" entry instead.

-- 
Joel

[-- Attachment #2: 0001-Replace-info-ada-exceptions-by-ada-exceptions-in-lis.patch --]
[-- Type: text/x-diff, Size: 2467 bytes --]

From 569128117ccf37ff83033a69929d5c76f1d8acb3 Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Tue, 12 Nov 2013 13:43:44 +0400
Subject: [PATCH] Replace "info-ada-exceptions" by "ada-exceptions" in
 -list-features

Rather than having -list-features report support for the GDB/MI
commands providing access to Ada exception catchpoints with one entry,
and the GDB/MI command providing the list of Ada exceptions with
a second entry, this patch merges it all within one single entry.
This is OK, because all these commands were added within a short
amount of time, and within the same release cycle; and it reduces
a bit the size of the output.

gdb/ChangeLog:

        * mi/mi-main.c (mi_cmd_list_features): Replace "info-ada-exceptions"
        entry with "ada-exceptions".

gdb/doc/ChangeLog:

        * gdb.texinfo (GDB/MI Miscellaneous Commands): Delete
        the documentation of "info-ada-exceptions" in the output
        of the "-list-features" command.  Add the documentation
        of the "ada-exception" entry instead.
---
 gdb/doc/gdb.texinfo | 6 ++++--
 gdb/mi/mi-main.c    | 2 +-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index de419ea..2c4234a 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -35080,8 +35080,10 @@ Indicates that changes to breakpoints and breakpoints created via the
 CLI will be announced via async records.
 @item ada-task-info
 Indicates support for the @code{-ada-task-info} command.
-@item info-ada-exceptions
-Indicates support for the @code{-info-ada-exceptions} command.
+@item ada-exceptions
+Indicates support for the following commands, all related to Ada
+exceptions: @code{-info-ada-exceptions}, @code{-catch-assert} and
+@code{-catch-exception}.
 @end table
 
 @subheading The @code{-list-target-features} Command
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index bf0fce3..c3f7221 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1815,7 +1815,7 @@ mi_cmd_list_features (char *command, char **argv, int argc)
       ui_out_field_string (uiout, NULL, "data-read-memory-bytes");
       ui_out_field_string (uiout, NULL, "breakpoint-notifications");
       ui_out_field_string (uiout, NULL, "ada-task-info");
-      ui_out_field_string (uiout, NULL, "info-ada-exceptions");
+      ui_out_field_string (uiout, NULL, "ada-exceptions");
       
 #if HAVE_PYTHON
       if (gdb_python_initialized)
-- 
1.8.1.2


^ permalink raw reply	[flat|nested] 42+ messages in thread

* [RFC] New GDB/MI command "-info-gdb-mi-command"
  2013-11-11 15:22 ` Tom Tromey
  2013-11-12  9:18   ` Joel Brobecker
@ 2013-11-12 12:11   ` Joel Brobecker
  2013-11-12 17:04     ` Eli Zaretskii
  2013-11-12 21:17     ` André Pönitz
  1 sibling, 2 replies; 42+ messages in thread
From: Joel Brobecker @ 2013-11-12 12:11 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches

[sorry for the duplicate, Tom, mishandling of the --to= send-email option]

Hi Tom,

> We could add a way to check for specific commands.  Then new commands
> would never need to be added to the feature list.

What do you think of the following?

This patch adds a new GDB/MI command meant for graphical frontends
trying to determine whether a given GDB/MI command exists or not.

Examples:

    -info-gdb-mi-command unsupported-command
    ^done,command={exists="false"}
    (gdb)
    -info-gdb-mi-command symbol-list-lines
    ^done,command={exists="true"}
    (gdb)

At the moment, this is the only piece of information that this
command returns.

Eventually, and if needed, we can extend it to provide
command-specific pieces of information, such as updates to
the command's syntax since inception.  This could become,
for instance:

    -info-gdb-mi-command symbol-list-lines
    ^done,command={exists="true",features=[]}
    (gdb)
    -info-gdb-mi-command catch-assert
    ^done,command={exists="true",features=["conditions"]}

In the first case, it would mean that no extra features,
while in the second, it announces that the -catch-assert
command in this version of the debugger supports a feature
called "condition" - exact semantics to be documented with
combined with the rest of the queried command's documentation.

But for now, we start small, and only worry about existance.
And to bootstrap the process, I have added an entry in the
output of the -list-features command as well ("info-gdb-mi-command"),
allowing the graphical frontends to go through the following process:

  1. Send -list-features, collect info from there as before;
  2. Check if the output contains "info-gdb-mi-command".
     If it does, then support for various commands can be
     queried though -info-gdb-mi-command. Newer commands
     will be expected to always be checked via this new
     -info-gdb-mi-command.

gdb/ChangeLog:

        * mi/mi-cmds.h (mi_cmd_info_gdb_mi_command): Declare.
        * mi/mi-cmd-info.c (mi_cmd_info_gdb_mi_command): New function.
        * mi/mi-cmds.c (mi_cmds): Add -info-gdb-mi-command command.
        * mi/mi-main.c (mi_cmd_list_features): Add "info-gdb-mi-command"
        field to output of "-list-features".

        * NEWS: Add entry for new -info-gdb-mi-command.

gdb/doc/ChangeLog:

        * gdb.texinfo (GDB/MI Miscellaneous Commands): Document
        the new -info-gdb-mi-command GDB/MI command.  Document
        the meaning of "-info-gdb-mi-command" in the output of
        -list-features.

gdb/testsuite/ChangeLog:

        * gdb.mi/mi-i-cmd.exp: New file.

Tested on x86_64-linux.  Comments?

Thanks,
-- 
Joel

---
 gdb/NEWS                          |  3 +++
 gdb/doc/gdb.texinfo               | 46 +++++++++++++++++++++++++++++++++++++++
 gdb/mi/mi-cmd-info.c              | 21 ++++++++++++++++++
 gdb/mi/mi-cmds.c                  |  1 +
 gdb/mi/mi-cmds.h                  |  1 +
 gdb/mi/mi-main.c                  |  1 +
 gdb/testsuite/gdb.mi/mi-i-cmd.exp | 31 ++++++++++++++++++++++++++
 7 files changed, 104 insertions(+)
 create mode 100644 gdb/testsuite/gdb.mi/mi-i-cmd.exp

diff --git a/gdb/NEWS b/gdb/NEWS
index 38209e8..cb1f95a 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -151,6 +151,9 @@ show startup-with-shell
 
 * MI changes
 
+  ** The new command -info-gdb-mi-command allows the user to determine
+     whether a GDB/MI command is supported or not.
+
   ** The -trace-save MI command can optionally save trace buffer in Common
      Trace Format now.
 
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index de419ea..db81ac2 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -35034,6 +35034,50 @@ default shows this information when you start an interactive session.
 (gdb)
 @end smallexample
 
+@subheading The @code{-info-gdb-mi-command} Command
+@findex -info-gdb-mi-command
+
+@subsubheading Synopsis
+
+@smallexample
+ -info-gdb-mi-command CMD_NAME
+@end smallexample
+
+Query support for the @sc{gdb/mi} command named @var{CMD_NAME}
+(the leading dash (@code{-}) in the command name should be omitted).
+
+@subsubheading @value{GDBN} Command
+
+There is no corresponding @value{GDBN} command.
+
+@subsubheading Result
+
+The result is a tuple.  There is currently only one field:
+
+@table @samp
+@item exists
+This field is equal to @code{"true"} if the @sc{gdb/mi} command exists,
+@code{"false"} otherwise.
+
+@end table
+
+@subsubheading Example
+
+Here is an example where the @sc{gdb/mi} command does not exist:
+
+@smallexample
+-info-gdb-mi-command unsupported-command
+^done,command=@{exists="false"@}
+@end smallexample
+
+And here is an example where the @sc{gdb/mi} command is known
+to the debugger:
+
+@smallexample
+-info-gdb-mi-command symbol-list-lines
+^done,command=@{exists="true"@}
+@end smallexample
+
 @subheading The @code{-list-features} Command
 @findex -list-features
 
@@ -35082,6 +35126,8 @@ CLI will be announced via async records.
 Indicates support for the @code{-ada-task-info} command.
 @item info-ada-exceptions
 Indicates support for the @code{-info-ada-exceptions} command.
+@item info-gdb-mi-command
+Indicates support for the @code{-info-gdb-mi-command} command.
 @end table
 
 @subheading The @code{-list-target-features} Command
diff --git a/gdb/mi/mi-cmd-info.c b/gdb/mi/mi-cmd-info.c
index aa4d210..18f4927 100644
--- a/gdb/mi/mi-cmd-info.c
+++ b/gdb/mi/mi-cmd-info.c
@@ -71,6 +71,27 @@ mi_cmd_info_ada_exceptions (char *command, char **argv, int argc)
   do_cleanups (old_chain);
 }
 
+/* Implement the "-info-gdb-mi-command" GDB/MI command.  */
+
+void
+mi_cmd_info_gdb_mi_command (char *command, char **argv, int argc)
+{
+  const char *cmd_name;
+  struct mi_cmd *cmd;
+  struct ui_out *uiout = current_uiout;
+  struct cleanup *old_chain;
+
+  /* This command takes exactly one argument.  */
+  if (argc != 1)
+    error (_("Usage: -info-gdb-mi-command MI_COMMAND_NAME"));
+  cmd_name = argv[0];
+  cmd = mi_lookup (cmd_name);
+
+  old_chain = make_cleanup_ui_out_tuple_begin_end (uiout, "command");
+  ui_out_field_string (uiout, "exists", cmd != NULL ? "true" : "false");
+  do_cleanups (old_chain);
+}
+
 void
 mi_cmd_info_os (char *command, char **argv, int argc)
 {
diff --git a/gdb/mi/mi-cmds.c b/gdb/mi/mi-cmds.c
index 496a8aa..aed62b2 100644
--- a/gdb/mi/mi-cmds.c
+++ b/gdb/mi/mi-cmds.c
@@ -125,6 +125,7 @@ static struct mi_cmd mi_cmds[] =
   DEF_MI_CMD_MI ("inferior-tty-set", mi_cmd_inferior_tty_set),
   DEF_MI_CMD_MI ("inferior-tty-show", mi_cmd_inferior_tty_show),
   DEF_MI_CMD_MI ("info-ada-exceptions", mi_cmd_info_ada_exceptions),
+  DEF_MI_CMD_MI ("info-gdb-mi-command", mi_cmd_info_gdb_mi_command),
   DEF_MI_CMD_MI ("info-os", mi_cmd_info_os),
   DEF_MI_CMD_MI ("interpreter-exec", mi_cmd_interpreter_exec),
   DEF_MI_CMD_MI ("list-features", mi_cmd_list_features),
diff --git a/gdb/mi/mi-cmds.h b/gdb/mi/mi-cmds.h
index cb8aac1..4ea95fa 100644
--- a/gdb/mi/mi-cmds.h
+++ b/gdb/mi/mi-cmds.h
@@ -74,6 +74,7 @@ extern mi_cmd_argv_ftype mi_cmd_gdb_exit;
 extern mi_cmd_argv_ftype mi_cmd_inferior_tty_set;
 extern mi_cmd_argv_ftype mi_cmd_inferior_tty_show;
 extern mi_cmd_argv_ftype mi_cmd_info_ada_exceptions;
+extern mi_cmd_argv_ftype mi_cmd_info_gdb_mi_command;
 extern mi_cmd_argv_ftype mi_cmd_info_os;
 extern mi_cmd_argv_ftype mi_cmd_interpreter_exec;
 extern mi_cmd_argv_ftype mi_cmd_list_features;
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index bf0fce3..f0846da 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1816,6 +1816,7 @@ mi_cmd_list_features (char *command, char **argv, int argc)
       ui_out_field_string (uiout, NULL, "breakpoint-notifications");
       ui_out_field_string (uiout, NULL, "ada-task-info");
       ui_out_field_string (uiout, NULL, "info-ada-exceptions");
+      ui_out_field_string (uiout, NULL, "info-gdb-mi-command");
       
 #if HAVE_PYTHON
       if (gdb_python_initialized)
diff --git a/gdb/testsuite/gdb.mi/mi-i-cmd.exp b/gdb/testsuite/gdb.mi/mi-i-cmd.exp
new file mode 100644
index 0000000..a115451
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-i-cmd.exp
@@ -0,0 +1,31 @@
+# Copyright 2013 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+gdb_exit
+if [mi_gdb_start] {
+    continue
+}
+
+mi_gdb_test "-info-gdb-mi-command unsupported-command" \
+            "\\^done,command=\\\{exists=\"false\"\\\}" \
+            "-info-gdb-mi-command unsupported-command"
+
+mi_gdb_test "-info-gdb-mi-command symbol-list-lines" \
+            "\\^done,command=\\\{exists=\"true\"\\\}" \
+            "-info-gdb-mi-command symbol-list-lines"
+
-- 
1.8.1.2

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [RFC] Add ada-exception-catchpoints to -list-features command output.
  2013-11-12 11:25   ` Joel Brobecker
@ 2013-11-12 16:39     ` Eli Zaretskii
  2013-11-13  3:02       ` Joel Brobecker
  0 siblings, 1 reply; 42+ messages in thread
From: Eli Zaretskii @ 2013-11-12 16:39 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

> Date: Tue, 12 Nov 2013 13:54:16 +0400
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: gdb-patches@sourceware.org
> 
> Thanks for the review. As discussed with Tom, I've decided to
> merge the two entries into one. Attached is a patch that does that,
> together with a revised manual update.  Can you tell me what you think?
> In particular, I am wondering whether the "all related to Ada
> exceptions" really brings anything in this context other than noise.

Looks good to me, although I suggest a minor correction below.

> +Indicates support for the following commands, all related to Ada
> +exceptions:                                   ^^^^^^^^^^^

I suggest "all of them related" here.

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [RFC] New GDB/MI command "-info-gdb-mi-command"
  2013-11-12 12:11   ` [RFC] New GDB/MI command "-info-gdb-mi-command" Joel Brobecker
@ 2013-11-12 17:04     ` Eli Zaretskii
  2013-11-12 17:48       ` Joel Brobecker
  2013-11-12 21:17     ` André Pönitz
  1 sibling, 1 reply; 42+ messages in thread
From: Eli Zaretskii @ 2013-11-12 17:04 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: tromey, gdb-patches

> From: Joel Brobecker <brobecker@adacore.com>
> Cc: gdb-patches@sourceware.org
> Date: Tue, 12 Nov 2013 15:25:04 +0400
> 
> This patch adds a new GDB/MI command meant for graphical frontends
> trying to determine whether a given GDB/MI command exists or not.
> 
> Examples:
> 
>     -info-gdb-mi-command unsupported-command
>     ^done,command={exists="false"}
>     (gdb)
>     -info-gdb-mi-command symbol-list-lines
>     ^done,command={exists="true"}
>     (gdb)

Sounds good to me.

> +@subheading The @code{-info-gdb-mi-command} Command
> +@findex -info-gdb-mi-command

This should be prominently indexed with @cindex entries, as this
command is very important, and should be easily found.

> +@subsubheading Synopsis
> +
> +@smallexample
> + -info-gdb-mi-command CMD_NAME
> +@end smallexample
> +
> +Query support for the @sc{gdb/mi} command named @var{CMD_NAME}

Ts-ts-ts... ASCII art habits die hard.  Use @var in the example, and
don't upcase CMD_NAME (it is upcased in Info anyway, and will look
better in print in lower case).

> +(the leading dash (@code{-}) in the command name should be omitted).

Is this wise?  How about if we support both with and without the dash?

> +There is no corresponding @value{GDBN} command.

Having a way of querying that in CLI would facilitate better .gdbinit
files, I think.

> +@smallexample
> +-info-gdb-mi-command symbol-list-lines
> +^done,command=@{exists="true"@}
> +@end smallexample

Btw, why "command="?  Perhaps "result="?

Other than that, the documentation is fine with me.

Thanks.

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [RFC] New GDB/MI command "-info-gdb-mi-command"
  2013-11-12 17:04     ` Eli Zaretskii
@ 2013-11-12 17:48       ` Joel Brobecker
  2013-11-12 18:34         ` Eli Zaretskii
  0 siblings, 1 reply; 42+ messages in thread
From: Joel Brobecker @ 2013-11-12 17:48 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: tromey, gdb-patches

> > This patch adds a new GDB/MI command meant for graphical frontends
> > trying to determine whether a given GDB/MI command exists or not.
> > 
> > Examples:
> > 
> >     -info-gdb-mi-command unsupported-command
> >     ^done,command={exists="false"}
> >     (gdb)
> >     -info-gdb-mi-command symbol-list-lines
> >     ^done,command={exists="true"}
> >     (gdb)
> 
> Sounds good to me.
> 
> > +@subheading The @code{-info-gdb-mi-command} Command
> > +@findex -info-gdb-mi-command
> 
> This should be prominently indexed with @cindex entries, as this
> command is very important, and should be easily found.
> 
> > +@subsubheading Synopsis
> > +
> > +@smallexample
> > + -info-gdb-mi-command CMD_NAME
> > +@end smallexample
> > +
> > +Query support for the @sc{gdb/mi} command named @var{CMD_NAME}
> 
> Ts-ts-ts... ASCII art habits die hard.  Use @var in the example, and
> don't upcase CMD_NAME (it is upcased in Info anyway, and will look
> better in print in lower case).

Thanks for the documentation review. I will fix them and post
a new patch after we confirm the final version of the command.

> > +(the leading dash (@code{-}) in the command name should be omitted).
> 
> Is this wise?  How about if we support both with and without the dash?

It's just easier to program, as this is how commands are stored in
GDB and also looked up by the GDB/MI commadn parser.

This can be argued as weak justification, and it is, but we don't
really need to do any better, IMO.  Since this is a command in a mode
mostly intended for machines, I didn't see the point in supporting
the other version, or both. I still think the current syntax is fine
as clearly documented. But I can implement the more natural one instead,
if we want. I think providing two modes of operation would be overkill,
though.

> > +There is no corresponding @value{GDBN} command.
> 
> Having a way of querying that in CLI would facilitate better .gdbinit
> files, I think.

Can you give some ideas as to how it would be used. I thought this
command to be completely irrelevant to CLI, so didn't even start
to consider the idea of providing it in CLI. Remember also that
you can always execute a GDB/MI command from CLI using
"interpreter-exec".

> > +@smallexample
> > +-info-gdb-mi-command symbol-list-lines
> > +^done,command=@{exists="true"@}
> > +@end smallexample
> 
> Btw, why "command="?  Perhaps "result="?

I don't really have an opinion. I eventually selected "command"
because the output describing a command, and "command" was also
less generic than the first choice I made ("info"). If people
think that "result=" is more MI-like, it's commpletely appropriate
to make the change.

Thanks!
-- 
Joel

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [RFC] New GDB/MI command "-info-gdb-mi-command"
  2013-11-12 17:48       ` Joel Brobecker
@ 2013-11-12 18:34         ` Eli Zaretskii
  2013-11-13  3:19           ` Joel Brobecker
  0 siblings, 1 reply; 42+ messages in thread
From: Eli Zaretskii @ 2013-11-12 18:34 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: tromey, gdb-patches

> Date: Tue, 12 Nov 2013 21:43:19 +0400
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: tromey@redhat.com, gdb-patches@sourceware.org
> 
> > > +There is no corresponding @value{GDBN} command.
> > 
> > Having a way of querying that in CLI would facilitate better .gdbinit
> > files, I think.
> 
> Can you give some ideas as to how it would be used.

Like this perhaps:

  if ($_featurep(FOO))
    lets-use-FOO
  end

> Remember also that you can always execute a GDB/MI command from CLI
> using "interpreter-exec".

But their output is not easily caught and used like above.

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [RFC] New GDB/MI command "-info-gdb-mi-command"
  2013-11-12 12:11   ` [RFC] New GDB/MI command "-info-gdb-mi-command" Joel Brobecker
  2013-11-12 17:04     ` Eli Zaretskii
@ 2013-11-12 21:17     ` André Pönitz
  2013-11-13  2:47       ` Joel Brobecker
  1 sibling, 1 reply; 42+ messages in thread
From: André Pönitz @ 2013-11-12 21:17 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: Tom Tromey, gdb-patches

On Tue, Nov 12, 2013 at 03:25:04PM +0400, Joel Brobecker wrote:
> [sorry for the duplicate, Tom, mishandling of the --to= send-email option]
> 
> Hi Tom,
> 
> > We could add a way to check for specific commands.  Then new commands
> > would never need to be added to the feature list.
> 
> What do you think of the following?
> 
> This patch adds a new GDB/MI command meant for graphical frontends
> trying to determine whether a given GDB/MI command exists or not.
> 
> Examples:
> 
>     -info-gdb-mi-command unsupported-command
>     ^done,command={exists="false"}
>     (gdb)
>     -info-gdb-mi-command symbol-list-lines
>     ^done,command={exists="true"}
>     (gdb)

I am not sure I agree with the judgement of benefits here. The basic 
yes/no information is already there:

    (gdb) -unsupported-command
    ^error,msg="Undefined MI command: unsupported-command"
    (gdb) -symbol-list-lines
    ^error,msg="-symbol-list-lines: Usage: SOURCE_FILENAME"

It's not nice, but "works".

In addition, a yes-or-no is not even what might be needed.

Look e.g. at the "python" advertisement in -list-features output
^done,features=["frozen-varobjs","pending-breakpoints","thread-info",
"data-read-memory-bytes","breakpoint-notifications","ada-task-info","python"]

It does not indicate whether it is properly installed (datadir...) nor
whether the version of Python is compatible with the script I want to
execute. So in practice, checking -list-features is just extra effort
giving only a subset of the information I would need for an "ok to use"
decision, and it's quicker and more reliable to just execute the command
and handle errors.

It's hard to imagine that this will ever cover enough of GDB features
and questions a frontend needs to have answered.

Andre'

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [RFC] New GDB/MI command "-info-gdb-mi-command"
  2013-11-12 21:17     ` André Pönitz
@ 2013-11-13  2:47       ` Joel Brobecker
  2013-11-14  0:36         ` André Pönitz
  2013-11-14 19:03         ` Pedro Alves
  0 siblings, 2 replies; 42+ messages in thread
From: Joel Brobecker @ 2013-11-13  2:47 UTC (permalink / raw)
  To: André Pönitz; +Cc: Tom Tromey, gdb-patches

> I am not sure I agree with the judgement of benefits here. The basic 
> yes/no information is already there:
> 
>     (gdb) -unsupported-command
>     ^error,msg="Undefined MI command: unsupported-command"
>     (gdb) -symbol-list-lines
>     ^error,msg="-symbol-list-lines: Usage: SOURCE_FILENAME"
> 
> It's not nice, but "works".

I disagree with your assessment of "works". I can think of a number
of scenarios where this would be problematic:

The first and most obvious to me is the case where the debugger is
run with a non-English LANG. If you base your detection on parsing
the error msg, then i18n ruins your plan. And if you base your detection
on the presence of the error alone, then commands that take no argument
may return an error, which by no means indicates that the command does not
exist.

> In addition, a yes-or-no is not even what might be needed.

Well, the IDE team at AdaCore needs that information in order to
support the variety of GDB versions out there, and I also agreed
with them that this was a sensible request.

> Look e.g. at the "python" advertisement in -list-features output
> ^done,features=["frozen-varobjs","pending-breakpoints","thread-info",
> "data-read-memory-bytes","breakpoint-notifications","ada-task-info","python"]
> 
> It does not indicate whether it is properly installed (datadir...) nor
> whether the version of Python is compatible with the script I want to
> execute. So in practice, checking -list-features is just extra effort
> giving only a subset of the information I would need for an "ok to use"
> decision, and it's quicker and more reliable to just execute the command
> and handle errors.
> 
> It's hard to imagine that this will ever cover enough of GDB features
> and questions a frontend needs to have answered.

If we were discussing about the specific issues regarding the use of
Python in your example, I would say that this is outside the scope of
this new command.

If you are trying to make a general point, then can you please tell
us how you think we can improve it? If not, you are free to find it
useless and to prefer to just use your execution test. But I definitely
think it's cleaner to query the debugger with a well documented interface,
rather than relying on detecting certain kinds of errors.

-- 
Joel

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [RFC] Add ada-exception-catchpoints to -list-features command output.
  2013-11-12 16:39     ` Eli Zaretskii
@ 2013-11-13  3:02       ` Joel Brobecker
  0 siblings, 0 replies; 42+ messages in thread
From: Joel Brobecker @ 2013-11-13  3:02 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

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

> > Thanks for the review. As discussed with Tom, I've decided to
> > merge the two entries into one. Attached is a patch that does that,
> > together with a revised manual update.  Can you tell me what you think?
> > In particular, I am wondering whether the "all related to Ada
> > exceptions" really brings anything in this context other than noise.
> 
> Looks good to me, although I suggest a minor correction below.
> 
> > +Indicates support for the following commands, all related to Ada
> > +exceptions:                                   ^^^^^^^^^^^
> 
> I suggest "all of them related" here.

I made the correction you suggested, and pushed the attached patch.

Thank you!

-- 
Joel

[-- Attachment #2: 0001-Replace-info-ada-exceptions-by-ada-exceptions-in-lis.patch --]
[-- Type: text/x-diff, Size: 3602 bytes --]

From 93973826c486a6b7b03eddb201e3ce994dd0fb5b Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Tue, 12 Nov 2013 13:43:44 +0400
Subject: [PATCH] Replace "info-ada-exceptions" by "ada-exceptions" in
 -list-features

Rather than having -list-features report support for the GDB/MI
commands providing access to Ada exception catchpoints with one entry,
and the GDB/MI command providing the list of Ada exceptions with
a second entry, this patch merges it all within one single entry.
This is OK, because all these commands were added within a short
amount of time, and within the same release cycle; and it reduces
a bit the size of the output.

gdb/ChangeLog:

        * mi/mi-main.c (mi_cmd_list_features): Replace "info-ada-exceptions"
        entry with "ada-exceptions".

gdb/doc/ChangeLog:

        * gdb.texinfo (GDB/MI Miscellaneous Commands): Delete
        the documentation of "info-ada-exceptions" in the output
        of the "-list-features" command.  Add the documentation
        of the "ada-exception" entry instead.
---
 gdb/ChangeLog       | 5 +++++
 gdb/doc/ChangeLog   | 8 ++++++++
 gdb/doc/gdb.texinfo | 6 ++++--
 gdb/mi/mi-main.c    | 2 +-
 4 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 279e3e2..6c20cce 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2013-11-13  Joel Brobecker  <brobecker@adacore.com>
 
+	* mi/mi-main.c (mi_cmd_list_features): Replace "info-ada-exceptions"
+	entry with "ada-exceptions".
+
+2013-11-13  Joel Brobecker  <brobecker@adacore.com>
+
 	* symfile.c (reread_symbols): Move call to set_objfile_per_bfd
 	after re-initialization of OBJFILE's obstack.
 
diff --git a/gdb/doc/ChangeLog b/gdb/doc/ChangeLog
index 2b13843..968201e 100644
--- a/gdb/doc/ChangeLog
+++ b/gdb/doc/ChangeLog
@@ -1,5 +1,13 @@
+2013-11-13  Joel Brobecker  <brobecker@adacore.com>
+
+	* gdb.texinfo (GDB/MI Miscellaneous Commands): Delete
+	the documentation of "info-ada-exceptions" in the output
+	of the "-list-features" command.  Add the documentation
+	of the "ada-exception" entry instead.
+
 2013-11-12  Joel Brobecker  <brobecker@adacore.com>
 
+
 	* gdb.texinfo (GDB/MI Miscellaneous Commands): Fix the first
 	word of a couple of sentences to start with a capital letter.
 
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index de419ea..84acd5c 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -35080,8 +35080,10 @@ Indicates that changes to breakpoints and breakpoints created via the
 CLI will be announced via async records.
 @item ada-task-info
 Indicates support for the @code{-ada-task-info} command.
-@item info-ada-exceptions
-Indicates support for the @code{-info-ada-exceptions} command.
+@item ada-exceptions
+Indicates support for the following commands, all of them related to Ada
+exceptions: @code{-info-ada-exceptions}, @code{-catch-assert} and
+@code{-catch-exception}.
 @end table
 
 @subheading The @code{-list-target-features} Command
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index bf0fce3..c3f7221 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1815,7 +1815,7 @@ mi_cmd_list_features (char *command, char **argv, int argc)
       ui_out_field_string (uiout, NULL, "data-read-memory-bytes");
       ui_out_field_string (uiout, NULL, "breakpoint-notifications");
       ui_out_field_string (uiout, NULL, "ada-task-info");
-      ui_out_field_string (uiout, NULL, "info-ada-exceptions");
+      ui_out_field_string (uiout, NULL, "ada-exceptions");
       
 #if HAVE_PYTHON
       if (gdb_python_initialized)
-- 
1.8.1.2


^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [RFC] New GDB/MI command "-info-gdb-mi-command"
  2013-11-12 18:34         ` Eli Zaretskii
@ 2013-11-13  3:19           ` Joel Brobecker
  0 siblings, 0 replies; 42+ messages in thread
From: Joel Brobecker @ 2013-11-13  3:19 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: tromey, gdb-patches

> > Can you give some ideas as to how it would be used.
> 
> Like this perhaps:
> 
>   if ($_featurep(FOO))
>     lets-use-FOO
>   end
> 
> > Remember also that you can always execute a GDB/MI command from CLI
> > using "interpreter-exec".
> 
> But their output is not easily caught and used like above.

This is true. But I have to say that I am still a little unsure
whether this CLI feature will actually be used, should we implement it.
Can we wait for an actual request before we do so, especially for
a query about GDB/MI when in CLI mode? That way, we then know that
we provide something people actually need. If you know you would
use that feature now, then of course that qualifies...

I think this feature can also be treated independently from
the new GDB/MI command. Adding this new feature wouldn't affect
this patch, I would think.

-- 
Joel

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [RFC] New GDB/MI command "-info-gdb-mi-command"
  2013-11-13  2:47       ` Joel Brobecker
@ 2013-11-14  0:36         ` André Pönitz
  2013-11-14  9:48           ` Joel Brobecker
  2013-11-14 19:03         ` Pedro Alves
  1 sibling, 1 reply; 42+ messages in thread
From: André Pönitz @ 2013-11-14  0:36 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: Tom Tromey, gdb-patches

On Wed, Nov 13, 2013 at 06:15:14AM +0400, Joel Brobecker wrote:
> > I am not sure I agree with the judgement of benefits here. The basic 
> > yes/no information is already there:
> > 
> >     (gdb) -unsupported-command
> >     ^error,msg="Undefined MI command: unsupported-command"
> >     (gdb) -symbol-list-lines
> >     ^error,msg="-symbol-list-lines: Usage: SOURCE_FILENAME"
> > 
> > It's not nice, but "works".
> 
> I disagree with your assessment of "works". I can think of a number
> of scenarios where this would be problematic:
> 
> The first and most obvious to me is the case where the debugger is
> run with a non-English LANG. If you base your detection on parsing
> the error msg, then i18n ruins your plan.

For me the context was "frontends". I assume they run external tools in an
environment that's as predictable as they need. If a user defined LANG is
problematic for a frontend, I would assume the frontend forces debugger
startup in a LANG that it knows to handle.

> And if you base your detection on the presence of the error alone, then
> commands that take no argument may return an error, which by no means
> indicates that the command does not exist.

I also assumed that such error message will not start with "Undefined
MI command:". 

> > In addition, a yes-or-no is not even what might be needed.
> 
> Well, the IDE team at AdaCore needs that information in order to support
> the variety of GDB versions out there, and I also agreed with them that
> this was a sensible request.

That's fine. You want a feature, and you implement it, and you are in a
position to get it in. I was merely jumping on the plural in "frontend_s_"
and only because I mistook that as "all", not as "possibly more than one".

> > Look e.g. at the "python" advertisement in -list-features output
> > ^done,features=["frozen-varobjs","pending-breakpoints","thread-info",
> > "data-read-memory-bytes","breakpoint-notifications","ada-task-info","python"]
> > 
> > It does not indicate whether it is properly installed (datadir...) nor
> > whether the version of Python is compatible with the script I want to
> > execute. So in practice, checking -list-features is just extra effort
> > giving only a subset of the information I would need for an "ok to use"
> > decision, and it's quicker and more reliable to just execute the
> > command and handle errors.
> > 
> > It's hard to imagine that this will ever cover enough of GDB features
> > and questions a frontend needs to have answered.
> 
> If we were discussing about the specific issues regarding the use of
> Python in your example, I would say that this is outside the scope of
> this new command.

We really aren't. "python" is an obvious case, but I could also have used
"thread-info" as example. Why should a frontend ask whether "-thread-info"
is supported, parse output, and switch code paths between "-thread-info"
and "info threads" (and hope that an announcement of "-thread-info"
existence is backed by an implementation) when it could just fire
"-thread-info" and handle a possible error by falling back on "info
thread"?

This would mean that users of well-behaved gdb builds/installation lose one
roundtrip, and the frontend needs to implement three funtions (ask, either,
or) instead of two (call, fallback)

> If you are trying to make a general point, then can you please tell us
> how you think we can improve it? If not, you are free to find it useless
> and to prefer to just use your execution test.

I was indeeed trying to make a general point insofar as that I think it
does not help users to introduce, or strengthen, a _third_ way to
describe "the state of the nation" (first being actual behaviour of the
code, second the documentation, potential third the -info-gdb-mi-command
output). 

I actually think this very thread proves the point. You said "Recently, I
added new coommands for Ada exception$ catching, but forgot about
-list-features" which means there _are_ builds of gdb which support the
feature, but don't announce it.

Anyway, to finish this: My fourth assumption was that the "[RFC]" in the
subject  was short-hand for "Request for comments" and that "interested
parties" were invited to comment. But I got it now.  

Regards,
Andre'

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [RFC] New GDB/MI command "-info-gdb-mi-command"
  2013-11-14  0:36         ` André Pönitz
@ 2013-11-14  9:48           ` Joel Brobecker
  2013-11-14 18:31             ` André Pönitz
  0 siblings, 1 reply; 42+ messages in thread
From: Joel Brobecker @ 2013-11-14  9:48 UTC (permalink / raw)
  To: André Pönitz; +Cc: Tom Tromey, gdb-patches

> Anyway, to finish this: My fourth assumption was that the "[RFC]" in the
> subject  was short-hand for "Request for comments" and that "interested
> parties" were invited to comment. But I got it now.

I actually welcome comments, and you were right about the meaning
of the "RFC". But receiving comments does not necessarily means
agreeing with all of them. We're having a discussion, and I am
trying to explain why I think the approach you are suggesting is
not as practical as the one I am proposing.

> > The first and most obvious to me is the case where the debugger is
> > run with a non-English LANG. If you base your detection on parsing
> > the error msg, then i18n ruins your plan.
> 
> For me the context was "frontends". I assume they run external tools
> in an environment that's as predictable as they need. If a user
> defined LANG is problematic for a frontend, I would assume the
> frontend forces debugger startup in a LANG that it knows to handle.

The problem with overriding the user's LANG settings, is that you
are essentially turning i18n off, thus forcing the user to see
all messages from the debugger in English. Many people find that
unacceptable, and I would agree with them. Besides, we've done
a fair amount of work to allow i18n, so it would be a shame to
see that turned off by a frontend, just to because they depend
on the wording of a specific error message (which may change, btw).

> This would mean that users of well-behaved gdb builds/installation
> lose one roundtrip, and the frontend needs to implement three funtions
> (ask, either, or) instead of two (call, fallback)

I agree that FEs shouldn't be in the business of verifying that
the underlying debugger is correctly built or installed. That's
not what the new command is about.

> I was indeeed trying to make a general point insofar as that I think it
> does not help users to introduce, or strengthen, a _third_ way to
> describe "the state of the nation" (first being actual behaviour of the
> code, second the documentation, potential third the -info-gdb-mi-command
> output). 

OK, I see. You're objecting to the concept itself, not the command.
My stance is that I have a different assessment of the situation.
I hope you'll understand why I personally think your first way
(behavior of the code) is not practical - you even had to quote
"works" when you proposed your approach; for the second (documentation),
I hope you mean "-list-features" and not the GDB manual, and I explained
why I think this is not going to scale well; that's why Tom proposed
the idea of this new command.

Remember also that I think the current design leaves the door open
for providing more information. For instance, we could let command
announce features added after the command creation.

> I actually think this very thread proves the point. You said
> "Recently, I added new coommands for Ada exception$ catching, but
> forgot about -list-features" which means there _are_ builds of gdb
> which support the feature, but don't announce it.

Very possible. But I am pretty sure that no FE actually uses those
features, yet - not even the AdaCore FE, since I haven't announced
the feature to them yet either. For those few builds, it's OK for
the FE to use the fallback mechanism.

-- 
Joel

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [RFC] New GDB/MI command "-info-gdb-mi-command"
  2013-11-14  9:48           ` Joel Brobecker
@ 2013-11-14 18:31             ` André Pönitz
  0 siblings, 0 replies; 42+ messages in thread
From: André Pönitz @ 2013-11-14 18:31 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: Tom Tromey, gdb-patches

On Thu, Nov 14, 2013 at 01:32:53PM +0400, Joel Brobecker wrote:
> > For me the context was "frontends". I assume they run external tools
> > in an environment that's as predictable as they need. If a user
> > defined LANG is problematic for a frontend, I would assume the
> > frontend forces debugger startup in a LANG that it knows to handle.
> 
> The problem with overriding the user's LANG settings, is that you
> are essentially turning i18n off, thus forcing the user to see
> all messages from the debugger in English. Many people find that
> unacceptable, and I would agree with them. Besides, we've done
> a fair amount of work to allow i18n, so it would be a shame to
> see that turned off by a frontend, just to because they depend
> on the wording of a specific error message (which may change, btw).

I think it's ok to let the frontend align debugger message output with the
needs of the frontend's users. This might be translated output, it might be
untranslated output, it might even be something re-phrased by the frontend.
Especially for frontends supporting more than one debugger backend, each
with different terminology, I see some value in using backend-agnostic
messaging.

It's nice that GDB provides translations, it's also nice that it gives
the option to be used untranslated.

[When I think about it, it's not even either-or. "Message based feature
discovery" could use an second untranslated debugger instance, while
the actual debugging runs in a trancelated instance...]

> > This would mean that users of well-behaved gdb builds/installation
> > lose one roundtrip, and the frontend needs to implement three funtions
> > (ask, either, or) instead of two (call, fallback)
> 
> I agree that FEs shouldn't be in the business of verifying that
> the underlying debugger is correctly built or installed. That's
> not what the new command is about.

Unless the frontend bundles a "verified" build of the debugger it has to
cope with what's installed in the system. That needs feature discovery, one
way or the other, including recognition of what it considers "bad"
installation, "bad" from the frontend's point of view, not necessarily
from the distribution's point of view. 

> > I was indeeed trying to make a general point insofar as that I think it
> > does not help users to introduce, or strengthen, a _third_ way to
> > describe "the state of the nation" (first being actual behaviour of the
> > code, second the documentation, potential third the -info-gdb-mi-command
> > output). 
> 
> OK, I see. You're objecting to the concept itself, not the command.
> My stance is that I have a different assessment of the situation.
> I hope you'll understand why I personally think your first way
> (behavior of the code) is not practical - you even had to quote
> "works" when you proposed your approach; for the second (documentation),
> I hope you mean "-list-features" and not the GDB manual,

I did mean the manual. It's now certainly much better aligned to the
implementation, especially for the MI bits, than it was at the 6.x times,
but there are still discrepancies every now and then.

Btw, one thing that I feel missing there are hints about the first
appearance of a feature. Even when a frontend is ready to limit the range
of supported versions of GDB', e.g. to "only >= 7.2" or "only >= 7.4"
it's really hard to find out whether it's "safe" to use a feature that's
mentioned in the manual.

> and I explained why I think this is not going to scale well; that's why
> Tom proposed the idea of this new command.

I think we agree that -list-features does not scale well, and that
-info-gdb-mi-command is conceptual better. We do not agree on the absolute
practical utility of the feature, but that's not a problem.  It was ...
just a comment.

Andre'

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [RFC] New GDB/MI command "-info-gdb-mi-command"
  2013-11-13  2:47       ` Joel Brobecker
  2013-11-14  0:36         ` André Pönitz
@ 2013-11-14 19:03         ` Pedro Alves
  2013-11-14 19:37           ` Pedro Alves
  1 sibling, 1 reply; 42+ messages in thread
From: Pedro Alves @ 2013-11-14 19:03 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: André Pönitz, Tom Tromey, gdb-patches

On 11/13/2013 02:15 AM, Joel Brobecker wrote:
>> I am not sure I agree with the judgement of benefits here. The basic 
>> > yes/no information is already there:
>> > 
>> >     (gdb) -unsupported-command
>> >     ^error,msg="Undefined MI command: unsupported-command"
>> >     (gdb) -symbol-list-lines
>> >     ^error,msg="-symbol-list-lines: Usage: SOURCE_FILENAME"
>> > 
>> > It's not nice, but "works".
> I disagree with your assessment of "works". I can think of a number
> of scenarios where this would be problematic:
> 
> The first and most obvious to me is the case where the debugger is
> run with a non-English LANG. If you base your detection on parsing
> the error msg, then i18n ruins your plan. And if you base your detection
> on the presence of the error alone, then commands that take no argument
> may return an error, which by no means indicates that the command does not
> exist.

Yeah.  I think that points out that errors like "Undefined MI command:" and
"Usage:" errors are in a different class of errors from errors caused
by user input though.  The former should never ever be seen by the user.
They're "internal" gdb<->frontend errors.  We could/should tag these
differently somehow, so that the frontend doesn't have to parse a
free form string.  Like:

 "^error,msg="..."
 "^error,msg="...",code="unknown-command"
 "^error,msg="...",code="usage"

or some such.

This does not invalidate listing features in -list-features, as
it's often useful to know upfront whether some feature is supported,
so the frontend can disable parts of the GUI that won't make sense
for the current target/session.

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [RFC] New GDB/MI command "-info-gdb-mi-command"
  2013-11-14 19:03         ` Pedro Alves
@ 2013-11-14 19:37           ` Pedro Alves
  2013-11-14 20:30             ` Tom Tromey
  0 siblings, 1 reply; 42+ messages in thread
From: Pedro Alves @ 2013-11-14 19:37 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: André Pönitz, Tom Tromey, gdb-patches

On 11/14/2013 06:44 PM, Pedro Alves wrote:

> Yeah.  I think that points out that errors like "Undefined MI command:" and
> "Usage:" errors are in a different class of errors from errors caused
> by user input though.  The former should never ever be seen by the user.
> They're "internal" gdb<->frontend errors.  We could/should tag these
> differently somehow, so that the frontend doesn't have to parse a
> free form string.  Like:
> 
>  "^error,msg="..."
>  "^error,msg="...",code="unknown-command"
>  "^error,msg="...",code="usage"
> 
> or some such.
> 
> This does not invalidate listing features in -list-features, as
> it's often useful to know upfront whether some feature is supported,
> so the frontend can disable parts of the GUI that won't make sense
> for the current target/session.
> 

Something like this:

-whatever
^error,msg="Undefined MI command: whatever",error="unknown-command"
(gdb)
-list-thread-groups --frame 1 --frame 1
^error,msg="Duplicate '--frame' option",error="command-usage"
(gdb)

Frontends must ignore unknown attributes, so it's backwards
compatible.

Just a POC.  Of course, we'd have to go audit all MI "error" calls.

---

 gdb/exceptions.h  |    6 ++++++
 gdb/mi/mi-main.c  |   13 ++++++++++++-
 gdb/mi/mi-parse.c |   20 +++++++++++++-------
 3 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/gdb/exceptions.h b/gdb/exceptions.h
index 129d29a..07599ae 100644
--- a/gdb/exceptions.h
+++ b/gdb/exceptions.h
@@ -93,6 +93,12 @@ enum errors {
      aborted as the inferior state is no longer valid.  */
   TARGET_CLOSE_ERROR,

+  /* An unknown command was executed.  */
+  UNKNOWN_COMMAND_ERROR,
+
+  /* Wrong command usage.  */
+  COMMAND_USAGE_ERROR,
+
   /* Add more errors here.  */
   NR_ERRORS
 };
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index bbf944a..c2a3988 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -2021,7 +2021,18 @@ mi_print_exception (const char *token, struct gdb_exception exception)
     fputs_unfiltered ("unknown error", raw_stdout);
   else
     fputstr_unfiltered (exception.message, '"', raw_stdout);
-  fputs_unfiltered ("\"\n", raw_stdout);
+  fputs_unfiltered ("\"", raw_stdout);
+
+  switch (exception.error)
+    {
+      case UNKNOWN_COMMAND_ERROR:
+	fputs_unfiltered (",error=\"unknown-command\"", raw_stdout);
+	break;
+      case COMMAND_USAGE_ERROR:
+	fputs_unfiltered (",error=\"command-usage\"", raw_stdout);
+	break;
+    }
+  fputs_unfiltered ("\n", raw_stdout);
 }

 void
diff --git a/gdb/mi/mi-parse.c b/gdb/mi/mi-parse.c
index 9994307..2c9d267 100644
--- a/gdb/mi/mi-parse.c
+++ b/gdb/mi/mi-parse.c
@@ -285,7 +285,8 @@ mi_parse (const char *cmd, char **token)
   /* Find the command in the MI table.  */
   parse->cmd = mi_lookup (parse->command);
   if (parse->cmd == NULL)
-    error (_("Undefined MI command: %s"), parse->command);
+    throw_error (UNKNOWN_COMMAND_ERROR,
+		 _("Undefined MI command: %s"), parse->command);

   /* Skip white space following the command.  */
   chp = skip_spaces_const (chp);
@@ -324,7 +325,8 @@ mi_parse (const char *cmd, char **token)

 	  option = "--thread-group";
 	  if (parse->thread_group != -1)
-	    error (_("Duplicate '--thread-group' option"));
+	    throw_error (COMMAND_USAGE_ERROR,
+			 _("Duplicate '--thread-group' option"));
 	  chp += tgs;
 	  if (*chp != 'i')
 	    error (_("Invalid thread group id"));
@@ -338,7 +340,8 @@ mi_parse (const char *cmd, char **token)

 	  option = "--thread";
 	  if (parse->thread != -1)
-	    error (_("Duplicate '--thread' option"));
+	    throw_error (COMMAND_USAGE_ERROR,
+			 _("Duplicate '--thread' option"));
 	  chp += ts;
 	  parse->thread = strtol (chp, &endp, 10);
 	  chp = endp;
@@ -349,7 +352,8 @@ mi_parse (const char *cmd, char **token)

 	  option = "--frame";
 	  if (parse->frame != -1)
-	    error (_("Duplicate '--frame' option"));
+	    throw_error (COMMAND_USAGE_ERROR,
+			 _("Duplicate '--frame' option"));
 	  chp += fs;
 	  parse->frame = strtol (chp, &endp, 10);
 	  chp = endp;
@@ -367,7 +371,8 @@ mi_parse (const char *cmd, char **token)
 	  parse->language = language_enum (lang_name);
 	  if (parse->language == language_unknown
 	      || parse->language == language_auto)
-	    error (_("Invalid --language argument: %s"), lang_name);
+	    throw_error (COMMAND_USAGE_ERROR,
+			 _("Invalid --language argument: %s"), lang_name);

 	  do_cleanups (old_chain);
 	}
@@ -414,7 +419,8 @@ mi_parse_print_values (const char *name)
 	    || strcmp (name, mi_simple_values) == 0)
      return PRINT_SIMPLE_VALUES;
    else
-     error (_("Unknown value for PRINT_VALUES: must be: \
+     throw_error (COMMAND_USAGE_ERROR,
+		  _("Unknown value for PRINT_VALUES: must be: \
 0 or \"%s\", 1 or \"%s\", 2 or \"%s\""),
-	    mi_no_values, mi_all_values, mi_simple_values);
+		  mi_no_values, mi_all_values, mi_simple_values);
 }

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [RFC] New GDB/MI command "-info-gdb-mi-command"
  2013-11-14 19:37           ` Pedro Alves
@ 2013-11-14 20:30             ` Tom Tromey
  2013-11-15  5:35               ` Joel Brobecker
  0 siblings, 1 reply; 42+ messages in thread
From: Tom Tromey @ 2013-11-14 20:30 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Joel Brobecker, André Pönitz, gdb-patches

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> Just a POC.  Of course, we'd have to go audit all MI "error" calls.

It seems like a reasonable idea to me.

Tom

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [RFC] New GDB/MI command "-info-gdb-mi-command"
  2013-11-14 20:30             ` Tom Tromey
@ 2013-11-15  5:35               ` Joel Brobecker
  2013-11-15 12:39                 ` Pedro Alves
  0 siblings, 1 reply; 42+ messages in thread
From: Joel Brobecker @ 2013-11-15  5:35 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Pedro Alves, André Pönitz, gdb-patches

> Pedro> Just a POC.  Of course, we'd have to go audit all MI "error" calls.
> 
> It seems like a reasonable idea to me.

The idea of a specific and documented error code seems much more robust
to me.

Regarding invalid switches, we may have to extend the current proposal
to allow the command to specific what in the usage caused problem?
In my proposal, it was easy to extend by adding a "feature=[...]"
list to the output. Or maybe that's overkill? Or use list-features
for that instead?

I'd like us to decide to something I can go and implement. Either way,
I think we can start by concentrating with the initial goal, which is
to determine whether a command exists or not. People seem to have reacted
more positively to the idea of try-and-fallback approach, shall we go
with Pedro's idea (without the "invalid switch"/"usage" part)?

Thanks,
-- 
Joel

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [RFC] New GDB/MI command "-info-gdb-mi-command"
  2013-11-15  5:35               ` Joel Brobecker
@ 2013-11-15 12:39                 ` Pedro Alves
  2013-11-15 14:38                   ` Joel Brobecker
  0 siblings, 1 reply; 42+ messages in thread
From: Pedro Alves @ 2013-11-15 12:39 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: Tom Tromey, André Pönitz, gdb-patches

On 11/15/2013 03:30 AM, Joel Brobecker wrote:
>> Pedro> Just a POC.  Of course, we'd have to go audit all MI "error" calls.
>>
>> It seems like a reasonable idea to me.
> 
> The idea of a specific and documented error code seems much more robust
> to me.
> 
> Regarding invalid switches, we may have to extend the current proposal
> to allow the command to specific what in the usage caused problem?

Not sure about that.  Sounds more complicated than it's worth it.

> In my proposal, it was easy to extend by adding a "feature=[...]"
> list to the output. Or maybe that's overkill? Or use list-features
> for that instead?

As list-features already exists, and works just as well, that might
indeed be overkill.  Or put another way, is there a use case that
list-features doesn't cover, or something about "feature=[]"
that'd make ours and frontend writers' lives easier?  Just like with
list-features, we'd always have to manually take care of listing the
new command feature in "features=[]", so on our end it doesn't seem
to buy anything.
IOW, thinking in terms of KISS seems to suggest sticking with
list-features.

> I'd like us to decide to something I can go and implement. Either way,
> I think we can start by concentrating with the initial goal, which is
> to determine whether a command exists or not.

Yeah.  I have no problem with your proposal.  There's actually one
case where it works, and '^error,code="unknown-command"' does not,
which is when a command works and has effects without options.  In such
cases, you can't probe for the command's existence without causing
the (side) effects.

> People seem to have reacted
> more positively to the idea of try-and-fallback approach, shall we go
> with Pedro's idea (without the "invalid switch"/"usage" part)?

If I had infinite time, I'd go for all of the above.  Command to
probe existence of commands, and make ^error indicate both
unknown command, and bad usage.  :-)

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [RFC] New GDB/MI command "-info-gdb-mi-command"
  2013-11-15 12:39                 ` Pedro Alves
@ 2013-11-15 14:38                   ` Joel Brobecker
  2013-11-15 14:40                     ` Pedro Alves
  0 siblings, 1 reply; 42+ messages in thread
From: Joel Brobecker @ 2013-11-15 14:38 UTC (permalink / raw)
  To: Pedro Alves; +Cc: Tom Tromey, André Pönitz, gdb-patches

> > Regarding invalid switches, we may have to extend the current proposal
> > to allow the command to specific what in the usage caused problem?
> 
> Not sure about that.  Sounds more complicated than it's worth it.
> 
> > In my proposal, it was easy to extend by adding a "feature=[...]"
> > list to the output. Or maybe that's overkill? Or use list-features
> > for that instead?
> 
> As list-features already exists, and works just as well, that might
> indeed be overkill.  Or put another way, is there a use case that
> list-features doesn't cover, or something about "feature=[]"
> that'd make ours and frontend writers' lives easier?  Just like with
> list-features, we'd always have to manually take care of listing the
> new command feature in "features=[]", so on our end it doesn't seem
> to buy anything.
> IOW, thinking in terms of KISS seems to suggest sticking with
> list-features.

OK, I think will work well enough in practice, and, really, worrying
about a few more bytes at a time was a bit of an overreaction :).

> > I'd like us to decide to something I can go and implement. Either way,
> > I think we can start by concentrating with the initial goal, which is
> > to determine whether a command exists or not.
> 
> Yeah.  I have no problem with your proposal.  There's actually one
> case where it works, and '^error,code="unknown-command"' does not,
> which is when a command works and has effects without options.  In such
> cases, you can't probe for the command's existence without causing
> the (side) effects.

I think the intent was not to provide a probing mechanism, but
rather to provide an approach where the FE just fires the command
when it needs to, and then fallback on a CLI-based approach if
detecting an 'unknown-command' error.

But, on the other hand, I am thinking that some FEs might still
want to probe ahead of time, for instance if they do not wish to
provide a fallback mechanism (thus disabling the relevant parts
of the GUI ahead of time); or even if it is easier programatically
for them to probe, instead of having to handle this specific error.

> > People seem to have reacted
> > more positively to the idea of try-and-fallback approach, shall we go
> > with Pedro's idea (without the "invalid switch"/"usage" part)?
> 
> If I had infinite time, I'd go for all of the above.  Command to
> probe existence of commands, and make ^error indicate both
> unknown command, and bad usage.  :-)

I don't have infinite amount of time, but the first 2 (new GDB/MI
command and new ^error for unknown commands) are fairly small tasks,
so I'm happy sending patches for both. That way, we get the best
of both worlds, without must cost, I think, in terms of extra
maintenance, since both patches would be pretty small, and localized.

For invalid usage, I could add that to my list, but that'll have
to be next year... (/me wishes I would say that on Dec 31st...)

-- 
Joel

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [RFC] New GDB/MI command "-info-gdb-mi-command"
  2013-11-15 14:38                   ` Joel Brobecker
@ 2013-11-15 14:40                     ` Pedro Alves
  2013-11-18 17:12                       ` [RFA GDB/MI] Help determine if GDB/MI command exists or not Joel Brobecker
  0 siblings, 1 reply; 42+ messages in thread
From: Pedro Alves @ 2013-11-15 14:40 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: Tom Tromey, André Pönitz, gdb-patches

On 11/15/2013 12:39 PM, Joel Brobecker wrote:

>> Yeah.  I have no problem with your proposal.  There's actually one
>> case where it works, and '^error,code="unknown-command"' does not,
>> which is when a command works and has effects without options.  In such
>> cases, you can't probe for the command's existence without causing
>> the (side) effects.
> 
> I think the intent was not to provide a probing mechanism, but
> rather to provide an approach where the FE just fires the command
> when it needs to, and then fallback on a CLI-based approach if
> detecting an 'unknown-command' error.

Yeah.  Just thinking about how we'd cover all bases if we took
only one approach.

> But, on the other hand, I am thinking that some FEs might still
> want to probe ahead of time, for instance if they do not wish to
> provide a fallback mechanism (thus disabling the relevant parts
> of the GUI ahead of time); 

Right, that's the reasoning I usually throw around too.
It's the same reasoning we probe things in the RSP upfront
with qSupported.  I now notice that the -list-features docu
doesn't talk about that explicitly, but it could be nice to
suggest it.

> or even if it is easier programatically
> for them to probe, instead of having to handle this specific error.

>>> People seem to have reacted
>>> more positively to the idea of try-and-fallback approach, shall we go
>>> with Pedro's idea (without the "invalid switch"/"usage" part)?
>>
>> If I had infinite time, I'd go for all of the above.  Command to
>> probe existence of commands, and make ^error indicate both
>> unknown command, and bad usage.  :-)
> 
> I don't have infinite amount of time, but the first 2 (new GDB/MI
> command and new ^error for unknown commands) are fairly small tasks,
> so I'm happy sending patches for both. That way, we get the best
> of both worlds, without must cost, I think, in terms of extra
> maintenance, since both patches would be pretty small, and localized.

That sounds good to me.

> For invalid usage, I could add that to my list, but that'll have
> to be next year... (/me wishes I would say that on Dec 31st...)

:-)

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 42+ messages in thread

* [RFA GDB/MI] Help determine if GDB/MI command exists or not
  2013-11-15 14:40                     ` Pedro Alves
@ 2013-11-18 17:12                       ` Joel Brobecker
  2013-11-18 17:13                         ` [RFA 1/2] New GDB/MI command "-info-gdb-mi-command" Joel Brobecker
                                           ` (2 more replies)
  0 siblings, 3 replies; 42+ messages in thread
From: Joel Brobecker @ 2013-11-18 17:12 UTC (permalink / raw)
  To: gdb-patches

Hello,

Re: http://www.sourceware.org/ml/gdb-patches/2013-11/msg00382.html

This series introduces 2 patches meant to help front-ends determine
whether a given GDB/MI command exists or not.

  - Patch #1 introduces a new command "-info-gdb-mi-command"
    The patch is very close to what was proposed in the original
    RFC, with only minor corrections, based on feedback received then;
    It's basically document changes, IIRC.

  - Path #2 implements Pedro's idea of adding an error code to
    the "^error" result record.

    I took Pedro's patch nearly verbatim, removing the bits that
    dealt with invalid command-line usage (this part is left for
    another time, if the need becomes a little more explicit).
    I did notice that the additonal variable looked an awful lot
    like an error code, so I found it odd that we'd name if "error="
    in the patch. And then I realized that Pedro's first email did
    say "code", so I assumed it was a brain fart, and fixed the patch
    to use "code".

    Additional remarks on that patch inside the patch's revision log
    (to be part of the commit when eventually pushed).

    I added NEWS, doco, and testcase as well.

Thank you,
-- 
Joel

^ permalink raw reply	[flat|nested] 42+ messages in thread

* [RFA 1/2] New GDB/MI command "-info-gdb-mi-command"
  2013-11-18 17:12                       ` [RFA GDB/MI] Help determine if GDB/MI command exists or not Joel Brobecker
@ 2013-11-18 17:13                         ` Joel Brobecker
  2013-11-18 17:29                           ` Eli Zaretskii
  2013-11-18 17:21                         ` [RFA 2/2] Add "undefined-command" error code at end of ^error result Joel Brobecker
  2013-11-19 15:05                         ` [RFA GDB/MI] Help determine if GDB/MI command exists or not Pedro Alves
  2 siblings, 1 reply; 42+ messages in thread
From: Joel Brobecker @ 2013-11-18 17:13 UTC (permalink / raw)
  To: gdb-patches

Hello,

This is mostly a re-based version of:
    [RFC] New GDB/MI command "-info-gdb-mi-command"
    http://www.sourceware.org/ml/gdb-patches/2013-11/msg00311.html

I made the documentation fixes mentioned by Eli, although I wonder
if the corrections were done well enough:
  - Added the @cindex entry for the new command;
  - Fixed the markup for the command's argument.
I think it's worth a second review - I am not sure I did enough.

Regarding some questions Eli had:

  | > +(the leading dash (@code{-}) in the command name should be omitted).
  | Is this wise?  How about if we support both with and without the dash?

I now think that it was indeed the correct choice.  Not only does it
facilitate implementation (but only marginally), it also is consistent
with the current output.  For instance, notice how GDB names the command
in the following error message:

    -unsupported
    ^error,msg="Undefined MI command: unsupported"
                                      ^^^^^^^^^^^
                                    (no leading dash)

Also, looking at the grammar, the leading dash isn't listed
as part of what they call the "operation"

    mi-command ==>
    [ token ] "-" operation ( " " option )* [ " --" ] ( " " parameter )* nl

The other question was:

  | > +-info-gdb-mi-command symbol-list-lines
  | > +^done,command=@{exists="true"@}
  | > +@end smallexample
  |
  | Btw, why "command="?  Perhaps "result="?

I only have a mild opinion. But the choice I made seems consistent
with other commands. Eg:
  + -list-features => features=
  + -thread-info => threads=
  + -list-thread-groups => groups=

~~~ Actual Commit's Revision Log ~~~

This patch adds a new GDB/MI command meant for graphical frontends
trying to determine whether a given GDB/MI command exists or not.

Examples:

    -info-gdb-mi-command unsupported-command
    ^done,command={exists="false"}
    (gdb)
    -info-gdb-mi-command symbol-list-lines
    ^done,command={exists="true"}
    (gdb)

At the moment, this is the only piece of information that this
command returns.

Eventually, and if needed, we can extend it to provide
command-specific pieces of information, such as updates to
the command's syntax since inception.  This could become,
for instance:

    -info-gdb-mi-command symbol-list-lines
    ^done,command={exists="true",features=[]}
    (gdb)
    -info-gdb-mi-command catch-assert
    ^done,command={exists="true",features=["conditions"]}

In the first case, it would mean that no extra features,
while in the second, it announces that the -catch-assert
command in this version of the debugger supports a feature
called "condition" - exact semantics to be documented with
combined with the rest of the queried command's documentation.

But for now, we start small, and only worry about existance.
And to bootstrap the process, I have added an entry in the
output of the -list-features command as well ("info-gdb-mi-command"),
allowing the graphical frontends to go through the following process:

  1. Send -list-features, collect info from there as before;
  2. Check if the output contains "info-gdb-mi-command".
     If it does, then support for various commands can be
     queried though -info-gdb-mi-command. Newer commands
     will be expected to always be checked via this new
     -info-gdb-mi-command.

gdb/ChangeLog:

        * mi/mi-cmds.h (mi_cmd_info_gdb_mi_command): Declare.
        * mi/mi-cmd-info.c (mi_cmd_info_gdb_mi_command): New function.
        * mi/mi-cmds.c (mi_cmds): Add -info-gdb-mi-command command.
        * mi/mi-main.c (mi_cmd_list_features): Add "info-gdb-mi-command"
        field to output of "-list-features".

        * NEWS: Add entry for new -info-gdb-mi-command.

gdb/doc/ChangeLog:

        * gdb.texinfo (GDB/MI Miscellaneous Commands): Document
        the new -info-gdb-mi-command GDB/MI command.  Document
        the meaning of "-info-gdb-mi-command" in the output of
        -list-features.

gdb/testsuite/ChangeLog:

        * gdb.mi/mi-i-cmd.exp: New file.

Tested on x86_64-linux.  OK to commit?

Thank you,
-- 
Joel

---
 gdb/NEWS                          |  3 +++
 gdb/doc/gdb.texinfo               | 47 +++++++++++++++++++++++++++++++++++++++
 gdb/mi/mi-cmd-info.c              | 21 +++++++++++++++++
 gdb/mi/mi-cmds.c                  |  1 +
 gdb/mi/mi-cmds.h                  |  1 +
 gdb/mi/mi-main.c                  |  1 +
 gdb/testsuite/gdb.mi/mi-i-cmd.exp | 37 ++++++++++++++++++++++++++++++
 7 files changed, 111 insertions(+)
 create mode 100644 gdb/testsuite/gdb.mi/mi-i-cmd.exp

diff --git a/gdb/NEWS b/gdb/NEWS
index 9fc3638..e61c79f 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -153,6 +153,9 @@ show startup-with-shell
 
   ** All MI commands now accept an optional "--language" option.
 
+  ** The new command -info-gdb-mi-command allows the user to determine
+     whether a GDB/MI command is supported or not.
+
   ** The -trace-save MI command can optionally save trace buffer in Common
      Trace Format now.
 
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 19e9aa5..e85b5b6 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -35069,6 +35069,51 @@ default shows this information when you start an interactive session.
 (gdb)
 @end smallexample
 
+@subheading The @code{-info-gdb-mi-command} Command
+@cindex @code{-info-gdb-mi-command}
+@findex -info-gdb-mi-command
+
+@subsubheading Synopsis
+
+@smallexample
+ -info-gdb-mi-command @var{cmd_name}
+@end smallexample
+
+Query support for the @sc{gdb/mi} command named @var{cmd_name}
+(the leading dash (@code{-}) in the command name should be omitted).
+
+@subsubheading @value{GDBN} Command
+
+There is no corresponding @value{GDBN} command.
+
+@subsubheading Result
+
+The result is a tuple.  There is currently only one field:
+
+@table @samp
+@item exists
+This field is equal to @code{"true"} if the @sc{gdb/mi} command exists,
+@code{"false"} otherwise.
+
+@end table
+
+@subsubheading Example
+
+Here is an example where the @sc{gdb/mi} command does not exist:
+
+@smallexample
+-info-gdb-mi-command unsupported-command
+^done,command=@{exists="false"@}
+@end smallexample
+
+And here is an example where the @sc{gdb/mi} command is known
+to the debugger:
+
+@smallexample
+-info-gdb-mi-command symbol-list-lines
+^done,command=@{exists="true"@}
+@end smallexample
+
 @subheading The @code{-list-features} Command
 @findex -list-features
 
@@ -35122,6 +35167,8 @@ exceptions: @code{-info-ada-exceptions}, @code{-catch-assert} and
 @item language-option
 Indicates that all @sc{gdb/mi} commands accept the @option{--language}
 option (@pxref{Context management}).
+@item info-gdb-mi-command
+Indicates support for the @code{-info-gdb-mi-command} command.
 @end table
 
 @subheading The @code{-list-target-features} Command
diff --git a/gdb/mi/mi-cmd-info.c b/gdb/mi/mi-cmd-info.c
index aa4d210..18f4927 100644
--- a/gdb/mi/mi-cmd-info.c
+++ b/gdb/mi/mi-cmd-info.c
@@ -71,6 +71,27 @@ mi_cmd_info_ada_exceptions (char *command, char **argv, int argc)
   do_cleanups (old_chain);
 }
 
+/* Implement the "-info-gdb-mi-command" GDB/MI command.  */
+
+void
+mi_cmd_info_gdb_mi_command (char *command, char **argv, int argc)
+{
+  const char *cmd_name;
+  struct mi_cmd *cmd;
+  struct ui_out *uiout = current_uiout;
+  struct cleanup *old_chain;
+
+  /* This command takes exactly one argument.  */
+  if (argc != 1)
+    error (_("Usage: -info-gdb-mi-command MI_COMMAND_NAME"));
+  cmd_name = argv[0];
+  cmd = mi_lookup (cmd_name);
+
+  old_chain = make_cleanup_ui_out_tuple_begin_end (uiout, "command");
+  ui_out_field_string (uiout, "exists", cmd != NULL ? "true" : "false");
+  do_cleanups (old_chain);
+}
+
 void
 mi_cmd_info_os (char *command, char **argv, int argc)
 {
diff --git a/gdb/mi/mi-cmds.c b/gdb/mi/mi-cmds.c
index 496a8aa..aed62b2 100644
--- a/gdb/mi/mi-cmds.c
+++ b/gdb/mi/mi-cmds.c
@@ -125,6 +125,7 @@ static struct mi_cmd mi_cmds[] =
   DEF_MI_CMD_MI ("inferior-tty-set", mi_cmd_inferior_tty_set),
   DEF_MI_CMD_MI ("inferior-tty-show", mi_cmd_inferior_tty_show),
   DEF_MI_CMD_MI ("info-ada-exceptions", mi_cmd_info_ada_exceptions),
+  DEF_MI_CMD_MI ("info-gdb-mi-command", mi_cmd_info_gdb_mi_command),
   DEF_MI_CMD_MI ("info-os", mi_cmd_info_os),
   DEF_MI_CMD_MI ("interpreter-exec", mi_cmd_interpreter_exec),
   DEF_MI_CMD_MI ("list-features", mi_cmd_list_features),
diff --git a/gdb/mi/mi-cmds.h b/gdb/mi/mi-cmds.h
index cb8aac1..4ea95fa 100644
--- a/gdb/mi/mi-cmds.h
+++ b/gdb/mi/mi-cmds.h
@@ -74,6 +74,7 @@ extern mi_cmd_argv_ftype mi_cmd_gdb_exit;
 extern mi_cmd_argv_ftype mi_cmd_inferior_tty_set;
 extern mi_cmd_argv_ftype mi_cmd_inferior_tty_show;
 extern mi_cmd_argv_ftype mi_cmd_info_ada_exceptions;
+extern mi_cmd_argv_ftype mi_cmd_info_gdb_mi_command;
 extern mi_cmd_argv_ftype mi_cmd_info_os;
 extern mi_cmd_argv_ftype mi_cmd_interpreter_exec;
 extern mi_cmd_argv_ftype mi_cmd_list_features;
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 83d524a..e4251c9 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1817,6 +1817,7 @@ mi_cmd_list_features (char *command, char **argv, int argc)
       ui_out_field_string (uiout, NULL, "ada-task-info");
       ui_out_field_string (uiout, NULL, "ada-exceptions");
       ui_out_field_string (uiout, NULL, "language-option");
+      ui_out_field_string (uiout, NULL, "info-gdb-mi-command");
       
 #if HAVE_PYTHON
       if (gdb_python_initialized)
diff --git a/gdb/testsuite/gdb.mi/mi-i-cmd.exp b/gdb/testsuite/gdb.mi/mi-i-cmd.exp
new file mode 100644
index 0000000..5285d31
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-i-cmd.exp
@@ -0,0 +1,37 @@
+# Copyright 2013 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+gdb_exit
+if [mi_gdb_start] {
+    continue
+}
+
+# First, verify that the debugger correctly advertises support
+# for the -info-gdb-mi-command command.
+mi_gdb_test "-list-features" \
+            "\\^done,features=\\\[.*\"info-gdb-mi-command\".*\\\]" \
+            "-list-features should include \"info-gdb-mi-command\""
+
+mi_gdb_test "-info-gdb-mi-command unsupported-command" \
+            "\\^done,command=\\\{exists=\"false\"\\\}" \
+            "-info-gdb-mi-command unsupported-command"
+
+mi_gdb_test "-info-gdb-mi-command symbol-list-lines" \
+            "\\^done,command=\\\{exists=\"true\"\\\}" \
+            "-info-gdb-mi-command symbol-list-lines"
+
-- 
1.8.1.2

^ permalink raw reply	[flat|nested] 42+ messages in thread

* [RFA 2/2] Add "undefined-command" error code at end of ^error result...
  2013-11-18 17:12                       ` [RFA GDB/MI] Help determine if GDB/MI command exists or not Joel Brobecker
  2013-11-18 17:13                         ` [RFA 1/2] New GDB/MI command "-info-gdb-mi-command" Joel Brobecker
@ 2013-11-18 17:21                         ` Joel Brobecker
  2013-11-18 17:29                           ` Eli Zaretskii
  2013-11-19 11:19                           ` Pedro Alves
  2013-11-19 15:05                         ` [RFA GDB/MI] Help determine if GDB/MI command exists or not Pedro Alves
  2 siblings, 2 replies; 42+ messages in thread
From: Joel Brobecker @ 2013-11-18 17:21 UTC (permalink / raw)
  To: gdb-patches

... when trying to execute an undefined GDB/MI command. When trying
to execute a GDB/MI command which does not exist, the current error
result record looks like this:

    -unsupported
    ^error,msg="Undefined MI command: unsupported"

The only indication that the command does not exist is the error
message. It would be a little fragile for a consumer to rely solely
on the contents of the error message in order to determine whether
a command exists or not.

This patch improves the situation by adding concept of error
code, starting with one well-defined error code ("undefined-command")
identifying errors due to a non-existant command. Here is the new
output:

    -unsupported
    ^error,msg="Undefined MI command: unsupported",code="undefined-command"

This error code is only displayed when the corresponding error
condition is met. Otherwise, the error record remains unchanged.
For instance:

    -symbol-list-lines foo.adb
    ^error,msg="-symbol-list-lines: Unknown source file name."

For frontends to be able to know whether they can rely on this
variable, a new entry "undefined-command-error-code" has been
added to the "-list-features" command.  Another option would be
to always generate an error="..." variable (for the default case,
we could decide for instance that the error code is the empty string).
But it seems more efficient to provide that info in "-list-features"
and then only add the error code when meaningful.

gdb/ChangeLog:

        (from Pedro Alves  <palves@redhat.com>)
        (from Joel Brobecker  <brobecker@adacore.com>)
        * exceptions.h (enum_errors) <UNKNOWN_COMMAND_ERROR>: New enum.
        * mi/mi-parse.c (mi_parse): Thow UNKNOWN_COMMAND_ERROR instead
        of a regular error when the GDB/MI command does not exist.
        * mi/mi-main.c (mi_cmd_list_features): Add
        "undefined-command-error-code".
        (mi_print_exception): Print an "undefined-command"
        error code if EXCEPTION.ERROR in UNKNOWN_COMMAND_ERROR.
        * NEWS: Add entry documenting the new "code" variable in
        "^error" result records.

gdb/doc/ChangeLog:

        * gdb.texinfo (GDB/MI Result Records): Fix the syntax of the
        "^error" result record concerning the error message.  Document
        the error code that may also be part of that result record.
        (GDB/MI Miscellaneous Commands): Document the
        "undefined-command-error-code" element in the output of
        the "-list-features" GDB/MI command.

gdb/testsuite/ChangeLog:

        * gdb.mi/mi-undefined-cmd.exp: New testcase.

Tested on x86_64-linux.  OK to commit?
(hmmm, now that I have spent all that time typing everything up,
I am wondering if I should rename UNKNOWN_COMMAND_ERROR into
UNDEFINED_COMMAND_ERROR - no real biggie either way...)

Thanks!
-- 
Joel

---
 gdb/NEWS                                  |  4 ++++
 gdb/doc/gdb.texinfo                       | 17 ++++++++++++++--
 gdb/exceptions.h                          |  3 +++
 gdb/mi/mi-main.c                          | 12 ++++++++++-
 gdb/mi/mi-parse.c                         |  3 ++-
 gdb/testsuite/gdb.mi/mi-undefined-cmd.exp | 33 +++++++++++++++++++++++++++++++
 6 files changed, 68 insertions(+), 4 deletions(-)
 create mode 100644 gdb/testsuite/gdb.mi/mi-undefined-cmd.exp

diff --git a/gdb/NEWS b/gdb/NEWS
index e61c79f..d22c56a 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -156,6 +156,10 @@ show startup-with-shell
   ** The new command -info-gdb-mi-command allows the user to determine
      whether a GDB/MI command is supported or not.
 
+  ** The "^error" result record returned when trying to execute an undefined
+     GDB/MI command now provides a variable named "code" whose content is the
+     "undefined-command" error code.
+
   ** The -trace-save MI command can optionally save trace buffer in Common
      Trace Format now.
 
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index e85b5b6..f0662ff 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -29315,11 +29315,20 @@ which threads are resumed.
 @findex ^connected
 @value{GDBN} has connected to a remote target.
 
-@item "^error" "," @var{c-string}
+@item "^error" "," "msg=" @var{c-string} [ "," "code=" @var{c-string} ]
 @findex ^error
-The operation failed.  The @code{@var{c-string}} contains the corresponding
+The operation failed.  The @var{msg} variable contains the corresponding
 error message.
 
+If present, the @var{code} variable provides an error code on which
+consumers can rely in order to detect the corresponding error condition.
+At present, only one error code is defined:
+
+@table @samp
+@item "undefined-command"
+Indicates that the command causing the error does not exist.
+@end table
+
 @item "^exit"
 @findex ^exit
 @value{GDBN} has terminated.
@@ -35169,6 +35178,10 @@ Indicates that all @sc{gdb/mi} commands accept the @option{--language}
 option (@pxref{Context management}).
 @item info-gdb-mi-command
 Indicates support for the @code{-info-gdb-mi-command} command.
+@item undefined-command-error-code
+Indicates support for the "undefined-command" error code in error result
+records, produced when trying to execute an undefined @sc{gdb/mi} command
+(@pxref{GDB/MI Result Records}).
 @end table
 
 @subheading The @code{-list-target-features} Command
diff --git a/gdb/exceptions.h b/gdb/exceptions.h
index 129d29a..d9be897 100644
--- a/gdb/exceptions.h
+++ b/gdb/exceptions.h
@@ -93,6 +93,9 @@ enum errors {
      aborted as the inferior state is no longer valid.  */
   TARGET_CLOSE_ERROR,
 
+  /* An unknown command was executed.  */
+  UNKNOWN_COMMAND_ERROR,
+
   /* Add more errors here.  */
   NR_ERRORS
 };
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index e4251c9..53075af 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1818,6 +1818,7 @@ mi_cmd_list_features (char *command, char **argv, int argc)
       ui_out_field_string (uiout, NULL, "ada-exceptions");
       ui_out_field_string (uiout, NULL, "language-option");
       ui_out_field_string (uiout, NULL, "info-gdb-mi-command");
+      ui_out_field_string (uiout, NULL, "undefined-command-error-code");
       
 #if HAVE_PYTHON
       if (gdb_python_initialized)
@@ -2023,7 +2024,16 @@ mi_print_exception (const char *token, struct gdb_exception exception)
     fputs_unfiltered ("unknown error", raw_stdout);
   else
     fputstr_unfiltered (exception.message, '"', raw_stdout);
-  fputs_unfiltered ("\"\n", raw_stdout);
+  fputs_unfiltered ("\"", raw_stdout);
+
+  switch (exception.error)
+    {
+      case UNKNOWN_COMMAND_ERROR:
+	fputs_unfiltered (",code=\"undefined-command\"", raw_stdout);
+	break;
+    }
+
+  fputs_unfiltered ("\n", raw_stdout);
 }
 
 void
diff --git a/gdb/mi/mi-parse.c b/gdb/mi/mi-parse.c
index 9994307..3bd7400 100644
--- a/gdb/mi/mi-parse.c
+++ b/gdb/mi/mi-parse.c
@@ -285,7 +285,8 @@ mi_parse (const char *cmd, char **token)
   /* Find the command in the MI table.  */
   parse->cmd = mi_lookup (parse->command);
   if (parse->cmd == NULL)
-    error (_("Undefined MI command: %s"), parse->command);
+    throw_error (UNKNOWN_COMMAND_ERROR,
+		 _("Undefined MI command: %s"), parse->command);
 
   /* Skip white space following the command.  */
   chp = skip_spaces_const (chp);
diff --git a/gdb/testsuite/gdb.mi/mi-undefined-cmd.exp b/gdb/testsuite/gdb.mi/mi-undefined-cmd.exp
new file mode 100644
index 0000000..8df0a76
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-undefined-cmd.exp
@@ -0,0 +1,33 @@
+# Copyright 2013 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+gdb_exit
+if [mi_gdb_start] {
+    continue
+}
+
+
+# First, verify that the debugger correctly advertises support
+# for the "undefined-command" error code...
+mi_gdb_test "-list-features" \
+            "\\^done,features=\\\[.*\"undefined-command-error-code\".*\\\]" \
+            "-list-features should include \"undefined-command-error-code\""
+
+mi_gdb_test "-undefined-command" \
+            "\\^error,.*,code=\"undefined-command\"" \
+            "error code when executing undefined command"
-- 
1.8.1.2

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [RFA 2/2] Add "undefined-command" error code at end of ^error result...
  2013-11-18 17:21                         ` [RFA 2/2] Add "undefined-command" error code at end of ^error result Joel Brobecker
@ 2013-11-18 17:29                           ` Eli Zaretskii
  2013-11-19  6:02                             ` Joel Brobecker
  2013-11-19 11:19                           ` Pedro Alves
  1 sibling, 1 reply; 42+ messages in thread
From: Eli Zaretskii @ 2013-11-18 17:29 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

> From: Joel Brobecker <brobecker@adacore.com>
> Date: Mon, 18 Nov 2013 21:11:59 +0400
> 
> This patch improves the situation by adding concept of error
> code, starting with one well-defined error code ("undefined-command")
> identifying errors due to a non-existant command. Here is the new
> output:
> 
>     -unsupported
>     ^error,msg="Undefined MI command: unsupported",code="undefined-command"
> 
> This error code is only displayed when the corresponding error
> condition is met. Otherwise, the error record remains unchanged.
> For instance:
> 
>     -symbol-list-lines foo.adb
>     ^error,msg="-symbol-list-lines: Unknown source file name."

Doesn't this constitute a reason for bumping the MI revision?

> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -156,6 +156,10 @@ show startup-with-shell
>    ** The new command -info-gdb-mi-command allows the user to determine
>       whether a GDB/MI command is supported or not.
>  
> +  ** The "^error" result record returned when trying to execute an undefined
> +     GDB/MI command now provides a variable named "code" whose content is the
> +     "undefined-command" error code.

OK, but I would mention the fact that this can be inquired about, as
you described in your message.

> +@item "^error" "," "msg=" @var{c-string} [ "," "code=" @var{c-string} ]
>  @findex ^error
> -The operation failed.  The @code{@var{c-string}} contains the corresponding
> +The operation failed.  The @var{msg} variable contains the corresponding
>  error message.
>  
> +If present, the @var{code} variable provides an error code on which

The markup is wrong here: "code" is not a variable, it is a literal
symbol.  You probably meant "c-string" instead.

Otherwise, the documentation parts are OK.

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [RFA 1/2] New GDB/MI command "-info-gdb-mi-command"
  2013-11-18 17:13                         ` [RFA 1/2] New GDB/MI command "-info-gdb-mi-command" Joel Brobecker
@ 2013-11-18 17:29                           ` Eli Zaretskii
  2013-11-19  4:35                             ` Joel Brobecker
  0 siblings, 1 reply; 42+ messages in thread
From: Eli Zaretskii @ 2013-11-18 17:29 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

> From: Joel Brobecker <brobecker@adacore.com>
> Date: Mon, 18 Nov 2013 21:11:58 +0400
> 
> Regarding some questions Eli had:
> 
>   | > +(the leading dash (@code{-}) in the command name should be omitted).
>   | Is this wise?  How about if we support both with and without the dash?
> 
> I now think that it was indeed the correct choice.  Not only does it
> facilitate implementation (but only marginally), it also is consistent
> with the current output.  For instance, notice how GDB names the command
> in the following error message:
> 
>     -unsupported
>     ^error,msg="Undefined MI command: unsupported"
>                                       ^^^^^^^^^^^
>                                     (no leading dash)

Your example shows _output_ from MI.  By contrast, we are talking
about _input_.  When I send commands to MI, I cannot omit the leading
dash, so it can be very natural to consider it part of the command.

We don't have to advertise that we support the dash, 

> Also, looking at the grammar, the leading dash isn't listed
> as part of what they call the "operation"

IMO, this line of reasoning makes little sense to users.  Grammars are
for programs, not for people.

> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -153,6 +153,9 @@ show startup-with-shell
>  
>    ** All MI commands now accept an optional "--language" option.
>  
> +  ** The new command -info-gdb-mi-command allows the user to determine
> +     whether a GDB/MI command is supported or not.
> +

OK for this part.

> +Here is an example where the @sc{gdb/mi} command does not exist:
> +
> +@smallexample
> +-info-gdb-mi-command unsupported-command
> +^done,command=@{exists="false"@}
> +@end smallexample
> +
> +And here is an example where the @sc{gdb/mi} command is known
> +to the debugger:

You want @noindent before "And here".

The documentation parts are OK with that change.

Thanks.

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [RFA 1/2] New GDB/MI command "-info-gdb-mi-command"
  2013-11-18 17:29                           ` Eli Zaretskii
@ 2013-11-19  4:35                             ` Joel Brobecker
  2013-11-19 16:11                               ` Eli Zaretskii
                                                 ` (2 more replies)
  0 siblings, 3 replies; 42+ messages in thread
From: Joel Brobecker @ 2013-11-19  4:35 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

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

> > I now think that it was indeed the correct choice.  Not only does it
> > facilitate implementation (but only marginally), it also is consistent
> > with the current output.  For instance, notice how GDB names the command
> > in the following error message:
> > 
> >     -unsupported
> >     ^error,msg="Undefined MI command: unsupported"
> >                                       ^^^^^^^^^^^
> >                                     (no leading dash)
> 
> Your example shows _output_ from MI.  By contrast, we are talking
> about _input_.  When I send commands to MI, I cannot omit the leading
> dash, so it can be very natural to consider it part of the command.
> 
> We don't have to advertise that we support the dash, 
> 
> > Also, looking at the grammar, the leading dash isn't listed
> > as part of what they call the "operation"
> 
> IMO, this line of reasoning makes little sense to users.  Grammars are
> for programs, not for people.

To me, documentation is not an issue.  I confess that I remain
unconvinced in this case, especially since this is a command meant
for programs rather than humans, so the risk of using it improperly
is low, given the clear documentation.  However, I don't have a strong
opinion, and supporting both forms is pretty easy, so unless someone
strongly objects to allowing the second form, I've just gone ahead and
added it.

Updated patch attached. And for review convenience, I am also attaching
a diff of the changes I made on top of path #1 (to get to the updated
patch).

gdb/ChangeLog:

        * mi/mi-cmds.h (mi_cmd_info_gdb_mi_command): Declare.
        * mi/mi-cmd-info.c (mi_cmd_info_gdb_mi_command): New function.
        * mi/mi-cmds.c (mi_cmds): Add -info-gdb-mi-command command.
        * mi/mi-main.c (mi_cmd_list_features): Add "info-gdb-mi-command"
        field to output of "-list-features".

        * NEWS: Add entry for new -info-gdb-mi-command.

gdb/doc/ChangeLog:

        * gdb.texinfo (GDB/MI Miscellaneous Commands): Document
        the new -info-gdb-mi-command GDB/MI command.  Document
        the meaning of "-info-gdb-mi-command" in the output of
        -list-features.

gdb/testsuite/ChangeLog:

        * gdb.mi/mi-i-cmd.exp: New file.

Re-tested on x86_64-linux.  OK to commit?

Thank you,
-- 
Joel

[-- Attachment #2: 0001-New-GDB-MI-command-info-gdb-mi-command.patch --]
[-- Type: text/x-diff, Size: 10540 bytes --]

From 5a8ff8bf85f7d455c3f75312e30ec5c1e077ae01 Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Tue, 12 Nov 2013 14:51:30 +0400
Subject: [PATCH] New GDB/MI command "-info-gdb-mi-command"

This patch adds a new GDB/MI command meant for graphical frontends
trying to determine whether a given GDB/MI command exists or not.

Examples:

    -info-gdb-mi-command unsupported-command
    ^done,command={exists="false"}
    (gdb)
    -info-gdb-mi-command symbol-list-lines
    ^done,command={exists="true"}
    (gdb)

At the moment, this is the only piece of information that this
command returns.

Eventually, and if needed, we can extend it to provide
command-specific pieces of information, such as updates to
the command's syntax since inception.  This could become,
for instance:

    -info-gdb-mi-command symbol-list-lines
    ^done,command={exists="true",features=[]}
    (gdb)
    -info-gdb-mi-command catch-assert
    ^done,command={exists="true",features=["conditions"]}

In the first case, it would mean that no extra features,
while in the second, it announces that the -catch-assert
command in this version of the debugger supports a feature
called "condition" - exact semantics to be documented with
combined with the rest of the queried command's documentation.

But for now, we start small, and only worry about existance.
And to bootstrap the process, I have added an entry in the
output of the -list-features command as well ("info-gdb-mi-command"),
allowing the graphical frontends to go through the following process:

  1. Send -list-features, collect info from there as before;
  2. Check if the output contains "info-gdb-mi-command".
     If it does, then support for various commands can be
     queried though -info-gdb-mi-command. Newer commands
     will be expected to always be checked via this new
     -info-gdb-mi-command.

gdb/ChangeLog:

        * mi/mi-cmds.h (mi_cmd_info_gdb_mi_command): Declare.
        * mi/mi-cmd-info.c (mi_cmd_info_gdb_mi_command): New function.
        * mi/mi-cmds.c (mi_cmds): Add -info-gdb-mi-command command.
        * mi/mi-main.c (mi_cmd_list_features): Add "info-gdb-mi-command"
        field to output of "-list-features".

        * NEWS: Add entry for new -info-gdb-mi-command.

gdb/doc/ChangeLog:

        * gdb.texinfo (GDB/MI Miscellaneous Commands): Document
        the new -info-gdb-mi-command GDB/MI command.  Document
        the meaning of "-info-gdb-mi-command" in the output of
        -list-features.

gdb/testsuite/ChangeLog:

        * gdb.mi/mi-i-cmd.exp: New file.
---
 gdb/NEWS                          |  3 +++
 gdb/doc/gdb.texinfo               | 53 +++++++++++++++++++++++++++++++++++++++
 gdb/mi/mi-cmd-info.c              | 29 +++++++++++++++++++++
 gdb/mi/mi-cmds.c                  |  1 +
 gdb/mi/mi-cmds.h                  |  1 +
 gdb/mi/mi-main.c                  |  1 +
 gdb/testsuite/gdb.mi/mi-i-cmd.exp | 46 +++++++++++++++++++++++++++++++++
 7 files changed, 134 insertions(+)
 create mode 100644 gdb/testsuite/gdb.mi/mi-i-cmd.exp

diff --git a/gdb/NEWS b/gdb/NEWS
index 9fc3638..e61c79f 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -153,6 +153,9 @@ show startup-with-shell
 
   ** All MI commands now accept an optional "--language" option.
 
+  ** The new command -info-gdb-mi-command allows the user to determine
+     whether a GDB/MI command is supported or not.
+
   ** The -trace-save MI command can optionally save trace buffer in Common
      Trace Format now.
 
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 19e9aa5..41856b4 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -35069,6 +35069,57 @@ default shows this information when you start an interactive session.
 (gdb)
 @end smallexample
 
+@subheading The @code{-info-gdb-mi-command} Command
+@cindex @code{-info-gdb-mi-command}
+@findex -info-gdb-mi-command
+
+@subsubheading Synopsis
+
+@smallexample
+ -info-gdb-mi-command @var{cmd_name}
+@end smallexample
+
+Query support for the @sc{gdb/mi} command named @var{cmd_name}.
+
+Note that the dash (@code{-}) starting all @sc{gdb/mi} commands
+is technically not part of the command name (@pxref{GDB/MI Input
+Syntax}), and thus should be omitted in @var{cmd_name}.  However,
+for ease of use, this command also accepts the form with the leading
+dash.
+
+@subsubheading @value{GDBN} Command
+
+There is no corresponding @value{GDBN} command.
+
+@subsubheading Result
+
+The result is a tuple.  There is currently only one field:
+
+@table @samp
+@item exists
+This field is equal to @code{"true"} if the @sc{gdb/mi} command exists,
+@code{"false"} otherwise.
+
+@end table
+
+@subsubheading Example
+
+Here is an example where the @sc{gdb/mi} command does not exist:
+
+@smallexample
+-info-gdb-mi-command unsupported-command
+^done,command=@{exists="false"@}
+@end smallexample
+
+@noindent
+And here is an example where the @sc{gdb/mi} command is known
+to the debugger:
+
+@smallexample
+-info-gdb-mi-command symbol-list-lines
+^done,command=@{exists="true"@}
+@end smallexample
+
 @subheading The @code{-list-features} Command
 @findex -list-features
 
@@ -35122,6 +35173,8 @@ exceptions: @code{-info-ada-exceptions}, @code{-catch-assert} and
 @item language-option
 Indicates that all @sc{gdb/mi} commands accept the @option{--language}
 option (@pxref{Context management}).
+@item info-gdb-mi-command
+Indicates support for the @code{-info-gdb-mi-command} command.
 @end table
 
 @subheading The @code{-list-target-features} Command
diff --git a/gdb/mi/mi-cmd-info.c b/gdb/mi/mi-cmd-info.c
index aa4d210..0fce25a 100644
--- a/gdb/mi/mi-cmd-info.c
+++ b/gdb/mi/mi-cmd-info.c
@@ -71,6 +71,35 @@ mi_cmd_info_ada_exceptions (char *command, char **argv, int argc)
   do_cleanups (old_chain);
 }
 
+/* Implement the "-info-gdb-mi-command" GDB/MI command.  */
+
+void
+mi_cmd_info_gdb_mi_command (char *command, char **argv, int argc)
+{
+  const char *cmd_name;
+  struct mi_cmd *cmd;
+  struct ui_out *uiout = current_uiout;
+  struct cleanup *old_chain;
+
+  /* This command takes exactly one argument.  */
+  if (argc != 1)
+    error (_("Usage: -info-gdb-mi-command MI_COMMAND_NAME"));
+  cmd_name = argv[0];
+
+  /* Normally, the command name (aka the "operation" in the GDB/MI
+     grammar), does not include the leading '-' (dash).  But for
+     the user's convenience, allow the user to specify the command
+     name to be with or without that leading dash.  */
+  if (cmd_name[0] == '-')
+    cmd_name++;
+
+  cmd = mi_lookup (cmd_name);
+
+  old_chain = make_cleanup_ui_out_tuple_begin_end (uiout, "command");
+  ui_out_field_string (uiout, "exists", cmd != NULL ? "true" : "false");
+  do_cleanups (old_chain);
+}
+
 void
 mi_cmd_info_os (char *command, char **argv, int argc)
 {
diff --git a/gdb/mi/mi-cmds.c b/gdb/mi/mi-cmds.c
index c536d8a..58a8b89 100644
--- a/gdb/mi/mi-cmds.c
+++ b/gdb/mi/mi-cmds.c
@@ -125,6 +125,7 @@ static struct mi_cmd mi_cmds[] =
   DEF_MI_CMD_MI ("inferior-tty-set", mi_cmd_inferior_tty_set),
   DEF_MI_CMD_MI ("inferior-tty-show", mi_cmd_inferior_tty_show),
   DEF_MI_CMD_MI ("info-ada-exceptions", mi_cmd_info_ada_exceptions),
+  DEF_MI_CMD_MI ("info-gdb-mi-command", mi_cmd_info_gdb_mi_command),
   DEF_MI_CMD_MI ("info-os", mi_cmd_info_os),
   DEF_MI_CMD_MI ("interpreter-exec", mi_cmd_interpreter_exec),
   DEF_MI_CMD_MI ("list-features", mi_cmd_list_features),
diff --git a/gdb/mi/mi-cmds.h b/gdb/mi/mi-cmds.h
index cb8aac1..4ea95fa 100644
--- a/gdb/mi/mi-cmds.h
+++ b/gdb/mi/mi-cmds.h
@@ -74,6 +74,7 @@ extern mi_cmd_argv_ftype mi_cmd_gdb_exit;
 extern mi_cmd_argv_ftype mi_cmd_inferior_tty_set;
 extern mi_cmd_argv_ftype mi_cmd_inferior_tty_show;
 extern mi_cmd_argv_ftype mi_cmd_info_ada_exceptions;
+extern mi_cmd_argv_ftype mi_cmd_info_gdb_mi_command;
 extern mi_cmd_argv_ftype mi_cmd_info_os;
 extern mi_cmd_argv_ftype mi_cmd_interpreter_exec;
 extern mi_cmd_argv_ftype mi_cmd_list_features;
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 793204d..48c8d09 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1817,6 +1817,7 @@ mi_cmd_list_features (char *command, char **argv, int argc)
       ui_out_field_string (uiout, NULL, "ada-task-info");
       ui_out_field_string (uiout, NULL, "ada-exceptions");
       ui_out_field_string (uiout, NULL, "language-option");
+      ui_out_field_string (uiout, NULL, "info-gdb-mi-command");
       
 #if HAVE_PYTHON
       if (gdb_python_initialized)
diff --git a/gdb/testsuite/gdb.mi/mi-i-cmd.exp b/gdb/testsuite/gdb.mi/mi-i-cmd.exp
new file mode 100644
index 0000000..d460559
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-i-cmd.exp
@@ -0,0 +1,46 @@
+# Copyright 2013 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+gdb_exit
+if [mi_gdb_start] {
+    continue
+}
+
+# First, verify that the debugger correctly advertises support
+# for the -info-gdb-mi-command command.
+mi_gdb_test "-list-features" \
+            "\\^done,features=\\\[.*\"info-gdb-mi-command\".*\\\]" \
+            "-list-features should include \"info-gdb-mi-command\""
+
+mi_gdb_test "-info-gdb-mi-command unsupported-command" \
+            "\\^done,command=\\\{exists=\"false\"\\\}" \
+            "-info-gdb-mi-command unsupported-command"
+
+# Same test as above, but including the leading '-' in the command name.
+mi_gdb_test "-info-gdb-mi-command -unsupported-command" \
+            "\\^done,command=\\\{exists=\"false\"\\\}" \
+            "-info-gdb-mi-command -unsupported-command"
+
+mi_gdb_test "-info-gdb-mi-command symbol-list-lines" \
+            "\\^done,command=\\\{exists=\"true\"\\\}" \
+            "-info-gdb-mi-command symbol-list-lines"
+
+# Same test as above, but including the leading '-' in the command name.
+mi_gdb_test "-info-gdb-mi-command -symbol-list-lines" \
+            "\\^done,command=\\\{exists=\"true\"\\\}" \
+            "-info-gdb-mi-command -symbol-list-lines"
-- 
1.8.1.2


[-- Attachment #3: info-gdb-mi-command-updates.diff --]
[-- Type: text/x-diff, Size: 2833 bytes --]

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index f0662ff..4227f31 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -35088,8 +35088,13 @@ default shows this information when you start an interactive session.
  -info-gdb-mi-command @var{cmd_name}
 @end smallexample
 
-Query support for the @sc{gdb/mi} command named @var{cmd_name}
-(the leading dash (@code{-}) in the command name should be omitted).
+Query support for the @sc{gdb/mi} command named @var{cmd_name}.
+
+Note that the dash (@code{-}) starting all @sc{gdb/mi} commands
+is technically not part of the command name (@pxref{GDB/MI Input
+Syntax}), and thus should be omitted in @var{cmd_name}.  However,
+for ease of use, this command also accepts the form with the leading
+dash.
 
 @subsubheading @value{GDBN} Command
 
@@ -35120,6 +35120,7 @@ Here is an example where the @sc{gdb/mi} command does not exist:
 ^done,command=@{exists="false"@}
 @end smallexample
 
+@noindent
 And here is an example where the @sc{gdb/mi} command is known
 to the debugger:
 
diff --git a/gdb/mi/mi-cmd-info.c b/gdb/mi/mi-cmd-info.c
index 18f4927..0fce25a 100644
--- a/gdb/mi/mi-cmd-info.c
+++ b/gdb/mi/mi-cmd-info.c
@@ -85,6 +85,14 @@ mi_cmd_info_gdb_mi_command (char *command, char **argv, int argc)
   if (argc != 1)
     error (_("Usage: -info-gdb-mi-command MI_COMMAND_NAME"));
   cmd_name = argv[0];
+
+  /* Normally, the command name (aka the "operation" in the GDB/MI
+     grammar), does not include the leading '-' (dash).  But for
+     the user's convenience, allow the user to specify the command
+     name to be with or without that leading dash.  */
+  if (cmd_name[0] == '-')
+    cmd_name++;
+
   cmd = mi_lookup (cmd_name);
 
   old_chain = make_cleanup_ui_out_tuple_begin_end (uiout, "command");
diff --git a/gdb/testsuite/gdb.mi/mi-i-cmd.exp b/gdb/testsuite/gdb.mi/mi-i-cmd.exp
index 5285d31..d460559 100644
--- a/gdb/testsuite/gdb.mi/mi-i-cmd.exp
+++ b/gdb/testsuite/gdb.mi/mi-i-cmd.exp
@@ -31,7 +31,16 @@ mi_gdb_test "-info-gdb-mi-command unsupported-command" \
             "\\^done,command=\\\{exists=\"false\"\\\}" \
             "-info-gdb-mi-command unsupported-command"
 
+# Same test as above, but including the leading '-' in the command name.
+mi_gdb_test "-info-gdb-mi-command -unsupported-command" \
+            "\\^done,command=\\\{exists=\"false\"\\\}" \
+            "-info-gdb-mi-command -unsupported-command"
+
 mi_gdb_test "-info-gdb-mi-command symbol-list-lines" \
             "\\^done,command=\\\{exists=\"true\"\\\}" \
             "-info-gdb-mi-command symbol-list-lines"
 
+# Same test as above, but including the leading '-' in the command name.
+mi_gdb_test "-info-gdb-mi-command -symbol-list-lines" \
+            "\\^done,command=\\\{exists=\"true\"\\\}" \
+            "-info-gdb-mi-command -symbol-list-lines"

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [RFA 2/2] Add "undefined-command" error code at end of ^error result...
  2013-11-18 17:29                           ` Eli Zaretskii
@ 2013-11-19  6:02                             ` Joel Brobecker
  2013-11-19 16:16                               ` Eli Zaretskii
  0 siblings, 1 reply; 42+ messages in thread
From: Joel Brobecker @ 2013-11-19  6:02 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

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

Note to code reviewers: While working on Eli's comments, I've also
used that opportunity to rename UNKNOWN_COMMAND_ERROR Into
UNDEFINED_COMMAND_ERROR.  Everything should be nicely consistent, now.

> > This error code is only displayed when the corresponding error
> > condition is met. Otherwise, the error record remains unchanged.
> > For instance:
> > 
> >     -symbol-list-lines foo.adb
> >     ^error,msg="-symbol-list-lines: Unknown source file name."
> 
> Doesn't this constitute a reason for bumping the MI revision?

I do not think so, because the change is upward compatible
(consumers are expected to ignore fields they do not handle).

> > +  ** The "^error" result record returned when trying to execute an undefined
> > +     GDB/MI command now provides a variable named "code" whose content is the
> > +     "undefined-command" error code.
> 
> OK, but I would mention the fact that this can be inquired about, as
> you described in your message.

OK. New version attached.

> > +@item "^error" "," "msg=" @var{c-string} [ "," "code=" @var{c-string} ]
> >  @findex ^error
> > -The operation failed.  The @code{@var{c-string}} contains the corresponding
> > +The operation failed.  The @var{msg} variable contains the corresponding
> >  error message.
> >  
> > +If present, the @var{code} variable provides an error code on which
> 
> The markup is wrong here: "code" is not a variable, it is a literal
> symbol.  You probably meant "c-string" instead.

I've updated both "code" and "msg" (just above). Let me know if it reads
better for you. ("code" is the name of a variable in GDB/MI lexicon).

gdb/ChangeLog:

        (from Pedro Alves  <palves@redhat.com>)
        (from Joel Brobecker  <brobecker@adacore.com>)
        * exceptions.h (enum_errors) <UNDEFINED_COMMAND_ERROR>: New enum.
        * mi/mi-parse.c (mi_parse): Thow UNDEFINED_COMMAND_ERROR instead
        of a regular error when the GDB/MI command does not exist.
        * mi/mi-main.c (mi_cmd_list_features): Add
        "undefined-command-error-code".
        (mi_print_exception): Print an "undefined-command"
        error code if EXCEPTION.ERROR in UNDEFINED_COMMAND_ERROR.
        * NEWS: Add entry documenting the new "code" variable in
        "^error" result records.

gdb/doc/ChangeLog:

        * gdb.texinfo (GDB/MI Result Records): Fix the syntax of the
        "^error" result record concerning the error message.  Document
        the error code that may also be part of that result record.
        (GDB/MI Miscellaneous Commands): Document the
        "undefined-command-error-code" element in the output of
        the "-list-features" GDB/MI command.

gdb/testsuite/ChangeLog:

        * gdb.mi/mi-undefined-cmd.exp: New testcase.

All retested on x86_64-linux.  OK to check in?

Thank you,
-- 
Joel

[-- Attachment #2: 0002-Add-undefined-command-error-code-at-end-of-error-res.patch --]
[-- Type: text/x-diff, Size: 8879 bytes --]

From 5f2f7f77efae56fb3e1bced2cbd504ecd486e8ba Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Mon, 18 Nov 2013 16:55:16 +0400
Subject: [PATCH 2/2] Add "undefined-command" error code at end of ^error
 result...

... when trying to execute an undefined GDB/MI command. When trying
to execute a GDB/MI command which does not exist, the current error
result record looks like this:

    -unsupported
    ^error,msg="Undefined MI command: unsupported"

The only indication that the command does not exist is the error
message. It would be a little fragile for a consumer to rely solely
on the contents of the error message in order to determine whether
a command exists or not.

This patch improves the situation by adding concept of error
code, starting with one well-defined error code ("undefined-command")
identifying errors due to a non-existant command. Here is the new
output:

    -unsupported
    ^error,msg="Undefined MI command: unsupported",code="undefined-command"

This error code is only displayed when the corresponding error
condition is met. Otherwise, the error record remains unchanged.
For instance:

    -symbol-list-lines foo.adb
    ^error,msg="-symbol-list-lines: Unknown source file name."

For frontends to be able to know whether they can rely on this
variable, a new entry "undefined-command-error-code" has been
added to the "-list-features" command.  Another option would be
to always generate an error="..." variable (for the default case,
we could decide for instance that the error code is the empty string).
But it seems more efficient to provide that info in "-list-features"
and then only add the error code when meaningful.

gdb/ChangeLog:

        (from Pedro Alves  <palves@redhat.com>)
        (from Joel Brobecker  <brobecker@adacore.com>)
        * exceptions.h (enum_errors) <UNDEFINED_COMMAND_ERROR>: New enum.
        * mi/mi-parse.c (mi_parse): Thow UNDEFINED_COMMAND_ERROR instead
        of a regular error when the GDB/MI command does not exist.
        * mi/mi-main.c (mi_cmd_list_features): Add
        "undefined-command-error-code".
        (mi_print_exception): Print an "undefined-command"
        error code if EXCEPTION.ERROR in UNDEFINED_COMMAND_ERROR.
        * NEWS: Add entry documenting the new "code" variable in
        "^error" result records.

gdb/doc/ChangeLog:

        * gdb.texinfo (GDB/MI Result Records): Fix the syntax of the
        "^error" result record concerning the error message.  Document
        the error code that may also be part of that result record.
        (GDB/MI Miscellaneous Commands): Document the
        "undefined-command-error-code" element in the output of
        the "-list-features" GDB/MI command.

gdb/testsuite/ChangeLog:

        * gdb.mi/mi-undefined-cmd.exp: New testcase.
---
 gdb/NEWS                                  |  6 ++++++
 gdb/doc/gdb.texinfo                       | 19 +++++++++++++++---
 gdb/exceptions.h                          |  3 +++
 gdb/mi/mi-main.c                          | 12 ++++++++++-
 gdb/mi/mi-parse.c                         |  3 ++-
 gdb/testsuite/gdb.mi/mi-undefined-cmd.exp | 33 +++++++++++++++++++++++++++++++
 6 files changed, 71 insertions(+), 5 deletions(-)
 create mode 100644 gdb/testsuite/gdb.mi/mi-undefined-cmd.exp

diff --git a/gdb/NEWS b/gdb/NEWS
index e61c79f..8ef03d6 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -156,6 +156,12 @@ show startup-with-shell
   ** The new command -info-gdb-mi-command allows the user to determine
      whether a GDB/MI command is supported or not.
 
+  ** The "^error" result record returned when trying to execute an undefined
+     GDB/MI command now provides a variable named "code" whose content is the
+     "undefined-command" error code.  Support for this feature can be verified
+     by using the "-list-features" command, which should contain
+     "undefined-command-error-code".
+
   ** The -trace-save MI command can optionally save trace buffer in Common
      Trace Format now.
 
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 41856b4..9e86972 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -29315,10 +29315,19 @@ which threads are resumed.
 @findex ^connected
 @value{GDBN} has connected to a remote target.
 
-@item "^error" "," @var{c-string}
+@item "^error" "," "msg=" @var{c-string} [ "," "code=" @var{c-string} ]
 @findex ^error
-The operation failed.  The @code{@var{c-string}} contains the corresponding
-error message.
+The operation failed.  The @code{msg=@var{c-string}} variable contains
+the corresponding error message.
+
+If present, the @code{code=@var{c-string}} variable provides an error
+code on which consumers can rely in order to detect the corresponding
+error condition.  At present, only one error code is defined:
+
+@table @samp
+@item "undefined-command"
+Indicates that the command causing the error does not exist.
+@end table
 
 @item "^exit"
 @findex ^exit
@@ -35175,6 +35184,10 @@ Indicates that all @sc{gdb/mi} commands accept the @option{--language}
 option (@pxref{Context management}).
 @item info-gdb-mi-command
 Indicates support for the @code{-info-gdb-mi-command} command.
+@item undefined-command-error-code
+Indicates support for the "undefined-command" error code in error result
+records, produced when trying to execute an undefined @sc{gdb/mi} command
+(@pxref{GDB/MI Result Records}).
 @end table
 
 @subheading The @code{-list-target-features} Command
diff --git a/gdb/exceptions.h b/gdb/exceptions.h
index 129d29a..a3a28f4 100644
--- a/gdb/exceptions.h
+++ b/gdb/exceptions.h
@@ -93,6 +93,9 @@ enum errors {
      aborted as the inferior state is no longer valid.  */
   TARGET_CLOSE_ERROR,
 
+  /* An undefined command was executed.  */
+  UNDEFINED_COMMAND_ERROR,
+
   /* Add more errors here.  */
   NR_ERRORS
 };
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 48c8d09..3a0e6a8 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1818,6 +1818,7 @@ mi_cmd_list_features (char *command, char **argv, int argc)
       ui_out_field_string (uiout, NULL, "ada-exceptions");
       ui_out_field_string (uiout, NULL, "language-option");
       ui_out_field_string (uiout, NULL, "info-gdb-mi-command");
+      ui_out_field_string (uiout, NULL, "undefined-command-error-code");
       
 #if HAVE_PYTHON
       if (gdb_python_initialized)
@@ -2023,7 +2024,16 @@ mi_print_exception (const char *token, struct gdb_exception exception)
     fputs_unfiltered ("unknown error", raw_stdout);
   else
     fputstr_unfiltered (exception.message, '"', raw_stdout);
-  fputs_unfiltered ("\"\n", raw_stdout);
+  fputs_unfiltered ("\"", raw_stdout);
+
+  switch (exception.error)
+    {
+      case UNDEFINED_COMMAND_ERROR:
+	fputs_unfiltered (",code=\"undefined-command\"", raw_stdout);
+	break;
+    }
+
+  fputs_unfiltered ("\n", raw_stdout);
 }
 
 void
diff --git a/gdb/mi/mi-parse.c b/gdb/mi/mi-parse.c
index a2634f1..a092759 100644
--- a/gdb/mi/mi-parse.c
+++ b/gdb/mi/mi-parse.c
@@ -285,7 +285,8 @@ mi_parse (const char *cmd, char **token)
   /* Find the command in the MI table.  */
   parse->cmd = mi_lookup (parse->command);
   if (parse->cmd == NULL)
-    error (_("Undefined MI command: %s"), parse->command);
+    throw_error (UNDEFINED_COMMAND_ERROR,
+		 _("Undefined MI command: %s"), parse->command);
 
   /* Skip white space following the command.  */
   chp = skip_spaces_const (chp);
diff --git a/gdb/testsuite/gdb.mi/mi-undefined-cmd.exp b/gdb/testsuite/gdb.mi/mi-undefined-cmd.exp
new file mode 100644
index 0000000..8df0a76
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-undefined-cmd.exp
@@ -0,0 +1,33 @@
+# Copyright 2013 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+gdb_exit
+if [mi_gdb_start] {
+    continue
+}
+
+
+# First, verify that the debugger correctly advertises support
+# for the "undefined-command" error code...
+mi_gdb_test "-list-features" \
+            "\\^done,features=\\\[.*\"undefined-command-error-code\".*\\\]" \
+            "-list-features should include \"undefined-command-error-code\""
+
+mi_gdb_test "-undefined-command" \
+            "\\^error,.*,code=\"undefined-command\"" \
+            "error code when executing undefined command"
-- 
1.8.1.2


^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [RFA 2/2] Add "undefined-command" error code at end of ^error result...
  2013-11-18 17:21                         ` [RFA 2/2] Add "undefined-command" error code at end of ^error result Joel Brobecker
  2013-11-18 17:29                           ` Eli Zaretskii
@ 2013-11-19 11:19                           ` Pedro Alves
  2013-11-20  3:46                             ` Joel Brobecker
  1 sibling, 1 reply; 42+ messages in thread
From: Pedro Alves @ 2013-11-19 11:19 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

On 11/18/2013 05:11 PM, Joel Brobecker wrote:

> gdb/ChangeLog:
> 
>         (from Pedro Alves  <palves@redhat.com>)
>         (from Joel Brobecker  <brobecker@adacore.com>)
>         * exceptions.h (enum_errors) <UNKNOWN_COMMAND_ERROR>: New enum.
>         * mi/mi-parse.c (mi_parse): Thow UNKNOWN_COMMAND_ERROR instead

"Throw"

>         of a regular error when the GDB/MI command does not exist.
>         * mi/mi-main.c (mi_cmd_list_features): Add
>         "undefined-command-error-code".
>         (mi_print_exception): Print an "undefined-command"
>         error code if EXCEPTION.ERROR in UNKNOWN_COMMAND_ERROR.

s/in/is ?

>         * NEWS: Add entry documenting the new "code" variable in
>         "^error" result records.
> 
> gdb/doc/ChangeLog:
> 
>         * gdb.texinfo (GDB/MI Result Records): Fix the syntax of the
>         "^error" result record concerning the error message.  Document
>         the error code that may also be part of that result record.
>         (GDB/MI Miscellaneous Commands): Document the
>         "undefined-command-error-code" element in the output of
>         the "-list-features" GDB/MI command.
> 
> gdb/testsuite/ChangeLog:
> 
>         * gdb.mi/mi-undefined-cmd.exp: New testcase.
> 
> Tested on x86_64-linux.  OK to commit?

Looks good to me...

> (hmmm, now that I have spent all that time typing everything up,
> I am wondering if I should rename UNKNOWN_COMMAND_ERROR into
> UNDEFINED_COMMAND_ERROR - no real biggie either way...)

Yeah, I think so.  The CLI also says "undefined":

 (gdb) asdf
 Undefined command: "asdf".  Try "help".

Might as well be consistent throughout.

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [RFA GDB/MI] Help determine if GDB/MI command exists or not
  2013-11-18 17:12                       ` [RFA GDB/MI] Help determine if GDB/MI command exists or not Joel Brobecker
  2013-11-18 17:13                         ` [RFA 1/2] New GDB/MI command "-info-gdb-mi-command" Joel Brobecker
  2013-11-18 17:21                         ` [RFA 2/2] Add "undefined-command" error code at end of ^error result Joel Brobecker
@ 2013-11-19 15:05                         ` Pedro Alves
  2 siblings, 0 replies; 42+ messages in thread
From: Pedro Alves @ 2013-11-19 15:05 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

On 11/18/2013 05:11 PM, Joel Brobecker wrote:

>   - Path #2 implements Pedro's idea of adding an error code to
>     the "^error" result record.
> 
>     I took Pedro's patch nearly verbatim, removing the bits that
>     dealt with invalid command-line usage (this part is left for
>     another time, if the need becomes a little more explicit).
>     I did notice that the additonal variable looked an awful lot
>     like an error code, so I found it odd that we'd name if "error="
>     in the patch. And then I realized that Pedro's first email did
>     say "code", so I assumed it was a brain fart, and fixed the patch
>     to use "code".

Whoops, yeah.

Thanks,
-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [RFA 1/2] New GDB/MI command "-info-gdb-mi-command"
  2013-11-19  4:35                             ` Joel Brobecker
@ 2013-11-19 16:11                               ` Eli Zaretskii
  2013-12-02  3:26                               ` Joel Brobecker
  2013-12-02 14:53                               ` Pedro Alves
  2 siblings, 0 replies; 42+ messages in thread
From: Eli Zaretskii @ 2013-11-19 16:11 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

> Date: Tue, 19 Nov 2013 08:10:22 +0400
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: gdb-patches@sourceware.org
> 
> I don't have a strong opinion, and supporting both forms is pretty
> easy, so unless someone strongly objects to allowing the second
> form, I've just gone ahead and added it.

Thank you.

The documentation part is still OK.

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [RFA 2/2] Add "undefined-command" error code at end of ^error result...
  2013-11-19  6:02                             ` Joel Brobecker
@ 2013-11-19 16:16                               ` Eli Zaretskii
  0 siblings, 0 replies; 42+ messages in thread
From: Eli Zaretskii @ 2013-11-19 16:16 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

> Date: Tue, 19 Nov 2013 08:35:26 +0400
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: gdb-patches@sourceware.org
> 
> > > +  ** The "^error" result record returned when trying to execute an undefined
> > > +     GDB/MI command now provides a variable named "code" whose content is the
> > > +     "undefined-command" error code.
> > 
> > OK, but I would mention the fact that this can be inquired about, as
> > you described in your message.
> 
> OK. New version attached.

Thanks.

> > > +@item "^error" "," "msg=" @var{c-string} [ "," "code=" @var{c-string} ]
> > >  @findex ^error
> > > -The operation failed.  The @code{@var{c-string}} contains the corresponding
> > > +The operation failed.  The @var{msg} variable contains the corresponding
> > >  error message.
> > >  
> > > +If present, the @var{code} variable provides an error code on which
> > 
> > The markup is wrong here: "code" is not a variable, it is a literal
> > symbol.  You probably meant "c-string" instead.
> 
> I've updated both "code" and "msg" (just above). Let me know if it reads
> better for you. ("code" is the name of a variable in GDB/MI lexicon).

It's OK now.

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [RFA 2/2] Add "undefined-command" error code at end of ^error result...
  2013-11-19 11:19                           ` Pedro Alves
@ 2013-11-20  3:46                             ` Joel Brobecker
  2013-12-03  4:08                               ` pushed: " Joel Brobecker
  0 siblings, 1 reply; 42+ messages in thread
From: Joel Brobecker @ 2013-11-20  3:46 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

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

On Tue, Nov 19, 2013 at 11:13:30AM +0000, Pedro Alves wrote:
> On 11/18/2013 05:11 PM, Joel Brobecker wrote:
> 
> > gdb/ChangeLog:
> > 
> >         (from Pedro Alves  <palves@redhat.com>)
> >         (from Joel Brobecker  <brobecker@adacore.com>)
> >         * exceptions.h (enum_errors) <UNKNOWN_COMMAND_ERROR>: New enum.
> >         * mi/mi-parse.c (mi_parse): Thow UNKNOWN_COMMAND_ERROR instead
> 
> "Throw"
> 
> >         of a regular error when the GDB/MI command does not exist.
> >         * mi/mi-main.c (mi_cmd_list_features): Add
> >         "undefined-command-error-code".
> >         (mi_print_exception): Print an "undefined-command"
> >         error code if EXCEPTION.ERROR in UNKNOWN_COMMAND_ERROR.
> 
> s/in/is ?
> 
> >         * NEWS: Add entry documenting the new "code" variable in
> >         "^error" result records.
> > 
> > gdb/doc/ChangeLog:
> > 
> >         * gdb.texinfo (GDB/MI Result Records): Fix the syntax of the
> >         "^error" result record concerning the error message.  Document
> >         the error code that may also be part of that result record.
> >         (GDB/MI Miscellaneous Commands): Document the
> >         "undefined-command-error-code" element in the output of
> >         the "-list-features" GDB/MI command.
> > 
> > gdb/testsuite/ChangeLog:
> > 
> >         * gdb.mi/mi-undefined-cmd.exp: New testcase.
> > 
> > Tested on x86_64-linux.  OK to commit?
> 
> Looks good to me...

Thanks, Pedro! I've made the corrections for the errors you spotted.
And for the UNKNOWN_COMMAND_ERROR vs UNDEFINED_COMMAND_ERROR, I had
already made the changes in the first re-send, for the documentation
fixes.

This patch depends on patch #1,  not logically, but there are conflicts
areas that I'd rather not have to deal with if I don't have to. I am
just hoping for someone to review the patch (hint, hint! :-)). If it
takes too much time, I will put this one in first.

-- 
Joel

[-- Attachment #2: 0002-Add-undefined-command-error-code-at-end-of-error-res.patch --]
[-- Type: text/x-diff, Size: 8874 bytes --]

From 902182d958a76a09d9a551c5121fe15e10d2d5bc Mon Sep 17 00:00:00 2001
From: Joel Brobecker <brobecker@adacore.com>
Date: Mon, 18 Nov 2013 16:55:16 +0400
Subject: [PATCH 2/2] Add "undefined-command" error code at end of ^error
 result...

... when trying to execute an undefined GDB/MI command. When trying
to execute a GDB/MI command which does not exist, the current error
result record looks like this:

    -unsupported
    ^error,msg="Undefined MI command: unsupported"

The only indication that the command does not exist is the error
message. It would be a little fragile for a consumer to rely solely
on the contents of the error message in order to determine whether
a command exists or not.

This patch improves the situation by adding concept of error
code, starting with one well-defined error code ("undefined-command")
identifying errors due to a non-existant command. Here is the new
output:

    -unsupported
    ^error,msg="Undefined MI command: unsupported",code="undefined-command"

This error code is only displayed when the corresponding error
condition is met. Otherwise, the error record remains unchanged.
For instance:

    -symbol-list-lines foo.adb
    ^error,msg="-symbol-list-lines: Unknown source file name."

For frontends to be able to know whether they can rely on this
variable, a new entry "undefined-command-error-code" has been
added to the "-list-features" command.  Another option would be
to always generate an error="..." variable (for the default case,
we could decide for instance that the error code is the empty string).
But it seems more efficient to provide that info in "-list-features"
and then only add the error code when meaningful.

gdb/ChangeLog:

        (from Pedro Alves  <palves@redhat.com>)
        (from Joel Brobecker  <brobecker@adacore.com>)
        * exceptions.h (enum_errors) <UNDEFINED_COMMAND_ERROR>: New enum.
        * mi/mi-parse.c (mi_parse): Throw UNDEFINED_COMMAND_ERROR instead
        of a regular error when the GDB/MI command does not exist.
        * mi/mi-main.c (mi_cmd_list_features): Add
        "undefined-command-error-code".
        (mi_print_exception): Print an "undefined-command"
        error code if EXCEPTION.ERROR is UNDEFINED_COMMAND_ERROR.
        * NEWS: Add entry documenting the new "code" variable in
        "^error" result records.

gdb/doc/ChangeLog:

        * gdb.texinfo (GDB/MI Result Records): Fix the syntax of the
        "^error" result record concerning the error message.  Document
        the error code that may also be part of that result record.
        (GDB/MI Miscellaneous Commands): Document the
        "undefined-command-error-code" element in the output of
        the "-list-features" GDB/MI command.

gdb/testsuite/ChangeLog:

        * gdb.mi/mi-undefined-cmd.exp: New testcase.
---
 gdb/NEWS                                  |  6 ++++++
 gdb/doc/gdb.texinfo                       | 19 +++++++++++++++---
 gdb/exceptions.h                          |  3 +++
 gdb/mi/mi-main.c                          | 12 ++++++++++-
 gdb/mi/mi-parse.c                         |  3 ++-
 gdb/testsuite/gdb.mi/mi-undefined-cmd.exp | 33 +++++++++++++++++++++++++++++++
 6 files changed, 71 insertions(+), 5 deletions(-)
 create mode 100644 gdb/testsuite/gdb.mi/mi-undefined-cmd.exp

diff --git a/gdb/NEWS b/gdb/NEWS
index e61c79f..8ef03d6 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -156,6 +156,12 @@ show startup-with-shell
   ** The new command -info-gdb-mi-command allows the user to determine
      whether a GDB/MI command is supported or not.
 
+  ** The "^error" result record returned when trying to execute an undefined
+     GDB/MI command now provides a variable named "code" whose content is the
+     "undefined-command" error code.  Support for this feature can be verified
+     by using the "-list-features" command, which should contain
+     "undefined-command-error-code".
+
   ** The -trace-save MI command can optionally save trace buffer in Common
      Trace Format now.
 
diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 41856b4..4aef805 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -29315,10 +29315,19 @@ which threads are resumed.
 @findex ^connected
 @value{GDBN} has connected to a remote target.
 
-@item "^error" "," @var{c-string}
+@item "^error" "," "msg=" @var{c-string} [ "," "code=" @var{c-string} ]
 @findex ^error
-The operation failed.  The @code{@var{c-string}} contains the corresponding
-error message.
+The operation failed.  The @code{msg=@var{c-string}} variable contains
+the corresponding error message.
+
+If present, the @code{code=@var{c-string}} variable provides an error
+code on which consumers can rely on to detect the corresponding
+error condition.  At present, only one error code is defined:
+
+@table @samp
+@item "undefined-command"
+Indicates that the command causing the error does not exist.
+@end table
 
 @item "^exit"
 @findex ^exit
@@ -35175,6 +35184,10 @@ Indicates that all @sc{gdb/mi} commands accept the @option{--language}
 option (@pxref{Context management}).
 @item info-gdb-mi-command
 Indicates support for the @code{-info-gdb-mi-command} command.
+@item undefined-command-error-code
+Indicates support for the "undefined-command" error code in error result
+records, produced when trying to execute an undefined @sc{gdb/mi} command
+(@pxref{GDB/MI Result Records}).
 @end table
 
 @subheading The @code{-list-target-features} Command
diff --git a/gdb/exceptions.h b/gdb/exceptions.h
index 129d29a..a3a28f4 100644
--- a/gdb/exceptions.h
+++ b/gdb/exceptions.h
@@ -93,6 +93,9 @@ enum errors {
      aborted as the inferior state is no longer valid.  */
   TARGET_CLOSE_ERROR,
 
+  /* An undefined command was executed.  */
+  UNDEFINED_COMMAND_ERROR,
+
   /* Add more errors here.  */
   NR_ERRORS
 };
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 48c8d09..3a0e6a8 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1818,6 +1818,7 @@ mi_cmd_list_features (char *command, char **argv, int argc)
       ui_out_field_string (uiout, NULL, "ada-exceptions");
       ui_out_field_string (uiout, NULL, "language-option");
       ui_out_field_string (uiout, NULL, "info-gdb-mi-command");
+      ui_out_field_string (uiout, NULL, "undefined-command-error-code");
       
 #if HAVE_PYTHON
       if (gdb_python_initialized)
@@ -2023,7 +2024,16 @@ mi_print_exception (const char *token, struct gdb_exception exception)
     fputs_unfiltered ("unknown error", raw_stdout);
   else
     fputstr_unfiltered (exception.message, '"', raw_stdout);
-  fputs_unfiltered ("\"\n", raw_stdout);
+  fputs_unfiltered ("\"", raw_stdout);
+
+  switch (exception.error)
+    {
+      case UNDEFINED_COMMAND_ERROR:
+	fputs_unfiltered (",code=\"undefined-command\"", raw_stdout);
+	break;
+    }
+
+  fputs_unfiltered ("\n", raw_stdout);
 }
 
 void
diff --git a/gdb/mi/mi-parse.c b/gdb/mi/mi-parse.c
index a2634f1..a092759 100644
--- a/gdb/mi/mi-parse.c
+++ b/gdb/mi/mi-parse.c
@@ -285,7 +285,8 @@ mi_parse (const char *cmd, char **token)
   /* Find the command in the MI table.  */
   parse->cmd = mi_lookup (parse->command);
   if (parse->cmd == NULL)
-    error (_("Undefined MI command: %s"), parse->command);
+    throw_error (UNDEFINED_COMMAND_ERROR,
+		 _("Undefined MI command: %s"), parse->command);
 
   /* Skip white space following the command.  */
   chp = skip_spaces_const (chp);
diff --git a/gdb/testsuite/gdb.mi/mi-undefined-cmd.exp b/gdb/testsuite/gdb.mi/mi-undefined-cmd.exp
new file mode 100644
index 0000000..8df0a76
--- /dev/null
+++ b/gdb/testsuite/gdb.mi/mi-undefined-cmd.exp
@@ -0,0 +1,33 @@
+# Copyright 2013 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+load_lib mi-support.exp
+set MIFLAGS "-i=mi"
+
+gdb_exit
+if [mi_gdb_start] {
+    continue
+}
+
+
+# First, verify that the debugger correctly advertises support
+# for the "undefined-command" error code...
+mi_gdb_test "-list-features" \
+            "\\^done,features=\\\[.*\"undefined-command-error-code\".*\\\]" \
+            "-list-features should include \"undefined-command-error-code\""
+
+mi_gdb_test "-undefined-command" \
+            "\\^error,.*,code=\"undefined-command\"" \
+            "error code when executing undefined command"
-- 
1.8.1.2


^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [RFA 1/2] New GDB/MI command "-info-gdb-mi-command"
  2013-11-19  4:35                             ` Joel Brobecker
  2013-11-19 16:11                               ` Eli Zaretskii
@ 2013-12-02  3:26                               ` Joel Brobecker
  2013-12-02  3:51                                 ` Eli Zaretskii
  2013-12-02 14:53                               ` Pedro Alves
  2 siblings, 1 reply; 42+ messages in thread
From: Joel Brobecker @ 2013-12-02  3:26 UTC (permalink / raw)
  To: gdb-patches

Kind request for review. The code itself is fairly straightforward,
I think. It's more about making sure that we're defining the right
command...

Thank you!

On Tue, Nov 19, 2013 at 08:10:22AM +0400, Joel Brobecker wrote:
> > > I now think that it was indeed the correct choice.  Not only does it
> > > facilitate implementation (but only marginally), it also is consistent
> > > with the current output.  For instance, notice how GDB names the command
> > > in the following error message:
> > > 
> > >     -unsupported
> > >     ^error,msg="Undefined MI command: unsupported"
> > >                                       ^^^^^^^^^^^
> > >                                     (no leading dash)
> > 
> > Your example shows _output_ from MI.  By contrast, we are talking
> > about _input_.  When I send commands to MI, I cannot omit the leading
> > dash, so it can be very natural to consider it part of the command.
> > 
> > We don't have to advertise that we support the dash, 
> > 
> > > Also, looking at the grammar, the leading dash isn't listed
> > > as part of what they call the "operation"
> > 
> > IMO, this line of reasoning makes little sense to users.  Grammars are
> > for programs, not for people.
> 
> To me, documentation is not an issue.  I confess that I remain
> unconvinced in this case, especially since this is a command meant
> for programs rather than humans, so the risk of using it improperly
> is low, given the clear documentation.  However, I don't have a strong
> opinion, and supporting both forms is pretty easy, so unless someone
> strongly objects to allowing the second form, I've just gone ahead and
> added it.
> 
> Updated patch attached. And for review convenience, I am also attaching
> a diff of the changes I made on top of path #1 (to get to the updated
> patch).
> 
> gdb/ChangeLog:
> 
>         * mi/mi-cmds.h (mi_cmd_info_gdb_mi_command): Declare.
>         * mi/mi-cmd-info.c (mi_cmd_info_gdb_mi_command): New function.
>         * mi/mi-cmds.c (mi_cmds): Add -info-gdb-mi-command command.
>         * mi/mi-main.c (mi_cmd_list_features): Add "info-gdb-mi-command"
>         field to output of "-list-features".
> 
>         * NEWS: Add entry for new -info-gdb-mi-command.
> 
> gdb/doc/ChangeLog:
> 
>         * gdb.texinfo (GDB/MI Miscellaneous Commands): Document
>         the new -info-gdb-mi-command GDB/MI command.  Document
>         the meaning of "-info-gdb-mi-command" in the output of
>         -list-features.
> 
> gdb/testsuite/ChangeLog:
> 
>         * gdb.mi/mi-i-cmd.exp: New file.
> 
> Re-tested on x86_64-linux.  OK to commit?
> 
> Thank you,
> -- 
> Joel

> From 5a8ff8bf85f7d455c3f75312e30ec5c1e077ae01 Mon Sep 17 00:00:00 2001
> From: Joel Brobecker <brobecker@adacore.com>
> Date: Tue, 12 Nov 2013 14:51:30 +0400
> Subject: [PATCH] New GDB/MI command "-info-gdb-mi-command"
> 
> This patch adds a new GDB/MI command meant for graphical frontends
> trying to determine whether a given GDB/MI command exists or not.
> 
> Examples:
> 
>     -info-gdb-mi-command unsupported-command
>     ^done,command={exists="false"}
>     (gdb)
>     -info-gdb-mi-command symbol-list-lines
>     ^done,command={exists="true"}
>     (gdb)
> 
> At the moment, this is the only piece of information that this
> command returns.
> 
> Eventually, and if needed, we can extend it to provide
> command-specific pieces of information, such as updates to
> the command's syntax since inception.  This could become,
> for instance:
> 
>     -info-gdb-mi-command symbol-list-lines
>     ^done,command={exists="true",features=[]}
>     (gdb)
>     -info-gdb-mi-command catch-assert
>     ^done,command={exists="true",features=["conditions"]}
> 
> In the first case, it would mean that no extra features,
> while in the second, it announces that the -catch-assert
> command in this version of the debugger supports a feature
> called "condition" - exact semantics to be documented with
> combined with the rest of the queried command's documentation.
> 
> But for now, we start small, and only worry about existance.
> And to bootstrap the process, I have added an entry in the
> output of the -list-features command as well ("info-gdb-mi-command"),
> allowing the graphical frontends to go through the following process:
> 
>   1. Send -list-features, collect info from there as before;
>   2. Check if the output contains "info-gdb-mi-command".
>      If it does, then support for various commands can be
>      queried though -info-gdb-mi-command. Newer commands
>      will be expected to always be checked via this new
>      -info-gdb-mi-command.
> 
> gdb/ChangeLog:
> 
>         * mi/mi-cmds.h (mi_cmd_info_gdb_mi_command): Declare.
>         * mi/mi-cmd-info.c (mi_cmd_info_gdb_mi_command): New function.
>         * mi/mi-cmds.c (mi_cmds): Add -info-gdb-mi-command command.
>         * mi/mi-main.c (mi_cmd_list_features): Add "info-gdb-mi-command"
>         field to output of "-list-features".
> 
>         * NEWS: Add entry for new -info-gdb-mi-command.
> 
> gdb/doc/ChangeLog:
> 
>         * gdb.texinfo (GDB/MI Miscellaneous Commands): Document
>         the new -info-gdb-mi-command GDB/MI command.  Document
>         the meaning of "-info-gdb-mi-command" in the output of
>         -list-features.
> 
> gdb/testsuite/ChangeLog:
> 
>         * gdb.mi/mi-i-cmd.exp: New file.
> ---
>  gdb/NEWS                          |  3 +++
>  gdb/doc/gdb.texinfo               | 53 +++++++++++++++++++++++++++++++++++++++
>  gdb/mi/mi-cmd-info.c              | 29 +++++++++++++++++++++
>  gdb/mi/mi-cmds.c                  |  1 +
>  gdb/mi/mi-cmds.h                  |  1 +
>  gdb/mi/mi-main.c                  |  1 +
>  gdb/testsuite/gdb.mi/mi-i-cmd.exp | 46 +++++++++++++++++++++++++++++++++
>  7 files changed, 134 insertions(+)
>  create mode 100644 gdb/testsuite/gdb.mi/mi-i-cmd.exp
> 
> diff --git a/gdb/NEWS b/gdb/NEWS
> index 9fc3638..e61c79f 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -153,6 +153,9 @@ show startup-with-shell
>  
>    ** All MI commands now accept an optional "--language" option.
>  
> +  ** The new command -info-gdb-mi-command allows the user to determine
> +     whether a GDB/MI command is supported or not.
> +
>    ** The -trace-save MI command can optionally save trace buffer in Common
>       Trace Format now.
>  
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index 19e9aa5..41856b4 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -35069,6 +35069,57 @@ default shows this information when you start an interactive session.
>  (gdb)
>  @end smallexample
>  
> +@subheading The @code{-info-gdb-mi-command} Command
> +@cindex @code{-info-gdb-mi-command}
> +@findex -info-gdb-mi-command
> +
> +@subsubheading Synopsis
> +
> +@smallexample
> + -info-gdb-mi-command @var{cmd_name}
> +@end smallexample
> +
> +Query support for the @sc{gdb/mi} command named @var{cmd_name}.
> +
> +Note that the dash (@code{-}) starting all @sc{gdb/mi} commands
> +is technically not part of the command name (@pxref{GDB/MI Input
> +Syntax}), and thus should be omitted in @var{cmd_name}.  However,
> +for ease of use, this command also accepts the form with the leading
> +dash.
> +
> +@subsubheading @value{GDBN} Command
> +
> +There is no corresponding @value{GDBN} command.
> +
> +@subsubheading Result
> +
> +The result is a tuple.  There is currently only one field:
> +
> +@table @samp
> +@item exists
> +This field is equal to @code{"true"} if the @sc{gdb/mi} command exists,
> +@code{"false"} otherwise.
> +
> +@end table
> +
> +@subsubheading Example
> +
> +Here is an example where the @sc{gdb/mi} command does not exist:
> +
> +@smallexample
> +-info-gdb-mi-command unsupported-command
> +^done,command=@{exists="false"@}
> +@end smallexample
> +
> +@noindent
> +And here is an example where the @sc{gdb/mi} command is known
> +to the debugger:
> +
> +@smallexample
> +-info-gdb-mi-command symbol-list-lines
> +^done,command=@{exists="true"@}
> +@end smallexample
> +
>  @subheading The @code{-list-features} Command
>  @findex -list-features
>  
> @@ -35122,6 +35173,8 @@ exceptions: @code{-info-ada-exceptions}, @code{-catch-assert} and
>  @item language-option
>  Indicates that all @sc{gdb/mi} commands accept the @option{--language}
>  option (@pxref{Context management}).
> +@item info-gdb-mi-command
> +Indicates support for the @code{-info-gdb-mi-command} command.
>  @end table
>  
>  @subheading The @code{-list-target-features} Command
> diff --git a/gdb/mi/mi-cmd-info.c b/gdb/mi/mi-cmd-info.c
> index aa4d210..0fce25a 100644
> --- a/gdb/mi/mi-cmd-info.c
> +++ b/gdb/mi/mi-cmd-info.c
> @@ -71,6 +71,35 @@ mi_cmd_info_ada_exceptions (char *command, char **argv, int argc)
>    do_cleanups (old_chain);
>  }
>  
> +/* Implement the "-info-gdb-mi-command" GDB/MI command.  */
> +
> +void
> +mi_cmd_info_gdb_mi_command (char *command, char **argv, int argc)
> +{
> +  const char *cmd_name;
> +  struct mi_cmd *cmd;
> +  struct ui_out *uiout = current_uiout;
> +  struct cleanup *old_chain;
> +
> +  /* This command takes exactly one argument.  */
> +  if (argc != 1)
> +    error (_("Usage: -info-gdb-mi-command MI_COMMAND_NAME"));
> +  cmd_name = argv[0];
> +
> +  /* Normally, the command name (aka the "operation" in the GDB/MI
> +     grammar), does not include the leading '-' (dash).  But for
> +     the user's convenience, allow the user to specify the command
> +     name to be with or without that leading dash.  */
> +  if (cmd_name[0] == '-')
> +    cmd_name++;
> +
> +  cmd = mi_lookup (cmd_name);
> +
> +  old_chain = make_cleanup_ui_out_tuple_begin_end (uiout, "command");
> +  ui_out_field_string (uiout, "exists", cmd != NULL ? "true" : "false");
> +  do_cleanups (old_chain);
> +}
> +
>  void
>  mi_cmd_info_os (char *command, char **argv, int argc)
>  {
> diff --git a/gdb/mi/mi-cmds.c b/gdb/mi/mi-cmds.c
> index c536d8a..58a8b89 100644
> --- a/gdb/mi/mi-cmds.c
> +++ b/gdb/mi/mi-cmds.c
> @@ -125,6 +125,7 @@ static struct mi_cmd mi_cmds[] =
>    DEF_MI_CMD_MI ("inferior-tty-set", mi_cmd_inferior_tty_set),
>    DEF_MI_CMD_MI ("inferior-tty-show", mi_cmd_inferior_tty_show),
>    DEF_MI_CMD_MI ("info-ada-exceptions", mi_cmd_info_ada_exceptions),
> +  DEF_MI_CMD_MI ("info-gdb-mi-command", mi_cmd_info_gdb_mi_command),
>    DEF_MI_CMD_MI ("info-os", mi_cmd_info_os),
>    DEF_MI_CMD_MI ("interpreter-exec", mi_cmd_interpreter_exec),
>    DEF_MI_CMD_MI ("list-features", mi_cmd_list_features),
> diff --git a/gdb/mi/mi-cmds.h b/gdb/mi/mi-cmds.h
> index cb8aac1..4ea95fa 100644
> --- a/gdb/mi/mi-cmds.h
> +++ b/gdb/mi/mi-cmds.h
> @@ -74,6 +74,7 @@ extern mi_cmd_argv_ftype mi_cmd_gdb_exit;
>  extern mi_cmd_argv_ftype mi_cmd_inferior_tty_set;
>  extern mi_cmd_argv_ftype mi_cmd_inferior_tty_show;
>  extern mi_cmd_argv_ftype mi_cmd_info_ada_exceptions;
> +extern mi_cmd_argv_ftype mi_cmd_info_gdb_mi_command;
>  extern mi_cmd_argv_ftype mi_cmd_info_os;
>  extern mi_cmd_argv_ftype mi_cmd_interpreter_exec;
>  extern mi_cmd_argv_ftype mi_cmd_list_features;
> diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
> index 793204d..48c8d09 100644
> --- a/gdb/mi/mi-main.c
> +++ b/gdb/mi/mi-main.c
> @@ -1817,6 +1817,7 @@ mi_cmd_list_features (char *command, char **argv, int argc)
>        ui_out_field_string (uiout, NULL, "ada-task-info");
>        ui_out_field_string (uiout, NULL, "ada-exceptions");
>        ui_out_field_string (uiout, NULL, "language-option");
> +      ui_out_field_string (uiout, NULL, "info-gdb-mi-command");
>        
>  #if HAVE_PYTHON
>        if (gdb_python_initialized)
> diff --git a/gdb/testsuite/gdb.mi/mi-i-cmd.exp b/gdb/testsuite/gdb.mi/mi-i-cmd.exp
> new file mode 100644
> index 0000000..d460559
> --- /dev/null
> +++ b/gdb/testsuite/gdb.mi/mi-i-cmd.exp
> @@ -0,0 +1,46 @@
> +# Copyright 2013 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +load_lib mi-support.exp
> +set MIFLAGS "-i=mi"
> +
> +gdb_exit
> +if [mi_gdb_start] {
> +    continue
> +}
> +
> +# First, verify that the debugger correctly advertises support
> +# for the -info-gdb-mi-command command.
> +mi_gdb_test "-list-features" \
> +            "\\^done,features=\\\[.*\"info-gdb-mi-command\".*\\\]" \
> +            "-list-features should include \"info-gdb-mi-command\""
> +
> +mi_gdb_test "-info-gdb-mi-command unsupported-command" \
> +            "\\^done,command=\\\{exists=\"false\"\\\}" \
> +            "-info-gdb-mi-command unsupported-command"
> +
> +# Same test as above, but including the leading '-' in the command name.
> +mi_gdb_test "-info-gdb-mi-command -unsupported-command" \
> +            "\\^done,command=\\\{exists=\"false\"\\\}" \
> +            "-info-gdb-mi-command -unsupported-command"
> +
> +mi_gdb_test "-info-gdb-mi-command symbol-list-lines" \
> +            "\\^done,command=\\\{exists=\"true\"\\\}" \
> +            "-info-gdb-mi-command symbol-list-lines"
> +
> +# Same test as above, but including the leading '-' in the command name.
> +mi_gdb_test "-info-gdb-mi-command -symbol-list-lines" \
> +            "\\^done,command=\\\{exists=\"true\"\\\}" \
> +            "-info-gdb-mi-command -symbol-list-lines"
> -- 
> 1.8.1.2
> 

> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index f0662ff..4227f31 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -35088,8 +35088,13 @@ default shows this information when you start an interactive session.
>   -info-gdb-mi-command @var{cmd_name}
>  @end smallexample
>  
> -Query support for the @sc{gdb/mi} command named @var{cmd_name}
> -(the leading dash (@code{-}) in the command name should be omitted).
> +Query support for the @sc{gdb/mi} command named @var{cmd_name}.
> +
> +Note that the dash (@code{-}) starting all @sc{gdb/mi} commands
> +is technically not part of the command name (@pxref{GDB/MI Input
> +Syntax}), and thus should be omitted in @var{cmd_name}.  However,
> +for ease of use, this command also accepts the form with the leading
> +dash.
>  
>  @subsubheading @value{GDBN} Command
>  
> @@ -35120,6 +35120,7 @@ Here is an example where the @sc{gdb/mi} command does not exist:
>  ^done,command=@{exists="false"@}
>  @end smallexample
>  
> +@noindent
>  And here is an example where the @sc{gdb/mi} command is known
>  to the debugger:
>  
> diff --git a/gdb/mi/mi-cmd-info.c b/gdb/mi/mi-cmd-info.c
> index 18f4927..0fce25a 100644
> --- a/gdb/mi/mi-cmd-info.c
> +++ b/gdb/mi/mi-cmd-info.c
> @@ -85,6 +85,14 @@ mi_cmd_info_gdb_mi_command (char *command, char **argv, int argc)
>    if (argc != 1)
>      error (_("Usage: -info-gdb-mi-command MI_COMMAND_NAME"));
>    cmd_name = argv[0];
> +
> +  /* Normally, the command name (aka the "operation" in the GDB/MI
> +     grammar), does not include the leading '-' (dash).  But for
> +     the user's convenience, allow the user to specify the command
> +     name to be with or without that leading dash.  */
> +  if (cmd_name[0] == '-')
> +    cmd_name++;
> +
>    cmd = mi_lookup (cmd_name);
>  
>    old_chain = make_cleanup_ui_out_tuple_begin_end (uiout, "command");
> diff --git a/gdb/testsuite/gdb.mi/mi-i-cmd.exp b/gdb/testsuite/gdb.mi/mi-i-cmd.exp
> index 5285d31..d460559 100644
> --- a/gdb/testsuite/gdb.mi/mi-i-cmd.exp
> +++ b/gdb/testsuite/gdb.mi/mi-i-cmd.exp
> @@ -31,7 +31,16 @@ mi_gdb_test "-info-gdb-mi-command unsupported-command" \
>              "\\^done,command=\\\{exists=\"false\"\\\}" \
>              "-info-gdb-mi-command unsupported-command"
>  
> +# Same test as above, but including the leading '-' in the command name.
> +mi_gdb_test "-info-gdb-mi-command -unsupported-command" \
> +            "\\^done,command=\\\{exists=\"false\"\\\}" \
> +            "-info-gdb-mi-command -unsupported-command"
> +
>  mi_gdb_test "-info-gdb-mi-command symbol-list-lines" \
>              "\\^done,command=\\\{exists=\"true\"\\\}" \
>              "-info-gdb-mi-command symbol-list-lines"
>  
> +# Same test as above, but including the leading '-' in the command name.
> +mi_gdb_test "-info-gdb-mi-command -symbol-list-lines" \
> +            "\\^done,command=\\\{exists=\"true\"\\\}" \
> +            "-info-gdb-mi-command -symbol-list-lines"


-- 
Joel

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [RFA 1/2] New GDB/MI command "-info-gdb-mi-command"
  2013-12-02  3:26                               ` Joel Brobecker
@ 2013-12-02  3:51                                 ` Eli Zaretskii
  2013-12-02  4:41                                   ` Joel Brobecker
  0 siblings, 1 reply; 42+ messages in thread
From: Eli Zaretskii @ 2013-12-02  3:51 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

> Date: Mon, 2 Dec 2013 07:26:15 +0400
> From: Joel Brobecker <brobecker@adacore.com>
> 
> Kind request for review. The code itself is fairly straightforward,
> I think. It's more about making sure that we're defining the right
> command...
> 
> Thank you!

I already approved the documentation parts, didn't I?  If not, you
hereby have my approval.

Thanks.

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [RFA 1/2] New GDB/MI command "-info-gdb-mi-command"
  2013-12-02  3:51                                 ` Eli Zaretskii
@ 2013-12-02  4:41                                   ` Joel Brobecker
  0 siblings, 0 replies; 42+ messages in thread
From: Joel Brobecker @ 2013-12-02  4:41 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

> I already approved the documentation parts, didn't I?  If not, you
> hereby have my approval.

Yes, you did, thank you! I  should have  been more explicit here.
I'm waiting for code review.

Cheers :)
-- 
Joel

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [RFA 1/2] New GDB/MI command "-info-gdb-mi-command"
  2013-11-19  4:35                             ` Joel Brobecker
  2013-11-19 16:11                               ` Eli Zaretskii
  2013-12-02  3:26                               ` Joel Brobecker
@ 2013-12-02 14:53                               ` Pedro Alves
  2013-12-03  4:06                                 ` pushed: " Joel Brobecker
  2 siblings, 1 reply; 42+ messages in thread
From: Pedro Alves @ 2013-12-02 14:53 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: Eli Zaretskii, gdb-patches

On 11/19/2013 04:10 AM, Joel Brobecker wrote:

> Updated patch attached. And for review convenience, I am also attaching
> a diff of the changes I made on top of path #1 (to get to the updated
> patch).
> 
> gdb/ChangeLog:
> 
>         * mi/mi-cmds.h (mi_cmd_info_gdb_mi_command): Declare.
>         * mi/mi-cmd-info.c (mi_cmd_info_gdb_mi_command): New function.
>         * mi/mi-cmds.c (mi_cmds): Add -info-gdb-mi-command command.
>         * mi/mi-main.c (mi_cmd_list_features): Add "info-gdb-mi-command"
>         field to output of "-list-features".
> 
>         * NEWS: Add entry for new -info-gdb-mi-command.
> 
> gdb/doc/ChangeLog:
> 
>         * gdb.texinfo (GDB/MI Miscellaneous Commands): Document
>         the new -info-gdb-mi-command GDB/MI command.  Document
>         the meaning of "-info-gdb-mi-command" in the output of
>         -list-features.
> 
> gdb/testsuite/ChangeLog:
> 
>         * gdb.mi/mi-i-cmd.exp: New file.
> 
> Re-tested on x86_64-linux.  OK to commit?
> 

Looks fine to me.

(I don't have a strong opinion about the dash issue.)

> +# First, verify that the debugger correctly advertises support
> +# for the -info-gdb-mi-command command.
> +mi_gdb_test "-list-features" \
> +            "\\^done,features=\\\[.*\"info-gdb-mi-command\".*\\\]" \
> +            "-list-features should include \"info-gdb-mi-command\""

Nit, I'd suggest:

           "-list-features includes \"info-gdb-mi-command\""

-- 
Pedro Alves

^ permalink raw reply	[flat|nested] 42+ messages in thread

* pushed: [RFA 1/2] New GDB/MI command "-info-gdb-mi-command"
  2013-12-02 14:53                               ` Pedro Alves
@ 2013-12-03  4:06                                 ` Joel Brobecker
  0 siblings, 0 replies; 42+ messages in thread
From: Joel Brobecker @ 2013-12-03  4:06 UTC (permalink / raw)
  To: gdb-patches

> > gdb/ChangeLog:
> > 
> >         * mi/mi-cmds.h (mi_cmd_info_gdb_mi_command): Declare.
> >         * mi/mi-cmd-info.c (mi_cmd_info_gdb_mi_command): New function.
> >         * mi/mi-cmds.c (mi_cmds): Add -info-gdb-mi-command command.
> >         * mi/mi-main.c (mi_cmd_list_features): Add "info-gdb-mi-command"
> >         field to output of "-list-features".
> > 
> >         * NEWS: Add entry for new -info-gdb-mi-command.
> > 
> > gdb/doc/ChangeLog:
> > 
> >         * gdb.texinfo (GDB/MI Miscellaneous Commands): Document
> >         the new -info-gdb-mi-command GDB/MI command.  Document
> >         the meaning of "-info-gdb-mi-command" in the output of
> >         -list-features.
> > 
> > gdb/testsuite/ChangeLog:
> > 
> >         * gdb.mi/mi-i-cmd.exp: New file.
> > 
> > Re-tested on x86_64-linux.  OK to commit?
> > 
> 
> Looks fine to me.
> 
> (I don't have a strong opinion about the dash issue.)
> 
> > +# First, verify that the debugger correctly advertises support
> > +# for the -info-gdb-mi-command command.
> > +mi_gdb_test "-list-features" \
> > +            "\\^done,features=\\\[.*\"info-gdb-mi-command\".*\\\]" \
> > +            "-list-features should include \"info-gdb-mi-command\""
> 
> Nit, I'd suggest:
> 
>            "-list-features includes \"info-gdb-mi-command\""
> 

Thanks, Pedro. The suggestion does seem better to me too, so I applied
it. Patch now in.

-- 
Joel

^ permalink raw reply	[flat|nested] 42+ messages in thread

* pushed: [RFA 2/2] Add "undefined-command" error code at end of ^error result...
  2013-11-20  3:46                             ` Joel Brobecker
@ 2013-12-03  4:08                               ` Joel Brobecker
  0 siblings, 0 replies; 42+ messages in thread
From: Joel Brobecker @ 2013-12-03  4:08 UTC (permalink / raw)
  To: gdb-patches

> > > gdb/ChangeLog:
> > > 
> > >         (from Pedro Alves  <palves@redhat.com>)
> > >         (from Joel Brobecker  <brobecker@adacore.com>)
> > >         * exceptions.h (enum_errors) <UNKNOWN_COMMAND_ERROR>: New enum.
> > >         * mi/mi-parse.c (mi_parse): Thow UNKNOWN_COMMAND_ERROR instead
> > 
> > "Throw"
> > 
> > >         of a regular error when the GDB/MI command does not exist.
> > >         * mi/mi-main.c (mi_cmd_list_features): Add
> > >         "undefined-command-error-code".
> > >         (mi_print_exception): Print an "undefined-command"
> > >         error code if EXCEPTION.ERROR in UNKNOWN_COMMAND_ERROR.
> > 
> > s/in/is ?
> > 
> > >         * NEWS: Add entry documenting the new "code" variable in
> > >         "^error" result records.
> > > 
> > > gdb/doc/ChangeLog:
> > > 
> > >         * gdb.texinfo (GDB/MI Result Records): Fix the syntax of the
> > >         "^error" result record concerning the error message.  Document
> > >         the error code that may also be part of that result record.
> > >         (GDB/MI Miscellaneous Commands): Document the
> > >         "undefined-command-error-code" element in the output of
> > >         the "-list-features" GDB/MI command.
> > > 
> > > gdb/testsuite/ChangeLog:
> > > 
> > >         * gdb.mi/mi-undefined-cmd.exp: New testcase.
> > > 
> > > Tested on x86_64-linux.  OK to commit?

Now pushed.

-- 
Joel

^ permalink raw reply	[flat|nested] 42+ messages in thread

end of thread, other threads:[~2013-12-03  4:08 UTC | newest]

Thread overview: 42+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-10 17:16 [RFC] Add ada-exception-catchpoints to -list-features command output Joel Brobecker
2013-11-10 22:16 ` Eli Zaretskii
2013-11-12 11:25   ` Joel Brobecker
2013-11-12 16:39     ` Eli Zaretskii
2013-11-13  3:02       ` Joel Brobecker
2013-11-11 15:22 ` Tom Tromey
2013-11-12  9:18   ` Joel Brobecker
2013-11-12 12:11   ` [RFC] New GDB/MI command "-info-gdb-mi-command" Joel Brobecker
2013-11-12 17:04     ` Eli Zaretskii
2013-11-12 17:48       ` Joel Brobecker
2013-11-12 18:34         ` Eli Zaretskii
2013-11-13  3:19           ` Joel Brobecker
2013-11-12 21:17     ` André Pönitz
2013-11-13  2:47       ` Joel Brobecker
2013-11-14  0:36         ` André Pönitz
2013-11-14  9:48           ` Joel Brobecker
2013-11-14 18:31             ` André Pönitz
2013-11-14 19:03         ` Pedro Alves
2013-11-14 19:37           ` Pedro Alves
2013-11-14 20:30             ` Tom Tromey
2013-11-15  5:35               ` Joel Brobecker
2013-11-15 12:39                 ` Pedro Alves
2013-11-15 14:38                   ` Joel Brobecker
2013-11-15 14:40                     ` Pedro Alves
2013-11-18 17:12                       ` [RFA GDB/MI] Help determine if GDB/MI command exists or not Joel Brobecker
2013-11-18 17:13                         ` [RFA 1/2] New GDB/MI command "-info-gdb-mi-command" Joel Brobecker
2013-11-18 17:29                           ` Eli Zaretskii
2013-11-19  4:35                             ` Joel Brobecker
2013-11-19 16:11                               ` Eli Zaretskii
2013-12-02  3:26                               ` Joel Brobecker
2013-12-02  3:51                                 ` Eli Zaretskii
2013-12-02  4:41                                   ` Joel Brobecker
2013-12-02 14:53                               ` Pedro Alves
2013-12-03  4:06                                 ` pushed: " Joel Brobecker
2013-11-18 17:21                         ` [RFA 2/2] Add "undefined-command" error code at end of ^error result Joel Brobecker
2013-11-18 17:29                           ` Eli Zaretskii
2013-11-19  6:02                             ` Joel Brobecker
2013-11-19 16:16                               ` Eli Zaretskii
2013-11-19 11:19                           ` Pedro Alves
2013-11-20  3:46                             ` Joel Brobecker
2013-12-03  4:08                               ` pushed: " Joel Brobecker
2013-11-19 15:05                         ` [RFA GDB/MI] Help determine if GDB/MI command exists or not Pedro Alves

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).