Re: [patch] Slightly better resize support in TUI mode

[Pedro Alves <pedro at codesourcery dot com>]

Thanks for taking the time to work on the TUI.  I personally
appreciate it, being a TUI user myself.

On Monday 19 July 2010 22:20:38, Balazs Kezes wrote:

> First of all we need to make sure that readline works with correct screen
> sizes. When readline is not echoing it doesn't update its internal variables
> about the screen size. This is an issue because TUI uses curses functions to
> echo a character back so readline is in noecho mode. Here's the patch for that:
> 
> Changelog
> 
> 2010-07-19 Balazs Kezes (rlblaster@gmail.com)
> 
> 	* terminal.c (rl_resize_terminal): Make sure terminal size is updated
> 	even when readline is not echoing.

We don't maintain readline ourselves (although we do have a few local
patches, we much prefer not carrying them).  Could you check
whether this code has changed in more recent readline's than the one
we have in our tree, and submit this to readline upstream if not?
(we were recently talking about updating readline right after 7.2 branches,
and the the branching already happened, though the update hasn't yet).  In
any case, we also support building gdb against the system readline,
which again suggests that it is much better to have the fix upstream.

> The other issue is that 

(...)

(it is usually better to have each separate problem, and especially,
each patch submitted as a separate email.)

> in TUI's SIGWINCH handler the resize code is not
> called at all. In the resize code you need to call resize_term which is
> ncurses function in order to update its internal structures. This was placed
> into a #ifdef HAVE_RESIZE_TERM but I have not found any references to that
> macro so I removed the it. Also it seems to me that wresize is not called for
> the command window so I added a call to it too.

It sounds like the intent was to add an autoconf check for resize_term (so
that HAVE_RESIZE_TERM would appear in build/gdb/config.h):
  <http://linux.die.net/man/3/resize_term>
mentions that this is an extension.

> 
> *************** tui_sigwinch_handler (int signal)
> *** 817,822 ****
> --- 820,831 ----
>     /* Say that a resize was done so that the readline can do it later
>        when appropriate.  */
>     tui_set_win_resized_to (TRUE);
> +
> +   if (tui_active)
> +     {
> +       tui_resize_all ();
> +       tui_refresh_all_win ();
> +     }
>   }
>   #endif
> 

It's best to avoid doing non async signal safe work in the signal
handler.  Instead, set a global flag, and have the main code
handle the resize.  This is what the tui_set_win_resized_to
call shown above is doing -- setting the flag.  But, it sounds
like in this scenario, nobody's reacting to the flag.  That's
what we should fix.

-- 
Pedro Alves