diff --git a/README.md b/README.md index 9c81c1c..80074fb 100644 --- a/README.md +++ b/README.md @@ -145,7 +145,7 @@ Claude Code and Codex if you have them installed**. Each of those is on by defau change before it happens, and `--uninstall` removes what it recorded installing and nothing else (that record lives in `git config --global stack.managed*`). -- **Aliases** (`--aliases`): `git create|modify|absorb|reorder|move|fold|track|pop|restack|continue|abort|submit|sync|up|down|top|bottom` +- **Aliases** (`--aliases`): `git create|modify|absorb|reorder|move|fold|track|pop|restack|continue|abort|submit|sync|up|down|top|bottom|pr` and the short ones `git c|m|ab|rs|ss|u|d|t|b|co`, written with `git config --global`. Anything that clashes with a git builtin or an installed command is skipped, and an alias you already have is only replaced if you pass `--force` or say yes when asked. @@ -530,6 +530,26 @@ Flags: - `-s` / `--stack` only show the current stack - `-t` / `--trunk` check out the trunk of the current stack +### `git stack pr [branch]` + +> Alias: `git pr` + +Opens the pull request of a branch (the current one if you don't name one) in your browser. Without +a terminal it prints the URL instead, so `git pr | pbcopy` does what you'd hope. The PR comes from +what git-stack already knows locally and GitHub is only asked when nothing local knows the branch's +PR (one opened by hand on github.com, say). A branch with no PR yet tells you to `git ss` first. + +``` +❯ git pr +✔ Opened #418 (payments-retry-queue) in your browser + +❯ git pr --print billing-webhook-alerts +https://github.com/acme/payments/pull/421 +``` + +Flags: +- `--print` print the URL rather than opening it + ### `git stack sync` > Alias: `git sync` diff --git a/cmd/pr.go b/cmd/pr.go new file mode 100644 index 0000000..912fcbc --- /dev/null +++ b/cmd/pr.go @@ -0,0 +1,69 @@ +package cmd + +import ( + "runtime" + + "github.com/spf13/cobra" + + "github.com/DomBlack/git-stack/pkg/exec" +) + +// browserCommand is what opens a URL in the user's browser. +func browserCommand() string { + if runtime.GOOS == "darwin" { + return "open" + } + return "xdg-open" +} + +func newPRCmd(c *cli) *cobra.Command { + var printOnly bool + cmd := &cobra.Command{ + Use: "pr [branch]", + Short: "Open a branch's pull request in the browser", + Long: `Opens the pull request of a stacked branch (the current one by default) in +your browser. Without a terminal, or with --print, it prints the pull +request's URL instead. + +The pull request comes from what git-stack already knows locally; GitHub is +only asked when nothing local knows the branch's pull request.`, + Example: ` # open this branch's pull request + git pr + + # print the URL of feat-a's pull request + git pr --print feat-a`, + Args: cobra.MaximumNArgs(1), + ValidArgsFunction: c.completeStackedBranches, + RunE: func(cmd *cobra.Command, args []string) error { + ctx := cmd.Context() + a, repo, err := c.app(ctx) + if err != nil { + return err + } + var branch string + if len(args) == 1 { + branch = args[0] + } + pr, err := a.BranchPR(ctx, repo, branch) + if err != nil { + return err + } + rt := c.runtime() + rep := rt.Report + if printOnly || !rt.Interactive { + rep.Print(rep.Link(pr.URL) + "\n") + return nil + } + if _, err := rt.Runner.Run(ctx, exec.Cmd{Name: browserCommand(), Args: []string{pr.URL}}); err != nil { + rt.Log.Debug("open browser", "err", err) + rep.Warn("Couldn't open your browser; here's the link to #%d (%s)", pr.Number, pr.Head) + rep.Print(rep.Link(pr.URL) + "\n") + return nil + } + rep.Success("Opened %s (%s) in your browser", rep.Ref(pr.Number, pr.URL, pr.State), rep.Branch(pr.Head)) + return nil + }, + } + cmd.Flags().BoolVar(&printOnly, "print", false, "print the URL instead of opening it") + return cmd +} diff --git a/cmd/pr_test.go b/cmd/pr_test.go new file mode 100644 index 0000000..2eafe47 --- /dev/null +++ b/cmd/pr_test.go @@ -0,0 +1,122 @@ +package cmd + +import ( + "bytes" + "context" + "log/slog" + "strings" + "testing" + + "github.com/DomBlack/git-stack/pkg/exec" + "github.com/DomBlack/git-stack/pkg/exec/exectest" + "github.com/DomBlack/git-stack/pkg/git" + "github.com/DomBlack/git-stack/pkg/ui" +) + +// prFake runs git for real and stands in for gh and for both browser +// openers, so no test can ever open a real browser. +func prFake(openErr bool) *exectest.Fake { + f := exectest.New() + real := exec.New() + f.Fallback = func(c exec.Cmd) (exec.Result, error) { return real.Run(context.Background(), c) } + f.On("gh", "extension", "list").Reply("gh stack\tgithub/gh-stack\tv0.9.0\n") + f.On("gh", "pr", "list").Reply("[]") + for _, name := range []string{"open", "xdg-open"} { + f.On(name).Do(func(c exec.Cmd) (exec.Result, error) { + if openErr { + res := exec.Result{ExitCode: 3, Stderr: []byte("no method available for opening")} + return res, &exec.ExitError{Cmd: c, Result: res} + } + return exec.Result{}, nil + }) + } + return f +} + +// opened lists the URLs handed to a browser opener. +func opened(f *exectest.Fake) []string { + var out []string + for _, name := range []string{"open", "xdg-open"} { + for _, c := range f.CallsTo(name) { + out = append(out, strings.Join(c.Args, " ")) + } + } + return out +} + +// runInteractive runs the CLI as if stdin and stdout were a terminal. +func runInteractive(t *testing.T, f *exectest.Fake, dir string, args ...string) (string, string, error) { + t.Helper() + var out, errOut bytes.Buffer + streams := Streams{In: strings.NewReader(""), Out: &out, Err: &errOut} + c := &cli{streams: streams, newRunner: func(...exec.Option) exec.Runner { return f }} + c.rt = &Runtime{ + Globals: Globals{Cwd: dir}, Streams: streams, Interactive: true, + Runner: f, Git: git.New(f), Log: slog.New(slog.DiscardHandler), + Report: ui.NewReporter(streams.In, &out, &errOut, ui.ReporterOptions{}), + } + c.rtOnce.Do(func() {}) + root := newRootCmd(c) + root.SetArgs(append([]string{"--cwd", dir}, args...)) + err := root.Execute() + return out.String(), errOut.String(), err +} + +func TestPRCommandPrintsWithoutATerminal(t *testing.T) { + dir := stackRepo(t) // main -> a (#41) -> b -> c, on b + f := prFake(false) + for _, args := range [][]string{{"pr", "a"}, {"pr", "--print", "a"}} { + out, errOut, err := runWith(t, f, append([]string{"--cwd", dir}, args...)...) + if err != nil || out != "https://example.com/41\n" { + t.Errorf("%v: %q %q %v", args, out, errOut, err) + } + } + if got := opened(f); len(got) != 0 { + t.Errorf("nothing should open without a terminal: %v", got) + } +} + +func TestPRCommandOpensTheBrowser(t *testing.T) { + dir := stackRepo(t) + f := prFake(false) + out, errOut, err := runInteractive(t, f, dir, "pr", "a") + if err != nil || out != "ok: Opened #41 (a) in your browser\n" { + t.Errorf("pr a: %q %q %v", out, errOut, err) + } + if got := opened(f); len(got) != 1 || got[0] != "https://example.com/41" { + t.Errorf("opened %v", got) + } + + out, _, err = runInteractive(t, f, dir, "pr", "--print", "a") + if err != nil || out != "https://example.com/41\n" || len(opened(f)) != 1 { + t.Errorf("--print should only print: %q %v %v", out, err, opened(f)) + } +} + +func TestPRCommandPrintsWhenTheBrowserWontOpen(t *testing.T) { + dir := stackRepo(t) + f := prFake(true) + out, errOut, err := runInteractive(t, f, dir, "pr", "a") + if err != nil || out != "https://example.com/41\n" || !strings.Contains(errOut, "note: Couldn't open your browser") { + t.Errorf("pr a: %q %q %v", out, errOut, err) + } +} + +func TestPRCommandErrors(t *testing.T) { + dir := stackRepo(t) + f := prFake(false) + // b has no PR anywhere, the forge included. + _, errOut, err := runWith(t, f, "--cwd", dir, "pr") + if err == nil || err.Error() != "b has no pull request yet" { + t.Errorf("pr on b: %v %q", err, errOut) + } + if _, _, err := runWith(t, f, "--cwd", dir, "pr", "main"); err == nil || !strings.Contains(err.Error(), "main is a trunk") { + t.Errorf("pr main: %v", err) + } + if _, _, err := runWith(t, f, "--cwd", dir, "pr", "loose"); err == nil || err.Error() != "loose is not in a stack" { + t.Errorf("pr loose: %v", err) + } + if got := opened(f); len(got) != 0 { + t.Errorf("opened %v", got) + } +} diff --git a/cmd/root.go b/cmd/root.go index 44d8f8b..ed2a5c0 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -339,6 +339,7 @@ func newRootCmd(c *cli) *cobra.Command { newBottomCmd(c), newCheckoutCmd(c), newLogCmd(c), + newPRCmd(c), newInstallCmd(c), newMcpCmd(c), newCompletionCmd(c), diff --git a/docs/mcp.md b/docs/mcp.md index d8fd280..9a34217 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -48,6 +48,8 @@ Claude Code drafter. `git stack version` and `git stack update` have no MCP tools on purpose; they're about the binary, not the stack, and an agent replacing the server it's talking to mid session is not something we want to make easy. The server does report its version in the MCP handshake. +`git stack pr` has none either: it opens a browser, and an agent that wants a PR's URL already +gets it for every branch from `stack_view`. ## Errors diff --git a/pkg/app/pr.go b/pkg/app/pr.go new file mode 100644 index 0000000..76e83b9 --- /dev/null +++ b/pkg/app/pr.go @@ -0,0 +1,79 @@ +package app + +import ( + "context" + "errors" + + "github.com/DomBlack/git-stack/pkg/forge" + "github.com/DomBlack/git-stack/pkg/git" + "github.com/DomBlack/git-stack/pkg/stack" +) + +// BranchPR finds the pull request of a stacked branch (the current one when +// branch is ""). Local state answers first, the PR cache then gh stack's +// metadata, the same order View applies them in; the forge is only asked +// when neither knows the branch's PR, i.e. one opened outside git-stack or +// since the cache was last filled. The result always has a URL. +func (a *App) BranchPR(ctx context.Context, repo git.Repo, branch string) (forge.PullRequest, error) { + if branch == "" { + current, err := a.d.Git.CurrentBranch(ctx, repo) + if err != nil { + if errors.Is(err, git.ErrDetached) { + return forge.PullRequest{}, stack.New(stack.KindInvalidArgs, "HEAD is detached; name a stacked branch"). + WithSteps("git stack pr ") + } + return forge.PullRequest{}, err + } + branch = current + } + graph, err := a.d.Meta.Load(ctx, repo) + if err != nil { + return forge.PullRequest{}, err + } + s, idx, ok := graph.StackOf(branch) + if !ok { + if graph.IsTrunk(branch) { + return forge.PullRequest{}, stack.Newf(stack.KindInvalidArgs, "%s is a trunk; pull requests belong to the branches stacked on it", branch) + } + local, err := a.localBranches(ctx, repo) + if err != nil { + return forge.PullRequest{}, err + } + if _, ok := local[branch]; !ok { + return forge.PullRequest{}, stack.Newf(stack.KindNotInStack, "there's no branch called %s", branch) + } + return forge.PullRequest{}, stack.Newf(stack.KindNotInStack, "%s is not in a stack", branch). + WithSteps("git stack init " + branch + " to start tracking it") + } + + cached, _, _ := a.CachedPRs(repo) + pr, ok := PRsFor(cached)[branch] + if !ok { + if ref := s.Branches[idx].PR; ref != nil { + pr, ok = forge.PullRequest{Number: ref.Number, URL: ref.URL, Head: branch}, true + } + } + if !ok { + if a.d.Forge == nil { + return forge.PullRequest{}, ErrNoForge + } + var fresh []forge.PullRequest + if err := a.progress(ctx, PhaseRefresh, "Fetching pull requests", func(ctx context.Context) error { + fresh, err = a.RefreshPRs(ctx, repo) + return err + }); err != nil { + return forge.PullRequest{}, err + } + if pr, ok = PRsFor(fresh)[branch]; !ok { + return forge.PullRequest{}, stack.Newf(stack.KindInvalidArgs, "%s has no pull request yet", branch). + WithSteps("git stack submit") + } + } + if pr.URL == "" { + pr.URL = a.PRURLs(ctx, repo)(pr.Number) + } + if pr.URL == "" { + return forge.PullRequest{}, stack.Newf(stack.KindUnsupported, "can't work out the address of #%d (%s)", pr.Number, branch) + } + return pr, nil +} diff --git a/pkg/app/pr_test.go b/pkg/app/pr_test.go new file mode 100644 index 0000000..46331e5 --- /dev/null +++ b/pkg/app/pr_test.go @@ -0,0 +1,97 @@ +package app_test + +import ( + "context" + "errors" + "slices" + "testing" + + "github.com/DomBlack/git-stack/pkg/app" + "github.com/DomBlack/git-stack/pkg/cache" + "github.com/DomBlack/git-stack/pkg/config" + "github.com/DomBlack/git-stack/pkg/forge" + "github.com/DomBlack/git-stack/pkg/stack" +) + +func TestBranchPRComesFromLocalStateFirst(t *testing.T) { + _, repo, _ := fixture(t) + ff := &fakeForge{} + graph := stack.NewGraph([]stack.Stack{{Trunk: "main", Branches: []stack.Branch{ + {Name: "a", PR: &stack.PRRef{Number: 3}}, {Name: "b", PR: &stack.PRRef{Number: 5, URL: "u5"}}, {Name: "c"}, + }}}) + c := cache.New(repo) + if err := cache.Write(c, "prs", app.PRSnapshot{PRs: []forge.PullRequest{ + {Number: 9, Head: "c", State: forge.StateDraft, URL: "u9"}, + }}); err != nil { + t.Fatal(err) + } + a := app.New(app.Deps{Git: gitClient(), Meta: memMeta{graph}, Forge: ff, Cache: c, Config: config.Defaults()}) + ctx := context.Background() + + for _, tc := range []struct { + branch string + want forge.PullRequest + }{ + {"", forge.PullRequest{Number: 5, Head: "b", URL: "u5"}}, // the current branch, from gh stack's metadata + {"c", forge.PullRequest{Number: 9, Head: "c", URL: "u9", State: forge.StateDraft}}, // the PR cache + {"a", forge.PullRequest{Number: 3, Head: "a", URL: "https://forge.test/o/r/pull/3"}}, // a number with no URL + } { + got, err := a.BranchPR(ctx, repo, tc.branch) + if err != nil { + t.Fatalf("%q: %v", tc.branch, err) + } + if got.Number != tc.want.Number || got.Head != tc.want.Head || got.URL != tc.want.URL || got.State != tc.want.State { + t.Errorf("%q: got %+v, want %+v", tc.branch, got, tc.want) + } + } + if ff.calls != 0 { + t.Errorf("a PR known locally must not ask the forge (%d calls)", ff.calls) + } +} + +func TestBranchPRAsksTheForgeWhenNothingLocalKnows(t *testing.T) { + a, repo, _ := fixture(t) + ff := &fakeForge{prs: []forge.PullRequest{{Number: 12, Head: "c", State: forge.StateOpen, URL: "u12"}}} + var phases []app.Phase + a = app.New(app.Deps{Git: gitClient(), Meta: metaOf(a, t), Forge: ff, Cache: cache.New(repo), Config: config.Defaults(), + Progress: func(ctx context.Context, p app.Phase, _ string, fn func(context.Context) error) error { + phases = append(phases, p) + return fn(ctx) + }}) + ctx := context.Background() + + got, err := a.BranchPR(ctx, repo, "c") + if err != nil || got.Number != 12 || got.URL != "u12" { + t.Fatalf("c: %+v %v", got, err) + } + if ff.calls != 1 || !slices.Equal(phases, []app.Phase{app.PhaseRefresh}) { + t.Errorf("want one refresh under its own step: %d calls, phases %v", ff.calls, phases) + } + + _, err = a.BranchPR(ctx, repo, "a") + se, ok := errors.AsType[*stack.Error](err) + if !ok || se.Msg != "a has no pull request yet" || !slices.Contains(se.NextSteps, "git stack submit") { + t.Errorf("a: %v", err) + } + + ff.err = errors.New("offline") + if _, err := a.BranchPR(ctx, repo, "a"); err == nil || !errors.Is(err, ff.err) { + t.Errorf("a failed refresh should say so: %v", err) + } +} + +func TestBranchPRRefusesTrunksAndUntrackedBranches(t *testing.T) { + a, repo, _ := fixture(t) + ff := &fakeForge{} + a = app.New(app.Deps{Git: gitClient(), Meta: metaOf(a, t), Forge: ff, Cache: cache.New(repo), Config: config.Defaults()}) + ctx := context.Background() + for branch, want := range map[string]stack.Kind{"main": stack.KindInvalidArgs, "loose": stack.KindNotInStack, "nope": stack.KindNotInStack} { + _, err := a.BranchPR(ctx, repo, branch) + if se, ok := errors.AsType[*stack.Error](err); !ok || se.Kind != want { + t.Errorf("%s: %v, want kind %v", branch, err, want) + } + } + if ff.calls != 0 { + t.Errorf("refusals must not ask the forge (%d calls)", ff.calls) + } +} diff --git a/pkg/gitconfig/aliases.go b/pkg/gitconfig/aliases.go index a86eeb5..6464917 100644 --- a/pkg/gitconfig/aliases.go +++ b/pkg/gitconfig/aliases.go @@ -43,6 +43,7 @@ var DefaultAliases = []Alias{ {"bottom", "stack bottom"}, {"continue", "stack continue"}, {"abort", "stack abort"}, + {"pr", "stack pr"}, {"c", "stack create"}, {"m", "stack modify"}, {"ab", "stack absorb"}, diff --git a/pkg/install/install_test.go b/pkg/install/install_test.go index 6fb4312..d26d09d 100644 --- a/pkg/install/install_test.go +++ b/pkg/install/install_test.go @@ -192,7 +192,7 @@ func TestInstallAndUninstall(t *testing.T) { } func gitconfigDefaults() []string { - return []string{"create", "modify", "absorb", "reorder", "move", "fold", "track", "pop", "restack", "submit", "sync", "up", "down", "top", "bottom", "continue", "abort", "c", "m", "ab", "rs", "ss", "u", "d", "t", "b", "co"} + return []string{"create", "modify", "absorb", "reorder", "move", "fold", "track", "pop", "restack", "submit", "sync", "up", "down", "top", "bottom", "continue", "abort", "pr", "c", "m", "ab", "rs", "ss", "u", "d", "t", "b", "co"} } func TestSkillOptInIsAsked(t *testing.T) { diff --git a/pkg/install/skill/SKILL.md b/pkg/install/skill/SKILL.md index 075c344..09f24c6 100644 --- a/pkg/install/skill/SKILL.md +++ b/pkg/install/skill/SKILL.md @@ -59,5 +59,5 @@ the one calling. `git stack create -m "…"`, `git stack modify -a`, `git stack absorb`, `git stack reorder a b c`, `git stack move --onto `, `git stack fold`, `git stack init a b`, `git stack pop`, `git stack restack`, `git stack up/down/top/bottom`, `git stack checkout`, `git stack submit --no-edit`, -`git stack sync -f`. Aliases: `git c`, `git m`, `git ab`, `git reorder`, `git move`, `git fold`, `git track`, `git pop`, -`git rs`, `git u`, `git d`, `git t`, `git b`, `git co`, `git ss`. +`git stack sync -f`, `git stack pr --print` (a branch's PR URL; `stack_view` has them all). Aliases: `git c`, `git m`, `git ab`, `git reorder`, `git move`, `git fold`, `git track`, `git pop`, +`git rs`, `git u`, `git d`, `git t`, `git b`, `git co`, `git ss`, `git pr`.