From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from lndn.lancelotsix.com (lndn.lancelotsix.com [51.195.220.111]) by sourceware.org (Postfix) with ESMTPS id 82C243858D32 for ; Sat, 30 Mar 2024 23:48:57 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 82C243858D32 Authentication-Results: sourceware.org; dmarc=pass (p=reject dis=none) header.from=lancelotsix.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=lancelotsix.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 82C243858D32 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=51.195.220.111 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1711842538; cv=none; b=xhVJE26/HWzdMbkx9pOUxOEEaqAfuMT49OeQqf7X4Q7+FjZHfTmBCyskqAgYcbvnhbW1iPiNJUF8tFQ8Z32JA/06+9vxXn4ubsL6Vk8Zkc7cfRNcpfaxaecMPWnJPfJBlPz1qig3LESx/aGLNzBXh4DgjWpxt+BvxScQ448DuEQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1711842538; c=relaxed/simple; bh=u46mMkdnaAztjHezKSq0zMHehSZSE3m1Ye/AAHg/StI=; h=DKIM-Signature:Date:From:To:Subject:Message-ID:MIME-Version; b=gvQzxHnjtpC9nQav9Y3K3MJlklHeKldYbdsPVNUQM99EGzlRLvToV5IVNL6NYLisdGUp+c1+eXodRyLbwBwLuXQk26EgwAyh4p/sVnEpPllgKe+3VVJUsJzd//nm9tteOZ2PIGnko+16Qeg1+6rCxfGBxSOEly00ZIBZqMLwmS0= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from octopus (cust120-dsl54.idnet.net [212.69.54.120]) by lndn.lancelotsix.com (Postfix) with ESMTPSA id 73F2D80B16; Sat, 30 Mar 2024 23:48:56 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=lancelotsix.com; s=2021; t=1711842536; bh=u46mMkdnaAztjHezKSq0zMHehSZSE3m1Ye/AAHg/StI=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=kLDqdMzrhU71Z468Ar1ZV4A+gQPiWlC8YY9otlN6SgOXPSFVXj5Cwc6+tvrkR+R5U 9KukdNFpnCjh1SpWNlXITXtVflnnayrp91/QPdd3Bx8u2JnvvA4Ak27Jz7KkwkOAuG PpMeExciEMRpvBN9LT4xGTPvPSBljyR8Zj9+LD5AAZw4RzIam0Uh4hf9V5jydPL3N1 CR+2LHrKxRe/K0I07h9PsqCP6Opv7rodAVURYGcLW8V4Z3MYJKQGinpdiTi0u3sATL PCEVmJkQQRvSZWzV99b7YZsB+lMJBEMLIPGHDSw6d6QbAZReeNIk5UFAvmzyZaQWQy D2Z8xhVoU5p5g== Date: Sat, 30 Mar 2024 23:48:50 +0000 From: Lancelot SIX To: Andrew Burgess Cc: gdb-patches@sourceware.org Subject: Re: [PATCH 3/6] gdb: simplify completion_result::print_matches Message-ID: <20240330234850.qmlu3xtrspdd7zxm@octopus> References: <8f677c4387cb1b14d736112bd33e799e01df9167.1711712401.git.aburgess@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <8f677c4387cb1b14d736112bd33e799e01df9167.1711712401.git.aburgess@redhat.com> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.6.2 (lndn.lancelotsix.com [0.0.0.0]); Sat, 30 Mar 2024 23:48:56 +0000 (UTC) X-Spam-Status: No, score=-11.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Hi Andlew, I have a couple of minor comment below. > diff --git a/gdb/completer.c b/gdb/completer.c > index 9b4041da01a..2b3972213d8 100644 > --- a/gdb/completer.c > +++ b/gdb/completer.c > @@ -2311,23 +2311,30 @@ completion_tracker::build_completion_result (const char *text, > > if (m_lowest_common_denominator_unique) > { > - /* We don't rely on readline appending the quote char as > - delimiter as then readline wouldn't append the ' ' after the > - completion. */ > - char buf[2] = { (char) quote_char () }; > - > - match_list[0] = reconcat (match_list[0], match_list[0], > - buf, (char *) NULL); > - match_list[1] = NULL; > - > - /* If the tracker wants to, or we already have a space at the > - end of the match, tell readline to skip appending > - another. */ > - char *match = match_list[0]; > - bool completion_suppress_append > - = (suppress_append_ws () > - || (match[0] != '\0' > - && match[strlen (match) - 1] == ' ')); > + bool completion_suppress_append; > + > + if (from_readline ()) > + { > + /* We don't rely on readline appending the quote char as > + delimiter as then readline wouldn't append the ' ' after the > + completion. */ > + char buf[2] = { (char) quote_char () }; To be consistent with what is used below, "{ (char) quote_char (), '\0' }" > + > + match_list[0] = reconcat (match_list[0], match_list[0], buf, > + (char *) nullptr); > + > + /* If the tracker wants to, or we already have a space at the end > + of the match, tell readline to skip appending another. */ > + char *match = match_list[0]; > + completion_suppress_append > + = (suppress_append_ws () > + || (match[0] != '\0' > + && match[strlen (match) - 1] == ' ')); > + } > + else > + completion_suppress_append = false; > + > + match_list[1] = nullptr; > > return completion_result (match_list, 1, completion_suppress_append); > } > @@ -2449,21 +2456,14 @@ void > completion_result::print_matches (const std::string &prefix, > const char *word, int quote_char) > { > - if (this->number_matches == 1) > - printf_unfiltered ("%s%s\n", prefix.c_str (), this->match_list[0]); > - else > - { > - this->sort_match_list (); > + this->sort_match_list (); > > - for (size_t i = 0; i < this->number_matches; i++) > - { > - printf_unfiltered ("%s%s", prefix.c_str (), > - this->match_list[i + 1]); > - if (quote_char) > - printf_unfiltered ("%c", quote_char); > - printf_unfiltered ("\n"); > - } > - } > + 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); > > if (this->number_matches == max_completions) > { > diff --git a/gdb/testsuite/gdb.base/filename-completion.exp b/gdb/testsuite/gdb.base/filename-completion.exp > index f23e8671f40..66e5f411795 100644 > --- a/gdb/testsuite/gdb.base/filename-completion.exp > +++ b/gdb/testsuite/gdb.base/filename-completion.exp > @@ -63,8 +63,21 @@ proc run_tests { root } { > test_gdb_complete_none "file ${qc}${root}/xx" \ > "expand a non-existent filename" > > - test_gdb_complete_unique "file ${qc}${root}/a" \ > - "file ${qc}${root}/aaa/" "" false \ > + # The following test is split into separate cmd and tab calls as the > + # cmd versions will add a closing quote. It shouldn't be doing > + # this; this will be fixed in a later commit. Instead of testing the "wrong" behavior, if you intend to fix that in a later patch, I’d be tempted to prefer to just use "setup_kfail". > + test_gdb_complete_cmd_unique "file ${qc}${root}/a" \ > + "file ${qc}${root}/aaa/${qc}" \ > + "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 "file ${qc}${root}/" \ > -- > 2.25.4 > Best, Lancelot.