shell: name the rejected flag in the builtins' illegal option errors - #39230
Open
robobun wants to merge 5 commits into
Open
shell: name the rejected flag in the builtins' illegal option errors#39230robobun wants to merge 5 commits into
robobun wants to merge 5 commits into
Claude / Claude Code Review
completed
Aug 15, 2026 in 13m 5s
Code review found 1 potential issue
Found 1 candidates, confirmed 1. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 0 |
| 🟡 Nit | 0 |
| 🟣 Pre-existing | 1 |
| Severity | File:Line | Issue |
|---|---|---|
| 🟣 Pre-existing | src/runtime/shell/builtin/cp.rs:811-812 |
cp's parse_short returns Some(ContinueParsing) for R/v/n, silently dropping the rest of a flag cluster (pre-existing) |
Annotations
Check notice on line 812 in src/runtime/shell/builtin/cp.rs
claude / Claude Code Review
cp's parse_short returns Some(ContinueParsing) for R/v/n, silently dropping the rest of a flag cluster (pre-existing)
Pre-existing (not from this PR): the `b'R'`, `b'v'`, and `b'n'` arms just above return `Some(ParseFlagResult::ContinueParsing)` instead of `None`, so `parse_one_flag` exits the cluster loop early and silently drops the rest of the flags — `cp -Rv` never turns on verbose, and `cp -Rz` doesn't reject `z`. This is why the new test's cp row only covers `["-zR", "z"]` and omits the `["-Rz", "z"]` case that mkdir/ls/rm/mv all cover. Fix: return `None` for these three arms (as mkdir does for `p`/`v`).
Loading