improvement: move bsp responsiveness into status #5660
Conversation
| s"BSP ${icons.link}", | ||
| "info", | ||
| show = true, | ||
| tooltip = "Metals is connected to the build server.", |
There was a problem hiding this comment.
Perhaps outside the scope of this PR, but would it be possible to include the BSP type (Bloop, SBT, scala-cli, mill, etc.) in the tooltip and/or the main status bar text? 🙂
There was a problem hiding this comment.
Sure, that's actually not a problem at all.
|
Not sure if I understand everything correctly, but the message popup had the option to restart the build server: is this ability lost now? Would it be possible to maybe somehow keep this user action, like an on click handler for the BSP status bar icon, or a hyperlink in the tooltip ("Broken connection, build sever is not responding, [restart the build server]")? 🤔 |
Yes, on click on the status bar item (in VS Code). |
kasiaMarek
force-pushed
the
bsp-status
branch
from
3de50ee
to
81f6df7
Compare
| Some(reconnect) | ||
| ) | ||
| } | ||
| def connectedParams(serverName: String, icons: Icons): MetalsStatusParams = |
There was a problem hiding this comment.
Thank you for adding the build server name to the status! 👍 Another idea that could be a nice addition: when the Metals is trying to connect to the BSP (for example as a result of the user action after clicking on the status icon, or just in general while waiting for the build server to start) could we show appropriate message and icon, to make the process more visible and to let the user know that something is happening? 🙂
There was a problem hiding this comment.
I think we do show when we're connecting to the build server in metas status now. We could think about moving this to this new bsp status but that's not something I want to think about in this PR.
| "error", | ||
| show = true, | ||
| tooltip = | ||
| s"Broken connection, build sever ($serverName) is not responding. Press to reconnect.", |
There was a problem hiding this comment.
Question about the "Press to reconnect" part of the tooltip: it will work great for the VSCode, but about the other LSP clients, such as the Neovim with the nvim-metals plugin? It could expose the BSP status bar so that it would be shown in the user's statusline, but the "press" action probably won't be available. Maybe we could make the message more generic, WDYT? 🙂
There was a problem hiding this comment.
I added additional field commandTooltip.
metals/src/main/scala/scala/meta/internal/metals/MetalsServerConfig.scala
Show resolved
Hide resolved
kasiaMarek
force-pushed
the
bsp-status
branch
from
81f6df7
to
eeb196b
Compare
| underlying.logMessage(new MessageParams(MessageType.Log, params.text)) | ||
| } else if (params.logMessage.nonEmpty && !pendingShowMessage.get()) { | ||
| if (statusBarState == StatusBarState.ShowMessage) { | ||
| underlying.showMessage(new MessageParams(level, params.logMessage)) |
There was a problem hiding this comment.
Potentially if we have a command defined, we could do showMessageRequest instead so that users would be able to use the same command in other editora
There was a problem hiding this comment.
The drawback is that I added a new client command, so if the client doesn't implement it yet, pressing Reconnect. will confusingly do nothing.
There was a problem hiding this comment.
We could just use ServerCommands.BuildConnect command directly. If that is added to the capabilities then the editor should now to redirect the command to the server
There was a problem hiding this comment.
okay, I did that
Previously a message request would pop up when the build sever was not responding. Now we make this into a status.
Changes in the client API:
bspStatusBarProviderfield to initialisation params, same asstatusBarProviderhas possibile values "on", "off", "log-message", "show-message". Default being "show-message".statusTypefield toMetalsStatusParams, possible values "metals", "bsp".levelfield toMetalsStatusParams, possible values "info", "warn", "error" (can be mapped by client to e.g. color)commandTooltipfield toMetalsStatusParams.BSP status in VSCode: