-
Notifications
You must be signed in to change notification settings - Fork 62
8299689: Make use of JLine for Console as "opt-in" #88
Conversation
👋 Welcome back naoto! A progress list of the required criteria for merging this PR into |
Webrevs
|
@@ -31,15 +31,15 @@ | |||
* The provider used for instantiating JdkConsole instance can be | |||
* specified with the system property "jdk.console", whose value | |||
* designates the module name of the implementation, and which defaults | |||
* to "jdk.internal.le" (jline). If no providers is available, | |||
* to "java.base". If no providers is available, | |||
* or instantiation failed, java.base built-in Console implementation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The overall change looks fine but I think for the next edit that we should move most of this comment to Console as it's Console that selects the behavior and that skips errors. Also once the SM execution mode goes away then we can re-visit that behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense. The comment has been moved to Console
.
Hello Naoto, this looks fine to me. The |
@naotoj This change now passes all automated pre-integration checks. ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details. After integration, the commit message for the final commit will be:
You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been 5 new commits pushed to the
Please see this link for an up-to-date comparison between the source branch of this pull request and the ➡️ To integrate this PR with the above commit message to the |
/integrate |
Going to push as commit d49851a.
Your commit was automatically rebased without conflicts. |
Due to the fact that JLine spawns native processes to obtain terminal information on macOS/Linux, we decided to disable the JLine by default for performance degradation reasons. It is still possible to enable it by specifying it on the command line with
jdk.console
system property (not a public one though). Once we have a solution to avoid spawning processes, JLine may be back for use in the future.Progress
Issues
Reviewers
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk20 pull/88/head:pull/88
$ git checkout pull/88
Update a local copy of the PR:
$ git checkout pull/88
$ git pull https://git.openjdk.org/jdk20 pull/88/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 88
View PR using the GUI difftool:
$ git pr show -t 88
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk20/pull/88.diff