Re: [PATCH] Fix the processing of Meta-key commands in TUI

[Patrick Palka <patrick at parcs dot ath dot cx>]

On Thu, Aug 28, 2014 at 10:13 AM, Patrick Palka <patrick@parcs.ath.cx> wrote:
> On Thu, Aug 28, 2014 at 7:22 AM, Pedro Alves <palves@redhat.com> wrote:
>> On 08/27/2014 07:25 PM, Patrick Palka wrote:
>>> On Wed, 27 Aug 2014, Pedro Alves wrote:
>>
>>>> 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?
>>>
>>> I don't think the timeout is the issue here.  Even if the timeout is
>>> disabled via notimeout(), wgetch() will still attempt to interpret keypad
>>> sequences by reading multiple characters from stdin -- except that the
>>> read will now be a non-blocking one instead of a blocking one.
>>>
>>> One way or another, someone must read multiple keys from stdin in order
>>> to semantically distinguish between keypad keys and regular key
>>> sequences.  And when it turns out that the input is not or cannot be a
>>> keypad key then that someone must place the extraneous keys into a
>>> buffer and notify GDB's event handler that we missed their stdin events.
>>
>> Right, that's a given.  What I was talking about is fixing the
>> 1 second block in case the input stops before a sequence is complete.
>>
>>> If we handle the timeout ourselves (for instance by disabling keypad()
>>> and enabling notimeout()) then we'll be responsible for doing the
>>> lookahead, interpreting the sequences and buffering the keypresses.  I
>>> say let ncurses continue to handle the timeout so that we'll only be
>>> responsible for notifying the event handler.
>>>
>>> Though I may just be misunderstanding your proposal.
>>
>> The main idea was to not let ncurses ever block, as that prevents
>> gdb's event loop from handling target events.  If ncurses internally
>> already handled the timeout by comparing the time between
>> wgetch calls instead of doing a blocking select/poll internally, then
>> it'd be a bit easier, but it looks like it doesn't (GetEscdelay always
>> starts with the window's configured delay, on each wgetch call?), so we'd
>> need to track the timeout ourselves.  Even if it did, it wouldn't be that
>> different though.
>
> Is the internal timeout a big deal though?  The handling of the target
> event just gets temporarily delayed, not missed entirely, right?  And
> isn't this timeout only experienced when one presses ESC (which has no
> use in TUI) and/or attempts to manually type a function key sequence?
> I'm not sure why anyone would do that.
>
>>
>> What we'd need is:
>>
>>  #1 - set ncurses to be _always_ non-blocking/no-delay.
>>  #2 - on input, call wgetch, as today.
>>  #3 - due to no-delay, that now only looks ahead for as long as
>>       there are already bytes ready to be read from the input file / stdin.
>>  #4 - if the sequence looks like a _known_ escape sequence, but
>>       it isn't complete yet, then wgetch leaves already-read bytes
>>       in the fifo, and returns ERR.  That's the "the keys stay uninterpreted"
>>       comment in lib_getch.c.
>>  #5 - at this point, we need to wait for either:
>>       (a) further input, in case further input finishes the sequence.
>
> Not sure it's possible to make wgetch() behave this way.  From what I
> can tell, wgetch() will always return the key from its fifo if there's
> one available -- it won't check whether the fifo contents + a new key
> from stdin will make a complete sequence.  It won't even read from
> stdin if there's a key in the fifo.
>
>>       (b) the timeout to elapse, meaning no further input, and we should
>>           pass the raw chars to readline.
>>
>> For #5/(a), there's nothing to do, that's already what the
>> stdin handler does.
>>
>> For #5/(b), because we don't want to block in the stdin handler (tui_getc)
>> blocked waiting for the timeout, we would instead install a timer in gdb's event
>> loop whose callback was just be the regular TUI stdin input handler.  This time, given
>> enough time had elapsed with no further input, we want the raw chars.  If ncurses
>> internally knows that sufficient time has passed, then good, we only have to
>> call wgetch, and we know ncurses gives us the raw keys (the incomplete sequence).
>> If it doesn't (looks like it doesn't, but I may be wrong), then we just temporarily
>> switch off the keypad, and read one char, which returns us the raw escape char at
>> the head of the fifo, and leaves the rest in the fifo for subsequent reads.
>> As the fifo is now missing the escape char, we can go back to normal, with the
>> keypad enabled, and the next time we call wgetch should return us the head of
>> the fifo immediately, if there's anything there.
>>
>> Going back to step #4, in case the sequence is _unknown_ or the timeout
>> has expired:
>>
>>  #4.2 - if the sequence in the fifo is definitely an _unknown_ escape
>>     sequence, wgetch returns the first char in the fifo, leaving the
>>     remainder of the bytes in the fifo.  TBC, in this case, we _don't_
>>     get back ERR.  As there are more bytes in the fifo, then we need
>>     to compensate for the missed stdin events, like in your patch.  (*)
>>
>> (*) - so it sounds your patch would be necessary anyway.
>>
>> Oddly, even when doing:
>>
>>   nodelay (w, TRUE);
>>   notimeout (w, TRUE);
>>
>> in tui_getc, I _still_ get that a one second block within wgetch...
>> Looks like related to mouse event handling, even though mouse
>> events were not enabled...:
>
> Yeah same here.  I can't seem to find the magical invocation that
> actually disables this timeout.

So it looks like the (non-standard) ncurses variable ESCDELAY is what
controls the timeout:  wgetch() sets the timeout to ESCDELAY
milliseconds.  So do you suppose that we should set this variable