From patchwork Wed Aug 27 17:06:01 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 2557 Received: (qmail 24582 invoked by alias); 27 Aug 2014 17:06:11 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 24452 invoked by uid 89); 27 Aug 2014 17:06:07 -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, RP_MATCHES_RCVD, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Wed, 27 Aug 2014 17:06:06 +0000 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s7RH63iT029350 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 27 Aug 2014 13:06:03 -0400 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s7RH62Cq026206; Wed, 27 Aug 2014 13:06:02 -0400 Message-ID: <53FE0FF9.9010008@redhat.com> Date: Wed, 27 Aug 2014 18:06:01 +0100 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.7.0 MIME-Version: 1.0 To: Patrick Palka , gdb-patches@sourceware.org Subject: Re: [PATCH] Fix the processing of Meta-key commands in TUI References: <1408740286-29326-1-git-send-email-patrick@parcs.ath.cx> In-Reply-To: <1408740286-29326-1-git-send-email-patrick@parcs.ath.cx> Hey there, Thanks for looking at this! On 08/22/2014 09:44 PM, Patrick Palka wrote: > This patch fixes the annoying bug where key sequences such as Alt_F or > Alt_B (go forward or backwards by a word) do not behave promptly in TUI. > You have to press a third key in order for the key sequence to register. > > This is mostly ncurses' fault. Calling wgetch() normally causes ncurses > to read only a single key from stdin. However if the key read is the > start-sequence key (^[ a.k.a. ESC) then wgetch() reads TWO keys from > stdin, storing the 2nd key into an internal FIFO buffer and returning > the start-sequence key. The extraneous read of the 2nd key makes us > miss its corresponding stdin event, so the event loop blocks until a > third key is pressed. This explains why such key sequences do not > behave promptly in TUI. > > To fix this issue, we must somehow compensate for the missed stdin event > corresponding to the 2nd byte of a key sequence. This patch achieves > this by hacking up the stdin event handler to conditionally execute the > readline callback multiple multiple times in a row. This is done via a > new global variable, call_stdin_event_handler_again_p, which is set from > tui_getc() when we receive a start-sequence key and notice extra pending > input in the ncurses buffer. Hmm, I've been staring at this for a while trying to make sense of the whole thing. I think there's a larger issue here. This happens because we enable the "keypad" behavior. From the ncurses manual: The keypad option enables the keypad of the user's terminal. If enabled (bf is TRUE), the user can press a function key (such as an arrow key) and wgetch re‐ turns a single value representing the function key, as in KEY_LEFT. If disabled (bf is FALSE), curses does not treat function keys specially and the program has to interpret the escape sequences itself. If the keypad in the terminal can be turned on (made to transmit) and off (made to work locally), turning on this option causes the terminal keypad to be turned on when wgetch is called. The default value for keypad is false. readline must already be processing escape sequences itself, so then why we do enable this in the first place? The answer is that we want to intercept things like up/down -- when the TUI is active, those should scroll the source window instead of navigating readline's history. This is done in tui_getc: if (key_is_command_char (ch)) { /* Handle prev/next/up/down here. */ ch = tui_dispatch_ctrl_char (ch); } Hacking away keypad, like: gdb/tui/tui.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) indeed makes Alt_F, etc. work. The main reason I think there's a larger problem here, is that if curses is reading more than one char from stdin, then that means that is must be blocking for a bit waiting for the next character, which is a no-no in an async world, where we want to be processing target events at the same time. The man page says: While interpreting an input escape sequence, wgetch sets a timer while waiting for the next character. If notimeout(win, TRUE) is called, then wgetch does not set a timer. The purpose of the timeout is to differentiate between sequences received from a function key and those typed by a user. Looks like there's a default timeout of 1 second. Indeed if I set a breakpoint in wgetch and another right after wgetch is called, and then I press escape, I see that gdb is stuck inside wgetch for around one second. During that time, gdb's own event loop isn't being processed. Not sure exactly how this is usually handled. Seems like there are several knobs that might be able to turn this delay off. Sounds like we should enable that (whatever the option is), and handle the timeout ourselves? Thanks, Pedro Alves diff --git a/gdb/tui/tui.c b/gdb/tui/tui.c index c30b76c..6174c0f 100644 --- a/gdb/tui/tui.c +++ b/gdb/tui/tui.c @@ -398,7 +398,7 @@ tui_enable (void) tui_show_frame_info (0); tui_set_layout (SRC_COMMAND, TUI_UNDEFINED_REGS); tui_set_win_focus_to (TUI_SRC_WIN); - keypad (TUI_CMD_WIN->generic.handle, TRUE); + // keypad (TUI_CMD_WIN->generic.handle, TRUE); wrefresh (TUI_CMD_WIN->generic.handle); tui_finish_init = 0; }