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

Don't send textDocument/didSave unless server advertises support for it #550

Closed
tophattom opened this issue Apr 4, 2019 · 3 comments
Closed
Labels

Comments

@tophattom
Copy link

I ran into an issue with using Dart analyzer server in LSP mode. Every time I would save a file I would get an error saying Unknown method textDocument/didSave. Turns out this should be fixed in the client by not sending textDocument/didSave if the server doesn't support that.

More information: dart-lang/sdk#36464 and natebosch/vim-lsc#164

Please include in your report:

  • Minimal reproduction steps:
    Setup Dart language server and try saving Dart files:
    "dart": {
     	"command": ["<DART SDK PATH>/bin/dart", "<DART SDK PATH>/bin/snapshots/analysis_server.dart.snapshot", "--lsp", "--instrumentation-log-file", "/usr/local/var/log/lsp.sublime.txt"],
     	"enabled": false,
     	"languageId": "dart",
     	"scopes": [
     		"source.dart"
     	],
     	"syntaxes": [
     	     "Packages/Dart/Dart.tmLanguage"
     	]
    },

cc @DanTup

@DanTup
Copy link

DanTup commented Apr 4, 2019

(I'm a dev on the Dart LSP server)

I don't think the spec is super clear here (though I've opened microsoft/language-server-protocol#713 asking for it to be made clearer) but the reason I think this is a client issue is based on the table at microsoft/language-server-protocol#288 (comment). The didSave notification wasn't in the original version and therefore is opted in to by the server populating the save field of ServerCapabilities.textDocumentSync.

We could add an empty handler in the Dart server to ignore this , but I think it would generally improve compatibility with more servers if it wasn't sent by the client in this case.

@tomv564
Copy link
Contributor

tomv564 commented Apr 5, 2019

Yep, doesn't look like the nicest logic based on the table in that linked issue, but I agree we should follow those rules. A PR would be much appreciated!

@rwols rwols added the bug label Apr 6, 2019
@DanTup
Copy link

DanTup commented Apr 25, 2019

I'm not a sublime user and not well placed to contribute this, though FWIW the LSP spec was updated to make this clearer (our assumptions that it shouldn't be sent were correct): microsoft/language-server-protocol#713

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

No branches or pull requests

4 participants