-
Notifications
You must be signed in to change notification settings - Fork 105
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
base: main
Are you sure you want to change the base?
Scala cli support #997
Conversation
7bf703c
to
1ce9d10
Compare
import io.circe.generic.semiauto._ | ||
import io.circe._ | ||
|
||
sealed trait ScalaTarget { |
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.
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 { |
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.
Same here, this does not make sense to me. But maybe you need to keep it for compatibility.
Handling all the possible exceptions remains to be done
This also contains a beginning of a timeout implementation
* Run now directly by the runner, add cancelation * Refactor 🎉
* 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
* 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)
f4ad681
to
2849579
Compare
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.
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.
The packages are renamed to match other Scalacenter repositories. Proper kudos was added in readme section.
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:
Lastly, let's talk about the chosen solutions in this PR:
It will first be deployed on our staging.