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

Specify SaveOptions in server capabilities #320

Merged
merged 1 commit into from
Jun 2, 2023

Conversation

tarkah
Copy link
Contributor

@tarkah tarkah commented Jun 2, 2023

textDocument/didSave is specified by the server capability of SaveOptions or bool in the TextDocumentSyncOptions. Since this wasn't getting set, some clients may not send textDocument/didSave since the server never broadcasted this capability.

includeText is set to false since I didn't see anywhere we use the text contents when responding to this method.

I've confirmed this fixes a bug in the client helix as dub build lints weren't getting triggered since it wouldn't send the textDocument/didSave message unless this capability was sent.

Relevant docs: https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_didSave

Related issue on Helix side: helix-editor/helix#3957

`textDocument/didSave` is specified by the server capability of
SaveOptions or bool in the TextDocumentSyncOptions. Since this wasn't
getting set, some clients may not send `textDocument/didSave` since the
server never broadcasted this capability.

`includeText` is set to `false` since I didn't see anywhere we use the
text contents when responding to this method.

I've confirmed this fixes a bug in the client `helix` as dub build lints
weren't getting triggered since it wouldn't send the
`textDocument/didSave` message unless this capability was sent.
Copy link
Member

@WebFreak001 WebFreak001 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks! Yeah I had a special config with code-d & serve-d before the LSP options were able to ask the client for file watching like this, so I never upgraded this part of the code.

If content sync becomes a problem on saving, might want to include the text in the future and submit it to the text document manager, although I think that won't be necessary.

@WebFreak001
Copy link
Member

my bad on the test case failures, although I'm not sure why it's trying to download new versions when I include a dub.selections.json. Will investigate in master

@WebFreak001 WebFreak001 merged commit 94f0291 into Pure-D:master Jun 2, 2023
rtbo added a commit to rtbo/serve-d that referenced this pull request Jun 6, 2023
textDocument/didOpen
was not sent anymore by VS code since Pure-D#320
WebFreak001 pushed a commit that referenced this pull request Jun 8, 2023
textDocument/didOpen
was not sent anymore by VS code since #320
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

Successfully merging this pull request may close these issues.

2 participants