RFR: 8352693: Use a simpler console reader instead of JLine for System.console() [v4]

[Chen Liang]

On Wed, 16 Apr 2025 12:07:45 GMT, Jan Lahoda <jlahoda at openjdk.org> wrote:

>> The `java.io.Console` has several backends: a simple on in `java.base`, a more convenient one in `jdk.internal.le` (with line-reading based on JLine) and one for JShell.
>> 
>> The backend based on JLine is proving to be a somewhat problematic - JLine is very powerful, possibly too powerful and complex for the simple task of editing a line with no completion, no history, no variables, no commands, etc. As a consequence, there are inevitable sharp edges in this backend.
>> 
>> The idea in this PR is to replace the use of JLine in the `jdk.internal.le` backend with a simple escape code interpreter, that only handles a handful of keys/codes (left/right arrow, home, end, delete, backspace, enter), and ignores the rest. The goal is to have something simple with less surprising behavior.
>
> Jan Lahoda has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Reflecting review feedback: Adding makefile comment as suggested

src/java.base/share/classes/jdk/internal/io/BaseJdkConsoleImpl.java line 175:

> 173:     protected final Charset charset;
> 174:     protected final Object readLock;
> 175:     protected final Object writeLock;

Recommend mentioning that when both locks must be acquired, the `writeLock` must be acquired first. Unfortunately we cannot just use a `ReentrantReadWriteLock`.

src/java.base/share/classes/jdk/internal/io/BaseJdkConsoleImpl.java line 181:

> 179:     protected final Formatter formatter;
> 180: 
> 181:     protected abstract char[] readline(boolean password) throws IOException;

I recommend a more distinct method name, like `nextLine` or `implReadLine`; using `readline` to distinguish from `readLine` is weird.

src/java.base/share/classes/jdk/internal/io/BaseJdkConsoleImpl.java line 184:

> 182: 
> 183:     @SuppressWarnings("this-escape")
> 184:     public BaseJdkConsoleImpl(Charset charset) {

Suggestion:

    protected BaseJdkConsoleImpl(Charset charset) {

src/jdk.internal.le/share/classes/jdk/internal/console/JdkConsoleImpl.java line 38:

> 36:  * JdkConsole implementation based on the platform's TTY, with basic keyboard navigation.
> 37:  */
> 38: public final class JdkConsoleImpl extends BaseJdkConsoleImpl {

We should choose a different simple name for this class, especially when we are importing classes from `jdk.internal.io` package.

src/jdk.internal.le/share/classes/jdk/internal/console/JdkConsoleImpl.java line 57:

> 55:     }
> 56: 
> 57:     protected char[] readline(boolean password) throws IOException {

`@Override`

src/jdk.internal.le/share/classes/jdk/internal/console/LastErrorException.java line 28:

> 26: 
> 27: @SuppressWarnings("serial")
> 28: public class LastErrorException extends RuntimeException{

Suggestion:

public class LastErrorException extends RuntimeException {

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24242#discussion_r2055177151
PR Review Comment: https://git.openjdk.org/jdk/pull/24242#discussion_r2055179038
PR Review Comment: https://git.openjdk.org/jdk/pull/24242#discussion_r2055177261
PR Review Comment: https://git.openjdk.org/jdk/pull/24242#discussion_r2055183690
PR Review Comment: https://git.openjdk.org/jdk/pull/24242#discussion_r2055181215
PR Review Comment: https://git.openjdk.org/jdk/pull/24242#discussion_r2055181573