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

Track cursor offset before bias in Supermaven completion provider #18858

Merged
merged 3 commits into from
Oct 10, 2024

Conversation

kevmo314
Copy link
Contributor

@kevmo314 kevmo314 commented Oct 8, 2024

Track the cursor offset before biasing in the Supermaven completion provider to better determine if the text should be suggested. The underlying issue here is due to the way anchor biasing works, the completion provider is not able to determine if a given suggestion's cursor location no longer exists as it is always coalesced to a correct location (specifically, the end of the line).

This change updates that logic so the offset is stored independently of the buffer so it can be used to represent a location that may not exist in the buffer anymore to represent locations that have been deleted.

The net effect is that suggestions can be backspaced much more cleanly with Supermaven.

image

image

Release Notes:

@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Oct 8, 2024
@zed-industries-bot
Copy link

zed-industries-bot commented Oct 8, 2024

Messages
📖

This PR includes links to the following GitHub Issues: #17981
If this PR aims to close an issue, please include a Closes #ISSUE line at the top of the PR body.

Generated by 🚫 dangerJS against 4f61d84

Copy link
Contributor

@SomeoneToIgnore SomeoneToIgnore left a comment

Choose a reason for hiding this comment

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

Tested and it looks much better with backspacing and removal indeed, thank you!

@SomeoneToIgnore SomeoneToIgnore merged commit 21c27ce into zed-industries:main Oct 10, 2024
9 checks passed
SomeoneToIgnore pushed a commit that referenced this pull request Oct 14, 2024
Closes #19051
Closes #19182


#### How to reproduce this crash:
1. Open any file and input some ASCII characters.
2. Replace these characters with `你好`.
3. Press `backspace`.
4. Crash.



https://github.com/user-attachments/assets/ea5c5340-29a5-42c8-98c5-6e60770445a4



The issue lies with the `prefix_offset` introduced in #18858. After the
buffer is modified, this value is not always valid and may fall within a
`char boundary`, which results in a crash.



Release Notes:

- Fixed Supermaven crashing on deleting non-ASCII text
noaccOS pushed a commit to noaccOS/zed that referenced this pull request Oct 19, 2024
…d-industries#18858)

Track the cursor offset before biasing in the Supermaven completion
provider to better determine if the text should be suggested. The
underlying issue here is due to the way anchor biasing works, the
completion provider is not able to determine if a given suggestion's
cursor location no longer exists as it is always coalesced to a correct
location (specifically, the end of the line).

This change updates that logic so the offset is stored independently of
the buffer so it can be used to represent a location that may not exist
in the buffer anymore to represent locations that have been deleted.

The net effect is that suggestions can be backspaced much more cleanly
with Supermaven.


![image](https://github.com/user-attachments/assets/ff61aa09-54ea-4cad-b1ca-633a08bcdd96)


![image](https://github.com/user-attachments/assets/b49e2d6b-f1d3-41a1-9b75-c4bc3ac5f85b)

Release Notes:

- Improves zed-industries#17981 to prevent
suggesting completions based on out-of-date cursor locations.
noaccOS pushed a commit to noaccOS/zed that referenced this pull request Oct 19, 2024
Closes zed-industries#19051
Closes zed-industries#19182


#### How to reproduce this crash:
1. Open any file and input some ASCII characters.
2. Replace these characters with `你好`.
3. Press `backspace`.
4. Crash.



https://github.com/user-attachments/assets/ea5c5340-29a5-42c8-98c5-6e60770445a4



The issue lies with the `prefix_offset` introduced in zed-industries#18858. After the
buffer is modified, this value is not always valid and may fall within a
`char boundary`, which results in a crash.



Release Notes:

- Fixed Supermaven crashing on deleting non-ASCII text
@liam-k
Copy link

liam-k commented Oct 24, 2024

thank you for this fix!! i just read the changelog & realized THIS was what made supermaven so annoying and error-prone to me. helps a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed The user has signed the Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants