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.

Sign up for GitHub

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

Jump to bottom

REPL: decouple the UI (shell) from the compiler [ci: last-only] #5903

Merged
merged 19 commits into from
Jun 26, 2017
Merged

REPL: decouple the UI (shell) from the compiler [ci: last-only] #5903

merged 19 commits into from
Jun 26, 2017

Conversation

adriaanm
Copy link
Contributor

@adriaanm adriaanm commented May 13, 2017 •
edited
Loading

Split in front- and back-end. Drop shaded jline (was needed for spark shell; we should be able to offer a better solution with the coming frontend improvements).

Review follow-ups:

  • compare to 05cc3e2 for error message indenting
  • tab completion on first line should also result in tab completion once repl initializes
  • check we're ok with different sbt versions (standardization work on API for sbt / spark / .. will be a separate effort -- forthcoming!)

Known (temporary) regressions in this PR:

  • lost syntax highlighting because it was done in the compiler, while it should be in the shell (UI)
  • similarly, the repl no longer exposes an object that can be used to manipulate the shell, because we're trying to reduce this coupling
  • error reporting uses a heuristic to determine whether the offending line is on the console (to avoid repeating it) -- can we improve this?

@adriaanm adriaanm added the WIP label May 13, 2017
@scala-jenkins scala-jenkins added this to the 2.13.0-M2 milestone May 13, 2017
@som-snytt
Copy link
Contributor

Now I know why it's called a shell game.

@adriaanm
Copy link
Contributor Author

(Now that I have the tests passing, I'll work on addressing the first round of review feedback. Thanks!)

@adriaanm adriaanm force-pushed the repl-modularize branch 4 times, most recently from cb72f11 to ccf5afb Compare May 18, 2017 02:17
@adriaanm adriaanm mentioned this pull request May 19, 2017
Closed
9 tasks
@adriaanm adriaanm force-pushed the repl-modularize branch 2 times, most recently from cf64b1f to 6acad7d Compare June 22, 2017 18:32
@adriaanm
Improve repl error positions, tree gen
b06bfa5 
Parse user input as-is, and use that as content of
unit's source file content. The unit's body has the
trees resulting from wrapping and rewriting the user's
input to implement the repl's semantics (store the
result in a val etc).

Also pass on whether parsed trees had xml elements.

This allows us to be more precise in our error messages,
as positions map 1-1 to user input (as suggested by retronym,
who helped getting the range positions right).

Note: input may contain leading/trailing non-trees.
For example, whitespace/comments don't survive as trees,
thus we can't assume the trees cover the range pos
(0, buf.length); instead, compute it.

Goal: no string-based code manipulation. No there yet.

Handle assignment minimally to avoid memory leaks
So, don't hang on to assigned values.
Still provide some feedback that the assignment has
been executed --> `"mutated x"`.

Also, bring back pure expression warning to maintain
status quo -- but who wants to see those pure expr
warnings in the repl??

Rangepos invariants require most synthetic code
to have a transparent pos, but the presentation
compiler locates trees by position, which means
use code must have an opaque position, and we
must be able to navigate to it from the root,
which means all ancestor trees to user code must
cover the positions of said user code.
@adriaanm
Partial input only needed for completion
7cef381 
Move it there, and while we're at it, simplify Completion hierarchy
@adriaanm
Simplify completion hierarchy some more
7d5222c
@adriaanm
Copy link
Contributor Author

Alright, this should be ready for merge, if it looks good. I'm going to defer the work on simplifying wrapping to another PR, where we'll also standardize on the class-based wrapping, and just generally try to dial the wrapping noise back to a minimum. That, and upgrading to JLine 3 will have to wait for M3.

@felixmulder
Copy link
Contributor

@retronym, @adriaanm - hey guyz, Dotty has a Show type class in the library. It's used to render rhs in a friendly way, e.g:

scala> "\twaaaaaaaaaaaaaaaaaaaaaat"
res0: String =     waaaaaaaaaaaaaaaaaaaaaat

in scalac, but in dotty:

scala> "\twaaaaaaaaaaaaaaaaaaaaaat"
val res0: String = "\twaaaaaaaaaaaaaaaaaaaaaat"

As soon as case classes are hlists, there could be show instances for them as well by default. It requires a bit more fiddling, but it looks nice and is copy-pastable. The original idea (of course) came from ghci and the ocaml repl.

@adriaanm
Copy link
Contributor Author

Dotty has a Show type class in the library. It's used to render rhs in a friendly way

Cool, thanks for the pointer! Will be nice to have this as part of the std lib. (Maybe even in 2.13? /cc @szeiger)

@adriaanm
Copy link
Contributor Author

🎈

@adriaanm adriaanm merged commit c04f14e into scala:2.13.x Jun 26, 2017
@som-snytt
Copy link
Contributor

