[v2,0/4] Introduce the "with" command
Commit Message
On 6/19/19 1:34 AM, Philippe Waroquiers wrote:
>> New in v2:
> I played a little bit with this version, no bug encountered.
>
Thanks much, again.
> 2 small nits in the error message for unknown 'with settings':
> (gdb) with xxxx yyyy -- echo coucou
> Undefined withcommand: "xxxx". Try "help wit".
> (gdb)
>
> (this message is produced by lookup_cmd, that is not too
> much 'with' aware it seems ...)
Eh, somehow I missed adding a testcase for this scenario.
Thanks for noticing this.
I had code in with_command_1 to throw on unknown setting:
if (set_cmd == nullptr)
error (_("Unknown setting %s"), cmd);
but it's not reachable, because I'm passing "0" as
allow_unknown argument to lookup_cmd... I tried passing
"1" instead, and, of course that error becomes reachable.
However, if the setting is a prefixed setting, like
"with print elements", then lookup_cmd still throws
an error, near the bottom, from the else branch:
if (c->prefixlist && **line && !c->allow_unknown)
undef_cmd_error (c->prefixname, *line);
regardless of the allow_unknown parameter.
This made me reconsider, and I'm thinking that indeed,
passing allow_unknown as false is the right thing to do,
so that we get consistent error messages:
(gdb) with foo
Undefined set command: "foo". Try "help set".
(gdb) with print foo
Undefined set print command: "foo". Try "help set print".
(gdb) maint with foo
Undefined maintenance set command: "foo". Try "help maintenance set".
(gdb) maint with test-settings foo
Undefined maintenance set test-settings command: "foo". Try "help maintenance set test-settings".
I'm thinking that the errors talking about "set" instead of
"with" can be seen as a feature. If you consider the prefixed
case, like "with print foo", the error is telling you where
to look at the available settings for that prefix.
Looking at the lookup_cmd code, I realized that I was missing
a test for ambiguous settings too. Now added.
The funny missing space and 'h' were because we're supposed
to include a space in the CMDTYPE argument passed to
lookup_cmd, and I was passing "with", with no space. I added a new
parameter to with_command_1, so that we can pass down
"maintenance with " too. The completer function already had
the same parameter.
WDYT?
From 4db38e12c0b393231a7af955a1bd4e9573bfe19e Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Wed, 19 Jun 2019 13:29:08 +0100
Subject: [PATCH] fix
---
gdb/cli/cli-cmds.c | 14 +++++++-------
gdb/cli/cli-cmds.h | 8 +++++---
gdb/maint.c | 2 +-
gdb/testsuite/gdb.base/with.exp | 18 ++++++++++++++++++
4 files changed, 31 insertions(+), 11 deletions(-)
Comments
On Wed, 2019-06-19 at 14:05 +0100, Pedro Alves wrote:
> I'm thinking that the errors talking about "set" instead of
> "with" can be seen as a feature. If you consider the prefixed
> case, like "with print foo", the error is telling you where
> to look at the available settings for that prefix.
Yes, I think this is a good approach, and the error messages
are very clear.
Thanks
Philippe
@@ -214,7 +214,8 @@ show_command (const char *arg, int from_tty)
/* See cli/cli-cmds.h. */
void
-with_command_1 (cmd_list_element *setlist, const char *args, int from_tty)
+with_command_1 (const char *set_cmd_prefix,
+ cmd_list_element *setlist, const char *args, int from_tty)
{
const char *delim = strstr (args, "--");
const char *nested_cmd = nullptr;
@@ -225,11 +226,10 @@ with_command_1 (cmd_list_element *setlist, const char *args, int from_tty)
if (delim == nullptr || *skip_spaces (&delim[2]) == '\0')
nested_cmd = repeat_previous ();
- const char *cmd = args;
- cmd_list_element *set_cmd = lookup_cmd (&args, setlist, "with", 0, 1);
-
- if (set_cmd == nullptr)
- error (_("Unknown setting %s"), cmd);
+ cmd_list_element *set_cmd = lookup_cmd (&args, setlist, set_cmd_prefix,
+ /*allow_unknown=*/ 0,
+ /*ignore_help_classes=*/ 1);
+ gdb_assert (set_cmd != nullptr);
if (set_cmd->var == nullptr)
error (_("Cannot use this setting with the \"with\" command"));
@@ -308,7 +308,7 @@ with_command_completer_1 (const char *set_cmd_prefix,
static void
with_command (const char *args, int from_tty)
{
- with_command_1 (setlist, args, from_tty);
+ with_command_1 ("set ", setlist, args, from_tty);
}
/* "with" command completer. */
@@ -143,9 +143,11 @@ extern int source_verbose;
extern int trace_commands;
/* Common code for the "with" and "maintenance with" commands.
- SETLIST is the command list for corresponding "set" command: i.e.,
- "set" or "maintenance set". */
-extern void with_command_1 (cmd_list_element *setlist,
+ SET_CMD_PREFIX is the spelling of the corresponding "set" command
+ prefix: i.e., "set " or "maintenance set ". SETLIST is the command
+ element for the same "set" command prefix. */
+extern void with_command_1 (const char *set_cmd_prefix,
+ cmd_list_element *setlist,
const char *args, int from_tty);
/* Common code for the completers of the "with" and "maintenance with"
@@ -640,7 +640,7 @@ maintenance_show_cmd (const char *args, int from_tty)
static void
maintenance_with_cmd (const char *args, int from_tty)
{
- with_command_1 (maintenance_set_cmdlist, args, from_tty);
+ with_command_1 ("maintenance set ", maintenance_set_cmdlist, args, from_tty);
}
/* "maintenance with" command completer. */
@@ -220,6 +220,24 @@ with_test_prefix "run control" {
# Check errors.
with_test_prefix "errors" {
+ # Try both an unknown root setting and an unknown prefixed
+ # setting. The errors come from different locations in the
+ # sources.
+ gdb_test "with xxxx yyyy" \
+ "Undefined set command: \"xxxx\". Try \"help set\"\\."
+ gdb_test "with print xxxx yyyy" \
+ "Undefined set print command: \"xxxx yyyy\". Try \"help set print\"\\."
+ # Try one error case for "maint with", to make sure the right
+ # "maintenance with" prefix is shown.
+ gdb_test "maint with xxxx yyyy" \
+ "Undefined maintenance set command: \"xxxx\". Try \"help maintenance set\"\\."
+
+ # Try ambiguous settings.
+ gdb_test "with w" \
+ "Ambiguous set command \"w\": watchdog, width, write\\."
+ gdb_test "with print m" \
+ "Ambiguous set print command \"m\": max-depth, max-symbolic-offset\\."
+
gdb_test "with variable xxx=1" \
"Cannot use this setting with the \"with\" command"