[PATCH] Fix a pair of screen-resizing issues in TUI

[PATCH] Fix a pair of screen-resizing issues in TUI

[Patrick Palka]

This patch fixes a pair of TUI issues related to screen resizing:

1. In tui_handle_resize_during_io(), when the TUI screen gets resized,
we fail to update GDB's idea about the height of the output window.

You can see this bug by doing:

  a. Enter TUI mode.
  b. "show height"
  c. Resize the terminal.
  d. "show height"

And observe that despite resizing the terminal, the reported height
remains unchanged.  Note that a similar issue exists in the CLI.

The fix for this is simple: call tui_update_gdb_sizes() after performing
a resize, so that the "height" variable remains consistent with the
height of TUI's output window.

2. In tui_enable(), the call to tui_update_gdb_sizes() may clobber
readline's idea of the actual screen dimensions, and a subsequent
pending resize will use bogus terminal dimensions.

You can see this bug by doing:

  a. Enter TUI mode.
  b. Exit TUI mode.
  c. Resize the terminal.
  d. Enter TUI mode.
  e. Press a key to resize the screen.

And observe that the terminal gets incorrectly resized to the wrong
dimensions.  To fix this issue, we should oppurtunistically resize the
screen in tui_enable().  That way we eliminate the possibility of a
pending resize triggering right after we call tui_update_gdb_sizes().

gdb/ChangeLog:

	* tui/tui-io.c (tui_handle_resize_during_io): Call
	tui_update_gdb_sizes() after resizing the screen.
	* tui/tui.c (tui_enable): Resize the terminal before
	calling tui_update_gdb_sizes().
---
 gdb/tui/tui-io.c | 1 +
 gdb/tui/tui.c    | 7 +++++++
 2 files changed, 8 insertions(+)

diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c
index 94c3ec2..fccada5 100644
--- a/gdb/tui/tui-io.c
+++ b/gdb/tui/tui-io.c
@@ -767,6 +767,7 @@ tui_handle_resize_during_io (void)
     {
       tui_resize_all ();
       tui_refresh_all_win ();
+      tui_update_gdb_sizes ();
       tui_set_win_resized_to (FALSE);
     }
 }
diff --git a/gdb/tui/tui.c b/gdb/tui/tui.c
index 00e505d..53eb6d2 100644
--- a/gdb/tui/tui.c
+++ b/gdb/tui/tui.c
@@ -483,6 +483,13 @@ tui_enable (void)
 
   /* Restore TUI keymap.  */
   tui_set_key_mode (tui_current_key_mode);
+
+  /* Resize and refresh the screen.  */
+  if (tui_win_resized ())
+    {
+      tui_resize_all ();
+      tui_set_win_resized_to (FALSE);
+    }
   tui_refresh_all_win ();
 
   /* Update gdb's knowledge of its terminal.  */
-- 
2.2.1.212.gc5b9256

[PATCH] Fix truncation of TUI command history

[Patrick Palka]

If we submit a command while the prompt cursor is somewhere other than
at the end of the command line, the command line gets truncated as the
command window gets shifted one line up.  This happens because we fail
to properly move the cursor to the end of the command line before
transmitting the newline to ncurses.  We need to move the cursor because
when ncurses outputs a newline it truncates any text that appears
past the end of the cursor.

The fix is generic enough to work properly even in multi-line secondary
prompts like the quit prompt.

gdb/ChangeLog:

	* tui/tui-io.c (tui_getc): Move cursor to the end of the command
	line before printing a newline.
---
 gdb/tui/tui-io.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c
index 73dbcfc..94c3ec2 100644
--- a/gdb/tui/tui-io.c
+++ b/gdb/tui/tui-io.c
@@ -708,9 +708,15 @@ tui_getc (FILE *fp)
         }
       else
         {
-          wmove (w, TUI_CMD_WIN->detail.command_info.cur_line,
-                 TUI_CMD_WIN->detail.command_info.curch);
-          waddch (w, ch);
+	  /* Move cursor to the end of the command line before emitting the
+	     newline.  */
+	  int px = TUI_CMD_WIN->detail.command_info.curch;
+	  int py = TUI_CMD_WIN->detail.command_info.cur_line;
+	  px += rl_end - rl_point;
+	  py += px / TUI_CMD_WIN->generic.width;
+	  px %= TUI_CMD_WIN->generic.width;
+	  wmove (w, py, px);
+	  waddch (w, ch);
         }
     }
   
-- 
2.2.1.212.gc5b9256

Re: [PATCH] Fix truncation of TUI command history

[Patrick Palka]

This is an improved variant of the patch here
https://sourceware.org/ml/gdb-patches/2014-09/msg00109.html that also
works properly for secondary prompts (and overlong lines).

Re: [PATCH] Fix truncation of TUI command history

[Patrick Palka]

Ping.

Re: [PATCH] Fix truncation of TUI command history

[Patrick Palka]

Ping.

Re: [PATCH] Fix truncation of TUI command history

[Pedro Alves]

On 01/10/2015 03:49 AM, Patrick Palka wrote:

> We need to move the cursor because
> when ncurses outputs a newline it truncates any text that appears
> past the end of the cursor.

Can you merge this sentence into the comment in the code?

>        else
>          {
> -          wmove (w, TUI_CMD_WIN->detail.command_info.cur_line,
> -                 TUI_CMD_WIN->detail.command_info.curch);
> -          waddch (w, ch);
> +	  /* Move cursor to the end of the command line before emitting the
> +	     newline.  */

... as this as is doesn't explain _why_ we need to do that, which
may make the reader wonder.

> +	  int px = TUI_CMD_WIN->detail.command_info.curch;
> +	  int py = TUI_CMD_WIN->detail.command_info.cur_line;
> +	  px += rl_end - rl_point;
> +	  py += px / TUI_CMD_WIN->generic.width;
> +	  px %= TUI_CMD_WIN->generic.width;
> +	  wmove (w, py, px);
> +	  waddch (w, ch);
>          }
>      }
>    
> 

OK with that change.  Thanks!

Thanks,
Pedro Alves

Re: [PATCH] Fix a pair of screen-resizing issues in TUI

[Patrick Palka]

Note that the fix for issue #2 is a more direct variant of
https://sourceware.org/ml/gdb-patches/2014-09/msg00004.html

And that fix #1 addresses (at least on the TUI side) some of the "show
height" issues that Pedro raised in the same thread here
https://sourceware.org/ml/gdb-patches/2014-09/msg00127.html

Re: [PATCH] Fix a pair of screen-resizing issues in TUI

[Patrick Palka]

Ping.

Re: [PATCH] Fix a pair of screen-resizing issues in TUI

[Patrick Palka]

Ping.

Re: [PATCH] Fix a pair of screen-resizing issues in TUI

[Pedro Alves]

On 01/10/2015 03:49 AM, Patrick Palka wrote:
> This patch fixes a pair of TUI issues related to screen resizing:
> 
> 1. In tui_handle_resize_during_io(), when the TUI screen gets resized,
> we fail to update GDB's idea about the height of the output window.
> 
> You can see this bug by doing:
> 
>   a. Enter TUI mode.
>   b. "show height"
>   c. Resize the terminal.
>   d. "show height"
> 
> And observe that despite resizing the terminal, the reported height
> remains unchanged.  Note that a similar issue exists in the CLI.
> 
> The fix for this is simple: call tui_update_gdb_sizes() after performing
> a resize, so that the "height" variable remains consistent with the
> height of TUI's output window.
> 
> 2. In tui_enable(), the call to tui_update_gdb_sizes() may clobber
> readline's idea of the actual screen dimensions, and a subsequent
> pending resize will use bogus terminal dimensions.
> 
> You can see this bug by doing:
> 
>   a. Enter TUI mode.
>   b. Exit TUI mode.
>   c. Resize the terminal.
>   d. Enter TUI mode.
>   e. Press a key to resize the screen.
> 
> And observe that the terminal gets incorrectly resized to the wrong
> dimensions.  To fix this issue, we should oppurtunistically resize the
> screen in tui_enable().  That way we eliminate the possibility of a
> pending resize triggering right after we call tui_update_gdb_sizes().
> 
> gdb/ChangeLog:
> 
> 	* tui/tui-io.c (tui_handle_resize_during_io): Call
> 	tui_update_gdb_sizes() after resizing the screen.
> 	* tui/tui.c (tui_enable): Resize the terminal before
> 	calling tui_update_gdb_sizes().

OK.

Thanks!

-- 
Pedro Alves

[PATCH] Fix truncation of TUI command history

[Patrick Palka]

If we submit a command while the prompt cursor is somewhere other than
at the end of the command line, the command line gets truncated as the
command window gets shifted one line up.  This happens because we fail
to properly move the cursor to the end of the command line before
transmitting the newline to ncurses.  We need to move the cursor because
when ncurses outputs a newline it truncates any text that appears
past the end of the cursor.

	* tui/tui-io.c (tui_getc): Move cursor to the end of the command
	line before printing a newline.
---
 gdb/tui/tui-io.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/gdb/tui/tui-io.c b/gdb/tui/tui-io.c
index aa14790..eee5f1ac6 100644
--- a/gdb/tui/tui-io.c
+++ b/gdb/tui/tui-io.c
@@ -678,8 +678,9 @@ tui_getc (FILE *fp)
         }
       else
         {
+          /* Move cursor to the end of the command line.  */
           wmove (w, TUI_CMD_WIN->detail.command_info.cur_line,
-                 TUI_CMD_WIN->detail.command_info.curch);
+		 strlen (tui_rl_saved_prompt) + rl_end);
           waddch (w, ch);
         }
     }
-- 
2.1.0

Re: [PATCH] Fix truncation of TUI command history

[Pedro Alves]

Hi Patrick,

On 08/30/2014 07:11 PM, Patrick Palka wrote:
> index aa14790..eee5f1ac6 100644
> --- a/gdb/tui/tui-io.c
> +++ b/gdb/tui/tui-io.c
> @@ -678,8 +678,9 @@ tui_getc (FILE *fp)
>          }
>        else
>          {
> +          /* Move cursor to the end of the command line.  */
>            wmove (w, TUI_CMD_WIN->detail.command_info.cur_line,
> -                 TUI_CMD_WIN->detail.command_info.curch);
> +		 strlen (tui_rl_saved_prompt) + rl_end);
>            waddch (w, ch);

Does this still do the right thing with secondary prompts?

E.g., if you do "quit" while a program is running, and then
press enter a few times just to have the query repeat (and
then pagination kick in, for extra stress).

Thanks,
Pedro Alves

Re: [PATCH] Fix truncation of TUI command history

[Patrick Palka]

On Thu, Sep 4, 2014 at 5:44 AM, Pedro Alves <palves@redhat.com> wrote:
> Hi Patrick,
>
> On 08/30/2014 07:11 PM, Patrick Palka wrote:
>> index aa14790..eee5f1ac6 100644
>> --- a/gdb/tui/tui-io.c
>> +++ b/gdb/tui/tui-io.c
>> @@ -678,8 +678,9 @@ tui_getc (FILE *fp)
>>          }
>>        else
>>          {
>> +          /* Move cursor to the end of the command line.  */
>>            wmove (w, TUI_CMD_WIN->detail.command_info.cur_line,
>> -                 TUI_CMD_WIN->detail.command_info.curch);
>> +              strlen (tui_rl_saved_prompt) + rl_end);
>>            waddch (w, ch);
>
> Does this still do the right thing with secondary prompts?
>
> E.g., if you do "quit" while a program is running, and then
> press enter a few times just to have the query repeat (and
> then pagination kick in, for extra stress).
>
> Thanks,
> Pedro Alves
>

Good point.  It doesn't work correctly in this case.  I'll try to find
a better solution.