Just back from vacation and look fwd to trying it out. Is there an initiative to provide a uniform API for Scala 2 and 3/dotty? I saw a comment about a dotr rewrite, but I didn't see if unification was a goal. Choices like jline or not would be nice to decouple from folks writing REPL support. Can I use Amirite I mean Ammonite with Scala 2 or 3-alpha in a pluggable way?

@adriaanm
Copy link
Contributor Author

Welcome back! I'd very much like for us to converge on a compiler-backed API that can be provided uniformly across versions, as well as consumed by multiple frontends.

@adriaanm
Copy link
Contributor Author

Consider this the proto-strawman proposal.

@lrytz lrytz added the release-notes worth highlighting in next release notes label Jun 27, 2017
This was referenced Jun 27, 2017
Closed
Closed
@retronym retronym mentioned this pull request Jul 7, 2017
Closed
37 tasks
eed3si9n added a commit to eed3si9n/zinc that referenced this pull request Nov 21, 2017
@eed3si9n
Implement compiler bridge for 2.13.0-M2
0fd57e5 
Fixes sbt#395, sbt/sbt#3427

In scala/scala#5903 Scala compiler's REPL-related classes went through some changes, including move to a different package.
This implements a new compiler bridge tracking the changes.

To verify that the new bridge compiles under 2.13, we need to compile it using sbt 1.0.3, which in turn requires a bridge compatible with Scala 2.13.0-M2.
To work around this chicken-egg, I've manually created a bridge and published it to Maven Central as "org.scala-sbt" % "compiler-bridge_2.13.0-M2" % "1.1.0-M1-bootstrap2".
eed3si9n added a commit to eed3si9n/zinc that referenced this pull request Nov 21, 2017
@eed3si9n
Implement compiler bridge for 2.13.0-M2
c60ff23 
Fixes sbt#395, sbt/sbt#3427

In scala/scala#5903 Scala compiler's REPL-related classes went through some changes, including move to a different package.
This implements a new compiler bridge tracking the changes.

To verify that the new bridge compiles under 2.13, we need to compile it using sbt 1.0.3, which in turn requires a bridge compatible with Scala 2.13.0-M2.
To work around this chicken-egg, I've manually created a bridge and published it to Maven Central as "org.scala-sbt" % "compiler-bridge_2.13.0-M2" % "1.1.0-M1-bootstrap2".
adriaanm
adriaanm commented Jan 23, 2018
View reviewed changes
src/repl-frontend/scala/tools/nsc/Interpreter.scala
import scala.tools.nsc.interpreter.shell.{ILoop, ReplReporterImpl, ShellConfig}
import scala.tools.nsc.reporters.Reporter

// Pretty gross contortion to satisfy the de facto interface expected by sbt.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

/cc @smarter: this is the grossness we use to integrate with sbt ;-)

@hrhino hrhino mentioned this pull request Jun 5, 2018
Merged
eed3si9n added a commit to eed3si9n/scala that referenced this pull request May 14, 2019
@eed3si9n
Implement compiler bridge for 2.13.0-M2
b5c52a0 
Fixes scala#395, sbt/sbt#3427

In scala#5903 Scala compiler's REPL-related classes went through some changes, including move to a different package.
This implements a new compiler bridge tracking the changes.

To verify that the new bridge compiles under 2.13, we need to compile it using sbt 1.0.3, which in turn requires a bridge compatible with Scala 2.13.0-M2.
To work around this chicken-egg, I've manually created a bridge and published it to Maven Central as "org.scala-sbt" % "compiler-bridge_2.13.0-M2" % "1.1.0-M1-bootstrap2".
lrytz pushed a commit to lrytz/scala that referenced this pull request Nov 5, 2019
@eed3si9n
Implement compiler bridge for 2.13.0-M2
b29e3e1 
Fixes scala#395, sbt/sbt#3427

In scala#5903 Scala compiler's REPL-related classes went through some changes, including move to a different package.
This implements a new compiler bridge tracking the changes.

To verify that the new bridge compiles under 2.13, we need to compile it using sbt 1.0.3, which in turn requires a bridge compatible with Scala 2.13.0-M2.
To work around this chicken-egg, I've manually created a bridge and published it to Maven Central as "org.scala-sbt" % "compiler-bridge_2.13.0-M2" % "1.1.0-M1-bootstrap2".

Rewritten from sbt/zinc@c60ff23
@SethTisue SethTisue mentioned this pull request Feb 11, 2020
Closed
@lrytz lrytz mentioned this pull request Nov 14, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dwijnand dwijnand dwijnand left review comments

@som-snytt som-snytt som-snytt left review comments

@retronym retronym retronym approved these changes

@lrytz lrytz lrytz approved these changes

Assignees
No one assigned
Labels
release-notes worth highlighting in next release notes
Projects
None yet
Milestone
2.13.0-M2
8 participants
@adriaanm @som-snytt @eed3si9n @retronym @dwijnand @lrytz @felixmulder @scala-jenkins