[RFC] fix annoying completion bug on directories

[RFC] fix annoying completion bug on directories

[Pierre Muller]

  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;
+}

Re: [RFC] fix annoying completion bug on directories

[Pedro Alves]

A Sunday 06 July 2008 00:37:22, Pierre Muller wrote:
>   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!
>

Oh boy, thanks for looking into this!
This has been annoying me a lot as well.

<warning note>
 I went ahead and tried to understand your changes, but beware
 that my readline+GDB foo is low.
</warning note>

>   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:
>

...


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

Urgl.  This means that finding the completion word is
always somewhat broken on the next command, when the next
command should use a different word break character
list?

>   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.

Did you consider breaking complete_line in two?  There's more
you could skip that just the completion calls.
Hmm, doesn't look like it would be a maintainable result.  Basically
the parsing, command lookups and most of the comments would
have to be duplicated, or a weird abstraction would have to
be employed.  Not sure if the result would be better at all.


>
>   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.
>

Yay!

>   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.

In these cases, in addition to analising the log file, please
rerun the test to confirm it was spurious.  Cygwin testing
is indeed flackier than linux.

Just do make check RUNTESTFLAGS="failingtest.exp" a couple of
times.  If it's always failing now, it's a regression.  If
the failure goes aways, then also retest with with the
patch unapplied.  You should see the flackiness too.  If
not, it may be a regression, but you'll have to judge from
the logs -- it may just be difficult to reproduce flackiness.

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

Take a look at the completion.exp test for the test that does:
send_gdb "file ./gdb.base/complet\t"

Maybe you could do something similar?  Event this fails
to complete the '/' the first time without your patch:
send_gdb "dir ..\t"

>
> ChangeLog entry:
>
> 2008-06-06  Pierre Muller  <muller@ics.u-strasbg.fr>
        ^

You have to do something about that calendar :-)

>
> 	* gdb/completer.c: Include gdb_assert.h

Please also update Makefile.in.  Still needed until we have
automatic dependency tracking...

