From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 18686 invoked by alias); 20 Jul 2010 21:15:58 -0000 Received: (qmail 18674 invoked by uid 22791); 20 Jul 2010 21:15:56 -0000 X-SWARE-Spam-Status: No, hits=-1.7 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM X-Spam-Check-By: sourceware.org Received: from mail-bw0-f41.google.com (HELO mail-bw0-f41.google.com) (209.85.214.41) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 20 Jul 2010 21:15:49 +0000 Received: by bwz9 with SMTP id 9so4031253bwz.0 for ; Tue, 20 Jul 2010 14:15:46 -0700 (PDT) MIME-Version: 1.0 Received: by 10.204.49.84 with SMTP id u20mr5548157bkf.185.1279660546297; Tue, 20 Jul 2010 14:15:46 -0700 (PDT) Received: by 10.204.119.2 with HTTP; Tue, 20 Jul 2010 14:15:46 -0700 (PDT) In-Reply-To: <201007201642.37113.pedro@codesourcery.com> References: <201007201642.37113.pedro@codesourcery.com> Date: Tue, 20 Jul 2010 21:15:00 -0000 Message-ID: Subject: Re: [patch] Slightly better resize support in TUI mode From: Balazs Kezes To: Pedro Alves Cc: gdb-patches@sourceware.org Content-Type: text/plain; charset=ISO-8859-1 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2010-07/txt/msg00320.txt.bz2 > 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. Done (I've sent a mail to the readline guys). > 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. You are right. Actually there is a function which reacts to that flag and it is called tui_handle_resize_during_io (I've missed this somehow). But we need to add a call to tui_resize_all there. I think in tui_resize_all we shouldn't be using rl_get_screen_size for getting the screen size. Later in that function we call tui_update_gdb_size which also tells readline to update its screen size to command window's size. When I called tui_resize_all in it was just after readline's sigwinch handler which determined the correct sizes for the terminal so I could use that data to calculate the new sizes for the windows. But when I do this delayed handling it doesn't work well because, I think, we can't rely on readline for correct terminal size. In short I think the line rl_get_screen_size (&screenheight, &screenwidth); should be replaced to something else. I'll look into this tomorrow evening whether this is the source of the problem. -- Balazs On 7/20/10, Pedro Alves wrote: > 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): > > 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 >