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

implement textDocument/didSave #165

Closed
prabirshrestha opened this issue Aug 26, 2017 · 7 comments
Closed

implement textDocument/didSave #165

prabirshrestha opened this issue Aug 26, 2017 · 7 comments
Labels

Comments

@prabirshrestha
Copy link

In vim-lsp I automatically didSave when a file is saved. Would be good if this was implemented.

https://github.com/Microsoft/language-server-protocol/blob/master/protocol.md#textDocument_didSave

@rcjsuen
Copy link
Owner

rcjsuen commented Aug 26, 2017

@prabirshrestha I don't understand. The protocol states the following:

The document save notification is sent from the client to the server when the document was saved in the client.

This is a notification from the client to the server. I don't think this is something for the language server to implement.

@prabirshrestha
Copy link
Author

My bad. I was sending a request instead of notification and was getting not supported error by the server. Sorry for the noise.

@rcjsuen
Copy link
Owner

rcjsuen commented Aug 26, 2017

@prabirshrestha Thank you for checking, Prabir. :)

You should also check if the language server you are connected to is actually interested in that event before sending it. Check out the ServerCapabilities.textDocumentSync section in the protocol.

The ServerCapabilities.textDocumentSync.save setting may not be set. In that case, you don't want to waste resources sending something over the wire if the language server you are talking to isn't even interested in the that event.

@prabirshrestha
Copy link
Author

prabirshrestha commented Aug 26, 2017

Seems like I need to update vim-lsp. Filed a bug to track this. prabirshrestha/vim-lsp#29. Thanks for the details.

@rcjsuen
Copy link
Owner

rcjsuen commented Aug 27, 2017

@prabirshrestha Actually, I think I might have been wrong about when a client should be sending that notification to the server. Sorry about that.

I think a client might have to always send the notification unless the ServerCapabilities.textDocumentSync.save value is defined and set to false. Version 2.x of the protocol had the textDocument/didSave notification but it wasn't possible for the server to declare whether it was interested in the event or not so I think the expectation is that the notification will always be sent to the server unless it explicitly says it isn't interested in it.

For example, my language server only sets a TextDocumentSyncKind number for its ServerCapabilities.textDocumentSync property but VS Code still sends me textDocument/didSave notifications although it doesn't include the document's text in the notification.

@rcjsuen
Copy link
Owner

rcjsuen commented Aug 27, 2017

@prabirshrestha Actually, ServerCapabilities.textDocumentSync.save won't be set to false because it's a SaveOptions object and not a boolean. So I think the correct behaviour would be for the client to not send the notification only if that property is undefined.

I could be wrong of course so you may want to open an issue and ask for clarifications.

Sorry for all the confusion that I'm causing!

@prabirshrestha
Copy link
Author

Filed an issue at microsoft/language-server-protocol#288.

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

2 participants