From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id YAwGGsJqYGb7RScAWB0awg (envelope-from ) for ; Wed, 05 Jun 2024 09:40:18 -0400 Authentication-Results: simark.ca; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=V7Wn9Z41; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 64D761E0C0; Wed, 5 Jun 2024 09:40:18 -0400 (EDT) Received: from server2.sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 4D2191E092 for ; Wed, 5 Jun 2024 09:40:16 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 168A03A48A2A for ; Wed, 5 Jun 2024 13:40:16 +0000 (GMT) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id A41DA398CC0D for ; Wed, 5 Jun 2024 13:36:29 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org A41DA398CC0D Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org A41DA398CC0D Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1717594593; cv=none; b=fmZ1ix9+6XUkwLqvhWPXOQSaNp0P/l0xiUvYHnyEuMpennnxbpJaK7C9tDmSuRpFtVSXvL2n/JYWBfDVJoAxOUr1+MV7xdcHfxNL9bqxVKb+5X1WSEnOqN/m2meLdzwu8aAgDgEyxThVZ/DHCRbXwceOYvWWeX4wQUQYmOIvHsA= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1717594593; c=relaxed/simple; bh=svhDQ3RhzWNHyunZDV2qpXUtEAuHiEsf3j5kDEXK5xI=; h=DKIM-Signature:From:To:Subject:Date:Message-Id:MIME-Version; b=ucaz4ASListhW/mM/H/jYIiucTazVVeRaLFlF5wRtWhj7bKVWM6trZHeCwMVsM/wZXoI9AVAIOld91jvEQ+Q0YTYu5n83egVpXdIyO9/kkSqLTAnfnhYc/DJL9+9zq7lItgigfm+KUvHaZIf9Imnwhwwh09yjcfWADpEp1Y/mpI= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1717594589; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=NtnWJ6M+f4luOXuo5dVKHwoXCx+l6cnC2wkHDr/FPLk=; b=V7Wn9Z41R1DMD07+ctiwcWxvfYc8owu+VyPhq99Qi2DV5QvCA9/g2z1aXAWJu4CgNKMMCV okNTNFeWMJI/g/nox6EcZ2YpvqOPFMBVaVJYR16dfM8Erm54qoPQOFVu1qAueU9mTOwEIg ObOKQQZGa56YrpDlLJXOQCAr1Nz49tI= Received: from mail-wr1-f71.google.com (mail-wr1-f71.google.com [209.85.221.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-328-mUzawdeINfWTatbacpX_jg-1; Wed, 05 Jun 2024 09:36:21 -0400 X-MC-Unique: mUzawdeINfWTatbacpX_jg-1 Received: by mail-wr1-f71.google.com with SMTP id ffacd0b85a97d-35dc5934390so3567804f8f.0 for ; Wed, 05 Jun 2024 06:36:20 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1717594579; x=1718199379; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=NtnWJ6M+f4luOXuo5dVKHwoXCx+l6cnC2wkHDr/FPLk=; b=Aguc8aepY49LtZfTjpeNswTLG6J1dJj4VeQdvA5ySNBerXh/ZpxxTxU4W8sWGgPdz9 o7UZq2RV3Y71qUbagTvYeYRgQHQlbyFJ9eVsfWkxXiY/GayTFsSpzXraIIVXWVl/sqMO fLzfuiXWfyCgyU/XswtWRRz6ctlFchUs2Cpavjpa52x/oYvCarwYq+4YEQzVDI1YPtbz iAdadOkO/bF7GJ5QCEcBbwkBnJAVrT+d8lK9L2MRwuVfzL3ie4Gwos6wtk2yK9jP95A7 s0WeNQTd3GaQKx0jhDO2UOKkYdBcRwGg1o6SzMUAoMEqjsfPz/FjUJZxwrj92OW2Kd5k wRFw== X-Gm-Message-State: AOJu0YyYTMTZH0ZbA43KpOGlSyLyeD6ktzxGGAzn7MYlz7VPV5RcJ4C+ 3A6qeLggVFZs1gtzbChfsE5IZNqkEhKIpJcRXD6Zn0ssi7vB7r9f2cW+FrS/9Q4UGcRVatT7Jlf /pq6aV3E+FyVZiv2cpaDe+PZ5GN643zX2A0S3BqtguCWQAMAG0GDb9q6dcmtZNMWt+5GNenVdoZ b9bYlWbjx9fw1RsOdsemt0dvg5yFkbYuTwnIlPlcyiWCE= X-Received: by 2002:adf:ea92:0:b0:354:fa7d:dcfe with SMTP id ffacd0b85a97d-35e8e1a4b55mr2039911f8f.23.1717594579194; Wed, 05 Jun 2024 06:36:19 -0700 (PDT) X-Google-Smtp-Source: AGHT+IGF8Baxr4xpV4Ya1w//vdJRdDFNjAi/59FstwTX/gH/E0mCsE7h4D/GFSsnhmC+WCrzG9pPFA== X-Received: by 2002:adf:ea92:0:b0:354:fa7d:dcfe with SMTP id ffacd0b85a97d-35e8e1a4b55mr2039890f8f.23.1717594578660; Wed, 05 Jun 2024 06:36:18 -0700 (PDT) Received: from localhost ([31.111.84.186]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-35dd04cada4sm14541737f8f.40.2024.06.05.06.36.17 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 05 Jun 2024 06:36:17 -0700 (PDT) From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Andrew Burgess Subject: [PATCHv3 5/7] gdb: add match formatter mechanism for 'complete' command output Date: Wed, 5 Jun 2024 14:36:05 +0100 Message-Id: X-Mailer: git-send-email 2.25.4 In-Reply-To: References: MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="US-ASCII"; x-default=true X-Spam-Status: No, score=-11.8 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org This commit solves a problem that existed prior to the previous commit, but the previous commit made more common. When completing a filename with the 'complete' command GDB will always add a trailing quote character, even if the completion is a directory name, in which case it would be better if the trailing quote was not added. Consider: (gdb) complete file '/tmp/xx file '/tmp/xxx/' The completion offered here is really only a partial completion, we've completed up to the end of the next directory name, but, until we have a filename then the completion is not finished and the trailing quote should not be added. This would match the readline behaviour, e.g.: (gdb) file '/tmp/xx (gdb) file '/tmp/xxx/ In this case readline completes the directory name, but doesn't add the trailing quote character. Remember that the 'complete' command is intended for tools like e.g. emacs in order that they can emulate GDB's standard readline completion when implementing a CLI of their own. As such, not adding the trailing quote in this case matches the readline behaviour, and seems like the right way to go. To achieve this, I've added a new function pointer member variable completion_result::m_match_formatter. This contains a pointer to a callback function which is used by the 'complete' command to format each result. The default behaviour of this callback function is to just append the quote character (the character from before the completion string) to the end of the completion result. This matches the current behaviour. However, for filename completion we override the default value of m_match_formatter, this new function checks if the completion result is a directory or not. If the completion result is a directory then the closing quote is not added, instead we add a trailing '/' character. The code to add a trailing '/' character already exists within the filename_completer function. This is no longer needed in this location, instead this code is moved into the formatter callback. Tests are updated to handle the changes in functionality, this removes an xfail added in the previous commit. --- gdb/completer.c | 92 ++++++++++++++----- gdb/completer.h | 45 ++++++++- .../gdb.base/filename-completion.exp | 68 ++++++++++---- 3 files changed, 156 insertions(+), 49 deletions(-) diff --git a/gdb/completer.c b/gdb/completer.c index 1fa82ecad87..fc33f5a742d 100644 --- a/gdb/completer.c +++ b/gdb/completer.c @@ -45,6 +45,8 @@ #include "completer.h" +#include + /* Forward declarations. */ static const char *completion_find_completion_word (completion_tracker &tracker, const char *text, @@ -356,6 +358,28 @@ gdb_completer_file_name_quote (char *text, int match_type ATTRIBUTE_UNUSED, return strdup (str.c_str ()); } +/* The function is used to update the completion word MATCH before + displaying it to the user in the 'complete' command output. This + function is only used for formatting filename or directory names. + + This function checks to see if the completion word MATCH is a directory, + in which case a trailing "/" (forward-slash) is added, otherwise + QUOTE_CHAR is added as a trailing quote. + + Return the updated completion word as a string. */ + +static std::string +filename_match_formatter (const char *match, char quote_char) +{ + std::string result (match); + if (std::filesystem::is_directory (gdb_tilde_expand (match))) + result += "/"; + else + result += quote_char; + + return result; +} + /* Generate filename completions of WORD, storing the completions into TRACKER. This is used for generating completions for commands that only accept unquoted filenames as well as for commands that accept @@ -365,6 +389,8 @@ static void filename_completer_generate_completions (completion_tracker &tracker, const char *word) { + tracker.set_match_format_func (filename_match_formatter); + int subsequent_name = 0; while (1) { @@ -383,20 +409,6 @@ filename_completer_generate_completions (completion_tracker &tracker, 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), word, word)); } @@ -1646,10 +1658,25 @@ int max_completions = 200; /* Initial size of the table. It automagically grows from here. */ #define INITIAL_COMPLETION_HTAB_SIZE 200 +/* The function is used to update the completion word MATCH before + displaying it to the user in the 'complete' command output. This + default function is used in all cases except those where a completion + function overrides this function by calling set_match_format_func. + + This function returns MATCH with QUOTE_CHAR appended. If QUOTE_CHAR is + the null-character then the returned string will just contain MATCH. */ + +static std::string +default_match_formatter (const char *match, char quote_char) +{ + return std::string (match) + quote_char; +} + /* See completer.h. */ completion_tracker::completion_tracker (bool from_readline) - : m_from_readline (from_readline) + : m_from_readline (from_readline), + m_match_format_func (default_match_formatter) { discard_completions (); } @@ -2352,7 +2379,8 @@ completion_tracker::build_completion_result (const char *text, match_list[1] = nullptr; - return completion_result (match_list, 1, completion_suppress_append); + return completion_result (match_list, 1, completion_suppress_append, + m_match_format_func); } else { @@ -2389,7 +2417,8 @@ completion_tracker::build_completion_result (const char *text, htab_traverse_noresize (m_entries_hash.get (), func, &builder); match_list[builder.index] = NULL; - return completion_result (match_list, builder.index - 1, false); + return completion_result (match_list, builder.index - 1, false, + m_match_format_func); } } @@ -2397,18 +2426,23 @@ completion_tracker::build_completion_result (const char *text, completion_result::completion_result () : match_list (NULL), number_matches (0), - completion_suppress_append (false) + completion_suppress_append (false), + m_match_formatter (default_match_formatter) {} /* See completer.h */ completion_result::completion_result (char **match_list_, size_t number_matches_, - bool completion_suppress_append_) + bool completion_suppress_append_, + match_format_func_t match_formatter_) : match_list (match_list_), number_matches (number_matches_), - completion_suppress_append (completion_suppress_append_) -{} + completion_suppress_append (completion_suppress_append_), + m_match_formatter (match_formatter_) +{ + gdb_assert (m_match_formatter != nullptr); +} /* See completer.h */ @@ -2421,10 +2455,12 @@ completion_result::~completion_result () completion_result::completion_result (completion_result &&rhs) noexcept : match_list (rhs.match_list), - number_matches (rhs.number_matches) + number_matches (rhs.number_matches), + m_match_formatter (rhs.m_match_formatter) { rhs.match_list = NULL; rhs.number_matches = 0; + rhs.m_match_formatter = default_match_formatter; } /* See completer.h */ @@ -2474,12 +2510,18 @@ completion_result::print_matches (const std::string &prefix, { this->sort_match_list (); - char buf[2] = { (char) quote_char, '\0' }; size_t off = this->number_matches == 1 ? 0 : 1; for (size_t i = 0; i < this->number_matches; i++) - printf_unfiltered ("%s%s%s\n", prefix.c_str (), - this->match_list[i + off], buf); + { + gdb_assert (this->m_match_formatter != nullptr); + std::string formatted_match + = this->m_match_formatter (this->match_list[i + off], + (char) quote_char); + + printf_unfiltered ("%s%s\n", prefix.c_str (), + formatted_match.c_str ()); + } if (this->number_matches == max_completions) { diff --git a/gdb/completer.h b/gdb/completer.h index 200d8a9b3af..71303e2ae9a 100644 --- a/gdb/completer.h +++ b/gdb/completer.h @@ -247,12 +247,24 @@ struct completion_match_result struct completion_result { + /* The type of a function that is used to format completion results when + using the 'complete' command. MATCH is the completion word to be + printed, and QUOTE_CHAR is a trailing quote character to (possibly) + add at the end of MATCH. QUOTE_CHAR can be the null-character in + which case no trailing quote should be added. + + Return the possibly modified completion match word which should be + presented to the user. */ + using match_format_func_t = std::string (*) (const char *match, + char quote_char); + /* Create an empty result. */ completion_result (); /* Create a result. */ completion_result (char **match_list, size_t number_matches, - bool completion_suppress_append); + bool completion_suppress_append, + match_format_func_t match_format_func); /* Destroy a result. */ ~completion_result (); @@ -274,10 +286,15 @@ struct completion_result completions, otherwise, each line of output consists of PREFIX followed by one of the possible completion words. - The QUOTE_CHAR is appended after each possible completion word and - should be the quote character that appears before the completion word, - or the null-character if there is no quote before the completion - word. */ + The QUOTE_CHAR is usually appended after each possible completion + word and should be the quote character that appears before the + completion word, or the null-character if there is no quote before + the completion word. + + The QUOTE_CHAR is not always appended to the completion output. For + example, filename completions will not append QUOTE_CHAR if the + completion is a directory name. This is all handled by calling this + function. */ void print_matches (const std::string &prefix, const char *word, int quote_char); @@ -305,6 +322,12 @@ struct completion_result /* Whether readline should suppress appending a whitespace, when there's only one possible completion. */ bool completion_suppress_append; + +private: + /* A function which formats a single completion match ready for display + as part of the 'complete' command output. Different completion + functions can set different formatter functions. */ + match_format_func_t m_match_formatter; }; /* Object used by completers to build a completion match list to hand @@ -441,6 +464,14 @@ class completion_tracker bool from_readline () const { return m_from_readline; } + /* Set the function used to format the completion word before displaying + it to the user to F, this is used by the 'complete' command. */ + void set_match_format_func (completion_result::match_format_func_t f) + { + gdb_assert (f != nullptr); + m_match_format_func = f; + } + private: /* The type that we place into the m_entries_hash hash table. */ @@ -535,6 +566,10 @@ class completion_tracker interactively. The 'complete' command is a way to generate completions not to be displayed by readline. */ bool m_from_readline; + + /* The function used to format the completion word before it is printed + in the 'complete' command output. */ + completion_result::match_format_func_t m_match_format_func; }; /* Return a string to hand off to readline as a completion match diff --git a/gdb/testsuite/gdb.base/filename-completion.exp b/gdb/testsuite/gdb.base/filename-completion.exp index 3e3b9b29ba4..0467d5c425e 100644 --- a/gdb/testsuite/gdb.base/filename-completion.exp +++ b/gdb/testsuite/gdb.base/filename-completion.exp @@ -58,6 +58,49 @@ proc setup_directory_tree {} { return $root } +# This proc started as a copy of test_gdb_complete_multiple, however, this +# version does some extra work. See the original test_gdb_complete_multiple +# for a description of all the arguments. +# +# When using the 'complete' command with filenames, GDB will add a trailing +# quote for filenames, and a trailing "/" for directory names. As the +# trailing "/" is also added in the tab-completion output the +# COMPLETION_LIST will include the "/" character, but the trailing quote is +# only added when using the 'complete' command. +# +# Pass the trailing quote will be passed as END_QUOTE_CHAR, this proc will +# run the tab completion test, and will then add the trailing quote to those +# entries in COMPLETION_LIST that don't have a trailing "/" before running +# the 'complete' command test. +proc test_gdb_complete_filename_multiple { + cmd_prefix completion_word add_completed_line completion_list + {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 $testname + } + + if { $start_quote_char eq "" && $end_quote_char ne "" } { + set updated_completion_list {} + + foreach entry $completion_list { + if {[string range $entry end end] ne "/"} { + set entry $entry$end_quote_char + } + lappend updated_completion_list $entry + } + + set completion_list $updated_completion_list + set end_quote_char "" + } + + test_gdb_complete_cmd_multiple $cmd_prefix $completion_word \ + $completion_list $start_quote_char $end_quote_char $max_completions \ + $testname +} + # Run filename completetion tests for those command that accept quoting and # escaping of the filename argument. # @@ -82,35 +125,22 @@ proc run_quoting_and_escaping_tests { root } { test_gdb_complete_none "$cmd ${qc}${root}/xx" \ "expand a non-existent filename" - # The following test is split into separate cmd and tab calls - # so we can xfail the cmd version. The cmd version will add a - # closing quote, it shouldn't be doing this. This will be - # fixed in a later commit. - if { $qc ne "" } { - setup_xfail "*-*-*" - } - test_gdb_complete_cmd_unique "file ${qc}${root}/a" \ - "file ${qc}${root}/aaa/" \ + test_gdb_complete_unique "file ${qc}${root}/a" \ + "file ${qc}${root}/aaa/" "" false \ "expand a unique directory name" - if { [readline_is_used] } { - test_gdb_complete_tab_unique "file ${qc}${root}/a" \ - "file ${qc}${root}/aaa/" "" \ - "expand a unique directory name" - } - test_gdb_complete_unique "file ${qc}${root}/cc2" \ "file ${qc}${root}/cc2${qc}" " " false \ "expand a unique filename" - test_gdb_complete_multiple "$cmd ${qc}${root}/" \ + test_gdb_complete_filename_multiple "$cmd ${qc}${root}/" \ "b" "b" { "bb1/" "bb2/" } "" "${qc}" false \ "expand multiple directory names" - test_gdb_complete_multiple "$cmd ${qc}${root}/" \ + test_gdb_complete_filename_multiple "$cmd ${qc}${root}/" \ "c" "c" { "cc1/" "cc2" @@ -122,14 +152,14 @@ proc run_quoting_and_escaping_tests { root } { if { $qc ne "" } { set sp " " - test_gdb_complete_multiple "file ${qc}${root}/aaa/" \ + test_gdb_complete_filename_multiple "file ${qc}${root}/aaa/" \ "a" "a${sp}" { "aa bb" "aa cc" } "" "${qc}" false \ "expand filenames containing spaces" - test_gdb_complete_multiple "file ${qc}${root}/bb1/" \ + test_gdb_complete_filename_multiple "file ${qc}${root}/bb1/" \ "a" "a" { "aa\"bb" "aa'bb" -- 2.25.4