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]

RE: [RFC-version3] Fix completion bug for directories


Thanks for the approval.

I committed the patch, with 
the modifications that you suggested.

Concerning the "complete" command,
it wouldn't be too hard to implement it...
I will try to send a patch fixing this shortly.


Pierre Muller
Pascal language support maintainer for GDB




> -----Message d'origine-----
> De?: gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Tom Tromey
> Envoyé?: Wednesday, March 25, 2009 1:31 AM
> À?: Pierre Muller
> Cc?: gdb-patches@sourceware.org
> Objet?: Re: [RFC-version3] Fix completion bug for directories
> 
> >>>>> "Pierre" == Pierre Muller <muller@ics.u-strasbg.fr> writes:
> 
> Pierre> This is a new patch to fix the problem with completion on
> Pierre> directories.
> 
> Oh, nice.  This has bothered me a bit.  I even dug into it once, but
> it looked like a pain.
> 
> Thanks for posting URLs of the previous revisions of this patch.  That
> helps a lot.
> 
> One question I have is whether this works with the "complete" command.
> (I suspect it does not.)  When I looked into this I sort of concluded
> that we would be better off not messing with readline's notions of
> word breaks, and furthermore that we'd be better off just adding
> filename completion directly to gdb.  But, that was a lot more work
> than I wanted to do...
> 
> I'd really like it to work with "complete", since that is what Emacs
> uses :-).  But I suppose it is not really a requirement for the patch
> -- it remains an improvement even if it is not "perfect".
> 
> Pierre> I do get some random success/failure differences
> Pierre> in
> Pierre> gdb.threads/schedlock.exp
> Pierre> or
> Pierre> gdb.threads/attach-into-signal.exp
> Pierre> but I understood from other emails that these are
> Pierre> non-deterministic tests that do fail/succeed more or less
> Pierre> randomly.
> 
> Yes, that's correct.
> 
> Pierre>   Apparently, there are no listings of the header
> Pierre> dependencies in Makefile.in anymore, I suppose that
> Pierre> these are now generated automatically somehow now, is this
> right?
> 
> Yes, dependency tracking is now automatic.
> 
> Pierre> I didn't find anything in the generated Makefile,
> Pierre> how does that work?
> 
> The dependencies themselves are stored in a .deps subdir.
> You need GNU make for this feature to be enabled.
> I can provide a fuller explanation if you want...
> 
> 
> A few nits on the patch:
> 
> +   if REASON is handle_brkchars:
> 
> Capitalize: "If".  There's a few of these.
> 
> +   depending gon the current command in line_buffer.
> 
> Typo: s/gon/on/
> 
>     When FOR_HELP is false, we will call a sub-command's completion
> -   function.  */
> +   function.
> 
> It seems to me that this entire sentence ought to be deleted.
> 
> This patch is ok with those changes.  Thanks!
> 
> Tom

For the record, here is the final patch I checked in.

Index: completer.c
===================================================================
RCS file: /cvs/src/src/gdb/completer.c,v
retrieving revision 1.33
diff -u -p -r1.33 completer.c
--- completer.c	6 Feb 2009 21:33:58 -0000	1.33
+++ completer.c	25 Mar 2009 10:48:49 -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,45 @@ 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 on 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.  */
+ */
 
 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 +534,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 +596,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 +627,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 +676,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 +698,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 +707,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 +721,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,27 +747,50 @@ 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.
+
+   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.
 
-/* Like complete_line_internal, but always passes 0 for FOR_HELP.  */
+   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
Index: completer.h
===================================================================
RCS file: /cvs/src/src/gdb/completer.h,v
retrieving revision 1.16
diff -u -p -r1.16 completer.h
--- completer.h	6 Feb 2009 21:33:58 -0000	1.16
+++ completer.h	25 Mar 2009 10:48:49 -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: top.c
===================================================================
RCS file: /cvs/src/src/gdb/top.c,v
retrieving revision 1.161
diff -u -p -r1.161 top.c
--- top.c	3 Mar 2009 13:35:24 -0000	1.161
+++ top.c	25 Mar 2009 10:48:50 -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: 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
--- testsuite/gdb.base/completion.exp	3 Jan 2009 05:58:03 -0000	1.36
+++ testsuite/gdb.base/completion.exp	25 Mar 2009 10:48:50 -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  {


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