Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
121 changes: 113 additions & 8 deletions internal/cli/pr.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"
"os"
"os/exec"
"regexp"
"strconv"
"strings"

Expand Down Expand Up @@ -48,20 +49,32 @@ func prViewCmd() *cobra.Command {
)

cmd := &cobra.Command{
Use: "view <number>",
Use: "view [<number>]",
Short: "View a pull request",
Args: cobra.ExactArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
number, err := strconv.Atoi(args[0])
if err != nil {
return fmt.Errorf("invalid PR number: %s", args[0])
}
Long: `View a pull request by number, or view the PR for the current branch.

If no number is given, finds and displays the pull request whose head
branch matches the current git branch.`,
Args: cobra.MaximumNArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
forge, owner, repoName, _, err := resolve.Repo(flagRepo, flagForgeType)
if err != nil {
return err
}

var number int
if len(args) > 0 {
number, err = strconv.Atoi(args[0])
if err != nil {
return fmt.Errorf("invalid PR number: %s", args[0])
}
} else {
number, err = findPRForCurrentBranch(cmd.Context(), forge, owner, repoName)
if err != nil {
return err
}
}

pr, err := forge.PullRequests().Get(cmd.Context(), owner, repoName, number)
if err != nil {
return fmt.Errorf("getting PR #%d: %w", number, err)
Expand Down Expand Up @@ -610,10 +623,13 @@ The argument can be a PR number or a full URL:
localBranch = flagBranch
}

if !flagDetach {
_ = storePRForBranch(ctx, localBranch, number)
}

if pr.Head.Fork != nil {
return checkoutForkPR(ctx, domain, pr, remoteRef, localBranch, flagRemoteName, flagDetach, flagForce)
}

return checkoutSameRepoPR(ctx, remoteRef, localBranch, flagDetach, flagForce)
},
}
Expand Down Expand Up @@ -744,3 +760,92 @@ func gitCheckout(ctx context.Context, remote, remoteRef, localBranch string, det
resetCmd.Stderr = os.Stderr
return resetCmd.Run()
}

func findPRForCurrentBranch(ctx context.Context, f forges.Forge, owner, repo string) (int, error) {
out, err := exec.CommandContext(ctx, "git", "branch", "--show-current").Output()
if err != nil {
return 0, fmt.Errorf("getting current branch: %w (not in a git repository?)", err)
}
localBranch := strings.TrimSpace(string(out))
if localBranch == "" {
return 0, fmt.Errorf("not on a branch (detached HEAD state)")
}

// Check cache first (set by 'pr checkout')
if n, err := loadPRForBranch(ctx, localBranch); err == nil {
return n, nil
}

// If that yields nothing, fall back to API query. This API call is really
// slow for Gitea since the Head filter is not actually implemented.
headOwner := owner
if remoteOwner, err := resolve.OwnerForBranch(localBranch); err == nil {
headOwner = remoteOwner
}

prs, err := f.PullRequests().List(ctx, owner, repo, forges.ListPROpts{
Head: headOwner + ":" + localBranch,
State: "all",
Limit: 100,
})
if err != nil {
return 0, fmt.Errorf("listing PRs for branch %q: %w", localBranch, err)
}

// Need to filter the results again for owner:branch since the API results
// don't respect the filter in case of Gitea.
var matching []forges.PullRequest
for _, pr := range prs {
prHeadOwner := owner
if pr.Head.Fork != nil {
prHeadOwner = pr.Head.Fork.Owner
}
if pr.Head.Ref == localBranch && prHeadOwner == headOwner {
matching = append(matching, pr)
}
}

if len(matching) < 1 {
return 0, fmt.Errorf("no pull request found for branch %q", localBranch)
}

// Prefer open PRs over closed/merged ones (a branch may be reused)
for _, pr := range matching {
if pr.State == "open" {
// Store the PR number into local git config so that the next 'forge
// pr view' call is a lot faster.
_ = storePRForBranch(ctx, localBranch, pr.Number)
return pr.Number, nil
}
}

// No open PR, return the first match (closed/merged) but don't cache it
return matching[0].Number, nil
}

func storePRForBranch(ctx context.Context, branch string, number int) error {
key := fmt.Sprintf("branch.%s.forge-pr", branch)
return exec.CommandContext(ctx, "git", "config", "--local", key, strconv.Itoa(number)).Run()
}

