From patchwork Fri Jan 6 10:25:35 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 62788 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 2F2C3385843A for ; Fri, 6 Jan 2023 10:27:58 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 2F2C3385843A DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1673000878; bh=DJ7DTbm2xnqDrGzBOP7Nw1t2brxhR6yvk8XNYwIYEfk=; h=To:Cc:Subject:Date:In-Reply-To:References:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=C9/fg83RxV1GkXKiU+XyKpYKMsQ3khti+sxhop75Xy6Yl8QSanmDkuu3b2/0DFSCR 1e/NoYNswQ+Efc+dx30syKy3xK1jqXiCqfFfLI93oPcHTvnBhkp0V+lpIrqijkceUf BtPzxYngqYf57m6tjO/35KbotZ4vx1jfqdlcDCfs= X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org 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 4D855385700E for ; Fri, 6 Jan 2023 10:26:11 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 4D855385700E Received: from mail-ej1-f71.google.com (mail-ej1-f71.google.com [209.85.218.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-130-t4aYhPmPNPWcFVccCt7P0A-1; Fri, 06 Jan 2023 05:26:09 -0500 X-MC-Unique: t4aYhPmPNPWcFVccCt7P0A-1 Received: by mail-ej1-f71.google.com with SMTP id qb2-20020a1709077e8200b00842b790008fso872156ejc.21 for ; Fri, 06 Jan 2023 02:26:09 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; 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=DJ7DTbm2xnqDrGzBOP7Nw1t2brxhR6yvk8XNYwIYEfk=; b=pyiCNOJd/6F8h6LcrifjTZ9tNnO/WFl9Q8qotMlAhAh4n0Y2WqV1gCmpWFWLzLbR7m zgZJxzc2ZqQALXv0pslnsGbrhhR7qp3U/d/s/IlHLSwaJ/7MLdaLeUp+vln/q6Tpcb7K iLv4F1ZhCwQ4B0OMjElquHiBb0RJqZXtcjEvI3rx/d2U9OilQ/rXe2TPt9QA/L01GAmz ySuzz9akyWSq3l7fNI1+s0iSeGq6HhA2F3pQ/qFTi+1SAT6v44FxbSaGTz1MQE1+RlrF gXDccFob+mbOWMhaF4EaqQ1XdawrLoTLaBU0z0hzgTQhf1o6/2x/5sVPXC1tnpkhPbFA V+vQ== X-Gm-Message-State: AFqh2krbJ3bIuZURAzZaNAms0uRj3TopZuvl0pBtlR8xP5EieRHDFNUh vJk99n+7qtlXjvt2sgqU/LhlN/apRHVlix05YjI8N/1asX6Ulb/jDUu5iwmjLzXGbWRThr6ZYyJ b3uCt7XKs5sZsSz/hqy0q/8xp98UuC8zQ5UyPmXh63fqcl7FZK268MwTaeAhSYvbwnZ6oEgUb6A == X-Received: by 2002:a17:906:ad93:b0:7c1:100d:331a with SMTP id la19-20020a170906ad9300b007c1100d331amr48045997ejb.70.1673000768195; Fri, 06 Jan 2023 02:26:08 -0800 (PST) X-Google-Smtp-Source: AMrXdXuQ+IFHt6DkDxICS8p7yjrpuVGxtyOMy7hRjoYWtBCpRFbY/Cil/JzDH394MacLWHxH+xhbtw== X-Received: by 2002:a17:906:ad93:b0:7c1:100d:331a with SMTP id la19-20020a170906ad9300b007c1100d331amr48045974ejb.70.1673000767752; Fri, 06 Jan 2023 02:26:07 -0800 (PST) Received: from localhost (95.72.115.87.dyn.plus.net. [87.115.72.95]) by smtp.gmail.com with ESMTPSA id k19-20020a17090646d300b007c10fe64c5dsm265047ejs.86.2023.01.06.02.26.07 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 06 Jan 2023 02:26:07 -0800 (PST) To: gdb-patches@sourceware.org Cc: Andrew Burgess Subject: [PATCH 08/15] gdb/tui: improve errors from tui focus command Date: Fri, 6 Jan 2023 10:25:35 +0000 Message-Id: <8dcaa9d935cbfb4136cf5cab804f622b6f6a4c5b.1673000632.git.aburgess@redhat.com> 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 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, KAM_SHORT, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, 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 X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Andrew Burgess via Gdb-patches From: Andrew Burgess Reply-To: Andrew Burgess Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org Sender: "Gdb-patches" This commit improves (I think) the errors from the tui focus command. There are a number of errors that can be triggered by the focus command, they include: (1) Window name "NAME" is ambiguous (2) Unrecognized window name "NAME" (3) Window "NAME" cannot be focused Error (1) is triggered when the user gives a partial window name, and the name matches multiple windows in the current layout. It is worth noting that the ambiguity must be within the current layout, if the partial name matches one window in the current layout, and one window not currently displayed, then this is not ambiguous, and focus will shift to the selected window in the current layout. This error was not previous being tested, but in this commit I make use of the Python API to trigger this error. Error (3) is simple enough, and was already being tested. This is triggered by something like 'focus status'. The named window needs to be present in the current layout, and non-focusable in order to trigger the error. Error (2) is what I'd like to improve in this commit. This error triggers if the name the user gives doesn't match any window in the current layout. Even if GDB does know about the window, but the window isn't in the current layout, then GDB will say it doesn't recognize the window name. In this commit I propose to change this. I plan to split this error into three different errors. These will be: (a) Unrecognized window name "NAME" (b) No windows matching "NAME" in the current layout (c) Window "NAME" is not in the current layout Error (a) is the same as before, but will now only trigger if GDB doesn't know about window NAME at all. If the window is known, but not in the current layout then one of the other errors will trigger. Error (b) will trigger if NAME is ambiguous for multiple windows that are not in the current layout. If NAME identifies a single window in the current layout then that window will continue to be selected, just as it currently is. Only in the case where NAME doesn't identify a window in the current layout do we then check all the other known windows, if NAME matches multiple of these, then (b) is triggered. Finally, error (c) is used when NAME uniquely identifies a single window that is not in the current layout. The hope with these new errors is that the user will have a better understanding of what went wrong. Instead of GDB claiming to not know about a window, the mention of the current layout will hint to the user that they should first switch layouts. There are tests included for all the new errors. --- gdb/testsuite/gdb.tui/tui-focus.exp | 81 +++++++++++++++++++++++++++-- gdb/tui/tui-layout.c | 11 +++- gdb/tui/tui-layout.h | 45 ++++++++++++++++ gdb/tui/tui-win.c | 40 +++++++++++++- 4 files changed, 170 insertions(+), 7 deletions(-) diff --git a/gdb/testsuite/gdb.tui/tui-focus.exp b/gdb/testsuite/gdb.tui/tui-focus.exp index 0563aacd0a2..4ad2e5327c7 100644 --- a/gdb/testsuite/gdb.tui/tui-focus.exp +++ b/gdb/testsuite/gdb.tui/tui-focus.exp @@ -13,10 +13,10 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . -# Minimal testcase that just checks that the various "layout $foo" -# commands do not cause gdb to crash. +# Testcase that just checks the tui 'focus' command works as expected. tuiterm_env +load_lib gdb-python.exp standard_testfile @@ -33,7 +33,8 @@ if {[skip_tui_tests]} { # Each test specification is a tuple where the first item is the name of a # window, and the second item is a boolean indicating if we expect that # window to be present in the default (src) layout. -foreach spec {{src true} {cmd true} {status false} {regs false} {asm false}} { +foreach spec {{src true} {cmd true} {status false} {regs false} \ + {asm false} {unknown false}} { lassign $spec window valid_p with_test_prefix "window=$window" { @@ -52,9 +53,12 @@ foreach spec {{src true} {cmd true} {status false} {regs false} {asm false}} { if {$window == "status"} { Term::check_region_contents "check focus error" 0 16 80 1 \ "^Window \"$window\" cannot be focused\\s*" - } else { + } elseif {$window == "unknown"} { Term::check_region_contents "check focus error" 0 16 80 1 \ "^Unrecognized window name \"$window\"\\s*" + } else { + Term::check_region_contents "check focus error" 0 16 80 1 \ + "^Window \"$window\" is not in the current layout\\s*" } } @@ -62,3 +66,72 @@ foreach spec {{src true} {cmd true} {status false} {regs false} {asm false}} { Term::command "focus prev" } } + +# Use the Python TUI API to exercise some of the ambigous window name +# handling parts of the 'focus' command. +Term::clean_restart 24 80 $binfile +if {![skip_python_tests]} { + # Create a very simple tui window. + gdb_py_test_silent_cmd \ + [multi_line_input \ + "python" \ + "class TestWindow:" \ + " def __init__(self, win):" \ + " pass" \ + "" \ + " def render(self):" \ + " pass" \ + "end"] \ + "setup dummy window class" \ + true + + # Register the window with a set of similar names. + gdb_test_no_output "python gdb.register_window_type(\"test1\", TestWindow)" + gdb_test_no_output "python gdb.register_window_type(\"test2\", TestWindow)" + gdb_test_no_output "python gdb.register_window_type(\"test3\", TestWindow)" + + # Create a layout containing just one of the above windows. + gdb_test_no_output "tui new-layout example1 test2 1 status 1 cmd 1" + + # Create a layout containing two of the above windows. + gdb_test_no_output "tui new-layout example2 test1 1 test2 1 status 1 cmd 1" + + if {![Term::prepare_for_tui]} { + unsupported "TUI not supported" + return + } + + # Try to focus using an ambiguous, partial window name. This + # should fail as the default layout (src) doesn't include any + # windows matching this name. + Term::command_no_prompt_prefix "focus test" + Term::check_region_contents "check no matching window focus message" \ + 0 16 80 1 \ + "^No windows matching \"test\" in the current layout\\s*" + + # Now select a layout that includes a single window that matches + # the ambiguous, partial name 'test', and disable tui mode. + Term::command "layout example1" + send_gdb "tui disable\n" + + # Reactivate tui mode and try to set focus using the ambiguous, + # partial window name. This should succeed though, as, within the + # current layout, the partial name is not actually ambiguous. + send_gdb "focus test\n" + gdb_assert [Term::wait_for_region_contents 0 19 80 1 \ + "^Focus set to test2 window\\.\\s*"] \ + "check test2 focus message" + + # Now select a layout that includes two windows that matches the + # ambiguous, partial name 'test', and disable tui mode. + Term::command "layout example2" + send_gdb "tui disable\n" + + # Reactivate tui mode and try to set focus using the ambiguous, + # partial window name. This will fail as now the layout includes + # multiple windows that match 'test'. + send_gdb "focus test\n" + gdb_assert [Term::wait_for_region_contents 0 22 80 1 \ + "^Window name \"test\" is ambiguous\\s*"] \ + "check ambiguous focus message" +} diff --git a/gdb/tui/tui-layout.c b/gdb/tui/tui-layout.c index 27abee02087..447a20cb614 100644 --- a/gdb/tui/tui-layout.c +++ b/gdb/tui/tui-layout.c @@ -29,7 +29,6 @@ #include "cli/cli-decode.h" #include "cli/cli-utils.h" #include -#include #include #include "tui/tui.h" @@ -353,6 +352,16 @@ make_standard_window (const char *) static std::unordered_map *known_window_types; +/* See tui-layout.h. */ + +all_known_window_names_range +all_known_window_names () +{ + auto begin = all_known_window_names_iterator (known_window_types->begin ()); + auto end = all_known_window_names_iterator (known_window_types->end ()); + return all_known_window_names_range (begin, end); +} + /* Helper function that returns a TUI window, given its name. */ static tui_win_info * diff --git a/gdb/tui/tui-layout.h b/gdb/tui/tui-layout.h index 206f1117445..9ce3ef39507 100644 --- a/gdb/tui/tui-layout.h +++ b/gdb/tui/tui-layout.h @@ -26,6 +26,9 @@ #include "tui/tui.h" #include "tui/tui-data.h" +#include "gdbsupport/iterator-range.h" + +#include /* Values that can be returned when handling a request to adjust a window's size. */ @@ -364,4 +367,46 @@ typedef std::function window_factory; extern void tui_register_window (const char *name, window_factory &&factory); +/* An iterator class for known tui window names. This is just a wrapper + around an iterator of the underlying data structure that holds the + known tui windows. This iterator only reveals the window names. */ + +struct all_known_window_names_iterator +{ + using known_window_types_iterator + = std::unordered_map::iterator; + + all_known_window_names_iterator (known_window_types_iterator &&iter) + : m_iter (std::move (iter)) + { /* Nothing. */ } + + all_known_window_names_iterator &operator++ () + { + ++m_iter; + return *this; + } + + const std::string &operator* () const + { return (*m_iter).first; } + + bool operator!= (const all_known_window_names_iterator &other) const + { return m_iter != other.m_iter; } + +private: + + /* The underlying iterator. */ + known_window_types_iterator m_iter; +}; + +/* A range adapter that makes it possible to iterate over the names of all + known tui windows. */ + +using all_known_window_names_range + = iterator_range; + +/* Return a range that can be used to walk over the name of all known tui + windows in a range-for loop. */ + +extern all_known_window_names_range all_known_window_names (); + #endif /* TUI_TUI_LAYOUT_H */ diff --git a/gdb/tui/tui-win.c b/gdb/tui/tui-win.c index 492a5191025..008189eb99b 100644 --- a/gdb/tui/tui-win.c +++ b/gdb/tui/tui-win.c @@ -728,8 +728,44 @@ tui_set_focus_command (const char *arg, int from_tty) else win_info = tui_partial_win_by_name (arg); - if (win_info == NULL) - error (_("Unrecognized window name \"%s\""), arg); + if (win_info == nullptr) + { + /* When WIN_INFO is nullptr this can either mean that the window name + is unknown to GDB, or that the window is not in the current + layout. To try and help the user, give a different error + depending on which of these is the case. */ + std::string matching_window_name; + bool is_ambiguous = false; + + for (const std::string &name : all_known_window_names ()) + { + /* Look through all windows in the current layout, if the window + is in the current layout then we're not interested is it. */ + for (tui_win_info *item : all_tui_windows ()) + if (item->name () == name) + continue; + + if (startswith (name, arg)) + { + if (matching_window_name.empty ()) + matching_window_name = name; + else + is_ambiguous = true; + } + }; + + if (!matching_window_name.empty ()) + { + if (is_ambiguous) + error (_("No windows matching \"%s\" in the current layout"), + arg); + else + error (_("Window \"%s\" is not in the current layout"), + matching_window_name.c_str ()); + } + else + error (_("Unrecognized window name \"%s\""), arg); + } /* If a window is part of the current layout then it will have a tui_win_info associated with it and be visible, otherwise, there will