>  /* 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)

Please add somewhere describing the change you're making, and why
it's needed.

>  {
> +  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;
> +    }
> +

Another alternative would be to add a new parameter to
the function, rename it, and add a new complete_line function
as wrapper to the old one:

char **
complete_or_set_brkchars (const char *text, char *line_buffer,
                          int point, int what_to_do)
{
  ...
}

char **
complete_line (const char *text, char *line_buffer, int point)
{
  complete_or_set_brkchars (text, line_buffer, complete_please);
}

static char *
gdb_completion_word_break (void)
{
  complete_or_set_brkchars (rl_line_buffer,
                            rl_line_buffer,
                            rl_point, set_brkchars_please);
}


> -		  rl_completer_word_break_characters =
> +		  current_gdb_completer_word_break_characters =
>  		    gdb_completer_file_name_break_characters;
>  		}
> -
> +  rl_completer_word_break_characters =
> +    current_gdb_completer_word_break_characters;
>    return list;

You changed a bunch of rl_completer_word_break_characters
to current_gdb_completer_word_break_characters, only to in
the end set them to be the same.  If you do it the other way
around, you'd only be adding a single:

current_gdb_completer_word_break_characters
  = rl_completer_word_break_characters;

at the end of complete_line.


>  }
>
> +static char *
> +gdb_completion_word_break ()
                              ^
                              (void)

> +{
> +  gdb_assert (complete_line (NULL, rl_line_buffer, rl_point) == NULL);

Please, no side effects in assert statements.  We always build
with them on, but it's bad practice to rely on that.  If you want
to keep the assert, do it like:

char **list = complete_line (NULL, rl_line_buffer, rl_point);
gdb_assert (line == NULL);

And please add a comment somewhere describing what this is for.

> +
> +  return current_gdb_completer_word_break_characters;
> +}

from readline.c:complete.c:

char
_rl_find_completion_word (fp, dp)
     int *fp, *dp;
{
  int scan, end, found_quote, delimiter, pass_next, isbrk;
  char quote_char, *brkchars;

  end = rl_point;
  found_quote = delimiter = 0;
  quote_char = '\0';

  brkchars = 0;
  if (rl_completion_word_break_hook)
    brkchars = (*rl_completion_word_break_hook) ();
  if (brkchars == 0)
    brkchars = rl_completer_word_break_characters;


It looks like you could get rid of current_gdb_completer_word_break_characters 
entirely, and return either NULL or rl_completer_word_break_characters
in your new hook; or if keeping current_gdb_comp... not set 
rl_completer_word_break_characters in complete_line at all?

Also, doesn't filename_completer always get text == word now?
If so, there's some code in there that turns dead (both the
if text < word, and the else clauses).


> +void
> +_initialize_completer (void)
> +{
> +  rl_completion_word_break_hook = gdb_completion_word_break;

Did you consider putting this in init_main near the other rl_
initializations?  OTHO, rl_completer_word_break_characters
was indeed already mostly only tweaked in this file, and it
goes in hand with this hook...

-- 
Pedro Alves

Spurius results for cygwin selftest.exp(was Re: [RFC] fix annoying completion bug on directories)

[Pierre Muller]

> >   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.
> 
> In these cases, in addition to analising the log file, please
> rerun the test to confirm it was spurious.  Cygwin testing
> is indeed flackier than linux.

  This is indeed a spurious FAIL that has to do with that
third thread that start on cygwin right after the command prompt is
printed out.
  Does anyone know why there is a third thread starting?
Debugging threads under cygwin is quite difficult, you never really know
where they are coming from...

  I am really surprised that now, by default, gdb will print 
debug message, while the inferior is running, without stopping it.
  For me, when gdb is started with the default options,
gdb should only output text if it stops the debuggee, otherwise
it becomes difficult for the average user to handle.
  But I understood that the [New thread... message was
already the default on Linux targets..
> 
> Just do make check RUNTESTFLAGS="failingtest.exp" a couple of
> times.  If it's always failing now, it's a regression.  If
> the failure goes aways, then also retest with with the
> patch unapplied.  You should see the flackiness too.  If
> not, it may be a regression, but you'll have to judge from
> the logs -- it may just be difficult to reproduce flackiness.

  I got it give PASS or FAIL several times,
it's really a question of how much time there is
in between the (gdb) prompt and the [New thread...] message.

  I could of of course update the selftest.exp test
to also accept the "[New thread ...]" message,
but I would like your opinions about this before,
we could easily suppress these messages by setting
print_thread_events to zero by default.


Pierre Muller
Pascal language support maintainer for GDB

  

[RFA] fix annoying completion bug on directories

[Pierre Muller]

  Thanks again for the comments, Pedro.

> Objet : Re: [RFC] fix annoying completion bug on directories
> 
> A Sunday 06 July 2008 00:37:22, Pierre Muller wrote:
> >   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!
> >
> 
> Oh boy, thanks for looking into this!
> This has been annoying me a lot as well.

  Does it also deserve a NEWS entry?

 
> <warning note>
>  I went ahead and tried to understand your changes, but beware
>  that my readline+GDB foo is low.
> </warning note>
> 
> >   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:
> >
> 
> ...
> 
> 
> >   This explains why at the second try,
> > the word found by _rl_find_completion_word
> > is now "/usr/inclu".
> >
> 
> Urgl.  This means that finding the completion word is
> always somewhat broken on the next command, when the next
> command should use a different word break character
> list?
 That's what I understood, but I did not
really check.

> >   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.
> 
> Did you consider breaking complete_line in two?  There's more
> you could skip that just the completion calls.
> Hmm, doesn't look like it would be a maintainable result.  Basically
> the parsing, command lookups and most of the comments would
> have to be duplicated, or a weird abstraction would have to
> be employed.  Not sure if the result would be better at all.
> 
 As you suggested, I put the code into a new static
function named complete_line_1, see the new patch version
below.

> >
> >   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.
> >
> 
> Yay!
> 
> >   I found that this works nicely...
> >
I reran the tests on cygwin without any
changes, and moved the discussion about the
spurious test results to another email.


> >   I found no change in the tests dealing with completion,
> > but I also don't really know how to add such a test...
> >
> 
> Take a look at the completion.exp test for the test that does:
> send_gdb "file ./gdb.base/complet\t"
> 
> Maybe you could do something similar?  Event this fails
> to complete the '/' the first time without your patch:
> send_gdb "dir ..\t"

  I tried to create something...

> >
> > ChangeLog entry:
> >
> > 2008-06-06  Pierre Muller  <muller@ics.u-strasbg.fr>
>         ^
> 
> You have to do something about that calendar :-)

  I had to compensate for the stuff I tagged as being in December, I guess.

 
> >
> > 	* gdb/completer.c: Include gdb_assert.h
> 
> Please also update Makefile.in.  Still needed until we have
> automatic dependency tracking...
 
I added this one.
 
> >  /* 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)
> 
> Please add somewhere describing the change you're making, and why
> it's needed.

I hope that the new patch answers this too.

> >  {
> > +  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;
> > +    }
> > +
> 
> Another alternative would be to add a new parameter to
> the function, rename it, and add a new complete_line function
> as wrapper to the old one:
> 
> char **
> complete_or_set_brkchars (const char *text, char *line_buffer,
>                           int point, int what_to_do)
> {
>   ...
> }

 That's what I did, but I called it complete_line_1
 
> char **
> complete_line (const char *text, char *line_buffer, int point)
> {
>   complete_or_set_brkchars (text, line_buffer, complete_please);
> }
> 
> static char *
> gdb_completion_word_break (void)
> {
>   complete_or_set_brkchars (rl_line_buffer,
>                             rl_line_buffer,
>                             rl_point, set_brkchars_please);
> }
> 
> 
> > -		  rl_completer_word_break_characters =
> > +		  current_gdb_completer_word_break_characters =
> >  		    gdb_completer_file_name_break_characters;
> >  		}
> > -
> > +  rl_completer_word_break_characters =
> > +    current_gdb_completer_word_break_characters;
> >    return list;
> 
> You changed a bunch of rl_completer_word_break_characters
> to current_gdb_completer_word_break_characters, only to in
> the end set them to be the same.  If you do it the other way
> around, you'd only be adding a single:
> 
> current_gdb_completer_word_break_characters
>   = rl_completer_word_break_characters;
> 
> at the end of complete_line.
> 

  I totally removed that new variable,
as it wasn't really useful.
> >  }
> >
> > +static char *
> > +gdb_completion_word_break ()
>                               ^
>                               (void)
> 
> > +{
> > +  gdb_assert (complete_line (NULL, rl_line_buffer, rl_point) ==
> NULL);
> 
> Please, no side effects in assert statements.  We always build
> with them on, but it's bad practice to rely on that.  If you want
> to keep the assert, do it like:
>
> char **list = complete_line (NULL, rl_line_buffer, rl_point);
> gdb_assert (line == NULL);

  Didn't know that rule, code changed accordingly.
 
> And please add a comment somewhere describing what this is for.

Done in the comments above complete_line_1

> > +
> > +  return current_gdb_completer_word_break_characters;
> > +}
> 
> from readline.c:complete.c:
> 
> char
> _rl_find_completion_word (fp, dp)
>      int *fp, *dp;
> {
>   int scan, end, found_quote, delimiter, pass_next, isbrk;
>   char quote_char, *brkchars;
> 
>   end = rl_point;
>   found_quote = delimiter = 0;
>   quote_char = '\0';
> 
>   brkchars = 0;
>   if (rl_completion_word_break_hook)
>     brkchars = (*rl_completion_word_break_hook) ();
>   if (brkchars == 0)
>     brkchars = rl_completer_word_break_characters;
> 
> 
> It looks like you could get rid of
> current_gdb_completer_word_break_characters
> entirely, and return either NULL or rl_completer_word_break_characters
> in your new hook; or if keeping current_gdb_comp... not set
> rl_completer_word_break_characters in complete_line at all?
> 
> Also, doesn't filename_completer always get text == word now?
> If so, there's some code in there that turns dead (both the
> if text < word, and the else clauses).
> 
 
You seem to be right here, but I would prefer not 
to touch this for now...

> > +void
> > +_initialize_completer (void)
> > +{
> > +  rl_completion_word_break_hook = gdb_completion_word_break;
> 
> Did you consider putting this in init_main near the other rl_
> initializations?  OTHO, rl_completer_word_break_characters
> was indeed already mostly only tweaked in this file, and it
> goes in hand with this hook...

Done as you suggested.


Pierre Muller
Pascal language support maintainer for GDB





gdb ChangeLog entry:

2008-07-08  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_1): New static function, 
	contains most of previous complete_line implementation,
	extended to handle only setting of
rl_completer_word_break_characters.
	(complete_line): Now simply calls complete_line_1 with
	only_brkchars=0.
	(gdb_completion_word_break): New static function.
	Calls complete_line_1 with only_brkchars=1.
	* top.c (init_main): Set  rl_completion_word_break_hook to
	gdb_completion_word_break_characters;
	* Makefile (completer.o): Add gdb_assert_h dependency.
	
	
Index: gdb/Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/Makefile.in,v
retrieving revision 1.1029
diff -u -p -r1.1029 Makefile.in
--- gdb/Makefile.in	27 Jun 2008 11:54:21 -0000	1.1029
+++ gdb/Makefile.in	8 Jul 2008 19:36:43 -0000
@@ -2009,7 +2009,7 @@ complaints.o: complaints.c $(defs_h) $(c
 	$(command_h) $(gdbcmd_h)
 completer.o: completer.c $(defs_h) $(symtab_h) $(gdbtypes_h)
$(expression_h) \
 	$(filenames_h) $(language_h) $(cli_decode_h) $(gdbcmd_h) \
-	$(readline_h) $(completer_h)
+	$(readline_h) $(completer_h) $(gdb_assert_h)
 copying.o: copying.c $(defs_h) $(command_h) $(gdbcmd_h)
 corefile.o: corefile.c $(defs_h) $(gdb_string_h) $(inferior_h) $(symtab_h)
\
 	$(command_h) $(gdbcmd_h) $(bfd_h) $(target_h) $(gdbcore_h) \
Index: gdb/completer.h
===================================================================
RCS file: /cvs/src/src/gdb/completer.h,v
retrieving revision 1.14
diff -u -p -r1.14 completer.h
--- gdb/completer.h	6 Jun 2008 20:58:08 -0000	1.14
+++ gdb/completer.h	8 Jul 2008 19:36:43 -0000
@@ -33,6 +33,8 @@ extern char **command_completer (char *,
 
 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: 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	8 Jul 2008 19:36:43 -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"
 
@@ -458,19 +459,19 @@ command_completer (char *text, char *wor
    "file Make" "file" (word break hard to screw up here)
    "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.
-
-   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)
+/* Completion is done in two parts:
+   - first phase, with only_brkchars=1, 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.
+   -second phase, with only_brkchars=0, called by complete_line function,
+   is used to get the list of posible completions.  */
+ 
+static char **
+complete_line_1 (const char *text, char *line_buffer, int point, 
+		 int only_brkchars)
 {
   char **list = NULL;
   char *tmp_command, *p;
@@ -484,7 +485,6 @@ complete_line (const char *text, char *l
      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();
 
@@ -547,12 +547,14 @@ 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.  */
@@ -576,7 +578,8 @@ complete_line (const char *text, char *l
 		{
 		  /* It is a prefix command; what comes after it is
 		     a subcommand (e.g. "info ").  */
-		  list = complete_on_cmdlist (*c->prefixlist, p, word);
+		  if (!only_brkchars)
+		    list = complete_on_cmdlist (*c->prefixlist, p, word);
 
 		  /* Ensure that readline does the right thing
 		     with respect to inserting quotes.  */
@@ -585,7 +588,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);
 		  rl_completer_word_break_characters =
 		    gdb_completer_command_word_break_characters;
 		}
@@ -621,7 +625,8 @@ complete_line (const char *text, char *l
 			   p--)
 			;
 		    }
-		  list = (*c->completer) (p, word);
+		  if (!only_brkchars)
+		    list = (*c->completer) (p, word);
 		}
 	    }
 	  else
