Re: [PATCH] Ensure rl_get_screen_size() returns the actual terminal dimensions

[Pedro Alves <palves@redhat.com>]

On 09/04/2014 12:58 PM, Patrick Palka wrote:
>> readline's rl_sigwinch_handler already call rl_resize_terminal, and then
>> chains into the TUI's handler.  So, why do we need this?

> Sometimes we overwrite readline's screen size by calling
> tui_update_gdb_sizes() like done in tui_enable().  When that's done
> rl_get_screen_size() will not return the real terminal dimensions, but
> rather the dimensions that were set by tui_update_gdb_sizes().  So in
> the following scenario the TUI will get resized to the wrong
> dimensions:
> 
> 0. Run GDB.
> 1. Enter TUI mode.
> 2. Exit TUI mode.
> 3. Resize the terminal.
> 4. Enter TUI mode again.
> 5. Press a key so that the TUI will resize itself.
> 
> Here we resize the terminal (causing RL's SIGWINCH handler to run,
> updating RL's idea of the terminal size).  Then we enter TUI mode
> which calls tui_update_gdb_sizes() which resets RL's idea of the
> terminal size.  Then we call rl_get_screen_size() in tui_getc() which
> returns not the terminal dimensions but the dimensions of the TUI's
> command window as set by tui_update_gdb_sizes().  So in the end the
> TUI gets resized to incorrect dimensions.

Hmm, OK, but...  I think the story is just unraveling though.  :-)
E.g., trying with your patch, I do:

0. Run GDB.
1. Enter TUI mode.
2. "show height" -> 15.  Looks legit.
3. Exit TUI mode.
4. "show height" -> 45
5. Resize the terminal to something smaller than 45 lines.
6. "show height" -> still 45 (!?!?)
7. Enter TUI mode again.  I have now about 3 visible lines in my cmd window.
8. "show height" -> 15       (!?!?)
9. Exit TUI mode.
A. "show height" -> _14_     (looks like it's the legit size now.)
B. Enter TUI mode again.  I still still about 3 lines height.
C. "show height" -> _3_      (finally ??)

The screen size handling looks seriously confused.

Hmm, if I instead enter/exit/enter in a row, then the height
looks reasonable, like:

0. Run GDB.
1. "show height" -> 45
2. Resize the terminal to something smaller than 45 lines.
3. Enter TUI mode.
4. Exit TUI mode.
5. Enter TUI mode immediately again.
6. "show height" -> 3.  Looks legit.
7. Exit TUI mode.
8. "show height" -> 14     (looks legit)

I notice we don't really need the TUI to see misbehavior, though:

0. Run GDB.
1. "show height" -> 45
2. Resize the terminal to something smaller than 45 lines.
3. "show height" -> still 45 (!?!?)

Sounds like GDB should have a SIGWINCH handler even if not running
the TUI, that marks the height as invalid, so the next time GDB needs
the height, it's refetched from readline.

> 
> Also, when we are blocking inside a secondary prompt, readline's
> SIGWINCH handler does not seem to trigger for some reason. So when we
> resize the window during a secondary prompt, our SIGWINCH handler gets
> triggered but readline's handler doesn't which means that the
> dimensions returned by rl_get_screen_size() will be wrong.  I'm not
> really sure why this happens.
> 
> Explicitly calling rl_resize_terminal() fixes both of these issues.

So, it actually sounds like we _should_ be calling rl_resize_terminal
to get the full "root" window's sizes, and then _always_ end
up calling tui_update_gdb_sizes -> rl_set_screen_size ?

That, and maybe set rl_catch_sigwinch=1 as it sounds like
readline's signal handler would end up useless.

Thanks,
Pedro Alves