[PATCH][gdb/tui] Add tui auto-refresh-screen on/off

[Tom de Vries <tdevries@suse.de>]

[-- Attachment #1: Type: text/plain, Size: 173 bytes --]

[was: Re: [RFC][gdb/tui] Add tui auto-refresh-screen on/off ]

Changes:
- added docs entry
- added test-case
- set off by default in testsuite

Any comments?

Thanks,
- Tom

[-- Attachment #2: 0001-gdb-tui-Add-tui-auto-refresh-screen-on-off.patch --]
[-- Type: text/x-patch, Size: 12103 bytes --]

[gdb/tui] Add tui auto-refresh-screen on/off

As reported in PR28482, the tui screen may become garbled.

A common workaround is to do ^L (or equivalently,  issue the refresh
command).

There are two drawbacks to this workaround:
- you need to known it in order to be able to use it.  A new user that
  doesn't know about the workaround may well take having a garbled screen
  as an indication of software maturity, and decide to not invest further.
- the workaround is so frequently used that people have started to try to
  automate the workaround outside gdb [1].

Introduce a new tui on/off variable auto-refresh-screen that automatically
issues the refresh command when displaying the prompt, which fixes the problem
observed in PR28482.  It may be possible that this needs to be done at more
locations.

A drawback of automating the refresh is that it may cause (more) flickering
(I haven't observed that, but I imagine it could).  This is mentioned in the
help text.

The default setting is on.

The setting is switched off by default in the testsuite, because we want
don't want to paper over issues during testing.

Build on x86_64-linux, tested with all .exp test-cases that contain
tuiterm_env.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28482

[1] https://stackoverflow.com/questions/38803783/how-to-automatically-refresh-gdb-in-tui-mode

---
 gdb/doc/gdb.texinfo             | 10 ++++-
 gdb/testsuite/gdb.tui/hello.c   | 26 ++++++++++++
 gdb/testsuite/gdb.tui/hello.exp | 88 +++++++++++++++++++++++++++++++++++++++++
 gdb/testsuite/lib/tuiterm.exp   | 56 ++++++++++++++++++++++++--
 gdb/tui/tui-hooks.c             |  2 +
 gdb/tui/tui-win.c               | 31 +++++++++++++++
 gdb/tui/tui-win.h               |  4 ++
 7 files changed, 213 insertions(+), 4 deletions(-)

diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
index 631a7c03b31..cb42effd076 100644
--- a/gdb/doc/gdb.texinfo
+++ b/gdb/doc/gdb.texinfo
@@ -28862,7 +28862,9 @@ Make the command window active for scrolling.
 
 @item refresh
 @kindex refresh
-Refresh the screen.  This is similar to typing @kbd{C-L}.
+@anchor{refresh}
+Refresh the screen.  This is similar to typing @kbd{C-L}.  See also
+@ref{auto-refresh-screen}.
 
 @item tui reg @var{group}
 @kindex tui reg
@@ -28973,6 +28975,12 @@ The default display uses more space for line numbers and starts the
 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 auto-refresh-screen @r{[}on@r{|}off@r{]}
+@kindex set tui auto-refresh-screen
+@anchor{auto-refresh-screen}
+Control whether the TUI screen is refreshed automatically.  This may
+cause (more) flickering.  Defaults to on.  See also @ref{refresh}.
 @end table
 
 Note that the colors of the TUI borders can be controlled using the
diff --git a/gdb/testsuite/gdb.tui/hello.c b/gdb/testsuite/gdb.tui/hello.c
new file mode 100644
index 00000000000..3c71310da36
--- /dev/null
+++ b/gdb/testsuite/gdb.tui/hello.c
@@ -0,0 +1,26 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2021 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/>.  */
+
+#include <stdio.h>
+
+int
+main ()
+{
+  printf ("Hello");	/* First printf.  */
+  printf (" World\n");  /* Second printf.  */
+  return 0;		/* Return from main.  */
+}
diff --git a/gdb/testsuite/gdb.tui/hello.exp b/gdb/testsuite/gdb.tui/hello.exp
new file mode 100644
index 00000000000..e76975d7ebd
--- /dev/null
+++ b/gdb/testsuite/gdb.tui/hello.exp
@@ -0,0 +1,88 @@
+# Copyright 2021 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/>.
+
+# Verify that breakpoint marker is shown.
+
+tuiterm_env
+
+standard_testfile
+
+if {[prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] == -1} {
+    return -1
+}
+
+# Only run on native boards.
+if { [use_gdb_stub] || [target_info gdb_protocol] == "extended-remote" } {
+    untested "not supported"
+    return
+}
+
+# Get line numbers.
+set first_printf [gdb_get_line_number "First printf"]
+set second_printf [gdb_get_line_number "Second printf"]
+set return_from_main [gdb_get_line_number "Return from main"]
+
+# Check that these are subsequent lines.
+gdb_assert { $second_printf == [expr $first_printf + 1] }
+gdb_assert { $return_from_main == [expr $second_printf + 1] }
+
+foreach_with_prefix auto-refresh-screen { off on } {
+    Term::clean_restart 24 80 $testfile
+
+    gdb_test_no_output "set tui auto-refresh-screen ${auto-refresh-screen}"
+
+    if {![Term::enter_tui]} {
+	unsupported "TUI not supported"
+	return
+    }
+
+    set text [Term::get_all_lines]
+    gdb_assert {![string match "No Source Available" $text]} \
+	"initial source listing"
+
+    Term::command "break main"
+    Term::check_contents "Breakpoint set" "Breakpoint 1 at"
+
+    Term::command "run"
+    Term::check_contents "Run to main" \
+	[join \
+	     [list \
+		  "\\|B\\+>    $first_printf \[^\r\n\]*" \
+		  "\\|       $second_printf \[^\r\n\]*" \
+		  "\\|       $return_from_main" \
+		 ] "\n"]
+
+    Term::command "next"
+    Term::check_contents "after first next" \
+	[join \
+	     [list \
+		  "\\|B\\+     $first_printf \[^\r\n\]*" \
+		  "\\|  >    $second_printf \[^\r\n\]*" \
+		  "\\|       $return_from_main" \
+		 ] "\n"]
+
+    Term::allow_scroll 1
+    Term::command "next"
+    if { ${auto-refresh-screen} == "off" } {
+	setup_kfail tui/14332 *-*-*
+    }
+    Term::check_contents "after second next" \
+	[join \
+	     [list \
+		  "\\|B\\+     $first_printf \[^\r\n\]*" \
+		  "\\|       $second_printf \[^\r\n\]*" \
+		  "\\|  >    $return_from_main" \
+		 ] "\n"]
+}
diff --git a/gdb/testsuite/lib/tuiterm.exp b/gdb/testsuite/lib/tuiterm.exp
index 8ab8d07b545..d6cbc734b8e 100644
--- a/gdb/testsuite/lib/tuiterm.exp
+++ b/gdb/testsuite/lib/tuiterm.exp
@@ -33,6 +33,9 @@ namespace eval Term {
 
     variable _resize_count
 
+    # Whether we should error out on scrolling or not.
+    variable _allow_scroll
+
     proc _log { what } {
 	verbose "+++ $what"
     }
@@ -70,11 +73,17 @@ namespace eval Term {
 	}
     }
 
+    # Erase line Y.
+    proc _clear_line {y} {
+	variable _cols
+	_clear_in_line 0 $_cols $y
+    }
+
     # Erase the lines from SY to just before EY.
     proc _clear_lines {sy ey} {
 	variable _cols
 	while {$sy < $ey} {
-	    _clear_in_line 0 $_cols $sy
+	    _clear_line $sy
 	    incr sy
 	}
     }
@@ -102,6 +111,41 @@ namespace eval Term {
 	}
     }
 
+    # Do scroll:
+    # - move lines one up, discarding first line
+    # - add empty line.
+    proc _scroll {} {
+	variable _allow_scroll
+	variable _rows
+	variable _cols
+	variable _chars
+	variable _cur_row
+
+	if { ! $_allow_scroll } {
+	    error "FIXME scroll"
+	}
+
+	# Move lines up.
+	for {set i 1} {$i < $_rows} {incr i} {
+	    for {set j 0} {$j < $_cols} {incr j} {
+		set _chars($j,[expr $i - 1]) $_chars($j,$i)
+	    }
+	}
+
+	# Add empty line.
+	set last_line [expr $_rows - 1]
+	_clear_line $last_line
+
+	# Update _cur_row to stay within the screen.
+	incr _cur_row -1
+    }
+
+    # Set scroll allowed on/off.
+    proc allow_scroll { arg } {
+	variable _allow_scroll
+	set _allow_scroll $arg
+    }
+
     # Linefeed.
     proc _ctl_0x0a {} {
 	_log_cur "Line feed" {
@@ -110,7 +154,7 @@ namespace eval Term {
 
 	    incr _cur_row 1
 	    if {$_cur_row >= $_rows} {
-		error "FIXME scroll"
+		_scroll
 	    }
 	}
     }
@@ -484,7 +528,7 @@ namespace eval Term {
 			set _cur_col 0
 			incr _cur_row
 			if {$_cur_row >= $_rows} {
-			    error "FIXME scroll"
+			    _scroll
 			}
 		    }
 		}
@@ -503,12 +547,14 @@ namespace eval Term {
 	variable _cur_row
 	variable _attrs
 	variable _resize_count
+	variable _allow_scroll
 
 	set _rows $rows
 	set _cols $cols
 	set _cur_col 0
 	set _cur_row 0
 	set _resize_count 0
+	set _allow_scroll 0
 	array set _attrs {
 	    intensity normal
 	    fg default
@@ -596,6 +642,10 @@ namespace eval Term {
 		::clean_restart $executable
 	    }
 	    ::gdb_test_no_output "set pagination off"
+	    # This is a workaround, and we should aim to have all tests
+	    # passing without it.  So, disable by default.
+	    ::gdb_test_no_output "set tui auto-refresh-screen off" \
+		"set tui auto-refresh-screen off by default"
 	}
     }
 
diff --git a/gdb/tui/tui-hooks.c b/gdb/tui/tui-hooks.c
index 52519aecf17..359efa39901 100644
--- a/gdb/tui/tui-hooks.c
+++ b/gdb/tui/tui-hooks.c
@@ -185,6 +185,8 @@ tui_before_prompt (const char *current_gdb_prompt)
   tui_refresh_frame_and_register_information ();
   from_stack = false;
   from_source_symtab = false;
+  if (auto_refresh_screen)
+    tui_refresh_all_win ();
 }
 
 /* Observer for the normal_stop notification.  */
diff --git a/gdb/tui/tui-win.c b/gdb/tui/tui-win.c
index a51e7b9f6da..987cecbc7f6 100644
--- a/gdb/tui/tui-win.c
+++ b/gdb/tui/tui-win.c
@@ -975,6 +975,25 @@ parse_scrolling_args (const char *arg,
     }
 }
 
+bool auto_refresh_screen = true;
+
+/* Callback for "set tui auto-refresh-screen".  */
+
+static void
+tui_set_auto_refresh_screen (const char *ignore, int from_tty,
+			     struct cmd_list_element *c)
+{
+}
+
+/* Callback for "show tui auto-refresh-screen".  */
+
+static void
+tui_show_auto_refresh_screen (struct ui_file *file, int from_tty,
+			      struct cmd_list_element *c, const char *value)
+{
+  printf_filtered (_("TUI auto-refresh-screen is %s.\n"), value);
+}
+
 /* Function to initialize gdb commands, for tui window
    manipulation.  */
 
@@ -1114,6 +1133,18 @@ the line numbers and uses less horizontal space."),
 			   tui_set_compact_source, tui_show_compact_source,
 			   &tui_setlist, &tui_showlist);
 
+  add_setshow_boolean_cmd ("auto-refresh-screen", class_tui,
+			   &auto_refresh_screen, _("\
+Enable/disable TUI auto-refresh-screen."), _("\
+Show whether TUI auto-refresh-screen is enabled/disabled."), _("\
+In some cases, the TUI screen may become garbled, which can be fixed\n\
+by doing ^L or issuing the refresh command.  This variable controls\n\
+whether the TUI screen is refreshed automatically.  This may cause\n\
+more flickering.  Defaults to on."),
+			   tui_set_auto_refresh_screen,
+			   tui_show_auto_refresh_screen,
+			   &tui_setlist, &tui_showlist);
+
   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 e9da9b98477..9bac6ec16a3 100644
--- a/gdb/tui/tui-win.h
+++ b/gdb/tui/tui-win.h
@@ -51,4 +51,8 @@ struct cmd_list_element **tui_get_cmd_list (void);
 /* Whether compact source display should be used.  */
 extern bool compact_source;
 
+/* Whether tui should automatically refresh the screen to reduce screen
+   garbling.  */
+extern bool auto_refresh_screen;
+
 #endif /* TUI_TUI_WIN_H */