Skip to content
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

Preferred platform and Scala version in cross-built projects #13

Open
olafurpg opened this issue Feb 8, 2019 · 9 comments
Open

Preferred platform and Scala version in cross-built projects #13

olafurpg opened this issue Feb 8, 2019 · 9 comments

Comments

@olafurpg
Copy link
Member

olafurpg commented Feb 8, 2019

Currently, Metals uses a basic heuristic to select a default built target when multiple targets are associated with a single source file. For example, when a user edits a shared source file for Scala.js and JVM build targets Metals automatically picks the JVM build target and the users can't override/customize that choice.

It would be good if Metals exposed some control over the default heuristic via configuration, for example:

  • Preferred platform: [JVM, JS, Native]
  • Preferred Scala version: [2.12, 2.11, 2.13]

Users could reorder those lists to for example tell Metals to prefer JS on 2.11 over JVM 2.12.

@olafurpg
Copy link
Member Author

Related discussion in https://gitter.im/scalameta/metals?at=5cc167b54b4cb471d953beaa on the expected behavior of "preferred platform"

olafurpg pushed a commit to olafurpg/metals that referenced this issue Sep 27, 2019
Previously, if two build targets in the repo were eligible for a source
file, Metals would sometimes prioritize a target with an unsupported
Scala version over a target with a supported Scala version. This
resulted in completions/hover/parameterHints not to work while editing
cross-built sources.

This PR introduces a change to the logic where we select a build target
for a given source file. Now we sort the elibible candidates by two
criteria: the Scala version and the platform (JVM/JS/Native). This
change takes the first step towards
scalameta/metals-feature-requests#13, which is
about making this logic configurable.
olafurpg pushed a commit to olafurpg/metals that referenced this issue Sep 27, 2019
Previously, if two build targets in the repo were eligible for a source
file, Metals would sometimes prioritize a target with an unsupported
Scala version over a target with a supported Scala version. This
resulted in completions/hover/parameterHints not to work while editing
cross-built sources.

This PR introduces a change to the logic where we select a build target
for a given source file. Now we sort the elibible candidates by two
criteria: the Scala version and the platform (JVM/JS/Native). This
change takes the first step towards
scalameta/metals-feature-requests#13, which is
about making this logic configurable.
@olafurpg
Copy link
Member Author

First step towards implementing this feature scalameta/metals#948. The next step would be to expose configuration to to customize the priority when selecting a single build target from multiple candidates.

@ekrich
Copy link

ekrich commented Apr 18, 2020

Background

I was having difficulty with the cross build for Dotty/JVM on https://github.com/ekrich/sconfig using sbt.

2020.04.17 10:00:38 WARN  no build target for: /Users/eric/workspace/sconfig/sconfig/sharedScala3/src/main/scala/org/ekrich/config/ConfigSyntax.scala

The problem here is there is no direct way to separate scala-2 and scala-0/scala-3 source that are not portable such as enum like you can for individual Scala 2 versions like scala-2.11, scala-2.12 etc. in sbt.

Making this work is done via the following method in sbt.
https://github.com/ekrich/sconfig/blob/master/build.sbt#L185-L189

Workaround

The method as recommended here works but is tricky. ☝️ April 17, 2020 7:32 AM

You need to Connect to build server after the method suggested and then wait for the re-index.

Idea

Allow the user to use a drop down to select the Platform and version when using Metals?

  • Have BSP return all the project versions, JVM 2.13, JS 2.12, Native 2.11 etc.

  • Have BSP return the current version - this is similar to what you suggest as a heuristic above. There is a ThisBuild / scalaVersion and I am not sure if the first crossProject(JVMPlatform, NativePlatform, JSPlatform) is available as a preference.

  • Allow Metals to set the current version via BSP. Then the developer could select a version (via a dropdown?) in the editor via Metals and if you selected Native 2.11 or JVM 0.23.0-RC1 then it would be set in the Build Server and re-index?

@olafurpg
Copy link
Member Author

BSP is already able to represent cross-built projects. The reason you get "no build target" for Dotty sources is that sbt bloopInstall only exports the build for one scalaVersion. In Mill, the full cross-build is exported to Bloop. I'm not sure yet which approach is better because it introduces a large overhead to export the full cross-build (you need to download dependencies for all Scala versions, you generate more projects, ...).

Cross-building is arguably a power-user feature so I think the current situation with sbt where we only export one Scala version it not too bad. There are several available solutions:

  • Users can export a custom scalaVersion by manually running sbt ++2.13.1 bloopInstall from the terminal and then execute "Connect to build server" in Metals.
  • If you have a lot of Dotty-only sources, then you can manually create a non-publishable module with scalaVersion := dottyVersion like we do in the Metals build https://github.com/scalameta/metals/blob/9d6f2e99e1b09904b38b72393bd8b123677041dc/build.sbt#L320 The benfit of this approach is that it also works out of the box with IntelliJ.

@ekrich
Copy link

ekrich commented Apr 21, 2020

I am totally happy with the ability to select a version in sbt above and "Connect to build server" to be able to deal with Dotty and or other versions for the JVM. I am also not sure how many people have cross builds but maybe that will be more popular with people experimenting by making their apps or libraries work with Dotty.

Any sort of feature that would allow the user to select a platform and version would be welcome of course so that metals would communicate to the build server to set the platform and version. This would also require some sort of UI work too.

I do think that Metals should only work with one version at a time to minimize the work it needs to do.

It would be neat if you could somehow select native, 2.11.12 for example and then when you run a main or tests, Metals would run the Native ones, or JS or JVM depending on which is selected given that full Native support is supported in the future.

@olafurpg
Copy link
Member Author

Cross-building remains a power-user feature so it's difficult to prioritize improvements to the UX in the scenario when there exists a decent workaround. I think the highest priority issue at the moment is to make Metals accessible to Scala beginners who may not be experienced developers, or familiar with sbt or the Scala tooling ecosystem.

@ekrich
Copy link

ekrich commented Apr 22, 2020

I totally agree, just wanted to mention it in case someone wanted to tackle it.

@mdedetrich
Copy link

I guess I have a contrarian view here, I work a fair bit with cross built projects amongst multiple Scala versions and especially when dealing with Scala 2/Scala 3 projects I do think that the default should be to create a bloop definition for all of the projects Scala versions.

My argument for this is that otherwise the default user experience is quite terrible, for a while I thought my metal's install was broken when working on a cross Scala 2/ Scala 3 project until I happened to come across this issue (running sbt +bloopInstall solved my specific issue).

At least to me, making bloop/metals support crossScalaVersions seems like low hanging fruit for me especially if mill happens to support it. I would be happy even if its behind a configuration value that is by default set to off if downloading dependencies really is a big concern (although I would still argue that by default it should be on with the option of setting it to off).

@mdedetrich
Copy link

I have created a currently in draft PR at scalameta/metals#5637 to prototype the solution

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants