Add completer for skip numbers
Commit Message
Add completer to various commands that accept skip numbers:
- skip enable
- skip disable
- skip delete
- info skip
These commands also accept ranges, but I am not too sure of how to do
that properly, so I went for the simpler goal of complete just numbers.
A future idea would be to make a re-usable and well-tested completer for
numbers and ranges. I think it could at least be re-used for breakpoint
numbers (for example with the "enable breakpoints" command).
gdb/ChangeLog:
* skip.c (complete_skip_number): New function.
(_initialize_step_skip): Add completers to some skip commands.
gdb/testsuite/ChangeLog:
* gdb.base/skip.exp: Add standard_testfile. Add "skip delete"
completer tests.
---
gdb/skip.c | 35 +++++++++++++++++++-----
gdb/testsuite/gdb.base/skip.exp | 48 +++++++++++++++++++++++++++++++++
2 files changed, 76 insertions(+), 7 deletions(-)
Comments
On 11/10/2018 05:39 PM, Simon Marchi wrote:
> Add completer to various commands that accept skip numbers:
>
> - skip enable
> - skip disable
> - skip delete
> - info skip
>
> These commands also accept ranges, but I am not too sure of how to do
> that properly, so I went for the simpler goal of complete just numbers.
>
> A future idea would be to make a re-usable and well-tested completer for
> numbers and ranges. I think it could at least be re-used for breakpoint
> numbers (for example with the "enable breakpoints" command).
And threads.
>
> gdb/ChangeLog:
>
> * skip.c (complete_skip_number): New function.
> (_initialize_step_skip): Add completers to some skip commands.
>
> gdb/testsuite/ChangeLog:
>
> * gdb.base/skip.exp: Add standard_testfile. Add "skip delete"
> completer tests.
> ---
> gdb/skip.c | 35 +++++++++++++++++++-----
> gdb/testsuite/gdb.base/skip.exp | 48 +++++++++++++++++++++++++++++++++
> 2 files changed, 76 insertions(+), 7 deletions(-)
>
> diff --git a/gdb/skip.c b/gdb/skip.c
> index 13db0f6b43c6..f2a1df79e8bf 100644
> --- a/gdb/skip.c
> +++ b/gdb/skip.c
> @@ -641,6 +641,23 @@ function_name_is_marked_for_skip (const char *function_name,
> return false;
> }
>
> +/* Completer for skip numbers. */
> +
> +static void
> +complete_skip_number (cmd_list_element *cmd,
> + completion_tracker &completer,
> + const char *text, const char *word)
> +{
> + size_t word_len = strlen (word);
> +
> + for (const skiplist_entry &entry : skiplist_entries)
> + {
> + gdb::unique_xmalloc_ptr<char> name (xstrprintf ("%d", entry.number ()));
> + if (strncmp (word, name.get (), word_len) == 0)
> + completer.add_completion (std::move (name));
> + }
> +}
> +
> void
> _initialize_step_skip (void)
> {
> @@ -676,28 +693,31 @@ If no function name is given, skip the current function."),
> &skiplist);
> set_cmd_completer (c, location_completer);
>
> - add_cmd ("enable", class_breakpoint, skip_enable_command, _("\
> + c = add_cmd ("enable", class_breakpoint, skip_enable_command, _("\
> Enable skip entries. You can specify numbers (e.g. \"skip enable 1 3\"), \
> ranges (e.g. \"skip enable 4-8\"), or both (e.g. \"skip enable 1 3 4-8\").\n\n\
> If you don't specify any numbers or ranges, we'll enable all skip entries.\n\n\
> Usage: skip enable [NUMBER | RANGE]..."),
> - &skiplist);
> + &skiplist);
> + set_cmd_completer (c, complete_skip_number);
>
> - add_cmd ("disable", class_breakpoint, skip_disable_command, _("\
> + c = add_cmd ("disable", class_breakpoint, skip_disable_command, _("\
> Disable skip entries. You can specify numbers (e.g. \"skip disable 1 3\"), \
> ranges (e.g. \"skip disable 4-8\"), or both (e.g. \"skip disable 1 3 4-8\").\n\n\
> If you don't specify any numbers or ranges, we'll disable all skip entries.\n\n\
> Usage: skip disable [NUMBER | RANGE]..."),
> - &skiplist);
> + &skiplist);
> + set_cmd_completer (c, complete_skip_number);
>
> - add_cmd ("delete", class_breakpoint, skip_delete_command, _("\
> + c = add_cmd ("delete", class_breakpoint, skip_delete_command, _("\
> Delete skip entries. You can specify numbers (e.g. \"skip delete 1 3\"), \
> ranges (e.g. \"skip delete 4-8\"), or both (e.g. \"skip delete 1 3 4-8\").\n\n\
> If you don't specify any numbers or ranges, we'll delete all skip entries.\n\n\
> Usage: skip delete [NUMBER | RANGES]..."),
> - &skiplist);
> + &skiplist);
> + set_cmd_completer (c, complete_skip_number);
>
> - add_info ("skip", info_skip_command, _("\
> + c = add_info ("skip", info_skip_command, _("\
> Display the status of skips. You can specify numbers (e.g. \"skip info 1 3\"), \
> ranges (e.g. \"skip info 4-8\"), or both (e.g. \"skip info 1 3 4-8\").\n\n\
> If you don't specify any numbers or ranges, we'll show all skips.\n\n\
> @@ -705,6 +725,7 @@ Usage: skip info [NUMBER | RANGES]...\n\
> The \"Type\" column indicates one of:\n\
> \tfile - ignored file\n\
> \tfunction - ignored function"));
> + set_cmd_completer (c, complete_skip_number);
>
> add_setshow_boolean_cmd ("skip", class_maintenance,
> &debug_skip, _("\
> diff --git a/gdb/testsuite/gdb.base/skip.exp b/gdb/testsuite/gdb.base/skip.exp
> index 223c93d0d9b6..ee328bf35c9b 100644
> --- a/gdb/testsuite/gdb.base/skip.exp
> +++ b/gdb/testsuite/gdb.base/skip.exp
> @@ -16,6 +16,8 @@
> # This file was written by Justin Lebar. (justin.lebar@gmail.com)
> # And further hacked on by Doug Evans. (dje@google.com)
>
> +standard_testfile
> +
> if { [prepare_for_testing "failed to prepare" "skip" \
> {skip.c skip1.c } \
> {debug nowarnings}] } {
> @@ -297,3 +299,49 @@ with_test_prefix "step using -fi + -fu" {
> gdb_test "step" ".*" "step 4"; # Skip over test_skip()
> gdb_test "step" "test_skip_file_and_function \\(\\) at.*" "step 5"; # Return from skip1_test_skip_file_and_function()
> }
> +
> +with_test_prefix "skip delete completion" {
> + global binfile
> + clean_restart "${binfile}"
> + if ![runto_main] {
> + fail "can't run to main"
> + return
> + }
> +
> + # Create a bunch of skips, don't care what they are.
> + for {set i 0} {$i < 12} {incr i} {
> + gdb_test "skip" ".*" "add skip $i"
> + }
> +
> + gdb_test "complete skip delete " \
> + [multi_line "skip delete 1" \
> + "skip delete 10" \
> + "skip delete 11" \
> + "skip delete 12" \
> + "skip delete 2" \
> + "skip delete 3" \
> + "skip delete 4" \
> + "skip delete 5" \
> + "skip delete 6" \
> + "skip delete 7" \
> + "skip delete 8" \
> + "skip delete 9"]
> + gdb_test "complete skip delete 1" \
> + [multi_line "skip delete 1" \
> + "skip delete 10" \
> + "skip delete 11" \
> + "skip delete 12"]
> + gdb_test "complete skip delete 1 " \
> + [multi_line "skip delete 1 1" \
> + "skip delete 1 10" \
> + "skip delete 1 11" \
> + "skip delete 1 12" \
> + "skip delete 1 2" \
> + "skip delete 1 3" \
> + "skip delete 1 4" \
> + "skip delete 1 5" \
> + "skip delete 1 6" \
> + "skip delete 1 7" \
> + "skip delete 1 8" \
> + "skip delete 1 9"]
> +}
Please use the lib/completion-support.exp routines
for testing this. Those exercise both the complete command
and actual TAB completion. Above you want to use
test_gdb_complete_multiple.
You should also add:
- a test that exercises unique completions like "skip delete 12",
with test_gdb_complete_unique.
- a test that exercises no completions at all, like "skip delete 2",
with test_gdb_complete_none.
- some test like "skip delete a1" to make sure we don't
mistakenly complete that a1 into a1/a10/a11/a12.
As for a range completer, doesn't e.g., "skip delete 1-2<tab>" already
try to complete the "2", and "skip delete 1-" present all the numbers?
I'd think so, given that '-' is part of the default word break chars
set (default_word_break_characters). You should add tests for that too,
IMO, since people will very naturally use it. Even if not super smart
(e.g. 2-[TAB] ideally wouldn't present "1"), it's still useful as is.
Thanks,
Pedro Alves
On 2018-11-10 14:07, Pedro Alves wrote:
> On 11/10/2018 05:39 PM, Simon Marchi wrote:
>> Add completer to various commands that accept skip numbers:
>>
>> - skip enable
>> - skip disable
>> - skip delete
>> - info skip
>>
>> These commands also accept ranges, but I am not too sure of how to do
>> that properly, so I went for the simpler goal of complete just
>> numbers.
>>
>> A future idea would be to make a re-usable and well-tested completer
>> for
>> numbers and ranges. I think it could at least be re-used for
>> breakpoint
>> numbers (for example with the "enable breakpoints" command).
>
> And threads.
Right.
> Please use the lib/completion-support.exp routines
> for testing this. Those exercise both the complete command
> and actual TAB completion. Above you want to use
> test_gdb_complete_multiple.
>
> You should also add:
>
> - a test that exercises unique completions like "skip delete 12",
> with test_gdb_complete_unique.
>
> - a test that exercises no completions at all, like "skip delete 2",
> with test_gdb_complete_none.
>
> - some test like "skip delete a1" to make sure we don't
> mistakenly complete that a1 into a1/a10/a11/a12.
>
> As for a range completer, doesn't e.g., "skip delete 1-2<tab>" already
> try to complete the "2", and "skip delete 1-" present all the numbers?
> I'd think so, given that '-' is part of the default word break chars
> set (default_word_break_characters). You should add tests for that
> too,
> IMO, since people will very naturally use it. Even if not super smart
> (e.g. 2-[TAB] ideally wouldn't present "1"), it's still useful as is.
Thanks for all the feedback, I hope I have addressed everything in v2.
Simon
@@ -641,6 +641,23 @@ function_name_is_marked_for_skip (const char *function_name,
return false;
}
+/* Completer for skip numbers. */
+
+static void
+complete_skip_number (cmd_list_element *cmd,
+ completion_tracker &completer,
+ const char *text, const char *word)
+{
+ size_t word_len = strlen (word);
+
+ for (const skiplist_entry &entry : skiplist_entries)
+ {
+ gdb::unique_xmalloc_ptr<char> name (xstrprintf ("%d", entry.number ()));
+ if (strncmp (word, name.get (), word_len) == 0)
+ completer.add_completion (std::move (name));
+ }
+}
+
void
_initialize_step_skip (void)
{
@@ -676,28 +693,31 @@ If no function name is given, skip the current function."),
&skiplist);
set_cmd_completer (c, location_completer);
- add_cmd ("enable", class_breakpoint, skip_enable_command, _("\
+ c = add_cmd ("enable", class_breakpoint, skip_enable_command, _("\
Enable skip entries. You can specify numbers (e.g. \"skip enable 1 3\"), \
ranges (e.g. \"skip enable 4-8\"), or both (e.g. \"skip enable 1 3 4-8\").\n\n\
If you don't specify any numbers or ranges, we'll enable all skip entries.\n\n\
Usage: skip enable [NUMBER | RANGE]..."),
- &skiplist);
+ &skiplist);
+ set_cmd_completer (c, complete_skip_number);
- add_cmd ("disable", class_breakpoint, skip_disable_command, _("\
+ c = add_cmd ("disable", class_breakpoint, skip_disable_command, _("\
Disable skip entries. You can specify numbers (e.g. \"skip disable 1 3\"), \
ranges (e.g. \"skip disable 4-8\"), or both (e.g. \"skip disable 1 3 4-8\").\n\n\
If you don't specify any numbers or ranges, we'll disable all skip entries.\n\n\
Usage: skip disable [NUMBER | RANGE]..."),
- &skiplist);
+ &skiplist);
+ set_cmd_completer (c, complete_skip_number);
- add_cmd ("delete", class_breakpoint, skip_delete_command, _("\
+ c = add_cmd ("delete", class_breakpoint, skip_delete_command, _("\
Delete skip entries. You can specify numbers (e.g. \"skip delete 1 3\"), \
ranges (e.g. \"skip delete 4-8\"), or both (e.g. \"skip delete 1 3 4-8\").\n\n\
If you don't specify any numbers or ranges, we'll delete all skip entries.\n\n\
Usage: skip delete [NUMBER | RANGES]..."),
- &skiplist);
+ &skiplist);
+ set_cmd_completer (c, complete_skip_number);
- add_info ("skip", info_skip_command, _("\
+ c = add_info ("skip", info_skip_command, _("\
Display the status of skips. You can specify numbers (e.g. \"skip info 1 3\"), \
ranges (e.g. \"skip info 4-8\"), or both (e.g. \"skip info 1 3 4-8\").\n\n\
If you don't specify any numbers or ranges, we'll show all skips.\n\n\
@@ -705,6 +725,7 @@ Usage: skip info [NUMBER | RANGES]...\n\
The \"Type\" column indicates one of:\n\
\tfile - ignored file\n\
\tfunction - ignored function"));
+ set_cmd_completer (c, complete_skip_number);
add_setshow_boolean_cmd ("skip", class_maintenance,
&debug_skip, _("\
@@ -16,6 +16,8 @@
# This file was written by Justin Lebar. (justin.lebar@gmail.com)
# And further hacked on by Doug Evans. (dje@google.com)
+standard_testfile
+
if { [prepare_for_testing "failed to prepare" "skip" \
{skip.c skip1.c } \
{debug nowarnings}] } {
@@ -297,3 +299,49 @@ with_test_prefix "step using -fi + -fu" {
gdb_test "step" ".*" "step 4"; # Skip over test_skip()
gdb_test "step" "test_skip_file_and_function \\(\\) at.*" "step 5"; # Return from skip1_test_skip_file_and_function()
}
+
+with_test_prefix "skip delete completion" {
+ global binfile
+ clean_restart "${binfile}"
+ if ![runto_main] {
+ fail "can't run to main"
+ return
+ }
+
+ # Create a bunch of skips, don't care what they are.
+ for {set i 0} {$i < 12} {incr i} {
+ gdb_test "skip" ".*" "add skip $i"
+ }
+
+ gdb_test "complete skip delete " \
+ [multi_line "skip delete 1" \
+ "skip delete 10" \
+ "skip delete 11" \
+ "skip delete 12" \
+ "skip delete 2" \
+ "skip delete 3" \
+ "skip delete 4" \
+ "skip delete 5" \
+ "skip delete 6" \
+ "skip delete 7" \
+ "skip delete 8" \
+ "skip delete 9"]
+ gdb_test "complete skip delete 1" \
+ [multi_line "skip delete 1" \
+ "skip delete 10" \
+ "skip delete 11" \
+ "skip delete 12"]
+ gdb_test "complete skip delete 1 " \
+ [multi_line "skip delete 1 1" \
+ "skip delete 1 10" \
+ "skip delete 1 11" \
+ "skip delete 1 12" \
+ "skip delete 1 2" \
+ "skip delete 1 3" \
+ "skip delete 1 4" \
+ "skip delete 1 5" \
+ "skip delete 1 6" \
+ "skip delete 1 7" \
+ "skip delete 1 8" \
+ "skip delete 1 9"]
+}