Skip to content

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
@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
mtoohey31 pushed a commit to mtoohey31/helix that referenced this pull request Jun 2, 2024
Vulpesx pushed a commit to Vulpesx/helix that referenced this pull request Jun 7, 2024
smortime pushed a commit to smortime/helix that referenced this pull request Jul 10, 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