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

Static capabilities #15

Closed
rwols opened this issue May 1, 2020 · 18 comments
Closed

Static capabilities #15

rwols opened this issue May 1, 2020 · 18 comments

Comments

@rwols
Copy link

rwols commented May 1, 2020

I don't quite understand why this language server needs to register all of its capabilities dynamically:

:: --> jedi-language-server initialize(1): {'rootPath': '/home/raoul/.config/sublime-text-3/Packages/LSP', 'capabilities': {'experimental': {}, 'workspace': {'executeCommand': {}, 'didChangeConfiguration': {}, 'configuration': 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]}}, 'applyEdit': True, 'workspaceFolders': True}, 'textDocument': {'typeDefinition': {'dynamicRegistration': True, 'linkSupport': True}, 'codeAction': {'dynamicRegistration': True, 'codeActionLiteralSupport': {'codeActionKind': {'valueSet': []}}}, 'implementation': {'dynamicRegistration': True, 'linkSupport': True}, 'rename': {'dynamicRegistration': True}, 'definition': {'dynamicRegistration': True, 'linkSupport': True}, 'completion': {'dynamicRegistration': True, 'completionItem': {'snippetSupport': True}, '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': {'parameterInformation': {'labelOffsetSupport': True}, 'documentationFormat': ['markdown', 'plaintext']}}, 'colorProvider': {'dynamicRegistration': True}, 'publishDiagnostics': {'relatedInformation': True}, 'formatting': {'dynamicRegistration': True}, 'rangeFormatting': {'dynamicRegistration': True}, 'references': {'dynamicRegistration': True}, 'declaration': {'dynamicRegistration': True, 'linkSupport': 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]}}, 'documentHighlight': {'dynamicRegistration': True}, 'synchronization': {'willSaveWaitUntil': True, 'dynamicRegistration': True, 'willSave': True, 'didSave': True}}}, 'initializationOptions': {}, 'processId': 7057, 'workspaceFolders': [{'name': 'LSP', 'uri': 'file:///home/raoul/.config/sublime-text-3/Packages/LSP'}, {'name': 'LSP-eslint', 'uri': 'file:///home/raoul/.config/sublime-text-3/Packages/LSP-eslint'}, {'name': 'metals-sublime', 'uri': 'file:///home/raoul/.config/sublime-text-3/Packages/metals-sublime'}, {'name': 'lsp_utils', 'uri': 'file:///home/raoul/.config/sublime-text-3/Packages/lsp_utils'}, {'name': 'LSP-json', 'uri': 'file:///home/raoul/.config/sublime-text-3/Packages/LSP-json'}, {'name': 'UnitTesting', 'uri': 'file:///home/raoul/.config/sublime-text-3/Packages/UnitTesting'}, {'name': 'LSP-vue', 'uri': 'file:///home/raoul/.config/sublime-text-3/Packages/LSP-vue'}, {'name': 'LSP-css', 'uri': 'file:///home/raoul/.config/sublime-text-3/Packages/LSP-css'}, {'name': 'LSP-html', 'uri': 'file:///home/raoul/.config/sublime-text-3/Packages/LSP-html'}, {'name': 'LSP-intelephense', 'uri': 'file:///home/raoul/.config/sublime-text-3/Packages/LSP-intelephense'}, {'name': 'LSP-typescript', 'uri': 'file:///home/raoul/.config/sublime-text-3/Packages/LSP-typescript'}, {'name': 'LSP-elm', 'uri': 'file:///home/raoul/.config/sublime-text-3/Packages/LSP-elm'}, {'name': 'User', 'uri': 'file:///home/raoul/.config/sublime-text-3/Packages/User'}, {'name': 'tonic', 'uri': 'file:///home/raoul/Documents/Programming/tonic'}], 'rootUri': 'file:///home/raoul/.config/sublime-text-3/Packages/LSP', 'clientInfo': {'name': 'Sublime Text LSP'}}
:: <<< jedi-language-server 1: {'capabilities': {'referencesProvider': False, 'hoverProvider': False, 'renameProvider': False, 'definitionProvider': False, 'executeCommandProvider': {'commands': []}, 'documentSymbolProvider': False, 'documentHighlightProvider': False, 'documentRangeFormattingProvider': False, 'workspaceSymbolProvider': False, 'workspace': {'workspaceFolders': {'changeNotifications': True, 'supported': True}}, 'documentFormattingProvider': False, 'textDocumentSync': 2, 'codeActionProvider': False}}
::  -> jedi-language-server initialized: {}
::  -> jedi-language-server textDocument/didOpen
:: <-- jedi-language-server workspace/configuration(03b5234d-8f0c-4316-9965-9f3911e0cef5): {'items': [{'section': 'jedi', 'scopeUri': None}]}
:: >>> jedi-language-server 03b5234d-8f0c-4316-9965-9f3911e0cef5: [None]
:: <-- jedi-language-server client/registerCapability(8f4015fb-a9b9-4c52-8d6a-cb46c4220d3d): {'registrations': [{'id': 'de8a2314-4162-4c6d-8d6a-f620854f465f', 'method': 'textDocument/completion', 'registerOptions': {'triggerCharacters': ['.', "'", '"']}}]}
:: >>> jedi-language-server 8f4015fb-a9b9-4c52-8d6a-cb46c4220d3d: None
:: <-- jedi-language-server client/registerCapability(88ac0d23-4fa3-4a94-98cb-e75688ffb04a): {'registrations': [{'id': '8b2739bf-9fc0-4809-a48d-0bd551987b87', 'method': 'textDocument/definition', 'registerOptions': {}}]}
:: >>> jedi-language-server 88ac0d23-4fa3-4a94-98cb-e75688ffb04a: None
:: <-- jedi-language-server client/registerCapability(3a30f99e-f728-47db-956a-bd9beb364c6b): {'registrations': [{'id': '780e31e2-9b02-4585-b3c2-a21e027cf2a8', 'method': 'textDocument/documentHighlight', 'registerOptions': {}}]}
:: >>> jedi-language-server 3a30f99e-f728-47db-956a-bd9beb364c6b: None
:: <-- jedi-language-server client/registerCapability(1f5b653f-6ca0-45a4-ae42-00071d96d874): {'registrations': [{'id': '2133486f-74c0-4022-b783-98f4febb33a5', 'method': 'textDocument/documentSymbol', 'registerOptions': {}}]}
:: >>> jedi-language-server 1f5b653f-6ca0-45a4-ae42-00071d96d874: None
:: <-- jedi-language-server client/registerCapability(d67ef90d-a7e5-4728-b088-57695a3140cb): {'registrations': [{'id': '595a229e-94b1-4fea-8d47-6e41dd026ff5', 'method': 'textDocument/hover', 'registerOptions': {}}]}
:: >>> jedi-language-server d67ef90d-a7e5-4728-b088-57695a3140cb: None
:: <-- jedi-language-server client/registerCapability(7c7d37b8-cace-47dc-8ea9-a5adb97928d7): {'registrations': [{'id': '90ba73d4-b6b1-41ca-a93f-61ee324cb138', 'method': 'textDocument/references', 'registerOptions': {}}]}
:: >>> jedi-language-server 7c7d37b8-cace-47dc-8ea9-a5adb97928d7: None
:: <-- jedi-language-server client/registerCapability(04bab9f0-ef3a-4169-9266-b86d066e7095): {'registrations': [{'id': '8c0863db-96c7-4c9e-8c69-9cef51acb92b', 'method': 'textDocument/rename', 'registerOptions': {}}]}
:: >>> jedi-language-server 04bab9f0-ef3a-4169-9266-b86d066e7095: None
:: <-- jedi-language-server client/registerCapability(cfe3dfd6-c3a1-407d-a5ac-bfc403097b7f): {'registrations': [{'id': '1e058f76-d10c-432a-8c45-f64e639c5fbb', 'method': 'textDocument/signatureHelp', 'registerOptions': {'triggerCharacters': ['(', ',', ')']}}]}
:: >>> jedi-language-server cfe3dfd6-c3a1-407d-a5ac-bfc403097b7f: None
:: <-- jedi-language-server client/registerCapability(66429cb1-ed0f-4efd-8a44-740d2481338e): {'registrations': [{'id': '91701d2c-5e7c-4334-965e-84d88254de85', 'method': 'workspace/symbol', 'registerOptions': {}}]}
:: >>> jedi-language-server 66429cb1-ed0f-4efd-8a44-740d2481338e: None
:: <-- jedi-language-server client/registerCapability(82c9352c-06f4-4a39-9169-5a4c7a0d1f7a): {'registrations': [{'id': '58635467-db25-41c9-b78d-8c31d68c856e', 'method': 'textDocument/didOpen', 'registerOptions': {}}]}
:: >>> jedi-language-server 82c9352c-06f4-4a39-9169-5a4c7a0d1f7a: None
:: <-- jedi-language-server client/registerCapability(5f60e62b-e254-4e14-9bc1-84ca459b4095): {'registrations': [{'id': '086b4cc1-8511-43db-bf9a-c3e66410db49', 'method': 'textDocument/didChange', 'registerOptions': {}}]}
:: >>> jedi-language-server 5f60e62b-e254-4e14-9bc1-84ca459b4095: None
:: <-- jedi-language-server client/registerCapability(35874c00-9974-4aa3-a032-76f08914f4ca): {'registrations': [{'id': '1e6b6b84-4f18-4521-8625-f663e1a32187', 'method': 'textDocument/didSave', 'registerOptions': {}}]}
:: >>> jedi-language-server 35874c00-9974-4aa3-a032-76f08914f4ca: None
:: <-  jedi-language-server window/showMessage: {'type': 3, 'message': 'jedi-language-server initialized'}

Wouldn't it be easier to put all this stuff in the initialize response?

@pappasam
Copy link
Owner

pappasam commented May 1, 2020

Yep, I've been mulling over this myself! That said, I haven't run into any situations so far (since 10 days ago at least) where I've regretted dynamic registration for performance reasons, and it's resulted in some pretty clean code (or at least as clean as I can make it with the pygls interface). This topic is actually part of an ongoing conversation in pygls: openlawlibrary/pygls#112

@rwols
Copy link
Author

rwols commented May 1, 2020

But will you ever de-register as completionProvider during the lifetime of the server?

@pappasam
Copy link
Owner

pappasam commented May 1, 2020

I doubt I will! This is definitely something I'm happy to add the the roadmap. I'll also prioritize it immediately if we notice sluggish server start times, or if it's clearly demonstrated that moving some things to static configuration simplifies the code.

If you have any ideas regarding implementation, I'm all ears!

@rwols
Copy link
Author

rwols commented May 2, 2020

The start up time doesn't matter that much to me :)

One thing you can do to make this much more performant is to collect all your registrations and send them in one big list with a single request, instead of one request per capability.

@harismandal
Copy link

Thank you @pappasam for implementing this! The implementation seems much cleaner than pyls to me.

I am trying to use the server in Emacs with lsp-mode and dynamic registration of capabilities seems to cause issues.

The client seems to permit dynamic registration of only certain capabilities. As I am not familiar with the LSP specification, I can open up an issue to discuss it there.

After moving completion registration to initialize response like this, completetion starts working for me. I didn't test all of the capabilities, as I can use a previous version of the server with static capabilities for now.

@pappasam
Copy link
Owner

pappasam commented May 3, 2020

@harismandal I'm trying to enable end-users to configure things like "triggerCharacters". See this issue, which is still an active discussion on pygls: openlawlibrary/pygls#112

I'm hoping a resolution will be as simple as moving logic from INITIALIZED to INITIALIZE. I'm working on adding a feature today and will look into this once that's merged

@rwols
Copy link
Author

rwols commented May 3, 2020

If all you want to do is to let users decide what the trigger chars are, then you can use initializationOptions. These are passed in the initialize request and you can then set your trigger chars to whatever in the initialize response.

IMO letting the user decide what the trigger chars are is bad anyway. The completion trigger char is ., and the signatureHelp trigger chars are ( and ,, end of story.

@pappasam
Copy link
Owner

pappasam commented May 3, 2020

@rwols completion characters are somewhat complicated by jedi's dictionary key completion support. I add additional completion characters " and ' to auto-complete dictionary keys, but I understand this decision might annoy some users who'd rather disable it.

davidhalter/jedi#1478

That said, can you please test out the latest version 0.11.0 and let me know if textDocument/documentSymbol registers dynamically or statically? If it registers statically, i may have an idea of how to elegantly resolve this.

@rwols
Copy link
Author

rwols commented May 3, 2020

The trigger chars are a broken part of the LSP spec. Language servers should not be bothered by this and just provide their maximum amount of trigger chars.

For instance: if your language server was also capable of handling Lua files, you would be forced to advertise : as another trigger char, but it would only apply to Lua files.

Not showing the auto-complete widget in certain scenarios is a job for editors, not language servers.

@rwols
Copy link
Author

rwols commented May 3, 2020

In the case of SublimeText, we would have to adjust the auto_complete_selector to not trigger " for plain strings, only for when the cursor is in a dictionary context.

@harismandal
Copy link

harismandal commented May 3, 2020

That said, can you please test out the latest version 0.11.0 and let me know if textDocument/documentSymbol registers dynamically or statically? If it registers statically, i may have an idea of how to elegantly resolve this.

They are not sent in the initialize response for me.

[Trace - 05:58:18 PM] Received response 'initialize - (3)' in 162ms.
Result: {
  "capabilities": {
    "workspace": {
      "workspaceFolders": {
        "changeNotifications": true,
        "supported": true
      }
    },
    "executeCommandProvider": {
      "commands": []
    },
    "renameProvider": null,
    "documentRangeFormattingProvider": null,
    "documentFormattingProvider": null,
    "codeActionProvider": null,
    "workspaceSymbolProvider": null,
    "documentSymbolProvider": null,
    "documentHighlightProvider": null,
    "referencesProvider": null,
    "definitionProvider": null,
    "hoverProvider": null,
    "textDocumentSync": 2
  }
}

[Trace - 05:58:18 PM] Received request 'client/registerCapability - (244f7c3d-d51d-4e07-866c-5da06bcda40d).
Params: {
  "registrations": [
    {
      "registerOptions": {
      },
      "method": "textDocument/documentSymbol",
      "id": "4cd02d73-16ca-46dd-a6d7-4472ad6107c1"
    }
  ]
}

@harismandal
Copy link

Maybe you can do something like this?

@pappasam
Copy link
Owner

pappasam commented May 3, 2020

@harismandal I'll give that a try!

pappasam added a commit that referenced this issue May 3, 2020
Now the only dynamic configurations are for diagnostics. Aims to resolve
#15
@pappasam pappasam reopened this May 3, 2020
@pappasam
Copy link
Owner

pappasam commented May 3, 2020

Can you try version 0.12.0 and let me know if this issue is resolved?

@pappasam pappasam mentioned this issue May 3, 2020
@harismandal
Copy link

Yes, it's working great for me now. Thanks for fixing this! I'll try to create a PR in lsp-mode for emacs mentioning the server and adding the configurations.

@harismandal
Copy link

Just for the sake of it, here are all capabilities being sent during initialization:

[Trace - 10:57:02 PM] Received response 'initialize - (19)' in 161ms.
Result: {
  "capabilities": {
    "workspace": {
      "workspaceFolders": {
        "changeNotifications": true,
        "supported": true
      }
    },
    "executeCommandProvider": {
      "commands": []
    },
    "renameProvider": true,
    "documentRangeFormattingProvider": null,
    "documentFormattingProvider": null,
    "codeActionProvider": null,
    "workspaceSymbolProvider": true,
    "documentSymbolProvider": true,
    "documentHighlightProvider": true,
    "referencesProvider": true,
    "definitionProvider": true,
    "signatureHelpProvider": {
      "triggerCharacters": [
        "(",
        ",",
        ")"
      ]
    },
    "completionProvider": {
      "triggerCharacters": [
        ".",
        "'",
        "\""
      ],
      "resolveProvider": null
    },
    "hoverProvider": true,
    "textDocumentSync": 2
  }
}

@pappasam pappasam closed this as completed May 4, 2020
@rwols
Copy link
Author

rwols commented May 4, 2020

"triggerCharacters": [
        "(",
        ",",
        ")"
      ]

The ) should not be a trigger char for signatureHelp. Means that if the user is done typing the args of a function and types the ) character, the editor requests textDocument/signatureHelp, and then you won't be in a function signature context.

@pappasam
Copy link
Owner

pappasam commented May 4, 2020

@rwols done

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