fix: preserve completion cursor boundaries - #529
codeforester merged 2 commits into
Conversation
| printf '%s\n' ' if ((cursor < word_count)); then' | ||
| printf '%s\n' ' cli_words=( "${completion_words[@]:1:cursor}" )' | ||
| printf '%s\n' ' else' | ||
| printf '%s\n' ' if ((cursor >= word_count)); then' |
There was a problem hiding this comment.
Altitude — the code edit is a behavior no-op; it does not deliver the hardening the issue asked for.
Swapping if ((cursor < word_count)) ... else ... to if ((cursor >= word_count)) ... else ... (with the two branch bodies swapped to match) produces byte-for-byte the same runtime behavior as before, for every value of cursor/word_count — cursor >= word_count is the exact negation of cursor < word_count, so branch A and branch B still fire under exactly the same conditions as on main.
Verified empirically: checking out this PR's new lib_cli.bats tests against the pre-PR lib_cli.sh (i.e. reverting only the .sh change, keeping the new regression tests) still passes all 40 tests, including the new cursor-boundary and set -u cases. That confirms the .sh change carries zero behavioral difference — the only thing this PR actually adds is test coverage.
Issue #527 explicitly asks for "an implementation shape that cannot lose the empty current word during future completion changes," but the new shape has the identical risk surface as the old one (a future edit can drop the cli_words+=("") line just as easily in either ordering). The tests are valuable and should be kept, but the .sh hunk doesn't structurally reduce the regression risk it claims to close — consider a comment pinning the invariant next to line 1625/1629, or extracting the clamp logic so it can be exercised directly instead of only through the printf-generated string.
There was a problem hiding this comment.
Addressed in e255cc4. The generated adapter now slices the words once and appends the empty current word under an explicit cursor-at/end invariant, with the invariant documented beside the append. The focused 40-test CLI suite and the full 683-test repository gate pass.
Summary
set -u.Issue
Fixes #527
Validation
bats lib/bash/cli/tests/lib_cli.bats(40 tests passed)../tests/validate.sh(682 tests passed; artifact, release, concurrency, and quality contracts passed).shellcheck --shell=bash --severity=warning lib/bash/cli/lib_cli.sh lib/bash/cli/tests/lib_cli.batsbash -n lib/bash/cli/lib_cli.shgit diff --checkAPI Impact
No command declaration, parser, handler, or option-resolution semantics changed. This preserves the documented nested-command completion behavior at the cursor boundary and makes the invariant regression-tested.