[pushed,gdb/tui] Fix TUI resizing for TERM=ansi
Commit Message
With TERM=ansi, when resizing a TUI window from LINES/COLUMNS 31/118
(maximized) to 20/78 (de-maximized), I get a garbled screen (that ^L doesn't
fix) and a message:
...
@@ resize done 0, size = 77x20
...
with the resulting width being 77 instead of the expected 78.
[ The discrepancy also manifests in CLI, filed as PR30346. ]
The discrepancy comes from tui_resize_all, where we ask readline for the
screen size:
...
rl_get_screen_size (&screenheight, &screenwidth);
...
As it happens, when TERM is set to ansi, readline decides that the terminal
cannot auto-wrap lines, and reserves one column to deal with that, and as a
result reports back one less than the actual screen width:
...
$ echo $COLUMNS
78
$ TERM=xterm gdb -ex "show width" -ex q
Number of characters gdb thinks are in a line is 78.
$ TERM=ansi gdb -ex "show width" -ex q
Number of characters gdb thinks are in a line is 77.
...
In tui_resize_all, we need the actual screen width, and using a screenwidth of
one less than the actual value garbles the screen.
This is currently not causing trouble in testing because we have a workaround
in place in proc Term::resize. If we disable the workaround:
...
- stty columns [expr {$_cols + 1}] < $::gdb_tty_name
+ stty columns $_cols < $::gdb_tty_name
...
and dump the screen we get the same type of screen garbling:
...
0 +---------------------------------------+|
1 ||
2 ||
3 ||
...
Another way to reproduce the problem is using command "maint info screen".
After starting gdb with TERM=ansi, entering TUI, and issuing the command, we
get:
...
Number of characters curses thinks are in a line is 78.
...
and after maximizing and demaximizing the window we get:
...
Number of characters curses thinks are in a line is 77.
...
If we use TERM=xterm, we do get the expected 78.
Fix this by:
- detecting when readline will report back less than the actual screen width,
- accordingly setting a new variable readline_hidden_cols,
- using readline_hidden_cols in tui_resize_all to fix the resize problem, and
- removing the workaround in Term::resize.
The test-case gdb.tui/empty.exp serves as regression test.
I've applied the same fix in tui_async_resize_screen, the new test-case
gdb.tui/resize-2.exp serves as a regression test for that change. Without
that fix, we have:
...
FAIL: gdb.tui/resize-2.exp: again: gdb width 80
...
Tested on x86_64-linux.
PR tui/30337
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30337
---
gdb/testsuite/gdb.tui/resize-2.exp | 89 ++++++++++++++++++++++++++++++
gdb/testsuite/lib/tuiterm.exp | 18 +++---
gdb/tui/tui-win.c | 4 ++
gdb/utils.c | 21 +++++++
gdb/utils.h | 7 +++
5 files changed, 129 insertions(+), 10 deletions(-)
create mode 100644 gdb/testsuite/gdb.tui/resize-2.exp
base-commit: bec5d8fc8c78e8a3da2c168366f33a856d55124b
Comments
>>>>> "Tom" == Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> writes:
Tom> As it happens, when TERM is set to ansi, readline decides that the terminal
Tom> cannot auto-wrap lines, and reserves one column to deal with that, and as a
Tom> result reports back one less than the actual screen width:
...
Tom> This is currently not causing trouble in testing because we have a workaround
Tom> in place in proc Term::resize. If we disable the workaround:
Thank you for tracking this down.
I never even considered this might be a readline issue.
Tom> + readline_hidden_cols = _rl_term_autowrap ? 0 : 1;
I hate to have a new dependency on a readline internal variable.
Don't some distros mark these as hidden in libreadline.so?
I feel like there was another bug along these lines.
However, I don't see another way to do it.
Maybe some official API could be added by the upstream readline.
Would you want to bring it up there?
Anyway it seems ok to me.
thanks,
Tom
new file mode 100644
@@ -0,0 +1,89 @@
+# Copyright 2023 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+# Test TUI resizing using maint screen info command.
+
+require allow_tui_tests
+
+tuiterm_env
+
+Term::clean_restart 24 80
+set screen_dim { 0 0 80 24 }
+
+# Use a layout with just a command window.
+gdb_test "tui new-layout command-layout cmd 1"
+
+if {![Term::prepare_for_tui]} {
+ unsupported "TUI not supported"
+ return 0
+}
+
+# Enter TUI.
+Term::command_no_prompt_prefix "layout command-layout"
+
+proc check_width { what n } {
+ set re "Number of characters $what thinks are in a line is $n"
+ Term::check_region_contents "$what width $n" {*}$::screen_dim $re
+}
+
+# Check that curses has the correct notion of screen width.
+Term::command "maint info screen"
+check_width curses 80
+check_width gdb 80
+
+# Resize with TUI enabled, wait for the resize message.
+Term::resize 40 90
+set screen_dim { 0 0 90 40 }
+
+# Check that curses has the correct notion of screen width after resize.
+Term::command "maint info screen"
+check_width curses 90
+check_width gdb 90
+
+# Temporarily disable TUI.
+gdb_test_multiple "tui disable" "" {
+ -re "$gdb_prompt $" {
+ pass $gdb_test_name
+ }
+}
+
+# Resize with TUI disabled, so don't wait for the resize message.
+Term::resize 24 80 0
+set screen_dim { 0 0 80 24 }
+gdb_test_multiple "" "two prompt redisplays after resize" {
+ -re "\r.*$gdb_prompt \r.*$gdb_prompt $" {
+ pass $gdb_test_name
+ }
+}
+
+# At this point, curses still thinks the width is 90. This doesn't look
+# harmful because TUI is disabled.
+gdb_test "maint info screen" \
+ "\r\nNumber of characters curses thinks are in a line is 90.\\r\n.*" \
+ "curses width after resize with TUI disabled"
+
+# Re-enable TUI.
+send_gdb "tui enable\n"
+# The "tui enable" command is issued on the CLI screen, on the TUI we have the
+# last command issued there: "tui disable".
+Term::wait_for "tui disable"
+
+# Check that curses has the correct notion of screen width after screen resize
+# with TUI disabled.
+Term::command "maint info screen"
+with_test_prefix again {
+ check_width curses 80
+ check_width gdb 80
+}
@@ -1111,7 +1111,7 @@ namespace eval Term {
}
}
- proc resize {rows cols} {
+ proc resize {rows cols {wait_for_msg 1}} {
variable _rows
variable _cols
variable _resize_count
@@ -1122,17 +1122,15 @@ namespace eval Term {
# explicit here. This also simplifies waiting for the redraw.
_do_resize $rows $_cols
stty rows $_rows < $::gdb_tty_name
- # Due to the strange column resizing behavior, and because we
- # don't care about this intermediate resize, we don't check
- # the size and the "@@ " prefix here.
- wait_for "resize done $_resize_count"
+ if { $wait_for_msg } {
+ wait_for "@@ resize done $_resize_count, size = ${_cols}x${rows}"
+ }
incr _resize_count
- # Somehow the number of columns transmitted to gdb is one less
- # than what we request from expect. We hide this weird
- # details from the caller.
_do_resize $_rows $cols
- stty columns [expr {$_cols + 1}] < $::gdb_tty_name
- wait_for "@@ resize done $_resize_count, size = ${_cols}x${rows}"
+ stty columns $_cols < $::gdb_tty_name
+ if { $wait_for_msg } {
+ wait_for "@@ resize done $_resize_count, size = ${_cols}x${rows}"
+ }
incr _resize_count
}
}
@@ -36,6 +36,7 @@
#include "gdbsupport/event-loop.h"
#include "gdbcmd.h"
#include "async-event.h"
+#include "utils.h"
#include "tui/tui.h"
#include "tui/tui-io.h"
@@ -528,6 +529,8 @@ tui_resize_all (void)
int screenheight, screenwidth;
rl_get_screen_size (&screenheight, &screenwidth);
+ screenwidth += readline_hidden_cols;
+
width_diff = screenwidth - tui_term_width ();
height_diff = screenheight - tui_term_height ();
if (height_diff || width_diff)
@@ -576,6 +579,7 @@ tui_async_resize_screen (gdb_client_data arg)
int screen_height, screen_width;
rl_get_screen_size (&screen_height, &screen_width);
+ screen_width += readline_hidden_cols;
set_screen_width_and_height (screen_width, screen_height);
/* win_resized is left set so that the next call to tui_enable()
@@ -1116,6 +1116,14 @@ static bool filter_initialized = false;
+/* See readline's rlprivate.h. */
+
+EXTERN_C int _rl_term_autowrap;
+
+/* See utils.h. */
+
+int readline_hidden_cols = 0;
+
/* Initialize the number of lines per page and chars per line. */
void
@@ -1144,6 +1152,19 @@ init_page_info (void)
/* Get the screen size from Readline. */
rl_get_screen_size (&rows, &cols);
+
+ /* Readline:
+ - ignores the COLUMNS variable when detecting screen width
+ (because rl_prefer_env_winsize defaults to 0)
+ - puts the detected screen width in the COLUMNS variable
+ (because rl_change_environment defaults to 1)
+ - may report one less than the detected screen width in
+ rl_get_screen_size (when _rl_term_autowrap == 0).
+ We could set readline_hidden_cols by comparing COLUMNS to cols as
+ returned by rl_get_screen_size, but instead simply use
+ _rl_term_autowrap. */
+ readline_hidden_cols = _rl_term_autowrap ? 0 : 1;
+
lines_per_page = rows;
chars_per_line = cols;
@@ -335,4 +335,11 @@ extern void copy_bitwise (gdb_byte *dest, ULONGEST dest_offset,
const gdb_byte *source, ULONGEST source_offset,
ULONGEST nbits, int bits_big_endian);
+/* When readline decides that the terminal cannot auto-wrap lines, it reduces
+ the width of the reported screen width by 1. This variable indicates
+ whether that's the case or not, allowing us to add it back where
+ necessary. See _rl_term_autowrap in readline/terminal.c. */
+
+extern int readline_hidden_cols;
+
#endif /* UTILS_H */