laughedelic / atom-ide-scala

:atom: Scala & Dotty support for Atom IDE (🧟‍♂️ zombie repo)
https://atom.io/packages/ide-scala
MIT License
48 stars 10 forks source link

Add server commands #34

Closed Jarlakxen closed 6 years ago

Jarlakxen commented 6 years ago

Implemente https://github.com/laughedelic/atom-ide-scala/issues/27. I putted some of the code in a companion object just to keep the ScalaLanguageClient class small.

laughedelic commented 6 years ago

Hi @Jarlakxen. Is this a work in progress?

Jarlakxen commented 6 years ago

Hi @laughedelic! The work is done for clearIndexCache and resetPresentationCompiler. I noticed a scalafixUnusedImports which is not mentioned in #27. Would you like to open a new issue? or I can try to do it all together.

laughedelic commented 6 years ago

Yes, it's better to add all available commands here. There's actually one more command: sbtConnect.

laughedelic commented 6 years ago

I think that all this can be drastically simplified: once you got supportedCommands, you can just iterate over them and add each of them.

There's a bit tricky part about command names. See Atom docs:

Command names must follow the namespace:action pattern, where namespace will typically be the name of your package, and action describes the behavior of your command. If either part consists of multiple words, these must be separated by hyphens. E.g. awesome-package:turn-it-up-to-eleven. All words should be lowercased.

Here's a draft:

supportedCommands.foreach { cmd =>

  val cmdName = "[A-Z]".r.replaceAllIn(cmd, { "-" + _.group(0).toLowerCase() })

  Atom.commands.add("atom-text-editor", s"metals:${cmdName}", { _ => 
    server.connection.executeCommand(
      new ExecuteCommandParams(command = cmd)
    )
  })
}

@Jarlakxen what do you think about this approach?

Jarlakxen commented 6 years ago

From what I understand that could work for clearIndexCache, resetPresentationCompiler and sbtConnect but not for scalafixUnusedImports. If that is correct, a solution could be define a whitelist of commands without special treatment like clearIndexCache, resetPresentationCompiler and sbtConnect and implement scalafixUnusedImports in other place.

laughedelic commented 6 years ago

I'm not sure I get it, why does scalafixUnusedImports need a special treatment?

Jarlakxen commented 6 years ago

I saw this part of the code https://github.com/scalameta/metals/blob/master/metaserver/src/main/scala/scala/meta/languageserver/ScalametaServices.scala#L367 and OrganizeImports.removeUnused(params.arguments, symbolIndex) use params: ExecuteCommandParams. That arguments contains a TextDocumentIdentifier. Maybe I didn't understand the functionality but from what I understand I need to pass a reference to the file from which I want to remove the unused imports.

laughedelic commented 6 years ago

I see. I think this is also solvable, but let's keep things simple for now and just skip this command:

laughedelic commented 6 years ago

@Jarlakxen I just tried it out with the latest published server and it seems to work fine. At least I see that it receives the command request. Let's merge it!