Skip to content
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

Add native Dotty REPL #2991

Merged
merged 90 commits into from
Aug 23, 2017
Merged

Add native Dotty REPL #2991

merged 90 commits into from
Aug 23, 2017

Conversation

felixmulder
Copy link
Contributor

Adding this here to check using CI that all is well in the world so far.

Will probably be ready for review in a day or two.

@felixmulder felixmulder force-pushed the topic/new-repl branch 8 times, most recently from 812f141 to 2fd10d9 Compare August 22, 2017 18:32
@OlivierBlanvillain OlivierBlanvillain self-assigned this Aug 23, 2017
@OlivierBlanvillain
Copy link
Contributor

#1440 and #2554 are fixed in the new repl, we should close these and delete the area:old-repl label after merge :)

final val REPL_ASSIGN_SUFFIX = "$assign"
final val REPL_RES_PREFIX = "res"

// TODO: remove!
Copy link
Contributor

Choose a reason for hiding this comment

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

^

@@ -197,7 +197,6 @@ class CompilationTests extends ParallelTesting {
compileDir("../compiler/src/dotty/tools/dotc/config", picklingOptions) +
compileDir("../compiler/src/dotty/tools/dotc/parsing", picklingOptions) +
compileDir("../compiler/src/dotty/tools/dotc/printing", picklingOptions) +
compileDir("../compiler/src/dotty/tools/dotc/repl", picklingOptions) +
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if it would make sense to keep all that code somewhere under /tests given that it helped detect some nasty pattern matching edge cases... WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe, it still has the same components, so adding this should be trivial 👍

`dotty-compiler`,
`dotty-compiler` % "test->test"
).
settings(commonNonBootstrappedSettings).
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought we the repl would be both complied by scala and bootstrapped?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh I guess adding the sources in dottyCompilerSettings does the trick...

|:load <path> interpret lines in a file
|:quit exit the interpreter
|:type <expression> evaluate the type of the given expression
|:reset reset the repl to its initial state, forgetting all session entries
Copy link
Contributor

Choose a reason for hiding this comment

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

:imports is missing from that list

val text =
"""The REPL has several commands available:
|
|:help print this summary or command-specific help
Copy link
Contributor

Choose a reason for hiding this comment

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

The command-specific part is not implemented

package repl
package terminal

/**
Copy link
Contributor

Choose a reason for hiding this comment

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

Old school scaladoc comments?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ammonite strikes again!

"val res0: Int = 2",
"var y: Int = 5")

expected === storedOutput().split("\n")
Copy link
Contributor

Choose a reason for hiding this comment

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

What does === do with a Set and an Array oO?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's a cooooool thingy that sorts, asserts and prints if assertion fails


import dotc.reporting.MessageRendering

/** Runs all tests contained withing `/repl/test-resources/scripts` */
Copy link
Contributor

Choose a reason for hiding this comment

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

s/withing/in/

@@ -20,6 +19,24 @@ class ConsoleInterface {
def run(args: Array[String],
bootClasspathString: String,
classpathString: String,
// TODO: initial commands needs to be run under some form of special
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we make an issue out of this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is: #3007

AnsiNav.resetForegroundColor
)

val autocompleteFilter: Filter = Filter.action("autocompleteFilter")(SpecialKeys.Tab :: Nil) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the completion behavior is a bit too aggressive, at least compared to my bash habits:

$ touch equals
$ touch equalsIgnoreCase
$ cat equ                               # Press tab
equals            equalsIgnoreCase
$ cat equals                            # Press tab again
equals            equalsIgnoreCase
$ cat equals                            # and again
equals            equalsIgnoreCase
scala> "".equ                           // Press tab
equals               equalsIgnoreCase
scala> "".equals                        // Press tab again
equalsIgnoreCase
scala> "".equalsIgnoreCase              // and again

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe, but that's the way it's implemented in ammonite. We could probably enhance it so that you get a more bash-like behaviour. But IMO outside of the scope of this PR being merged :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Low-hanging fruit perhaps?

@felixmulder felixmulder changed the title [WIP] Add native Dotty REPL Add native Dotty REPL Aug 23, 2017
Copy link
Contributor

@OlivierBlanvillain OlivierBlanvillain left a comment

Choose a reason for hiding this comment

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

LGTM!

@felixmulder felixmulder merged commit d70b8f4 into scala:master Aug 23, 2017
@rom1dep
Copy link

rom1dep commented Aug 29, 2017

FYI, on a clean clone,

./bin/dotr
The script is going to build the required jar files
Building dotty-interfaces...done
Building dotty-compiler...done
Building dotty library...done
Building tests...done
Starting dotty REPL...
Error: Could not find or load main class dotty.tools.dotc.repl.Main

@felixmulder
Copy link
Contributor Author

@rom1dep: #3026

@allanrenucci allanrenucci deleted the topic/new-repl branch December 14, 2017 19:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants