RFR 8166051: [jline] Cannot parse .inputrc with \r

[Robert Field]

The code that typically reads this file is in the Readline library, 
specifically bind.c -- this is what it does:

   while (line < end)
     {
       /* Find the end of this line. */
       for (i = 0; line + i != end && line[i] != '\n'; i++);

#if defined (__CYGWIN__)
       /* ``Be liberal in what you accept.'' */
       if (line[i] == '\n' && line[i-1] == '\r')
     line[i - 1] = '\0';
#endif

       /* Mark end of line. */
       line[i] = '\0';

       /* Skip leading whitespace. */
       while (*line && whitespace (*line))
         {
       line++;
       i--;
         }

       /* If the line is not a comment, then parse it. */
       if (*line && *line != '#')
     rl_parse_and_bind (line);

       /* Move to the next line. */
       line += i + 1;
       current_readline_init_lineno++;
     }

I think we should mimic this behavior.  Specifically, this means 
accepting \n and \r\n but not treating stand-alone \r as a special 
character.  I think this should be handled the same across platform.

As Jan points out, this also treats whitespace differently than jline.  
Specifically it trims the start but not end of the line.

Note: they read the whole file and then just operate on the string. It 
is a small file.  Rather than subclassing BufferedReader, that given the 
flexibility the exactly match their logic.

And I think we should contribute this back to jline.

-Robert


On 09/16/16 12:08, Robert Field wrote:
>
> On 09/16/16 11:31, Jonathan Gibbons wrote:
>> Perhaps your .inputrc reader should support an escape mechanism for 
>> handling problematic characters.
>
> .inputrc is a standard not under our control:
>
> https://www.gnu.org/software/bash/manual/html_node/Readline-Init-File.html 
>
>
> Though!  That gives a hint on how we should handle this: figure out 
> what bash does.
>
> -Robert
>
>
>>
>> -- Jon
>>
>> On 09/16/2016 09:26 AM, Robert Field wrote:
>>> BufferedReader provides platform independent behavior.  Which is 
>>> appropriate here because configuration files get copied between 
>>> platforms with different line-end standards.
>>> The proposed replacement exchanges this for a platform dependent 
>>> strategy.
>>> While it would address this one obscure failure, my concern is that 
>>> it will create new, possibly more common, problems. specifically 
>>> where a file where \r\n is used -- injecting \r into the results.
>>> Not sure what the right choice is.
>>> A compromise might be to accept \n or \r\n on all platforms, and \r 
>>> alone only on platforms that accept that as a line end.
>>> I am concerned with forking jline, esp. if it not a clear fix.
>>> Can we address this at a higher level?  More tolerant error handling?
>>>
>>> -Robert
>>>
>>>
>>> On 09/16/16 02:23, Jan Lahoda wrote:
>>>> Please review a change to how .inputrc is read. Currently, it uses 
>>>> BufferedReader.readLine(), which interprets '\r' as a line 
>>>> separator, and so the '\r' character cannot be used in the macros. 
>>>> The proposed change is to use System.getProperty("line.separator") 
>>>> and "\n" as line separators.
>>>>
>>>> Bug:
>>>> https://bugs.openjdk.java.net/browse/JDK-8166051
>>>>
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~jlahoda/8166051/webrev.00/
>>>>
>>>> Thanks!
>>>>
>>>> Jan
>>>
>>
>