var prRefRE = regexp.MustCompile(`^refs/pull/(\d+)/head$`)

func loadPRForBranch(ctx context.Context, branch string) (int, error) {
key := fmt.Sprintf("branch.%s.forge-pr", branch)
out, err := exec.CommandContext(ctx, "git", "config", "--get", key).Output()
if err == nil {
return strconv.Atoi(strings.TrimSpace(string(out)))
}

// Fall back to gh CLI's format (refs/pull/<n>/head in branch.<name>.merge)
mergeKey := fmt.Sprintf("branch.%s.merge", branch)
out, err = exec.CommandContext(ctx, "git", "config", "--get", mergeKey).Output()
if err != nil {
return 0, err
}
m := prRefRE.FindStringSubmatch(strings.TrimSpace(string(out)))
if m == nil {
return 0, fmt.Errorf("not a PR ref")
}
return strconv.Atoi(m[1])
}
8 changes: 5 additions & 3 deletions internal/cli/pr_checkout_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,18 @@ import (

// mockPRService implements forges.PullRequestService for testing.
type mockPRService struct {
pr *forges.PullRequest
err error
pr *forges.PullRequest
err error
listResult []forges.PullRequest
listErr error
}

func (m *mockPRService) Get(_ context.Context, _, _ string, _ int) (*forges.PullRequest, error) {
return m.pr, m.err
}

func (m *mockPRService) List(_ context.Context, _, _ string, _ forges.ListPROpts) ([]forges.PullRequest, error) {
return nil, nil
return m.listResult, m.listErr
}

func (m *mockPRService) Create(_ context.Context, _, _ string, _ forges.CreatePROpts) (*forges.PullRequest, error) {
Expand Down
162 changes: 150 additions & 12 deletions internal/cli/pr_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,15 @@ package cli

import (
"bytes"
"context"
"os"
"os/exec"
"path/filepath"
"strings"
"testing"

"github.com/git-pkgs/forge"
"github.com/git-pkgs/forge/internal/resolve"
)

func TestPRCmd(t *testing.T) {
Expand Down Expand Up @@ -74,18 +81,6 @@ func TestPRViewInvalidNumber(t *testing.T) {
}
}

func TestPRViewRequiresArg(t *testing.T) {
var buf bytes.Buffer
rootCmd.SetOut(&buf)
rootCmd.SetErr(&buf)
rootCmd.SetArgs([]string{"pr", "view"})

err := rootCmd.Execute()
if err == nil {
t.Fatal("expected error for missing argument")
}
}

func TestPRCreateRequiresTitleAndHead(t *testing.T) {
var buf bytes.Buffer
rootCmd.SetOut(&buf)
Expand Down Expand Up @@ -142,3 +137,146 @@ func TestPRDiffRequiresArg(t *testing.T) {
t.Fatal("expected error for missing argument")
}
}

func TestStorePRForBranch(t *testing.T) {
if _, err := exec.LookPath("git"); err != nil {
t.Skip("git not installed")
}

dir := t.TempDir()
t.Chdir(dir)

mustGitCmd(t, "init", "-q")
mustGitCmd(t, "config", "user.email", "test@test.com")
mustGitCmd(t, "config", "user.name", "Test")

if err := os.WriteFile(filepath.Join(dir, "README"), []byte("test"), 0644); err != nil {
t.Fatal(err)
}
mustGitCmd(t, "add", "README")
mustGitCmd(t, "commit", "-m", "init")
mustGitCmd(t, "checkout", "-b", "feature")

ctx := context.Background()

// Store PR number
if err := storePRForBranch(ctx, "feature", 42); err != nil {
t.Fatalf("storePRForBranch: %v", err)
}

// Load it back
n, err := loadPRForBranch(ctx, "feature")
if err != nil {
t.Fatalf("loadPRForBranch: %v", err)
}
if n != 42 {
t.Errorf("got %d, want 42", n)
}
}

func TestLoadPRForBranchGhFormat(t *testing.T) {
if _, err := exec.LookPath("git"); err != nil {
t.Skip("git not installed")
}

dir := t.TempDir()
t.Chdir(dir)

mustGitCmd(t, "init", "-q")
mustGitCmd(t, "config", "user.email", "test@test.com")
mustGitCmd(t, "config", "user.name", "Test")

if err := os.WriteFile(filepath.Join(dir, "README"), []byte("test"), 0644); err != nil {
t.Fatal(err)
}
mustGitCmd(t, "add", "README")
mustGitCmd(t, "commit", "-m", "init")
mustGitCmd(t, "checkout", "-b", "pr-branch")

// Set up gh CLI's format: branch.<name>.merge = refs/pull/<n>/head
mustGitCmd(t, "config", "branch.pr-branch.merge", "refs/pull/123/head")

ctx := context.Background()
n, err := loadPRForBranch(ctx, "pr-branch")
if err != nil {
t.Fatalf("loadPRForBranch: %v", err)
}
if n != 123 {
t.Errorf("got %d, want 123", n)
}
}

func TestFindPRForCurrentBranch(t *testing.T) {
if _, err := exec.LookPath("git"); err != nil {
t.Skip("git not installed")
}

dir := t.TempDir()
t.Chdir(dir)

mustGitCmd(t, "init", "-q")
mustGitCmd(t, "config", "user.email", "test@test.com")
mustGitCmd(t, "config", "user.name", "Test")
mustGitCmd(t, "remote", "add", "origin", "https://github.com/testowner/testrepo.git")

if err := os.WriteFile(filepath.Join(dir, "README"), []byte("test"), 0644); err != nil {
t.Fatal(err)
}
mustGitCmd(t, "add", "README")
mustGitCmd(t, "commit", "-m", "init")
mustGitCmd(t, "checkout", "-b", "feature")
mustGitCmd(t, "config", "branch.feature.remote", "origin")

// Set up mock forge that returns a PR for our branch
mockPR := &mockPRService{
listResult: []forges.PullRequest{
{
Number: 99,
State: "open",
Head: forges.PRBranch{
Ref: "feature",
},
},
},
}
resolve.SetTestForge(
&mockForge{prService: mockPR},
"testowner", "testrepo", "github.com",
)
t.Cleanup(resolve.ResetTestForge)

ctx := context.Background()
forge, owner, repo, _, err := resolve.Repo("", "")
if err != nil {
t.Fatalf("resolve.Repo: %v", err)
}

n, err := findPRForCurrentBranch(ctx, forge, owner, repo)
if err != nil {
t.Fatalf("findPRForCurrentBranch: %v", err)
}
if n != 99 {
t.Errorf("got %d, want 99", n)
}

// The PR number should now be cached
cached, err := loadPRForBranch(ctx, "feature")
if err != nil {
t.Fatalf("loadPRForBranch after find: %v", err)
}
if cached != 99 {
t.Errorf("cached PR = %d, want 99", cached)
}
}

func mustGitCmd(t *testing.T, args ...string) {
t.Helper()
cmd := exec.Command("git", args...)
cmd.Env = append(os.Environ(),
"GIT_CONFIG_GLOBAL=/dev/null",
"GIT_CONFIG_SYSTEM=/dev/null",
)
if out, err := cmd.CombinedOutput(); err != nil {
t.Fatalf("git %v: %v\n%s", args, err, out)
}
}
26 changes: 26 additions & 0 deletions internal/resolve/resolve.go
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,32 @@ func gitRemoteURL(name string) (string, error) {
return strings.TrimSpace(string(out)), nil
}

// OwnerForBranch returns the repository owner for the remote that the given
// branch tracks. This is useful for determining which fork a branch was pushed
// to when creating pull requests.
func OwnerForBranch(branch string) (string, error) {
remoteKey := fmt.Sprintf("branch.%s.remote", branch)
out, err := exec.Command("git", "config", "--get", remoteKey).Output()
if err != nil {
return "", err
}
remote := strings.TrimSpace(string(out))
if remote == "" {
return "", fmt.Errorf("no remote configured")
}

remoteURL, err := gitRemoteURL(remote)
if err != nil {
return "", err
}

_, owner, _, err := forges.ParseRepoURL(remoteURL)
if err != nil {
return "", err
}
return owner, nil
}

func newClient(domain string) *forges.Client {
token := TokenForDomain(domain)
var opts []forges.Option
Expand Down
Loading
Loading