-
-
Notifications
You must be signed in to change notification settings - Fork 358
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
fix: Errors not cleaning up in BSP for Scala 3 #2810
Conversation
The Scala 3 compiler is not calling `startUnit` as the Scala 2 compiler. I checked what sbt is doing and it sends a build/publishDiagnostics for all the compiled files at the end of compilation. So I'm doing the same here. I removed the `CompileProgress` and I'm calling fileVisited for all input files instead. I tried to not touch the `CompileProblemReporter` trait for now since it's public API and I don't want to deal with the binary compatibility issues. Moreover, our current `fileVisited` API seems more than enough
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.
Looks good to me.
@@ -492,28 +492,6 @@ class ZincWorkerImpl( | |||
.map(path => converter.toVirtualFile(path.toNIO)) | |||
.toArray | |||
|
|||
// We want to listed to all compiled files | |||
val compileProgress = reporter.map { 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.
The issue is most likely here scala/scala3#13082
Wouldn't CompileProgress be useful for reporting % of the compilation or does mill handle it differently?
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.
We are not reporting % of the compilation in Mill yet. I'm creating an issue so we don't forget about it.
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.
Thanks! Anyway, this needs the fix for Scala 3, but otherwise we can probably copy the logic from Bloop
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.
Issue created here: #2816
The Scala 3 compiler is not calling
startUnit
as the Scala 2 compiler. I checked what sbt is doing and it sends a build/publishDiagnostics for all the compiled files at the end of compilation. So I'm doing the same here.I removed the
CompileProgress
and I'm calling fileVisited for all input files instead.I tried to not touch the
CompileProblemReporter
trait for now since it's public API and I don't want to deal with the binary compatibility issues. Moreover, our currentfileVisited
API seems more than enough.Fixes #2426
Pull request: #2810