Repository navigation
fix(git): don't misdetect a value-taking option's argument as a patch flag - #3575
Merged
Merged
Conversation
… flag requests_raw_log_output() matched bare -p/-u/--patch tokens anywhere in the args, without checking whether the previous token was an option like --grep or -S that consumes the next token as its value. `git log --grep -p` searches commit messages for the literal string "-p" (verified against real git 2.53.0: no diff output, identical to --grep=-p) but RTK treated it as a patch request and skipped its own filtering/limit, dumping raw uncapped git log output for what is actually a plain grep search. Skip the value token after any known value-taking git log/diff option before checking for the patch flags.
aeppling
requested changes
Aug 19, 2026
These options only accept an attached value (-U3, --unified=3, --expand-tabs=4) — verified against git 2.53.0, where `git log --expand-tabs 4` fails with "fatal: ambiguous argument '4'" instead of treating 4 as the value. consumes_next_token_as_value() was swallowing the next token for them anyway, so a real -p right after one of these was misread as their value and the raw patch request went undetected. Addresses review feedback from @aeppling on #3575. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Audited every option in consumes_next_token_as_value() against real git 2.53.0 behavior (git log <opt> <token> -1, checking whether the token gets swallowed as the option's value or leaks through as a positional arg). --max-parents and --min-parents behave like -U: `git log --max-parents 2` fails with "ambiguous argument '2'", so a real -p right after them was being misread as their value. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Cross-checked git-log(1)'s full option list against consumes_next_token_as_value() (using git log <opt> <token> -1 against real git 2.53.0 to see whether <token> gets swallowed as the option's value or leaks through as a positional arg). --diff-algorithm and --diff-filter both take a required, separate-token value (`git log --diff-algorithm -p` rejects -p as an invalid algorithm rather than treating it as the patch flag) but were missing from the list, so a genuine -p right after either was misdetected as the option's own value and the raw-patch request went undetected. Every other bracket/optional-value option checked (--format, --pretty, --stat, -M, -C, -B, etc.) is correctly attached-value-only and stays out of the list. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
KuSh
force-pushed
the
fix/git-log-grep-value-misdetected-as-patch-flag
branch
from
August 19, 2026 22:08
c5a0e9d to
84169e2
Compare
Collaborator
Author
|
@aeppling removed the three problematic args and added a test to make sure they don’t get added back. Two more problematic args were removed as well: |
…values as flags has_limit_flag, has_format_flag, wants_merges, and parse_user_limit scanned args positionally with no awareness that a value belonging to --grep, --author, etc. can itself look like a flag (-5, --pretty, --merges), reproducing the same misdetection class this branch already fixed for -p. Unify these into a single log_arg_tokens tokenizer shared by requests_raw_log_output, real_flag_args, and parse_user_limit, which also stops at the -- pathspec separator so a literal path like -5 after -- isn't misread as a flag.
Collaborator
Author
|
The same issue was also present in |
…rough requests_raw_log_output only recognized -p/-u/--patch variants as needing the raw path. --stat, --numstat, --name-only, --name-status, --raw, --shortstat, --dirstat, and --summary change git's raw output shape the same way -p does, but weren't listed — RTK's injected --pretty=format + ---END--- markers can't coexist with them, so the diffstat/name-list block got misparsed as the start of the next commit, mangling output silently instead of just leaving it unfiltered. Also share one log_arg_tokens() pass across run_log's flag/limit checks instead of retokenizing per check, and drop a redundant "-n" match arm already covered by consumes_next_token_as_value.
Every other run_* function that inspects args for "--" (run_diff, run_checkout) calls args_utils::restore_double_dash() first to re-insert the "--" that clap's trailing_var_arg strips out. run_log never did, so requests_raw_log_output()/log_arg_tokens()'s own take_while(|arg| *arg != "--") check was dead code in production: by the time run_log saw args, the literal "--" was already gone. Concretely, `rtk git log -- -p` lost its "--" and -p (a file literally named -p, a valid pathspec) was misdetected as the real patch flag, routing to raw passthrough instead of RTK's filtered log. Added an end-to-end regression test (real rtk binary, real git repo) alongside the existing git_log_patch_output_matches_raw_git test, since restore_double_dash reads live process args and can't be exercised by calling run_log's internals directly. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
|
@KuSh LGTM :) |
aeppling
approved these changes
Aug 20, 2026
Merged
This was referenced Aug 27, 2026
social4hyq
pushed a commit
to social4hyq/homebrew-core
that referenced
this pull request
Sep 20, 2026
rtk 0.46.0 Created-by: HarmonybrewBot Commit-by: HarmonybrewBot Merged-by: HarmonybrewBot Description: Created by `brew bump` --- Created with `brew bump-formula-pr`.<details> <summary>release notes</summary> <pre>## [0.46.0](rtk-ai/rtk@v0.45.0...v0.46.0) (2026-08-26) ### Features - find: dispatch on find's grammar; compress find output for unmodeled predicates ([#3603](rtk-ai/rtk#3603)) - find: tee tail hint when rtk imposes the result cap ([#3603](rtk-ai/rtk#3603)) ### Bug Fixes - find: never-worse guard, recovery hint, and dispatch on find's grammar ([#3603](rtk-ai/rtk#3603)) - git: don't misdetect a value-taking option's argument as a patch flag ([#3575](rtk-ai/rtk#3575)) - cicd: stop benchmark.sh deleting the tracked scripts/benchmark harness ([#3595](rtk-ai/rtk#3595)) - tee: hash long recovery-file slugs to prevent collisions and shorten hints ([#3266](rtk-ai/rtk#3266)) - benchmark: avoid negative curl/cargo cases that fail the benchmark job ([#3430](rtk-ai/rtk#3430)) - test: accept both Ask and Allow verdicts in rewrite tests ([#3147](rtk-ai/rtk#3147)) — Closes [#3146](rtk-ai/rtk#3146) - core: decode process output using Windows console code page ([#2717](rtk-ai/rtk#2717)) — Closes [#2452](rtk-ai/rtk#2452) - git: preserve patch output from log commands ([#2951](rtk-ai/rtk#2951)) — Closes [#2944](rtk-ai/rtk#2944) - discover: sanitize drive-letter colon so Windows discover finds sessions ([#2952](rtk-ai/rtk#2952)) — Closes [#2919](rtk-ai/rtk#2919) - stream: decode lossily instead of dropping lines on invalid UTF-8 ([#2997](rtk-ai/rtk#2997)) — Closes [#2994](rtk-ai/rtk#2994) ### Other - test(find): use the platform temp dir instead of /tmp ([#3717](https://github.com/rtk-ai/rtk/pull/3717))</pre> <p>View the full release notes at <a href="https://github.com/rtk-ai/rtk/releases/tag/v0.46.0">https://github.com/rtk-ai/rtk/releases/tag/v0.46.0</a>.</p> </details> <hr> See merge request: Harmonybrew/homebrew-core!17826
4 tasks done
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
2 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
Follow-up on #2951.
requests_raw_log_output()matched bare-p/-u/--patchtokens anywhere in the args, without checking whether the previous token was an option like--grepor-Sthat consumes the next token as its value.git log --grep -psearches commit messages for the literal string-p(verified against git 2.53.0: no diff output, same as--grep=-p), but RTK treated it as a patch request and skipped its own filtering/limit, dumping raw uncappedgit logoutput for what is actually a plain grep search.Skip the value token after any known value-taking
git log/git diffoption before checking for the patch flags.Test plan
cargo fmt --all && cargo clippy --all-targets && cargo testrtk git log --grep -poutput inspected