-
Notifications
You must be signed in to change notification settings - Fork 37
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
fix: resolve #986 avoid wrong offset on Neovim for some postfix snippets #966
base: main
Are you sure you want to change the base?
Conversation
Other uses of |
detail: Some(snippet.description.clone().into()), | ||
insert_text_format: Some(InsertTextFormat::SNIPPET), | ||
..Default::default() | ||
}; | ||
if let Some(node_before_before_cursor) = &node_before_before_cursor { | ||
let node_content = node.get().clone().into_text(); | ||
let before = TextEdit { | ||
range: self.ctx.to_lsp_range(rng.start..self.from, &src), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The range of main edit can only start at the cursor, but rng.start
is before the cursor.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably not, the lsp spec says, "it must be a [single line] and they must contain the position at which completion has been requested," rather than "start with the position".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that's why it is ended in self.from.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe I should use the insertandreplace capability of lsp
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As the lsp spec said above, rng.start..self.from
may span across multiple lines but the spec requires it MUST be in a single line so I don't think this is a valid fix. Btw, I checked it in my local neovim, and at least the case in #986 can work as expected. It might be worth to checking whether some neovim completion plugins don't work while others do work. I use the default LazyVim plus the tinymist.lua
from the commit.
I am not sure if this is a common problem, so I am turning this pr in to a specific one for #986, thus the current implementation should be enough. |
9415642
to
12e74bb
Compare
12e74bb
to
5eb0668
Compare
resolve #986
According to lsp spec, additional text edit is not supposed to be used here.