[RFC] Make "directories" a variable

[RFC] Make "directories" a variable

[Doug Evans]

Hi.

I needed to be able to reference source_path from python,
but it's not possible today because it's not a parameter
(in the python gdb.parameter sense, and the alternative
of adding a patch to allow capturing the output of "show dir"
feels like a hack).

This patch makes it a parameter,
and I'm looking for early feedback on this approach.

I like the addition of "set directories mumble", but that's just me,
and I know better than to be wedded to it. :-)
An alternative is to making directories a variable without a setter,
or have "set dir mumble" flag an error and referring the user to
the "dir" command.
The way I look at it, "dir" is just a wrapper around "set dir".
Today "set dir" is "private/protected" (so to speak), and the public
interface is the "dir" command.
I don't have a strong opinion on making "set dir" "public", but
I do think, though, that source_path should be a parameter.

There's no current entry for "directory search list" in
enum var_types, but there are at least two variables that
are directory search lists (directories and libthread-db-search-path).
Adding enum var_dir_search_list (or some such) is a reasonable way to go.
I haven't done that yet pending approval of making "directories" a
parameter.

Tom suggested in IRC returning a list in python.
I don't have a strong opinion on that either,
it's trivial for the user to work with either form.

doc updates also deferred pending positive feedback.

2009-10-05  Doug Evans  <dje@google.com>

	Make "directories" a variable so gdb.parameter ("directories")
	works from python.
	* source.c (source_path_1): New static global.
	(strip_builtin_search_dir): New function.
	(set_directories_command): New function.
	(show_directories_1): Renamed from show_directories,
	all callers updated.
	(show_directories_command): New function.
	(_initialize_source): Remove "directories" add_cmd, replace with
	add_setshow_optional_filename_cmd.

	* testsuite/gdb.base/help.exp (show directories): Update expected
	output.

Index: source.c
===================================================================
RCS file: /cvs/src/src/gdb/source.c,v
retrieving revision 1.104
diff -u -p -u -p -r1.104 source.c
--- source.c	22 Sep 2009 22:34:17 -0000	1.104
+++ source.c	6 Oct 2009 01:50:12 -0000
@@ -66,7 +66,10 @@ static void line_info (char *, int);
 
 static void source_info (char *, int);
 
-static void show_directories (char *, int);
+/* "set directories foo" stores the value here,
+   which is later copied to SOURCE_PATH by set_directories.  */
+
+static char *source_path_1;
 
 /* Path of directories to search for source files.
    Same format as the PATH environment variable's value.  */
@@ -302,14 +305,73 @@ select_source_symtab (struct symtab *s)
   error (_("Can't find a default source file"));
 }
 \f
+/* Subroutine of set_directories_command to simplify it.
+   Strip one $cdir or $cwd from PATH (by storing '\0').
+   Returns non-zero if a path element was removed, zero otherwise.  */
+
+static int
+strip_builtin_search_dir (char *path)
+{
+  char *p;
+  char *compare_here;
+  char *strip_here;
+
+  p = strrchr (path, DIRNAME_SEPARATOR);
+  if (p != NULL)
+    {
+      compare_here = p + 1;
+      strip_here = p;
+    }
+  else
+    {
+      compare_here = path;
+      strip_here = path;
+    }
+
+  if (strcmp (compare_here, "$cdir") == 0
+      || strcmp (compare_here, "$cwd") == 0)
+    {
+      *strip_here = '\0';
+      return 1;
+    }
+
+  return 0;
+}
+
 static void
-show_directories (char *ignore, int from_tty)
+set_directories_command (char *args, int from_tty, struct cmd_list_element *c)
+{
+  /* "set dir mumble" doesn't prepend paths, it resets the entire
+     path list.  The theory is that set(show(dir)) should be a no-op.  */
+
+  /* We preserve the invariant that $cdir:$cwd is always at the end of
+     the list.  This is done by stripping any trailing $cdir:$cwd from
+     SOURCE_PATH_1, and using the same code as the "dir" command.
+     This code doesn't make an assumption on the ordering of cdir/cwd.  */
+  if (strip_builtin_search_dir (source_path_1))
+    (void) strip_builtin_search_dir (source_path_1);
+
+  xfree (source_path);
+  init_source_path ();
+  if (*source_path_1 != '\0')
+    mod_path (source_path_1, &source_path);
+}
+
+static void
+show_directories_1 (char *ignore, int from_tty)
 {
   puts_filtered ("Source directories searched: ");
   puts_filtered (source_path);
   puts_filtered ("\n");
 }
 
+static void
+show_directories_command (struct ui_file *file, int from_tty,
+			  struct cmd_list_element *c, const char *value)
+{
+  show_directories_1 (NULL, from_tty);
+}
+
 /* Forget what we learned about line positions in source files, and
    which directories contain them; must check again now since files
    may be found in a different directory now.  */
@@ -381,7 +443,7 @@ directory_command (char *dirname, int fr
       forget_cached_source_info ();
     }
   if (from_tty)
-    show_directories ((char *) 0, from_tty);
+    show_directories_1 ((char *) 0, from_tty);
 }
 
 /* Add a path given with the -d command line switch.
@@ -1926,6 +1988,7 @@ void
 _initialize_source (void)
 {
   struct cmd_list_element *c;
+
   current_source_symtab = 0;
   init_source_path ();
 
@@ -1948,16 +2011,27 @@ With no argument, reset the search path 
 
   set_cmd_completer (c, filename_completer);
 
-  add_cmd ("directories", no_class, show_directories, _("\
-Current search path for finding source files.\n\
+  add_setshow_optional_filename_cmd ("directories",
+				     class_files,
+				     &source_path_1,
+				     _("\
+Set the search path for finding source files."),
+				     _("\
+Show the search path for finding source files."),
+				     _("\
 $cwd in the path means the current working directory.\n\
-$cdir in the path means the compilation directory of the source file."),
-	   &showlist);
+$cdir in the path means the compilation directory of the source file.\n\
+GDB ensures the search path always ends with $cdir:$cwd by\n\
+appending these paths if necessary.\n\
+Setting the value to an empty string sets it to $cdir:$cwd, the default."),
+			    set_directories_command,
+			    show_directories_command,
+			    &setlist, &showlist);
 
   if (xdb_commands)
     {
       add_com_alias ("D", "directory", class_files, 0);
-      add_cmd ("ld", no_class, show_directories, _("\
+      add_cmd ("ld", no_class, show_directories_1, _("\
 Current search path for finding source files.\n\
 $cwd in the path means the current working directory.\n\
 $cdir in the path means the compilation directory of the source file."),
Index: testsuite/gdb.base/help.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/help.exp,v
retrieving revision 1.35
diff -u -p -u -p -r1.35 help.exp
--- testsuite/gdb.base/help.exp	13 Aug 2009 14:58:27 -0000	1.35
+++ testsuite/gdb.base/help.exp	6 Oct 2009 01:50:13 -0000
@@ -513,7 +513,7 @@ gdb_test "help show confirm" "Show wheth
 # test help show convenience
 gdb_test "help show convenience" "Debugger convenience \\(\"\\\$foo\"\\) variables\.\[\r\n\]+These variables are created when you assign them values;\[\r\n\]+thus, \"print \\\$foo=1\" gives \"\\\$foo\" the value 1\.  Values may be any type\.\[\r\n\]+A few convenience variables are given values automatically:\[\r\n\]+\"\\\$_\"holds the last address examined with \"x\" or \"info lines\",\[\r\n\]+\"\\\$__\" holds the contents of the last address examined with \"x\"\." "help show convenience"
 # test help show directories
-gdb_test "help show directories" "Current search path for finding source files\.\[\r\n\]+\\\$cwd in the path means the current working directory\.\[\r\n\]+\\\$cdir in the path means the compilation directory of the source file\." "help show directories"
+gdb_test "help show directories" "Show the search path for finding source files\.\[\r\n\]+\\\$cwd in the path means the current working directory\.\[\r\n\]+\\\$cdir in the path means the compilation directory of the source file\..*" "help show directories"
 # test help show editing
 gdb_test "help show editing" "Show editing of command lines as they are typed\.\[\r\n\]+Use \"on\" to enable the editing, and \"off\" to disable it\.\[\r\n\]+Without an argument, command line editing is enabled\.  To edit, use\[\r\n\]+EMACS-like or VI-like commands like control-P or ESC\." "help show editing"
 # test help show environment

Re: [RFC] Make "directories" a variable

[Eli Zaretskii]

> Date: Mon,  5 Oct 2009 19:10:38 -0700 (PDT)
> From: dje@google.com (Doug Evans)
> 
> 	(_initialize_source): Remove "directories" add_cmd, replace with
> 	add_setshow_optional_filename_cmd.

This will require a suitable change in the manual and a short note in
NEWS.

Thanks.

Re: [RFC] Make "directories" a variable

[Doug Evans]

[-- Attachment #1: Type: text/plain, Size: 1663 bytes --]

On Mon, Oct 5, 2009 at 7:10 PM, Doug Evans <dje@google.com> wrote:
> Hi.
>
> I needed to be able to reference source_path from python,
> but it's not possible today because it's not a parameter
> (in the python gdb.parameter sense, and the alternative
> of adding a patch to allow capturing the output of "show dir"
> feels like a hack).
>
> This patch makes it a parameter,
> and I'm looking for early feedback on this approach.
>
> I like the addition of "set directories mumble", but that's just me,
> and I know better than to be wedded to it. :-)
> An alternative is to making directories a variable without a setter,
> or have "set dir mumble" flag an error and referring the user to
> the "dir" command.
> The way I look at it, "dir" is just a wrapper around "set dir".
> Today "set dir" is "private/protected" (so to speak), and the public
> interface is the "dir" command.
> I don't have a strong opinion on making "set dir" "public", but
> I do think, though, that source_path should be a parameter.
>
> There's no current entry for "directory search list" in
> enum var_types, but there are at least two variables that
> are directory search lists (directories and libthread-db-search-path).
> Adding enum var_dir_search_list (or some such) is a reasonable way to go.
> I haven't done that yet pending approval of making "directories" a
> parameter.
>
> Tom suggested in IRC returning a list in python.
> I don't have a strong opinion on that either,
> it's trivial for the user to work with either form.
>
> doc updates also deferred pending positive feedback.

Heh. add_path is smarter than I thought.
Here is a revised patch (a little bit simpler).

[-- Attachment #2: gdb-091006-dir-param-2.patch.txt --]
[-- Type: text/plain, Size: 6074 bytes --]

2009-10-05  Doug Evans  <dje@google.com>

	Make "directories" a variable so gdb.parameter ("directories")
	works from python.
	* source.c (source_path_1): New static global.
	(set_directories_command): New function.
	(show_directories_1): Renamed from show_directories,
	all callers updated.
	(show_directories_command): New function.
	(_initialize_source): Remove "directories" add_cmd, replace with
	add_setshow_optional_filename_cmd.

	* testsuite/gdb.base/help.exp (show directories): Update expected
	output.

Index: source.c
===================================================================
RCS file: /cvs/src/src/gdb/source.c,v
retrieving revision 1.104
diff -u -p -r1.104 source.c
--- source.c	22 Sep 2009 22:34:17 -0000	1.104
+++ source.c	6 Oct 2009 18:06:54 -0000
@@ -66,7 +66,10 @@ static void line_info (char *, int);
 
 static void source_info (char *, int);
 
-static void show_directories (char *, int);
+/* "set directories foo" stores the value here,
+   which is later copied to SOURCE_PATH by set_directories.  */
+
+static char *source_path_1;
 
 /* Path of directories to search for source files.
    Same format as the PATH environment variable's value.  */
@@ -303,13 +306,36 @@ select_source_symtab (struct symtab *s)
 }
 \f
 static void
-show_directories (char *ignore, int from_tty)
+set_directories_command (char *args, int from_tty, struct cmd_list_element *c)
+{
+  /* "set dir mumble" doesn't prepend paths, it resets the entire
+     path list.  The theory is that set(show(dir)) should be a no-op.  */
+
+  /* We preserve the invariant that $cdir:$cwd begins life at the end of
+     the list by calling init_source_path.  If they appear earlier in
+     SOURCE_PATH_1 then mod_path will move them appropriately.
+     mod_path will also remove duplicates.  */
+  xfree (source_path);
+  init_source_path ();
+  if (*source_path_1 != '\0')
+    mod_path (source_path_1, &source_path);
+}
+
+static void
+show_directories_1 (char *ignore, int from_tty)
 {
   puts_filtered ("Source directories searched: ");
   puts_filtered (source_path);
   puts_filtered ("\n");
 }
 
+static void
+show_directories_command (struct ui_file *file, int from_tty,
+			  struct cmd_list_element *c, const char *value)
+{
+  show_directories_1 (NULL, from_tty);
+}
+
 /* Forget what we learned about line positions in source files, and
    which directories contain them; must check again now since files
    may be found in a different directory now.  */
@@ -381,7 +407,7 @@ directory_command (char *dirname, int fr
       forget_cached_source_info ();
     }
   if (from_tty)
