This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[RFC] fix annoying completion bug on directories


  I was always annoyed by a small but annoying bug in the
gdb TAB completion for directories.

If you start gdb without args and type:
"(gdb) file /usr/inclu"
and you now type TAB to get the completion, 
you would expect to get 
"(gdb) file /usr/include/"
but instead you will get
"(gdb) file /usr/include "
but if you delete the "de " and type TAB char
again, you will get the correct answer!

  This all looked like some wrongly set parameter...

  It took me a while to figure out what is 
going on and why this bug appears:

  the function 
  _rl_find_completion_word,
called from rl_complete_internal
is determining the start of the "name"
that needs to be completed.
  But this is before gdb specific completion code 
is called, meaning that the _rl_find_completion_word
finds "inclu" as word the first time, because
standard delimiters are oriented towards usual words.
Only later (too late), completer.c complete_line function
is called.
  This functions sets the variable
rl_completer_word_break_characters,
a char pointer listing all chars that
should mark the start or end of a word,
according to the current context.
  At that point, this char pointer is changed to
a filename specific constant that will accept
'/' as part of a word.

  This explains why at the second try,
the word found by _rl_find_completion_word
is now "/usr/inclu".

  In both cases, the completion to include is found,
but again in the first case the word "include"
alone is not recognized as a directory and thus a
space is added at the end, whereas
"/usr/include" is recognized as a directory
and the '/' is appended correctly.

  Finding the bug is one thing, finding a fix for
it is another...
   The variable rl_completer_word_break_characters
is set in one function, called
complete_line_function from completer.c source.

  As the code is rather convoluted, I tried to just 
created a "special" case for which only this variable 
would be set, without really trying to find 
the completions.

  Luckily, there is a hook function inside 
_rl_find_completion_word that allows to
change the default value of the list of chars
considered as marking word start/end.
 
  So I wrote a hook that calls complete_line_function
with a first NULL char *, rl_line_buffer s the second arg
and rl_point as the third.
  The NULL as first arg is used as a trigger 
not to try to create a list of completions,
but only to set the current value to this list of
word delimiter chars.
  It seems that this first parameter is always
non NULL, otherwise, an inconditional call to strlen would
generate a SIGSEGV.

  I found that this works nicely...

  I ran the testsuite on cygwin, and found one
change, but in gdb.gdb/selftest.
  FAIL (timeout) xgdb is at prompt

  I suspect that this is due to a problem with the
[New thread %s] output, that are now default.
Because this output appears right after the gdb prompt,
and thus the match with a gdb prompt exactly at the end
is not found.

By the way, this results in an odd situation for cygwin:
by default, new threads generate a notice,
but nothing for exiting threads...
  I saw something about harmonization of thread
information for new threads, shouldn't we do the same for
exiting threads?

  I found no change in the tests dealing with completion,
but I also don't really know how to add such a test...
  

Pierre Muller
Pascal language support maintainer for GDB




ChangeLog entry:

2008-06-06  Pierre Muller  <muller@ics.u-strasbg.fr>

	* gdb/completer.c: Include gdb_assert.h
	(current_gdb_completer_word_break_characters): New static variable.
	(complete_line): If first arg is NULL, only set 
	current_gdb_completer_word_break_characters.
	(gdb_completion_word_break): New static function.
	(_initialize_completer): New initializer, sets readline word break 
	chars hook to gdb_completion_word_break.


Index: gdb/completer.c
===================================================================
RCS file: /cvs/src/src/gdb/completer.c,v
retrieving revision 1.26
diff -u -p -r1.26 completer.c
--- gdb/completer.c	9 Jun 2008 19:25:14 -0000	1.26
+++ gdb/completer.c	4 Jul 2008 23:38:18 -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"
 
@@ -57,6 +58,8 @@ char *line_completion_function (const ch
 
 /* Variables which are necessary for fancy command line editing.  */
 
+static char *current_gdb_completer_word_break_characters = NULL;
+
 /* When completing on command names, we remove '-' from the list of
    word break characters, since we use it in command names.  If the
    readline library sees one in any of the current completion strings,
@@ -472,20 +475,29 @@ command_completer (char *text, char *wor
 char **
 complete_line (const char *text, char *line_buffer, int point)
 {
+  int only_brkchars = 0;
   char **list = NULL;
   char *tmp_command, *p;
+  
   /* Pointer within tmp_command which corresponds to text.  */
   char *word;
   struct cmd_list_element *c, *result_list;
 
+  /* If text is NULL, we only want to set the variable
+     current_gdb_completer_word_break_characters.  */
+  if (!text)
+    {
+      only_brkchars = 1;
+      text = line_buffer;
+    }
+ 
   /* Choose the default set of word break characters to break completions.
      If we later find out that we are doing completions on command strings
      (as opposed to strings supplied by the individual command completer
      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_gdb_completer_word_break_characters = 
     current_language->la_word_break_characters();
 
   /* Decide whether to complete on a list of gdb commands or on symbols. */
@@ -547,16 +559,19 @@ complete_line (const char *text, char *l
 	     This we can deal with.  */
 	  if (result_list)
 	    {
-	      list = complete_on_cmdlist (*result_list->prefixlist, p,
-					  word);
+	      if (!only_brkchars)
+		list = complete_on_cmdlist (*result_list->prefixlist, p,
+					    word);
 	    }
 	  else
 	    {
-	      list = complete_on_cmdlist (cmdlist, p, word);
+	      if (!only_brkchars)
+		list = complete_on_cmdlist (cmdlist, p, word);
 	    }
 	  /* Ensure that readline does the right thing with respect to
 	     inserting quotes.  */
-	  rl_completer_word_break_characters =
+
+	  current_gdb_completer_word_break_characters = 
 	    gdb_completer_command_word_break_characters;
 	}
     }
@@ -575,18 +590,20 @@ complete_line (const char *text, char *l
 	      if (c->prefixlist)
 		{
 		  /* It is a prefix command; what comes after it is
-		     a subcommand (e.g. "info ").  */
-		  list = complete_on_cmdlist (*c->prefixlist, p, word);
+		      a subcommand (e.g. "info ").  */
+		  if (!only_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 =
+		  current_gdb_completer_word_break_characters = 
 		    gdb_completer_command_word_break_characters;
 		}
 	      else if (c->enums)
 		{
-		  list = complete_on_enum (c->enums, p, word);
-		  rl_completer_word_break_characters =
+		  if (!only_brkchars)
+		    list = complete_on_enum (c->enums, p, word);
+		  current_gdb_completer_word_break_characters = 
 		    gdb_completer_command_word_break_characters;
 		}
 	      else
@@ -608,7 +625,7 @@ complete_line (const char *text, char *l
 			     && strchr
(gdb_completer_file_name_break_characters, p[-1]) == NULL;
 			   p--)
 			;
-		      rl_completer_word_break_characters =
+		      current_gdb_completer_word_break_characters = 
 			gdb_completer_file_name_break_characters;
 		    }
 		  else if (c->completer == location_completer)
@@ -621,7 +638,8 @@ complete_line (const char *text, char *l
 			   p--)
 			;
 		    }
-		  list = (*c->completer) (p, word);
+		  if (!only_brkchars)
+		    list = (*c->completer) (p, word);
 		}
 	    }
 	  else
@@ -642,11 +660,12 @@ complete_line (const char *text, char *l
 		    break;
 		}
 
-	      list = complete_on_cmdlist (result_list, q, word);
+	      if (!only_brkchars)
+		list = complete_on_cmdlist (result_list, q, word);
 
 	      /* Ensure that readline does the right thing
 		 with respect to inserting quotes.  */
-	      rl_completer_word_break_characters =
+	      current_gdb_completer_word_break_characters = 
 		gdb_completer_command_word_break_characters;
 	    }
 	}
@@ -662,7 +681,8 @@ complete_line (const char *text, char *l
 	    }
 	  else if (c->enums)
 	    {
-	      list = complete_on_enum (c->enums, p, word);
+	      if (!only_brkchars)
+		list = complete_on_enum (c->enums, p, word);
 	    }
 	  else
 	    {
@@ -676,7 +696,7 @@ complete_line (const char *text, char *l
 			 && strchr
(gdb_completer_file_name_break_characters, p[-1]) == NULL;
 		       p--)
 		    ;
-		  rl_completer_word_break_characters =
+		  current_gdb_completer_word_break_characters = 
 		    gdb_completer_file_name_break_characters;
 		}
 	      else if (c->completer == location_completer)
@@ -687,14 +707,25 @@ complete_line (const char *text, char *l
 		       p--)
 		    ;
 		}
-	      list = (*c->completer) (p, word);
+	      if (!only_brkchars)
+		list = (*c->completer) (p, word);
 	    }
 	}
     }
-
+  rl_completer_word_break_characters =
+    current_gdb_completer_word_break_characters;
   return list;
 }
 
+static char *
+gdb_completion_word_break ()
+{
+  gdb_assert (complete_line (NULL, rl_line_buffer, rl_point) == NULL);
+ 
+  return current_gdb_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
@@ -821,3 +852,10 @@ skip_quoted (char *str)
 {
   return skip_quoted_chars (str, NULL, NULL);
 }
+
+
+void
+_initialize_completer (void)
+{
+  rl_completion_word_break_hook = gdb_completion_word_break;
+}



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]