Skip to content

feat(diff): review a GitHub pull request with --pr - #663

Closed
habakan wants to merge 1 commit into
modem-dev:mainfrom
habakan:feat/diff-pr-flag
Closed

feat(diff): review a GitHub pull request with --pr#663
habakan wants to merge 1 commit into
modem-dev:mainfrom
habakan:feat/diff-pr-flag

Conversation

@habakan

@habakan habakan commented Aug 2, 2026

Copy link
Copy Markdown

Summary

Implements hunk diff --pr <number|url> [--repo <owner/repo>] — review a GitHub pull request directly, without a manual gh pr checkout.

Closes #465.

Hunk stays a local diff viewer: it shells out to the user's authenticated GitHub CLI (gh pr diff <ref> --patch) and feeds the resulting patch through the existing patch pipeline. A PR is just another patch source, so no new rendering/loader path was needed.

hunk diff --pr 68
hunk diff --pr https://github.com/modem-dev/hunk/pull/68
hunk diff --pr 68 --repo modem-dev/hunk

Changes

  • src/core/github.ts (new): fetchPullRequestPatch() wraps gh, with friendly errors when gh is missing (ENOENT), the PR can't be fetched (surfaces gh stderr — not-authenticated / not-found), or the diff is empty.
  • src/core/cli.ts: --pr / --repo options on diff; parseDiffCommand returns a kind: "patch" input when --pr is set. --pr combined with a target / --staged / pathspecs errors out.
  • src/core/types.ts: PatchCommandInput.label lets an in-memory patch carry a header label (PR #68), since it has no backing file. loadPatchChangeset honors it.
  • Docs (generate:docs) + changeset + tests.

Scope (deliberately an MVP)

Per the issue, this is the inbound direction only — open a PR to review it. Left as follow-ups so this PR stays focused:

Happy to adjust the surface (e.g. patch vs diff, flag naming, --repo inclusion) if you'd prefer a different shape.

Verification

  • bun run typecheck — clean.
  • bun test src/core/cli.test.ts src/core/github.test.ts src/core/loaders.test.ts — pass. New coverage: github.test.ts (success, --repo passthrough, URL label, gh missing, non-zero exit, empty diff) and cli.test.ts (diff --pr → patch input, --pr + target rejected).
  • bun run lint — 0 warnings/errors.
  • bun run check:docs — clean after regen.
  • End-to-end against a real PR via authenticated gh: fetches the patch and parses to kind: "patch", label: "PR #68".

Add `hunk diff --pr <number|url> [--repo <owner/repo>]`. Hunk shells out to
the authenticated GitHub CLI (`gh pr diff --patch`) and feeds the patch through
the existing patch loader, so a pull request opens for review without a manual
`gh pr checkout`.

- `src/core/github.ts`: `fetchPullRequestPatch()` wraps `gh`, with friendly
  errors when `gh` is missing, the PR can't be fetched, or the diff is empty.
- `PatchCommandInput.label` lets an in-memory patch carry a header label
  (`PR modem-dev#68`) since it has no backing file.
- Scope: patch display only. Importing unresolved review threads (modem-dev#487/modem-dev#460)
  and current-branch PR auto-detection are intentionally left as follow-ups.

Closes modem-dev#465
@greptile-apps

greptile-apps Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

PR author is not in the allowed authors list.

@vercel

vercel Bot commented Aug 2, 2026

Copy link
Copy Markdown

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

A member of the Team first needs to authorize it.

@benvinegar

benvinegar commented Aug 2, 2026

Copy link
Copy Markdown
Member

Did you try:

 gh pr diff 663 --patch | hunk patch -

If that's too tough to remember (it is), you could make a little shell fn and add it to your .bashrc/.zshrc:

hunk-pr() {
    gh pr diff "$1" --patch "${@:2}" | hunk patch -
}

Usage:

  hunk-pr 663
  hunk-pr 663 --repo modem-dev/hunk

I'd prefer if we documented + shared this workflow vs. coupling hunk to gh and making remote fetches. It really expands the surface area a lot.

@habakan

habakan commented Aug 3, 2026

Copy link
Copy Markdown
Author

That makes sense — keeping hunk decoupled from gh and avoiding remote fetches is a fair call, and the pipe workflow covers the core use case well.

I've turned this into a docs PR instead: #672 documents gh pr diff 123 --patch | hunk patch - and your hunk-pr shell function in the website workflow docs and README. Closing this one in favor of that.

@habakan habakan closed this Aug 3, 2026
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.

Review a PR directly by URL/number (hunk diff --pr)

2 participants