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

Scala cli support #997

Open
wants to merge 50 commits into
base: main
Choose a base branch
from
Open

Conversation

rochala
Copy link
Collaborator

@rochala rochala commented Dec 20, 2023

First, I want to thank Marwan @Maeeen for adding the initial support for Scala-CLI 🎉

Now, let's talk about this PR.
It contains few major improvements and refactors. They had to be done sooner or later, so I took the opportunity to address multiple issue at once.

  1. We've dropped play-json in favor of circe. The reason is, Scastie had a technical debt and long awaiting migration of the database, as many codecs were adjusted to support all snippets. Now we have only derived codecs, which will simplify the development in a major way. If you're wondering why I also migrated to circe, it has better support for http4s.

  2. The packages are renamed to match other Scalacenter repositories. Proper kudos was added in readme section.

  3. The database migration script is included in this PR, which will handle all the above. It had to happen anyway, as we now have abstraction over snippet, whether it is Sbt one or Scala-CLI one.

Now lets move to the next part.
This PR doesn't address all the problems. After this PR is merged the following PR's will be added:

  • reformat of the codebase - I didn't want to clutter this PR even more,
  • migration to akka typed actors,

Lastly, let's talk about the chosen solutions in this PR:

  • Scala-CLI communicates over BSP. To make running of snippets separate of build tool, we're running them manually. Sadly it is not as fast as we hoped, so in the future, we should take effort to reuse JVM.
  • Metals needs to know dependencies and Scala version. There were 2 approaches - 1st, send a task to BSP and then pass it to metals or parse it manually. I chose the latter as metals runner is already under heavy load. That said, when Scala-cli changes the directives, metals can stop working for them. We should be able to handle it in a better fashion, when proper library is extracted from Scala-CLI, which would allow us to manually extract directives at client level.

It will first be deployed on our staging.

@rochala rochala requested a review from adpi2 December 20, 2023 10:00
balancer/src/main/scala/org/scastie/balancer/Server.scala Outdated Show resolved Hide resolved
balancer/src/main/scala/org/scastie/balancer/Server.scala Outdated Show resolved Hide resolved
api/src/main/scala/org/scastie/api/SbtState.scala Outdated Show resolved Hide resolved
import io.circe.generic.semiauto._
import io.circe._

sealed trait ScalaTarget {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This definition of ScalaTarget is really weird. It mixes 3 notions: the build tool (sbt or Scala CLI), the language version (Scala 2, Scala 3 and the Typelevel Scala) and the platform (JVM, Native or JS).

I propose to redefine it like that:

enum ScalaTarget:
  case ScalaCli
  case Sbt(scalaVersion: ScalaVersion, platform: Platform)

enum ScalaVersion:
  case Scala2(scalaVersion: String)
  case Scala3(scalaVersion: String)
  case Typelevel(scalaVersion: String)

enum Platform:
  case Jvm
  case ScalaJs(scalaJsVersion: String)
  case ScalaNative(scalaNativeVersion: String)


import org.scastie.api

sealed trait ScalaTargetType {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, this does not make sense to me. But maybe you need to keep it for compatibility.

sbt-launch.jar Outdated Show resolved Hide resolved
Maeeen and others added 25 commits September 14, 2024 13:48
* base of ui

* quick import fix

* remove println

* Machin

* Parsing works (please dont run this on your machine, it loops)

* Commit from my laptop

* Fix runner with Scala-CLI and beginning of metals support

* Make MetalsDispatcher return the config

* Client now understands Metals

* METALS NOW WORKS!!!

* Fix of convert to scala cli

* Remove print statemtents
* Notes for test

* tests
* Update the LB to handle SBT and SCli snippets differently

* Fix of some small bugs (scalacenter#7)
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

Successfully merging this pull request may close these issues.

3 participants