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

[LS] Code actions don't return version with textDocument #851

Closed
non25 opened this issue Mar 2, 2021 · 5 comments
Closed

[LS] Code actions don't return version with textDocument #851

non25 opened this issue Mar 2, 2021 · 5 comments
Labels
bug Something isn't working Fixed Fixed in master branch. Pending production release.

Comments

@non25
Copy link

non25 commented Mar 2, 2021

This prevents code-actions from working on editors which expect returned version number to be >= of what was sent to the LS.

tsserver sends same version back and code-actions work just fine.

https://github.com/neovim/neovim/blob/c1fbc2ddf15b2f44b615f90b2511349ab974cb83/runtime/lua/vim/lsp/util.lua#L265-L272
Here's code from neovim-lsp that checks version number.

Will edit more information when I find the differences between svelteserver and tsserver regarding sending version number.

Edit: For now these don't work:

Code Actions:                                                                     
1. (svelte) Disable missing-declaration for this line
2. Import 'teenStyle' from module "./helpers"
@dummdidumm
Copy link
Member

Probably this line needs to get the correct version

VersionedTextDocumentIdentifier.create(pathToUrl(change.fileName), 0),

@dummdidumm dummdidumm added the bug Something isn't working label Mar 3, 2021
@dummdidumm
Copy link
Member

For Svelte files this should be doable, for TS / JS files however we should leave it as currently because we simply do not know what to set it to since we don't get the current document versions of those.

@non25
Copy link
Author

non25 commented Mar 3, 2021

For Svelte files this should be doable, for TS / JS files however we should leave it as currently because we simply do not know what to set it to since we don't get the current document versions of those.

Do you mean the situation when we need to apply changes to external js/ts files due to code-action in svelte component?

Can't think of an example right now. 🤔

It is interesting, I haven't tested what would happen if I use rename in .ts file. Would that change be reflected in svelte components... 🤔
Brb. 😄

Wow... it won't. So it looks like svelte language server should be configured for .ts|.js files in svelte projects too ?

@dummdidumm
Copy link
Member

It does not and making this possible is tracked in #580

Renames in Svelte files that affects JS files are an example of where the language server returns edits for JS/TS files.

@mfussenegger
Copy link

For Svelte files this should be doable, for TS / JS files however we should leave it as currently because we simply do not know what to set it to since we don't get the current document versions of those.

In the spec the textDocument of the TextDocumentEdit is a OptionalVersionedTextDocumentIdentifier, so the version can be null, and it should be null instead of 0 if the server cannot provide the correct version number.

dummdidumm pushed a commit to dummdidumm/language-tools that referenced this issue Apr 14, 2021
Setting it to 0 is against the spec since clients may reject the edits then. Set it to null instead.
sveltejs#851
dummdidumm added a commit that referenced this issue Apr 14, 2021
Setting it to 0 is against the spec since clients may reject the edits then. Set it to null instead.
#851
@dummdidumm dummdidumm added the Fixed Fixed in master branch. Pending production release. label Apr 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Fixed Fixed in master branch. Pending production release.
Projects
None yet
Development

No branches or pull requests

3 participants