This is the mail archive of the gdb-patches@sources.redhat.com 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: Patch: complete -vs- duplicates, take 2


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  <tromey@redhat.com>
> 
>         * 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


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