Only apply textEdit offset when LSP column is less than cursor column
#654
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When the range of a `textEdit` completion starts after the current column, applying a negative offset mangles completion text. Additionally, treat the LSP range as zero-indexed in both comparison and offset calculation.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I was having an issue with method/property completions in TypeScript. Pressing C-X C-O immediately after the dot worked, as did doing it after typing one or two characters after the dot. However completing something like
new Function().appto "apply" didn't.After some debugging, I realized that the text of the completion was getting mangled, so it no longer had the prefix the filter function was looking for. When the column reported by the language server is greater than the current cursor column, the offset calculated by the
textEdithandler becomes negative, andd.wordis set to the last few characters ofnewText. I think #161, which introduced the offset, was only meant to apply in cases where LSP column is less than cursor column.I also looked into the reason this happens in the first place. typescript-language-server finds the dot accessor
.or?.and tries to send a completion that includes it (presumably to allow tsserver to replace.with?.). It calculates the range based on the current column and the length of the accessor. But, at the same time the completion is requested, it receives atextDocument/didChangenotification replacing document contents with a shorter line, one that only includes the text up to the dot.For example, when I type
new Function().appand press C-X C-O, this is the document the language server receives (15 characters on that line):And the completion is requested with the starting position at character 17. So the length of the dot is 1, the starting position of the
textEditspan is 17 - 1 = 16, which is greater than the column insideCompletionReply. Trying to find the text of the accessor it gets an empty string; the result is a correctnewTextnot including the accessor dot and with a broken range.The math typescript-language-server uses is wrong in that case, but I don't want to open an issue there, because I'm not sure this is a correct request from the LSP client. This PR doesn't fix the underlying problem, but it does work around the completions issue after
new Function().appby removing a bug and leaving a not fully correct behavior in its place. Feel free to reject if it's too hacky.(Apologies for the edits, I couldn't find the right PR to reference.)