On Thu, 11 May 2023 14:17:56 GMT, Jan Lahoda <jlah...@openjdk.org> wrote:

>> To support JShell and other usecases, the JDK uses JLine, which provides 
>> line-editing functionality inside a terminal.
>> 
>> JLine has several ways to work with the terminal, based on various 
>> additional libraries and tools. Most of them are not directly usable inside 
>> the JDK, so currently, on non-Windows platforms, the JLine inside the JDK 
>> will use external programs, like `stty`, to inspect the terminal's 
>> properties and to switch the terminal to raw mode.
>> 
>> This is slightly problematic, as the external programs might be missing, and 
>> while this is not that big problem for JShell, it might be a problem for 
>> other potential uses of JLine, like using it inside `System.console()`.
>> 
>> So, the proposal here is to call the corresponding native methods directly, 
>> on selected platforms for now (Linux and Mac OS/X), instead of invoking the 
>> external programs. On Windows, this has always been the case, we are using a 
>> custom implementation of the interface that maps native and Java function 
>> for JNA there, and the proposal is to do the same here. We take the 
>> appropriate mapping interface for JNA, and provide hand-written 
>> implementation for it, using JNI.
>> 
>> The Windows implementation is mostly unchanged, the changes are mostly 
>> non-Windows only. The overview of the changes is:
>>  - `LastErrorException` is moved from the Windows-specific code to the 
>> platform-neutral code, as it is used by all the native implementations. This 
>> is the only change that directly affects the Windows-specific code
>>  -  
>> `unix/classes/jdk/internal/org/jline/terminal/impl/jna/JnaNativePty.java` 
>> and 
>> `unix/classes/jdk/internal/org/jline/terminal/impl/jna/JnaTerminalProvider.java`
>>  are created based on the corresponding files from JLine. In JLine, these 
>> files directly call platform-specific implementations, but in this patch, 
>> the platform specific code is commented out, and replaced with calls to 
>> `JDKNativePty`, which is a new platform-specific class, that delegates to 
>> the actual platform-specific implementations. Note the `JnaNativePty.java` 
>> and `JnaTerminalProvider.java` already exist in the Windows part, but have 
>> different pieces of code commented out, which makes them substantially 
>> different.
>>  - for Linux and Mac OS/X, there are platform-specific implementations based 
>> on the corresponding code from JLine, with the hand-written JNI-based 
>> implementation of the JNA-based existing interfaces. They also have an 
>> implementation of `JDKNativePty`, which just delegates to the given 
>> platform's `"Nati...
>
> Jan Lahoda has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Reflecting review feedback.

Marked as reviewed by vromero (Reviewer).

-------------

PR Review: https://git.openjdk.org/jdk/pull/13687#pullrequestreview-1423023134

Reply via email to