[0/2] Make use of gdb::options for info variabels|functions|args|locals

Message ID 20190711155305.GO23204@embecosm.com
State New, archived
Headers

Commit Message

Andrew Burgess July 11, 2019, 3:53 p.m. UTC
  * Pedro Alves <palves@redhat.com> [2019-07-11 14:36:23 +0100]:

> On 7/11/19 2:20 PM, Andrew Burgess wrote:
> > Additional use of the gdb::options framework.
> > 
> > --
> > 
> > Andrew Burgess (2):
> >   gdb: Allow quoting around string options in the gdb::option framework
> 
> Ahaha, that didn't take long.  Thanks for doing this.  LGTM.
> 
> >   gdb: Make use of gdb::option framework for some info commands
> 
> This LGTM to me too, but, I was surprised to find this doesn't add
> completers at the same time?

Something like this, maybe?

Thanks,
Andrew

---

[PATCH] gdb: Add command completers for some info commands

Add command completion for info variables, functions, args, and
locals.  This completer only completes the command line options as
these commands all take a regexp which GDB can't really offer
completions for.

gdb/ChangeLog:

	* cli/cli-utils.c (info_print_command_completer): New function.
	* cli/cli-utils.h: Add 'completer.h' include, and forward
	declaration for 'struct cmd_list_element'.
	(info_print_command_completer): Declare.
	* stack.c (_initialize_stack): Add completer for 'info locals' and
	'info args'.
	* symtab.c (_initialize_symtab): Add completer for 'info
	variables' and 'info functions'.
---
 gdb/ChangeLog       | 11 +++++++++++
 gdb/cli/cli-utils.c | 19 +++++++++++++++++++
 gdb/cli/cli-utils.h | 13 +++++++++++++
 gdb/stack.c         | 10 ++++++----
 gdb/symtab.c        | 19 +++++++++++++------
 5 files changed, 62 insertions(+), 10 deletions(-)
  

Comments

Pedro Alves July 11, 2019, 4:05 p.m. UTC | #1
On 7/11/19 4:53 PM, Andrew Burgess wrote:
> * Pedro Alves <palves@redhat.com> [2019-07-11 14:36:23 +0100]:
> 
>> On 7/11/19 2:20 PM, Andrew Burgess wrote:
>>> Additional use of the gdb::options framework.
>>>
>>> --
>>>
>>> Andrew Burgess (2):
>>>   gdb: Allow quoting around string options in the gdb::option framework
>>
>> Ahaha, that didn't take long.  Thanks for doing this.  LGTM.
>>
>>>   gdb: Make use of gdb::option framework for some info commands
>>
>> This LGTM to me too, but, I was surprised to find this doesn't add
>> completers at the same time?
> 
> Something like this, maybe?

_Exactly_ like that.  Perfect.

So far, I've listed this sort of improvement under
"Completion improvements" in NEWS.  I think you lists these commands
there too, and it'd be an obvious change.

> +
> +  const char *word = advance_to_expression_complete_word_point (tracker, text);
> +  symbol_completer (ignore, tracker, text, word);
> +  return;

Redundant "return".

Thanks,
Pedro Alves
  
Tom Tromey July 11, 2019, 4:08 p.m. UTC | #2
>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:

Andrew> +void
Andrew> +info_print_command_completer (struct cmd_list_element *ignore,
Andrew> +			      completion_tracker &tracker,
Andrew> +			      const char *text, const char * /* word */)
Andrew> +{
Andrew> +  const auto group
Andrew> +    = make_info_print_options_def_group (nullptr);
Andrew> +  if (gdb::option::complete_options
Andrew> +      (tracker, &text, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_OPERAND, group))
Andrew> +    return;
Andrew> +
Andrew> +  const char *word = advance_to_expression_complete_word_point (tracker, text);
Andrew> +  symbol_completer (ignore, tracker, text, word);
Andrew> +  return;
Andrew> +}

I didn't really read the patch but this unnecessary return popped out at me.
I think gdb style is not to do this.

Tom
  
Andrew Burgess July 11, 2019, 7:21 p.m. UTC | #3
* Pedro Alves <palves@redhat.com> [2019-07-11 17:05:18 +0100]:

> On 7/11/19 4:53 PM, Andrew Burgess wrote:
> > * Pedro Alves <palves@redhat.com> [2019-07-11 14:36:23 +0100]:
> > 
> >> On 7/11/19 2:20 PM, Andrew Burgess wrote:
> >>> Additional use of the gdb::options framework.
> >>>
> >>> --
> >>>
> >>> Andrew Burgess (2):
> >>>   gdb: Allow quoting around string options in the gdb::option framework
> >>
> >> Ahaha, that didn't take long.  Thanks for doing this.  LGTM.
> >>
> >>>   gdb: Make use of gdb::option framework for some info commands
> >>
> >> This LGTM to me too, but, I was surprised to find this doesn't add
> >> completers at the same time?
> > 
> > Something like this, maybe?
> 
> _Exactly_ like that.  Perfect.
> 
> So far, I've listed this sort of improvement under
> "Completion improvements" in NEWS.  I think you lists these commands
> there too, and it'd be an obvious change.
> 
> > +
> > +  const char *word = advance_to_expression_complete_word_point (tracker, text);
> > +  symbol_completer (ignore, tracker, text, word);
> > +  return;
> 
> Redundant "return".

Pushed with "return" fixed, and a NEWS entry.

Thanks,
Andrew



> 
> Thanks,
> Pedro Alves
  

Patch

diff --git a/gdb/cli/cli-utils.c b/gdb/cli/cli-utils.c
index cd3dfe65a2b..be830ee9f9d 100644
--- a/gdb/cli/cli-utils.c
+++ b/gdb/cli/cli-utils.c
@@ -473,3 +473,22 @@  extract_info_print_options (info_print_options *opts,
   if (*args != nullptr && **args == '\0')
     *args = nullptr;
 }
+
+/* See documentation in cli-utils.h.  */
+
+void
+info_print_command_completer (struct cmd_list_element *ignore,
+			      completion_tracker &tracker,
+			      const char *text, const char * /* word */)
+{
+  const auto group
+    = make_info_print_options_def_group (nullptr);
+  if (gdb::option::complete_options
+      (tracker, &text, gdb::option::PROCESS_OPTIONS_UNKNOWN_IS_OPERAND, group))
+    return;
+
+  const char *word = advance_to_expression_complete_word_point (tracker, text);
+  symbol_completer (ignore, tracker, text, word);
+  return;
+}
+
diff --git a/gdb/cli/cli-utils.h b/gdb/cli/cli-utils.h
index a3826be6824..17cdd842b2f 100644
--- a/gdb/cli/cli-utils.h
+++ b/gdb/cli/cli-utils.h
@@ -20,6 +20,10 @@ 
 #ifndef CLI_CLI_UTILS_H
 #define CLI_CLI_UTILS_H
 
+#include "completer.h"
+
+struct cmd_list_element;
+
 /* *PP is a string denoting a number.  Get the number.  Advance *PP
    after the string and any trailing whitespace.
 
@@ -66,6 +70,15 @@  struct info_print_options
 extern void extract_info_print_options (info_print_options *opts,
 					const char **args);
 
+/* Function that can be used as a command completer for 'info variable'
+   and friends.  This offers command option completion as well as symbol
+   completion.  At the moment all symbols are offered for all commands.  */
+
+extern void info_print_command_completer (struct cmd_list_element *ignore,
+					  completion_tracker &tracker,
+					  const char *text,
+					  const char * /* word */);
+
 /* Throws an error telling the user that ARGS starts with an option
    unrecognized by COMMAND.  */
 
diff --git a/gdb/stack.c b/gdb/stack.c
index 175f2116a5b..9b1d1a68568 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -3423,18 +3423,20 @@  Print information about a stack frame selected by level.\n\
 Usage: info frame level LEVEL"),
 	   &info_frame_cmd_list);
 
-  add_info ("locals", info_locals_command,
-	    info_print_args_help (_("\
+  cmd = add_info ("locals", info_locals_command,
+		  info_print_args_help (_("\
 All local variables of current stack frame or those matching REGEXPs.\n\
 Usage: info locals [-q] [-t TYPEREGEXP] [NAMEREGEXP]\n\
 Prints the local variables of the current stack frame.\n"),
 				  _("local variables")));
-  add_info ("args", info_args_command,
-	    info_print_args_help (_("\
+  set_cmd_completer_handle_brkchars (cmd, info_print_command_completer);
+  cmd = add_info ("args", info_args_command,
+		  info_print_args_help (_("\
 All argument variables of current stack frame or those matching REGEXPs.\n\
 Usage: info args [-q] [-t TYPEREGEXP] [NAMEREGEXP]\n\
 Prints the argument variables of the current stack frame.\n"),
 				  _("argument variables")));
+  set_cmd_completer_handle_brkchars (cmd, info_print_command_completer);
 
   if (dbx_commands)
     add_com ("func", class_stack, func_command, _("\
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 46691122187..41898992c19 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -5997,28 +5997,35 @@  symbol_set_symtab (struct symbol *symbol, struct symtab *symtab)
 void
 _initialize_symtab (void)
 {
+  cmd_list_element *c;
+
   initialize_ordinary_address_classes ();
 
-  add_info ("variables", info_variables_command,
-	    info_print_args_help (_("\
+  c = add_info ("variables", info_variables_command,
+		info_print_args_help (_("\
 All global and static variable names or those matching REGEXPs.\n\
 Usage: info variables [-q] [-t TYPEREGEXP] [NAMEREGEXP]\n\
 Prints the global and static variables.\n"),
 				  _("global and static variables")));
+  set_cmd_completer_handle_brkchars (c, info_print_command_completer);
   if (dbx_commands)
-    add_com ("whereis", class_info, info_variables_command,
-	     info_print_args_help (_("\
+    {
+      c = add_com ("whereis", class_info, info_variables_command,
+		   info_print_args_help (_("\
 All global and static variable names, or those matching REGEXPs.\n\
 Usage: whereis [-q] [-t TYPEREGEXP] [NAMEREGEXP]\n\
 Prints the global and static variables.\n"),
 				   _("global and static variables")));
+      set_cmd_completer_handle_brkchars (c, info_print_command_completer);
+    }
 
-  add_info ("functions", info_functions_command,
-	    info_print_args_help (_("\
+  c = add_info ("functions", info_functions_command,
+		info_print_args_help (_("\
 All function names or those matching REGEXPs.\n\
 Usage: info functions [-q] [-t TYPEREGEXP] [NAMEREGEXP]\n\
 Prints the functions.\n"),
 				  _("functions")));
+  set_cmd_completer_handle_brkchars (c, info_print_command_completer);
 
   /* FIXME:  This command has at least the following problems:
      1.  It prints builtin types (in a very strange and confusing fashion).