[binutils-gdb] gdb/tui: improve errors from tui focus command

[binutils-gdb] gdb/tui: improve errors from tui focus command

[Andrew Burgess]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=58c6d2ac109965f8e38908146448073ab1152e81

commit 58c6d2ac109965f8e38908146448073ab1152e81
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Thu Dec 22 16:26:37 2022 +0000

    gdb/tui: improve errors from tui focus command
    
    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 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.
    
    This error was not previous being tested, but in this commit I make
    use of the Python API to trigger and test 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 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.

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/tui-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 <http://www.gnu.org/licenses/>.
 
-# 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.
 
 require allow_tui_tests
 
 tuiterm_env
+load_lib gdb-python.exp
 
 standard_testfile
 
@@ -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=$window" {
 
@@ -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 == "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*"
+	    }
 	}
 
 	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\", 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..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 <ctype.h>
-#include <unordered_map>
 #include <unordered_set>
 
 #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.  */
 
-static std::unordered_map<std::string, window_factory> *known_window_types;
+static window_types_map *known_window_types;
+
+/* See tui-layout.h.  */
+
+known_window_names_range
+all_known_window_names ()
+{
+  auto begin = known_window_names_iterator (known_window_types->begin ());
+  auto end = known_window_names_iterator (known_window_types->end ());
+  return known_window_names_range (begin, end);
+}
 
 /* Helper function that returns a TUI window, given its name.  */
 
@@ -377,7 +386,7 @@ tui_get_window_by_name (const std::string &name)
 static void
 initialize_known_windows ()
 {
-  known_window_types = new std::unordered_map<std::string, window_factory>;
+  known_window_types = new window_types_map;
 
   known_window_types->emplace (SRC_NAME,
 			       make_standard_window<SRC_WIN,
diff --git a/gdb/tui/tui-layout.h b/gdb/tui/tui-layout.h
index 206f1117445..ff354fb7c2f 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 <unordered_map>
 
 /* 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_info *win,
 
 typedef std::function<tui_win_info * (const char *name)> window_factory;
 
+/* The type for a data structure that maps a window name to that window's
+   factory function.  */
+typedef std::unordered_map<std::string, window_factory> 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.  */
 
 extern void tui_register_window (const char *name, window_factory &&factory);
 
+/* 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 = 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!= (const 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 known_window_names_range
+  = iterator_range<known_window_names_iterator>;
+
+/* 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 = 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