Skip to content

fix(cli): accept hunk diff A B as the two-commit range A..B - #678

Open
HackAttack wants to merge 2 commits into
modem-dev:mainfrom
HackAttack:fix/diff-two-commit-args
Open

fix(cli): accept hunk diff A B as the two-commit range A..B#678
HackAttack wants to merge 2 commits into
modem-dev:mainfrom
HackAttack:fix/diff-two-commit-args

Conversation

@HackAttack

Copy link
Copy Markdown
Contributor

Git treats diff A B and diff A..B as the same two-commit review, but Hunk only understood the dotted spelling: a second positional was always folded into pathspecs, so hunk diff main feature silently reviewed main limited to a pathspec named feature.

Two positionals now normalize to A..B, except where the old reading is the intended one: a first target that already spells a range, or a second token that exists on disk, which is what diff <rev> <path> without a -- separator relies on.

@greptile-apps

greptile-apps Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

PR author is not in the allowed authors list.

@vercel

vercel Bot commented Aug 4, 2026

Copy link
Copy Markdown

@HackAttack is attempting to deploy a commit to the Modem Team on Vercel.

A member of the Team first needs to authorize it.

@benvinegar benvinegar left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two argument-parsing issues need to be fixed before merge.

This comment was generated by Pi using OpenAI Codex gpt-5.6-sol

Comment thread src/core/cli.ts Outdated
Comment thread src/core/cli.ts Outdated
Git treats `diff A B` and `diff A..B` as the same two-commit review, but
Hunk only understood the dotted spelling: a second positional was always
folded into pathspecs, so `hunk diff main feature` silently reviewed
`main` limited to a pathspec named `feature`.

Two positionals now normalize to `A..B`, except where the old reading is
the intended one: a first target that already spells a range, or a
second token that exists on disk, which is what `diff <rev> <path>`
without a `--` separator relies on.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
@HackAttack
HackAttack force-pushed the fix/diff-two-commit-args branch from a0f42b1 to 1121005 Compare August 9, 2026 15:45
`hunk diff A B` normalized to the Git range `A..B` while parsing, before
VCS detection. jj and Sapling read `..` as a revset over the commits
between the endpoints, so a diverged from-side contributed nothing and
its changes never appeared as removals. Verified against jj 0.44: `jj
diff -r a..b` omits a deletion that `jj diff --from a --to b` reports.

The endpoints now travel unjoined as `rangeEndpoints`, and each adapter
names them in its own syntax: Git `A..B`, jj `--from`/`--to`, Sapling a
`-r` per side.

Parsing also stopped asking the filesystem whether a token is a revision
or a pathspec. That answer moved with the working directory, and it read
deleted files and globs as revisions. Two positionals are two commits; a
pathspec needs `--`, unless a side already spells a range or there are
more positionals than a commit pair can hold.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
@HackAttack
HackAttack force-pushed the fix/diff-two-commit-args branch from 1121005 to 0ce9385 Compare August 9, 2026 15:48
@HackAttack

Copy link
Copy Markdown
Contributor Author

Both true, fixed!

@HackAttack
HackAttack requested a review from benvinegar August 9, 2026 15:50
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.

2 participants