[RFA] File-name completion improvements

[RFA] File-name completion improvements

[Eli Zaretskii]

I was never happy with GDB's completion: I think it doesn't quite help as 
much as it could.  For example, there are commands which mostly operate 
on files, but insist to complete on symbols instead.

So I will be submitting a series of patches to make this better.  But 
first, I need to make the file-name completion DTRT; currently, it 
doesn't.  For example, try "dir >foo TAB" or "dir foo:bar TAB" (assuming 
you have a directory or a file which begin with `foo' and `bar').

(Yes, I know "dir >foo" is nonsense, but GDB shouldn't prevent me from 
saying that; there are GDB commands which can use redirection near file 
names.)

The following patches correct several minor but annoying problems in 
file-name completion:

  - completion on a file name in a list of file names didn't work;
  - GDB would not always append a slash if the completion is a directory;
  - completion failed when the file name had non-file-name characters,
    such as redirection, around it;
  - on DOS/Windows, completion would fail with files with a drive letter.

Okay to commit?

2001-02-12  Eli Zaretskii  <eliz@is.elta.co.il>

	* completer.c (gdb_completer_file_name_break_characters): Remove
	slash from file-name break characters.
	[__MSDOS__]: Special definition for DOS/Windows file names.
	(line_completion_function): When completing on file names, bump
	`p' to the first file-name constituent character of `word', before
	invoking the completer.

--- gdb/completer.c~0	Fri Dec 15 03:01:46 2000
+++ gdb/completer.c	Mon Feb 12 00:28:42 2001
@@ -64,7 +64,13 @@ static char *gdb_completer_command_word_
    break characters any characters that are commonly used in file
    names, such as '-', '+', '~', etc.  Otherwise, readline displays
    incorrect completion candidates.  */
-static char *gdb_completer_file_name_break_characters = " \t\n*|\"';:?/><";
+#ifdef __MSDOS__
+/* MS-DOS and MS-Windows use colon as part of the drive spec, and most
+   programs support @foo style response files.  */
+static char *gdb_completer_file_name_break_characters = " \t\n*|\"';?><@";
+#else
+static char *gdb_completer_file_name_break_characters = " \t\n*|\"';:?><";
+#endif
 
 /* Characters that can be used to quote completion strings.  Note that we
    can't include '"' because the gdb C parser treats such quoted sequences
@@ -348,10 +354,25 @@ line_completion_function (char *text, in
 		    {
 		      /* It is a normal command; what comes after it is
 		         completed by the command's completer function.  */
-		      list = (*c->completer) (p, word);
 		      if (c->completer == filename_completer)
-			rl_completer_word_break_characters =
-			  gdb_completer_file_name_break_characters;
+			{
+			  char *fbc = gdb_completer_file_name_break_characters;
+
+			  /* Many commands which want to complete on
+			     file names accept several file names, as
+			     in "run foo bar >>baz".  So we don't want
+			     to complete the entire text after the
+			     command, just the last word.  To this
+			     end, we need to find the beginning of the
+			     file name starting at word and going
+			     backwards.  */
+			  for (p = word;
+			       p > tmp_command && strchr (fbc, p[-1]) == NULL;
+			       p--)
+			    ;
+			  rl_completer_word_break_characters = fbc;
+			}
+		      list = (*c->completer) (p, word);
 		    }
 		}
 	      else
@@ -397,10 +418,19 @@ line_completion_function (char *text, in
 	      else
 		{
 		  /* It is a normal command.  */
-		  list = (*c->completer) (p, word);
 		  if (c->completer == filename_completer)
-		    rl_completer_word_break_characters =
-		      gdb_completer_file_name_break_characters;
+		    {
+		      char *fbc = gdb_completer_file_name_break_characters;
+
+		      /* See the commentary above about the specifics
+			 of file-name completion.  */
+		      for (p = word;
+			   p > tmp_command && strchr (fbc, p[-1]) == NULL;
+			   p--)
+			;
+		      rl_completer_word_break_characters = fbc;
+		    }
+		  list = (*c->completer) (p, word);
 		}
 	    }
 	}

Re: [RFA] File-name completion improvements

[Fernando Nasser]

It is hard to imagine why 
list = (*c->completer) (p, word);
was before the break character list was set.  
Really weird.


Thanks for catching this.


Before committing, would you mind simplifying it a little bit.
Instead of changing this:
rl_completer_word_break_characters =
                         gdb_completer_file_name_break_characters;
into this:
char *fbc = gdb_completer_file_name_break_characters;
rl_completer_word_break_characters = fbc;
so you can use the shorthand here:
p > tmp_command && strchr (fbc, p[-1]) == NULL;
just use the full name:
p > tmp_command && strchr (gdb_completer_file_name_break_characters, \
                           p[-1]) == NULL;

I know, it is an awful long variable name.


After committing, please post the final version so it makes into the list archives.


Thanks again for the patch.
Fernando


Eli Zaretskii wrote:
> 
> I was never happy with GDB's completion: I think it doesn't quite help as
> much as it could.  For example, there are commands which mostly operate
> on files, but insist to complete on symbols instead.
> 
> So I will be submitting a series of patches to make this better.  But
> first, I need to make the file-name completion DTRT; currently, it
> doesn't.  For example, try "dir >foo TAB" or "dir foo:bar TAB" (assuming
> you have a directory or a file which begin with `foo' and `bar').
> 
> (Yes, I know "dir >foo" is nonsense, but GDB shouldn't prevent me from
> saying that; there are GDB commands which can use redirection near file
> names.)
> 
> The following patches correct several minor but annoying problems in
> file-name completion:
> 
>   - completion on a file name in a list of file names didn't work;
>   - GDB would not always append a slash if the completion is a directory;
>   - completion failed when the file name had non-file-name characters,
>     such as redirection, around it;
>   - on DOS/Windows, completion would fail with files with a drive letter.
> 
> Okay to commit?
> 
> 2001-02-12  Eli Zaretskii  <eliz@is.elta.co.il>
> 
>         * completer.c (gdb_completer_file_name_break_characters): Remove
>         slash from file-name break characters.
>         [__MSDOS__]: Special definition for DOS/Windows file names.
>         (line_completion_function): When completing on file names, bump
>         `p' to the first file-name constituent character of `word', before
>         invoking the completer.
> 
> --- gdb/completer.c~0   Fri Dec 15 03:01:46 2000
> +++ gdb/completer.c     Mon Feb 12 00:28:42 2001
> @@ -64,7 +64,13 @@ static char *gdb_completer_command_word_
>     break characters any characters that are commonly used in file
>     names, such as '-', '+', '~', etc.  Otherwise, readline displays
>     incorrect completion candidates.  */
> -static char *gdb_completer_file_name_break_characters = " \t\n*|\"';:?/><";
> +#ifdef __MSDOS__
> +/* MS-DOS and MS-Windows use colon as part of the drive spec, and most
> +   programs support @foo style response files.  */
> +static char *gdb_completer_file_name_break_characters = " \t\n*|\"';?><@";
> +#else
> +static char *gdb_completer_file_name_break_characters = " \t\n*|\"';:?><";
> +#endif
> 
>  /* Characters that can be used to quote completion strings.  Note that we
>     can't include '"' because the gdb C parser treats such quoted sequences
> @@ -348,10 +354,25 @@ line_completion_function (char *text, in
>                     {
>                       /* It is a normal command; what comes after it is
>                          completed by the command's completer function.  */
> -                     list = (*c->completer) (p, word);
>                       if (c->completer == filename_completer)
> -                       rl_completer_word_break_characters =
> -                         gdb_completer_file_name_break_characters;
> +                       {
> +                         char *fbc = gdb_completer_file_name_break_characters;
> +
> +                         /* Many commands which want to complete on
> +                            file names accept several file names, as
> +                            in "run foo bar >>baz".  So we don't want
> +                            to complete the entire text after the
> +                            command, just the last word.  To this
> +                            end, we need to find the beginning of the
> +                            file name starting at word and going
> +                            backwards.  */
> +                         for (p = word;
> +                              p > tmp_command && strchr (fbc, p[-1]) == NULL;
> +                              p--)
> +                           ;
> +                         rl_completer_word_break_characters = fbc;
> +                       }
> +                     list = (*c->completer) (p, word);
>                     }
>                 }
>               else
> @@ -397,10 +418,19 @@ line_completion_function (char *text, in
>               else
>                 {
>                   /* It is a normal command.  */
> -                 list = (*c->completer) (p, word);
>                   if (c->completer == filename_completer)
> -                   rl_completer_word_break_characters =
> -                     gdb_completer_file_name_break_characters;
> +                   {
> +                     char *fbc = gdb_completer_file_name_break_characters;
> +
> +                     /* See the commentary above about the specifics
> +                        of file-name completion.  */
> +                     for (p = word;
> +                          p > tmp_command && strchr (fbc, p[-1]) == NULL;
> +                          p--)
> +                       ;
> +                     rl_completer_word_break_characters = fbc;
> +                   }
> +                 list = (*c->completer) (p, word);
>                 }
>             }
>         }

-- 
Fernando Nasser
Red Hat - Toronto                       E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

Re: [RFA] File-name completion improvements

[Eli Zaretskii]

On Wed, 14 Feb 2001, Fernando Nasser wrote:

> Instead of changing this:
> rl_completer_word_break_characters =
>                          gdb_completer_file_name_break_characters;
> into this:
> char *fbc = gdb_completer_file_name_break_characters;
> rl_completer_word_break_characters = fbc;
> so you can use the shorthand here:
> p > tmp_command && strchr (fbc, p[-1]) == NULL;
> just use the full name:
> p > tmp_command && strchr (gdb_completer_file_name_break_characters, \
>                            p[-1]) == NULL;
> 
> I know, it is an awful long variable name.

The _real_ reason for that funky shorthand was not that it's long to
type ("M-/" in Emacs makes that problem go away ;-).  The real reason
was that, due to reindentation, the code either looks ugly or gets
past column 80.  If people don't mind that, there's no point in
introducing a new variable.

> After committing, please post the final version so it makes into the
> list archives.

Will do.

Re: [RFA] File-name completion improvements

[Eli Zaretskii]

On Wed, 14 Feb 2001, Fernando Nasser wrote:

> After committing, please post the final version so it makes into the list archives.

Here's what I committed:

2001-02-18  Eli Zaretskii  <eliz@is.elta.co.il>

	* completer.c (gdb_completer_file_name_break_characters): Remove
	slash from file-name break characters.
	[__MSDOS__]: Special definition for DOS/Windows file names.
	(line_completion_function): When completing on file names, bump
	`p' to the first file-name constituent character of `word', before
	invoking the completer.

--- gdb/completer.c~0	Fri Dec 15 03:01:46 2000
+++ gdb/completer.c	Sat Feb 17 12:22:10 2001
@@ -64,7 +64,13 @@ static char *gdb_completer_command_word_
    break characters any characters that are commonly used in file
    names, such as '-', '+', '~', etc.  Otherwise, readline displays
    incorrect completion candidates.  */
-static char *gdb_completer_file_name_break_characters = " \t\n*|\"';:?/><";
+#ifdef __MSDOS__
+/* MS-DOS and MS-Windows use colon as part of the drive spec, and most
+   programs support @foo style response files.  */
+static char *gdb_completer_file_name_break_characters = " \t\n*|\"';?><@";
+#else
+static char *gdb_completer_file_name_break_characters = " \t\n*|\"';:?><";
+#endif
 
 /* Characters that can be used to quote completion strings.  Note that we
    can't include '"' because the gdb C parser treats such quoted sequences
@@ -348,10 +354,25 @@ line_completion_function (char *text, in
 		    {
 		      /* It is a normal command; what comes after it is
 		         completed by the command's completer function.  */
-		      list = (*c->completer) (p, word);
 		      if (c->completer == filename_completer)
-			rl_completer_word_break_characters =
-			  gdb_completer_file_name_break_characters;
+			{
+			  /* Many commands which want to complete on
+			     file names accept several file names, as
+			     in "run foo bar >>baz".  So we don't want
+			     to complete the entire text after the
+			     command, just the last word.  To this
+			     end, we need to find the beginning of the
+			     file name starting at `word' and going
+			     backwards.  */
+			  for (p = word;
+			       p > tmp_command
+				 && strchr (gdb_completer_file_name_break_characters, p[-1]) == NULL;
+			       p--)
+			    ;
+			  rl_completer_word_break_characters =
+			    gdb_completer_file_name_break_characters;
+			}
+		      list = (*c->completer) (p, word);
 		    }
 		}
 	      else
@@ -397,10 +418,19 @@ line_completion_function (char *text, in
 	      else
 		{
 		  /* It is a normal command.  */
-		  list = (*c->completer) (p, word);
 		  if (c->completer == filename_completer)
-		    rl_completer_word_break_characters =
-		      gdb_completer_file_name_break_characters;
+		    {
+		      /* See the commentary above about the specifics
+			 of file-name completion.  */
+		      for (p = word;
+			   p > tmp_command
+			     && strchr (gdb_completer_file_name_break_characters, p[-1]) == NULL;
+			   p--)
+			;
+		      rl_completer_word_break_characters =
+			gdb_completer_file_name_break_characters;
+		    }
+		  list = (*c->completer) (p, word);
 		}
 	    }
 	}

Re: [RFA] More completion improvements

[Eli Zaretskii]

On Sun, 18 Feb 2001, Eli Zaretskii wrote:

> On Wed, 14 Feb 2001, Fernando Nasser wrote:
> 
> > After committing, please post the final version so it makes into the list archives.
> 
> Here's what I committed:
> 
> 2001-02-18  Eli Zaretskii  <eliz@is.elta.co.il>
> 
> 	* completer.c (gdb_completer_file_name_break_characters): Remove
> 	slash from file-name break characters.
> 	[__MSDOS__]: Special definition for DOS/Windows file names.
> 	(line_completion_function): When completing on file names, bump
> 	`p' to the first file-name constituent character of `word', before
> 	invoking the completer.

Now that this is done, it's time to use it.  The following patches make 
some commands use file-name completion instead of the default symbol-name 
completion.  In addition, set-demangling-style now correctly completes on 
the known style names.

People whose approval I'm seeking are:

Fernando is listed as the maintainer of the rdi stuff.  Chris is 
responsible for win32-nat.c.  I think solib.c and tracepoint.c are 
Michael Snyder's.  cli-cmds.c seems to be Fernando's again.  I couldn't 
identify who are the maintainers for the rest.

2001-02-17  Eli Zaretskii  <eliz@is.elta.co.il>

	* demangle.c (demangling_style_names): New variable.
	(_initialize_demangler): Fill demangling_style_names with the
	names of known demangling styles from libiberty_demanglers[].  Use
	add_set_enum_cmd instead of add_set_cmd, to get completion on
	demangling style names.

	* proc-api.c (_initialize_proc_api): Make `procfs-file' use
	file-name completion.

	* remote-rdi.c (_initialize_remote_rdi): Ditto for `rdilogfile'.

	* solib.c (_initialize_solib): Ditto for `solib-search-path' and
	`solib-absolute-prefix'.

	* tracepoint.c (_initialize_tracepoint): Ditto for
	`save-tracepoints'.

	* win32-nat.c (_initialize_inftarg): Ditto for `dll-symbols'.

2001-02-12  Eli Zaretskii  <eliz@is.elta.co.il>

	* cli/cli-cmds.c (init_cli_cmds): Make `shell' and `make' use
	file-name completion.

	* infcmd.c (_initialize_infcmd): Make the following commands use
	the file-name completer: `tty', `args', `path', `paths', and
	`run'.

--- gdb/cli/cli-cmds.c~0	Fri Feb  2 21:37:48 2001
+++ gdb/cli/cli-cmds.c	Mon Feb 12 23:22:28 2001
@@ -778,9 +778,10 @@
 		  "Generic command for showing gdb debugging flags",
 		  &showdebuglist, "show debug ", 0, &showlist);
 
-  add_com ("shell", class_support, shell_escape,
-	   "Execute the rest of the line as a shell command.  \n\
+  c = add_com ("shell", class_support, shell_escape,
+	       "Execute the rest of the line as a shell command.  \n\
 With no arguments, run an inferior shell.");
+  c->completer = filename_completer;
 
   /* NOTE: cagney/2000-03-20: Being able to enter ``(gdb) !ls'' would
      be a really useful feature.  Unfortunately, the below wont do
@@ -791,8 +792,9 @@
   if (xdb_commands)
     add_com_alias ("!", "shell", class_support, 0);
 
-  add_com ("make", class_support, make_command,
-       "Run the ``make'' program using the rest of the line as arguments.");
+  c = add_com ("make", class_support, make_command,
+          "Run the ``make'' program using the rest of the line as arguments.");
+  c->completer = filename_completer;
   add_cmd ("user", no_class, show_user,
 	   "Show definitions of user defined commands.\n\
 Argument is the name of the user defined command.\n\
--- gdb/infcmd.c~0	Wed Jan 31 03:24:00 2001
+++ gdb/infcmd.c	Mon Feb 12 23:03:02 2001
@@ -40,6 +40,7 @@
 #endif
 #include "event-top.h"
 #include "parser-defs.h"
+#include "completer.h"
 
 /* Functions exported for general use: */
 
@@ -1792,16 +1793,18 @@
 {
   struct cmd_list_element *c;
 
-  add_com ("tty", class_run, tty_command,
-	   "Set terminal for future runs of program being debugged.");
+  c= add_com ("tty", class_run, tty_command,
+	      "Set terminal for future runs of program being debugged.");
+  c->completer = filename_completer;
 
   add_show_from_set
-    (add_set_cmd ("args", class_run, var_string_noescape,
-		  (char *) &inferior_args,
-		  "Set argument list to give program being debugged when it is started.\n\
+    (c = add_set_cmd ("args", class_run, var_string_noescape,
+		      (char *) &inferior_args,
+		      "Set argument list to give program being debugged when it is started.\n\
 Follow this command with any number of args, to be passed to the program.",
 		  &setlist),
      &showlist);
+  c->completer = filename_completer;
 
   c = add_cmd
     ("environment", no_class, environment_info,
@@ -1829,12 +1832,13 @@
 	       &setlist);
   c->completer = noop_completer;
 
-  add_com ("path", class_files, path_command,
-	   "Add directory DIR(s) to beginning of search path for object files.\n\
+  c = add_com ("path", class_files, path_command,
+	       "Add directory DIR(s) to beginning of search path for object files.\n\
 $cwd in the path means the current working directory.\n\
 This path is equivalent to the $PATH shell variable.  It is a list of\n\
 directories, separated by colons.  These directories are searched to find\n\
 fully linked executable files and separately compiled object files as needed.");
+  c->completer = filename_completer;
 
   c = add_cmd ("paths", no_class, path_info,
 	       "Current search path for finding object files.\n\
@@ -1926,13 +1930,14 @@
   add_com_alias ("c", "cont", class_run, 1);
   add_com_alias ("fg", "cont", class_run, 1);
 
-  add_com ("run", class_run, run_command,
+  c = add_com ("run", class_run, run_command,
 	   "Start debugged program.  You may specify arguments to give it.\n\
 Args may include \"*\", or \"[...]\"; they are expanded using \"sh\".\n\
 Input and output redirection with \">\", \"<\", or \">>\" are also allowed.\n\n\
 With no arguments, uses arguments last specified (with \"run\" or \"set args\").\n\
 To cancel previous arguments and run with no arguments,\n\
 use \"set args\" without arguments.");
+  c->completer = filename_completer;
   add_com_alias ("r", "run", class_run, 1);
   if (xdb_commands)
     add_com ("R", class_run, run_no_args_command,


--- gdb/demangle.c~0	Fri Dec 15 03:01:46 2000
+++ gdb/demangle.c	Sat Feb 17 13:43:02 2001
@@ -49,6 +49,11 @@ extern void _initialize_demangler (void)
 
 static char *current_demangling_style_string;
 
+/* The array of names of the known demanglyng styles.  Generated by
+   _initialize_demangler from libiberty_demanglers[] array.  */
+
+static const char **demangling_style_names;
+
 static void set_demangling_command (char *, int, struct cmd_list_element *);
 
 /* Set current demangling style.  Called by the "set demangle-style"
@@ -173,12 +178,26 @@ void
 _initialize_demangler (void)
 {
   struct cmd_list_element *set, *show;
+  int i, ndems;
 
-  set = add_set_cmd ("demangle-style", class_support, var_string_noescape,
-		     (char *) &current_demangling_style_string,
-		     "Set the current C++ demangling style.\n\
+  /* Fill the demangling_style_names[] array.  */
+  for (ndems = 0;
+       libiberty_demanglers[ndems].demangling_style != unknown_demangling; 
+       ndems++)
+    ;
+  demangling_style_names = xmalloc (ndems * sizeof (char *));
+  for (i = 0;
+       libiberty_demanglers[i].demangling_style != unknown_demangling; 
+       i++)
+    demangling_style_names[i] =
+      xstrdup (libiberty_demanglers[i].demangling_style_name);
+
+  set = add_set_enum_cmd ("demangle-style", class_support,
+			  demangling_style_names,
+			  (const char **) &current_demangling_style_string,
+			  "Set the current C++ demangling style.\n\
 Use `set demangle-style' without arguments for a list of demangling styles.",
-		     &setlist);
+			  &setlist);
   show = add_show_from_set (set, &showlist);
   set->function.sfunc = set_demangling_command;
 
--- gdb/proc-api.c~0	Sun Jul 30 04:48:26 2000
+++ gdb/proc-api.c	Sat Feb 17 21:54:32 2001
@@ -777,5 +777,6 @@ _initialize_proc_api (void)
 
   add_show_from_set (c, &showlist);
   c->function.sfunc = set_procfs_file_cmd;
+  c->completer = filename_completer;
 }
--- gdb/remote-rdi.c~0	Fri Feb  2 21:14:32 2001
+++ gdb/remote-rdi.c	Sat Feb 17 22:42:10 2001
@@ -32,6 +32,7 @@
 #include "gdbthread.h"
 #include "gdbcore.h"
 #include "breakpoint.h"
+#include "completer.h"
 
 #ifdef USG
 #include <sys/types.h>
@@ -1021,6 +1022,8 @@ rdilogenable_command (char *args, int fr
 void
 _initialize_remote_rdi (void)
 {
+  struct cmd_list_element *c;
+
   init_rdi_ops ();
   add_target (&arm_rdi_ops);
 
@@ -1028,14 +1031,15 @@ _initialize_remote_rdi (void)
   Adp_SetLogfile (log_filename);
   Adp_SetLogEnable (log_enable);
 
-  add_cmd ("rdilogfile", class_maintenance,
-	   rdilogfile_command,
-	   "Set filename for ADP packet log.\n\
+  c = add_cmd ("rdilogfile", class_maintenance,
+	       rdilogfile_command,
+	       "Set filename for ADP packet log.\n\
 This file is used to log Angel Debugger Protocol packets.\n\
 With a single argument, sets the logfile name to that value.\n\
 Without an argument, shows the current logfile name.\n\
 See also: rdilogenable\n",
 	   &maintenancelist);
+  c->completer = filename_completer;
 
   add_cmd ("rdilogenable", class_maintenance,
 	   rdilogenable_command,
--- gdb/solib.c~0	Wed Jan 31 03:24:02 2001
+++ gdb/solib.c	Sat Feb 17 22:42:36 2001
@@ -37,6 +37,7 @@
 #include "environ.h"
 #include "language.h"
 #include "gdbcmd.h"
+#include "completer.h"
 
 #include "solist.h"
 
@@ -789,6 +790,8 @@ sharedlibrary_command (char *args, int f
 void
 _initialize_solib (void)
 {
+  struct cmd_list_element *c;
+
   add_com ("sharedlibrary", class_files, sharedlibrary_command,
 	   "Load shared object library symbols for files matching REGEXP.");
   add_info ("sharedlibrary", info_sharedlibrary_command,
@@ -806,18 +809,20 @@ must be loaded manually, using `sharedli
      &showlist);
 
   add_show_from_set
-    (add_set_cmd ("solib-absolute-prefix", class_support, var_filename,
-		  (char *) &solib_absolute_prefix,
-		  "Set prefix for loading absolute shared library symbol files.\n\
+    (c = add_set_cmd ("solib-absolute-prefix", class_support, var_filename,
+		      (char *) &solib_absolute_prefix,
+		      "Set prefix for loading absolute shared library symbol files.\n\
 For other (relative) files, you can add values using `set solib-search-path'.",
 		  &setlist),
      &showlist);
+  c->completer = filename_completer;
+
   add_show_from_set
-    (add_set_cmd ("solib-search-path", class_support, var_string,
-		  (char *) &solib_search_path,
-		  "Set the search path for loading non-absolute shared library symbol files.\n\
+    (c = add_set_cmd ("solib-search-path", class_support, var_string,
+		      (char *) &solib_search_path,
+		      "Set the search path for loading non-absolute shared library symbol files.\n\
 This takes precedence over the environment variables PATH and LD_LIBRARY_PATH.",
 		  &setlist),
      &showlist);
-
+  c->completer = filename_completer;
 }
--- gdb/tracepoint.c~0	Fri Dec 15 03:01:50 2000
+++ gdb/tracepoint.c	Sat Feb 17 22:42:54 2001
@@ -32,6 +32,7 @@
 #include "tracepoint.h"
 #include "remote.h"
 #include "linespec.h"
+#include "completer.h"
 
 #include "ax.h"
 #include "ax-gdb.h"
@@ -2600,6 +2601,8 @@ get_traceframe_number (void)
 void
 _initialize_tracepoint (void)
 {
+  struct cmd_list_element *c;
+
   tracepoint_chain = 0;
   tracepoint_count = 0;
   traceframe_number = -1;
@@ -2651,9 +2654,10 @@ last tracepoint set.");
 
   add_info_alias ("tp", "tracepoints", 1);
 
-  add_com ("save-tracepoints", class_trace, tracepoint_save_command,
-	   "Save current tracepoint definitions as a script.\n\
+  c = add_com ("save-tracepoints", class_trace, tracepoint_save_command,
+	       "Save current tracepoint definitions as a script.\n\
 Use the 'source' command in another debug session to restore them.");
+  c->completer = filename_completer;
 
   add_com ("tdump", class_trace, trace_dump_command,
 	   "Print everything collected at the current tracepoint.");
--- gdb/win32-nat.c~0	Sat Jan 27 21:32:32 2001
+++ gdb/win32-nat.c	Sat Feb 17 22:41:32 2001
@@ -30,6 +30,7 @@
 #include "target.h"
 #include "gdbcore.h"
 #include "command.h"
+#include "completer.h"
 #include <signal.h>
 #include <sys/types.h>
 #include <fcntl.h>
@@ -1377,10 +1378,13 @@ init_child_ops (void)
 void
 _initialize_inftarg (void)
 {
+  struct cmd_list_element *c;
+
   init_child_ops ();
 
-  add_com ("dll-symbols", class_files, dll_symbol_command,
-	   "Load dll library symbols from FILE.");
+  c = add_com ("dll-symbols", class_files, dll_symbol_command,
+	       "Load dll library symbols from FILE.");
+  c->completer = filename_completer;
 
   auto_solib_add = 1;
   add_com_alias ("sharedlibrary", "dll-symbols", class_alias, 1);

Re: [RFA] More completion improvements

[Fernando Nasser]

Eli Zaretskii wrote:
> 
> People whose approval I'm seeking are:
> 
> Fernando is listed as the maintainer of the rdi stuff.  Chris is
> responsible for win32-nat.c.  I think solib.c and tracepoint.c are
> Michael Snyder's.  cli-cmds.c seems to be Fernando's again.  I couldn't
> identify who are the maintainers for the rest.
> 

Eli,

The distribution of maintainership per file breaks in certain cases. 
One example is the CLI that is still scattered around (I need to spend
some more weekends collecting the pieces and moving them to gdb/cli).

As long you only touch things that affect the CLI command syntax,
command creation etc. without interfering with the actual
implementation, I still can give you the approval.

In the general case, it is always nice to see if the maintainer of the
file has some objection though, as he/she may have a better idea of how
something should be used or some other detail that requires a "field
expert" opinion.
This does not apply to your patch though.  You are just giving extra
information to the CLI so it can do a smarter completion.

The exception would be the extra help lines you've added.  They do
conform to the CLI requirements (first line format), so they are OK with
me.  You're are Mr. Documentation :-), so I will not argue about the
help text.  They either add information that is already in the manual or
that relate to the completion itself.

I am approving it.  IMHO this should be enough in the specific case of
this patch.

Thanks for the patch.

Fernando



P.S.: I debug Insight with the CLI (ironic, isn't it? --  but just
imagine two sets of windows for the "top-gdb" and for the "inferior"
gdb), and I am a completion addict, so I am eager to use the
improvements.

-- 
Fernando Nasser
Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

Re: [RFA] More completion improvements

[Eli Zaretskii]

> Date: Sun, 18 Feb 2001 09:55:28 -0500
> From: Fernando Nasser <fnasser@redhat.com>
> 
> In the general case, it is always nice to see if the maintainer of the
> file has some objection though, as he/she may have a better idea of how
> something should be used or some other detail that requires a "field
> expert" opinion.
> This does not apply to your patch though.  You are just giving extra
> information to the CLI so it can do a smarter completion.

Okay, thanks for clarifying this.  I think I'll wait for a day or two,
to let others chance to send comments, just to be on the safe side.

> The exception would be the extra help lines you've added.

Ahm... what extra help lines?  I don't think I added anything, I just
changed indentation a bit where it was required by calling a different
function (e.g., add_set_enum_cmd instead of add_set_cmd), which moves
the left parenthesis a bit, and thus changes indentation of the
following lines.

> I am a completion addict

That makes two of us.  I'm so addicted to completion I cannot use any
interface that lacks it without being disgusted.

Re: [RFA] More completion improvements

[Kevin Buettner]

On Feb 18,  9:31am, Eli Zaretskii wrote:

> 	* solib.c (_initialize_solib): Ditto for `solib-search-path' and
> 	`solib-absolute-prefix'.

Eli,

Your solib.c changes are approved.

I have a minor style criticism.  I would prefer to see function calls
with an embedded assignment expression written as a separate assignment
statement followed by the function call.  E.g, instead of writing...

  add_show_from_set
    (c = add_set_cmd ("solib-absolute-prefix", class_support, var_filename,
		      (char *) &solib_absolute_prefix,
		      "Set prefix for loading absolute shared library symbol files.\n\
For other (relative) files, you can add values using `set solib-search-path'.",
		  &setlist),
     &showlist);

...write this instead:

  c = add_set_cmd ("solib-absolute-prefix", class_support, var_filename,
		   (char *) &solib_absolute_prefix,
		   "Set prefix for loading absolute shared library symbol files.\n\
For other (relative) files, you can add values using `set solib-search-path'.",
		  &setlist);
  add_show_from_set (c, &showlist);

The GNU coding standards do not explicitly address this case, but they
do address a similar case regarding if-conditions.  See:

    http://www.gnu.org/prep/standards_24.html#SEC24

Kevin

Re: [RFA] More completion improvements

[Fernando Nasser]

Kevin,

This style for the add_show_from_set() call is everywhere. As we need to
revise the creation of CLI commands anyway, this will not last long
enough for us to consider a major cleanup.  It will just go away
naturally...

Regards,
Fernando


Kevin Buettner wrote:
> 
> On Feb 18,  9:31am, Eli Zaretskii wrote:
> 
> >       * solib.c (_initialize_solib): Ditto for `solib-search-path' and
> >       `solib-absolute-prefix'.
> 
> Eli,
> 
> Your solib.c changes are approved.
> 
> I have a minor style criticism.  I would prefer to see function calls
> with an embedded assignment expression written as a separate assignment
> statement followed by the function call.  E.g, instead of writing...
> 
>   add_show_from_set
>     (c = add_set_cmd ("solib-absolute-prefix", class_support, var_filename,
>                       (char *) &solib_absolute_prefix,
>                       "Set prefix for loading absolute shared library symbol files.\n\
> For other (relative) files, you can add values using `set solib-search-path'.",
>                   &setlist),
>      &showlist);
> 
> ...write this instead:
> 
>   c = add_set_cmd ("solib-absolute-prefix", class_support, var_filename,
>                    (char *) &solib_absolute_prefix,
>                    "Set prefix for loading absolute shared library symbol files.\n\
> For other (relative) files, you can add values using `set solib-search-path'.",
>                   &setlist);
>   add_show_from_set (c, &showlist);
> 
> The GNU coding standards do not explicitly address this case, but they
> do address a similar case regarding if-conditions.  See:
> 
>     http://www.gnu.org/prep/standards_24.html#SEC24
> 
> Kevin

-- 
Fernando Nasser
Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

Re: [RFA] More completion improvements

[Kevin Buettner]

On Feb 18, 12:53pm, Fernando Nasser wrote:

> This style for the add_show_from_set() call is everywhere. As we need to
> revise the creation of CLI commands anyway, this will not last long
> enough for us to consider a major cleanup.  It will just go away
> naturally...

Fernando,

I took another look, and I don't see the style that I was objecting to
used *anywhere* with add_show_from_set().  Perhaps, I was not clear
enough in my original message.  What I find (mildly) objectionable is
changing code which looks like this:

    add_show_from_set (add_set_cmd (...), &showlist);

to:

    add_show_from_set ((c = add_set_cmd (...)), &showlist);

Note the addition of an embedded assignment expression.  Instead, if
it becomes necessary to add an assignment, it should be added as a
separate statment, thusly:

    c = add_set_cmd (...);
    add_show_from_set (c, &showlist);

Kevin

Re: [RFA] More completion improvements

[Fernando Nasser]

Kevin Buettner wrote:
> 
> On Feb 18, 12:53pm, Fernando Nasser wrote:
> 
> I took another look, and I don't see the style that I was objecting to
> used *anywhere* with add_show_from_set().  Perhaps, I was not clear
> enough in my original message.  What I find (mildly) objectionable is
> changing code which looks like this:
> 
>     add_show_from_set (add_set_cmd (...), &showlist);
> 
> to:
> 
>     add_show_from_set ((c = add_set_cmd (...)), &showlist);
> 
> Note the addition of an embedded assignment expression.  Instead, if
> it becomes necessary to add an assignment, it should be added as a
> separate statment, thusly:
> 
>     c = add_set_cmd (...);
>     add_show_from_set (c, &showlist);
> 

I see what you mean now.  I thought you were referring to the 1st form.

Yes, the second one has side effects and should be split.

Eli, can you do that before checking it in?

Thanks. 


-- 
Fernando Nasser
Red Hat Canada Ltd.                     E-Mail:  fnasser@redhat.com
2323 Yonge Street, Suite #300
Toronto, Ontario   M4P 2C9

Re: [RFA] More completion improvements

[Eli Zaretskii]

> Date: Sun, 18 Feb 2001 13:47:14 -0500
> From: Fernando Nasser <fnasser@redhat.com>
> 
> Yes, the second one has side effects and should be split.
> 
> Eli, can you do that before checking it in?

Absolutely.

Re: [RFA] More completion improvements

[Eli Zaretskii]

On Sun, 18 Feb 2001, Fernando Nasser wrote:

> Yes, the second one has side effects and should be split.
> 
> Eli, can you do that before checking it in?

Changed and committed.