Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
Stainless2k opened this issue Apr 22, 2024 · 8 comments
Open

Comments

@Stainless2k
Copy link

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;
            }
          });
gotoerror.mp4

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
Copy link
Collaborator

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
Copy link
Collaborator

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

@Stainless2k
Copy link
Author

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
Copy link
Collaborator

kaisalmen commented Apr 24, 2024

@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
Copy link
Author

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
Copy link
Collaborator

@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
Copy link
Collaborator

@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.

@joneugster
Copy link

Might not be a helpful comment, but I'm also just here searching for a way to disable ctrl + hover completely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants