public inbox for gdb-cvs@sourceware.org
help / color / mirror / Atom feed
* [binutils-gdb] gdb: remove special case completion word handling for filenames
@ 2024-03-25 18:29 Andrew Burgess
  0 siblings, 0 replies; only message in thread
From: Andrew Burgess @ 2024-03-25 18:29 UTC (permalink / raw)
  To: gdb-cvs

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=c66e8e5c8d2a0bf68d9af65f3e4823eb01862f56

commit c66e8e5c8d2a0bf68d9af65f3e4823eb01862f56
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Mon Jan 15 13:44:34 2024 +0000

    gdb: remove special case completion word handling for filenames
    
    This commit removes some code which is special casing the filename
    completion logic.  The code in question relates to finding the
    beginning of the completion word and was first introduced, or modified
    into its existing form in commit 7830cf6fb9571c3357b1a0 (from 2001).
    
    The code being removed moved the start of the completion word backward
    until a character in gdb_completer_file_name_break_characters was
    found, or until we reached the end of the actual command.
    
    However, I doubt that this is needed any more.  The filename completer
    has a corresponding filename_completer_handle_brkchars function which
    provides gdb_completer_file_name_break_characters as the word break
    characters to readline, and also sets rl_completer_quote_characters.
    As such, I would expect readline to be able to correctly find the
    start of the completion word.
    
    There is one change which I've needed to make as a consequence of
    removing the above code, and I think this is a bug fix.
    
    In complete_line_internal_normal_command we initialised temporary
    variable P to the CMD_ARGS; this is the complete text after the
    command name.  Meanwhile, complete_line_internal_normal_command also
    accepts an argument WORD, which is the completion word that readline
    found for us.
    
    In the code I removed P was updated, it was first set to WORD, and
    then moved backwards to the "new" start of the completion word.
    
    But notice, the default for P is the complete command argument text,
    and only if we are performing filename completion do we modify P to be
    the completion word.
    
    We then passed P through to the actual commands completion function.
    
    If we are doing anything other than filename completion then the value
    of P passed is the complete argument text.
    
    If we are doing filename completion then the value of P passed is the
    completion word.
    
    In filename_completer we get two arguments TEXT and WORD, the TEXT
    argument is the value of P which is the "new" completion word, while
    WORD is the completion word that readline calculated.
    
    After simplifying complete_line_internal_normal_command, and the
    temporary P is removed, we always pass the complete argument text into
    TEXT, while WORD remains the completion word that readline found.
    
    Previously in filename_completer we actually tried to generate
    completions based on TEXT, which worked fine as TEXT actually
    contained the completion word that we found in
    complete_line_internal_normal_command.  But I believe that we should
    be fine to use the completion word that readline found, so I have
    updated filename_completer to generate completions based on WORD.
    
    If I'm correct, then I don't expect to see any user visible changes
    after this commit.

Diff:
---
 gdb/completer.c | 25 ++++---------------------
 1 file changed, 4 insertions(+), 21 deletions(-)

diff --git a/gdb/completer.c b/gdb/completer.c
index 330c39598c5..04354120621 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -210,7 +210,7 @@ filename_completer (struct cmd_list_element *ignore,
   while (1)
     {
       gdb::unique_xmalloc_ptr<char> p_rl
-	(rl_filename_completion_function (text, subsequent_name));
+	(rl_filename_completion_function (word, subsequent_name));
       if (p_rl == NULL)
 	break;
       /* We need to set subsequent_name to a non-zero value before the
@@ -239,7 +239,7 @@ filename_completer (struct cmd_list_element *ignore,
 	}
 
       tracker.add_completion
-	(make_completion_match_str (std::move (p_rl), text, word));
+	(make_completion_match_str (std::move (p_rl), word, word));
     }
 }
 
@@ -1193,23 +1193,6 @@ complete_line_internal_normal_command (completion_tracker &tracker,
 				       complete_line_internal_reason reason,
 				       struct cmd_list_element *c)
 {
-  const char *p = cmd_args;
-
-  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 > command
-	     && strchr (gdb_completer_file_name_break_characters,
-			p[-1]) == NULL;
-	   p--)
-	;
-    }
-
   if (reason == handle_brkchars)
     {
       completer_handle_brkchars_ftype *brkchars_fn;
@@ -1223,11 +1206,11 @@ complete_line_internal_normal_command (completion_tracker &tracker,
 	       (c->completer));
 	}
 
-      brkchars_fn (c, tracker, p, word);
+      brkchars_fn (c, tracker, cmd_args, word);
     }
 
   if (reason != handle_brkchars && c->completer != NULL)
-    (*c->completer) (c, tracker, p, word);
+    (*c->completer) (c, tracker, cmd_args, word);
 }
 
 /* Internal function used to handle completions.

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2024-03-25 18:29 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-25 18:29 [binutils-gdb] gdb: remove special case completion word handling for filenames Andrew Burgess

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).