-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Remove differences in diagnostic printing in the REPL #13266
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
Conversation
6909f49
to
d2b2c46
Compare
c1944c1
to
4c8ca9a
Compare
4c8ca9a
to
235d367
Compare
Who's the closest to being the REPL maintainer? |
235d367
to
b220edd
Compare
can you explain the |
+ val repl = taskKey[Unit]("spawns a repl with the correct classpath") // (1)
..
- // Spawns a repl with the correct classpath
- addCommandAlias("repl", "scala3-compiler-bootstrapped/console") // (2)
..
+ repl := (Compile / console).value, // (3)
+ Compile / console / scalacOptions := Nil, // (4)
..
+ repl := (`scala3-compiler-bootstrapped` / repl).value, // (5) Sure. (1), (2), and (3): I was thinking (2) did the double-compilation of bootstrapping and then building a bootstrapped compiler for the REPL, so I was looking to copy how (5): But then the root project doesn't aggregate (4): Finally, I don't want the scalacOptions used in compiling the (bootstrapped) compiler from tainting the out-the-box experience of using the REPL, e.g. by enabling |
b220edd
to
8fd820a
Compare
Rebased because this conflicted with #13000. Notice from the output diff how this PR removes any remaining differences. |
8fd820a
to
aaa110b
Compare
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.
The user-facing changes look very good to me. This is a nice improvement in usability.
I'm satisfied by the build.sbt
explanation (to the extent that I understand the dotty build, anyway).
And, Dale walked me through the implementation just now and I see nothing to object to. A few more comments might help future maintainers.
@bjornregnell any thoughts?
Footnote that Scala 2 REPL enables
|
@som-snytt True, but regardless, the sbt task should have the same defaults as what users get. I would be in favor of a separate PR that added |
For example: scala> @deprecated("no", "nah") class C; new C there were 1 deprecation warning(s); re-run with -deprecation for details // defined class C val res0: C = C@4273cda6
aaa110b
to
8f226e5
Compare
@dwijnand the CI failed on the merge commit if this PR. It then failed on nightly. |
Does GitHub Actions run on the branch then, rather than a merge commit? I'll fix up master asap. |
No description provided.