[08/15] gdb/tui: improve errors from tui focus command

Message ID 8dcaa9d935cbfb4136cf5cab804f622b6f6a4c5b.1673000632.git.aburgess@redhat.com
State New
Headers
Series Mixed bag of TUI tests and fixes |

Commit Message

Andrew Burgess Jan. 6, 2023, 10:25 a.m. UTC
  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(-)
  

Patch

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 <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.
 
 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 <ctype.h>
-#include <unordered_map>
 #include <unordered_set>
 
 #include "tui/tui.h"
@@ -353,6 +352,16 @@  make_standard_window (const char *)
 
 static std::unordered_map<std::string, window_factory> *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 <unordered_map>
 
 /* Values that can be returned when handling a request to adjust a
    window's size.  */
@@ -364,4 +367,46 @@  typedef std::function<tui_win_info * (const char *name)> 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<std::string, window_factory>::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<all_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 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