@@ -642,7 +647,8 @@ 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.  */
@@ -662,7 +668,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
 	    {
@@ -687,7 +694,8 @@ complete_line (const char *text, char *l
 		       p--)
 		    ;
 		}
-	      list = (*c->completer) (p, word);
+	      if (!only_brkchars)
+		list = (*c->completer) (p, word);
 	    }
 	}
     }
@@ -695,6 +703,33 @@ complete_line (const char *text, char *l
   return list;
 }
 
+/* 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_1 (rl_line_buffer, rl_line_buffer, rl_point, 1);
+  gdb_assert (list == NULL);
+  return rl_completer_word_break_characters;
+}
+/* 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.  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_1 (text, line_buffer, point, 0);
+} 
+
 /* 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: gdb/top.c
===================================================================
RCS file: /cvs/src/src/gdb/top.c,v
retrieving revision 1.143
diff -u -p -r1.143 top.c
--- gdb/top.c	10 Jun 2008 11:57:28 -0000	1.143
+++ gdb/top.c	8 Jul 2008 19:36:43 -0000
@@ -1524,6 +1524,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 ();


gdb/testsuite ChangeLog entry:
2008-07-08  Pierre Muller  <muller@ics.u-strasbg.fr>

	* gdb.base/completion.exp: Add test for completer problem for 
	filename completion on the first try.

Index: gdb.base/completion.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/completion.exp,v
retrieving revision 1.31
diff -u -r1.31 completion.exp
--- gdb.base/completion.exp	9 Jun 2008 19:25:15 -0000	1.31
+++ gdb.base/completion.exp	8 Jul 2008 22:13:16 -0000
@@ -712,23 +712,62 @@
 set fullsrcdir [pwd]
 cd ${mydir}
 
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
 # If the directory name contains a '+' we must escape it, adding a
backslash.
 # If not, the test below will fail because it will interpret the '+' as a 
 # regexp operator. We use string_to_regexp for this purpose.
 
-gdb_test "cd ${fullsrcdir}" \
-         "Working directory [string_to_regexp ${fullsrcdir}].*" \
-         "cd to \${srcdir}"
+# Test the file correctly adds a '/' at end of completion
+# even the first time a file completion is used
+
+send_gdb "dir ${srcdir}/gdb.bas\t"
+
+gdb_expect {
+  -re "^dir ${srcdir}/gdb.base/$" {
+    pass "complete dir ${srcdir}/gdb.base/"
+  }
+  -re "^dir ${srcdir}/gdb.base $" {
+    send_gdb "\b\b\t"
+    gdb_expect {
+       -re "(.*e/)$" {
+	 set current_line "$expect_out(1,string)"
+         fail "wrong first, correct second completion for command
'${current_line}'"
+       }
+       timeout { fail "dir ${srcdir}/gdb.base/ (timeout 2)" }
+    }
+  }
+  timeout { fail "dir ${srcdir}/gdb.base/ (timeout)" }
+}
+
+
+send_gdb "complet\t"
 
-send_gdb "complete file ./gdb.base/compl\n"
 sleep 1
+
 gdb_expect  {
-    -re "file ./gdb.base/completion\\.exp.*$gdb_prompt $"
-	{ pass "complete-command 'file ./gdb.base/compl'"}
-    -re ".*$gdb_prompt $"       { fail "complete-command 'file
./gdb.base/compl'" }
-    timeout         { fail "(timeout) complete-command 'file
./gdb.base/compl'" }
+    -re ".*completion\\.exp $"
+	{ pass "complete-command 'dir ${srcdir}/gdb.base/complet'"}
+    -re ".*$gdb_prompt $" 
+        { fail "complete-command 'dir ${srcdir}/gdb.base/complet'" }
+    timeout 
+        { fail "(timeout) complete-command 'dir
${srcdir}/gdb.base/complet'" }
 }
 
+# Press return and don't care about errors
+
+gdb_test " " ".*" "Back to normal"
+
+# Change to testsuite source directory
+gdb_test "cd ${fullsrcdir}" \
+         "Working directory [string_to_regexp ${fullsrcdir}].*" \
+         "cd to \${srcdir}"
+
+
+
 send_gdb "file ./gdb.base/complet\t"
 sleep 1
 gdb_expect  {
@@ -747,6 +786,8 @@
         timeout         { fail "(timeout) complete 'file
./gdb.base/complet'" }
         }
 
+gdb_test " " ".*" "Back to normal"
+
 send_gdb "info func marke\t"
 sleep 1
 gdb_expect  {
@@ -780,15 +821,15 @@
             { send_gdb "\n"
               gdb_expect {
                       -re "Requires an argument.*child.*parent.*$gdb_prompt
$"\
-                                        { pass "complete 'set
follow-fork-mode'"}
+                                        { pass "complete 'set
follow-fork-mode '"}
                       -re "Ambiguous item \"\"\\..*$gdb_prompt $"\
                                         { pass "complete 'set
follow-fork-mode'"}
-                      -re ".*$gdb_prompt $" { fail "complete 'set
follow-fork-mode'"}
-                      timeout           {fail "(timeout) complete 'set
follow-fork-mode'"}
+                      -re ".*$gdb_prompt $" { fail "complete 'set
follow-fork-mode '"}
+                      timeout           {fail "(timeout) complete 'set
follow-fork-mode '"}
                      }
             }
-        -re ".*$gdb_prompt $"       { fail "complete 'set
follow-fork-mode'" }
-        timeout         { fail "(timeout) complete 'set follow-fork-mode'"
}
+        -re ".*$gdb_prompt $"       { fail "complete 'set follow-fork-mode
'" }
+        timeout         { fail "(timeout) complete 'set follow-fork-mode '"
}
         }
 
 # Restore globals modified in this test...

Re: [RFA] fix annoying completion bug on directories

[Daniel Jacobowitz]

FYI, I worked on this once:

  http://sourceware.org/ml/gdb/2004-01/msg00222.html

The result was rl_completion_word_break_hook, which is exactly what
you're using here.  But I never got back to working on it, so we never
took advantage of the new hook even though we imported a new enough
readline.

Thanks for doing this!

Sorry, this is very complicated code; I will have to find another
time to review it.  But I wanted to thank you for fixing the bug.

-- 
Daniel Jacobowitz
CodeSourcery

RE: [RFA] fix annoying completion bug on directories

[Pierre Muller]

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Daniel Jacobowitz
> Envoyé : Wednesday, July 09, 2008 12:31 AM
> À : Pierre Muller
> Cc : 'Pedro Alves'; gdb-patches@sourceware.org
> Objet : Re: [RFA] fix annoying completion bug on directories
> 
> FYI, I worked on this once:
> 
>   http://sourceware.org/ml/gdb/2004-01/msg00222.html

  A pity I didn't find this myself earlier...
It would have simplified my debugging part...
 
> The result was rl_completion_word_break_hook, which is exactly what
> you're using here.  But I never got back to working on it, so we never
> took advantage of the new hook even though we imported a new enough
> readline.

  So the hook is there because you asked for it!
  This is really neat, without this, I would have
abandoned it.
 
> Thanks for doing this!

Thanks for get the hook!
 
> Sorry, this is very complicated code; I will have to find another
> time to review it.  But I wanted to thank you for fixing the bug.

Thanks, glad to see you've put it on your (probably very long)
'list of patches that I need to review'...
 
 Can I do anything else to ease your pain?


Pierre Muller
Pascal language support maintainer for GDB

Re: Spurius results for cygwin selftest.exp(was Re: [RFC] fix annoying completion bug on directories)

[Christopher Faylor]

On Wed, Jul 09, 2008 at 12:16:04AM +0200, Pierre Muller wrote:
>> >   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.
>> 
>> In these cases, in addition to analising the log file, please
>> rerun the test to confirm it was spurious.  Cygwin testing
>> is indeed flackier than linux.
>
>  This is indeed a spurious FAIL that has to do with that
>third thread that start on cygwin right after the command prompt is
>printed out.
>  Does anyone know why there is a third thread starting?
>Debugging threads under cygwin is quite difficult, you never really know
>where they are coming from...

Cygwin starts a signal thread during process creation and, if you are
running with CYGWIN=tty will start a couple more threads to deal with
tty I/O.  If the process is using select() or poll() it will start
threads for that too.  It should not start extra threads when using ptys
however.

The test suite isn't setting CYGWIN=tty somewhere is it?  It used to be
necessary that you set CYGWIN=tty in order to use ptys and sometimes
people still insist that it is required.  If is is set somewhere that
could account for extra, unneeded, threads.

Cygwin 1.5.x it may also start a thread-per-fd to deal with pipe I/O.
I've removed that annoyance from the not-yet-released 1.7.x version.  (I
introduced another pipe problem in the process of doing that but hey,
that's software)

Does debugging the process cause an extra thread to be created by
Windows, too?  I don't remember.  I know it creates a thread when you
attach to a process but I don't know if it does just in a standard
debug when a process started via CreateProcess.

cgf

Re: Spurius results for cygwin selftest.exp(was Re: [RFC] fix annoying completion bug on directories)

[Daniel Jacobowitz]

On Thu, Jul 10, 2008 at 05:17:46PM -0400, Christopher Faylor wrote:
> Does debugging the process cause an extra thread to be created by
> Windows, too?  I don't remember.  I know it creates a thread when you
> attach to a process but I don't know if it does just in a standard
> debug when a process started via CreateProcess.

I know one is created when you Control-C, but I'm not sure about when
you hit a breakpoint.

-- 
Daniel Jacobowitz
CodeSourcery

Re: Spurius results for cygwin selftest.exp(was Re: [RFC] fix annoying completion bug on directories)

[Pedro Alves]

A Thursday 10 July 2008 22:47:38, Daniel Jacobowitz wrote:
> On Thu, Jul 10, 2008 at 05:17:46PM -0400, Christopher Faylor wrote:
> > Does debugging the process cause an extra thread to be created by
> > Windows, too?  I don't remember.  I know it creates a thread when you
> > attach to a process but I don't know if it does just in a standard
> > debug when a process started via CreateProcess.
>
> I know one is created when you Control-C, but I'm not sure about when
> you hit a breakpoint.

None is created you hit hit a breakpoint, but one is created with
DebugBreakProcess -- this injects a thread in the debuggee whose sole
job is to hit a breakpoint.

None extra is created when you debug a process started via CreateProcess.

Attaching does create a new thread, and you can actually create a
bunch of them by detaching/attaching from the same process several
times.

-- 
Pedro Alves

[RFA-updated] fix annoying completion bug on directories

[Pierre Muller]

 My patch conflicts with the last commit made
by  Tom Tromey yesterday to fix 'help' command completion.

  Here is an updated patch that should cleanly apply to 
current cvs HEAD.

   Checked on cygwin, no testsuite changes.


gdb ChangeLog entry:

2008-07-12  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 type.
	(complete_line_internal): Change last arg to 
	complete_line_internal_reason type.
	Adapt code to that change.
	(complete_line, command_completer): Adapt to
	complete_line_internal change.
	(gdb_completion_word_break): New static function.
	Calls complete_line_internal with reason=handle_brkchars.
	* top.c (init_main): Set  rl_completion_word_break_hook to
	gdb_completion_word_break_characters;
	* Makefile (completer.o): Add gdb_assert_h dependency.	
	

Index: gdb/Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/Makefile.in,v
retrieving revision 1.1035
diff -u -p -r1.1035 Makefile.in
--- gdb/Makefile.in	11 Jul 2008 11:07:38 -0000	1.1035
+++ gdb/Makefile.in	12 Jul 2008 05:46:22 -0000
@@ -2009,7 +2009,7 @@ complaints.o: complaints.c $(defs_h) $(c
 	$(command_h) $(gdbcmd_h)
 completer.o: completer.c $(defs_h) $(symtab_h) $(gdbtypes_h)
$(expression_h) \
 	$(filenames_h) $(language_h) $(cli_decode_h) $(gdbcmd_h) \
-	$(readline_h) $(completer_h)
+	$(readline_h) $(completer_h) $(gdb_assert_h)
 copying.o: copying.c $(defs_h) $(command_h) $(gdbcmd_h)
 corefile.o: corefile.c $(defs_h) $(gdb_string_h) $(inferior_h) $(symtab_h)
\
 	$(command_h) $(gdbcmd_h) $(bfd_h) $(target_h) $(gdbcore_h) \
Index: gdb/completer.h
===================================================================
RCS file: /cvs/src/src/gdb/completer.h,v
retrieving revision 1.14
diff -u -p -r1.14 completer.h
--- gdb/completer.h	6 Jun 2008 20:58:08 -0000	1.14
+++ gdb/completer.h	12 Jul 2008 05:46:22 -0000
@@ -33,6 +33,8 @@ extern char **command_completer (char *,
 
 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: gdb/completer.c
===================================================================
RCS file: /cvs/src/src/gdb/completer.c,v
retrieving revision 1.27
diff -u -p -r1.27 completer.c
--- gdb/completer.c	11 Jul 2008 15:07:52 -0000	1.27
+++ gdb/completer.c	12 Jul 2008 05:46:23 -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"
 
@@ -451,24 +452,47 @@ expression_completer (char *text, char *
    "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;
@@ -482,7 +506,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();
 
@@ -545,12 +568,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.  */
@@ -574,18 +599,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;
 		}
