Let VSCode start the language server #4304

Duhemm · 2018-04-12T17:38:01Z

We used to start the language server with launchIDE. However, this
means that the language server is dependent of sbt being running and
configured, which is not a good user experience for newcomers.

For newcomers, we want the language server to start when they create a
new Scala file in an empty directory, for instance, which means that no
build is already configured.

This commits takes us closer to this experience by using load-plugin
to inject the Dotty plugin. This means that the Dotty plugin can now be
loaded even if there's no build configured, which happens if the user
opens VSCode in an empty directory, for instance.

We use the recently introduced --addPluginSbtFile to provide this experience.

Duhemm · 2018-04-12T17:39:25Z

vscode-dotty/src/extension.ts

@@ -17,12 +17,9 @@ export function activate(context: ExtensionContext) {
  outputChannel = vscode.window.createOutputChannel('Dotty Language Client');

  const artifactFile = `${vscode.workspace.rootPath}/.dotty-ide-artifact`
+  const defaultArtifact = "ch.epfl.lamp:dotty-language-server_0.8:0.8.0-bin-SNAPSHOT"


If this get in, we'd need to find a way to keep these values up-to-date with the reality. When's the vscode extension published? Every 6 weeks, too?

You can get the current nightly base of dotty from http://dotty.epfl.ch/versions/latest-nightly-base, we could also have a file to get the current release version.

Duhemm · 2018-04-12T17:40:04Z

vscode-dotty/src/extension.ts

+      configureIDE().then((res) => {
+        run({
+          command: "java",
+          args: ["-classpath", classPath, "dotty.tools.languageserver.Main", "-stdio"]


Any reason why we extract the classpath with coursier fetch and then run this way rather than doing coursier launch?

We display a progress notification while coursier fetch is running, if we use coursier launch we don't know when downloading has finished.

smarter · 2018-04-12T17:43:13Z

vscode-dotty/package.json

@@ -24,6 +24,7 @@
  ],
  "main": "./out/src/extension",
  "activationEvents": [
+    "onLanguage:scala",


The problem with doing this is that we conflict with every other language server for Scala

What can we do instead? The goal of this PR is to be able to get the Dotty IDE working without any prior setup (so, nobody would have generated .dotty-ide.json).

Now, if there's no configuration for the IDE a message is shown asking the user whether she wants to start the IDE or not. If there's a conflict with another language server, she can just say no. If the IDE is configured, then it'll automatically start up.

smarter · 2018-04-12T17:48:53Z

vscode-dotty/package.json

@@ -51,7 +51,7 @@
    "vscode:prepublish": "npm install && ./node_modules/.bin/tsc -p ./",
    "compile": "./node_modules/.bin/tsc -p ./",
    "test": "node ./node_modules/vscode/bin/test",
-    "postinstall": "node ./node_modules/vscode/bin/install && curl -L -o out/coursier https://github.com/coursier/coursier/raw/v1.0.0/coursier && curl -L -o out/load-plugin.jar https://github.com/scalacenter/load-plugin/releases/download/v0.1.0/load-plugin_2.12-0.1.0.jar"
+    "postinstall": "node ./node_modules/vscode/bin/install && curl -L -o out/coursier https://github.com/coursier/coursier/raw/v1.0.0/coursier && curl -L -o out/load-plugin.jar https://oss.sonatype.org/content/repositories/releases/ch/epfl/scala/load-plugin_2.12/0.1.0+2-496ac670/load-plugin_2.12-0.1.0+2-496ac670.jar"


Can't you use coursier to fetch the load-plugins jar?

allanrenucci · 2018-04-12T18:45:41Z

vscode-dotty/src/extension.ts

+
+    sbtProc.on('close', (code: number) => {
+      if (code != 0) {
+        let msg = "Configuring the IDE failed."


const msg?

laughedelic · 2018-04-12T19:42:41Z

It's cool to see load-plugin in action 👏 But I expected that it happens in the server:

user opens a file in the client (editor)
client launches server (either reading from .dotty-ide-artifact or using some default)
server checks if .dotty-ide.json is in place, if not runs load-plugin and configureIDE

This way client implementation stays minimal: it only needs to launch server using coursier. And this is important IMO because other clients may or may not reimplement the same thing: users experience will vary. If server takes care of its own automatic setup, then clients are simple and users get same awesome experience regardless of their editor of choice.

P.S. I don't know any details about Dotty LS so probably there's a good reason for doing it this way and what I say is completely invalid.

Duhemm · 2018-05-14T15:07:56Z

Hi @laughedelic!

I agree that this could be pushed to the server implementation rather than the client, but I think it has been done this way for two reasons:

Make the client more interactive (for instance, show that it is downloading stuff). I'm not sure how this could be done if the server was the one in charge of downloading sbt, and load-plugin, but LSP may very well have something for that...
Have fewer dependencies in the LSP server. I think that coursier comes with a ton of dependencies, unfortunately.

I'd be open to try the approach that you suggest. Having the smallest client possible is definitely a big bonus. I think that @smarter may be able to shed more light on why the client works the way it does.

Duhemm · 2018-05-14T15:11:53Z

@smarter I think I've addressed your comments, apart from the one regarding the activationEvents, because I don't know what to do.

The goal is to be able to start the Dotty LSP and automatically configure it when the user opens a .scala file, so I don't see any way around the conflict with other LSP servers.

smarter · 2018-05-14T19:59:31Z

First we need to actually test how language server interact in VSCode. What happens when I have both one of the Scala 2 LS and the Dotty LS with this PR enabled at the same time, in a Scala 2 project? The Dotty LS shouldn't be doing anything since the sbt plugin won't find any Dotty project, so it's possible that things "just work". If they don't, it might help to actually shut down the Dotty LS if no Dotty project has been found, that way only one LS would run at a given time for a given project.

Duhemm · 2018-06-11T12:29:55Z

I changed it so that the extension will ask the user whether the IDE should be started when it detects that there's no configuration (no .dotty-ide.json, no build.sbt) :

Duhemm · 2018-08-17T07:43:03Z

I updated the PR to use --addPluginSbtFile.

smarter · 2018-09-14T14:25:28Z

vscode-dotty/src/extension.ts

+      runLanguageServer(coursierPath, languageServerArtifactFile)
+    } else if (!fs.existsSync(buildSbtFile)) {
+      vscode.window.showInformationMessage(
+          "This looks like an unconfigured project. Would you like to start Dotty IDE?",


"an unconfigured Scala project", "start the Dotty IDE"

Also maybe remember the choice of the user by creating an empty file ".dotty-ide-disabled" ?

smarter · 2018-09-14T14:25:46Z

vscode-dotty/src/extension.ts

-        if (err) {
-          outputChannel.append(`Unable to parse ${portFile}`)
-          throw err
+  const sbtArtifact = "org.scala-sbt:sbt-launch:1.2.0"


Upgrade to 1.2.3

smarter · 2018-09-14T14:28:40Z

vscode-dotty/src/extension.ts

+function configureIDE(sbtClasspath: string, languageServerScalaVersion: string, dottyPluginSbtFile: string) {
+  return vscode.window.withProgress({
+    location: vscode.ProgressLocation.Window,
+    title: 'Configuring IDE...'


"Configuring the IDE for Dotty"

smarter · 2018-09-14T14:29:20Z

project/Build.scala

        val packageJson = workingDir / "package.json"
+        // val _ = (managedResources in Compile).value


Can be deleted.

We used to start the language server with `launchIDE`. However, this means that the language server is dependent of sbt being running and configured, which is not a good user experience for newcomers. For newcomers, we want the language server to start when they create a new Scala file in an empty directory, for instance, which means that no build is already configured. This commits takes us closer to this experience by using `load-plugin` to inject the Dotty plugin. This means that the Dotty plugin can now be loaded even if there's no build configured, which happens if the user opens VSCode in an empty directory, for instance.

Use `if-absent` so that we don't load the plugins and apply transformations to the build if this is not necessary.

And retrieve everything (sbt, dotty-language-server, load-plugin) in parallel.

When the extension detects that this is an un-configured project (no `.dotty-ide-artifact`, no `build.sbt`), it shows a message asking the user whether she wants the IDE to be started.

Duhemm · 2018-09-14T15:42:22Z

@smarter Comments addressed.

smarter · 2018-09-14T15:44:52Z

OK, feel free to merge.

Duhemm added itype:enhancement stat:wip area:ide labels Apr 12, 2018

Duhemm commented Apr 12, 2018

View reviewed changes

smarter reviewed Apr 12, 2018

View reviewed changes

allanrenucci reviewed Apr 12, 2018

View reviewed changes

Duhemm force-pushed the topic/start-lsp-from-vscode branch from 6fd30e8 to 02ce769 Compare May 14, 2018 12:32

Duhemm added stat:needs review and removed stat:wip labels May 14, 2018

Duhemm changed the title ~~[WIP] Let VSCode start the language server~~ Let VSCode start the language server May 14, 2018

allanrenucci removed the stat:needs review label May 28, 2018

allanrenucci assigned Duhemm May 28, 2018

Duhemm force-pushed the topic/start-lsp-from-vscode branch from 02ce769 to 462e850 Compare June 11, 2018 12:25

Duhemm force-pushed the topic/start-lsp-from-vscode branch from 462e850 to cfcacc1 Compare August 17, 2018 07:39

Duhemm added the stat:needs review label Sep 4, 2018

Duhemm assigned smarter and unassigned Duhemm Sep 4, 2018

smarter approved these changes Sep 14, 2018

View reviewed changes

Duhemm added 5 commits September 14, 2018 17:13

Update load-plugin

dd1c52d

Use `if-absent` so that we don't load the plugins and apply transformations to the build if this is not necessary.

Retrieve load-plugin with coursier

ed6d567

And retrieve everything (sbt, dotty-language-server, load-plugin) in parallel.

Remove hardcoded versions from vscode extension

03e33c3

Update minimal sbt version

45e37b8

Duhemm added 4 commits September 14, 2018 17:13

Use load-plugin only when there's no build.sbt

993ef22

Show message to start the IDE

76a52c9

When the extension detects that this is an un-configured project (no `.dotty-ide-artifact`, no `build.sbt`), it shows a message asking the user whether she wants the IDE to be started.

Replace load-plugin with --addPluginSbtFile

aadb30f

Address review comments

8a48286

Duhemm force-pushed the topic/start-lsp-from-vscode branch from cfcacc1 to 8a48286 Compare September 14, 2018 15:41

Duhemm merged commit 0a2734b into scala:master Sep 14, 2018

Duhemm deleted the topic/start-lsp-from-vscode branch September 14, 2018 16:04

		val packageJson = workingDir / "package.json"
		// val _ = (managedResources in Compile).value

Let VSCode start the language server #4304

Let VSCode start the language server #4304

Uh oh!

Conversation

Duhemm commented Apr 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

laughedelic commented Apr 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Duhemm commented May 14, 2018

Uh oh!

Duhemm commented May 14, 2018

Uh oh!

smarter commented May 14, 2018

Uh oh!

Duhemm commented Jun 11, 2018

Uh oh!

Duhemm commented Aug 17, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Duhemm commented Sep 14, 2018

Uh oh!

smarter commented Sep 14, 2018

Uh oh!

Uh oh!

Duhemm commented Apr 12, 2018 •

edited

Loading

laughedelic commented Apr 12, 2018 •

edited

Loading