* [RFC PATCH] gdb: add command to clear value history
@ 2022-10-19 3:39 Rustam Kovhaev
2022-10-19 8:30 ` Bruno Larsen
2022-10-20 18:44 ` Tom Tromey
0 siblings, 2 replies; 4+ messages in thread
From: Rustam Kovhaev @ 2022-10-19 3:39 UTC (permalink / raw)
To: gdb-patches; +Cc: rkovhaev
Hello,
While debugging an application I wanted to clear the value history and I
could not find an option to do it.
I might be the only one who gets lost in the value history and I am not
sure if anyone else needs it, but I thought it would be nice to have it.
And I hope this does not break anything.
Please let me know what you think. Thanks!
Let's have the functionality to clear the value history.
Introduce 'set values clear' command to clear the value history.
Signed-off-by: Rustam Kovhaev <rkovhaev@gmail.com>
---
gdb/value.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/gdb/value.c b/gdb/value.c
index 605e52dee82..726b5ef9084 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -1971,6 +1971,15 @@ show_values (const char *num_exp, int from_tty)
if (from_tty && num_exp)
set_repeat_arguments ("+");
}
+
+static struct cmd_list_element *setvallist;
+
+static void
+clear_values (const char *ignore, int from_tty)
+{
+ value_history.clear ();
+}
+
\f
enum internalvar_kind
{
@@ -4343,6 +4352,13 @@ Convenience functions are defined via the Python API."
Elements of value history around item number IDX (or last ten)."),
&showlist);
+ add_cmd("clear", no_class, clear_values, _("\
+Clear value history."), &setvallist);
+
+ add_basic_prefix_cmd ("values", class_support, _("\
+Generic command for setting value history parameters"),
+ &setvallist, 0, &setlist);
+
add_com ("init-if-undefined", class_vars, init_if_undefined_command, _("\
Initialize a convenience variable if necessary.\n\
init-if-undefined VARIABLE = EXPRESSION\n\
--
2.37.3
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC PATCH] gdb: add command to clear value history
2022-10-19 3:39 [RFC PATCH] gdb: add command to clear value history Rustam Kovhaev
@ 2022-10-19 8:30 ` Bruno Larsen
2022-10-20 18:44 ` Tom Tromey
1 sibling, 0 replies; 4+ messages in thread
From: Bruno Larsen @ 2022-10-19 8:30 UTC (permalink / raw)
To: Rustam Kovhaev, gdb-patches
On 19/10/2022 05:39, Rustam Kovhaev via Gdb-patches wrote:
> Hello,
Hi Rustam!
Thanks for this patch! I am indifferent to the new command, as I never
needed it but I can't see it impacting anything else, so I'm fine with
it being added.
However, the command "clear" already exists. It clears all breakpoints.
You could either add "values" as a subcommand to the existing command,
by having it added around gdb/breakpoint.c:14434, but doing so might
change how the bare "clear" command works (I'm not sure here), or add
something like "values clear" instead, which I think it's a bit too big.
This specific point I'll defer to other maintainers on helping you.
I also noticed a few specific things that I'll explain inline.
> While debugging an application I wanted to clear the value history and I
> could not find an option to do it.
> I might be the only one who gets lost in the value history and I am not
> sure if anyone else needs it, but I thought it would be nice to have it.
> And I hope this does not break anything.
> Please let me know what you think. Thanks!
>
> Let's have the functionality to clear the value history.
> Introduce 'set values clear' command to clear the value history.
GDB tends to prefer a more objective commit message style, even if it
comes from a personal need. Your message is fine for the RFC but it
needs to be iterated for it to go in. See commits
e4014689b9a1b9aa0dde8f8a358401774566fe8b and
70175292616118bad315296ba6180f375326bb6c for some exaples.
>
> Signed-off-by: Rustam Kovhaev <rkovhaev@gmail.com>
We don't really use Signed-off-by tags in GDB.
> ---
> gdb/value.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/gdb/value.c b/gdb/value.c
> index 605e52dee82..726b5ef9084 100644
> --- a/gdb/value.c
> +++ b/gdb/value.c
> @@ -1971,6 +1971,15 @@ show_values (const char *num_exp, int from_tty)
> if (from_tty && num_exp)
> set_repeat_arguments ("+");
> }
> +
> +static struct cmd_list_element *setvallist;
> +
> +static void
> +clear_values (const char *ignore, int from_tty)
> +{
> + value_history.clear ();
> +}
> +
> \f
> enum internalvar_kind
> {
> @@ -4343,6 +4352,13 @@ Convenience functions are defined via the Python API."
> Elements of value history around item number IDX (or last ten)."),
> &showlist);
>
> + add_cmd("clear", no_class, clear_values, _("\
> +Clear value history."), &setvallist);
You forgot to add a space before ( here.
> +
> + add_basic_prefix_cmd ("values", class_support, _("\
> +Generic command for setting value history parameters"),
> + &setvallist, 0, &setlist);
> +
Since you are using &setvallist and &setlist here, your command ends up
being "set values clear" instead of "clear values". You probably want to
use &cmdlist instead.
> add_com ("init-if-undefined", class_vars, init_if_undefined_command, _("\
> Initialize a convenience variable if necessary.\n\
> init-if-undefined VARIABLE = EXPRESSION\n\
It would be good if you added a test for this command. I think it would
go well in gdb.base/printcmd.exp, adding some extra lines like
gdb_test_no_output "clear values" "test clearing values"
gdb_test "print 1" ".1 = 1" "test that values were cleared"
With all this being said, I like the direction of your patch, I hope to
see it going upstream soon :-)
Cheers,
Bruno
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC PATCH] gdb: add command to clear value history
2022-10-19 3:39 [RFC PATCH] gdb: add command to clear value history Rustam Kovhaev
2022-10-19 8:30 ` Bruno Larsen
@ 2022-10-20 18:44 ` Tom Tromey
2022-10-23 19:53 ` Rustam Kovhaev
1 sibling, 1 reply; 4+ messages in thread
From: Tom Tromey @ 2022-10-20 18:44 UTC (permalink / raw)
To: Rustam Kovhaev via Gdb-patches; +Cc: Rustam Kovhaev
>>>>> "Rustam" == Rustam Kovhaev via Gdb-patches <gdb-patches@sourceware.org> writes:
Rustam> While debugging an application I wanted to clear the value history and I
Rustam> could not find an option to do it.
Rustam> I might be the only one who gets lost in the value history and I am not
Rustam> sure if anyone else needs it, but I thought it would be nice to have it.
Rustam> And I hope this does not break anything.
Rustam> Please let me know what you think. Thanks!
Thanks for the patch.
In addition to what Bruno said, a new feature should have a
documentation patch and also a note in NEWS.
Rustam> +static void
Rustam> +clear_values (const char *ignore, int from_tty)
Rustam> +{
Rustam> + value_history.clear ();
This is indented too much.
Tom
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC PATCH] gdb: add command to clear value history
2022-10-20 18:44 ` Tom Tromey
@ 2022-10-23 19:53 ` Rustam Kovhaev
0 siblings, 0 replies; 4+ messages in thread
From: Rustam Kovhaev @ 2022-10-23 19:53 UTC (permalink / raw)
To: Tom Tromey, Bruno Larsen; +Cc: Rustam Kovhaev via Gdb-patches
On Thu, Oct 20, 2022 at 12:44:38PM -0600, Tom Tromey wrote:
>
> Thanks for the patch.
>
> In addition to what Bruno said, a new feature should have a
> documentation patch and also a note in NEWS.
>
Hi Bruno, Tom,
Thanks for the review and the comments. I will send a proper patch.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-10-23 19:56 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-19 3:39 [RFC PATCH] gdb: add command to clear value history Rustam Kovhaev
2022-10-19 8:30 ` Bruno Larsen
2022-10-20 18:44 ` Tom Tromey
2022-10-23 19:53 ` Rustam Kovhaev
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).