Skip to content

Conversation

@Akirathan
Copy link
Member

Fixes #8313

Pull Request Description

Add jline module to the distribution so that our REPL is usable again.

Important Notes

  • No more: "WARNING: Unable to create a system terminal, creating a dumb terminal " warning when starting REPL
  • Arrow keys works as expected in REPL
  • Back search (the default shortcut CTRL + R) works as expected.

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

  • The documentation has been updated, if necessary.
  • Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
  • All code follows the
    Scala,
    Java,
    and
    Rust
    style guides. In case you are using a language not listed above, follow the Rust style guide.
  • All code has been tested:
    • Unit tests have been written where possible.
    • If GUI codebase was changed, the GUI was tested when built using ./run ide build.

@Akirathan Akirathan added CI: No changelog needed Do not require a changelog entry for this PR. CI: Clean build required CI runners will be cleaned before and after this PR is built. labels Dec 7, 2023
)
)
java --module-path %comp-dir% -Dpolyglot.compiler.IterativePartialEscape=true %EXTRA_OPTS% %JAVA_OPTS% -m org.enso.runtime/org.enso.EngineRunnerBootLoader %*
java --module-path %comp-dir% --add-modules org.jline -Dpolyglot.compiler.IterativePartialEscape=true %EXTRA_OPTS% %JAVA_OPTS% -m org.enso.runtime/org.enso.EngineRunnerBootLoader %*
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be better to avoid these add-modules modifications. If we are sure we want to use org.jline as a module in enso, then I'd suggest to modify module-info.java to require static org.jline. Then the standard JVM launcher shall pick the org.jline module up, when it is available.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea. Let's avoid --add-modules.

@Akirathan
Copy link
Member Author

@JaroslavTulach At the end, this PR is minimalistic - just 8 new lines. It turns out it was enough to set the thread context class loader before the jLine initialization.

@Akirathan Akirathan added CI: Ready to merge This PR is eligible for automatic merge and removed CI: Clean build required CI runners will be cleaned before and after this PR is built. labels Dec 13, 2023
@mergify mergify bot merged commit 1bde4c5 into develop Dec 13, 2023
@mergify mergify bot deleted the wip/akirathan/8313-jline branch December 13, 2023 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI: No changelog needed Do not require a changelog entry for this PR. CI: Ready to merge This PR is eligible for automatic merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove dumb terminal - implement jline

5 participants