-    show_directories ((char *) 0, from_tty);
+    show_directories_1 ((char *) 0, from_tty);
 }
 
 /* Add a path given with the -d command line switch.
@@ -1926,6 +1952,7 @@ void
 _initialize_source (void)
 {
   struct cmd_list_element *c;
+
   current_source_symtab = 0;
   init_source_path ();
 
@@ -1948,16 +1975,27 @@ With no argument, reset the search path 
 
   set_cmd_completer (c, filename_completer);
 
-  add_cmd ("directories", no_class, show_directories, _("\
-Current search path for finding source files.\n\
+  add_setshow_optional_filename_cmd ("directories",
+				     class_files,
+				     &source_path_1,
+				     _("\
+Set the search path for finding source files."),
+				     _("\
+Show the search path for finding source files."),
+				     _("\
 $cwd in the path means the current working directory.\n\
-$cdir in the path means the compilation directory of the source file."),
-	   &showlist);
+$cdir in the path means the compilation directory of the source file.\n\
+GDB ensures the search path always ends with $cdir:$cwd by\n\
+appending these paths if necessary.\n\
+Setting the value to an empty string sets it to $cdir:$cwd, the default."),
+			    set_directories_command,
+			    show_directories_command,
+			    &setlist, &showlist);
 
   if (xdb_commands)
     {
       add_com_alias ("D", "directory", class_files, 0);
-      add_cmd ("ld", no_class, show_directories, _("\
+      add_cmd ("ld", no_class, show_directories_1, _("\
 Current search path for finding source files.\n\
 $cwd in the path means the current working directory.\n\
 $cdir in the path means the compilation directory of the source file."),
Index: testsuite/gdb.base/help.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/help.exp,v
retrieving revision 1.35
diff -u -p -r1.35 help.exp
--- testsuite/gdb.base/help.exp	13 Aug 2009 14:58:27 -0000	1.35
+++ testsuite/gdb.base/help.exp	6 Oct 2009 18:06:54 -0000
@@ -513,7 +513,7 @@ gdb_test "help show confirm" "Show wheth
 # test help show convenience
 gdb_test "help show convenience" "Debugger convenience \\(\"\\\$foo\"\\) variables\.\[\r\n\]+These variables are created when you assign them values;\[\r\n\]+thus, \"print \\\$foo=1\" gives \"\\\$foo\" the value 1\.  Values may be any type\.\[\r\n\]+A few convenience variables are given values automatically:\[\r\n\]+\"\\\$_\"holds the last address examined with \"x\" or \"info lines\",\[\r\n\]+\"\\\$__\" holds the contents of the last address examined with \"x\"\." "help show convenience"
 # test help show directories
-gdb_test "help show directories" "Current search path for finding source files\.\[\r\n\]+\\\$cwd in the path means the current working directory\.\[\r\n\]+\\\$cdir in the path means the compilation directory of the source file\." "help show directories"
+gdb_test "help show directories" "Show the search path for finding source files\.\[\r\n\]+\\\$cwd in the path means the current working directory\.\[\r\n\]+\\\$cdir in the path means the compilation directory of the source file\..*" "help show directories"
 # test help show editing
 gdb_test "help show editing" "Show editing of command lines as they are typed\.\[\r\n\]+Use \"on\" to enable the editing, and \"off\" to disable it\.\[\r\n\]+Without an argument, command line editing is enabled\.  To edit, use\[\r\n\]+EMACS-like or VI-like commands like control-P or ESC\." "help show editing"
 # test help show environment

Re: [RFC] Make "directories" a variable

[Tom Tromey]

>>>>> "Doug" == Doug Evans <dje@google.com> writes:

Doug> I like the addition of "set directories mumble", but that's just me,
Doug> and I know better than to be wedded to it. :-)

This part is totally fine by me.  I can't imagine ever actually using
it -- but that is true for any number of already existing parameters :)

Doug> Tom suggested in IRC returning a list in python.
Doug> I don't have a strong opinion on that either,
Doug> it's trivial for the user to work with either form.

There are three reasons I think a python list is better.

1. It conforms more nicely to Python programmer expectations and is
   simply a better API.  This really is a list.
2. Most uses of this parameter will want to use the list form, not the
   string form.
3. If it is a string, then most users will have to write Python code to
   unpack and interpret it.  This is bad because some list elements
   require special treatment, the string is formatted differently on
   different hosts, etc.

Tom