Re: [Bug-readline] Forwarding input from ncurses to readline

[Ulf Magnusson]

On Sat, Feb 21, 2015 at 1:22 AM, Ulf Magnusson <address@hidden> wrote:
> On Fri, Feb 20, 2015 at 3:36 PM, Ulf Magnusson <address@hidden> wrote:
>> On Fri, Feb 20, 2015 at 3:30 PM, Chet Ramey <address@hidden> wrote:
>>> On 2/20/15 9:20 AM, Ulf Magnusson wrote:
>>>> On Fri, Feb 20, 2015 at 3:27 AM, Chet Ramey <address@hidden> wrote:
>>>>> On 2/18/15 9:43 PM, Ulf Magnusson wrote:
>>>>>
>>>>>> Re. rl_change_environment, there's this in the source:
>>>>>>
>>>>>> /* If this is non-zero, readline will set LINES and COLUMNS in the
>>>>>>    environment when it handles SIGWINCH. */
>>>>>> int rl_change_environment = 1;
>>>>>>
>>>>>> However, afaics readline does not update LINES and COLUMNS in the
>>>>>> SIGWINCH handler, at least not in the non-bash case. Is there some
>>>>>> motivation for this (besides being a bit tricky due to putenv/setenv not 
>>>>>> being
>>>>>> async-signal-safe)? The comment seems a little misleading either way. :)
>>>>>
>>>>> It does.  The SIGWINCH handler just sets a flag (_rl_caught_signal) and, 
>>>>> as
>>>>> a side effect, calls any application-specific SIGWINCH handler to allow it
>>>>> to do the same.  When readline does its signal handling via the
>>>>> RL_CHECK_SIGNALS macro, it notices that it caught SIGWINCH and calls
>>>>> _rl_signal_handler with SIGWINCH as the argument.  _rl_signal_handler
>>>>> calls rl_resize_terminal, which calls _rl_get_screen_size, which 
>>>>> eventually
>>>>> calls sh_set_lines_and_columns.
>>>>>
>>>>
>>>> Hmm... it's odd that ncurses doesn't pick up the updated LINES and COLUMNS
>>>> variables then. It also forwards the signal in its SIGWINCH handler.
>>>
>>> I thought you said you had forced readline to not catch signals; if
>>> readline doesn't install signal handlers none of this is relevant.
>>>
>>
>> That was later. I'm thinking of what caused the original problem.
>> Should have been
>> clearer.
>>
>> /Ulf
>
> I think I see what's going on now. Might be a readline bug.
>
> When using the alternate interface, signal handlers are installed only
> for the duration of each rl_callback_read_char() call. They are
> installed at the rl_set_signals() call and uninstalled at the
> CALLBACK_READ_RETURN() on the last line of the function (this might be
> bug). This means that a SIGWINCH that arrives between two calls to
> rl_callback_read_char() will not be seen (and neither will other
> signals, most likely).
>
> Here's a program that demonstrates the issue:
>
>
> #include <readline/readline.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <sys/select.h>
> #include <unistd.h>
>
> static void got_line(char *dummy) {
>     printf("LINES: %s  COLUMNS: %s\n", getenv("LINES"), getenv("COLUMNS"));
> }
>
> int main(void) {
>     fd_set read_fd_set;
>
>     rl_callback_handler_install("> ", got_line);
>     FD_ZERO(&read_fd_set);
>     FD_SET(STDIN_FILENO, &read_fd_set);
>     for (;;) {
>         select(STDIN_FILENO + 1, &read_fd_set, NULL, NULL, NULL);
>         rl_callback_read_char();
>     }
> }
>
>
> Pressing enter a few times and resizing the terminal in between shows
> that LINES and COLUMNS never change. The select() is important here as
> the program blocks inside rl_callback_read_char() otherwise, which
> hides the issue.
>
> /Ulf

This comment makes it look like it's intentional:

"Signal handlers are only installed when the application calls back
into readline, so readline doesn't `steal' signals from the
application."

That seems to make letting readline deal with signals much less useful
for the alternate interface though. Maybe the manual could clarify
that you need to e.g. unprep the terminal yourself if a signal if a
signal arrives between two rl_callback_read_char() calls.

/Ulf