Better support for mill BSP #3308
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ckipp01
commented
Nov 28, 2021
| @@ -334,7 +328,3 @@ class BspConnector( | |||
| } | |||
| } | |||
| } | |||
|
|
|||
| object BspConnector { | |||
| final val BLOOP_SELECTED = "BLOOP" | |||
There was a problem hiding this comment.
I realized we probably don't even need this, so I ripped it out of all the places and now just use Bloop from BloopServers.name. It simplifies it a bit.
| @@ -282,14 +281,8 @@ class BspConnector( | |||
| args => bspConfigGenerator.runUnconditionally(buildTool, args) | |||
| ) | |||
| .map(status => handleGenerationStatus(buildTool, status)) | |||
| case Right(connectionDetails) | |||
There was a problem hiding this comment.
I think this may have been what @dos65 was hinting at in my previous pr, but I realized this also isn't needed here.
| this.major > that.major || | ||
| (this.major == that.major && this.minor > that.minor) || | ||
| (this.major == that.major && this.minor == that.minor && this.patch > that.patch) || | ||
| // 3.0.0-RC1 > 3.0.0-M1 | ||
| this.releaseCandidate.isDefined && that.milestone.isDefined || | ||
| baseVersionEqual && this.releaseCandidate.isDefined && that.milestone.isDefined || |
There was a problem hiding this comment.
So I realized the logic here was missing a bit since previously it was showing that SemVer.isCompatibleVersion("0.10.0-M4", "0.9.10") was true since it wasn't correctly handling the cases where there was a major/minor/patch different plus one was a milestone or release candidate. You. can see what I mean more clearly in the tests I added for this.
ckipp01
force-pushed
the
millBsp
branch
2 times, most recently
from
26bea0c
to
d6c914c
Compare
tgodzik
approved these changes
Nov 29, 2021
dos65
approved these changes
Nov 29, 2021
metals/src/main/scala/scala/meta/internal/builds/BuildServerProvider.scala
Outdated
Show resolved
Hide resolved
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds the same type of functionality that we have with sbt where Mill can now also be a build server meaning that
metals.generate-bsp-configand alsometals.bsp-switchwill apply to it. This also cleans up a few things that I did in #3304, fixes a bug inSemver, and also introduces a few small changes inBuildServerProvider.scalathat are needed if a build tool and the name they use as a build server don't match.Relates to com-lihaoyi/mill#1546 and also closes #3307