From: Andrew Burgess Date: Tue, 2 Jan 2024 17:08:30 +0000 (+0000) Subject: gdb: add trailing '/' when using 'complete' with directory names X-Git-Tag: upstream/2.42~111 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=3b9ff5d9009d587aa76ec7bbaab4439e26a52c50;p=platform%2Fupstream%2Fbinutils.git gdb: add trailing '/' when using 'complete' with directory names This patch contains work pulled from this previously proposed patch: https://inbox.sourceware.org/gdb-patches/20210213220752.32581-2-lsix@lancelotsix.com/ But has been modified by me. Credit for the original idea and implementation goes to Lancelot, any bugs in this new iteration belong to me. Consider the executable `/tmp/foo/my_exec', and if we assume `/tmp' is empty other than the `foo' sub-directory, then currently within GDB, if I type: (gdb) file /tmp/f and then hit TAB, GDB completes this to: (gdb) file /tmp/foo/ notice that not only did GDB fill in the whole of `foo', but GDB also added a trailing '/' character. This is done within readline when the path that was just completed is a directory. However, if I instead do: (gdb) complete file /tmp/f file /tmp/foo I now see the completed directory name, but the trailing '/' is missing. The reason is that, in this case, the completions are not offered via readline, but are handled entirely within GDB, and so readline never gets the chance to add the trailing '/' character. The above patch added filename option support to GDB, which included completion of the filename options. This initially suffered from the same problem that I've outlined above, but the above patch proposed a solution to this problem, but this solution only applied to filename options (which have still not been added to GDB), and was mixed in with the complete filename options support. This patch pulls out just the fix for the trailing "/" problem, and applies it to GDB's general filename completion. This patch does not add filename options to GDB, that can always be done later, but I think this small part is itself a useful fix. One of the biggest changes I made in this version is that I got rid of the set_from_readline member function, instead, I now pass the value of m_from_readline into the completion_tracker constructor. I then moved the addition of the trailing '/' into filename_completer so that it is applied in the general filename completion case. I also added a call to gdb_tilde_expand which was missing from the original patch, I haven't tested, but I suspect that this meant that the original patch would not add the trailing '/' if the user entered a path starting with a tilde character. When writing the test for this patch I ran into two problems. The first was that the procedures in lib/completion-support.exp relied on the command being completed for the test name. This is fine for many commands, but not when completing a filename, if we use the command in this case the test name will (potentially) include the name of the directory in which the test is being run, which means we can't compare results between two runs of GDB from different directories. So in this commit I've gone through completion-support.exp and added a new (optional) testname argument to many of the procedures, this allows me to give a unique test name, that doesn't include the path for my new tests. The second issue was in the procedure make_tab_completion_list_re, this builds the completion list which is displayed after a double tab when there are multiple possible completions. The procedure added the regexp ' +' after each completion, and then added another ' +' at the very end of the expected output. So, if we expected to match the name of two functions 'f1' and 'f2' the generated regexp would be: 'f1 +f2 + +'. This would match just fine, the actual output would be: 'f1 f2 ', notice that we get two spaces after each function name. However, if we complete two directory names 'd1' and 'd2' then the output will be 'd1/ d2/ '. Notice that now we only have a single space between each match, however, we do get the '/' added instead. What happens is that when presenting the matches, readline always adds the appropriate trailing character; if we performed tab completion of 'break f1' then, as 'f1' is a unique match, we'd get 'break f1 ' with a trailing space added. However, if we complete 'file d1' then we get 'file d1/'. Then readline is adding a single space after each possible match, including the last one, which accounts for the trailing space character. To resolve this I've simply remove the addition o the second ' +' within make_tab_completion_list_re, for the function completion example I gave above the expected pattern is now 'f1 +f2 +', which for the directory case we expect 'd1/ +d2/ +', both of which work just fine. Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=16265 Co-Authored-By: Lancelot SIX Approved-By: Tom Tromey Reviewed-By: John Baldwin --- diff --git a/gdb/completer.c b/gdb/completer.c index d69ddcc..8e2f031 100644 --- a/gdb/completer.c +++ b/gdb/completer.c @@ -31,6 +31,7 @@ #include #include "linespec.h" #include "cli/cli-decode.h" +#include "gdbsupport/gdb_tilde_expand.h" /* FIXME: This is needed because of lookup_cmd_1 (). We should be calling a hook instead so we eliminate the CLI dependency. */ @@ -225,6 +226,20 @@ filename_completer (struct cmd_list_element *ignore, if (p[strlen (p) - 1] == '~') continue; + /* Readline appends a trailing '/' if the completion is a + directory. If this completion request originated from outside + readline (e.g. GDB's 'complete' command), then we append the + trailing '/' ourselves now. */ + if (!tracker.from_readline ()) + { + std::string expanded = gdb_tilde_expand (p_rl.get ()); + struct stat finfo; + const bool isdir = (stat (expanded.c_str (), &finfo) == 0 + && S_ISDIR (finfo.st_mode)); + if (isdir) + p_rl.reset (concat (p_rl.get (), "/", nullptr)); + } + tracker.add_completion (make_completion_match_str (std::move (p_rl), text, word)); } @@ -1474,7 +1489,8 @@ int max_completions = 200; /* See completer.h. */ -completion_tracker::completion_tracker () +completion_tracker::completion_tracker (bool from_readline) + : m_from_readline (from_readline) { discard_completions (); } @@ -1671,8 +1687,8 @@ make_completion_match_str (gdb::unique_xmalloc_ptr &&match_name, completion_result complete (const char *line, char const **word, int *quote_char) { - completion_tracker tracker_handle_brkchars; - completion_tracker tracker_handle_completions; + completion_tracker tracker_handle_brkchars (false); + completion_tracker tracker_handle_completions (false); completion_tracker *tracker; /* The WORD should be set to the end of word to complete. We initialize @@ -1902,7 +1918,7 @@ gdb_completion_word_break_characters_throw () /* New completion starting. Get rid of the previous tracker and start afresh. */ delete current_completion.tracker; - current_completion.tracker = new completion_tracker (); + current_completion.tracker = new completion_tracker (true); completion_tracker &tracker = *current_completion.tracker; @@ -2308,7 +2324,7 @@ gdb_rl_attempted_completion_function_throw (const char *text, int start, int end if (end == 0 || !current_completion.tracker->use_custom_word_point ()) { delete current_completion.tracker; - current_completion.tracker = new completion_tracker (); + current_completion.tracker = new completion_tracker (true); complete_line (*current_completion.tracker, text, rl_line_buffer, rl_point); diff --git a/gdb/completer.h b/gdb/completer.h index d96c4b5..4faef4e 100644 --- a/gdb/completer.h +++ b/gdb/completer.h @@ -328,7 +328,7 @@ public: class completion_tracker { public: - completion_tracker (); + explicit completion_tracker (bool from_readline); ~completion_tracker (); DISABLE_COPY_AND_ASSIGN (completion_tracker); @@ -423,6 +423,11 @@ public: completion_result build_completion_result (const char *text, int start, int end); + /* Tells if the completion task is triggered by readline. See + m_from_readline. */ + bool from_readline () const + { return m_from_readline; } + private: /* The type that we place into the m_entries_hash hash table. */ @@ -512,6 +517,11 @@ private: track the maximum possible size of the lowest common denominator, which we know as each completion is added. */ size_t m_lowest_common_denominator_max_length = 0; + + /* Indicates that the completions are to be displayed by readline + interactively. The 'complete' command is a way to generate completions + not to be displayed by readline. */ + bool m_from_readline; }; /* Return a string to hand off to readline as a completion match diff --git a/gdb/linespec.c b/gdb/linespec.c index 76a93d7..7be65c8 100644 --- a/gdb/linespec.c +++ b/gdb/linespec.c @@ -1784,7 +1784,7 @@ linespec_parse_basic (linespec_parser *parser) if (!parser->completion_quote_char && strcmp (PARSER_STREAM (parser), ":") == 0) { - completion_tracker tmp_tracker; + completion_tracker tmp_tracker (false); const char *source_filename = PARSER_EXPLICIT (parser)->source_filename.get (); symbol_name_match_type match_type diff --git a/gdb/testsuite/gdb.base/filename-completion.exp b/gdb/testsuite/gdb.base/filename-completion.exp new file mode 100644 index 0000000..20ca227 --- /dev/null +++ b/gdb/testsuite/gdb.base/filename-completion.exp @@ -0,0 +1,94 @@ +# Copyright 2024 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +# Tests for filename completion. Create a directory tree on the test +# machine and try completing filenames within the tree. + +load_lib completion-support.exp + +# Setup a directory tree in which completion tests can be run. The +# structure is: +# +# root/ [ DIRECTORY ] +# aaa/ [ DIRECTORY ] +# bb1/ [ DIRECTORY ] +# bb2/ [ DIRECTORY ] +# cc1/ [ DIRECTORY ] +# cc2 [ FILE ] +proc setup_directory_tree {} { + set root [standard_output_file "root"] + + remote_exec host "mkdir -p ${root}" + remote_exec host "mkdir -p ${root}/aaa" + remote_exec host "mkdir -p ${root}/bb1" + remote_exec host "mkdir -p ${root}/bb2" + remote_exec host "mkdir -p ${root}/cc1" + remote_exec host "touch ${root}/cc2" + + return $root +} + +# Run filename completetion tests. ROOT is the base directory as +# returned from setup_directory_tree, though, if ROOT is a +# sub-directory of the user's home directory ROOT might have been +# modified to replace the $HOME prefix with a single "~" character. +proc run_tests { root } { + test_gdb_complete_none "file ${root}/xx" \ + "expand a non-existent filename" + + test_gdb_complete_unique "file ${root}/a" \ + "file ${root}/aaa/" "" false \ + "expand a unique filename" + + test_gdb_complete_multiple "file ${root}/" \ + "b" "b" { + "bb1/" + "bb2/" + } "" "" false \ + "expand multiple directory names" + + test_gdb_complete_multiple "file ${root}/" \ + "c" "c" { + "cc1/" + "cc2" + } "" "" false \ + "expand mixed directory and file names" +} + +gdb_start + +set root [setup_directory_tree] + +run_tests $root + +# This test relies on using the $HOME directory. We could make this +# work for remote hosts, but right now, this isn't supported. +if {![is_remote host]} { + + # The users home directory. + set home $::env(HOME) + + # Check if ROOT is within the $HOME directory. If it is then we can + # rerun the tests, but replacing the $HOME part with "~". + if { [string compare -length [string length $home] $root $home] == 0 } { + # Convert the $HOME prefix in to ~. + set tilde_root "~[string range $root [string length $home] end]" + + with_test_prefix "with tilde" { + # And rerun the tests. + run_tests $tilde_root + } + } +} diff --git a/gdb/testsuite/lib/completion-support.exp b/gdb/testsuite/lib/completion-support.exp index 3196b04..0103d6b 100644 --- a/gdb/testsuite/lib/completion-support.exp +++ b/gdb/testsuite/lib/completion-support.exp @@ -50,7 +50,6 @@ proc make_tab_completion_list_re { completion_list } { append completion_list_re [string_to_regexp $c] append completion_list_re $ws } - append completion_list_re $ws return $completion_list_re } @@ -85,21 +84,25 @@ proc clear_input_line { test } { # Test that completing LINE with TAB completes to nothing. -proc test_gdb_complete_tab_none { line } { +proc test_gdb_complete_tab_none { line { testname "" } } { set line_re [string_to_regexp $line] - set test "tab complete \"$line\"" + if { $testname eq "" } { + set testname "tab complete \"$line\"" + } else { + set testname "tab complete: $testname" + } send_gdb "$line\t" - gdb_test_multiple "" "$test" { + gdb_test_multiple "" "$testname" { -re "^$line_re$completion::bell_re$" { - pass "$test" + pass $gdb_test_name } -re "$line_re\[^ \]+ $" { - fail "$test" + fail $gdb_test_name } } - clear_input_line $test + clear_input_line $testname } # Test that completing INPUT_LINE with TAB completes to @@ -107,22 +110,26 @@ proc test_gdb_complete_tab_none { line } { # appended after EXPECTED_OUTPUT. Normally that's a whitespace, but # in some cases it's some other character, like a colon. -proc test_gdb_complete_tab_unique { input_line complete_line_re append_char_re } { +proc test_gdb_complete_tab_unique { input_line complete_line_re append_char_re {testname ""} } { - set test "tab complete \"$input_line\"" + if { $testname eq "" } { + set testname "tab complete \"$input_line\"" + } else { + set testname "tab complete: $testname" + } send_gdb "$input_line\t" set res 1 - gdb_test_multiple "" "$test" { + gdb_test_multiple "" "$testname" { -re "^$complete_line_re$append_char_re$" { - pass "$test" + pass $gdb_test_name } timeout { - fail "$test (timeout)" + fail "$gdb_test_name (timeout)" set res -1 } } - clear_input_line $test + clear_input_line $testname return $res } @@ -132,7 +139,8 @@ proc test_gdb_complete_tab_unique { input_line complete_line_re append_char_re } # to hit the max-completions limit. proc test_gdb_complete_tab_multiple { input_line add_completed_line \ - completion_list {max_completions 0}} { + completion_list {max_completions false} \ + {testname ""} } { global gdb_prompt set input_line_re [string_to_regexp $input_line] @@ -146,9 +154,13 @@ proc test_gdb_complete_tab_multiple { input_line add_completed_line \ "\\*\\*\\* List may be truncated, max-completions reached\\. \\*\\*\\*" } - set test "tab complete \"$input_line\"" + if { $testname eq "" } { + set testname "tab complete \"$input_line\"" + } else { + set testname "tab complete: $testname" + } send_gdb "$input_line\t" - gdb_test_multiple "" "$test (first tab)" { + gdb_test_multiple "" "$testname (first tab)" { -re "^${input_line_re}${completion::bell_re}$add_completed_line_re$" { send_gdb "\t" # If we auto-completed to an ambiguous prefix, we need an @@ -159,52 +171,61 @@ proc test_gdb_complete_tab_multiple { input_line add_completed_line \ } else { set maybe_bell "" } - gdb_test_multiple "" "$test (second tab)" { + gdb_test_multiple "" "$testname (second tab)" { -re "^${maybe_bell}\r\n$expected_re\r\n$gdb_prompt " { - gdb_test_multiple "" "$test (second tab)" { + gdb_test_multiple "" "$testname (second tab)" { -re "^$input_line_re$add_completed_line_re$" { - pass "$test" + pass $gdb_test_name } } } -re "${maybe_bell}\r\n.+\r\n$gdb_prompt $" { - fail "$test" + fail $gdb_test_name } } } } - clear_input_line $test + clear_input_line $testname } # Test that completing LINE with the complete command completes to # nothing. -proc test_gdb_complete_cmd_none { line } { - gdb_test_no_output "complete $line" "cmd complete \"$line\"" +proc test_gdb_complete_cmd_none { line { testname "" } } { + if { $testname eq "" } { + set testname "cmd complete \"$line\"" + } else { + set testname "cmd complete: $testname" + } + gdb_test_no_output "complete $line" $testname } # Test that completing LINE with the complete command completes to # COMPLETE_LINE_RE. # Returns 1 if the test passed, 0 if it failed, -1 if it timed out. -proc test_gdb_complete_cmd_unique { input_line complete_line_re } { +proc test_gdb_complete_cmd_unique { input_line complete_line_re {testname ""} } { global gdb_prompt set res 0 set cmd "complete $input_line" set cmd_re [string_to_regexp $cmd] - set test "cmd complete \"$input_line\"" - gdb_test_multiple $cmd $test { + if { $testname eq "" } { + set testname "cmd complete \"$input_line\"" + } else { + set testname "cmd complete: $testname" + } + gdb_test_multiple $cmd $testname { -re "^$cmd_re\r\n$complete_line_re\r\n$gdb_prompt $" { - pass $test + pass $gdb_test_name set res 1 } -re "$gdb_prompt $" { - fail "$test" + fail $gdb_test_name } timeout { - fail "$test (timeout)" + fail "$gdb_test_name (timeout)" set res -1 } } @@ -217,7 +238,7 @@ proc test_gdb_complete_cmd_unique { input_line complete_line_re } { # MAX_COMPLETIONS then we expect the completion to hit the # max-completions limit. -proc test_gdb_complete_cmd_multiple { cmd_prefix completion_word completion_list {start_quote_char ""} {end_quote_char ""} {max_completions 0}} { +proc test_gdb_complete_cmd_multiple { cmd_prefix completion_word completion_list {start_quote_char ""} {end_quote_char ""} {max_completions false} {testname ""} } { global gdb_prompt set expected_re [make_cmd_completion_list_re $cmd_prefix $completion_list $start_quote_char $end_quote_char] @@ -229,24 +250,28 @@ proc test_gdb_complete_cmd_multiple { cmd_prefix completion_word completion_list } set cmd_re [string_to_regexp "complete $cmd_prefix$completion_word"] - set test "cmd complete \"$cmd_prefix$completion_word\"" - gdb_test_multiple "complete $cmd_prefix$completion_word" $test { + if { $testname eq "" } { + set testname "cmd complete \"$cmd_prefix$completion_word\"" + } else { + set testname "cmd complete: $testname" + } + gdb_test_multiple "complete $cmd_prefix$completion_word" $testname { -re "^$cmd_re\r\n$expected_re$gdb_prompt $" { - pass $test + pass $gdb_test_name } -re "$gdb_prompt $" { - fail "$test" + fail $gdb_test_name } } } # Test that completing LINE completes to nothing. -proc test_gdb_complete_none { input_line } { +proc test_gdb_complete_none { input_line { testname "" } } { if { [readline_is_used] } { - test_gdb_complete_tab_none $input_line + test_gdb_complete_tab_none $input_line $testname } - test_gdb_complete_cmd_none $input_line + test_gdb_complete_cmd_none $input_line $testname } # Test that completing INPUT_LINE completes to COMPLETE_LINE_RE. @@ -265,7 +290,7 @@ proc test_gdb_complete_none { input_line } { # of a regular expression (as COMPLETE_LINE_RE). See # test_gdb_complete_unique below. -proc test_gdb_complete_unique_re { input_line complete_line_re {append_char " "} {max_completions 0}} { +proc test_gdb_complete_unique_re { input_line complete_line_re {append_char " "} {max_completions false} {testname ""} } { set append_char_re [string_to_regexp $append_char] # Trim COMPLETE LINE, for the case we're completing a command with leading @@ -292,14 +317,14 @@ proc test_gdb_complete_unique_re { input_line complete_line_re {append_char " "} # speeds up testing if necessary. set test_result [test_gdb_complete_cmd_unique $input_line\ - $expected_output_re] + $expected_output_re $testname] if { $test_result != 1 } { return $test_result } if { [readline_is_used] } { set test_result [test_gdb_complete_tab_unique $input_line \ - $complete_line_re $append_char_re] + $complete_line_re $append_char_re $testname] } return $test_result } @@ -307,9 +332,9 @@ proc test_gdb_complete_unique_re { input_line complete_line_re {append_char " "} # Like TEST_GDB_COMPLETE_UNIQUE_RE, but COMPLETE_LINE is a string, not # a regular expression. -proc test_gdb_complete_unique { input_line complete_line {append_char " "} {max_completions 0}} { +proc test_gdb_complete_unique { input_line complete_line {append_char " "} {max_completions false} {testname ""} } { set complete_line_re [string_to_regexp $complete_line] - test_gdb_complete_unique_re $input_line $complete_line_re $append_char $max_completions + test_gdb_complete_unique_re $input_line $complete_line_re $append_char $max_completions $testname } # Test that completing "CMD_PREFIX + COMPLETION_WORD" adds @@ -320,12 +345,13 @@ proc test_gdb_complete_unique { input_line complete_line {append_char " "} {max_ proc test_gdb_complete_multiple { cmd_prefix completion_word add_completed_line completion_list - {start_quote_char ""} {end_quote_char ""} {max_completions 0} + {start_quote_char ""} {end_quote_char ""} {max_completions false} + {testname ""} } { if { [readline_is_used] } { - test_gdb_complete_tab_multiple "$cmd_prefix$completion_word" $add_completed_line $completion_list $max_completions + test_gdb_complete_tab_multiple "$cmd_prefix$completion_word" $add_completed_line $completion_list $max_completions $testname } - test_gdb_complete_cmd_multiple $cmd_prefix $completion_word $completion_list $start_quote_char $end_quote_char $max_completions + test_gdb_complete_cmd_multiple $cmd_prefix $completion_word $completion_list $start_quote_char $end_quote_char $max_completions $testname } # Test that all the substring prefixes of INPUT from [0..START) to