* [PATCH] Add completer for skip numbers @ 2018-11-10 17:39 Simon Marchi 2018-11-10 19:07 ` Pedro Alves 0 siblings, 1 reply; 3+ messages in thread From: Simon Marchi @ 2018-11-10 17:39 UTC (permalink / raw) To: gdb-patches; +Cc: Simon Marchi 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(-) 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"] +} -- 2.19.1 ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Add completer for skip numbers 2018-11-10 17:39 [PATCH] Add completer for skip numbers Simon Marchi @ 2018-11-10 19:07 ` Pedro Alves 2018-11-11 16:52 ` Simon Marchi 0 siblings, 1 reply; 3+ messages in thread From: Pedro Alves @ 2018-11-10 19:07 UTC (permalink / raw) To: Simon Marchi, gdb-patches 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 ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Add completer for skip numbers 2018-11-10 19:07 ` Pedro Alves @ 2018-11-11 16:52 ` Simon Marchi 0 siblings, 0 replies; 3+ messages in thread From: Simon Marchi @ 2018-11-11 16:52 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches 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 ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-11-11 16:52 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-11-10 17:39 [PATCH] Add completer for skip numbers Simon Marchi 2018-11-10 19:07 ` Pedro Alves 2018-11-11 16:52 ` Simon Marchi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox