From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25165 invoked by alias); 17 Feb 2015 13:24:24 -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 25149 invoked by uid 89); 17 Feb 2015 13:24:24 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.5 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.2 X-HELO: mail-pa0-f50.google.com Received: from mail-pa0-f50.google.com (HELO mail-pa0-f50.google.com) (209.85.220.50) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Tue, 17 Feb 2015 13:24:23 +0000 Received: by pabkx10 with SMTP id kx10so6299198pab.13 for ; Tue, 17 Feb 2015 05:24:21 -0800 (PST) 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=Z5wcOcxzleUXLt7gufvO9OJf6D0QK/eiiNwNdhlW1jQ=; b=AV3UI2ndR+cdjKLU05mtYmNqEA3x61uXg11lS2jkLYETYamEluRFJ56ONxZanuDDxr 2qkJKHzM+zuJDV1ILToaS27NB85bE+ETQLrUjMvd4LJ6N+KXdul2TKTRY9PITARNEiOs DFj/eeSdZ45mvbNHY2vl3m0VxWAZJrwgnT4QCb+PnMhEB6vxxA9ZSYvDyutBODfVZbkO gIZEQmwoJZWnbxQtDR02llz6PsCW/VRVkOVACz6JMcdBdkvhAlSbKzUI1QZz9oMKBb4C MLwWus1UJJKH21nUUtEjMcVcwH1bZY8oMD8h4WH0PJgOHzV4Waq+/bV/ou7fMsjYhen8 wKkg== X-Gm-Message-State: ALoCoQm020orfa8P9DNf8cIac2UHsJ5l4ZNMlngjZBwe5A9BlD7COVhkm/wc+ag4NdBFTnAGGoKv X-Received: by 10.68.90.4 with SMTP id bs4mr49347574pbb.151.1424179461258; Tue, 17 Feb 2015 05:24:21 -0800 (PST) MIME-Version: 1.0 Received: by 10.70.127.195 with HTTP; Tue, 17 Feb 2015 05:24:01 -0800 (PST) In-Reply-To: <54E316A8.9070705@redhat.com> References: <1420689885-31156-1-git-send-email-patrick@parcs.ath.cx> <1420689885-31156-2-git-send-email-patrick@parcs.ath.cx> <54AE6A19.9070901@redhat.com> <54AE848B.1050606@redhat.com> <54E27241.6010302@redhat.com> <54E316A8.9070705@redhat.com> From: Patrick Palka Date: Tue, 17 Feb 2015 13:24:00 -0000 Message-ID: Subject: Re: [PATCH 2/3] TUI: Don't print KEY_RESIZE keys To: Pedro Alves Cc: "gdb-patches@sourceware.org" Content-Type: text/plain; charset=UTF-8 X-SW-Source: 2015-02/txt/msg00437.txt.bz2 On Tue, Feb 17, 2015 at 5:23 AM, Pedro Alves wrote: > On 02/17/2015 12:52 AM, Patrick Palka wrote: >> On Mon, Feb 16, 2015 at 5:42 PM, Pedro Alves wrote: > >> Good point... When leaving TUI mode, we disable ncurses via endwin(). >> When re-entering TUI mode, we are re-enabling ncurses through the >> first call to refresh(). If we manage to sync ncurses' knowledge of >> the terminal dimensions (via the call to resize_terminall() in >> tui_resize_all() I believe) before re-enabling ncurses then a >> KEY_RESIZE should not get placed into the queue. > > Exactly. > >> >>> >>> ISTM that we're just resizing too late still. Indeed, the patch below, >>> which makes us resize earlier, fixes the issue for me too. Are you >>> aware of any other use case that might cause KEY_RESIZE? >> >> Nice, it fixes the issue for me too. No more KEY_RESIZE keys. And I >> am not aware of another way to trigger KEY_RESIZE keys. > > Alright, great. > >>> (Note: Whatever the order of calls in tui_enable, we'll potentially be >>> showing stale contents momentarily and then overwriting with correct ones. >>> We get that today too. IMO, today's even worse, as it can show windows with >>> the wrong size for a moment, and that might be more visible flicker than showing >>> the wrong contents with the correct border sizes already. ISTM the fix for >>> that would be to decouple "update windows' contents" from actually >>> wrefresh/display'ing windows. That looks like much more work than worth >>> it though. I can only see this if I step through the code within tui_enable. >>> In a normal not-being-debugged run, I can't notice any flicker.) >> >> I have never noticed such flickering. But it is does not surprise me >> that TUI has the potential to flicker like hell, see >> https://sourceware.org/bugzilla/show_bug.cgi?id=13378 > > Yeah... > >> The patch makes a lot of sense to me... I appreciate your taking such >> an in-depth look at such a tedious issue! > > :-) These discussions have resulted in several fixes (from you), so > I think it's been well worth it, IMO. Thank _you_ for poking at > the TUI. > >> Is it OK to commit patches >> 1 and 3 of this series once you commit the below patch? > > Yes please. I've now pushed mine in. Actually, I will not commit patch #3 because it no longer applies cleanly after the CLI/TUI tab completion consolidation and incorporating sigwinch handling will allow for the complete eradication of tui_handle_resize_during_io(). > >> >> (As a side note it boggles my mind that [vertically] resizing the >> terminal while inside TUI is horribly broken, yet indirectly resizing >> TUI by temporarily exiting TUI, resizing within the CLI then >> re-entering TUI works just fine. Both of these methods ought to be >> running the same resizing logic! Something tells me most of >> tui_resize_all() is unnecessary and broken.) > > I think that that's missing is integrating SIGWINCH handling with the > event loop. That is, nothing is waking up the event loop to react to > SIGWINCH and do the resize, so the resize ends up happening on > next IO (next key press). Something along these lines: > > In the TUI setup code where we install the SIGWINCH signal > handler create a new event loop source for the SIGWINCH signal: > > sigwinch_token = > create_async_signal_handler (async_sigwinch_handler, NULL); > > Have the real SIGWINCH handler mark that async event source: > > static void > handle_sigwinch (int sig) > { > mark_async_signal_handler (sigwinch_token); > } > > And then when that event source's callback is called > by the event loop, resize the TUI: > > static void > async_sigwinch_handler (gdb_client_data arg) > { > /* Trigger TUI resize here. */ > } > > Would you like to try that? > > Thanks, > Pedro Alves >