fix/ls 1 flag 2058#2061
Open
pen-pal wants to merge 3 commits into
Open
Conversation
Pure refactor with no behavior change — pulls the flag-merging and path handling into a standalone function so it can be unit-tested without spawning an `ls` subprocess. The forced `-la` enforcement and `-l`/`-a`/ `-h` stripping move with it. Existing tests pass unchanged. Prepares ground for rtk-ai#2058 fix where the strip list needs to grow to cover display-format flags that override `-l`. Signed-off-by: penpal <unameme@proton.me>
`rtk ls -1 path/` returned `(empty)` (or raw filenames on the fallback path) because BSD `/bin/ls` honors the last format flag — `ls -la -1` produces filename-only output the parser can't decode. Strip `-1`, `-C`, `-m`, `-x` from short-flag combinations alongside the already-enforced `-l`/`-a`/`-h`. RTK's compact output is already one-per-line, so users get the layout they asked for. Compound flags like `-1r` preserve `-r`. Includes the rtk-ai#2058 regression test on the primary failure case. Broader coverage (placement variations, sibling flags, compound splits, mixed groups, repetition) follows in a separate test commit. Signed-off-by: penpal <unameme@proton.me>
Adds 14 more unit tests around the fix from the previous commit so future refactors can't silently regress it: - placement variations: `-1` bare, before path, after path, no path - sibling format flags: `-C`, `-m`, `-x` - harmless flag preservation: `-r`, `-t`, `-S`, `--reverse` - compound splits: `-1r` → `-r`, `-lar` → `-r` - multiple format flags combined: `-1C`, `-1Cmx` - format flags mixed with -la: `-la1`, `-1la`, `-l1a` - compound with several kept chars: `-1rt`, `-r1t` - repeated format flag across args: `[-1 -1 -r path/]` - end-to-end safety net: BSD filename-only output through compact_ls The repetition test in particular guards against someone refactoring the per-arg char filter into a global one-shot strip, which would pass the simple cases but lose state across args. Signed-off-by: penpal <unameme@proton.me>
|
|
YOMXXX
approved these changes
May 24, 2026
Contributor
YOMXXX
left a comment
YOMXXX
left a comment
There was a problem hiding this comment.
Reviewed the ls -1 fix. The build_ls_args extraction is clean, the short-format flags that override -l on BSD are stripped before execution, and the regression coverage around -1, compound flags, and repeated format flags is solid.
One non-blocking follow-up: GNU ls --format=single-column, --format=commas, etc. can express the same class of display-format override through a long option and still pass through today. That can be tracked separately; this PR fixes the reported short-flag regression.
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
3 participants
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
rtk ls -1 path/Always Return(empty)#2058:rtk ls -1 path/returned(empty)on BSD/bin/lsbecausels -la -1honors the last format flag, producing filename-only output the parser can't decode.-1,-C,-m,-x) alongside the already-enforced-l/-a/-hbefore invoking ls. RTK's compact output is one-per-line anyway, so users get the layout they asked for.build_ls_args), fix (+ primary regression test), test (14 more edge cases covering placement, sibling flags, compound splits, mixed groups, repetition).Test plan
cargo fmt --all && cargo clippy --all-targets && cargo test— 1943 pass, 0 fail, clippy cleanPATH=/bin:/usr/bin rtk ls -1 /tmp/testdir/returns compact output (a.txt 0B...), not(empty)or raw filenames