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

* Re: [PATCH] [gdb/tui] Add set tui hide-last-column on/off/auto
  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
  1 sibling, 1 reply; 5+ messages in thread
From: Andrew Burgess @ 2023-05-10 13:48 UTC (permalink / raw)
  To: Tom de Vries, gdb-patches; +Cc: Tom Tromey

Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> writes:

> 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

I took a look at this patch, and the two bugs gdb/30388 and gdb/30419.
It wasn't clear to me if you were actually proposing this patch for
inclusion based on the comments in gdb/30419 - or if this was more RFC.

But I did have one incredibly minor nit...

> ---
>  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.

... you should mention that 'auto' is the default.

Thanks,
Andrew

> +
>  @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

* Re: [PATCH] [gdb/tui] Add set tui hide-last-column on/off/auto
  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:20 ` Tom Tromey
  2023-05-22 13:45   ` Tom de Vries
  1 sibling, 1 reply; 5+ messages in thread
From: Tom Tromey @ 2023-05-10 15:20 UTC (permalink / raw)
  To: Tom de Vries via Gdb-patches; +Cc: Tom de Vries, Tom Tromey

>>>>> "Tom" == Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> writes:

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

I wonder if it's possible to also set TERMCAP to deal with this.

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

Another option would be to change the test suite to also have the glitch
and to set 'TERM=xterm' or something.

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

So IIUC, the idea here is that sometimes you may want to try out
TERM=ansi interactively, even though your actual terminal is different.
This of course causes problems because TERM doesn't match the
capabilities of the actual terminal.  So then this setting works around
one bug that's exposed?

If that's accurate, and you still want this setting, then perhaps it
should be under "maint".  Ordinary users should not be doing this kind
of thing and, for all we know, there are other incompatibilities that we
just haven't stumbled across yet.

Tom

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] [gdb/tui] Add set tui hide-last-column on/off/auto
  2023-05-10 13:48 ` Andrew Burgess
@ 2023-05-10 15:21   ` Tom de Vries
  0 siblings, 0 replies; 5+ messages in thread
From: Tom de Vries @ 2023-05-10 15:21 UTC (permalink / raw)
  To: Andrew Burgess, gdb-patches; +Cc: Tom Tromey

On 5/10/23 15:48, Andrew Burgess wrote:
> Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
>> 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
> 
> I took a look at this patch, and the two bugs gdb/30388 and gdb/30419.
> It wasn't clear to me if you were actually proposing this patch for
> inclusion based on the comments in gdb/30419 - or if this was more RFC.
> 

Hi Andrew,

thanks for the review.

I've submitted this intentionally as a patch, with the idea that it's a 
concrete solution to a hopefully clearly described problem.

But I understand there could a discussion akin to what you'd have for an 
RFC in terms of: is this a problem that we want to fix, is the solution 
worth the cost of maintenance etc.

Also, it could be suggested that auto is not a good default, and off 
would be better because it does not change current behaviour.  Or, that 
it's better as a maintainance command.

Anyway, if you have comments/question/recommendations on those issues, 
I'm happy to hear about it.

> But I did have one incredibly minor nit...

Yes, thanks for pointing that out.  I'll fix that once we move closer to 
actually committing this, and settle on a default.

Thanks,
- Tom

> 
>> ---
>>   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.
> 
> ... you should mention that 'auto' is the default.
> 
> Thanks,
> Andrew
> 
>> +
>>   @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

* Re: [PATCH] [gdb/tui] Add set tui hide-last-column on/off/auto
  2023-05-10 15:20 ` Tom Tromey
@ 2023-05-22 13:45   ` Tom de Vries
  0 siblings, 0 replies; 5+ messages in thread
From: Tom de Vries @ 2023-05-22 13:45 UTC (permalink / raw)
  To: Tom Tromey, Tom de Vries via Gdb-patches

On 5/10/23 17:20, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Tom> But it's different when we define TERM=ansi, because of the implications for
> Tom> the xenl flag:
> Tom> ...
> Tom> $ TERM=xterm tput xenl; echo $?
> Tom> 0
> Tom> $ TERM=ansi tput xenl; echo $?
> Tom> 1
> Tom> ...
> Tom> which stands for "eat newline glitch".
> 
> I wonder if it's possible to also set TERMCAP to deal with this.
> 
> Tom> At this point, we could just disregard this as a user problem, but there is an
> Tom> interest in having this TERM=ansi working somewhat.  This is because the
> Tom> native TERM for our tuiterm terminal emulator is ansi, and it's worthwhile:
> Tom> - being able to compare behaviour in the tuiterm and an actual terminal
> Tom>   emulator like konsole or gnome-terminal (which typically will have the
> Tom>   glitch), as well as
> Tom> - playing around with the TUI and explore new scenarios with TUI running in a
> Tom>   mode as close as possible to tuiterm to increase the likeliness that the
> Tom>   scenario can be reproduced in the tuiterm.
> 
> Another option would be to change the test suite to also have the glitch
> and to set 'TERM=xterm' or something.
> 

I've given that a try, patch series submitted here ( 
https://sourceware.org/pipermail/gdb-patches/2023-May/199761.html ).

> Tom> So, we expose the fix at user level using a "set tui hide-last-column
> Tom> on/off/auto", with auto behaving like this:
> Tom> ...
> Tom> $ TERM=xterm gdb -q  -ex "show tui hide-last-column"
> Tom> TUI last column hiding is "auto; currently off".
> Tom> $ TERM=ansi gdb -q -ex "show tui hide-last-column"
> Tom> TUI last column hiding is "auto; currently on".
> Tom> ...
> Tom> and use "set tui hide-last-column off" in the testsuite.
> 
> So IIUC, the idea here is that sometimes you may want to try out
> TERM=ansi interactively, even though your actual terminal is different.
> This of course causes problems because TERM doesn't match the
> capabilities of the actual terminal.  So then this setting works around
> one bug that's exposed?
> 
> If that's accurate, and you still want this setting, then perhaps it
> should be under "maint".  Ordinary users should not be doing this kind
> of thing and, for all we know, there are other incompatibilities that we
> just haven't stumbled across yet.

If the patch series with TERM=ansi-for-tui is accepted, then my main 
concern is addressed (reproducible behaviour of TUI between tuiterm and 
regular terminal emulator like xterm), and I don't necessarily care 
anymore about running TUI with TERM=ansi, so I could drop this.

Thanks,
- Tom

^ 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).