Repository navigation
feat(git): add ls-files and tag listing subcommands - #1927
AkhileshManda wants to merge 1 commit into
Conversation
a1e6dd3 to
e173f86
Compare
|
Hey @pszymkowiak following up on this PR, happy to address any comments if needed |
|
This pull request has been automatically marked as stale due to 90 days of inactivity. If this pull request is still relevant, please leave any comment (for example, "bump"), and we'll keep it open. Your contribution is very much appreciated — we're sorry we haven't been able to review it yet. |
|
bump |
|
#2859 (merged 2026-07-08) added |
Narrowed PR to read-only subcommands: - ls-files: Collapses paths into a directory-grouped tree. --depth N (default 1) controls grouping granularity. Passes through on -z, --format, -v, or --debug. - tag (listing form only): Bare git tag or with -l/--list enriches output with creation dates and annotated-tag messages. Write ops and non-listing forms (create, delete, custom --format) pass through directly. Flag checks move onto the shared argument tokenizer from arg_tokenizer.rs. Hook rewrite in rules.rs only matches git tag in its listing form.
e173f86 to
2f2f37f
Compare
|
Rebased onto current
|
KuSh
left a comment
There was a problem hiding this comment.
Round 1 — CHANGES REQUESTED
Claim: adds rtk git ls-files (tracked paths shown as per-directory counts, --depth N) and rtk git tag listing (dates and annotated messages, newest 20), and the hook rewrites git ls-files and the listing forms of git tag.
Scope: accept if narrowed to "rtk git ls-files with an output that still contains every path, rewritten only where its output is not piped". The git tag handler leaves this PR; its evidence goes on #1526 (round 1, frozen). Two other open PRs touch the same commands: #2943 groups ls-files into the same counts and is closed as a duplicate of this one, and #1816 is asked to narrow to git ls-tree, so ls-files is this PR's. #2940 asks for the same handler.
Ran: native git vs develop @ffa565134 vs head @2f2f37fd3 vs head merged onto develop @7bff768c4, every row through rtk rewrite first and then the form it returns; rtk's own repo (798 tracked files, 424 tags) and a 10-file repo with a non-ASCII path, a path with a space, annotated, lightweight and empty-message tags, and a tag named we|ird; 34 + 10 + 8 rows; invariants: non-default targets (single-path lookups, -m, -o, pathspecs, git -C), over-cap + accounting (424 tags = 20 + "404 more"), under the hook (pipes into head, tail, cat, grep, wc, xargs), every decision; savings: git ls-files 96% on the full listing and −773% on a one-path lookup; git tag 93% on 424 tags and −187% on 6 tags, vs the 20% floor (CONTRIBUTING.md)
Pins: 35 decisions, 35 mutated, 24 survived the suite (run_ls_files 7, run_tag 14, the tag savings entry and the registry's second-capture fallback 2; one more ls-files survivor is an equivalent mutant, since the tokenizer already types a -z after -- as a positional)
Matrix: never — running now; pin check: never — running now
CI @2f2f37fd3: never built (the CI run is action_required, and the branch conflicts with develop). Local gate on the head: cargo fmt --all --check fails (17 hunks in git_cmd.rs), cargo clippy --all-targets fails (collapsible_if at git_cmd.rs:3644), cargo test passes (4089). On the merge with develop the PR does not compile until GitCommand::git_name (added by #4348) gets LsFiles and Tag arms.
Blocks merge (3, frozen at round 1)
src/cmds/git/git_cmd.rs:3752— the path list is replaced by directory counts. Everyls-filesquestion whose answer is a path now gets a count with no path in it, and no hint of how to get the paths back.git ls-files Cargo.toml(is it tracked?) returns. (1 files)plus a total line: 96 B instead of 11 B, and the name is gone.git ls-files -mwithsrc/main.rsmodified returnssrc/ (1 files).git ls-files '*.toml'turns 85 paths into 9 counts, andgit ls-files src/filtersturns 63 paths into one line,src/ (63 files). The same happens to a pipe consumer, because the git rule isProducerOnly:git ls-files | head -5is rewritten tortk git ls-files | head -5and returns 5 counts, andgit ls-files | catreturns 19 lines instead of 798. A grouping that keeps the names clears the floor: one line per directory with its file names (src/cmds/git/ diff_cmd.rs gh_cmd.rs …) is 16,158 B against 26,211 B on this repo's 798 paths (38%), and 41% on'*.toml'. The rows a fix has to pass: every path ofgit ls-filesin a real repo can be rebuilt from the output, including names with a space and C-quoted ones; a one-path lookup is byte-identical to git;git ls-files | head -5andgit ls-files | catare byte-identical to git; the savings test runs on a captured real listing (#2943 carries one,tests/fixtures/git_ls_files_raw.txtby @AnayGarodia, which can be reused with credit). #4086 asks for the same kind of folding inrtk find.src/cmds/git/git_cmd.rs:3685— output that is not a bare path list is still grouped. The passthrough is an allowlist of four flags, so every other mode gets grouped as if its lines were paths.git ls-files --stagebecomes 775 lines like100644 9a99…10 0 tests/ (2 files).-tkeeps the status letters as part of the prefix (H src/),--eolgroups oni/lf, and-u,-fand--resolve-undohave the same shape. Two more cases reach the grouping.--abbrevis declared as taking a separate value, but git only accepts--abbrev[=<n>], so in-s --abbrev -zthe-zis read as abbrev's value and the NUL-separated output gets grouped. And a C-quoted path ("src/\303\251.rs") opens a separate"src/bucket.--depth Nshows a file that sits exactly N levels deep as a directory (--depth 2printssrc/lib.rs/ (1 files)), and--depth 0prints/ (10 files). Turning it around fixes the whole class: group only the plain listing modes (-c,-o,-m,-d,-i,-k,--exclude*, pathspecs) and pass everything else through. The test needs one row per mode above, each byte-identical to git.src/cmds/git/git_cmd.rs:3697— unpinned behaviour inrun_ls_files. The tests call the helpers, neverrun_ls_files, so none of its own decisions is pinned: skipping the passthrough branch (if passthrough→if false && passthrough), forcing the passthrough's exit code to 0, changing the default depth from 1 to 2, sorting by name instead of count, returning 0 whengit ls-filesfails, emptying the(no tracked files)message, and dropping the total line each pass all 4089 tests. Whatever replaces the grouping in item 1 needs tests that call the code path the hook reaches, on capturedgit ls-filesoutput, and each mutation above has to turn one of them red.
Filed as follow-ups
- #1526 —
git taglisting, cut by the narrowed claim; the evidence below goes there as a comment. The filter saves bytes only by hiding tags: below 20 tags it is larger than git (22 B → 109 B, and-l 'v0.5*'45 B → 129 B), and above 20 it keeps 20. The hint it prints,git tag -l '<pattern>', is rewritten back into the filter and capped again (-l 'v0*': 88 → 20), so the full list cannot be reached under the hook. It also changes git's answer:-l -n3drops every annotation line (1,253 → 21 lines, no notice); a tag that shares a name with a branch prints astags/v1.0(%(refname:short), where git useslstrip=2); the forced--sort=-version:refnameoverridestag.sort; a tag namedwe|irdprints aswe ird;git tag | tail -1returns the "… and 404 more" line. And for write forms:rtk git tag -- v3creates the tag and prints(no tags)(positionals are only counted before--), andrtk git tag -a v3.0runs the editor with stdout on a pipe, where develop's passthrough gives it the terminal. Its test,test_tag_listing_detection, re-checks tokenizer output and never callsrun_tag, so 14 mutations ofrun_tagpass all 4089 tests, among them the write-flag set (-d,--delete),-l, the positional rule, the--formatpassthrough, the cap and the exit codes.
Checked and correct — no need to re-verify
-z,-v,--format,--debugpass through byte-identical (-z: 26,211 B on both sides), and a-zafter--is not taken as the flag.--error-unmatch nonexistent.rskeeps git's exit 1 and its stderr.- The hook leaves
git tag v9.9.9,git tag -d …andgit tag -a … -m …unrewritten, and the write forms keep git's exit codes (1 for a missing tag, 128 for an existing one). git -C src ls-fileskeeps the global option.- The hook already leaves
$(git ls-files),git ls-files > files.txtandgit ls-files | while read funrewritten; only| head,| tailand| catreach the handler. - On the merge with develop, once the
git_namearms are added,rtk git ls-files -hhands git's usage through (exit 129, 38 lines, identical).
Your questions
None open since the round-0 request.
Next
Rebase onto develop (the CLI moved from src/main.rs to src/lib.rs, and GitCommand::git_name and its git_name_round_trips_every_subcommand test need the new arm), take git tag out, and fix items 1–3 with their rows. Then cargo fmt --all and cargo clippy --all-targets clean. After that, this merges.
Rounds: 1. Threads: 3 open (waiting on the author), 0 resolved this round.
| return Ok(0); | ||
| } | ||
|
|
||
| let grouped = group_by_depth(&files, depth); |
There was a problem hiding this comment.
Blocker 1: every path here becomes a directory count, including under | head and | cat, where the git rule rewrites the producer. The rows a fix has to pass are in the review body.
| sorted | ||
| } | ||
|
|
||
| fn ls_files_is_passthrough(tokens: &[Token<'_>]) -> bool { |
There was a problem hiding this comment.
Blocker 2: the passthrough is a four-flag allowlist, so --stage, -t, --eol, -u, -f, --resolve-undo and -s --abbrev -z are still grouped as if their lines were paths. Grouping only the plain listing modes and passing the rest through closes the class; rows in the review body.
| /// | ||
| /// Passes through unchanged when `-z`, `--format`, `-v`, or `--debug` are present | ||
| /// (scripting / custom-format invocations where transformation would break caller). | ||
| fn run_ls_files(args: &[String], verbose: u8, global_args: &[String]) -> Result<i32> { |
There was a problem hiding this comment.
Blocker 3: no test calls run_ls_files, and seven mutations of its own decisions pass all 4089 tests (listed in the review body). Each needs a test on captured git ls-files output that it turns red.
Summary
Narrowed this PR to the read-only git subcommands per review feedback from @KuSh:
git ls-files: Collapses tracked file paths into a directory-grouped tree.--depth N(default 1) controls grouping granularity. Passes through verbatim on-z,--format,-v, or--debug.git tag(listing form only): Enriches tag listing with creation dates and annotated-tag messages. Write operations (-a,-s,-d,-f,-v, etc.) and tag creation/deletion pass through unmodified to git. The hook rewrite inrules.rsmatches only the listing form (git tag,git tag -l,git tag --list) so tag creation and deletion stay unrewritten.The write subcommands (
rm,cherry-pick,rebase,merge) have been removed from this PR and can return in a separate PR with guards. Checkout passthrough has also been removed since #2859 added nativegit checkoutsupport.Updates & Improvements
upstream/develop.src/cmds/git/git_cmd.rs.-z,--format,-v,--debug, and tag listing/write flags) ported to the shared argument tokenizer from feat(core): add arg_tokenizer and migrate git/grep/dotnet/golangci arg parsing onto it #3681 (src/core/arg_tokenizer.rs).group_by_depth,parse_depth_arg,ls_files_is_passthrough, and tag listing/write detection.rtk git tagwhile tag creation and deletion stay unrewritten.