search

emacs-lsp / lsp-metals

lsp-mode :heart: metals
https://emacs-lsp.github.io/lsp-metals
GNU General Public License v3.0
58 stars 33 forks source link

Update fallback version #110

Closed logc closed 3 months ago

logc commented 5 months ago

Using metals version 1.3.1, there is a deprecation message that suggests to update from Scala 3.2.0 to 3.3.3 as fallback version.

Since the 3.3.x series is LTS, I thought it would make sense to have that as a default value in the upstream package, instead of just solving it locally for my projects, or picking 3.2.2 as the closest supported version.

The entire message reads: LSP :: You are using fallback Scala version 3.2.0, which is not supported in this version of Metals. Please upgrade to Scala version 3.3.3.

My metals server, installed with Coursier, outputs these supported versions:


$ metals --version
metals 1.3.1

# Note:
#   Supported Scala versions:
#     Scala 3: 3.1.3, 3.2.2, 3.3.1, 3.3.2, 3.3.3
#     Scala 2:
#       2.11.12
#       2.12.19, 2.12.18, 2.12.17, 2.12.16, 2.12.12, 2.12.13, 2.12.14, 2.12.15
#       2.13.14, 2.13.11, 2.13.12, 2.13.13, 2.13.7, 2.13.8, 2.13.9, 2.13.10```
kurnevsky commented 3 months ago

Sorry, somehow missed this MR. Does it make sense to use automatic instead?

logc commented 3 months ago

I didn't know that automatic is a valid value for that defcustom variable. If that is the case, why isn't that the default for the fallback version, anyway?

I am not adamant about any particular version. An automatically picked version would be better for me. :smile:

wjoel commented 3 months ago

The default is "unset/None", but (from the comments in Metals, https://github.com/scalameta/metals/blob/de38d6b99d208be73b1c6ee6e03cdca4961200d3/metals/src/main/scala/scala/meta/internal/metals/UserConfiguration.scala#L611) VS Code needed a different way to express "use the default".

Not sure if nil would work here, otherwise "automatic" is a good choice. Better than using a fixed value as the default here in lsp-metals, as it overrides Metals' default, and (as we've seen) can break over time as Metals deprecates support for old Scala versions.

logc commented 3 months ago

If there is a better way, by removing the variable altogether, feel free to close my pull request.

kurnevsky commented 3 months ago

Changed to automatic in 0dc938be1190d147e7013e3dce08ac8bff5d1662