public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] [gdb/tui] Add set tui hide-last-column on/off/auto
@ 2023-05-09 13:20 Tom de Vries
  2023-05-10 13:48 ` Andrew Burgess
  2023-05-10 15:20 ` Tom Tromey
  0 siblings, 2 replies; 5+ messages in thread
From: Tom de Vries @ 2023-05-09 13:20 UTC (permalink / raw)
  To: gdb-patches; +Cc: Tom Tromey

Consider:
...
$ gdb -q -iex "set tui border-kind ascii" -tui
...

This gets us the expected lower border of src window, status line and prompt:
...
+----------------------------------------+
No process In:               L??   PC: ??
(gdb)
...

With TERM=ansi, we have instead:
...
+----------------------------------------+
No process In:               L??   PC: ??(
gdb)
...
This is PR tui/30388.  After doing a ^L we get the expected result.

There is a related problem with wrapping.

Say we have a terminal with COLUMNS == 40, and we type a repetitive numerical
string to just before wrapping, we have (annotating blinking cursor position
using "_"):
...
+--------------------------------------+
In:                        L??   PC: ??
(gdb) 789012345678901234567890123456789_
...

Now, after typing a 0, we have:
...
+--------------------------------------+
In:                        L??   PC: ??
(gdb) 7890123456789012345678901234567890
...
So, there's no cursor anymore.

After doing ^L we have:
...
+--------------------------------------+
In:                        L??   PC: ??
(gdb) 7890123456789012345678901234567890
                                       _
...
and now the cursor is stuck at the end of the next line.

After typing another char, 1, we have:
...
(gdb) 7890123456789012345678901234567890
                                       1
...
and again the cursor is gone.

After doing ^L again, we have the expected:
...
(gdb) 7890123456789012345678901234567890
1_
...

Both of these problems are in a way self-inflicted, and not the fault of gdb
or ncurses.

When doing a TERM=<term> export in a terminal emulator, we specify the
capabilities of the terminal to the application.

That is fine if we limit capabilities, by say overriding a default
TERM=xterm-256color with TERM=xterm-16color.

But it's different when we define TERM=ansi, because of the implications for
the xenl flag:
...
$ TERM=xterm tput xenl; echo $?
0
$ TERM=ansi tput xenl; echo $?
1
...
which stands for "eat newline glitch".

When we run with TERM=ansi, we tell the application that the terminal doesn't
have the glitch, while it still does, and you can expect to run into trouble.

At this point, we could just disregard this as a user problem, but there is an
interest in having this TERM=ansi working somewhat.  This is because the
native TERM for our tuiterm terminal emulator is ansi, and it's worthwhile:
- being able to compare behaviour in the tuiterm and an actual terminal
  emulator like konsole or gnome-terminal (which typically will have the
  glitch), as well as
- playing around with the TUI and explore new scenarios with TUI running in a
  mode as close as possible to tuiterm to increase the likeliness that the
  scenario can be reproduced in the tuiterm.

So, can we do something about this?  An observation that can be made is that
TERM=ansi doesn't seem to be causing problems in CLI mode.  This is because
readline defines a concept "reliable auto-wrap", and considers that if the
terminal capabilities include flags am and xenl, there's indeed reliable
auto-wrap.  Consequently, for TERM=ansi it considers there's no reliable
auto-wrap, and deals with this by hiding the last column of the terminal.

Fix both of the problems mentioned above by doing the same in TUI: if readline
hides the last column, also hide the last column in TUI.

This is a good solution, but for the tuiterm unnecessary, because it doesn't
have the glitch, and there's no need for for TUI (or indeed CLI) to work
around this.  We could just update the test-cases, but it would make writing
and debugging TUI test-cases less intuitive.

So, we expose the fix at user level using a "set tui hide-last-column
on/off/auto", with auto behaving like this:
...
$ TERM=xterm gdb -q  -ex "show tui hide-last-column"
TUI last column hiding is "auto; currently off".
$ TERM=ansi gdb -q -ex "show tui hide-last-column"
TUI last column hiding is "auto; currently on".
...
and use "set tui hide-last-column off" in the testsuite.

[ FWIW, there is currently no way to switch off the hidden column in CLI,
though I suppose it can be worked around by tricking readline and setting
width to 81 in an 80 COLUMNS terminal.  If that indeed works, we could
introduce a similar workaround for CLI that does this under the hood, but atm
this doesn't look useful to me. ]

Tested on x86_64-linux.

PR tui/30388
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30388
---
 gdb/doc/gdb.texinfo                 |  9 +++
 gdb/testsuite/gdb.tui/resize.exp    | 23 +++++++-
 gdb/testsuite/gdb.tui/wrap-line.exp | 13 +++++
 gdb/testsuite/lib/tuiterm.exp       | 14 +++++
 gdb/tui/tui-layout.c                |  7 ++-
 gdb/tui/tui-win.c                   | 88 ++++++++++++++++++++++++++++-
 gdb/tui/tui-win.h                   |  3 +
 7 files changed, 151 insertions(+), 6 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 8c4177c1901..f582f484704 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -30316,6 +30316,15 @@ source text at the next tab stop; the compact display uses only as
 much space as is needed for the line numbers in the current file, and
 only a single space to separate the line numbers from the source.
 
+@item set tui hide-last-column @r{[}on@r{|}off@r{|}auto@r{]}
+@kindex set tui hide-last-column
+Determines whether the TUI hides the last screen column.  This is
+useful when working around problems with incorrect terminal
+capabilities, for instance when using a terminal emulator that has the
+@code{xenl} capability while specifying a @env{TERM} setting such as
+@code{ansi} that implies that it doesn't.  If set to @code{auto}, TUI
+will hide the last column if readline does the same.
+
 @kindex set debug tui
 @item set debug tui @r{[}on|off@r{]}
 Turn on or off display of @value{GDBN} internal debug messages relating
diff --git a/gdb/testsuite/gdb.tui/resize.exp b/gdb/testsuite/gdb.tui/resize.exp
index 60d5116886b..53ea6f990ba 100644
--- a/gdb/testsuite/gdb.tui/resize.exp
+++ b/gdb/testsuite/gdb.tui/resize.exp
@@ -35,7 +35,24 @@ if {![Term::enter_tui]} {
     return
 }
 
-Term::check_contents "source at startup" "\\|.*21 *return 0"
+with_test_prefix startup {
+    Term::check_contents "source" "\\|.*21 *return 0"
+    Term::check_box "source box" 0 0 80 15
+}
+
+with_test_prefix resize=1 {
+    Term::resize 40 90
+    Term::check_box "source box" 0 0 90 26
+}
 
-Term::resize 40 90
-Term::check_box "source box after resize" 0 0 90 26
+with_test_prefix hide-last-column=on {
+    Term::command "set tui hide-last-column on"
+    Term::check_box "source box" 0 0 89 26
+    Term::check_contents_not "Hidden last colum empty" "\[^ \]\n"
+
+    with_test_prefix resize=2 {
+	Term::resize 24 80
+	Term::check_box "source box" 0 0 79 15
+	Term::check_contents_not "Hidden last colum empty" "\[^ \]\n"
+    }
+}
diff --git a/gdb/testsuite/gdb.tui/wrap-line.exp b/gdb/testsuite/gdb.tui/wrap-line.exp
index b28170808b8..f3d612fc7d0 100644
--- a/gdb/testsuite/gdb.tui/wrap-line.exp
+++ b/gdb/testsuite/gdb.tui/wrap-line.exp
@@ -126,6 +126,16 @@ proc test_wrap_cli_tui { auto_detected_width } {
 	set wrap_width $::cols
 
 	test_wrap $wrap_width
+
+	with_test_prefix hide-last-column=on {
+	    Term::command "set tui hide-last-column on"
+
+	    # Now that we've told curses to hide the last column, check that indeed
+	    # we wrap one char earlier.
+	    set wrap_width [expr $::cols - 1]
+
+	    test_wrap $wrap_width
+	}
     }
 }
 
@@ -158,6 +168,9 @@ with_test_prefix width-auto-detected {
 	}
     }
 
+    # As in Term::clean_restart.
+    gdb_test_no_output "set tui hide-last-column off"
+
     # Run tests with auto-detected screen width.
     test_wrap_cli_tui 1
 }
diff --git a/gdb/testsuite/lib/tuiterm.exp b/gdb/testsuite/lib/tuiterm.exp
index 5e4235da942..ddf18ad2c34 100644
--- a/gdb/testsuite/lib/tuiterm.exp
+++ b/gdb/testsuite/lib/tuiterm.exp
@@ -820,6 +820,12 @@ namespace eval Term {
 	    }
 
 	    ::gdb_test_no_output "set pagination off"
+
+	    # We have TERM=ansi, so the default hide-last-column == auto will
+	    # select on, which is a good value for an actual terminal emulator
+	    # with am && xenl behaviour, but the TUI term doens't have xenl
+	    # behaviour, so we don't have to work around it.
+	    ::gdb_test_no_output "set tui hide-last-column off"
 	}
     }
 
@@ -1004,6 +1010,14 @@ namespace eval Term {
 	gdb_assert {[regexp -- $regexp $contents]} $test_name
     }
 
+    # As check_contents, but checks whether the text contents of the terminal
+    # doesn't match the regular expression.
+    proc check_contents_not {test_name regexp} {
+	dump_screen
+	set contents [get_all_lines]
+	gdb_assert {![regexp -- $regexp $contents]} $test_name
+    }
+
     # Get the region of the screen described by X, Y, WIDTH,
     # and HEIGHT, and separate the lines using SEP.
     proc get_region { x y width height sep } {
diff --git a/gdb/tui/tui-layout.c b/gdb/tui/tui-layout.c
index 50c568fb7d7..107c84431e4 100644
--- a/gdb/tui/tui-layout.c
+++ b/gdb/tui/tui-layout.c
@@ -77,8 +77,11 @@ tui_apply_current_layout (bool preserve_cmd_win_size_p)
   for (tui_win_info *win_info : tui_windows)
     win_info->make_visible (false);
 
-  applied_layout->apply (0, 0, tui_term_width (), tui_term_height (),
-			 preserve_cmd_win_size_p);
+  applied_layout->apply (0, 0,
+			 (tui_hide_last_column
+			  ? tui_term_width () - 1
+			  : tui_term_width ()),
+			 tui_term_height (), preserve_cmd_win_size_p);
 
   /* Keep the list of internal windows up-to-date.  */
   for (int win_type = SRC_WIN; (win_type < MAX_MAJOR_WINDOWS); win_type++)
diff --git a/gdb/tui/tui-win.c b/gdb/tui/tui-win.c
index 7eac03f47a1..68003e6bdd6 100644
--- a/gdb/tui/tui-win.c
+++ b/gdb/tui/tui-win.c
@@ -346,6 +346,74 @@ tui_set_var_cmd (const char *null_args,
     tui_rehighlight_all ();
 }
 
+/* Variable corresponding to the "tui hide-last-column" setting.  */
+
+static enum auto_boolean tui_hide_last_column_setting = AUTO_BOOLEAN_AUTO;
+
+/* See tui-win.h.  */
+
+bool tui_hide_last_column;
+
+/* Show current value of the "tui hide-last-column" setting.  */
+
+static void
+tui_show_hide_last_column (struct ui_file *file, int from_tty,
+		       struct cmd_list_element *c,
+		       const char *value)
+{
+  if (strcmp (value, "auto") == 0)
+    gdb_printf (file,
+		_("TUI last column hiding is \"%s; currently %s\".\n"), value,
+		tui_hide_last_column ? "on" : "off");
+  else
+    gdb_printf (file,
+		_("TUI last column hiding is \"%s\".\n"), value);
+}
+
+/* Update tui_hide_last_column according to the current value of the
+   "tui hide-last-column" setting.  */
+
+static void
+update_tui_hide_last_column ()
+{
+  switch (tui_hide_last_column_setting)
+    {
+    case AUTO_BOOLEAN_AUTO:
+      tui_hide_last_column = readline_hidden_cols ? true : false;
+      break;
+    case AUTO_BOOLEAN_FALSE:
+      tui_hide_last_column = false;
+      break;
+    case AUTO_BOOLEAN_TRUE:
+      tui_hide_last_column = true;
+      break;
+    default:
+      gdb_assert_not_reached ("");
+    }
+}
+
+/* Set "tui hide-last-column".  */
+
+static void
+tui_set_hide_last_column (const char *null_args,
+			  int from_tty, struct cmd_list_element *c)
+{
+  bool prev_tui_hide_last_column = tui_hide_last_column;
+  update_tui_hide_last_column ();
+
+  if (!tui_active)
+    return;
+
+  if (!prev_tui_hide_last_column && tui_hide_last_column)
+    {
+      /* We're going to ignore the last column, clear it first.  */
+      clear ();
+      refresh ();
+    }
+
+  tui_apply_current_layout (false);
+  tui_refresh_all_win ();
+}
 \f
 
 /* True if TUI resizes should print a message.  This is used by the
@@ -454,7 +522,7 @@ tui_update_gdb_sizes (void)
 
   if (tui_active)
     {
-      width = TUI_CMD_WIN->width;
+      width = TUI_CMD_WIN->width + (tui_hide_last_column ? 1 : 0);
       height = TUI_CMD_WIN->height;
     }
   else
@@ -587,6 +655,12 @@ tui_async_resize_screen (gdb_client_data arg)
     }
   else
     {
+      if (tui_hide_last_column)
+	{
+	  /* Make sure the hidden last column is cleared.  */
+	  clear ();
+	  refresh ();
+	}
       tui_set_win_resized_to (false);
       tui_resize_all ();
       tui_refresh_all_win ();
@@ -1304,6 +1378,18 @@ When enabled, the left margin will use '_' and '0' instead of spaces."),
 			   &maintenance_set_cmdlist,
 			   &maintenance_show_cmdlist);
 
+  add_setshow_auto_boolean_cmd ("hide-last-column", class_tui,
+				&tui_hide_last_column_setting, _("\
+Set whether TUI should hide the last column."), _("\
+Show whether TUI should hide the last column."), _("\
+When enabled, TUI will hide the last column."),
+				tui_set_hide_last_column,
+				tui_show_hide_last_column,
+				&tui_setlist, &tui_showlist);
+  /* If tui_hide_last_column_setting == auto by default, determine the actual
+     on/off value.  */
+  update_tui_hide_last_column ();
+
   tui_border_style.changed.attach (tui_rehighlight_all, "tui-win");
   tui_active_border_style.changed.attach (tui_rehighlight_all, "tui-win");
 }
diff --git a/gdb/tui/tui-win.h b/gdb/tui/tui-win.h
index 3d35f1dfb7f..c10199eb37b 100644
--- a/gdb/tui/tui-win.h
+++ b/gdb/tui/tui-win.h
@@ -58,4 +58,7 @@ extern bool style_tui_current_position;
 /* Whether to replace the spaces in the left margin with '_' and '0'.  */
 extern bool tui_left_margin_verbose;
 
+/* Whether the tui should hide the last screen column.  */
+extern bool tui_hide_last_column;
+
 #endif /* TUI_TUI_WIN_H */

base-commit: d9cc4b060dd23724e1acf974aed3d1b72c8459e5
-- 
2.35.3


^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-05-22 13:45 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-09 13:20 [PATCH] [gdb/tui] Add set tui hide-last-column on/off/auto Tom de Vries
2023-05-10 13:48 ` Andrew Burgess
2023-05-10 15:21   ` Tom de Vries
2023-05-10 15:20 ` Tom Tromey
2023-05-22 13:45   ` Tom de Vries

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).