From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24230 invoked by alias); 28 Apr 2015 01:08:48 -0000 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 Received: (qmail 24220 invoked by uid 89); 28 Apr 2015 01:08:48 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL,BAYES_00,KAM_LAZY_DOMAIN_SECURITY,RCVD_IN_DNSWL_LOW autolearn=no version=3.3.2 X-HELO: mail-pd0-f174.google.com Received: from mail-pd0-f174.google.com (HELO mail-pd0-f174.google.com) (209.85.192.174) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Tue, 28 Apr 2015 01:08:47 +0000 Received: by pdbqd1 with SMTP id qd1so146504137pdb.2 for ; Mon, 27 Apr 2015 18:08:45 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc:content-type; bh=Y/cyBznOrv4qLduw69vVeRL6Wkf+VCClNTJBBVCUi7Y=; b=kWctUzlPLydovNxRCQt3+k3sw81hOqXps+yQE31bW/r6IXA8fnvrC/3241AFbga34V y31+0AYSy9Aw3/xFSWpOuRdr4zi5FqJnZWSGrtjvxwSqnkW7l5+wNth+vPAC00aNOSZu mnUxJ8S5V/IofpGZh8WepBp8s7yZceVaROU1+o/CHkRKMhvm3hGSLGpByKsPftA4oc1V l602xBZQK05sBpAodvZZijqP8gMuVtnsmgmHA9at4wI3wIY/XfeEnVFDJqCk3N89yj+k Wzwid1efHbzi8BlaidIsnqXgLhyNtoJrMaJXA2qzJALe2nJQi16mI0nSz71Ygho8C6zL ShRw== X-Gm-Message-State: ALoCoQmcwbvhj83fTOkg2azNuvHLLE13OUVfRvZqpcGFJC2Q18XaQEWBLOLJrdyTyi169fOGJdSq X-Received: by 10.70.136.169 with SMTP id qb9mr27268327pdb.46.1430183325567; Mon, 27 Apr 2015 18:08:45 -0700 (PDT) MIME-Version: 1.0 Received: by 10.70.102.99 with HTTP; Mon, 27 Apr 2015 18:08:25 -0700 (PDT) In-Reply-To: <553E6535.2030306@redhat.com> References: <1429836801-14218-1-git-send-email-patrick@parcs.ath.cx> <1429836801-14218-2-git-send-email-patrick@parcs.ath.cx> <553E6535.2030306@redhat.com> From: Patrick Palka Date: Tue, 28 Apr 2015 09:17:00 -0000 Message-ID: Subject: Re: [PATCH 2/3] Update our idea of our terminal's dimensions even outside of TUI To: Pedro Alves Cc: "gdb-patches@sourceware.org" Content-Type: text/plain; charset=UTF-8 X-SW-Source: 2015-04/txt/msg01025.txt.bz2 On Mon, Apr 27, 2015 at 12:35 PM, Pedro Alves wrote: > 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.) Good idea... I'll do this. > >> @@ -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. */ Good point, it's better to explicitly mention that win_resized is left set. > >> + } >> + 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 (); Dang, I totally missed that. I'll post a patch for this too. Just an FYI, you have commented on patches 2/3 and 3/3 but not 1/3. > > Thanks, > Pedro Alves >