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 crash on LSP text edits with invalid ranges #9649

Merged

Conversation

skyl4b
Copy link
Contributor

@skyl4b skyl4b commented Feb 17, 2024

This PR attempts to fix #9624.

While testing helix-gpt out, helix crashed many times for me. I decided to take a shot at fixing this issue myself, with a simple check inside the Transaction change edit's closure. When the range is invalid, I simply discarded the edit and logged it.

As noted in the issue, this is mainly a problem inside helix-gpt itself, but a crash is undesirable.

@the-mikedavis the-mikedavis added C-bug Category: This is a bug A-language-server Area: Language server client S-waiting-on-review Status: Awaiting review from a maintainer. labels Feb 17, 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.

There's also an lsp_range_to_range helper that handles this case, but it handles it by capping range.start to range.end. I think discarding the edit makes more sense in this case. What do you think @pascalkuthe?

Copy link
Member

@pascalkuthe pascalkuthe left a comment

Choose a reason for hiding this comment

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

Invalid completions already have some logic dedicated to them and I would like to check that there isn't some new bug here before merging this

@skyl4b
Copy link
Contributor Author

skyl4b commented Mar 4, 2024

If there's anything else I can do to help with this issue, please let me know.

@pascalkuthe pascalkuthe merged commit d3bfa3e into helix-editor:master Apr 6, 2024
6 checks passed
@skyl4b skyl4b deleted the fix-crash-lsp-invalid-edit-range branch April 6, 2024 10:17
postsolar pushed a commit to postsolar/helix that referenced this pull request Apr 20, 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 C-bug Category: This is a bug S-waiting-on-review Status: Awaiting review from a maintainer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Helix crashes on long completions from helix-gpt LSP
3 participants