`didChangeConfiguration` not being listened to

ckipp01 commented 10 months ago

Describe the bug

I think this might actually be due to a bunch of BSP errors happening that I reported in https://github.com/VirtusLab/scala-cli/issues/2412, but I've noticed that in this situation even though "enableSemanticHighlighting": false has been turned on, it's still returned. For example here is some of the LSP logs showing this with some things in between redacted.

[Trace - 10:49:19 AM] Received notification 'initialized'
Params: {}

[Trace - 10:49:19 AM] Sending request 'workspace/configuration - (1)'
Params: {
  "items": [
    {
      "scopeUri": "file:///Users/ckipp/Documents/scala-workspace/skan/",
      "section": "metals"
    }
  ]
}

[Trace - 10:49:19 AM] Received notification 'workspace/didChangeConfiguration'
Params: {
  "settings": {
    "metals": {
      "showImplicitArguments": true,
      "superMethodLensesEnabled": true,
      "serverVersion": "latest.snapshot",
      "enableSemanticHighlighting": false,
      "showInferredType": true,
      "showImplicitConversionsAndClasses": true
    }
  }
}

[Trace - 10:49:19 AM] Sending request 'workspace/configuration - (2)'
Params: {
  "items": [
    {
      "scopeUri": "file:///Users/ckipp/Documents/scala-workspace/skan/",
      "section": "metals"
    }
  ]
}

