This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 2/3] Update our idea of our terminal's dimensions even outside of TUI
- From: Pedro Alves <palves at redhat dot com>
- To: Patrick Palka <patrick at parcs dot ath dot cx>, gdb-patches at sourceware dot org
- Date: Mon, 27 Apr 2015 17:35:01 +0100
- Subject: Re: [PATCH 2/3] Update our idea of our terminal's dimensions even outside of TUI
- Authentication-results: sourceware.org; auth=none
- References: <1429836801-14218-1-git-send-email-patrick at parcs dot ath dot cx> <1429836801-14218-2-git-send-email-patrick at parcs dot ath dot cx>
On 04/24/2015 01:53 AM, Patrick Palka wrote:
> When in the CLI, GDB's "width" and "height" variables are not kept in sync
> when the underlying terminal gets resized.
>
> This patch fixes this issue by making sure sure to update GDB's "width"
> and "height" variables in the !tui_active case of our SIGWINCH handler.
>
> gdb/ChangeLog:
>
> * tui/tui-win.c (tui_sigwinch_handler): Remove now-stale comment.
> (tui_sigwinch_handler): Still update our idea of
> the terminal's width and height even when TUI is not active.
(A next step for a rainy day would be to more the signal handler to
core code, and make this work even when the TUI is not compiled in.)
> @@ -850,15 +844,26 @@ tui_sigwinch_handler (int signal)
> static void
> tui_async_resize_screen (gdb_client_data arg)
> {
> - if (!tui_active)
> - return;
> -
> rl_resize_terminal ();
> - tui_resize_all ();
> - tui_refresh_all_win ();
> - tui_update_gdb_sizes ();
> - tui_set_win_resized_to (FALSE);
> - tui_redisplay_readline ();
> +
> + if (!tui_active)
> + {
> + int screen_height, screen_width;
> +
> + rl_get_screen_size (&screen_height, &screen_width);
> + set_screen_width_and_height (screen_width, screen_height);
> +
> + /* win_resized will be untoggled and the windows resized in the next call
> + to tui_enable(). */
Hmm, can we please avoid "untoggle"? I'm not a native speaker, but it
game me pause, as assuming toggle is x^=1, then untoggle is x^=1 too,
thus toggle==untoggle. :-) I'd rather use "unset".
OK with that change.
FWIW, the comment gives a bit of pause. I'd suggest something like this
instead:
/* win_resized is left set so that the next call to tui_enable
resizes windows. */
> + }
> + else
> + {
> + tui_resize_all ();
> + tui_refresh_all_win ();
> + tui_update_gdb_sizes ();
> + tui_set_win_resized_to (FALSE);
> + tui_redisplay_readline ();
A preexisting problem (thus not be fixed by this patch), but
I note that this seems racy. If another SIGWINCH comes in after
between tui_resize_all and tui_set_win_resized_to, we'll clear
tui_set_win_resized_to and lose the need to resize in tui_enable:
/* Resize windows before anything might display/refresh a
window. */
if (tui_win_resized ())
{
tui_resize_all ();
tui_set_win_resized_to (FALSE);
}
That's why the mainline code that handles a signal should always clear
the signal handler's control variable before actually reacting to
it, like:
tui_set_win_resized_to (FALSE);
tui_resize_all ();
tui_refresh_all_win ();
tui_update_gdb_sizes ();
tui_redisplay_readline ();
Thanks,
Pedro Alves