From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 30281 invoked by alias); 25 Jan 2002 15:40:21 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 30132 invoked from network); 25 Jan 2002 15:40:05 -0000 Received: from unknown (HELO cygnus.com) (205.180.230.5) by sources.redhat.com with SMTP; 25 Jan 2002 15:40:05 -0000 Received: from redhat.com (rtl.sfbay.redhat.com [205.180.230.21]) by runyon.cygnus.com (8.8.7-cygnus/8.8.7) with ESMTP id HAA19209; Fri, 25 Jan 2002 07:39:57 -0800 (PST) Message-ID: <3C517C33.13B8A9D4@redhat.com> Date: Fri, 25 Jan 2002 07:40:00 -0000 From: Fernando Nasser Organization: Red Hat Canada X-Mailer: Mozilla 4.78 [en] (X11; U; Linux 2.4.9-13 i686) X-Accept-Language: en MIME-Version: 1.0 To: tromey@redhat.com CC: gdb-patches@sources.redhat.com, Eli Zaretskii Subject: Re: Patch: complete -vs- duplicates, take 2 References: <87666g1nws.fsf@creche.redhat.com> Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit X-SW-Source: 2002-01/txt/msg00736.txt.bz2 Sorry for the delay Tom. It looks OK for me, but I would like Eli's opinion as he is the one who has spent more time fixing the completer lately. Eli? Regards to all, Fernando Tom Tromey wrote: > > Here's try #2 at making the `complete' command remove duplicates. > This time completion via readline doesn't have duplicate removal done > twice. > > What I did is move the guts of line_completion_function into a new > function. Then I changed complete_command to use the new function and > do duplicate removal as it prints the items. > > Ok? > > Tom > > Index: ChangeLog > from Tom Tromey > > * cli/cli-cmds.c (compare_strings): New function. > (complete_command): Only print each unique item once. > * completer.h (complete_line): Declare. > * completer.c (complete_line): New function. > (line_completion_function): Use it. > > Index: completer.c > =================================================================== > RCS file: /cvs/src/src/gdb/completer.c,v > retrieving revision 1.8 > diff -u -r1.8 completer.c > --- completer.c 2001/07/15 18:57:06 1.8 > +++ completer.c 2002/01/05 21:20:50 > @@ -361,246 +361,150 @@ > "file ../gdb.stabs/we" "ird" (needs to not break word at slash) > */ > > -/* 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 > - command's completer function, the stuff specific to symbol completion > - is in make_symbol_completion_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. > > - MATCHES is the number of matches that have currently been collected from > - calling this completion function. When zero, then we need to initialize, > - otherwise the initialization has already taken place and we can just > - return the next potential completion string. > - > 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. > - > - Returns NULL if there are no more completions, else a pointer to a string > - which is a possible completion, it is the caller's responsibility to > - free the string. */ > + should pretend that the line ends at POINT. */ > > -char * > -line_completion_function (char *text, int matches, char *line_buffer, int point) > +char ** > +complete_line (char *text, char *line_buffer, int point) > { > - static char **list = (char **) NULL; /* Cache of completions */ > - static int index; /* Next cached completion */ > - char *output = NULL; > + 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 (matches == 0) > - { > - /* The caller is beginning to accumulate a new set of completions, so > - we need to find all of them now, and cache them for returning one at > - a time on future calls. */ > - > - if (list) > - { > - /* Free the storage used by LIST, but not by the strings inside. > - This is because rl_complete_internal () frees the strings. */ > - xfree (list); > - } > - list = 0; > - index = 0; > + /* 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. */ > > - /* 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 = > + gdb_completer_word_break_characters; > > - rl_completer_word_break_characters = > - gdb_completer_word_break_characters; > - > /* Decide whether to complete on a list of gdb commands or on symbols. */ > - tmp_command = (char *) alloca (point + 1); > - p = tmp_command; > + tmp_command = (char *) alloca (point + 1); > + p = tmp_command; > > - strncpy (tmp_command, line_buffer, point); > - tmp_command[point] = '\0'; > - /* Since text always contains some number of characters leading up > - to point, we can find the equivalent position in tmp_command > - by subtracting that many characters from the end of tmp_command. */ > - word = tmp_command + point - strlen (text); > + strncpy (tmp_command, line_buffer, point); > + tmp_command[point] = '\0'; > + /* Since text always contains some number of characters leading up > + to point, we can find the equivalent position in tmp_command > + by subtracting that many characters from the end of tmp_command. */ > + word = tmp_command + point - strlen (text); > > - if (point == 0) > - { > - /* An empty line we want to consider ambiguous; that is, it > - could be any command. */ > - c = (struct cmd_list_element *) -1; > - result_list = 0; > - } > - else > - { > - c = lookup_cmd_1 (&p, cmdlist, &result_list, 1); > - } > + if (point == 0) > + { > + /* An empty line we want to consider ambiguous; that is, it > + could be any command. */ > + c = (struct cmd_list_element *) -1; > + result_list = 0; > + } > + else > + { > + c = lookup_cmd_1 (&p, cmdlist, &result_list, 1); > + } > > - /* Move p up to the next interesting thing. */ > - while (*p == ' ' || *p == '\t') > - { > - p++; > - } > + /* Move p up to the next interesting thing. */ > + while (*p == ' ' || *p == '\t') > + { > + p++; > + } > + > + if (!c) > + { > + /* It is an unrecognized command. So there are no > + possible completions. */ > + list = NULL; > + } > + else if (c == (struct cmd_list_element *) -1) > + { > + char *q; > > - if (!c) > + /* lookup_cmd_1 advances p up to the first ambiguous thing, but > + doesn't advance over that thing itself. Do so now. */ > + q = p; > + while (*q && (isalnum (*q) || *q == '-' || *q == '_')) > + ++q; > + if (q != tmp_command + point) > { > - /* It is an unrecognized command. So there are no > - possible completions. */ > + /* There is something beyond the ambiguous > + command, so there are no possible completions. For > + example, "info t " or "info t foo" does not complete > + to anything, because "info t" can be "info target" or > + "info terminal". */ > list = NULL; > } > - else if (c == (struct cmd_list_element *) -1) > + else > { > - char *q; > - > - /* lookup_cmd_1 advances p up to the first ambiguous thing, but > - doesn't advance over that thing itself. Do so now. */ > - q = p; > - while (*q && (isalnum (*q) || *q == '-' || *q == '_')) > - ++q; > - if (q != tmp_command + point) > + /* We're trying to complete on the command which was ambiguous. > + This we can deal with. */ > + if (result_list) > { > - /* There is something beyond the ambiguous > - command, so there are no possible completions. For > - example, "info t " or "info t foo" does not complete > - to anything, because "info t" can be "info target" or > - "info terminal". */ > - list = NULL; > + list = complete_on_cmdlist (*result_list->prefixlist, p, > + word); > } > else > { > - /* We're trying to complete on the command which was ambiguous. > - This we can deal with. */ > - if (result_list) > - { > - list = complete_on_cmdlist (*result_list->prefixlist, p, > - word); > - } > - else > - { > - list = complete_on_cmdlist (cmdlist, p, word); > - } > - /* Insure that readline does the right thing with respect to > - inserting quotes. */ > - rl_completer_word_break_characters = > - gdb_completer_command_word_break_characters; > + list = complete_on_cmdlist (cmdlist, p, word); > } > + /* Insure that readline does the right thing with respect to > + inserting quotes. */ > + rl_completer_word_break_characters = > + gdb_completer_command_word_break_characters; > } > - else > + } > + else > + { > + /* We've recognized a full command. */ > + > + if (p == tmp_command + point) > { > - /* We've recognized a full command. */ > + /* There is no non-whitespace in the line beyond the command. */ > > - if (p == tmp_command + point) > + if (p[-1] == ' ' || p[-1] == '\t') > { > - /* There is no non-whitespace in the line beyond the command. */ > - > - if (p[-1] == ' ' || p[-1] == '\t') > - { > - /* The command is followed by whitespace; we need to complete > - on whatever comes after command. */ > - 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); > - > - /* Insure that readline does the right thing > - with respect to inserting quotes. */ > - rl_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 = > - gdb_completer_command_word_break_characters; > - } > - else > - { > - /* It is a normal command; what comes after it is > - completed by the command's completer function. */ > - if (c->completer == filename_completer) > - { > - /* Many commands which want to complete on > - file names accept several file names, as > - in "run foo bar >>baz". So we don't want > - to complete the entire text after the > - command, just the last word. To this > - end, we need to find the beginning of the > - file name by starting at `word' and going > - backwards. */ > - for (p = word; > - p > tmp_command > - && strchr (gdb_completer_file_name_break_characters, p[-1]) == NULL; > - p--) > - ; > - rl_completer_word_break_characters = > - gdb_completer_file_name_break_characters; > - } > - else if (c->completer == location_completer) > - { > - /* Commands which complete on locations want to > - see the entire argument. */ > - for (p = word; > - p > tmp_command > - && p[-1] != ' ' && p[-1] != '\t'; > - p--) > - ; > - } > - list = (*c->completer) (p, word); > - } > - } > - else > + /* The command is followed by whitespace; we need to complete > + on whatever comes after command. */ > + if (c->prefixlist) > { > - /* The command is not followed by whitespace; we need to > - complete on the command itself. e.g. "p" which is a > - command itself but also can complete to "print", "ptype" > - etc. */ > - char *q; > - > - /* Find the command we are completing on. */ > - q = p; > - while (q > tmp_command) > - { > - if (isalnum (q[-1]) || q[-1] == '-' || q[-1] == '_') > - --q; > - else > - break; > - } > - > - list = complete_on_cmdlist (result_list, q, word); > + /* It is a prefix command; what comes after it is > + a subcommand (e.g. "info "). */ > + list = complete_on_cmdlist (*c->prefixlist, p, word); > > /* Insure that readline does the right thing > - with respect to inserting quotes. */ > + with respect to inserting quotes. */ > rl_completer_word_break_characters = > gdb_completer_command_word_break_characters; > } > - } > - else > - { > - /* There is non-whitespace beyond the command. */ > - > - if (c->prefixlist && !c->allow_unknown) > - { > - /* It is an unrecognized subcommand of a prefix command, > - e.g. "info adsfkdj". */ > - list = NULL; > - } > else if (c->enums) > { > list = complete_on_enum (c->enums, p, word); > + rl_completer_word_break_characters = > + gdb_completer_command_word_break_characters; > } > else > { > - /* It is a normal command. */ > + /* It is a normal command; what comes after it is > + completed by the command's completer function. */ > if (c->completer == filename_completer) > { > - /* See the commentary above about the specifics > - of file-name completion. */ > + /* Many commands which want to complete on > + file names accept several file names, as > + in "run foo bar >>baz". So we don't want > + to complete the entire text after the > + command, just the last word. To this > + end, we need to find the beginning of the > + file name by starting at `word' and going > + backwards. */ > for (p = word; > p > tmp_command > && strchr (gdb_completer_file_name_break_characters, p[-1]) == NULL; > @@ -611,6 +515,8 @@ > } > else if (c->completer == location_completer) > { > + /* Commands which complete on locations want to > + see the entire argument. */ > for (p = word; > p > tmp_command > && p[-1] != ' ' && p[-1] != '\t'; > @@ -620,7 +526,119 @@ > list = (*c->completer) (p, word); > } > } > + else > + { > + /* The command is not followed by whitespace; we need to > + complete on the command itself. e.g. "p" which is a > + command itself but also can complete to "print", "ptype" > + etc. */ > + char *q; > + > + /* Find the command we are completing on. */ > + q = p; > + while (q > tmp_command) > + { > + if (isalnum (q[-1]) || q[-1] == '-' || q[-1] == '_') > + --q; > + else > + break; > + } > + > + list = complete_on_cmdlist (result_list, q, word); > + > + /* Insure that readline does the right thing > + with respect to inserting quotes. */ > + rl_completer_word_break_characters = > + gdb_completer_command_word_break_characters; > + } > + } > + else > + { > + /* There is non-whitespace beyond the command. */ > + > + if (c->prefixlist && !c->allow_unknown) > + { > + /* It is an unrecognized subcommand of a prefix command, > + e.g. "info adsfkdj". */ > + list = NULL; > + } > + else if (c->enums) > + { > + list = complete_on_enum (c->enums, p, word); > + } > + else > + { > + /* It is a normal command. */ > + if (c->completer == filename_completer) > + { > + /* See the commentary above about the specifics > + of file-name completion. */ > + for (p = word; > + p > tmp_command > + && strchr (gdb_completer_file_name_break_characters, p[-1]) == NULL; > + p--) > + ; > + rl_completer_word_break_characters = > + gdb_completer_file_name_break_characters; > + } > + else if (c->completer == location_completer) > + { > + for (p = word; > + p > tmp_command > + && p[-1] != ' ' && p[-1] != '\t'; > + p--) > + ; > + } > + list = (*c->completer) (p, word); > + } > + } > + } > + > + return list; > +} > + > +/* 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 > + command's completer function, the stuff specific to symbol completion > + is in make_symbol_completion_list. > + > + TEXT is the caller's idea of the "word" we are looking at. > + > + MATCHES is the number of matches that have currently been collected from > + calling this completion function. When zero, then we need to initialize, > + otherwise the initialization has already taken place and we can just > + return the next potential completion string. > + > + 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. > + > + Returns NULL if there are no more completions, else a pointer to a string > + which is a possible completion, it is the caller's responsibility to > + free the string. */ > + > +char * > +line_completion_function (char *text, int matches, char *line_buffer, int point) > +{ > + static char **list = (char **) NULL; /* Cache of completions */ > + static int index; /* Next cached completion */ > + char *output = NULL; > + > + if (matches == 0) > + { > + /* The caller is beginning to accumulate a new set of completions, so > + we need to find all of them now, and cache them for returning one at > + a time on future calls. */ > + > + if (list) > + { > + /* Free the storage used by LIST, but not by the strings inside. > + This is because rl_complete_internal () frees the strings. */ > + xfree (list); > } > + index = 0; > + list = complete_line (text, line_buffer, point); > } > > /* If we found a list of potential completions during initialization then > Index: completer.h > =================================================================== > RCS file: /cvs/src/src/gdb/completer.h,v > retrieving revision 1.4 > diff -u -r1.4 completer.h > --- completer.h 2001/07/15 18:57:06 1.4 > +++ completer.h 2002/01/05 21:20:50 > @@ -19,6 +19,8 @@ > #if !defined (COMPLETER_H) > #define COMPLETER_H 1 > > +extern char **complete_line (char *text, char *line_buffer, int point); > + > extern char *line_completion_function (char *, int, char *, int); > > extern char *readline_line_completion_function (char *text, int matches); > Index: cli/cli-cmds.c > =================================================================== > RCS file: /cvs/src/src/gdb/cli/cli-cmds.c,v > retrieving revision 1.10 > diff -u -r1.10 cli-cmds.c > --- cli/cli-cmds.c 2001/09/01 21:38:05 1.10 > +++ cli/cli-cmds.c 2002/01/05 21:20:52 > @@ -202,6 +202,15 @@ > help_cmd (command, gdb_stdout); > } > > +/* String compare function for qsort. */ > +static int > +compare_strings (const void *arg1, const void *arg2) > +{ > + const char **s1 = (const char **) arg1; > + const char **s2 = (const char **) arg2; > + return strcmp (*s1, *s2); > +} > + > /* The "complete" command is used by Emacs to implement completion. */ > > /* ARGSUSED */ > @@ -210,7 +219,7 @@ > { > int i; > int argpoint; > - char *completion; > + char **completions; > > dont_repeat (); > > @@ -218,12 +227,36 @@ > arg = ""; > argpoint = strlen (arg); > > - for (completion = line_completion_function (arg, i = 0, arg, argpoint); > - completion; > - completion = line_completion_function (arg, ++i, arg, argpoint)) > + completions = complete_line (arg, arg, argpoint); > + > + if (completions) > { > - printf_unfiltered ("%s\n", completion); > - xfree (completion); > + int item, size; > + > + for (size = 0; completions[size]; ++size) > + ; > + qsort (completions, size, sizeof (char *), compare_strings); > + > + /* We do extra processing here since we only want to print each > + unique item once. */ > + item = 0; > + while (item < size) > + { > + int next_item; > + printf_unfiltered ("%s\n", completions[item]); > + next_item = item + 1; > + while (next_item < size > + && ! strcmp (completions[item], completions[next_item])) > + { > + xfree (completions[next_item]); > + ++next_item; > + } > + > + xfree (completions[item]); > + item = next_item; > + } > + > + xfree (completions); > } > } > -- Fernando Nasser Red Hat Canada Ltd. E-Mail: fnasser@redhat.com 2323 Yonge Street, Suite #300 Toronto, Ontario M4P 2C9