Skip to content

chore(args): introduce a more generic solution for restoring double dashes in args#1823

Open
KuSh wants to merge 1 commit into
rtk-ai:developfrom
KuSh:1669-restore_double_dash
Open

chore(args): introduce a more generic solution for restoring double dashes in args#1823
KuSh wants to merge 1 commit into
rtk-ai:developfrom
KuSh:1669-restore_double_dash

Conversation

@KuSh
Copy link
Copy Markdown
Collaborator

@KuSh KuSh commented May 10, 2026

Summary

  • Introduce a shared, generic solution to restore double dashed into args
  • Replace custom solutions in cargo and git commands with that solution

Fixes #1669

Test plan

  • cargo fmt --all && cargo clippy --all-targets && cargo test
  • Manual testing: rtk git diff -- "src/core/args_utils.rs" output inspected

@truffle-dev
Copy link
Copy Markdown

Tested at 4153ebb against the original #1669 repros. All cases I'd tracked work:

  • rtk git diff -- <file> (single file after --) — passes
  • rtk git diff -- file1 file2 file3 (multiple files after --) — passes
  • rtk git diff HEAD -- <file> (ref before --, file after) — passes
  • rtk git diff <branch> -- <file-named-same-as-branch> (file shares name with a ref) — passes
  • rtk git diff --cached -- <file> — passes
  • rtk git diff (no args) — passes
  • rtk git diff <bare-ref-with-no-dash> — passes

cargo test --release args_utils is 15/15 green; the unit tests cover the swallowed-by-clap, preserved-by-clap, no----in-original, and interleaved----and-args shapes.

One behavioral note worth flagging in case it's intentional: with the old normalize_diff_args heuristic, rtk git diff main (where main is both a branch and a file on disk) would have had -- injected based on the filesystem check and rendered the file diff. With this PR, the same command falls through to git which then exits with fatal: ambiguous argument 'main'. That looks like the right call to me — restoring what clap consumed is predictable, second-guessing the user with filesystem heuristics is not — but flagging it explicitly so the design choice is on the record.

Happy to close #1679 once this lands. Want me to do it now or wait until merge?

@KuSh
Copy link
Copy Markdown
Collaborator Author

KuSh commented May 10, 2026

Thanks a lot for the detailled test and report, very much appreciated!

One behavioral note worth flagging in case it's intentional: with the old normalize_diff_args heuristic, rtk git diff main (where main is both a branch and a file on disk) would have had -- injected based on the filesystem check and rendered the file diff. With this PR, the same command falls through to git which then exits with fatal: ambiguous argument 'main'.

The problem would have happen without rtk and should be handled by the LLM IMHO

Happy to close #1679 once this lands. Want me to do it now or wait until merge?

As we have a release branch, merging doesn't mean available in a release, we will close it once released. Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

rtk does not handle git diff correctly in some cases

2 participants