@@ -621,7 +648,8 @@ complete_line_internal (const char *text
 			   p--)
 			;
 		    }
-		  list = (*c->completer) (p, word);
+		  if (reason != handle_brkchars)
+		    list = (*c->completer) (p, word);
 		}
 	    }
 	  else
@@ -642,7 +670,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.  */
@@ -650,7 +679,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
 	{
@@ -664,7 +693,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
 	    {
@@ -689,7 +719,8 @@ complete_line_internal (const char *text
 		       p--)
 		    ;
 		}
-	      list = (*c->completer) (p, word);
+	      if (reason != handle_brkchars)
+		list = (*c->completer) (p, word);
 	    }
 	}
     }
@@ -697,21 +728,43 @@ complete_line_internal (const char *text
   return list;
 }
 
-/* Like complete_line_internal, but always passes 0 for FOR_HELP.  */
+/* 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.  
+
+   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 (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: gdb/top.c
===================================================================
RCS file: /cvs/src/src/gdb/top.c,v
retrieving revision 1.146
diff -u -p -r1.146 top.c
--- gdb/top.c	11 Jul 2008 11:07:39 -0000	1.146
+++ gdb/top.c	12 Jul 2008 05:46:23 -0000
@@ -1531,6 +1531,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 ();

Re: [RFA] fix annoying completion bug on directories

[Eli Zaretskii]

> From: "Pierre Muller" <muller@ics.u-strasbg.fr>
> Date: Wed, 9 Jul 2008 00:16:04 +0200
> Content-Language: en-us
> 
>   Does it also deserve a NEWS entry?

No, I don't think so.