-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
improvement: Small fixes to allow using Metals with scaladoc with sbt #16816
Conversation
@@ -1,7 +1,4 @@ | |||
// Used by VersionUtil to get gitHash and commitDate | |||
libraryDependencies += "org.eclipse.jgit" % "org.eclipse.jgit" % "4.11.0.201803080745-r" | |||
|
|||
|
|||
Compile / unmanagedSourceDirectories += |
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.
I think this is no longer needed and was causing some issues when importing all projects within sbt.
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.
So is this supposed to be working when using Bloop or sbt as your build server? I was excited to try this out, but sbt as your server won't work since bspEnabled := false
is still brought in by commonBootstrappedSettings
and trying with Bloop still just results in the following for me:
2023.02.06 17:06:29 WARN no build target for: /Users/ckipp/Documents/scala-workspace/dotty/scaladoc/src/dotty/tools/scaladoc/DRI.scala
I'm using 0.11.10+114-19c22191-SNAPSHOT
so I should have the changes necessary in Metals.
This should work in sbt, there is an issue with Bloop that's related to the bridges PR that Alex created (though I hope to make an easier approach here).
You need to set it to true, I didn't want to change it since for most people working on the compiler it might mean a lot mroe compilations (I think 3 times compiling the compiler?) |
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.
I think we should add a note, but other than that, LGTM! I just did #16846 with this ha, since I've been wanting to tackle that for a while but was annoyed without editor help. So thanks!
This would require just setting `bspEnabled := true`, but not for sjsJUnitTests which seem to cause inifinite loop of compilations. I also need to work a bit on Bloop to also make it work. This still needs scalameta/metals#4938
e21ab05
to
7f6a2f7
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.
LGTM!
This would require just setting
bspEnabled := true
, but not for sjsJUnitTests which seem to cause inifinite loop of compilations.I also need to work a bit on Bloop to also make it work.
This still needs scalameta/metals#4938