RFR: JDK-8185426: Jshell crashing on autocompletion

[Jan Lahoda]

Thanks for the comments. An updated webrev is here:
http://cr.openjdk.java.net/~jlahoda/8185426/webrev.01/

Thanks,
    Jan

On 17.8.2017 02:18, Robert Field wrote:
>
> On 07/28/17 07:45, Jan Lahoda wrote:
>> I'd like to ask for a review of a fix for bug:
>> https://bugs.openjdk.java.net/browse/JDK-8185426
>>
>> Proposed webrev:
>> http://cr.openjdk.java.net/~jlahoda/8185426/webrev.00/
>>
>> The problem here is that: jshell has a stateful completion, pressing
>> <tab> several times will produce different outputs. The state of the
>> completion is currently reset when the user performs action different
>> than "COMPLETE" as determined by KeyMap. Sadly, even if the action is
>> COMPLETE, JLine may (if there are further characters in the input)
>> decide it won't perform the complete action, rather print the tab
>> character. See:
>> http://hg.openjdk.java.net/jdk10/jdk10/jdk/file/0861f2298fd7/src/jdk.internal.le/share/classes/jdk/internal/jline/console/ConsoleReader.java#l2669
>>
>>
>> This then can lead to the problem, as the completion may be using the
>> stale state.
>>
>> So the patch is trying to ensure the completion state is reset if
>> complete is not called: the state is reset always, and the complete
>> method will restore it if needed.
>>
>> How does this look?
>>
>> Thanks,
>>     Jan
>
> The change makes ConsoleIOContext public, adds two static fields and
> embeds some test-specific code to the class.  This complicates the class
> and mixes it with test code.
> I think it would be better to add a public test access class, which
> holds these statics and the test-specific code.  You would still need a
> simple hook to get to this test support.
>
> Nit: the initialization of todo (line 252) is placed after a comment
> that does not document it -- they should be switched.
>
> The meanings, invariants, and transitions of the new CompletionState
> fields should be documented.  In general, ConsoleIOContext should be
> documented.  The one existing comment (on the
> ConsoleIOContext.complete() method) needs update.
>
> -Robert
>
>