Ctrl+hover (goto definition preview) causes textDocument/didOpen(/didClose) events

[nadako]

• VSCode Version: 1.36.1
• OS Version: Windows 10

Steps to Reproduce:

1. Hover on some symbol while holding Ctrl so the "definition preview" box appears.
2. Observe textDocument/didOpen sent to the language server. Also textDocument/didClose seems to be sent right after that.

While this doesn't sound like a big ussue, it can cause severe slowdowns if an extension is doing something on document open. For example the Haxe language server requests diagnostics for the opened file, which can be expensive at times.

In practice it's pretty bad, because the events happen on any ctrl+click, even if the user's intention is just quickly go to the definition.

If these events are intended, then perhaps they could contain information about the cause of document open (e.g. "intent": "preview"|"peek"|"edit"), so the language server can decide whether it should do additional work or not.

[Gama11]

I'd expect that publishing diagnostics on file open events is a pretty common pattern for language servers, so this would affect a lot more than just Haxe. It's especially silly because the diagnostics aren't even rendered in the preview.

[dbaeumer]

This already happens in the extension host (it is not LSP specific). The reason is to get the text around the destination (e.g. the content rendered in the upper part of the hover).

I like the idea of having an intent so that clients listening to the event can better react to it. But before this can make it's way into LSP we first need to have it in the VS Code API.

@jrieken is there another trick / solution you could think of?

[nadako]

But before this can make it's way into LSP we first need to have it in the VS Code API.

If I understand it correctly, alternatively the extension's LSP client could use that info to decide to NOT send events to the language server, which would fix this particular issue. Because it doesn't seem to make a lot of sense to send the events to the LS for this preview?

[jrieken]

@jrieken is there another trick / solution you could think of?

That's the designed behaviour because there is only one way of opening a document. The underlying problems seems to be that diagnostics are driven by open/closed documents, not by visible editors nor by visible tabs (the latter has no API)

[vscodebot]

The described behavior is how it is expected to work. If you disagree, please explain what is expected and what is not in more detail. See also our issue reporting guidelines.

Happy Coding!

[jrieken]

The API request to expose the tab model: #15178 (which might serve as duplicate here). Generally, I think there are different strategies how validation should happens (be driven by)

• project wide - when a file belongs to a project (like tsconfig.json, foo.csproj) I would expect that then every file for a project is checked
• editor wide - those documents that I see (visible editor, not open document) or better all tabs (API Access to "Open Editors" #15178)
• open documents - tricky as documents open/close without the user being able to observe that, e.g there are extensions that open all documents matching a certain criteria (like all css files). also, document usually don't close immediately when editors close

[nadako]

Well this boils down to the definition of "open document", because I wouldn't recognize that preview box as an open document. But then again one can argue that it is indeed an open document... I don't know :)

[nadako]

Regarding the "visible editors" API - to make it useful it has to be in the LSP as well, but does it make sense there conceptually?

[jrieken]

Regarding the "visible editors" API - to make it useful it has to be in the LSP as well, but does it make sense there conceptually?

I honestly don't know... If it doesn't, does then exposing the tab model makes sense? Tho the LSP client can abstract that way and call it 'none-project-validation-queue'; it being a list of documents that the user likely wants validation for.

[nadako]

What do you think about adding the "intent" property to the textDocument/didOpen data? That sounds useful and could be a simpler solution for the issue.

[jrieken]

could be a simpler solution for the issue.

It unfortunately isn't. For once, there is an API challenge because today the onDidOpenTextDocument-event fires the document, not an event object that can be extended. Then, internally it will be hard to funnel that information all the way across our different layers

[nadako]

I see, well then we'll think of some workarounds, thanks!

[jrieken]

Great. I think a pragmatic approach would be to check with vscode.window.visibleTextEditors

[nadako]

Yeah, the main problem is that we gotta communicate that to the language server that is also supposed to work with non-VSCode clients, but I guess the server could behave differently based on init configurations.