gdb: option completion for the 'skip' command

This commit adds proper option completion for the 'skip' command, the
skip_command function (skip.c) has been rewritten as a consequence.
All the existing functionality should have been retained, though some
of the error messages have changed as we now get the errors generated
by the option parsing code rather than the ones from skip_command.

I've added more skip tests to cover error cases that were not
previously tested.  And I've added a test for 'skip FUNCTION_NAME'
which was previously untested.

Consider the -gfile option for the skip command.  Previously the skip
command was hard coded to accept -gfile or -gfi, these were aliases.
But you couldn't pass -gfil.  Now we've switched to the general option
handling routine the only option is -gfile, but our option handling
code will accept any partial option name that uniquely identifies an
option, so -g, -gf, -gfi, -gfil, and -gfile are all valid, and all
aliases of each other.  The same is true for all the options that skip
accepts.

Because of this I've gone through and removed all references to the
short form option names that I can find.  Now that tab-completion
works, there's no need to advertise specific short form names, and as
discussed above, all unique short forms are still valid.

Reviewed-By: Eli Zaretskii <eliz@gnu.org>
Approved-By: Tom Tromey <tom@tromey.com>

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index fd7fc7533c638a66356ea0ee7f150b1a5646c684..c74c58c1d4a8df3046522edb8544c0e6a0252455 100644 (file)
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -6953,27 +6953,23 @@ The @var{options} argument is any useful combination of the following:
 
 @table @code
 @item -file @var{file}
-@itemx -fi @var{file}
 Functions in @var{file} will be skipped over when stepping.
 
 @item -gfile @var{file-glob-pattern}
-@itemx -gfi @var{file-glob-pattern}
 @cindex skipping over files via glob-style patterns
 Functions in files matching @var{file-glob-pattern} will be skipped
 over when stepping.
 
 @smallexample
