From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 1726) id 6D9C03858039; Fri, 27 Jan 2023 16:25:08 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 6D9C03858039 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1674836708; bh=8AUW9CUj12j8QnLQmX14ybHRafGaY2FPJQswzNStFRg=; h=From:To:Subject:Date:From; b=i7YEkU2StGXl5m6g3kUulhBiSUXn/KqvGo9Oa9xC/JroPn/+z7xqUHZm7Uc7OEgmS glUmlXwsVghO6fw6PZ8CvQpVyyjVbX+GaIA/8dVznPhdY/R9vv87uJ9iW83MeB6DjY v3ssAlnOEOt571YMKWYMHIyoTWhtw73GXvRkjTqg= Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Andrew Burgess To: gdb-cvs@sourceware.org Subject: [binutils-gdb] gdb/tui: improve errors from tui focus command X-Act-Checkin: binutils-gdb X-Git-Author: Andrew Burgess X-Git-Refname: refs/heads/master X-Git-Oldrev: efe1b6507b7e6ae5ee45af5b1568b910a3170750 X-Git-Newrev: 58c6d2ac109965f8e38908146448073ab1152e81 Message-Id: <20230127162508.6D9C03858039@sourceware.org> Date: Fri, 27 Jan 2023 16:25:08 +0000 (GMT) List-Id: https://sourceware.org/git/gitweb.cgi?p=3Dbinutils-gdb.git;h=3D58c6d2ac1099= 65f8e38908146448073ab1152e81 commit 58c6d2ac109965f8e38908146448073ab1152e81 Author: Andrew Burgess Date: Thu Dec 22 16:26:37 2022 +0000 gdb/tui: improve errors from tui focus command =20 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: =20 (1) Window name "NAME" is ambiguous =20 (2) Unrecognized window name "NAME" =20 (3) Window "NAME" cannot be focused =20 Error (1) is triggered when the user gives a partial window name, and the name matches multiple windows in the current layout. =20 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 or more windows not in the current layout, then this is not ambiguous, and focus will shift to the matching window in the current layout. =20 This error was not previous being tested, but in this commit I make use of the Python API to trigger and test this error. =20 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. =20 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. =20 In this commit I propose to to split this error into three different errors. These will be: =20 (a) Unrecognized window name "NAME" =20 (b) No windows matching "NAME" in the current layout =20 (c) Window "NAME" is not in the current layout =20 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. =20 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. =20 Finally, error (c) is used when NAME uniquely identifies a single window that is not in the current layout. =20 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. =20 There are tests included for all the new errors. Diff: --- gdb/testsuite/gdb.tui/tui-focus.exp | 85 +++++++++++++++++++++++++++++++++= +--- gdb/tui/tui-layout.c | 15 +++++-- gdb/tui/tui-layout.h | 49 +++++++++++++++++++++ gdb/tui/tui-win.c | 40 ++++++++++++++++- 4 files changed, 179 insertions(+), 10 deletions(-) diff --git a/gdb/testsuite/gdb.tui/tui-focus.exp b/gdb/testsuite/gdb.tui/tu= i-focus.exp index 9e2a1cd0935..72f80523af3 100644 --- a/gdb/testsuite/gdb.tui/tui-focus.exp +++ b/gdb/testsuite/gdb.tui/tui-focus.exp @@ -13,12 +13,12 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . =20 -# 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. =20 require allow_tui_tests =20 tuiterm_env +load_lib gdb-python.exp =20 standard_testfile =20 @@ -31,7 +31,8 @@ if {[prepare_for_testing "failed to prepare" ${testfile} = ${srcfile}]} { # 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 true} {regs false} {asm false}= } { +foreach spec {{src true} {cmd true} {status true} {regs false} \ + {asm false} {unknown false}} { lassign $spec window valid_p with_test_prefix "window=3D$window" { =20 @@ -54,8 +55,13 @@ foreach spec {{src true} {cmd true} {status true} {regs = false} {asm false}} { "^Focus set to $window window\\.\\s*" } } else { - Term::check_region_contents "check focus error" 0 16 80 1 \ - "^Unrecognized window name \"$window\"\\s*" + if {$window =3D=3D "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*" + } } =20 Term::check_box "check src box" 0 0 80 15 @@ -66,3 +72,72 @@ foreach spec {{src true} {cmd true} {status true} {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 {[allow_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\", TestWin= dow)" + gdb_test_no_output "python gdb.register_window_type(\"test2\", TestWin= dow)" + gdb_test_no_output "python gdb.register_window_type(\"test3\", TestWin= dow)" + + # 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 c= md 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..ecdcd4884a7 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 =20 #include "tui/tui.h" @@ -351,7 +350,17 @@ make_standard_window (const char *) shut down, causing crashes if any window destruction requires running Python code. */ =20 -static std::unordered_map *known_window_types; +static window_types_map *known_window_types; + +/* See tui-layout.h. */ + +known_window_names_range +all_known_window_names () +{ + auto begin =3D known_window_names_iterator (known_window_types->begin ()= ); + auto end =3D known_window_names_iterator (known_window_types->end ()); + return known_window_names_range (begin, end); +} =20 /* Helper function that returns a TUI window, given its name. */ =20 @@ -377,7 +386,7 @@ tui_get_window_by_name (const std::string &name) static void initialize_known_windows () { - known_window_types =3D new std::unordered_map; + known_window_types =3D new window_types_map; =20 known_window_types->emplace (SRC_NAME, make_standard_window =20 /* Values that can be returned when handling a request to adjust a window's size. */ @@ -358,10 +361,56 @@ extern void tui_adjust_window_width (struct tui_win_i= nfo *win, =20 typedef std::function window_factory; =20 +/* The type for a data structure that maps a window name to that window's + factory function. */ +typedef std::unordered_map window_types_map; + /* Register a new TUI window type. NAME is the name of the window type. FACTORY is a function that can be called to instantiate the window. */ =20 extern void tui_register_window (const char *name, window_factory &&factor= y); =20 +/* An iterator class that exposes just the window names from the + known_window_types map in tui-layout.c. This is just a wrapper around + an iterator of the underlying known_window_types map, but this just + exposes the window names. */ + +struct known_window_names_iterator +{ + using known_window_types_iterator =3D window_types_map::iterator; + + known_window_names_iterator (known_window_types_iterator &&iter) + : m_iter (std::move (iter)) + { /* Nothing. */ } + + known_window_names_iterator &operator++ () + { + ++m_iter; + return *this; + } + + const std::string &operator* () const + { return (*m_iter).first; } + + bool operator!=3D (const known_window_names_iterator &other) const + { return m_iter !=3D 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 known_window_names_range + =3D 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 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 =3D tui_partial_win_by_name (arg); =20 - if (win_info =3D=3D NULL) - error (_("Unrecognized window name \"%s\""), arg); + if (win_info =3D=3D 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 =3D 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 () =3D=3D name) + continue; + + if (startswith (name, arg)) + { + if (matching_window_name.empty ()) + matching_window_name =3D name; + else + is_ambiguous =3D 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); + } =20 /* 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