[RFC-version3] Fix completion bug for directories

["Pierre Muller" <muller@ics.u-strasbg.fr>]

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

This is a new patch to fix the
problem with completion on directories.

Please see
http://sourceware.org/ml/gdb-patches/2008-07/msg00072.html
and
http://sourceware.org/ml/gdb-patches/2008-07/msg00120.html
as these contain previous versions of my patch.

 To summarize, 
when you try to complete
(gdb) dir /usr/inclu
by pressing TAB key,
readline completion uses a default
set of characters to find out the last 'word' of a command line,
and, as '/' is in this set, only 'inclu' is 
sent as text argument to complete_line.
  The rl_completion_word_break_hook  allows to 
adapt the set of word_break characters
to the current command, and thus suppress '/' from
the list if a directory completion is expected.

 I ran the patch on two computer from the compile farm,
to which I now have access and besides the new
test that I added that succeeds with my patch and fails
without, I do get some random success/failure differences
in 
gdb.threads/schedlock.exp
or
gdb.threads/attach-into-signal.exp
but I understood from other emails that these are 
non-deterministic tests that do fail/succeed more or less
randomly.


Pierre Muller
Pascal language support maintainer for GDB

PS:
  Apparently, there are no listings of the header
dependencies in Makefile.in anymore, I suppose that
these are now generated automatically somehow now, is this right?
I didn't find anything in the generated Makefile,
how does that work?


gdb/ChangeLog entry:

2009-03-11  Pierre Muller  <muller@ics.u-strasbg.fr>

	Fix completer problem for filename completion on the first try.

	* gdb/completer.h (gdb_completion_word_break_characters): New
function.
	* gdb/completer.c: Include gdb_assert.h.
	(complete_line_internal_reason): New enum.
	(complete_line_internal): Change last argument type to
	complete_line_internal_reason.
	Modify function to handle the different
complete_line_internal_reason
	argument values.
	(complete_line): Adapt to change in complete_line_internal.
	(command_completer): Ditto.
	(gdb_completion_word_break_characters): Implement new function.
	* top.c (init_main): Set  rl_completion_word_break_hook to
	gdb_completion_word_break_characters.
	
gdb/testsuite/ChangeLog entry:

2009-03-11  Pierre Muller  <muller@ics.u-strasbg.fr>

	* gdb.base/completion.exp: Add a test for directory completion.