[Trace - 10:49:19 AM] Received notification 'textDocument/didOpen'
Params: {
  "textDocument": {
    "uri": "file:///Users/ckipp/Documents/scala-workspace/skan/skan/src/project.scala",
    "languageId": "scala",
    "version": 0,
    "text": "//\u003e using scala 3.3.1\n//\u003e using jvm 19\n//\u003e using dep com.olvind.tui::tui:0.0.7\n//\u003e using dep com.lihaoyi::upickle:3.1.3\n//\u003e using dep com.lihaoyi::os-lib:0.9.1\n//\u003e using dep dev.dirs:directories:26\n//\u003e using test.dep org.scalameta::munit::0.7.29\n//\u003e using options -deprecation -feature -explain -Wunused:all\n//\u003e using computeVersion git:dynver\n//\u003e using buildInfo\n\npackage skan\n\nimport tui.*\nimport tui.crossterm.Event\nimport tui.crossterm.KeyCode\nimport tui.widgets.ListWidget\n\nimport skan.ui.*\n\n@main def run(): Unit \u003d withTerminal: (jni, terminal) \u003d\u003e\n  val config \u003d Config.load()\n  val contextState \u003d ContextState.fromConfig(config)\n\n  def runBoard(state: ContextState): Unit \u003d\n    terminal.draw(frame \u003d\u003e Board.render(frame, state, config))\n    jni.read() match\n      case key: Event.Key \u003d\u003e\n        key.keyEvent().code() match\n          case char: KeyCode.Char if char.c() \u003d\u003d \u0027q\u0027 \u003d\u003e state.save(config)\n          case char: KeyCode.Char if char.c() \u003d\u003d \u0027k\u0027 \u003d\u003e\n            state.previous()\n            runBoard(state)\n          case char: KeyCode.Char if char.c() \u003d\u003d \u0027j\u0027 \u003d\u003e\n            state.next()\n            runBoard(state)\n          case char: KeyCode.Char if char.c() \u003d\u003d \u0027l\u0027 || char.c() \u003d\u003d \u0027h\u0027 \u003d\u003e\n            val newState \u003d state.switchColumn()\n            runBoard(newState)\n          case char: KeyCode.Char if char.c() \u003d\u003d \u0027n\u0027 \u003d\u003e\n            runNewItem(state, NewItemState.fresh())\n          case char: KeyCode.Char if char.c() \u003d\u003d \u0027x\u0027 \u003d\u003e\n            val newState \u003d state.deleteItem()\n            runBoard(newState)\n          case char: KeyCode.Char if char.c() \u003d\u003d \u0027c\u0027 \u003d\u003e\n            runContextMenu(state, ListWidget.State(selected \u003d Some(0)))\n          case char: KeyCode.Char if char.c() \u003d\u003d \u0027i\u0027 \u003d\u003e\n            runInfo(state)\n          case _: KeyCode.Enter \u003d\u003e\n            state.progress()\n            runBoard(state)\n          case _: KeyCode.Backspace \u003d\u003e\n            state.moveBack()\n            runBoard(state)\n          case _: KeyCode.Tab \u003d\u003e\n            val newState \u003d state.switchContext()\n            runBoard(newState)\n          case _ \u003d\u003e runBoard(state)\n      case _ \u003d\u003e runBoard(state)\n    end match\n  end runBoard\n\n  def runInfo(state: ContextState): Unit \u003d\n    terminal.draw(frame \u003d\u003e Info.render(frame, config))\n    jni.read() match\n      case key: Event.Key \u003d\u003e\n        key.keyEvent().code() match\n          case char: KeyCode.Char if char.c() \u003d\u003d \u0027q\u0027 \u003d\u003e\n            runBoard(contextState)\n          case _ \u003d\u003e runInfo(state)\n      case _ \u003d\u003e runInfo(state)\n\n  def runContextMenu(\n      contextState: ContextState,\n      menuState: ListWidget.State\n  ): Unit \u003d\n    terminal.draw(frame \u003d\u003e ContextMenu.render(frame, contextState, menuState))\n    jni.read() match\n      case key: Event.Key \u003d\u003e\n        key.keyEvent().code() match\n          case char: KeyCode.Char if char.c() \u003d\u003d \u0027j\u0027 \u003d\u003e\n            val index \u003d menuState.selected.getOrElse(0)\n            val selected \u003d\n              if index \u003e\u003d ContextAction.values.size - 1 then 0\n              else index + 1\n            menuState.selected \u003d Some(selected)\n            runContextMenu(contextState, menuState)\n          case char: KeyCode.Char if char.c() \u003d\u003d \u0027k\u0027 \u003d\u003e\n            val index \u003d menuState.selected.getOrElse(0)\n            val selected \u003d\n              if index \u003d\u003d 0 then ContextAction.values.size - 1\n              else index - 1\n            menuState.selected \u003d Some(selected)\n            runContextMenu(contextState, menuState)\n          case char: KeyCode.Char if char.c() \u003d\u003d \u0027q\u0027 \u003d\u003e\n            runBoard(contextState)\n          case _: KeyCode.Enter \u003d\u003e\n            if menuState.selected.isEmpty then runBoard(contextState)\n            else\n              ContextAction.fromOrdinal(menuState.selected.get) match\n                case ContextAction.EditCurrentContext \u003d\u003e\n                  runEditContext(contextState, contextState.activeContext)\n                case ContextAction.DeleteCurrentContext \u003d\u003e\n                  val newState \u003d contextState.deleteContext(config)\n                  runBoard(newState)\n                case ContextAction.CreateNewContext \u003d\u003e\n                  runNewContext(contextState, newContextName \u003d \"\")\n          case _ \u003d\u003e runContextMenu(contextState, menuState)\n      case _ \u003d\u003e runContextMenu(contextState, menuState)\n    end match\n  end runContextMenu\n\n  def runEditContext(contextState: ContextState, newContextName: String): Unit \u003d\n    terminal.draw(frame \u003d\u003e\n      EditContext.render(frame, contextState, newContextName)\n    )\n    jni.read() match\n      case key: Event.Key \u003d\u003e\n        key.keyEvent().code() match\n          case c: KeyCode.Char \u003d\u003e\n            val newText \u003d newContextName + c.c()\n            runEditContext(contextState, newText)\n          case _: KeyCode.Backspace \u003d\u003e\n            if newContextName.isEmpty() then\n              runEditContext(contextState, newContextName)\n            else\n              val newText \u003d\n                newContextName.substring(0, newContextName.length - 1)\n              runEditContext(contextState, newText)\n          case _: KeyCode.Enter \u003d\u003e\n            if newContextName.nonEmpty then\n              val newState \u003d contextState.renameContext(newContextName, config)\n              runBoard(newState)\n            else runBoard(contextState)\n          case _: KeyCode.Esc \u003d\u003e\n            runBoard(contextState)\n          case _ \u003d\u003e runEditContext(contextState, newContextName)\n      case _ \u003d\u003e runEditContext(contextState, newContextName)\n\n  def runNewContext(contextState: ContextState, newContextName: String): Unit \u003d\n    terminal.draw(frame \u003d\u003e\n      EditContext.render(frame, contextState, newContextName)\n    )\n    jni.read() match\n      case key: Event.Key \u003d\u003e\n        key.keyEvent().code() match\n          case c: KeyCode.Char \u003d\u003e\n            val newText \u003d newContextName + c.c()\n            runNewContext(contextState, newText)\n          case _: KeyCode.Backspace \u003d\u003e\n            if newContextName.isEmpty() then\n              runNewContext(contextState, newContextName)\n            else\n              val newText \u003d\n                newContextName.substring(0, newContextName.length - 1)\n              runNewContext(contextState, newText)\n          case _: KeyCode.Enter \u003d\u003e\n            if newContextName.nonEmpty then\n              val newState \u003d\n                contextState.addContext(newContextName, config.boardOrder)\n              runBoard(newState)\n            else runBoard(contextState)\n          case _: KeyCode.Esc \u003d\u003e\n            runBoard(contextState)\n          case _ \u003d\u003e runNewContext(contextState, newContextName)\n      case _ \u003d\u003e runNewContext(contextState, newContextName)\n\n  def runNewItem(contextState: ContextState, state: NewItemState): Unit \u003d\n    def handleNormalMode(keyCode: KeyCode) \u003d\n      keyCode match\n        case c: KeyCode.Char if c.c() \u003d\u003d \u0027i\u0027 \u003d\u003e\n          val newState \u003d state.switchInputMode()\n          runNewItem(contextState, newState)\n        case c: KeyCode.Char if c.c() \u003d\u003d \u0027q\u0027 \u003d\u003e\n          runBoard(contextState)\n        case _ \u003d\u003e runNewItem(contextState, state)\n\n    def handleTextInput(\n        keyCode: KeyCode,\n        char: Char \u003d\u003e Unit,\n        backSpace: () \u003d\u003e Unit\n    ) \u003d\n      keyCode match\n        case _: KeyCode.Esc \u003d\u003e\n          val newState \u003d state.switchInputMode()\n          runNewItem(contextState, newState)\n\n        case c: KeyCode.Char \u003d\u003e\n          char(c.c())\n          runNewItem(contextState, state)\n\n        case c: KeyCode.Backspace \u003d\u003e\n          backSpace()\n          runNewItem(contextState, state)\n\n        case _: KeyCode.Enter \u003d\u003e\n          val newState \u003d state.focusNext()\n          runNewItem(contextState, newState)\n\n        case _ \u003d\u003e runNewItem(contextState, state)\n\n    terminal.draw(f \u003d\u003e NewItem.render(f, state))\n\n    jni.read() match\n      case key: Event.Key \u003d\u003e\n        state.focusedInput match\n          case InputSection.Title \u003d\u003e\n            state.inputMode match\n              case InputMode.Normal \u003d\u003e\n                handleNormalMode(key.keyEvent().code())\n\n              case InputMode.Input \u003d\u003e\n                handleTextInput(\n                  key.keyEvent().code(),\n                  (char: Char) \u003d\u003e state.title \u003d state.title + char,\n                  () \u003d\u003e\n                    if state.title.nonEmpty then\n                      state.title \u003d\n                        state.title.substring(0, state.title.length - 1)\n                    else state.title\n                )\n\n          case InputSection.Description \u003d\u003e\n            state.inputMode match\n              case InputMode.Normal \u003d\u003e\n                handleNormalMode(key.keyEvent().code())\n\n              case InputMode.Input \u003d\u003e\n                handleTextInput(\n                  key.keyEvent().code(),\n                  (char: Char) \u003d\u003e state.description \u003d state.description + char,\n                  () \u003d\u003e\n                    if state.description.nonEmpty then\n                      state.description \u003d state.description\n                        .substring(0, state.description.length - 1)\n                    else state.description\n                )\n\n          case InputSection.Priority \u003d\u003e\n            key.keyEvent().code() match\n              case _: KeyCode.Enter \u003d\u003e\n                if state.title.isEmpty() \u0026\u0026 state.description.isEmpty() then\n                  runBoard(contextState)\n                else\n                  val newState \u003d contextState.withNewItem(\n                    BoardItem.fromInput(\n                      state.title,\n                      state.description,\n                      state.priority\n                    )\n                  )\n                  runBoard(newState)\n              case _: KeyCode.Tab \u003d\u003e\n                val newState \u003d state.copy(priority \u003d state.priority.shift())\n                runNewItem(contextState, newState)\n              case c: KeyCode.Char if c.c() \u003d\u003d \u0027q\u0027 \u003d\u003e\n                runBoard(contextState)\n              case _ \u003d\u003e runNewItem(contextState, state)\n\n      case _ \u003d\u003e runNewItem(contextState, state)\n    end match\n  end runNewItem\n\n  runBoard(contextState)\n"
  }
}

[Trace - 10:49:19 AM] Received request 'textDocument/semanticTokens/full - (2)'
Params: {
  "textDocument": {
    "uri": "file:///Users/ckipp/Documents/scala-workspace/skan/skan/src/project.scala"
  }
}

[Trace - 10:49:22 AM] Received response 'workspace/configuration - (1)' in 2051ms
Result: [
  {
    "showImplicitArguments": true,
    "superMethodLensesEnabled": true,
    "serverVersion": "latest.snapshot",
    "enableSemanticHighlighting": false,
    "showInferredType": true,
    "showImplicitConversionsAndClasses": true
  }
]
Error: null

[Trace - 10:49:22 AM] Received response 'workspace/configuration - (2)' in 2050ms
Result: [
  {
    "showImplicitArguments": true,
    "superMethodLensesEnabled": true,
    "serverVersion": "latest.snapshot",
    "enableSemanticHighlighting": false,
    "showInferredType": true,
    "showImplicitConversionsAndClasses": true
  }
]
Error: null

[Trace - 10:49:22 AM] Sending notification 'window/logMessage'
Params: {
  "type": 4,
  "message": "2023.10.16 10:49:22 INFO  time: Connected to build server in 0.68s"
}

[Trace - 10:49:22 AM] Sending notification 'window/logMessage'
Params: {
  "type": 4,
  "message": "2023.10.16 10:49:22 INFO  Connected to Build server: scala-cli v1.0.4"
}

[Trace - 10:49:22 AM] Sending notification 'window/logMessage'
Params: {
  "type": 4,
  "message": "2023.10.16 10:49:22 INFO  BSP server: [warn]  JVM that is hosting bloop is older than the requested runtime. Please run command `bloop exit`, and then use `--jvm` flag to restart Bloop"
}

[Trace - 10:49:22 AM] Sending notification 'window/logMessage'
Params: {
  "type": 4,
  "message": "2023.10.16 10:49:22 INFO  BSP server: [warn] COMMAND_LINE"
}

[Trace - 10:49:22 AM] Sending notification 'window/logMessage'
Params: {
  "type": 4,
  "message": "2023.10.16 10:49:22 INFO  BSP server: [warn] [current bloop jvm] /Users/ckipp/Library/Caches/Coursier/arc/https/github.com/adoptium/temurin17-binaries/releases/download/jdk-17%252B35/OpenJDK17-jdk_aarch64_mac_hotspot_17_35.tar.gz/jdk-17+35/Contents/Home"
}

[Trace - 10:49:22 AM] Sending notification 'window/logMessage'
Params: {
  "type": 4,
  "message": "2023.10.16 10:49:22 INFO  BSP server: [warn]                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^"
}

[Trace - 10:49:23 AM] Sending notification 'window/logMessage'
Params: {
  "type": 4,
  "message": "2023.10.16 10:49:22 INFO  BSP server: [warn]  JVM that is hosting bloop is older than the requested runtime. Please run command `bloop exit`, and then use `--jvm` flag to restart Bloop"
}

