TypeFox / monaco-languageclient

Repo hosts npm packages for monaco-languageclient, vscode-ws-jsonrpc, monaco-editor-wrapper, @typefox/monaco-editor-react and monaco-languageclient-examples
https://www.npmjs.com/package/monaco-languageclient
MIT License
992 stars 171 forks source link

[MonacoEditorReactComp] hover + ctrl triggers weird go to definitions #644

Open Stainless2k opened 2 months ago

Stainless2k commented 2 months ago

In our App we use MonacoEditorReactComp, python-lsp-server and all code files are on a server. So we register our own openCodeEditor() to handle go to definitions, since the client cant access the file system of the server.

This work fine when using the context menu or keybindings, but for some reason when hovering and pressing ctrl (just those 2 no clicking), it triggers a regular go to definitions request (checked debug log of python-lsp-server), which also generates a regular response. But then for some reason Monaco ignores the registered openCodeEditor() and tries to open the file, which causes and error. Also important here that the request/response here is exactly the same as using the context menu for example, so I assume its not a python-lsp-server problem.

After a lot of googling and trying to disable this 'shortcut', but I couldn't find a hover + ctrl shortcut any were. (pretty sure now it is not a official shortcut)

I did found an old issue in the monaco repo that seemed similar, but the author noticed it was their fault and closed it without explaining how they fixed it...

I created a minimal repo and could reproduce the problem, so I assume MonacoEditorReactComp or the wrapper might be the cause.

In this video this is the openCodeEditor()

  mEditor.registerEditorOpener({
            openCodeEditor(
              source,
              resource,
              selectionOrPosition?
            ): boolean | Promise<boolean> {
              window.alert('registerEditorOpener called!')

              return true;
            }
          });

https://github.com/TypeFox/monaco-languageclient/assets/5407961/7cac0d6a-4724-45aa-8244-c89e6d14765f

Debug output when using the context menu

2024-04-22 13:23:19,173 CEST - DEBUG - pylsp_jsonrpc.endpoint - Handling request from client {'jsonrpc': '2.0', 'id': 77, 'method': 'textDocument/definition', 'params': {'textDocument': {'uri': 'file:///workspace/model38.python'}, 'position': {'line': 0, 'character': 10}}}
2024-04-22 13:23:19,173 CEST - DEBUG - pylsp.config.config -   pylsp_definitions [hook]
      config: <pylsp.config.config.Config object at 0x7eb15b47eb90>
      workspace: <pylsp.workspace.Workspace object at 0x7eb15acef6d0>
      document: file:///workspace/model38.python
      position: {'line': 0, 'character': 10}

2024-04-22 13:23:19,176 CEST - DEBUG - pylsp.config.config -   finish pylsp_definitions --> [[{'uri': 'file:///home/leon/.cache/pypoetry/virtualenvs/publiqa-server-OJt85VVB-py3.11/lib/python3.11/site-packages/jedi/third_party/typeshed/stdlib/2and3/time.pyi', 'range': {'start': {'line': 0, 'character': 0}, 'end': {'line': 0, 'character': 4}}}]] [hook]

2024-04-22 13:23:19,176 CEST - DEBUG - pylsp_jsonrpc.endpoint - Got result from synchronous request handler: [{'uri': 'file:///home/leon/.cache/pypoetry/virtualenvs/publiqa-server-OJt85VVB-py3.11/lib/python3.11/site-packages/jedi/third_party/typeshed/stdlib/2and3/time.pyi', 'range': {'start': {'line': 0, 'character': 0}, 'end': {'line': 0, 'character': 4}}}]

Debug output when hovering and pressing ctrl

2024-04-22 13:24:31,393 CEST - DEBUG - pylsp_jsonrpc.endpoint - Handling request from client {'jsonrpc': '2.0', 'id': 79, 'method': 'textDocument/definition', 'params': {'textDocument': {'uri': 'file:///workspace/model38.python'}, 'position': {'line': 0, 'character': 10}}}
2024-04-22 13:24:31,393 CEST - DEBUG - pylsp.config.config -   pylsp_definitions [hook]
      config: <pylsp.config.config.Config object at 0x7eb15b47eb90>
      workspace: <pylsp.workspace.Workspace object at 0x7eb15acef6d0>
      document: file:///workspace/model38.python
      position: {'line': 0, 'character': 10}

2024-04-22 13:24:31,400 CEST - DEBUG - pylsp.config.config -   finish pylsp_definitions --> [[{'uri': 'file:///home/leon/.cache/pypoetry/virtualenvs/publiqa-server-OJt85VVB-py3.11/lib/python3.11/site-packages/jedi/third_party/typeshed/stdlib/2and3/time.pyi', 'range': {'start': {'line': 0, 'character': 0}, 'end': {'line': 0, 'character': 4}}}]] [hook]

2024-04-22 13:24:31,400 CEST - DEBUG - pylsp_jsonrpc.endpoint - Got result from synchronous request handler: [{'uri': 'file:///home/leon/.cache/pypoetry/virtualenvs/publiqa-server-OJt85VVB-py3.11/lib/python3.11/site-packages/jedi/third_party/typeshed/stdlib/2and3/time.pyi', 'range': {'start': {'line': 0, 'character': 0}, 'end': {'line': 0, 'character': 4}}}]
kaisalmen commented 2 months ago

Hi @Stainless2k thanks for reporting this. just a quick hint for now, but maybe it already is a good hin. You can overwrite the OpenEditor by using @codingame/monaco-vscode-editor-service-override function like this: https://github.com/TypeFox/monaco-languageclient/blob/main/packages/examples/src/langium/langium-dsl/config/extendedConfig.ts#L29 In the linked example there is only using a stub, but you can just replace it with your own implementation there.

kaisalmen commented 2 months ago

See also this example implementation from the monaco-vscode-api repo.

Stainless2k commented 2 months ago

While we could override the whole service, it's not really a satisfying solution here. Using registerEditorOpener works fine for us, it works when using the context menu or short cuts, etc. The bug only happens when hovering and pressing ctrl, no clicking just hovering. Hover + ctrl does not seem to be any offical shortcut thats why it's so confusing to the team.

kaisalmen commented 2 months ago

@Stainless2k is that file you are hovering over already know to the languageserver? I mean before opening. If not, something like this could help. I just updated the example and pushed the change. You use the vscode api to open a file and this will automatically announced to the language server. This way you don't have to open it in the editor beforehand (= hopefully fixing the hovering issue).

Stainless2k commented 2 months ago

The file is know to the server, the files are even on the server in our case. When the client request a file it just gets the text and connects to py-lsp basically.

But that's beside the point here, our issue is that for some reason Ctrl + hover does not use the custom open logic. Using go to definition works, when using anything else then Ctrl + hover.

Ctrl + hover is not even suppose to do any thing, we couldn't find any docs or code about Ctrl + hover. Also Ctrl + hover does not do anything on the Monaco playground.

Because of this we assume this problem originates somewhere in the wrapper

kaisalmen commented 2 months ago

@Stainless2k ok, I need to dig into this / your example. I don't have a good answer right now and a bit of time (it won't be today). It is very likely there is an unknown bug.

kaisalmen commented 1 month ago

@Stainless2k I just released new versions. Major version, because of breaking API enhancements, please check the wrapper CHANGELOG and react comp CHANGELOG. I updated the python examples, so we can directly observe your problem. If the python LS physically has the files available, then the hover issue is gone. But pyright does not create the files automatically. I need to understand if there are general means available to enforce this via the language server protocol or if we need some kind of server helper function.