[-- Attachment #2: completer-v3.patch --]
[-- Type: application/octet-stream, Size: 9846 bytes --]

Index: src/gdb/completer.c
===================================================================
RCS file: /cvs/src/src/gdb/completer.c,v
retrieving revision 1.33
diff -u -p -r1.33 completer.c
--- src/gdb/completer.c	6 Feb 2009 21:33:58 -0000	1.33
+++ src/gdb/completer.c	6 Mar 2009 16:27:38 -0000
@@ -22,6 +22,7 @@
 #include "expression.h"
 #include "filenames.h"		/* For DOSish file names.  */
 #include "language.h"
+#include "gdb_assert.h"
 
 #include "cli/cli-decode.h"
 
@@ -481,24 +482,47 @@ expression_completer (struct cmd_list_el
    "file ../gdb.stabs/we" "ird" (needs to not break word at slash)
  */
 
-/* Generate completions all at once.  Returns a NULL-terminated array
-   of strings.  Both the array and each element are allocated with
-   xmalloc.  It can also return NULL if there are no completions.
+typedef enum
+{
+  handle_brkchars,
+  handle_completions,
+  handle_help
+}
+complete_line_internal_reason;
+
+
+/* Internal function used to handle completions.
+
 
    TEXT is the caller's idea of the "word" we are looking at.
 
    LINE_BUFFER is available to be looked at; it contains the entire text
    of the line.  POINT is the offset in that line of the cursor.  You
    should pretend that the line ends at POINT.
-   
-   FOR_HELP is true when completing a 'help' command.  In this case,
+
+   REASON is of type complete_line_internal_reason.
+
+   if REASON is handle_brkchars:
+   Preliminary phase, called by gdb_completion_word_break_characters function,
+   is used to determine the correct set of chars that are word delimiters
+   depending gon the current command in line_buffer.
+   No completion list should be generated; the return value should be NULL.
+   This is checked by an assertion in that function.
+
+   if REASON is handle_completions:
+   Main phase, called by complete_line function, is used to get the list
+   of posible completions.
+
+   if REASON is handle_help:
+   Special case when completing a 'help' command.  In this case,
    once sub-command completions are exhausted, we simply return NULL.
    When FOR_HELP is false, we will call a sub-command's completion
-   function.  */
+   function.
+ */
 
 static char **
 complete_line_internal (const char *text, char *line_buffer, int point,
-			int for_help)
+			complete_line_internal_reason reason)
 {
   char **list = NULL;
   char *tmp_command, *p;
@@ -512,7 +536,6 @@ complete_line_internal (const char *text
      functions, which can be any string) then we will switch to the
      special word break set for command strings, which leaves out the
      '-' character used in some commands.  */
-
   rl_completer_word_break_characters =
     current_language->la_word_break_characters();
 
@@ -575,12 +598,14 @@ complete_line_internal (const char *text
 	     This we can deal with.  */
 	  if (result_list)
 	    {
-	      list = complete_on_cmdlist (*result_list->prefixlist, p,
-					  word);
+	      if (reason != handle_brkchars)
+		list = complete_on_cmdlist (*result_list->prefixlist, p,
+					    word);
 	    }
 	  else
 	    {
-	      list = complete_on_cmdlist (cmdlist, p, word);
+	      if (reason != handle_brkchars)
+		list = complete_on_cmdlist (cmdlist, p, word);
 	    }
 	  /* Ensure that readline does the right thing with respect to
 	     inserting quotes.  */
@@ -604,18 +629,20 @@ complete_line_internal (const char *text
 		{
 		  /* It is a prefix command; what comes after it is
 		     a subcommand (e.g. "info ").  */
-		  list = complete_on_cmdlist (*c->prefixlist, p, word);
+		  if (reason != handle_brkchars)
+		    list = complete_on_cmdlist (*c->prefixlist, p, word);
 
 		  /* Ensure that readline does the right thing
 		     with respect to inserting quotes.  */
 		  rl_completer_word_break_characters =
 		    gdb_completer_command_word_break_characters;
 		}
-	      else if (for_help)
+	      else if (reason == handle_help)
 		list = NULL;
 	      else if (c->enums)
 		{
-		  list = complete_on_enum (c->enums, p, word);
+		  if (reason != handle_brkchars)
+		    list = complete_on_enum (c->enums, p, word);
 		  rl_completer_word_break_characters =
 		    gdb_completer_command_word_break_characters;
 		}
@@ -651,7 +678,8 @@ complete_line_internal (const char *text
 			   p--)
 			;
 		    }
-		  list = (*c->completer) (c, p, word);
+		  if (reason != handle_brkchars)
+		    list = (*c->completer) (c, p, word);
 		}
 	    }
 	  else
@@ -672,7 +700,8 @@ complete_line_internal (const char *text
 		    break;
 		}
 
-	      list = complete_on_cmdlist (result_list, q, word);
+	      if (reason != handle_brkchars)
+		list = complete_on_cmdlist (result_list, q, word);
 
 	      /* Ensure that readline does the right thing
 		 with respect to inserting quotes.  */
@@ -680,7 +709,7 @@ complete_line_internal (const char *text
 		gdb_completer_command_word_break_characters;
 	    }
 	}
-      else if (for_help)
+      else if (reason == handle_help)
 	list = NULL;
       else
 	{
@@ -694,7 +723,8 @@ complete_line_internal (const char *text
 	    }
 	  else if (c->enums)
 	    {
-	      list = complete_on_enum (c->enums, p, word);
+	      if (reason != handle_brkchars)
+		list = complete_on_enum (c->enums, p, word);
 	    }
 	  else
 	    {
@@ -719,29 +749,52 @@ complete_line_internal (const char *text
 		       p--)
 		    ;
 		}
-	      list = (*c->completer) (c, p, word);
+	      if (reason != handle_brkchars)
+		list = (*c->completer) (c, p, word);
 	    }
 	}
     }
 
   return list;
 }
+/* Generate completions all at once.  Returns a NULL-terminated array
+   of strings.  Both the array and each element are allocated with
+   xmalloc.  It can also return NULL if there are no completions.
 
-/* Like complete_line_internal, but always passes 0 for FOR_HELP.  */
+   TEXT is the caller's idea of the "word" we are looking at.
+
+   LINE_BUFFER is available to be looked at; it contains the entire text
+   of the line.
+
+   POINT is the offset in that line of the cursor.  You
+   should pretend that the line ends at POINT.  */
 
 char **
 complete_line (const char *text, char *line_buffer, int point)
 {
-  return complete_line_internal (text, line_buffer, point, 0);
+  return complete_line_internal (text, line_buffer, point, handle_completions);
 }
 
 /* Complete on command names.  Used by "help".  */
 char **
 command_completer (struct cmd_list_element *ignore, char *text, char *word)
 {
-  return complete_line_internal (word, text, strlen (text), 1);
+  return complete_line_internal (word, text, strlen (text), handle_help);
 }
 
+/* Get the list of chars that are considered as word breaks
+   for the current command.  */
+
+char *
+gdb_completion_word_break_characters (void)
+{
+  char ** list;
+  list = complete_line_internal (rl_line_buffer, rl_line_buffer, rl_point,
+				 handle_brkchars);
+  gdb_assert (list == NULL);
+  return rl_completer_word_break_characters;
+}
+
 /* Generate completions one by one for the completer.  Each time we are
    called return another potential completion to the caller.
    line_completion just completes on commands or passes the buck to the
Index: src/gdb/completer.h
===================================================================
RCS file: /cvs/src/src/gdb/completer.h,v
retrieving revision 1.16
diff -u -p -r1.16 completer.h
--- src/gdb/completer.h	6 Feb 2009 21:33:58 -0000	1.16
+++ src/gdb/completer.h	6 Mar 2009 16:27:38 -0000
@@ -33,6 +33,8 @@ extern char **command_completer (struct 
 
 extern char *get_gdb_completer_quote_characters (void);
 
+extern char *gdb_completion_word_break_characters (void);
+
 /* Exported to linespec.c */
 
 extern char *skip_quoted_chars (char *, char *, char *);
Index: src/gdb/top.c
===================================================================
RCS file: /cvs/src/src/gdb/top.c,v
retrieving revision 1.161
diff -u -p -r1.161 top.c
--- src/gdb/top.c	3 Mar 2009 13:35:24 -0000	1.161
+++ src/gdb/top.c	6 Mar 2009 16:27:38 -0000
@@ -1543,6 +1543,7 @@ init_main (void)
   write_history_p = 0;
 
   /* Setup important stuff for command line editing.  */
+  rl_completion_word_break_hook = gdb_completion_word_break_characters;
   rl_completion_entry_function = readline_line_completion_function;
   rl_completer_word_break_characters = default_word_break_characters ();
   rl_completer_quote_characters = get_gdb_completer_quote_characters ();
Index: src/gdb/testsuite/gdb.base/completion.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/completion.exp,v
retrieving revision 1.36
diff -u -p -r1.36 completion.exp
--- src/gdb/testsuite/gdb.base/completion.exp	3 Jan 2009 05:58:03 -0000	1.36
+++ src/gdb/testsuite/gdb.base/completion.exp	9 Mar 2009 08:56:58 -0000
@@ -734,6 +734,36 @@ gdb_test "cd ${fullsrcdir}" \
          "Working directory [string_to_regexp ${fullsrcdir}].*" \
          "cd to \${srcdir}"
 
+
+# GDB used to fail adding / on directories, on the first try only
+send_gdb "dir ../testsu\t"
+gdb_expect {
+	-re "../testsuite/$" {
+	    pass "directory completion"
+	    send_gdb "gdb.bas\t"
+	}
+	-re "../testsuite $" {
+	    fail "directory completion (old gdb bug)"
+	    send_gdb "\b/gdb.bas\t"
+	}
+	default {
+	    fail "directory completion (timeout)"
+	    send_gdb "\ndir ../testsuite/gdb.bas\t"
+	}
+}
+
+gdb_expect {
+	-re "gdb.base/$" {
+	    pass "directory completion 2"
+	}
+	timeout {
+	    fail "directory completion 2"
+	}
+}
+
+gdb_test "\n" "" "Glob remaining of directory test"
+
+
 send_gdb "complete file ./gdb.base/compl\n"
 sleep 1
 gdb_expect  {