[Trace - 10:49:23 AM] Sending notification 'window/logMessage'
Params: {
  "type": 4,
  "message": "2023.10.16 10:49:22 INFO  BSP server: [warn] COMMAND_LINE"
}

[Trace - 10:49:23 AM] Sending notification 'window/logMessage'
Params: {
  "type": 4,
  "message": "2023.10.16 10:49:22 INFO  BSP server: [warn] [current bloop jvm] /Users/ckipp/Library/Caches/Coursier/arc/https/github.com/adoptium/temurin17-binaries/releases/download/jdk-17%252B35/OpenJDK17-jdk_aarch64_mac_hotspot_17_35.tar.gz/jdk-17+35/Contents/Home"
}

[Trace - 10:49:23 AM] Sending notification 'window/logMessage'
Params: {
  "type": 4,
  "message": "2023.10.16 10:49:22 INFO  BSP server: [warn]                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^"
}

[Trace - 10:49:23 AM] Sending response 'textDocument/semanticTokens/full - (2)'. Processing request took 3058ms
Result: {
  "data": [
    0,
    0,
    3,
    17,
    0,
    0,
   ...
    8,
    4
  ]
}

Notice how metals does respond with the textDocument/semantcTokens/full when it shouldn't

Expected behavior

If this is turned off by the client, it should never be sent.

Operating system

macOS

Editor/Extension

Nvim (nvim-metals)

Version of Metals

1.0.1+103-cc897ef4-SNAPSHOT

Extra context or search terms

No response

tgodzik commented 10 months ago

Thanks for reporting! This seems to work correctly for me in VS Code, it shouldn't be related to the build server at all. Unless the issue shows up only with multiple folders? @kasiaMarek could you confirm if this is an issue and whether we should hold off with th release?

ckipp01 commented 10 months ago

If you want exact reproduction steps (maybe this will help):

I have JAVA_HOME set to 11
I kill all java processes killall java just to ensure no Bloop is running
Then I open https://github.com/ckipp01/skan after doing a make setup-ide to ensure the .bsp and .metals get created where they should
Then after a bit I see the colors change since the tokens are returned and mess all the highlighting up:

kasiaMarek commented 10 months ago

Yes, looking into it. We should hold the release until this is resolved and I expect this to be connected to the changes around didChangeConfiguration not the build server. Thanks for the reporting ❤️

filipwiech commented 10 months ago

Hey @ckipp01, sorry to bother, but just out of curiosity, you've mentioned that:

Then after a bit I see the colors change since the tokens are returned and mess all the highlighting up

Regarding this, is there a problem with semantic tokens on the Metals side, or is it rather about something to do with Neovim or its theme? Just wondering if there's an issue to be reported here. :+1:

ckipp01 commented 10 months ago

Regarding this, is there a problem with semantic tokens on the Metals side, or is it rather about something to do with Neovim or its theme? Just wondering if there's an issue to be reported here. 👍

In my experience when things aren't fully compiling at times the highlighting that Metals sends back is less than optimal (see the image I have attached above). I also think the treesitter highlighting in Neovim is quite good, so I prefer to use that since I'm also a maintainer over there and like to spot when things look off.

tgodzik commented 10 months ago

We should probably not send semantic tokens if the project is not compiling to large extent :thinking: but that might be hard to figure out a heuristic

scalameta / metals