fix(hook): convert shell-native cursor unit to byte at the boundary (closes #6) - #11
Merged
Merged
Conversation
…loses #6) Shells send `--cursor` in their own unit: bash, zsh, clink, and nu count Unicode scalar values (= chars); pwsh counts UTF-16 code units (its PSReadLine / .NET native unit). The domain hook treated the value as a byte offset, which agreed by accident on ASCII but split mid-character on Japanese, emoji, or any other non-ASCII buffer. Reporter's PoC reproduced on real WSL bash: typing `ls ./おはよう ./test1`, moving the cursor between `う` and the space, then pressing Space repeatedly grew a misplaced gap between `は` and `よ` instead of accumulating after `う`. Same misalignment silently corrupted any expansion that ran against a multi-byte buffer in any shell. Conversion is done once at the cmd/app boundary by `app::hook::shell_cursor_to_byte`, which dispatches to `char_cursor_to_byte` (bash/zsh/clink/nu) or `utf16_cursor_to_byte` (pwsh). The inverse `byte_cursor_to_char` / `byte_cursor_to_utf16` runs on the way out inside `app::hook::render`, which wraps the domain renderer so its per-shell `format!` arms can stay byte-cursor-only. zsh is excluded from the output conversion because its renderer emits LBUFFER/RBUFFER substrings produced by a byte `split_at` — the cursor number itself never crosses the shell boundary there. The pwsh path handles the surrogate-pair case (emoji = 1 char / 4 bytes / 2 UTF-16 code units), rounding down to the previous boundary on a mid-surrogate cursor so the downstream slice stays UTF-8 valid. Bake-mode bash dispatcher (0.1.17, Git Bash) is unaffected — it slices `${READLINE_LINE:0:READLINE_POINT}` in pure bash, which is char-based, so the bake path was correct from the start. Tidy First: the three near-identical "insert space at cursor" short-circuits in cmd::hook::handle (oversize line, paste pending, config load failure) collapse into one helper `app::hook::insert_space_action`. Same PR keeps the cleanup local to the bug fix. Tests: - 28 new unit tests in `app::hook` covering the four conversion helpers across ASCII / Japanese (3-byte BMP) / emoji (4-byte outside BMP / 2 UTF-16 code units) / combining marks / RTL / NUL / empty / past-end clamping / round-trip symmetry against inverses. - 7 new e2e tests in `tests/cli_integration.rs::hook_*` driving `runex hook --shell <s>` for each of bash / zsh / pwsh / clink / nu with a multi-byte buffer and asserting the shell-correct cursor unit comes back. bash arm is the reporter's PoC verbatim; pwsh arm exercises the emoji surrogate-pair path. - Manual verify on real WSL bash with 0.1.18: reporter's PoC no longer reproduces. Spaces now accumulate only between `う` and `./test1`, never between `は` and `よ`. Bump version 0.1.17 → 0.1.18 + Cargo.lock + CHANGELOG.
8 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
--cursorin their own native unit — bash / zsh / clink / nu count Unicode scalar values (= chars), pwsh counts UTF-16 code units (.NET / PSReadLine native). The Rust hook used the raw value as a byte offset, which agreed by accident on ASCII but split mid-character on Japanese, emoji, or any non-ASCII input.ls ./おはよう ./test1, moving the cursor betweenうand the space, pressing Space repeatedly grew a misplaced gap betweenはandよinstead of accumulating afterう. Same misalignment silently corrupted any expansion that ran against a multi-byte buffer.cmd::hookconverts the incoming cursor to a byte offset exactly once at the cmd/app boundary viaapp::hook::shell_cursor_to_byte(dispatcheschar_cursor_to_bytefor bash/zsh/clink/nu,utf16_cursor_to_bytefor pwsh).app::hook::renderwraps the domain renderer and converts back on the way out. zsh skips output conversion because its renderer splits LBUFFER/RBUFFER by byte and the cursor number never reaches zsh — converting it would be wrong.utf16_cursor_to_bytehandles surrogate pairs (🎯 = 1 char / 4 bytes / 2 UTF-16 code units). Mid-surrogate cursor rounds down to the previous boundary so the slice stays UTF-8 valid.${READLINE_LINE:0:READLINE_POINT}in pure bash, which is char-based, so the bake path was correct from the start.app::hook::insert_space_action(line, byte_cursor)helper.Test plan
app::hook— four conversion helpers × ASCII / Japanese (3-byte BMP) / emoji (4-byte / 2 UTF-16) / combining / RTL / NUL / empty / past-end clamping / round-trip property against inversestests/cli_integration.rs—runex hook --shell <s>for bash / zsh / pwsh / clink / nu against a multi-byte buffer, asserting the shell-correct cursor unit comes back. bash arm is the reporter's PoC verbatim. pwsh arm exercises the emoji surrogate-pair path.うand./test1, never betweenはandよ.Critical files
runex/src/app/hook.rs— four conversion helpers +shell_cursor_to_bytedispatcher +insert_space_actionhelper +renderwrapper + 28 unit testsrunex/src/cmd/hook.rs— entry conversion at the cmd/app boundary + 3 short-circuit collapse onto the new helperrunex/src/domain/hook.rs—HookActiondoc updated to point at the new app-layer conversion (no behavior change in the domain)runex/src/main.rs—--cursorCLI arg doc updated to spell out the per-shell input unitrunex/tests/cli_integration.rs— 7 multi-byte e2e tests forrunex hookCHANGELOG.md— 0.1.18 entry underFixed/Internal/Testsrunex/Cargo.toml— 0.1.17 → 0.1.18Closes #6.