-(@value{GDBP}) skip -gfi utils/*.c
+(@value{GDBP}) skip -gfile utils/*.c
 @end smallexample
 
 @item -function @var{linespec}
-@itemx -fu @var{linespec}
 Functions named by @var{linespec} or the function containing the line
 named by @var{linespec} will be skipped over when stepping.
 @xref{Location Specifications}.
 
 @item -rfunction @var{regexp}
-@itemx -rfu @var{regexp}
 @cindex skipping over functions via regular expressions
 Functions whose name matches @var{regexp} will be skipped over when stepping.
 
@@ -6985,14 +6981,14 @@ the template arguments are.  Specifying the function to be skipped as a
 regular expression makes this easier.
 
 @smallexample
-(@value{GDBP}) skip -rfu ^std::(allocator|basic_string)<.*>::~?\1 *\(
+(@value{GDBP}) skip -rfunction ^std::(allocator|basic_string)<.*>::~?\1 *\(
 @end smallexample
 
 If you want to skip every templated C@t{++} constructor and destructor
 in the @code{std} namespace you can do:
 
 @smallexample
-(@value{GDBP}) skip -rfu ^std::([a-zA-z0-9_]+)<.*>::~?\1 *\(
+(@value{GDBP}) skip -rfunction ^std::([a-zA-z0-9_]+)<.*>::~?\1 *\(
 @end smallexample
 @end table
 

diff --git a/gdb/skip.c b/gdb/skip.c
index c845e7fc7053e80190a995446ee18808feacacee..5c8d409cb8c0f67bb6321adc79b4e1f0e1015dca 100644 (file)
--- a/gdb/skip.c
+++ b/gdb/skip.c
@@ -272,131 +272,180 @@ skip_function_command (const char *arg, int from_tty)
   skip_function (arg);
 }
 
+/* Storage for the "skip" command option values.  */
+
+struct skip_opts
+{
+  /* For the -file option.  */
+  std::string file;
+
+  /* For the -gfile option.  */
+  std::string gfile;
+
+  /* For the -function option.  */
+  std::string function;
+
+  /* For the -rfunction option.  */
+  std::string rfunction;
+};
+
+/* The options for the "skip" command.  */
+
+static const gdb::option::option_def skip_option_defs[] = {
+  gdb::option::filename_option_def<skip_opts> {
+    "file",
+    [] (skip_opts *opts) { return &opts->file; },
+    nullptr, /* show_cmd_cb */
+    nullptr, /* set_doc */
+  },
+
+  gdb::option::filename_option_def<skip_opts> {
+    "gfile",
+    [] (skip_opts *opts) { return &opts->gfile; },
+    nullptr, /* show_cmd_cb */
+    nullptr, /* set_doc */
+  },
+
+  gdb::option::string_option_def<skip_opts> {
+    "function",
+    [] (skip_opts *opts) { return &opts->function; },
+    nullptr, /* show_cmd_cb */
+    nullptr, /* set_doc */
+  },
+
+  gdb::option::string_option_def<skip_opts> {
+    "rfunction",
+    [] (skip_opts *opts) { return &opts->rfunction; },
+    nullptr, /* show_cmd_cb */
+    nullptr, /* set_doc */
+  },
+};
+
+/* Create the option_def_group for the "skip" command.  */
+
+static inline gdb::option::option_def_group
+make_skip_options_def_group (skip_opts *opts)
+{
+  return {{skip_option_defs}, opts};
+}
+
+/* Completion for the "skip" command.  */
+
+static void
+skip_command_completer (struct cmd_list_element *cmd,
+                       completion_tracker &tracker,
+                       const char *text, const char * /* word */)
+{
+  /* We only need to handle option completion here.  The sub-commands of
+     'skip' are handled automatically by the command system.  */
+  const auto group = make_skip_options_def_group (nullptr);
+  gdb::option::complete_options
+    (tracker, &text, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_OPERAND, group);
+}
+
 /* Process "skip ..." that does not match "skip file" or "skip function".  */
 
 static void
 skip_command (const char *arg, int from_tty)
 {
-  const char *file = NULL;
-  const char *gfile = NULL;
-  const char *function = NULL;
-  const char *rfunction = NULL;
-  int i;
-
-  if (arg == NULL)
+  if (arg == nullptr)
     {
       skip_function_command (arg, from_tty);
       return;
     }
 
-  gdb_argv argv (arg);
+  /* Parse command line options.  */
+  skip_opts opts;
+  const auto group = make_skip_options_def_group (&opts);
+  gdb::option::process_options
+    (&arg, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_OPERAND, group);
 
-  for (i = 0; argv[i] != NULL; ++i)
-    {
-      const char *p = argv[i];
-      const char *value = argv[i + 1];
+  /* Handle invalid argument combinations.  */
+  if (!opts.file.empty () && !opts.gfile.empty ())
+    error (_("Cannot specify both -file and -gfile."));
 
-      if (strcmp (p, "-fi") == 0
-         || strcmp (p, "-file") == 0)
-       {
-         if (value == NULL)
-           error (_("Missing value for %s option."), p);
-         file = value;
-         ++i;
-       }
-      else if (strcmp (p, "-gfi") == 0
-              || strcmp (p, "-gfile") == 0)
-       {
-         if (value == NULL)
-           error (_("Missing value for %s option."), p);
-         gfile = value;
-         ++i;
-       }
-      else if (strcmp (p, "-fu") == 0
-              || strcmp (p, "-function") == 0)
-       {
-         if (value == NULL)
-           error (_("Missing value for %s option."), p);
-         function = value;
-         ++i;
-       }
-      else if (strcmp (p, "-rfu") == 0
-              || strcmp (p, "-rfunction") == 0)
+  if (!opts.function.empty () && !opts.rfunction.empty ())
+    error (_("Cannot specify both -function and -rfunction."));
+
+  /* If there's anything left on the command line then this is an error if
+     a valid command line flag was also given, or we assume that it's a
+     function name if the user entered 'skip blah'.  */
+  if (*arg != '\0')
+    {
+      if (!opts.file.empty () || !opts.gfile.empty ()
+         || !opts.function.empty () || !opts.rfunction.empty ())
+       error (_("Junk after arguments: %s"), arg);
+      else if (*arg == '-')
        {
-         if (value == NULL)
-           error (_("Missing value for %s option."), p);
-         rfunction = value;
-         ++i;
+         const char *after_arg = skip_to_space (arg);
+         error (_("Invalid skip option: %.*s"), (int) (after_arg - arg), arg);
        }
-      else if (*p == '-')
-       error (_("Invalid skip option: %s"), p);
-      else if (i == 0)
+      else
        {
-         /* Assume the user entered "skip FUNCTION-NAME".
-            FUNCTION-NAME may be `foo (int)', and therefore we pass the
-            complete original arg to skip_function command as if the user
-            typed "skip function arg".  */
+         /* Assume the user entered "skip FUNCTION-NAME".  FUNCTION-NAME
+            may be `foo (int)', and therefore we pass the complete
+            original ARG to skip_function_command as if the user typed
+            "skip function arg".  */
          skip_function_command (arg, from_tty);
          return;
        }
-      else
-       error (_("Invalid argument: %s"), p);
     }
 
-  if (file != NULL && gfile != NULL)
-    error (_("Cannot specify both -file and -gfile."));
-
-  if (function != NULL && rfunction != NULL)
-    error (_("Cannot specify both -function and -rfunction."));
-
   /* This shouldn't happen as "skip" by itself gets punted to
-     skip_function_command.  */
-  gdb_assert (file != NULL || gfile != NULL
-             || function != NULL || rfunction != NULL);
+     skip_function_command, and if the user entered "skip blah" then this
+     will have been handled above.  */
+  gdb_assert (!opts.file.empty () || !opts.gfile.empty ()
+             || !opts.function.empty () || !opts.rfunction.empty ());
 
+  /* Create the skip list entry.  */
   std::string entry_file;
-  if (file != NULL)
-    entry_file = file;
-  else if (gfile != NULL)
-    entry_file = gfile;
+  if (!opts.file.empty ())
+    entry_file = opts.file;
+  else if (!opts.gfile.empty ())
+    entry_file = opts.gfile;
 
   std::string entry_function;
-  if (function != NULL)
-    entry_function = function;
-  else if (rfunction != NULL)
-    entry_function = rfunction;
+  if (!opts.function.empty ())
+    entry_function = opts.function;
+  else if (!opts.rfunction.empty ())
+    entry_function = opts.rfunction;
 
-  skiplist_entry::add_entry (gfile != NULL, std::move (entry_file),
-                            rfunction != NULL, std::move (entry_function));
+  skiplist_entry::add_entry (!opts.gfile.empty (),
+                            std::move (entry_file),
+                            !opts.rfunction.empty (),
+                            std::move (entry_function));
 
   /* I18N concerns drive some of the choices here (we can't piece together
      the output too much).  OTOH we want to keep this simple.  Therefore the
      only polish we add to the output is to append "(s)" to "File" or
      "Function" if they're a glob/regexp.  */
   {
-    const char *file_to_print = file != NULL ? file : gfile;
-    const char *function_to_print = function != NULL ? function : rfunction;
-    const char *file_text = gfile != NULL ? _("File(s)") : _("File");
-    const char *lower_file_text = gfile != NULL ? _("file(s)") : _("file");
+    std::string &file_to_print
+      = !opts.file.empty () ? opts.file : opts.gfile;
+    std::string &function_to_print
+      = !opts.function.empty () ? opts.function : opts.rfunction;
+
+    const char *file_text = !opts.gfile.empty () ? _("File(s)") : _("File");
+    const char *lower_file_text = !opts.gfile.empty () ? _("file(s)") : _("file");
     const char *function_text
-      = rfunction != NULL ? _("Function(s)") : _("Function");
+      = !opts.rfunction.empty () ? _("Function(s)") : _("Function");
 
-    if (function_to_print == NULL)
+    if (function_to_print.empty ())
       {
        gdb_printf (_("%s %s will be skipped when stepping.\n"),
-                   file_text, file_to_print);
+                   file_text, file_to_print.c_str ());
       }
-    else if (file_to_print == NULL)
+    else if (file_to_print.empty ())
       {
        gdb_printf (_("%s %s will be skipped when stepping.\n"),
-                   function_text, function_to_print);
+                   function_text, function_to_print.c_str ());
       }
     else
       {
        gdb_printf (_("%s %s in %s %s will be skipped"
                      " when stepping.\n"),
-                   function_text, function_to_print,
-                   lower_file_text, file_to_print);
+                   function_text, function_to_print.c_str (),
+                   lower_file_text, file_to_print.c_str ());
       }
   }
 }
