From patchwork Fri May 24 11:25:43 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 32845 Received: (qmail 14287 invoked by alias); 24 May 2019 11:25:48 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 14277 invoked by uid 89); 24 May 2019 11:25:48 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 spammy=stands X-HELO: mail-wm1-f68.google.com Received: from mail-wm1-f68.google.com (HELO mail-wm1-f68.google.com) (209.85.128.68) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 24 May 2019 11:25:47 +0000 Received: by mail-wm1-f68.google.com with SMTP id f204so9005232wme.0 for ; Fri, 24 May 2019 04:25:46 -0700 (PDT) Return-Path: Received: from ?IPv6:2001:8a0:f913:f700:4eeb:42ff:feef:f164? ([2001:8a0:f913:f700:4eeb:42ff:feef:f164]) by smtp.gmail.com with ESMTPSA id f16sm2138992wrx.58.2019.05.24.04.25.43 (version=TLS1_3 cipher=AEAD-AES128-GCM-SHA256 bits=128/128); Fri, 24 May 2019 04:25:44 -0700 (PDT) Subject: Re: [PATCH 07/24] Remove "show" command completers To: Sergio Durigan Junior References: <20190522205327.2568-1-palves@redhat.com> <20190522205327.2568-8-palves@redhat.com> <87y32xnf76.fsf@redhat.com> Cc: gdb-patches@sourceware.org From: Pedro Alves Message-ID: <86392fa6-b345-79a1-1870-d3c0324c968e@redhat.com> Date: Fri, 24 May 2019 12:25:43 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: <87y32xnf76.fsf@redhat.com> On 5/23/19 8:28 PM, Sergio Durigan Junior wrote: > On Wednesday, May 22 2019, Pedro Alves wrote: > >> The default command completer is symbol_completer, but it makes no >> sense for a "show" command to complete on symbols, or anything else, >> really. > > Agreed. > >> I wonder whether we should instead make the default be no completer. >> That seems like a much larger/complicated audit/change, so I'd like to >> move forward with this version, as it'll be covered by tests. I >> noticed this because a following patch will add a new >> gdb.base/settings.exp testcase that exercises all sorts of details of >> settings commands, including completing the show commands, using new >> representative "maint test-settings " >> commands. > > I'm wondering why you chose to remove call set_cmd_completer on all of > the add_setshow_* functions if you could actually have called it once on > add_setshow_cmd_full, which is a static function and just used on > cli-decode.c anyway. Wow, how did I miss that? Much simpler, thanks! Now that I do this, it stands out what made me look at this code originally (which I had forgotten). It's that the var_string "set" commands complete on symbols, but that doesn't make much sense; that's what I first noticed when I was writing the gdb.base/settings.exp testcase. When I fixed that, while looking at the code, I realized that the "show" commands shouldn't also complete on anything. And then just copied over the same code to all set/show commands, failing to notice that I could do it in add_setshow_cmd_full. Here's the updated patch. From f4ba132856c870156d145d556c6e77d4cafb60a9 Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Wed, 22 May 2019 18:52:35 +0100 Subject: [PATCH 07/24] Remove "show" command completers, "set" command completers for string commands The default command completer is symbol_completer, but it makes no sense for a "show" command to complete on symbols, or anything else, really. I wonder whether we should instead make the default be no completer. That seems like a much larger/complicated audit/change, so I'd like to move forward with this version, as it'll be covered by tests. I noticed this because a following patch will add a new gdb.base/settings.exp testcase that exercises all sorts of details of settings commands, including completing the show commands, using new representative "maint test-settings " commands. Also remove the completer for var_string and var_string_noescape commands. No point in completing symbols when GDB is expecting a string. gdb/ChangeLog: yyyy-mm-dd Pedro Alves * cli/cli-decode.c (add_setshow_cmd_full): Remove "show" completer. (add_setshow_string_cmd, add_setshow_string_noescape_cmd): Remove "set" completers. --- gdb/cli/cli-decode.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c index 72e2a970097..80158593b38 100644 --- a/gdb/cli/cli-decode.c +++ b/gdb/cli/cli-decode.c @@ -508,6 +508,9 @@ add_setshow_cmd_full (const char *name, full_show_doc, show_list); show->doc_allocated = 1; show->show_value_func = show_func; + /* Disable the default symbol completer. Doesn't make much sense + for the "show" command to complete on anything. */ + set_cmd_completer (show, nullptr); if (set_result != NULL) *set_result = set; @@ -632,11 +635,16 @@ add_setshow_string_cmd (const char *name, enum command_class theclass, struct cmd_list_element **set_list, struct cmd_list_element **show_list) { + cmd_list_element *set_cmd; + add_setshow_cmd_full (name, theclass, var_string, var, set_doc, show_doc, help_doc, set_func, show_func, set_list, show_list, - NULL, NULL); + &set_cmd, NULL); + + /* Disable the default symbol completer. */ + set_cmd_completer (set_cmd, nullptr); } /* Add element named NAME to both the set and show command LISTs (the @@ -658,6 +666,10 @@ add_setshow_string_noescape_cmd (const char *name, enum command_class theclass, set_func, show_func, set_list, show_list, &set_cmd, NULL); + + /* Disable the default symbol completer. */ + set_cmd_completer (set_cmd, nullptr); + return set_cmd; }