Skip to content

languages: Correctly calculate ranges in label_for_completion #44925

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.

Sign up for GitHub

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

Jump to bottom
Merged
SomeoneToIgnore merged 2 commits into from
Dec 16, 2025
Merged

languages: Correctly calculate ranges in label_for_completion #44925

SomeoneToIgnore merged 2 commits into from
Dec 16, 2025

Conversation

@Nereuxofficial
Copy link
Contributor

@Nereuxofficial Nereuxofficial commented Dec 15, 2025

Closes #44825

Release Notes:

  • Fixed a case where an incorrect match could be generated in label_for_completion
@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Dec 15, 2025
@maxdeviant maxdeviant changed the title fix: Correctly calculate ranges in label_for_completion Dec 15, 2025
@Nereuxofficial Nereuxofficial force-pushed the fix/code_label_crash branch 2 times, most recently from 3d6e080 to 0812872 Compare December 15, 2025 22:31
@MrSubidubi MrSubidubi changed the title languages: Correctly calculate ranges in label_for_completion Dec 15, 2025
SomeoneToIgnore
SomeoneToIgnore reviewed Dec 16, 2025
View reviewed changes
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.

Thank you, but cannot you add an actual test for this?
@SomeoneToIgnore SomeoneToIgnore self-assigned this Dec 16, 2025
@MMaikov
Copy link

MMaikov commented Dec 16, 2025

Thank you, while this fixes the issue I found a performance problem. After typing 'res' and getting the auto suggestion box to show up, scrolling through it is slow and zed logged a hangup. Repro is same as in #44825
hang-2025-12-16_09-03-17.miniprof.zip
@Nereuxofficial
Copy link
Contributor Author

Nereuxofficial commented Dec 16, 2025 •
edited
Loading

Thank you, but cannot you add an actual test for this?

Thanks for the review! I've now added a test which tests whether the ranges are correct for a string where "…" characters were inserted for empty tabstops. Let me know if i should make any further adjustments.
@Nereuxofficial
Copy link
Contributor Author

Nereuxofficial commented Dec 16, 2025

Thank you, while this fixes the issue I found a performance problem. After typing 'res' and getting the auto suggestion box to show up, scrolling through it is slow and zed logged a hangup. Repro is same as in #44825 hang-2025-12-16_09-03-17.miniprof.zip

I cannot reproduce this locally but from my interpretation this looks like not related to this code as it is not found in the trace. Could you try disabling your extensions temporarily and see whether that fixes the issue(as wasm_host.rs is consumes a lot of time in the statistics)?
SomeoneToIgnore
SomeoneToIgnore approved these changes Dec 16, 2025
View reviewed changes
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.

Thank you, will merge one nit on top, so that the text in the error is shown better.
@SomeoneToIgnore SomeoneToIgnore enabled auto-merge (squash) December 16, 2025 14:38
@SomeoneToIgnore SomeoneToIgnore merged commit da0960b into zed-industries:main Dec 16, 2025
23 checks passed
@MMaikov
Copy link

MMaikov commented Dec 16, 2025 •
edited
Loading

Thank you, while this fixes the issue I found a performance problem. After typing 'res' and getting the auto suggestion box to show up, scrolling through it is slow and zed logged a hangup. Repro is same as in #44825 hang-2025-12-16_09-03-17.miniprof.zip

I cannot reproduce this locally but from my interpretation this looks like not related to this code as it is not found in the trace. Could you try disabling your extensions temporarily and see whether that fixes the issue(as wasm_host.rs is consumes a lot of time in the statistics)?

I build it again in release mode and the problem was gone.

Note to self: always test performance in release mode.
@MrSubidubi
Copy link
Member

MrSubidubi commented Jan 2, 2026

/cherry-pick preview
@zed-zippy
Copy link
Contributor

zed-zippy bot commented Jan 2, 2026

🍒💥 Cherry-pick did not succeed
https://github.com/zed-industries/zed/actions/runs/20657811969
@MrSubidubi
Copy link
Member

MrSubidubi commented Jan 2, 2026

/cherry-pick stable
@zed-zippy
Copy link
Contributor

zed-zippy bot commented Jan 2, 2026

🍒💥 Cherry-pick did not succeed
https://github.com/zed-industries/zed/actions/runs/20657834524
MrSubidubi pushed a commit that referenced this pull request Jan 2, 2026
@Nereuxofficial @SomeoneToIgnore @MrSubidubi
languages: Correctly calculate ranges in label_for_completion (#44925)
c1f4f42 
Closes #44825

Release Notes:

- Fixed a case where an incorrect match could be generated in
label_for_completion

---------

Co-authored-by: Kirill Bulatov <mail4score@gmail.com>
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

4 participants

@Nereuxofficial @MMaikov @MrSubidubi @SomeoneToIgnore