Start with basic dynamic capabilities #911

rwols · 2020-03-14T17:40:26Z

We can always expand/change this in the future.
For now, we understand dynamic registrations of the following methods:

~~textDocument/didOpen~~
We notify the server if the server registers this method.
~~textDocument/willSave~~
We notify the server if the server registers this method.
~~textDocument/willSaveWaitUntil~~
We do the request if the server registers this method.
~~textDocument/didSave~~
We notify the server if the server registers this method.
~~textDocument/didClose~~
We notify the server if the server registers this method.
~~workspace/didChangeConfiguration~~
Registering this capability does nothing interesting: we only notify
a change in configuration when we initialize the session.
(this is a problem of ST3, but we can improve this in ST4)
workspace/didChangeWorkspaceFolders
Does something interesting: if the server doesn't support workspaces,
but does register this capability, we'll notify of changes in the
workspace.

Of course, if the server already advertised (static) capabilities for
e.g. TextDocumentSync, then registering for textDocument/didOpen does
nothing useful.

Let's focus on didChangeWorkspaceFolders for now.

tomv564 · 2020-03-14T17:43:39Z

What server needs dynamic registration on such a fundamental feature?

rchl · 2020-03-14T18:06:32Z

plugin/core/sessions.py

@@ -125,6 +129,7 @@ def __init__(self,
        self._on_post_initialize = on_post_initialize
        self._on_post_exit = on_post_exit
        self.capabilities = dict()  # type: Dict[str, Any]
+        self.dynamic_capabilities = BidirectionalDictionary()


Since I'm pretty new to the code, I was confused that capabilites actually means server_capabilities. Maybe worth renaming? And then also the same for dynamic_capabilities probably.

And it would be ideal if both are merged into one dict (or a custom form of dict). :)

And it would be ideal if both are merged into one dict (or a custom form of dict). :)

I agree, this pull request needs more work.

rchl · 2020-03-14T18:10:47Z

Of course, if the server already advertised (static) capabilities for
e.g. TextDocumentSync, then registering for textDocument/didOpen does
nothing useful.

And according to the specification, servers shouldn't do that:

Server must not register the same capability both statically through the initialize result and dynamically for the same document selector. If a server wants to support both static and dynamic registration it needs to check the client capability in the initialize request and only register the capability statically if the client doesn’t support dynamic registration for that capability.

But they do, of course.

rchl · 2020-03-14T18:18:53Z

What server needs dynamic registration on such a fundamental feature?

While eslint uses client/registerCapability, I don't think it has any purpose doing that as it doesn't ever unregister them (I'm working on some PRs to address that).
The idea with using registerCapability is to register and unregister features based on a change of settings in the client.

But in any case, do we really need to have specific examples of servers that use it? Isn't being spec-compliant and supporting more features generally a good thing? I think it will only save us work in the future if we do it now.

rwols · 2020-03-14T19:11:47Z

What server needs dynamic registration on such a fundamental feature?

OmniSharp wanted dynamic capabilities for text synchronization #661

I don't know whether OmniSharp advertises them as static capabilities in the meantime (they should, of course :))

ESLint registers workspace/didChangeConfiguration and workspace/didChangeWorkspaceFolders.

rls registers textDocument/rangeFormatting #372 (but only after the user sets up suitable settings!). I don't know whether this is still the case for the rust language server.

vscode-json-language-server also registers textDocument/rangeFormatting.

I haven't included textDocument/rangeFormatting for now.

Isn't being spec-compliant and supporting more features generally a good thing? I think it will only save us work in the future if we do it now.

I don't like writing speculative code as that's a waste of time, but in this case I consider the LSP spec as a baseline we should implement.

tomv564 · 2020-03-14T20:16:13Z

To clarify the Omnisharp case - they supported both static and dynamic configuration, but broke their static configuration logic. LSP doesn't exist to work around their bugs.

Range formatting was the common use case for dynamic capabilities, I think it would be better to start with that capability as its logic is not so deeply plumbed into LSP itself.

rwols · 2020-03-16T12:31:16Z

Oof, this looks to be more random than I initially thought.

So I have this idea of mapping the received methods to toggle server capabilities on/off. Perhaps there's a better strategy that I'm not seeing yet.

The client/registerCapability request sends a client method (e.g. textDocument/rangeFormatting), but we're supposed to map that to a server capability (e.g. rangeFormattingProvider).
Some server capability could have multiple client methods associated to it. E.g. textDocument/didOpen + textDocument/didClose both map to the textDocumentSync server capability.
Some client methods in the client/registerCapability request don't map to a server capability at all! For instance, workspace/didChangeConfiguration doesn't map to any server capability. If we decide to only notify workspace/didChangeConfiguration when it has been dynamically registered for the sake of eslint, a bunch of other language servers might break. So not a good idea. So, it looks like registering workspace/didChangeConfiguration is pointless: we notify it anyway, servers can ignore notifications anyway.
Some client methods in the client/registerCapability request get special treatment? For the workspace/didChangeWorkspaceFolders notification, we're supposed to store the registration ID in the workspace.workpaceFolders.changeNotifications member. But, this deviates from a general plan to store these registrations.

rchl · 2020-03-16T13:40:27Z

If we decide to only notify workspace/didChangeConfiguration when it has been dynamically registered for the sake of eslint, a bunch of other language servers might break.

Yes, it would break servers. https://github.com/vscode-langservers/vscode-json-languageserver, for example, doesn't register workspace/didChangeConfiguration dynamically but expects to get it.

For reference, here is a log of initialize request/response from VSCode and that server:

Request

Params: {
    "processId": 15667,
    "clientInfo": {
        "name": "vscode",
        "version": "1.43.0"
    },
    "rootPath": "/Users/me/project",
    "rootUri": "file:///Users/me/project",
    "capabilities": {
        "workspace": {
            "applyEdit": true,
            "workspaceEdit": {
                "documentChanges": true,
                "resourceOperations": [
                    "create",
                    "rename",
                    "delete"
                ],
                "failureHandling": "textOnlyTransactional"
            },
            "didChangeConfiguration": {
                "dynamicRegistration": true
            },
            "didChangeWatchedFiles": {
                "dynamicRegistration": true
            },
            "symbol": {
                "dynamicRegistration": true,
                "symbolKind": {
                    "valueSet": [
                        1,
                        2,
                        3,
                        4,
                        5,
                        6,
                        7,
                        8,
                        9,
                        10,
                        11,
                        12,
                        13,
                        14,
                        15,
                        16,
                        17,
                        18,
                        19,
                        20,
                        21,
                        22,
                        23,
                        24,
                        25,
                        26
                    ]
                }
            },
            "executeCommand": {
                "dynamicRegistration": true
            },
            "configuration": true,
            "workspaceFolders": true
        },
        "textDocument": {
            "publishDiagnostics": {
                "relatedInformation": true,
                "versionSupport": false,
                "tagSupport": {
                    "valueSet": [
                        1,
                        2
                    ]
                }
            },
            "synchronization": {
                "dynamicRegistration": true,
                "willSave": true,
                "willSaveWaitUntil": true,
                "didSave": true
            },
            "completion": {
                "dynamicRegistration": true,
                "contextSupport": true,
                "completionItem": {
                    "snippetSupport": true,
                    "commitCharactersSupport": true,
                    "documentationFormat": [
                        "markdown",
                        "plaintext"
                    ],
                    "deprecatedSupport": true,
                    "preselectSupport": true,
                    "tagSupport": {
                        "valueSet": [
                            1
                        ]
                    }
                },
                "completionItemKind": {
                    "valueSet": [
                        1,
                        2,
                        3,
                        4,
                        5,
                        6,
                        7,
                        8,
                        9,
                        10,
                        11,
                        12,
                        13,
                        14,
                        15,
                        16,
                        17,
                        18,
                        19,
                        20,
                        21,
                        22,
                        23,
                        24,
                        25
                    ]
                }
            },
            "hover": {
                "dynamicRegistration": true,
                "contentFormat": [
                    "markdown",
                    "plaintext"
                ]
            },
            "signatureHelp": {
                "dynamicRegistration": true,
                "signatureInformation": {
                    "documentationFormat": [
                        "markdown",
                        "plaintext"
                    ],
                    "parameterInformation": {
                        "labelOffsetSupport": true
                    }
                },
                "contextSupport": true
            },
            "definition": {
                "dynamicRegistration": true,
                "linkSupport": true
            },
            "references": {
                "dynamicRegistration": true
            },
            "documentHighlight": {
                "dynamicRegistration": true
            },
            "documentSymbol": {
                "dynamicRegistration": true,
                "symbolKind": {
                    "valueSet": [
                        1,
                        2,
                        3,
                        4,
                        5,
                        6,
                        7,
                        8,
                        9,
                        10,
                        11,
                        12,
                        13,
                        14,
                        15,
                        16,
                        17,
                        18,
                        19,
                        20,
                        21,
                        22,
                        23,
                        24,
                        25,
                        26
                    ]
                },
                "hierarchicalDocumentSymbolSupport": true
            },
            "codeAction": {
                "dynamicRegistration": true,
                "isPreferredSupport": true,
                "codeActionLiteralSupport": {
                    "codeActionKind": {
                        "valueSet": [
                            "",
                            "quickfix",
                            "refactor",
                            "refactor.extract",
                            "refactor.inline",
                            "refactor.rewrite",
                            "source",
                            "source.organizeImports"
                        ]
                    }
                }
            },
            "codeLens": {
                "dynamicRegistration": true
            },
            "formatting": {
                "dynamicRegistration": true
            },
            "rangeFormatting": {
                "dynamicRegistration": true
            },
            "onTypeFormatting": {
                "dynamicRegistration": true
            },
            "rename": {
                "dynamicRegistration": true,
                "prepareSupport": true
            },
            "documentLink": {
                "dynamicRegistration": true,
                "tooltipSupport": true
            },
            "typeDefinition": {
                "dynamicRegistration": true,
                "linkSupport": true
            },
            "implementation": {
                "dynamicRegistration": true,
                "linkSupport": true
            },
            "colorProvider": {
                "dynamicRegistration": true
            },
            "foldingRange": {
                "dynamicRegistration": true,
                "rangeLimit": 5000,
                "lineFoldingOnly": true
            },
            "declaration": {
                "dynamicRegistration": true,
                "linkSupport": true
            },
            "selectionRange": {
                "dynamicRegistration": true
            },
            "callHierarchy": {
                "dynamicRegistration": true
            },
            "semanticTokens": {
                "dynamicRegistration": true,
                "tokenTypes": [
                    "comment",
                    "keyword",
                    "number",
                    "regexp",
                    "operator",
                    "namespace",
                    "type",
                    "struct",
                    "class",
                    "interface",
                    "enum",
                    "typeParameter",
                    "function",
                    "member",
                    "macro",
                    "variable",
                    "parameter",
                    "property",
                    "label"
                ],
                "tokenModifiers": [
                    "declaration",
                    "documentation",
                    "static",
                    "abstract",
                    "deprecated",
                    "async",
                    "readonly"
                ]
            }
        },
        "window": {
            "workDoneProgress": true
        }
    },
    "initializationOptions": {
        "handledSchemaProtocols": [
            "file"
        ],
        "provideFormatter": false,
        "customCapabilities": {
            "rangeFormatting": {
                "editLimit": 1000
            }
        }
    },
    "trace": "verbose",
    "workspaceFolders": [
        {
            "uri": "file:///Users/me/project",
            "name": "project"
        }
    ]
}

Response

Result: {
    "capabilities": {
        "textDocumentSync": 2,
        "completionProvider": {
            "resolveProvider": false,
            "triggerCharacters": [
                "\"",
                ":"
            ]
        },
        "hoverProvider": true,
        "documentSymbolProvider": true,
        "documentRangeFormattingProvider": false,
        "colorProvider": {},
        "foldingRangeProvider": true,
        "selectionRangeProvider": true
    }
}

…ders for now

rwols · 2020-05-01T18:04:47Z

OK well being a bit more lax about correct handling of client/registerCapability I see this stuff for the jedi language server:

:: <-- jedi-language-server client/registerCapability(b7364b11-a1fd-44e2-8794-29c772ddf5ed): {'registrations': [{'registerOptions': {'triggerCharacters': ['.', "'", '"']}, 'id': '4b6e000c-be82-40ad-af9c-d6eb1a8e897b', 'method': 'textDocument/completion'}]}
:: >>> jedi-language-server b7364b11-a1fd-44e2-8794-29c772ddf5ed: None
:: <-- jedi-language-server client/registerCapability(5c800c97-07fa-44d5-9bf9-60fa20b1b28f): {'registrations': [{'registerOptions': {}, 'id': '35f5fc3a-3df8-4e82-baea-3ec4f5255548', 'method': 'textDocument/definition'}]}
:: >>> jedi-language-server 5c800c97-07fa-44d5-9bf9-60fa20b1b28f: None
:: <-- jedi-language-server client/registerCapability(533a0bf6-39f7-4681-8125-c716a201ba00): {'registrations': [{'registerOptions': {}, 'id': '6c3a74ef-4e39-43c6-96b6-4684e48ac221', 'method': 'textDocument/documentHighlight'}]}
:: >>> jedi-language-server 533a0bf6-39f7-4681-8125-c716a201ba00: None
:: <-- jedi-language-server client/registerCapability(8aa2f850-6b6c-41c2-bae7-42f65095475c): {'registrations': [{'registerOptions': {}, 'id': '45f4919f-109e-4d36-abde-4f946973ff4d', 'method': 'textDocument/documentSymbol'}]}
:: >>> jedi-language-server 8aa2f850-6b6c-41c2-bae7-42f65095475c: None
:: <-- jedi-language-server client/registerCapability(26ee7928-7e5d-40cb-87fa-af2a40bcd7e8): {'registrations': [{'registerOptions': {}, 'id': '0159b5a9-93f4-4e22-9971-fb6f89dbf799', 'method': 'textDocument/hover'}]}
:: >>> jedi-language-server 26ee7928-7e5d-40cb-87fa-af2a40bcd7e8: None
:: <-- jedi-language-server client/registerCapability(a1dc8b23-6765-4a72-a836-cc79b40ea2d9): {'registrations': [{'registerOptions': {}, 'id': '10b21d20-7888-41a0-9246-15dc10ac4799', 'method': 'textDocument/references'}]}
:: >>> jedi-language-server a1dc8b23-6765-4a72-a836-cc79b40ea2d9: None
:: <-- jedi-language-server client/registerCapability(34388f08-16b4-41a6-a731-be8fee258122): {'registrations': [{'registerOptions': {}, 'id': 'a44a88c0-4c57-4c61-bfb7-59f3133284a1', 'method': 'textDocument/rename'}]}
:: >>> jedi-language-server 34388f08-16b4-41a6-a731-be8fee258122: None
:: <-- jedi-language-server client/registerCapability(a510ec99-8a2f-4dc5-8af5-20d5485e1979): {'registrations': [{'registerOptions': {'triggerCharacters': ['(', ',', ')']}, 'id': '128b351f-1d38-45d6-b546-6ae8eb171fec', 'method': 'textDocument/signatureHelp'}]}
:: >>> jedi-language-server a510ec99-8a2f-4dc5-8af5-20d5485e1979: None
:: <-- jedi-language-server client/registerCapability(b7f3954d-4560-4a28-8e84-fa87d26506f0): {'registrations': [{'registerOptions': {}, 'id': '1edb7ab0-966c-416b-a16e-c95563540347', 'method': 'workspace/symbol'}]}
:: >>> jedi-language-server b7f3954d-4560-4a28-8e84-fa87d26506f0: None
:: <-- jedi-language-server client/registerCapability(54b01d8f-c309-4405-84fd-4e63bc42bb8e): {'registrations': [{'registerOptions': {}, 'id': '12eabf25-3cf8-429c-a7a4-176c3bbc06b9', 'method': 'textDocument/didOpen'}]}
:: >>> jedi-language-server 54b01d8f-c309-4405-84fd-4e63bc42bb8e: None
:: <-- jedi-language-server client/registerCapability(bedcfb92-1ce0-4407-8ae1-514381fd5368): {'registrations': [{'registerOptions': {}, 'id': '0e381836-9bfd-468f-bdb6-bd0327e8a4f0', 'method': 'textDocument/didChange'}]}
:: >>> jedi-language-server bedcfb92-1ce0-4407-8ae1-514381fd5368: None
:: <-- jedi-language-server client/registerCapability(cbe17d08-13ea-4302-a2bf-8a663742f219): {'registrations': [{'registerOptions': {}, 'id': 'e76c824c-23ae-4911-9b33-1dbd09b942fc', 'method': 'textDocument/didSave'}]}
:: >>> jedi-language-server cbe17d08-13ea-4302-a2bf-8a663742f219: None

rchl · 2020-05-01T18:18:27Z

plugin/core/sessions.py

+                    "workspace", {}).setdefault(
+                        "workspaceFolders", {})["changeNotifications"] = registration["id"]
+            else:
+                printf("WARNING: unknown registration method '{}' from {}".format(method, self.config.name))


Better to format like that than using second argument of printf?

rchl · 2020-05-01T18:22:19Z

Spec says A client opts in via the dynamicRegistration property on the specific client capabilities.. We don't do that.

rwols · 2020-05-01T18:32:01Z

Well, I think for now it's OK to mutate the self.capabilities.

Also, here are the interesting payloads from jedi condensed:

[{'method': 'textDocument/completion', 'registerOptions': {'triggerCharacters': ['.', "'", '"']}, 'id': '3b51186c-243c-49f8-b642-28b49b951262'}]
[{'method': 'textDocument/definition', 'registerOptions': {}, 'id': 'bbd3b794-341f-4dbb-b51d-0fd0add15aa6'}]
[{'method': 'textDocument/documentHighlight', 'registerOptions': {}, 'id': 'c69912a1-60b6-45f6-b849-e124d36c9960'}]
[{'method': 'textDocument/documentSymbol', 'registerOptions': {}, 'id': '256ebea2-39f1-4218-8491-13497cba42a4'}]
[{'method': 'textDocument/hover', 'registerOptions': {}, 'id': 'a00bc550-ed4d-4365-9a1a-d48bbc900cd3'}]
[{'method': 'textDocument/references', 'registerOptions': {}, 'id': 'ac59cb77-b80f-4e63-bcfb-2613cdcc4ef5'}]
[{'method': 'textDocument/rename', 'registerOptions': {}, 'id': 'af750de2-562a-400b-8c34-77065bedae46'}]
[{'method': 'textDocument/signatureHelp', 'registerOptions': {'triggerCharacters': ['(', ',', ')']}, 'id': 'b81a7bb2-f7c0-435e-b544-290c5bc532ac'}]
[{'method': 'workspace/symbol', 'registerOptions': {}, 'id': '9f27d6e4-9207-4b23-92dd-80d0517c4f7a'}]
[{'method': 'textDocument/didOpen', 'registerOptions': {}, 'id': '0122091d-0220-403d-be67-6bf568173d6e'}]
[{'method': 'textDocument/didChange', 'registerOptions': {}, 'id': 'dad15c65-4914-4379-9d02-623c9e65afef'}]
[{'method': 'textDocument/didSave', 'registerOptions': {}, 'id': '594c5727-f447-434c-b457-260b225e3b17'}]

rchl · 2020-05-01T18:36:24Z

Well, I think for now it's OK to mutate the self.capabilities.

Does that refer to my comment?

rwols · 2020-05-01T18:46:47Z

Does that refer to my comment?

I replied to this:

Spec says A client opts in via the dynamicRegistration property on the specific client capabilities.. We don't do that.

I thought you meant that we should keep two dicts: one with static capabilities and one with registered capabilities.

plugin/core/sessions.py

rchl · 2020-05-01T18:50:42Z

I thought you meant that we should keep two dicts: one with static capabilities and one with registered capabilities.

I mean that we (the client) should advertise support for dynamicRegistration. But now I see that there isn't one for workspace/didChangeWorkspaceFolders so never mind. :)

rchl · 2020-05-01T18:54:53Z

If we decide to only notify workspace/didChangeConfiguration when it has been dynamically registered for the sake of eslint, a bunch of other language servers might break.

Yes, it would break servers. https://github.com/vscode-langservers/vscode-json-languageserver, for example, doesn't register workspace/didChangeConfiguration dynamically but expects to get it.

For reference, here is a log of initialize request/response from VSCode and that server:

Request
Response

I've said that before. Let me check if that's really a problem now.

rwols · 2020-05-01T18:56:41Z

But does vscode-json-languageserver really need workspace change notifications?

rwols · 2020-05-01T18:57:02Z

Anyway the story is not done, I'll work on this more but it's something at least.

rchl · 2020-05-01T18:59:39Z

But does vscode-json-languageserver really need workspace change notifications?

Never mind. I was confused as this change is about workspace/didChangeWorkspaceFolders and not workspace/didChangeConfiguration so all good.

rwols requested review from tomv564 and rchl March 14, 2020 17:40

rchl reviewed Mar 14, 2020

View reviewed changes

rwols mentioned this pull request Mar 17, 2020

Reply to all requests #909

Merged

rwols mentioned this pull request Apr 10, 2020

Registering request handler for workspace/didChangeWorkspaceFolders failed sublimelsp/LSP-intelephense#26

Closed

Do client/registerCapability only for workspace/didChangeWorkspaceFol…

a0a2961

…ders for now

rwols force-pushed the dynamic-capabilities branch from d7ecd80 to a0a2961 Compare May 1, 2020 17:50

rchl reviewed May 1, 2020

View reviewed changes

plugin/core/sessions.py Show resolved Hide resolved

rchl approved these changes May 1, 2020

View reviewed changes

rwols merged commit 7fa1bb9 into master May 1, 2020

rwols deleted the dynamic-capabilities branch May 1, 2020 18:55

mjlbach mentioned this pull request Dec 21, 2020

LSP: add workaround for servers that expect (or ignore disabling) dynamicRegistration neovim/neovim#13448

Closed

the-mikedavis mentioned this pull request Feb 19, 2023

LSP: Send replies for malformed and unhandled RPC requests helix-editor/helix#6058

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Start with basic dynamic capabilities #911

Start with basic dynamic capabilities #911

rwols commented Mar 14, 2020 •

edited

Loading

tomv564 commented Mar 14, 2020

rchl Mar 14, 2020

rchl Mar 14, 2020

rwols Mar 14, 2020

rchl commented Mar 14, 2020 •

edited

Loading

rchl commented Mar 14, 2020 •

edited

Loading

rwols commented Mar 14, 2020

tomv564 commented Mar 14, 2020

rwols commented Mar 16, 2020 •

edited

Loading

rchl commented Mar 16, 2020 •

edited

Loading

rwols commented May 1, 2020

rchl May 1, 2020

rchl commented May 1, 2020

rwols commented May 1, 2020

rchl commented May 1, 2020

rwols commented May 1, 2020

rchl commented May 1, 2020 •

edited

Loading

rchl commented May 1, 2020

rwols commented May 1, 2020

rwols commented May 1, 2020

rchl commented May 1, 2020

Start with basic dynamic capabilities #911

Start with basic dynamic capabilities #911

Conversation

rwols commented Mar 14, 2020 • edited Loading

tomv564 commented Mar 14, 2020

rchl Mar 14, 2020

Choose a reason for hiding this comment

rchl Mar 14, 2020

Choose a reason for hiding this comment

rwols Mar 14, 2020

Choose a reason for hiding this comment

rchl commented Mar 14, 2020 • edited Loading

rchl commented Mar 14, 2020 • edited Loading

rwols commented Mar 14, 2020

tomv564 commented Mar 14, 2020

rwols commented Mar 16, 2020 • edited Loading

rchl commented Mar 16, 2020 • edited Loading

rwols commented May 1, 2020

rchl May 1, 2020

Choose a reason for hiding this comment

rchl commented May 1, 2020

rwols commented May 1, 2020

rchl commented May 1, 2020

rwols commented May 1, 2020

rchl commented May 1, 2020 • edited Loading

rchl commented May 1, 2020

rwols commented May 1, 2020

rwols commented May 1, 2020

rchl commented May 1, 2020

rwols commented Mar 14, 2020 •

edited

Loading

rchl commented Mar 14, 2020 •

edited

Loading

rchl commented Mar 14, 2020 •

edited

Loading

rwols commented Mar 16, 2020 •

edited

Loading

rchl commented Mar 16, 2020 •

edited

Loading

rchl commented May 1, 2020 •

edited

Loading