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

fix(lsp): find_completion_range off-by-one #11266

Merged
merged 1 commit into from
Jul 24, 2024

Conversation

RoloEdits
Copy link
Contributor

@RoloEdits RoloEdits commented Jul 21, 2024

Fixes an off-by-one error that caused index out of bounds for some LSP responses, or the eating of newlines for others.

Fixes: #10689

@kirawi kirawi added the A-language-server Area: Language server client label Jul 23, 2024
Copy link
Member

@the-mikedavis the-mikedavis left a comment

Choose a reason for hiding this comment

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

Instead I think we should cap end to text.len_chars() in find_completion_range in the if replace_mode block. @pascalkuthe might have more context for this change though

@pascalkuthe
Copy link
Member

This is not a python bug but a bugnwith our code. I knew of this for a whole but I thought I (or something else) already fried it.

This is however not the right fix for the issue. This was a logic bug introduced by me into this block:

end += text

Both the skip(1) and the + 1need to be removed. This should also fix completion replace eating newline and similar characters.

@pascalkuthe pascalkuthe added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. E-has-instructions Call for participation: Has instructions for fixing the issue and opening a PR labels Jul 24, 2024
@RoloEdits RoloEdits changed the title fix(lsp): add fallback for range completion edits fix(lsp): find_completion_range off-by-one Jul 24, 2024
@the-mikedavis the-mikedavis merged commit 7c5e5f4 into helix-editor:master Jul 24, 2024
6 checks passed
@RoloEdits RoloEdits deleted the fix-lsp-range-oob branch July 24, 2024 15:34
@SoraTenshi SoraTenshi mentioned this pull request Jul 27, 2024
38 tasks
SofusA pushed a commit to SofusA/helix-pull-diagnostics that referenced this pull request Aug 4, 2024
mxxntype pushed a commit to mxxntype/helix that referenced this pull request Aug 14, 2024
stackotter pushed a commit to stackotter/helix that referenced this pull request Aug 28, 2024
kyruzic pushed a commit to kyruzic/helix that referenced this pull request Sep 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-language-server Area: Language server client E-has-instructions Call for participation: Has instructions for fixing the issue and opening a PR S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Panic Error in Python with Pyright/Pylsp & Ruff
4 participants