migrate to metals-goto-location to window/showDocument

[ayoub-benali]

LSP Spec 3.16 defines Show Document Request

It would be nice for Metals to migrate the custom metals-goto-location to this new standard request to avoid custom client implementations.

Search terms
LSP, spec, show document

[kpbochenek]

Custom command contains additional field other-window that is not present in Show Document Request.
It makes particular functionality 'Analyze-stacktrace' much more pleasant to use.
Maybe setting external=true in Show Document Request could be interpreted that way, but this is a generic message and we should not hijack this field for specific metals use.

Interface is almost 1:1 same (uri == uri, range == selection) so without this 'other-window' flag migration could be instant.

[ayoub-benali]

I didn't know that otherWindow was used for Analyze-stacktrace. Since the draft was merged yesterday, it might makes sense to propose an extra flag for other window as it isn't finalized yet.

[kpbochenek]

Thanks for a link to this specific commit 👍 probably adding otherWindow is a little too specific, but maybe guys will have some suggestions how to modify it to allow servers to customize this message for their needs a little

[tgodzik]

I am not sure otherWindow is needed in LSP. We create either WebView with links to commands or code lenses with commands. So in reality it's not the server invoking the command, but the client itself. Having it in LSP spec doesn't really help us, we still need to invoke commands on the client side.

[laughedelic]

So in reality it's not the server invoking the command, but the client itself. Having it in LSP spec doesn't really help us, we still need to invoke commands on the client side.

I think it's the other way around:

The show document request is sent from a server to a client to ask the client to display a particular document in the user interface.

so clients are expected to implement a UI for that or invoke external program if the request has external flag. So as @ayoub-benali said, using this request could help to simplify client plugins implementation.

[tgodzik]

@laughedelic It will help us in some scenarios, but I haven't clarified that I was talking about the analyze stacktrace command. For that command we create a new document or webview, which users can use to navigate over the code. And that document already contains either links to commands or commands in code lenses, which clients need to know how to invoke.

And that is the only part of Metals that uses otherWindow, that's why I mean that having it in LSP doesn't help.
We could possibly modify it to try and have the client send a server command that would invoke the new LSP method, but not yet sure if it will work in all scenarios.

[tgodzik]

Anyway I added an issue in the repo to discuss it: microsoft/language-server-protocol#1140

[tgodzik]

It seems that actually the idea was included in the spec:

https://microsoft.github.io/language-server-protocol/specifications/specification-3-16/#window_showDocument

There is an option to use takeFocus, I will experiment with it one the new lsp4j release comes in.