aml-org / als

Language Server implementation for AML and AML-defined metadata
Other
27 stars 9 forks source link

--systemStream mode emits non LSP text, which breaks neovim #893

Open mhanberg opened 1 year ago

mhanberg commented 1 year ago

Your issue may already be reported! Please search on Github issues before creating one.

└─  als --systemStream
 DEBUG Main:main    Building LanguageServerImpl
 DEBUG ResolutionTaskManager:Processing request    changeState: PROCESSING_PROJECT
 DEBUG ResolutionTaskManager:Processing request    changeState: IDLE
 DEBUG WorkspaceList:buildWorkspaceAt    created default WorkspaceContentManager
 DEBUG ParserStagingArea:enqueue    enqueueing [CHANGE_CONFIG - ]
 DEBUG WorkspaceContentManager:Processing request    changeState: PROCESSING_PROJECT
 DEBUG WorkspaceContentManager:processTask    Tree unit:
 DEBUG Main:main    Launching services
 DEBUG Main:main    Connecting Client
 DEBUG WorkspaceContentManager:processTask    units for main file: [no main file]
 DEBUG WorkspaceContentManager:Processing request    changeState: PROCESSING_PROJECT
 DEBUG WorkspaceContentManager:processChangeConfigChanges    Processing Config Changes
 DEBUG WorkspaceContentManager:Processing request    next
 DEBUG WorkspaceContentManager:Processing request    changeState: IDLE
 DEBUG WorkspaceContentManager:init    Finished initialization for workspace at ''

It should only emit the rpc messages on stdout so that it can work with stdio lsp clients correctly.

I am using the following script and neovim code to start the LSP

#!/usr/bin/env bash

ALS_PATH=${ALS_PATH:-"$HOME/src/als/als-server/jvm/target/scala-2.12/als-server-assembly-5.4.0-SNAPSHOT.jar"}

java -jar "$ALS_PATH" "$@"
vim.api.nvim_create_autocmd("FileType", {
  group = vim.api.nvim_create_augroup("als", { clear = true }),
  pattern = "raml",
  callback = function()
    vim.lsp.start {
      name = "ALS",
      cmd = { "als", "--systemStream" },
      settings = {},
    }
  end,
})
llibarona commented 1 year ago

Hi Mitchell, by default we log into stdout How are you using ALS? is there a possibility for you to change one line of code in order to see if it solves your issues? in als-server/jvm/src/main/scala/org/mulesoft/als/server/lsp4j/Main.scala there is a line that says: val logger: Logger = PrintLnLogger if you are able to test this, you can change the logger to EmptyLogger instead and see if it solves your problem

Either way we will look into this issue, but it may be a quicker solution if this works for you

mhanberg commented 1 year ago

@llibarona I appreciate the quick response!

I currently have cloned the repository and built it using the instructions (which worked first try!).

I can make that modification locally to see if it works!

I would like to just be able to install it with homebrew brew install als, but i saw it wasn't on homebrew and I didn't have the time to create my own tap right now.

I would prefer to use the --systemStream option, as i think it will be more reliable, but i was able to get the tcp socket mode working with this. (there are some bits that are particular to my config but they are unrelated).

autocmd("FileType", {
  group = random,
  pattern = "raml",
  callback = function()
    local jobid = vim.fn.jobstart("als --port 9000 --listen")

    if jobid > 0 then
      vim.wait(1000, function() end)
      vim.lsp.start {
        name = "ALS",
        cmd = vim.lsp.rpc.connect("127.0.0.1", 9000),
        root_dir = vim.fs.dirname(vim.fs.find(".git", { upward = true })[1]),
        settings = {},
        capabilities = require("motch.lsp").capabilities,
        on_attach = require("motch.lsp").on_attach,
      }
    else
      vim.notify("Couldn't start ALS", vim.log.levels.WARN)
    end
  end,
})
mhanberg commented 1 year ago

@llibarona I made the following patch and it works now! I know nothing about Scala, but I believe this just logs to stderr instead of stdout.

diff --git a/als-common/shared/src/main/scala/org/mulesoft/als/logger/PrintlnLogger.scala b/als-common/shared/src/main/scala/org/mulesoft/als/logger/PrintlnLogger.scala
index f54f3e83a..cfa0fb57f 100644
--- a/als-common/shared/src/main/scala/org/mulesoft/als/logger/PrintlnLogger.scala
+++ b/als-common/shared/src/main/scala/org/mulesoft/als/logger/PrintlnLogger.scala
@@ -10,7 +10,7 @@ import scala.scalajs.js.annotation.{JSExportAll, JSExportTopLevel}
 @JSExportTopLevel("PrintLnLogger")
 object PrintLnLogger extends AbstractLogger {
   protected def executeLogging(msg: String, severity: MessageSeverity.Value): Unit =
-    println(msg)
+    Console.err.println(msg)

   override protected val settings: Option[LoggerSettings] = None
llibarona commented 1 year ago

Great to hear that @mhanberg ! I'm going to create an internal ticket in order to fix this in the future, but I'm happy you are able to use it already

mhanberg commented 1 year ago

awesome, thank you for all the help! Feel free to close this issue if you are tracking it elsewhere.

take care!

llibarona commented 1 year ago

I created the internal ticket: W-12607034 to view this in the future thanks for your great predisposition, and we are eager to hear about and use your extension using ALS!