-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Use JLine 3 in the REPL #4680
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use JLine 3 in the REPL #4680
Conversation
|
||
context match { | ||
case ParseContext.ACCEPT_LINE => | ||
// TODO: take into account cursor position |
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.
@lihaoyi In Ammonite, when do you consider a line to be complete or not?
I am thinking of doing something like: A line is considered incomplete
- if the cursor position is before the last non-whitespace character
- if the cursor position is before the last closing parenthese
Either (1) or (2)
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.
@allanrenucci not sure what context you are asking this in, but ENTER means "submit" instead of "add a newline to this cell" if parsing the cell results in a failure whose parse-index is right at the end of the cell's text (or the cursor is not on the last line of the cell)
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.
It makes sense. Thanks!
@sjrd Can you try it out on Windows? You can start the REPL from sbt by running the |
I got an exception while trying to run on windows:
|
@allanrenucci Note that we have a Windows VM at LAMP you can use to experiment, ask Fabien to get access. |
@Glavo The last commit should fix the issue you encountered |
In addition, autocomplete does not work under Windows. |
On Windows, the REPL doesn't work from the Dotty build. JLine is not able to create a terminal in the forked JVM created by sbt. @Glavo If you want to try out the repl on windows, here is what you can do:
Also |
a0f48c9
to
1dafbb4
Compare
When I tried to do this, I got an exception:
|
It looks like our
and we need to add the corresponding bat script (#1207) |
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.
LGTM, but we should follow up on the issue with Windows and the Dotty build.
@@ -23,6 +23,12 @@ object Reporter { | |||
simple.report(m) | |||
} | |||
} | |||
|
|||
/** A reporter that ignores reports */ | |||
object NoReporter extends Reporter { |
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.
Maybe call it SilentReporter ?
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.
Scalac has something similar called NoReporter
. It is more that silent, it doesn't even count errors
private val prompt = blue("scala> ") | ||
private val newLinePrompt = blue(" | ") | ||
|
||
/** Blockingly read line from `System.in` |
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.
Can we take an InputStream instead of hardcoding System.in ? For tests we shouldn't have to use System.in.
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.
JlineLine doesn't seem to let you configure the input stream and we should probably not modify System.in
. We don't do end to end testing for the REPL, the tests don't rely on JLine
// ENTER means SUBMIT when | ||
// - cursor is at end (discarding whitespaces) | ||
// - and, input line is complete | ||
val cursorIsAtEnd = line.indexWhere(!_.isWhitespace, from = cursor) >= 0 |
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.
Should be called cursorNotAtEnd no? In any case, it's a bit confusing that the comment above describes the conditions for submitting, but the code checks the conditions for not submitting, either the code logic or the comment should be changed.
.variable(LineReader.SECONDARY_PROMPT_PATTERN, "%M") | ||
.option(Option.INSERT_TAB, true) // at the beginning of the line, insert tab instead of completing | ||
.option(Option.AUTO_FRESH_LINE, true) // if not at start of line before prompt, move to new line | ||
.variable(SECONDARY_PROMPT_PATTERN, "%M") |
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.
add a comment explaining what this does?
@@ -97,39 +99,44 @@ final class JLineTerminal extends java.io.Closeable { | |||
/* message = */ "", | |||
/* missing = */ newLinePrompt) | |||
|
|||
case class TokenData(token: Token, start: Int, end: Int) |
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.
Add a documentation comment.
I think it is either a bug in sbt or JLine. It should not affect users, only contributors working on Windows. Since I'm not on Windows, it is a bit cumbersome for me to minimise and report an issue |
- cursor is at end (discarding whitespaces) - and, input line is complete
No description provided.