@@ -732,7 +781,7 @@ INIT_GDB_FILE (step_skip)
   static struct cmd_list_element *skiplist = NULL;
   struct cmd_list_element *c;
 
-  add_prefix_cmd ("skip", class_breakpoint, skip_command, _("\
+  c = add_prefix_cmd ("skip", class_breakpoint, skip_command, _("\
 Ignore a function while stepping.\n\
 \n\
 Usage: skip [FUNCTION-NAME]\n\
@@ -740,12 +789,13 @@ Usage: skip [FUNCTION-NAME]\n\
 If no arguments are given, ignore the current function.\n\
 \n\
 FILE-SPEC is one of:\n\
-       -fi|-file FILE-NAME\n\
-       -gfi|-gfile GLOB-FILE-PATTERN\n\
+       -file FILE-NAME\n\
+       -gfile GLOB-FILE-PATTERN\n\
 FUNCTION-SPEC is one of:\n\
-       -fu|-function FUNCTION-NAME\n\
-       -rfu|-rfunction FUNCTION-NAME-REGULAR-EXPRESSION"),
+       -function FUNCTION-NAME\n\
+       -rfunction FUNCTION-NAME-REGULAR-EXPRESSION"),
                  &skiplist, 1, &cmdlist);
+  set_cmd_completer_handle_brkchars (c, skip_command_completer);
 
   c = add_cmd ("file", class_breakpoint, skip_file_command, _("\
 Ignore a file while stepping.\n\

diff --git a/gdb/testsuite/gdb.base/skip.exp b/gdb/testsuite/gdb.base/skip.exp
index 7a5f297b67e030bd81d665243a99e4482f7c3b5c..9d55c540f12b9015bfa68029f984c40a5bd45adb 100644 (file)
--- a/gdb/testsuite/gdb.base/skip.exp
+++ b/gdb/testsuite/gdb.base/skip.exp
@@ -37,17 +37,22 @@ gdb_test "skip" "No default function now." "skip, no default function"
 
 # Test elided args.
 
-gdb_test "skip -fi" "Missing value for -fi option."
-gdb_test "skip -file" "Missing value for -file option."
-gdb_test "skip -fu" "Missing value for -fu option."
-gdb_test "skip -function" "Missing value for -function option."
-gdb_test "skip -rfu" "Missing value for -rfu option."
-gdb_test "skip -rfunction" "Missing value for -rfunction option."
+gdb_test "skip -fi" "-file requires an argument"
+gdb_test "skip -file" "-file requires an argument"
+gdb_test "skip -fu" "-function requires an argument"
+gdb_test "skip -function" "-function requires an argument"
+gdb_test "skip -rfu" "-rfunction requires an argument"
+gdb_test "skip -rfunction" "-rfunction requires an argument"
 
 # Test other invalid option combinations.
 
 gdb_test "skip -x" "Invalid skip option: -x"
-gdb_test "skip -rfu foo.* xyzzy" "Invalid argument: xyzzy"
+gdb_test "skip -q abc" "Invalid skip option: -q"
+gdb_test "skip -rfu foo.* xyzzy" "Junk after arguments: xyzzy"
+gdb_test "skip -function foo -rfunction foo*" \
+    "Cannot specify both -function and -rfunction\\."
+gdb_test "skip -file foo.c -gfile foo*.c" \
+    "Cannot specify both -file and -gfile\\."
 
 if {![runto_main]} {
     return
@@ -329,6 +334,17 @@ with_test_prefix "step using -fi + -fu" {
     gdb_test "step" "test_skip_file_and_function \\(\\) at.*" "step 5"; # Return from skip1_test_skip_file_and_function()
 }
 
+# Test that 'skip FUNCTION-NAME' is equivalent to 'skip function
+# FUNCTION-NAME'.
+
+gdb_test_no_output "delete skip"
+gdb_test "skip xxyyzz" "Function xxyyzz will be skipped when stepping\\."
+gdb_test "info skip" \
+    [multi_line \
+        "Num\\s+Enb\\s+Glob\\s+File\\s+RE\\s+Function" \
+        "10\\s+y\\s+n\\s+<none>\\s+n\\s+xxyyzz"] \
+    "info skip after 'skip xxyyzz'"
+
 with_test_prefix "skip delete completion" {
     clean_restart $::testfile