Re: [PATCH 1/3] [gdb/tui] Keep inferior output in cmd window with ^L

[Bruno Larsen <blarsen@redhat.com>]

On 30/05/2023 12:53, Tom de Vries via Gdb-patches wrote:
> Consider a hello world compiled with -g into an a.out:
> ...
> int main (void) {
>    printf ("hello\n");
>    return 0;
> }
> ...
>
> After starting TUI like this:
> ...
> $ gdb -q a.out -ex start -ex "tui enable"
> ...
> we do "echo \n" and type enter until the prompt is at the bottom of the screen.
>
> After doing next to execute the printf, we have:
> ...
> (gdb) next
> hello
> (gdb)
> ...
> but the entire screen scrolled, so:
> - the src window is missing the top border, and
> - the updates in the src and status window are misaligned with the current
>    screen.
>
> We can fix this by doing ^L, but that removes the "hello", giving us just:
> ...
> (gdb) next
> (gdb)
> ...
>
> The ^L key-combo (and likewise the command refresh) calls tui_refresh_all_win,
> which works by:
> - clearing the screen, and then
> - refreshing all visible windows.
> Since curses has no notion of the inferior output, this overwrites the
> "hello".
>
> Fix this by excluding the command window from ^L.
>
> Tested on x86_64-linux.
> ---

Thank you for working on this, this has bugged me for years at this point.

I can confirm that this fixes the issue you mentioned when the terminal 
scrolls. The problem is that now, if the output doesn't cause the screen 
to scroll, it doesn't show up on screen at all.

I also am not a great fan of the test name, I would suggest calling it 
gdb.tui/inferior-stdout-output.exp or something like that, hello sounds 
a little too generic IMO.

-- 
Cheers,
Bruno

>   gdb/testsuite/gdb.tui/hello.c   | 25 +++++++++++++++
>   gdb/testsuite/gdb.tui/hello.exp | 57 +++++++++++++++++++++++++++++++++
>   gdb/tui/tui-io.c                |  2 +-
>   gdb/tui/tui-win.c               | 18 ++++++++---
>   gdb/tui/tui-win.h               |  2 +-
>   gdb/tui/tui-wingeneral.c        |  5 ++-
>   gdb/tui/tui-wingeneral.h        |  2 +-
>   7 files changed, 103 insertions(+), 8 deletions(-)
>   create mode 100644 gdb/testsuite/gdb.tui/hello.c
>   create mode 100644 gdb/testsuite/gdb.tui/hello.exp
>
> diff --git a/gdb/testsuite/gdb.tui/hello.c b/gdb/testsuite/gdb.tui/hello.c
> new file mode 100644
> index 00000000000..c3f5c6ea2cb
> --- /dev/null
> +++ b/gdb/testsuite/gdb.tui/hello.c
> @@ -0,0 +1,25 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   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/>.  */
> +
> +#include <stdio.h>
> +
> +int
> +main (void)
> +{
> +  printf ("hello\n");
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.tui/hello.exp b/gdb/testsuite/gdb.tui/hello.exp
> new file mode 100644
> index 00000000000..9634d9d8059
> --- /dev/null
> +++ b/gdb/testsuite/gdb.tui/hello.exp
> @@ -0,0 +1,57 @@
> +# 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/>.
> +
> +# Check that a print by an inferior doesn't mess up the screen.
> +
> +tuiterm_env
> +
> +standard_testfile hello.c
> +
> +if {[build_executable "failed to prepare" ${testfile} ${srcfile}] == -1} {
> +    return -1
> +}
> +
> +Term::clean_restart 24 80 $testfile
> +
> +if {![runto_main]} {
> +    perror "test suppressed"
> +    return
> +}
> +
> +if {![Term::enter_tui]} {
> +    unsupported "TUI not supported"
> +    return
> +}
> +
> +# Make sure the prompt is at the last line.
> +Term::command "echo \\n"
> +Term::command "echo \\n"
> +Term::command "echo \\n"
> +Term::command "echo \\n"
> +
> +# Check the source box.
> +Term::check_box "before next: source box" 0 0 80 15
> +
> +# Make the inferior print "hello".
> +Term::command "next"
> +
> +# Refresh to redraw the screen.
> +Term::command "refresh"
> +
> +# Check the source box again.
> +Term::check_box "after next: source box" 0 0 80 15
> +
> +# Check that we can still see the printed "hello".
> +Term::check_contents "hello" "\nhello"
> diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c
> index a1eadcd937d..6ae7948b786 100644
> --- a/gdb/tui/tui-io.c
> +++ b/gdb/tui/tui-io.c
> @@ -1219,7 +1219,7 @@ tui_getc_1 (FILE *fp)
>     /* Handle the CTRL-L refresh for each window.  */
>     if (ch == '\f')
>       {
> -      tui_refresh_all_win ();
> +      tui_refresh_all_win (false);
>         return ch;
>       }
>   
> diff --git a/gdb/tui/tui-win.c b/gdb/tui/tui-win.c
> index 7abd1e225b9..3b41b59cdd3 100644
> --- a/gdb/tui/tui-win.c
> +++ b/gdb/tui/tui-win.c
> @@ -508,10 +508,20 @@ tui_win_info::right_scroll (int num_to_scroll)
>   
>   
>   void
> -tui_refresh_all_win (void)
> +tui_refresh_all_win (bool cmd_window)
>   {
> -  clearok (curscr, TRUE);
> -  tui_refresh_all ();
> +  if (cmd_window)
> +    clearok (curscr, TRUE);
> +  else
> +    for (tui_win_info *wi : all_tui_windows ())
> +      {
> +	if (wi == TUI_CMD_WIN)
> +	  continue;
> +
> +	redrawwin (wi->handle.get ());
> +      }
> +
> +  tui_refresh_all (cmd_window);
>   }
>   
>   void
> @@ -827,7 +837,7 @@ tui_refresh_all_command (const char *arg, int from_tty)
>     /* Make sure the curses mode is enabled.  */
>     tui_enable ();
>   
> -  tui_refresh_all_win ();
> +  tui_refresh_all_win (false);
>   }
>   
>   #define DEFAULT_TAB_LEN         8
> diff --git a/gdb/tui/tui-win.h b/gdb/tui/tui-win.h
> index 3d35f1dfb7f..41408a0af3d 100644
> --- a/gdb/tui/tui-win.h
> +++ b/gdb/tui/tui-win.h
> @@ -26,7 +26,7 @@
>   
>   extern void tui_set_win_focus_to (struct tui_win_info *);
>   extern void tui_resize_all (void);
> -extern void tui_refresh_all_win (void);
> +extern void tui_refresh_all_win (bool cmd_win = true);
>   extern void tui_rehighlight_all (void);
>   
>   extern chtype tui_border_ulcorner;
> diff --git a/gdb/tui/tui-wingeneral.c b/gdb/tui/tui-wingeneral.c
> index 82a023d09fe..73c047a8ac4 100644
> --- a/gdb/tui/tui-wingeneral.c
> +++ b/gdb/tui/tui-wingeneral.c
> @@ -198,10 +198,13 @@ tui_win_info::make_visible (bool visible)
>   /* Function to refresh all the windows currently displayed.  */
>   
>   void
> -tui_refresh_all ()
> +tui_refresh_all (bool cmd_window)
>   {
>     for (tui_win_info *win_info : all_tui_windows ())
>       {
> +      if (!cmd_window && win_info == tui_win_list[CMD_WIN])
> +	continue;
> +
>         if (win_info->is_visible ())
>   	win_info->refresh_window ();
>       }
> diff --git a/gdb/tui/tui-wingeneral.h b/gdb/tui/tui-wingeneral.h
> index 5b82fcd6b21..051dbf3b5ca 100644
> --- a/gdb/tui/tui-wingeneral.h
> +++ b/gdb/tui/tui-wingeneral.h
> @@ -28,7 +28,7 @@ struct tui_win_info;
>   
>   extern void tui_unhighlight_win (struct tui_win_info *);
>   extern void tui_highlight_win (struct tui_win_info *);
> -extern void tui_refresh_all ();
> +extern void tui_refresh_all (bool cmd_window = true);
>   
>   /* An RAII class that suppresses output on construction (calling
>      wnoutrefresh on the existing windows), and then flushes the output