Migrate to jline3 #2063
Migrate to jline3 #2063
Conversation
| @@ -47,7 +47,7 @@ | |||
| "request": "attach", | |||
| "projectName": "rascal", | |||
| "hostName": "localhost", | |||
| "port": 9001 | |||
| "port": 9213 | |||
There was a problem hiding this comment.
What is this port and why has it changed?
There was a problem hiding this comment.
it's for remote debugging, and I should revert it, or at least not part of this PR.
| @@ -232,26 +224,26 @@ public void decCallNesting() { | |||
| private static final Object dummy = new Object(); | |||
|
|
|||
| /** | |||
| * Promotes the monitor to the outputstream automatically if so required. | |||
| * Promotes the monitor to the PrintWriter automatically if so required. | |||
There was a problem hiding this comment.
Maybe add some explanation on the different constructors and how to use them? The current comments are quite terse (and non-existent for two of the four constructors).
There was a problem hiding this comment.
Yes, but that's not part of this PR, as I'm trying to reduce the changes to code not related to the REPL.
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private static void addIt(SortedSet<String> result, String v, String qualifier, String originalTerm) { | ||
| private static void addIt(SortedMap<String, String> result, String category, String v, String qualifier, String originalTerm) { |
There was a problem hiding this comment.
🤷🏼 I'm trying to be minimal here, and not change more than needed. The name is roughly: add these entries to the map in result?
| private final NavigableMap<String, String> setOptions; | ||
| private final BiConsumer<String, List<Candidate>> completeIdentifier; | ||
| private final BiConsumer<String, List<Candidate>> completeModule; | ||
| public RascalCommandCompletion(NavigableMap<String, String> setOptions, BiConsumer<String, List<Candidate>> completeIdentifier, BiConsumer<String, List<Candidate>> completeModule) { |
There was a problem hiding this comment.
Why the BiConsumer approach? Because a simple function returning a list of candidates is less efficient?
There was a problem hiding this comment.
In the end yes, and since this is called frequently, I thought to keep the design of jline3 (namely: here, append onto this list) around. I've not measured it if concatenation would be an acceptable performance tradeoff.
| // we do not have to filter out prefixes | ||
| word = RascalQualifiedNames.unescape(word); // remove escape that the interpreter cannot deal with | ||
| int rootedIndex = word.lastIndexOf("::"); | ||
| String moduleRoot = rootedIndex == -1? "": word.substring(0, rootedIndex); |
There was a problem hiding this comment.
Whitespace is a little messed up here
There was a problem hiding this comment.
which messy whitespace?
| sw.write(value, wrt); | ||
| } | ||
| catch (/*IOLimitReachedException*/ RuntimeException e) { | ||
| // ignore since this is what we wanted |
There was a problem hiding this comment.
I do not completely understand this, but does this mean e should be an instance of IOLimitReacheException but we cannot catch that directly? If so, should we not check for this explicitly to make sure we are not ignoring another RuntimeException?
This PR migrates the REPL to jline3, numerous issues are solved, but it also required a redesign of all the REPL classes.
This breaks some stuff, which could not be avoided. The most noticeble is that all Input&OutputStreams were removed from the evaluator. That was (in hindsight) not a good design choice, as they are about bytes, while we want to print text. This gets messy when we have to figure out which charset we should write to the stream.
Todo's:
main