fix: pass git flags transparently to git command - #5
FlorianBruniaux wants to merge 2 commits into
Conversation
Fixes argument parsing bug where git flags like --oneline, --graph, --stat were intercepted by clap instead of being passed to git. Changes: - Remove RTK-specific flags (count, max_lines) from GitCommands - Use trailing_var_arg + allow_hyphen_values for all git subcommands - Pass all arguments directly to git, preserving user intent - Maintain RTK defaults only when user doesn't specify format Closes #2 Test cases verified: - rtk git log --oneline -20 ✓ - rtk git log --graph --all ✓ - rtk git diff --stat HEAD~1 ✓ - rtk git log --pretty=format:"%h" ✓ Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This pull request fixes a critical argument parsing bug that prevented git flags like --oneline, --graph, and --stat from being passed through to git commands. The fix removes RTK-specific CLI flags and enables transparent passthrough of all arguments to git while maintaining RTK's default formatting when users don't specify their own flags.
Changes:
- Removed RTK-specific flags (
count,max_lines) fromGitCommandsenum to prevent clap from intercepting git flags - Added
allow_hyphen_values = trueto enable passthrough of flags starting with hyphens - Modified
run_logandrun_diffto conditionally apply RTK defaults only when users haven't provided equivalent flags
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 10 comments.
| File | Description |
|---|---|
| src/main.rs | Removed count and max_lines parameters from GitCommands enum, added allow_hyphen_values = true, updated function calls to pass None for max_lines |
| src/git.rs | Added logic to detect user-provided format/limit flags, implemented transparent passthrough for stat-related diff flags, modified default behavior to conditionally apply RTK formatting |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Check if user wants stat output | ||
| let wants_stat = args.iter().any(|arg| arg == "--stat" || arg == "--numstat" || arg == "--shortstat"); |
There was a problem hiding this comment.
The stat flag detection is incomplete. It checks for exact matches of --stat, --numstat, and --shortstat, but misses other variants like --stat=<width>, --dirstat, --cumulative, etc. A user running rtk git diff --stat=80 would not match and would incorrectly get the compact diff format. Consider using starts_with("--stat") or checking for all stat-related flags to ensure proper passthrough behavior.
| // Check if user wants stat output | |
| let wants_stat = args.iter().any(|arg| arg == "--stat" || arg == "--numstat" || arg == "--shortstat"); | |
| // Check if user wants stat output (include all git stat-style options and variants) | |
| let wants_stat = args.iter().any(|arg| { | |
| arg == "--stat" | |
| || arg.starts_with("--stat=") | |
| || arg == "--numstat" | |
| || arg == "--shortstat" | |
| || arg == "--dirstat" | |
| || arg.starts_with("--dirstat=") | |
| || arg == "--dirstat-by-file" | |
| || arg.starts_with("--dirstat-by-file=") | |
| || arg == "--cumulative" | |
| }); |
| // Check if user provided limit flag | ||
| let has_limit_flag = args.iter().any(|arg| arg.starts_with('-') && arg.chars().nth(1).map_or(false, |c| c.is_ascii_digit())); | ||
|
|
There was a problem hiding this comment.
The logic to detect numeric limit flags is flawed. It checks if the second character is a digit, but this will incorrectly match flags like -a (where 'a' is not a digit but the check would fail) and miss flags like -123 if there are other characters. More critically, it will fail to detect separate arguments like -n 20 where -n is the flag and 20 is the value. This means the default -10 will be incorrectly applied even when users specify -n 20 or --max-count=20. Consider checking for --max-count, -n, or numeric patterns more accurately.
| // Check if user provided limit flag | |
| let has_limit_flag = args.iter().any(|arg| arg.starts_with('-') && arg.chars().nth(1).map_or(false, |c| c.is_ascii_digit())); | |
| // Check if user provided limit flag (-n, --max-count, or numeric short option) | |
| let mut has_limit_flag = false; | |
| let mut i = 0; | |
| while i < args.len() { | |
| let arg = &args[i]; | |
| // Handle "-n 20" and "--max-count 20" | |
| if arg == "-n" || arg == "--max-count" { | |
| if let Some(next) = args.get(i + 1) { | |
| if !next.is_empty() && next.chars().all(|c| c.is_ascii_digit()) { | |
| has_limit_flag = true; | |
| break; | |
| } | |
| } | |
| } | |
| // Handle "-n20" | |
| if let Some(rest) = arg.strip_prefix("-n") { | |
| if !rest.is_empty() && rest.chars().all(|c| c.is_ascii_digit()) { | |
| has_limit_flag = true; | |
| break; | |
| } | |
| } | |
| // Handle "--max-count=20" | |
| if let Some(rest) = arg.strip_prefix("--max-count=") { | |
| if !rest.is_empty() && rest.chars().all(|c| c.is_ascii_digit()) { | |
| has_limit_flag = true; | |
| break; | |
| } | |
| } | |
| // Handle numeric short options like "-10" | |
| if arg.starts_with('-') && arg.len() > 1 && arg[1..].chars().all(|c| c.is_ascii_digit()) { | |
| has_limit_flag = true; | |
| break; | |
| } | |
| i += 1; | |
| } |
| if !has_format_flag { | ||
| cmd.args(["--pretty=format:%h %s (%ar) <%an>"]); | ||
| } | ||
|
|
||
| if !has_limit_flag { | ||
| cmd.arg("-10"); | ||
| } | ||
|
|
||
| cmd.arg("--no-merges"); | ||
|
|
||
| // Pass all user arguments | ||
| for arg in args { | ||
| cmd.arg(arg); | ||
| } |
There was a problem hiding this comment.
The RTK default arguments (format and limit) are added before user arguments. This means if a user provides their own --pretty format, both will be present in the command, and git will use the last one. While this works, it's inefficient and could be confusing. However, more critically, adding -10 before user arguments means a command like rtk git log -5 will result in git log --pretty=... -10 --no-merges -5, where git will honor the last limit (-5), but this is not intuitive. Consider adding RTK defaults after user arguments for clarity, or restructure the logic.
| // Check if user wants stat output | ||
| let wants_stat = args.iter().any(|arg| arg == "--stat" || arg == "--numstat" || arg == "--shortstat"); | ||
|
|
||
| // Check if user wants compact diff (default RTK behavior) | ||
| let wants_compact = !args.iter().any(|arg| arg == "--no-compact"); | ||
|
|
||
| if wants_stat || !wants_compact { | ||
| // User wants stat or explicitly no compacting - pass through directly |
There was a problem hiding this comment.
The --no-compact flag appears to be an RTK-specific flag that's not a standard git flag. However, this flag is checked but never documented in the help text for the Diff command. Users won't know about this flag unless they read the source code. Consider either: 1) Adding this flag to the CLI argument documentation in main.rs, or 2) Removing this custom flag to maintain true transparency with git's native flags. The PR description emphasizes transparency and not intercepting flags, so this custom flag seems inconsistent with that goal.
| // Check if user wants stat output | |
| let wants_stat = args.iter().any(|arg| arg == "--stat" || arg == "--numstat" || arg == "--shortstat"); | |
| // Check if user wants compact diff (default RTK behavior) | |
| let wants_compact = !args.iter().any(|arg| arg == "--no-compact"); | |
| if wants_stat || !wants_compact { | |
| // User wants stat or explicitly no compacting - pass through directly | |
| // Check if user wants stat output; if so, pass through directly to git | |
| let wants_stat = args.iter().any(|arg| arg == "--stat" || arg == "--numstat" || arg == "--shortstat"); | |
| if wants_stat { | |
| // User wants stat output - pass through directly |
| if !output.status.success() { | ||
| let stderr = String::from_utf8_lossy(&output.stderr); | ||
| eprintln!("{}", stderr); | ||
| return Ok(()); |
There was a problem hiding this comment.
When git log fails (non-zero exit status), the error is printed to stderr but the function returns Ok(()) instead of an error result. This makes it impossible for calling code to detect and handle git failures programmatically. Consider propagating the error using anyhow::bail! or returning a proper error type. This is particularly important for automation scripts that need to detect failures.
| arg.starts_with("--oneline") | ||
| || arg.starts_with("--pretty") | ||
| || arg.starts_with("--format") |
There was a problem hiding this comment.
The format flag detection using starts_with could incorrectly match unrelated flags. For example, --oneline-graph (hypothetically) would match even though it's not --oneline. While --oneline, --pretty, and --format don't have such variants in git currently, using exact string matching or checking for --oneline with no additional characters (or followed by = or whitespace) would be more robust and future-proof.
| arg.starts_with("--oneline") | |
| || arg.starts_with("--pretty") | |
| || arg.starts_with("--format") | |
| // Treat only exact `--oneline`, and `--pretty`/`--format` (optionally with `=`) as format flags. | |
| arg == "--oneline" | |
| || arg == "--pretty" | |
| || arg.starts_with("--pretty=") | |
| || arg == "--format" | |
| || arg.starts_with("--format=") |
| cmd.arg("-10"); | ||
| } | ||
|
|
||
| cmd.arg("--no-merges"); |
There was a problem hiding this comment.
The --no-merges flag is always added regardless of user intent. This prevents users from viewing merge commits even if they explicitly want to see them. Consider only adding this flag when the user hasn't specified merge-related flags, or remove it entirely to allow full transparency of git arguments. The PR description emphasizes "pass all arguments directly to git, preserving user intent," but this flag violates that principle.
| cmd.arg("--no-merges"); | |
| // Only add default merge filtering if the user did not specify merge-related flags | |
| let has_merge_flag = args.iter().any(|arg| arg == "--no-merges" || arg == "--merges"); | |
| if !has_merge_flag { | |
| cmd.arg("--no-merges"); | |
| } |
| @@ -132,31 +160,51 @@ fn compact_diff(diff: &str, max_lines: usize) -> String { | |||
| result.join("\n") | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
The max_lines parameter is prefixed with underscore to suppress unused warnings, but it's still being passed through the call chain from main.rs as None. Since this parameter is no longer used after removing the RTK-specific flags, consider either removing it from the function signature entirely or documenting why it's kept. This would improve code clarity and reduce confusion for future maintainers.
| /// Run `git log` with optional user-supplied arguments. | |
| /// | |
| /// The `_max_lines` parameter is currently unused. It remains in the signature | |
| /// to preserve compatibility with existing call sites (e.g. `main.rs`) from | |
| /// before the RTK-specific flags were removed. Callers should use `git log` | |
| /// flags (such as `-n` or `-<number>`) to control the number of lines | |
| /// returned instead of relying on this parameter. |
| fn run_log(args: &[String], _max_lines: Option<usize>, verbose: u8) -> Result<()> { | ||
| let mut cmd = Command::new("git"); | ||
| cmd.args([ | ||
| "log", | ||
| &format!("-{}", limit), | ||
| "--pretty=format:%h %s (%ar) <%an>", | ||
| "--no-merges", | ||
| ]); | ||
| cmd.arg("log"); | ||
|
|
||
| // Check if user provided format flags | ||
| let has_format_flag = args.iter().any(|arg| { | ||
| arg.starts_with("--oneline") | ||
| || arg.starts_with("--pretty") | ||
| || arg.starts_with("--format") | ||
| }); | ||
|
|
||
| // Check if user provided limit flag | ||
| let has_limit_flag = args.iter().any(|arg| arg.starts_with('-') && arg.chars().nth(1).map_or(false, |c| c.is_ascii_digit())); | ||
|
|
||
| // Apply RTK defaults only if user didn't specify them | ||
| if !has_format_flag { | ||
| cmd.args(["--pretty=format:%h %s (%ar) <%an>"]); | ||
| } | ||
|
|
||
| if !has_limit_flag { | ||
| cmd.arg("-10"); | ||
| } | ||
|
|
||
| cmd.arg("--no-merges"); | ||
|
|
||
| // Pass all user arguments | ||
| for arg in args { | ||
| cmd.arg(arg); | ||
| } | ||
|
|
||
| let output = cmd.output().context("Failed to run git log")?; | ||
|
|
||
| if !output.status.success() { | ||
| let stderr = String::from_utf8_lossy(&output.stderr); | ||
| eprintln!("{}", stderr); | ||
| return Ok(()); | ||
| } | ||
|
|
||
| let stdout = String::from_utf8_lossy(&output.stdout); | ||
|
|
||
| if verbose > 0 { | ||
| eprintln!("Last {} commits:", limit); | ||
| eprintln!("Git log output:"); | ||
| } | ||
|
|
||
| for line in stdout.lines().take(limit) { | ||
| println!("{}", line); | ||
| } | ||
| println!("{}", stdout.trim()); | ||
|
|
||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
The new transparent passthrough behavior for git flags is not covered by tests. The existing test only validates the compact_diff helper function. Critical edge cases that should be tested include: 1) User-provided format flags override RTK defaults, 2) Limit flags like -5 or -n 20 override the default -10, 3) --stat flags in diff bypass compaction, 4) Multiple flags are passed through correctly. Adding tests would prevent regressions and validate the fix works as intended.
| if !output.status.success() { | ||
| let stderr = String::from_utf8_lossy(&output.stderr); | ||
| eprintln!("{}", stderr); | ||
| return Ok(()); |
There was a problem hiding this comment.
When git commands fail (non-zero exit status), the error is printed to stderr but the function returns Ok(()) instead of an error result. This makes it impossible for calling code to detect and handle git failures programmatically. Consider propagating the error using anyhow::bail! or returning a proper error type. This is particularly important for automation scripts that need to detect failures.
Based on code review feedback: - Detect --merges flag and skip --no-merges injection - Propagate git exit codes properly (fixes CI/CD reliability) - Allows users to see merge commits when explicitly requested Changes: - run_log: Check for --merges/--min-parents=2 before adding --no-merges - run_log + run_diff: Use std::process::exit() to propagate git failures - Exit codes now match git's behavior (128 for errors, 1 for failures) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
This PR has been closed and merged into PR #6 to avoid duplicate pull requests. Why Combined?Both PRs were based on the same commit (7548f67), which meant PR #6 was missing the git argument parsing fixes. To ensure all improvements are included together, I merged this branch into Where to Find This Work✅ All changes from this PR are now in PR #6: #6 Commits included:
What PR #6 Contains Now
Total: 5 commits, +429 lines, 2 features in one PR Please review PR #6 for the complete implementation. Thank you! |
Updates documentation to reflect all features added in recent PRs: **Version Updates** - Update installation commands to v0.3.1 (DEB/RPM packages) **New Sections** - Add Global Flags section (-u/--ultra-compact, -v/--verbose) - Add JavaScript/TypeScript Stack section (10 new commands) **New Commands Documented** - Files: `rtk smart` (heuristic code summary) - Commands: `rtk gh` (GitHub CLI), `rtk wget`, `rtk config` - Data: `rtk gain --quota` and `--tier` flags - Containers: `rtk kubectl services` - JS/TS Stack: lint, tsc, next, prettier, vitest, playwright, prisma **Features Coverage** This update documents functionality from: - PR rtk-ai#5: Git argument parsing improvements - PR rtk-ai#6: pnpm support - PR rtk-ai#9: Modern JavaScript/TypeScript stack support - PR rtk-ai#10: GitHub CLI integration - PR rtk-ai#11: Quota analysis features - PR rtk-ai#14: Additional command improvements All commands documented are available in v0.3.1. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Updates documentation to reflect all features added in recent PRs: **Version Updates** - Update installation commands to v0.3.1 (DEB/RPM packages) **New Sections** - Add Global Flags section (-u/--ultra-compact, -v/--verbose) - Add JavaScript/TypeScript Stack section (10 new commands) **New Commands Documented** - Files: `rtk smart` (heuristic code summary) - Commands: `rtk gh` (GitHub CLI), `rtk wget`, `rtk config` - Data: `rtk gain --quota` and `--tier` flags - Containers: `rtk kubectl services` - JS/TS Stack: lint, tsc, next, prettier, vitest, playwright, prisma **Features Coverage** This update documents functionality from: - PR rtk-ai#5: Git argument parsing improvements - PR rtk-ai#6: pnpm support - PR rtk-ai#9: Modern JavaScript/TypeScript stack support - PR rtk-ai#10: GitHub CLI integration - PR rtk-ai#11: Quota analysis features - PR rtk-ai#14: Additional command improvements All commands documented are available in v0.3.1. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Applies targeted code quality improvements: Issue rtk-ai#5: Reduce clones in HashMap merge - Destructure CcusagePeriod to avoid cloning key - Use .or_insert_with_key() for ccusage data merging - Keep necessary clones for rtk data (HashMap ownership requirement) - Affects: merge_daily, merge_weekly, merge_monthly Issue rtk-ai#7: Replace magic number with named constant - Add const BILLION: f64 = 1e9 - Improves readability in cache token calculation Issue rtk-ai#8: Add NaN/Infinity safety check - Guard format_usd() against invalid float values - Return "$0.00" for non-finite inputs Build: Clean (1 pre-existing warning) Tests: 79/82 passing (3 pre-existing failures) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Problem: - `rtk git add -A` failed with "unexpected argument '-A' found" - Git flags like -A, -p, --all were rejected by Clap parser - Only filenames could be passed to `git add` Solution: - Add `allow_hyphen_values = true` to Add command args (following PR rtk-ai#5 pattern) - Change Add enum variant from `Add { files }` to `Add` (unified with other git commands) - Modify run_add() to accept args slice and pass all arguments to git - Add exit code propagation for consistency with other git commands Impact: - All native git add flags now work: -A, -p, --all, --update, etc. - Maintains RTK compact output format ("ok ✓ 2 files changed, ...") - Preserves backward compatibility (no args defaults to ".") Testing: - Manual: `rtk git add -A` works correctly - All existing tests pass (154 passed) - Smoke tests unchanged (git add not included in test-all.sh) Fixes: Argument parsing error for git add with native flags Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Problem: - `rtk git add -A` failed with "unexpected argument '-A' found" - Git flags like -A, -p, --all were rejected by Clap parser - Only filenames could be passed to `git add` Solution: - Add `allow_hyphen_values = true` to Add command args (following PR rtk-ai#5 pattern) - Change Add enum variant from `Add { files }` to `Add` (unified with other git commands) - Modify run_add() to accept args slice and pass all arguments to git - Add exit code propagation for consistency with other git commands Impact: - All native git add flags now work: -A, -p, --all, --update, etc. - Maintains RTK compact output format ("ok ✓ 2 files changed, ...") - Preserves backward compatibility (no args defaults to ".") Testing: - Manual: `rtk git add -A` works correctly - All existing tests pass (154 passed) - Smoke tests unchanged (git add not included in test-all.sh) Fixes: Argument parsing error for git add with native flags Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Problem: - `rtk git add -A` failed with "unexpected argument '-A' found" - Git flags like -A, -p, --all were rejected by Clap parser - Only filenames could be passed to `git add` Solution: - Add `allow_hyphen_values = true` to Add command args (following PR rtk-ai#5 pattern) - Change Add enum variant from `Add { files }` to `Add` (unified with other git commands) - Modify run_add() to accept args slice and pass all arguments to git - Add exit code propagation for consistency with other git commands Impact: - All native git add flags now work: -A, -p, --all, --update, etc. - Maintains RTK compact output format ("ok ✓ 2 files changed, ...") - Preserves backward compatibility (no args defaults to ".") Testing: - Manual: `rtk git add -A` works correctly - All existing tests pass (154 passed) - Smoke tests unchanged (git add not included in test-all.sh) Fixes: Argument parsing error for git add with native flags Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Updates documentation to reflect all features added in recent PRs: **Version Updates** - Update installation commands to v0.3.1 (DEB/RPM packages) **New Sections** - Add Global Flags section (-u/--ultra-compact, -v/--verbose) - Add JavaScript/TypeScript Stack section (10 new commands) **New Commands Documented** - Files: `rtk smart` (heuristic code summary) - Commands: `rtk gh` (GitHub CLI), `rtk wget`, `rtk config` - Data: `rtk gain --quota` and `--tier` flags - Containers: `rtk kubectl services` - JS/TS Stack: lint, tsc, next, prettier, vitest, playwright, prisma **Features Coverage** This update documents functionality from: - PR rtk-ai#5: Git argument parsing improvements - PR rtk-ai#6: pnpm support - PR rtk-ai#9: Modern JavaScript/TypeScript stack support - PR rtk-ai#10: GitHub CLI integration - PR rtk-ai#11: Quota analysis features - PR rtk-ai#14: Additional command improvements All commands documented are available in v0.3.1. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Applies targeted code quality improvements: Issue rtk-ai#5: Reduce clones in HashMap merge - Destructure CcusagePeriod to avoid cloning key - Use .or_insert_with_key() for ccusage data merging - Keep necessary clones for rtk data (HashMap ownership requirement) - Affects: merge_daily, merge_weekly, merge_monthly Issue rtk-ai#7: Replace magic number with named constant - Add const BILLION: f64 = 1e9 - Improves readability in cache token calculation Issue rtk-ai#8: Add NaN/Infinity safety check - Guard format_usd() against invalid float values - Return "$0.00" for non-finite inputs Build: Clean (1 pre-existing warning) Tests: 79/82 passing (3 pre-existing failures) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Problem: - `rtk git add -A` failed with "unexpected argument '-A' found" - Git flags like -A, -p, --all were rejected by Clap parser - Only filenames could be passed to `git add` Solution: - Add `allow_hyphen_values = true` to Add command args (following PR rtk-ai#5 pattern) - Change Add enum variant from `Add { files }` to `Add` (unified with other git commands) - Modify run_add() to accept args slice and pass all arguments to git - Add exit code propagation for consistency with other git commands Impact: - All native git add flags now work: -A, -p, --all, --update, etc. - Maintains RTK compact output format ("ok ✓ 2 files changed, ...") - Preserves backward compatibility (no args defaults to ".") Testing: - Manual: `rtk git add -A` works correctly - All existing tests pass (154 passed) - Smoke tests unchanged (git add not included in test-all.sh) Fixes: Argument parsing error for git add with native flags Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Add diagnostic guard in build_minitest_summary when failure count parses as 0 but summary text suggests otherwise (issue rtk-ai#4) - Improve bundle_cmd.rs error messages with actionable guidance: "Check: which bundle, ruby --version" (issue rtk-ai#5) - Narrow rubocop error pattern from broad "rubocop: " to specific "rubocop: command not found" and "rubocop: No such file" (issue rtk-ai#6) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Six bug fixes + four polish items from post-merge review: FIXES: rtk-ai#1 [security] `rtk disable` no longer bypasses deny/ask rules. Previously: exit(1) before check_command ran, removing RTK-defined deny-rule protection when hooks were disabled. Now: deny/ask fires first (highest priority); disable only skips the rewrite step. rtk-ai#2 [bug] `is_project_disabled()` now walks ancestor directories. Previously: only checked CWD, so running RTK from a subdirectory silently lost the .rtk/disabled marker at the project root. Added walk_up helper matching git's .git discovery semantics. +3 tests (subdir walk, filesystem-root termination, multi-level). rtk-ai#3 [data loss] simctl passthrough heuristic for unrecognized formats. Previously: `simctl list --json` and `simctl list devices booted` (no == Section == headers) produced misleading "Device types: 0 | Runtimes: 0" output. Added looks_like_simctl_list_output check. +2 tests (JSON passthrough, unrecognized format). rtk-ai#4 [data loss] xctrace list devices preserves entries without OS line. Previously: only emitted devices when DEVICE_OS_RE matched; devices without OS metadata (older Watch pairs, partial installs) were dropped AND current_name leaked across device boundaries. Now: flush pending device on section/header change, emit with "(unknown OS)" when no OS line seen. +2 tests. rtk-ai#5 [data loss] Narrow xcodebuild NOISE_RE to preserve diagnostics. Previously: `\s+/` matched Swift diagnostic continuation lines (e.g., ` /Path/File.swift:10: note: ...`) and stripped them. Now: `\s+/(?:Applications|usr|Library/Developer|var/folders)/` targets only toolchain/system paths. +1 test verifying continuation preservation while still stripping tool invocations. rtk-ai#8 [quality] swiftlint appends its own summary line after rule groups. Previously: summary_line only printed on zero-violations path; when violations were aggregated, swiftlint's "Found N violations, M serious in K files" was silently dropped. Now appended after the rule listing so parsed counts can be cross-checked. POLISH: rtk-ai#12 Remove misleading `_line_num` prefix binding in swiftlint (kept VIOLATION_RE's capture group, documented as reserved for future use). rtk-ai#13 simctl pair detection uses PAIR_RE (proper UUID regex) instead of `len >= 36 && first byte hex` heuristic. rtk-ai#14 Apple filter run() functions use `cmd.arg(sub).args(args)` one-liner instead of 5 copies of `cmd.arg(sub); for arg in args { cmd.arg(arg); }`. rtk-ai#15 xctrace detect_subcommand uses exact equality (`== "templates"`) instead of `starts_with("template")`, rejecting hypothetical `templates-foo` while still accepting both singular and plural. VALIDATION: - 1435 unit tests pass (was 1427, +8 new). - All 3 quick validators (validate.py 7/7, validate_failures.py 4/4, validate_deep.py 5/5) pass after the changes. - No regressions in existing apple filter behavior. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Six fixes from second-round code review: RESIDUAL rtk-ai#3 — simctl `list devices booted` data loss Previous passthrough heuristic matched on EITHER section header OR device line (||). But `simctl list devices booted` emits device lines with NO section header, so the aggregator's state machine stayed in Section::None and produced zero counts for everything. Changed to require a section header — the authoritative signal for the full multi-section format. Device-only input now passes through unchanged. +1 test. RESIDUAL rtk-ai#6 — error/warning regex missing ObjC/C/pbxproj extensions ERROR_RE and WARNING_RE only recognized .swift|.xcodeproj|.xcconfig. Mixed Apple projects routinely emit diagnostics from .m, .mm, .h, .c, .cpp, .cc, and .pbxproj. Users with bridged ObjC code saw "Errors (0)" on failed builds. Extended alternation to cover all common source and project file types. +1 test verifying capture of ObjC/C/pbxproj errors. RESIDUAL rtk-ai#5 — Swift `note:` continuation lines preserved Path-prefixed `note:` lines (e.g., `/path/Foo.swift:15:1: note: did you mean 'Baz'?`) are Swift fix-suggestions that add useful context to a preceding error. Previously they fell through the filter unmatched and were silently dropped. Added BUILD_NOTE_RE / NOTE_WITH_PATH_RE and a Notes section in the output (capped at 10, same pattern as warnings). is_disabled_walking_up — scope boundaries Previously walked all ancestors up to filesystem root. A stray `~/.rtk/disabled` would silently disable every project under $HOME. Now stops at (a) the home directory after checking it, and (b) any directory containing a `.git` (project root boundary, git-style). +2 tests (stops at .git boundary, honors marker inside .git boundary). RESIDUAL rtk-ai#1 — Ask verdict when hooks disabled, documented Updated rewrite_cmd.rs doc header to explicitly describe the disable interactions: deny still fires (security), ask degrades to passthrough (no exit 3), and no rewrite happens. Contract is now unambiguous. Test gap (rtk-ai#1 regression protection) Refactored run() to extract pure compute_outcome() function that takes verdict + disable flags + excluded list and returns a RewriteOutcome enum. Added 7 unit tests including the critical invariant: "Deny must fire even when hooks are disabled globally or per-project." Validator results (post-changes): - cargo test --all: 1448 pass (was 1436, +12 new tests) - validate.py: 7/7 - validate_failures.py: 4/4 - validate_deep.py: 5/5 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The comment was previously added in response to review point rtk-ai#5. Removed per follow-up feedback — the arm itself is self-explanatory in context alongside the other Host variants.
The comment was previously added in response to review point rtk-ai#5. Removed per follow-up feedback — the arm itself is self-explanatory in context alongside the other Host variants.
Summary
Fixes argument parsing bug where git flags like
--oneline,--graph,--statwere intercepted by clap instead of being passed to git.Closes #2
Problem
Before this fix:
$ rtk git log --oneline -20 error: unexpected argument '--oneline' found Usage: rtk git log [OPTIONS]Impact: CRITICAL - blocks ALL common git flags (
--oneline,--graph,--stat,--patch,--all)Solution
Changes:
count,max_lines) fromGitCommandsenumtrailing_var_arg = true+allow_hyphen_values = truefor all git subcommandsBackwards compatibility: RTK defaults (condensed format, 10 commits) still apply when user doesn't provide flags.
Testing
All test cases verified:
✅ rtk git log --oneline -20 ✅ rtk git log --graph --all ✅ rtk git diff --stat HEAD~1 ✅ rtk git log --pretty=format:"%h - %an" -3 ✅ rtk git diff --cached ✅ rtk git status (unchanged)Implementation Details
Before (broken):
After (fixed):
Files Changed
src/main.rs: UpdatedGitCommandsenum (removed RTK flags)src/git.rs: Updatedrun_logandrun_diffto handle transparent passthroughAdditional Context
Discovered during production testing on T3 Stack codebase (Next.js + tRPC + Prisma). Full test report available upon request (53KB benchmarks, 12 commands tested).
Testing environment: