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 wrong unit when split string #839

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

petricavalry
Copy link

@petricavalry petricavalry commented Sep 30, 2024

Fixes #837.

let (match_str, remaining_str) = suggestion.value.split_at(match_len);

split_at expect byte offset.

let match_len = self.working_details.shortest_base_string.width();

But we given (got from width) isn't.

Thanks for all amazing works in reedline.

@sholderbach
Copy link
Member

Yeah for anything involving string splitting .len is the relevant thing to get to the indices, while .width serves as a best guess for the layout width in the terminal. We need to be sure that the right thing is used in the right place.

There seems to be some preexisting history of display bugs leading to this

@petricavalry
Copy link
Author

petricavalry commented Sep 30, 2024

I'm not sure but I can't reproduce #793. match_len is used to split string only. It seems to have been modified incorrectly by #794 and cause this issues (#837).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Panic when completion menu items contains UTF-8 characters and (
2 participants