From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8692 invoked by alias); 5 Jul 2008 23:37:54 -0000 Received: (qmail 8682 invoked by uid 22791); 5 Jul 2008 23:37:52 -0000 X-Spam-Check-By: sourceware.org Received: from mailhost.u-strasbg.fr (HELO mailhost.u-strasbg.fr) (130.79.200.159) by sourceware.org (qpsmtpd/0.31) with ESMTP; Sat, 05 Jul 2008 23:37:18 +0000 Received: from baal.u-strasbg.fr (baal.u-strasbg.fr [IPv6:2001:660:2402::41]) by mailhost.u-strasbg.fr (8.14.2/jtpda-5.5pre1) with ESMTP id m65NbErp089311 for ; Sun, 6 Jul 2008 01:37:14 +0200 (CEST) Received: from mailserver.u-strasbg.fr (ms1.u-strasbg.fr [IPv6:2001:660:2402::141]) by baal.u-strasbg.fr (8.14.0/jtpda-5.5pre1) with ESMTP id m65NbEEV088279 for ; Sun, 6 Jul 2008 01:37:14 +0200 (CEST) Received: from d620muller ([130.79.244.152]) by mailserver.u-strasbg.fr (8.13.8/jtpda-5.5pre1) with ESMTP id m65NbAGb020252 for ; Sun, 6 Jul 2008 01:37:14 +0200 (CEST) From: "Pierre Muller" To: Subject: [RFC] fix annoying completion bug on directories Date: Sat, 05 Jul 2008 23:37:00 -0000 Message-ID: <000901c8def8$137b1bf0$3a7153d0$@u-strasbg.fr> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit X-Mailer: Microsoft Office Outlook 12.0 Content-Language: en-us X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.0 (mailhost.u-strasbg.fr [IPv6:2001:660:2402::159]); Sun, 06 Jul 2008 01:37:14 +0200 (CEST) X-Virus-Status: Clean Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2008-07/txt/msg00072.txt.bz2 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 * 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; +}