Skip to content

fix: floor word_end to char boundary in abbreviation expansion (panic on multibyte char before cursor)#1121

Merged
kronberger-droid merged 2 commits into
nushell:mainfrom
easyinplay:fix/abbrev-expansion-char-boundary-panic
Jul 21, 2026
Merged

fix: floor word_end to char boundary in abbreviation expansion (panic on multibyte char before cursor)#1121
kronberger-droid merged 2 commits into
nushell:mainfrom
easyinplay:fix/abbrev-expansion-char-boundary-panic

Conversation

@easyinplay

@easyinplay easyinplay commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Summary

try_expand_abbreviation_at_cursor in src/engine.rs panics when a multi-byte UTF-8 character (e.g. CJK text) sits immediately before the cursor at expansion time. This PR floors the word-end index down to the nearest char boundary before slicing, so the function returns None instead of panicking when the cursor is mid-char.

No change to Reedline's public API. The only observable behavior change: an input that previously aborted the process now expands nothing at that position (instead of panicking).

Before

try_expand_abbreviation_at_cursor computes the word end as a raw byte subtraction:

let word_end = cursor_position_in_buffer - offset; // offset: 0 on <enter>, 1 on <space>
let prefix = &buffer[..word_end];

When the byte just before the cursor belongs to a multi-byte char, word_end lands inside that char, and &buffer[..word_end] panics:

byte index N is not a char boundary; it is inside '中' (bytes 0..3) of `中`

Repro: paste CJK text into a reedline prompt with abbreviations enabled (both terminal bracketed paste and system-clipboard paste trigger it); the following expansion check slices the buffer at a non-char-boundary and the process panics. Minimal in-crate case: buffer "中", cursor at byte offset 1, try_expand_abbreviation_at_cursor(true) panics on the slice on current main.

After

word_end is floored to the nearest char boundary before slicing:

let mut word_end = cursor_position_in_buffer - offset;
while word_end > 0 && !buffer.is_char_boundary(word_end) {
    word_end -= 1;
}
let prefix = &buffer[..word_end];

The rest of the function is unchanged. The slice now always lands on a char boundary; when the cursor is mid-char there is no complete word to expand, so the function returns None instead of panicking.

Additional notes

  • No public API change; behavior change is limited to the mid-char case described above.
  • Adds a regression test try_expand_abbreviation_survives_multibyte_char_before_cursor in the existing engine::tests module: it places the cursor inside a multi-byte char and asserts the call returns without panicking. Verified the test panics on main and passes with the fix. Full cargo test --lib remains green (1173 passed).

`try_expand_abbreviation_at_cursor` computed `word_end` by subtracting a
raw byte `offset` (0 on <enter>, 1 on <space>) from the byte cursor
position, then sliced `&buffer[..word_end]`. When a multi-byte UTF-8 char
(e.g. pasted CJK text) sits immediately before the cursor, `word_end` can
land inside that char, panicking with "byte index N is not a char
boundary".

Floor `word_end` down to the nearest char boundary before slicing. Adds a
regression test that reproduces the panic with a multi-byte char before
the cursor.

Signed-off-by: easyinplay <easyinplay@gmail.com>
@kronberger-droid

Copy link
Copy Markdown
Collaborator

I have had this bug on my mind already, nice for you to pick it up.

But could it be that you used an LLM for this?
Since our PR template is not used and it just seems like it.
It's not a problem if you did, but it's good to know if you used one, since I factor it in in the review.
If not I am sorry for the assumption.

I will take a closer look next week.

@easyinplay

easyinplay commented Jul 18, 2026

Copy link
Copy Markdown
Contributor Author

I have had this bug on my mind already, nice for you to pick it up.

But could it be that you used an LLM for this? Since our PR template is not used and it just seems like it. It's not a problem if you did, but it's good to know if you used one, since I factor it in in the review. If not I am sorry for the assumption.

I will take a closer look next week.

Because I have been away from school for too long, my English may not be very accurate in describing some technical vocabulary. Therefore, I provided the relevant PR content to AI in Chinese, hoping that it could help me describe it as clearly as possible. Perhaps I missed the task of using a template, which I will pay attention to later.
Thank you very much for your work at Reedline

@kronberger-droid

Copy link
Copy Markdown
Collaborator

No problem, thanks for clarifying.
Great that we can overcome some of the language barrier this way.

I will take a look at it more closely this week.

@kronberger-droid

Copy link
Copy Markdown
Collaborator

One thing to add here.
I think you could reuse floor_char_boundary in menu_functions.rs::333 for this.

Signed-off-by: easyinplay <easyinplay@gmail.com>
@easyinplay

Copy link
Copy Markdown
Contributor Author

Done — reused floor_char_boundary from menu_functions as suggested (via crate::menu_functions::floor_char_boundary). Dropped the manual loop; behavior is unchanged. cargo test --lib (1173 passed) and cargo clippy stay green. Thanks for the pointer!

@kronberger-droid
kronberger-droid merged commit 46e8e13 into nushell:main Jul 21, 2026
7 checks passed
@easyinplay
easyinplay deleted the fix/abbrev-expansion-char-boundary-panic branch July 22, 2026 05:25
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.

2 participants