CodeLens and documentation symbols should also be notifications

[mickaelistria]

I'm trying to implement codelens in Eclipse IDE, and I have the impression that codelens shouldn't be a request, but a notification. Indeed, they're expected to be always shown and synchronized and apply on the whole file without a user specific action , and it's not natural from inside the IDE to re-invoke the codelens request whenever something change. It can additionally be a source of bugs because we cannot have guarantee that the last didChange message was properly saved and handled before that.
Like diagnostics, codelens and documentSymbols would probably better be notifications.

[mickaelistria]

For example, it's not clear when codelens are expected to be updated (on change only, on save, on something else...). And the spec doesn't, and probably couldn't, clarify nor enforce that. A notification-based workflow would solve this issue.

[dbaeumer]

Turning this into notification from the server to the client would solve this but would be a major change.

Instead I propose that we had a change notification from the server to the client to tell the client to re fetch the code lens information. This will allow the client to make use of its viewport information to only resolve those code lenses that are currently visible.

[mickaelistria]

+1

[dbaeumer]

@aeschli and I had a long discussion about pull verus push triggered by microsoft/vscode-languageserver-protocol-foldingprovider#4 and I want to repeat the outcome here:

• in general pull is preferred over push since it give the client more control over data it needs (and wants). Consider for example an editor is not visible, but it content changes due to a refactoring. In a push model the server would for example compute new code lenses although the client is not interested in them. In a pull model the client would ask for code lens when the file gets visible again.
• to make the push model work we could add notification about when editors or other UI components gets visible but that would make the LSP a lot harder to understand. The beauty right now is that the LSP doesn't talk about UI state.
• why are diagnostics a push model: the reason is that a pull model would force the client to understand the project system of the language to pull in the right times. For example a change in the tsconfig.json file can change the errors and warnings presented for TS files. This is nothing we want to give the client knowledge of. The server knows this.

To make a pull model work nicely and performant for the server we should enforce client capabilities (at least for new features). For example if a client never asks for folder it should set its capability to false so the server never spends cycles to compute folding which for example could be done as a side affect of reconciling an AST.

May be this rule makes sense: single file information is better provided by pull, project global information might be better provided by push.

[rcjsuen]

For the pull model, would it make sense to use VersionedTextDocumentIdentifier more liberally instead of a simple TextDocumentIdentifier to try and get around the "delayed" or "unprocessed" textDocument/didChange notifications mentioned in the original comment?

[dbaeumer]

I don't think that this is necessary since the protocol defines that all outstanding changes must be sync over before a query requests is sent (e.g the client must maintain the strict order in which these things appear on the client).

[matklad]

Consider for example an editor is not visible, but it content changes due to a refactoring. In a push model the server would for example compute new code lenses although the client is not interested in them.

Client could subscribe for particular notifications on interesting files. So, for example, it can subscribe for errors in all files and folding info in the current file. Dart's protocol has a protocol-level subscription mechanism for that.

Instead I propose that we had a change notification from the server to the client to tell the client to re fetch the code lens information.

"content changed" notification to the pull model seems somewhat analogous to the "subscriptions" service for push model.

[dbaeumer]

Another approach would be etags like in the Web.

[dbaeumer]

3.16 will have support to retrigger all code lens computations.