diff --git a/bitbucket/url.go b/bitbucket/url.go new file mode 100644 index 0000000..71b0e69 --- /dev/null +++ b/bitbucket/url.go @@ -0,0 +1,37 @@ +package bitbucket + +import ( + "fmt" + "strconv" + + "github.com/git-pkgs/forge" +) + +// ParsePath implements Forge.ParsePath for Bitbucket URLs. +func (f *bitbucketForge) ParsePath(parts []string) (*forges.ResourceRef, error) { + if len(parts) < 2 { + return nil, fmt.Errorf("URL path must contain owner/repo") + } + + ref := &forges.ResourceRef{ + Owner: parts[0], + Repo: parts[1], + } + + if len(parts) >= 4 { + num, err := strconv.Atoi(parts[3]) + if err != nil { + return nil, fmt.Errorf("invalid number %q", parts[3]) + } + ref.Number = num + + switch parts[2] { + case "pull-requests": + ref.Type = forges.ResourceTypePR + case "issues": + ref.Type = forges.ResourceTypeIssue + } + } + + return ref, nil +} diff --git a/bitbucket/url_test.go b/bitbucket/url_test.go new file mode 100644 index 0000000..51af804 --- /dev/null +++ b/bitbucket/url_test.go @@ -0,0 +1,75 @@ +package bitbucket + +import ( + "testing" + + forges "github.com/git-pkgs/forge" +) + +func TestParsePath(t *testing.T) { + tests := []struct { + name string + parts []string + wantOwner string + wantRepo string + wantResource forges.ResourceType + wantNumber int + wantErr bool + }{ + { + name: "repo only", + parts: []string{"owner", "repo"}, + wantOwner: "owner", wantRepo: "repo", + }, + { + name: "pull request", + parts: []string{"owner", "repo", "pull-requests", "123"}, + wantOwner: "owner", wantRepo: "repo", + wantResource: forges.ResourceTypePR, wantNumber: 123, + }, + { + name: "issue", + parts: []string{"owner", "repo", "issues", "456"}, + wantOwner: "owner", wantRepo: "repo", + wantResource: forges.ResourceTypeIssue, wantNumber: 456, + }, + { + name: "missing repo", + parts: []string{"owner"}, + wantErr: true, + }, + { + name: "invalid PR number", + parts: []string{"owner", "repo", "pull-requests", "abc"}, + wantErr: true, + }, + } + + f := &bitbucketForge{} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ref, err := f.ParsePath(tt.parts) + if tt.wantErr { + if err == nil { + t.Fatal("expected error") + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if ref.Owner != tt.wantOwner { + t.Errorf("owner: got %q, want %q", ref.Owner, tt.wantOwner) + } + if ref.Repo != tt.wantRepo { + t.Errorf("repo: got %q, want %q", ref.Repo, tt.wantRepo) + } + if ref.Type != tt.wantResource { + t.Errorf("resource: got %q, want %q", ref.Type, tt.wantResource) + } + if ref.Number != tt.wantNumber { + t.Errorf("number: got %d, want %d", ref.Number, tt.wantNumber) + } + }) + } +} diff --git a/forge.go b/forge.go index c944604..dbef2d4 100644 --- a/forge.go +++ b/forge.go @@ -23,6 +23,22 @@ var ErrNotSupported = errors.New("not supported by this forge") // ErrLabelExists is returned when creating a label that already exists. var ErrLabelExists = errors.New("label already exists") +// ResourceType identifies the kind of resource a URL points to. +type ResourceType string + +const ( + ResourceTypePR ResourceType = "pr" + ResourceTypeIssue ResourceType = "issue" +) + +// ResourceRef identifies a resource (PR, issue, etc.) within a repository. +type ResourceRef struct { + Owner string + Repo string + Type ResourceType + Number int +} + // HTTPError represents a non-OK HTTP response from a forge API. type HTTPError struct { StatusCode int @@ -52,6 +68,8 @@ type Forge interface { Collaborators() CollaboratorService CommitStatuses() CommitStatusService GetRateLimit(ctx context.Context) (*RateLimit, error) + // ParsePath parses URL path segments into a resource reference. + ParsePath(pathParts []string) (*ResourceRef, error) } // Client routes requests to the appropriate Forge based on the URL domain. diff --git a/forges_test.go b/forges_test.go index f903b82..7329fbd 100644 --- a/forges_test.go +++ b/forges_test.go @@ -505,6 +505,10 @@ func (m *mockForge) GetRateLimit(_ context.Context) (*RateLimit, error) { return nil, ErrNotSupported } +func (m *mockForge) ParsePath(_ []string) (*ResourceRef, error) { + return &ResourceRef{}, nil +} + type mockFileService struct{} func (m *mockFileService) Get(_ context.Context, _, _, _, _ string) (*FileContent, error) { diff --git a/gitea/url.go b/gitea/url.go new file mode 100644 index 0000000..8b1a8cf --- /dev/null +++ b/gitea/url.go @@ -0,0 +1,37 @@ +package gitea + +import ( + "fmt" + "strconv" + + "github.com/git-pkgs/forge" +) + +// ParsePath implements Forge.ParsePath for Gitea/Forgejo URLs. +func (f *giteaForge) ParsePath(parts []string) (*forges.ResourceRef, error) { + if len(parts) < 2 { + return nil, fmt.Errorf("URL path must contain owner/repo") + } + + ref := &forges.ResourceRef{ + Owner: parts[0], + Repo: parts[1], + } + + if len(parts) >= 4 { + num, err := strconv.Atoi(parts[3]) + if err != nil { + return nil, fmt.Errorf("invalid number %q", parts[3]) + } + ref.Number = num + + switch parts[2] { + case "pulls": + ref.Type = forges.ResourceTypePR + case "issues": + ref.Type = forges.ResourceTypeIssue + } + } + + return ref, nil +} diff --git a/gitea/url_test.go b/gitea/url_test.go new file mode 100644 index 0000000..abf135e --- /dev/null +++ b/gitea/url_test.go @@ -0,0 +1,75 @@ +package gitea + +import ( + "testing" + + forges "github.com/git-pkgs/forge" +) + +func TestParsePath(t *testing.T) { + tests := []struct { + name string + parts []string + wantOwner string + wantRepo string + wantResource forges.ResourceType + wantNumber int + wantErr bool + }{ + { + name: "repo only", + parts: []string{"owner", "repo"}, + wantOwner: "owner", wantRepo: "repo", + }, + { + name: "pull request", + parts: []string{"owner", "repo", "pulls", "123"}, + wantOwner: "owner", wantRepo: "repo", + wantResource: forges.ResourceTypePR, wantNumber: 123, + }, + { + name: "issue", + parts: []string{"owner", "repo", "issues", "456"}, + wantOwner: "owner", wantRepo: "repo", + wantResource: forges.ResourceTypeIssue, wantNumber: 456, + }, + { + name: "missing repo", + parts: []string{"owner"}, + wantErr: true, + }, + { + name: "invalid PR number", + parts: []string{"owner", "repo", "pulls", "abc"}, + wantErr: true, + }, + } + + f := &giteaForge{} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ref, err := f.ParsePath(tt.parts) + if tt.wantErr { + if err == nil { + t.Fatal("expected error") + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if ref.Owner != tt.wantOwner { + t.Errorf("owner: got %q, want %q", ref.Owner, tt.wantOwner) + } + if ref.Repo != tt.wantRepo { + t.Errorf("repo: got %q, want %q", ref.Repo, tt.wantRepo) + } + if ref.Type != tt.wantResource { + t.Errorf("resource: got %q, want %q", ref.Type, tt.wantResource) + } + if ref.Number != tt.wantNumber { + t.Errorf("number: got %d, want %d", ref.Number, tt.wantNumber) + } + }) + } +} diff --git a/github/url.go b/github/url.go new file mode 100644 index 0000000..a8903b1 --- /dev/null +++ b/github/url.go @@ -0,0 +1,37 @@ +package github + +import ( + "fmt" + "strconv" + + forges "github.com/git-pkgs/forge" +) + +// ParsePath implements Forge.ParsePath for GitHub URLs. +func (f *gitHubForge) ParsePath(parts []string) (*forges.ResourceRef, error) { + if len(parts) < 2 { + return nil, fmt.Errorf("URL path must contain owner/repo") + } + + ref := &forges.ResourceRef{ + Owner: parts[0], + Repo: parts[1], + } + + if len(parts) >= 4 { + num, err := strconv.Atoi(parts[3]) + if err != nil { + return nil, fmt.Errorf("invalid number %q", parts[3]) + } + ref.Number = num + + switch parts[2] { + case "pull": + ref.Type = forges.ResourceTypePR + case "issues": + ref.Type = forges.ResourceTypeIssue + } + } + + return ref, nil +} diff --git a/github/url_test.go b/github/url_test.go new file mode 100644 index 0000000..fd5fcf6 --- /dev/null +++ b/github/url_test.go @@ -0,0 +1,81 @@ +package github + +import ( + "testing" + + forges "github.com/git-pkgs/forge" +) + +func TestParsePath(t *testing.T) { + tests := []struct { + name string + parts []string + wantOwner string + wantRepo string + wantResource forges.ResourceType + wantNumber int + wantErr bool + }{ + { + name: "repo only", + parts: []string{"owner", "repo"}, + wantOwner: "owner", wantRepo: "repo", + }, + { + name: "pull request", + parts: []string{"owner", "repo", "pull", "123"}, + wantOwner: "owner", wantRepo: "repo", + wantResource: forges.ResourceTypePR, wantNumber: 123, + }, + { + name: "pull request with extra path", + parts: []string{"owner", "repo", "pull", "123", "files"}, + wantOwner: "owner", wantRepo: "repo", + wantResource: forges.ResourceTypePR, wantNumber: 123, + }, + { + name: "issue", + parts: []string{"owner", "repo", "issues", "456"}, + wantOwner: "owner", wantRepo: "repo", + wantResource: forges.ResourceTypeIssue, wantNumber: 456, + }, + { + name: "missing repo", + parts: []string{"owner"}, + wantErr: true, + }, + { + name: "invalid PR number", + parts: []string{"owner", "repo", "pull", "abc"}, + wantErr: true, + }, + } + + f := &gitHubForge{} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ref, err := f.ParsePath(tt.parts) + if tt.wantErr { + if err == nil { + t.Fatal("expected error") + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if ref.Owner != tt.wantOwner { + t.Errorf("owner: got %q, want %q", ref.Owner, tt.wantOwner) + } + if ref.Repo != tt.wantRepo { + t.Errorf("repo: got %q, want %q", ref.Repo, tt.wantRepo) + } + if ref.Type != tt.wantResource { + t.Errorf("resource: got %q, want %q", ref.Type, tt.wantResource) + } + if ref.Number != tt.wantNumber { + t.Errorf("number: got %d, want %d", ref.Number, tt.wantNumber) + } + }) + } +} diff --git a/gitlab/url.go b/gitlab/url.go new file mode 100644 index 0000000..a32173b --- /dev/null +++ b/gitlab/url.go @@ -0,0 +1,56 @@ +package gitlab + +import ( + "fmt" + "strconv" + "strings" + + "github.com/git-pkgs/forge" +) + +// ParsePath implements Forge.ParsePath for GitLab URLs. +func (f *gitLabForge) ParsePath(parts []string) (*forges.ResourceRef, error) { + if len(parts) < 2 { + return nil, fmt.Errorf("URL path must contain owner/repo") + } + + // Find /-/ separator marking resource paths + dashIdx := -1 + for i, part := range parts { + if part == "-" { + dashIdx = i + break + } + } + + ref := &forges.ResourceRef{} + + if dashIdx == -1 { + ref.Owner = strings.Join(parts[:len(parts)-1], "/") + ref.Repo = parts[len(parts)-1] + return ref, nil + } + + if dashIdx < 2 { + return nil, fmt.Errorf("URL path must contain owner/repo before /-/") + } + ref.Owner = strings.Join(parts[:dashIdx-1], "/") + ref.Repo = parts[dashIdx-1] + + if dashIdx+2 < len(parts) { + num, err := strconv.Atoi(parts[dashIdx+2]) + if err != nil { + return nil, fmt.Errorf("invalid number %q", parts[dashIdx+2]) + } + ref.Number = num + + switch parts[dashIdx+1] { + case "merge_requests": + ref.Type = forges.ResourceTypePR + case "issues": + ref.Type = forges.ResourceTypeIssue + } + } + + return ref, nil +} diff --git a/gitlab/url_test.go b/gitlab/url_test.go new file mode 100644 index 0000000..803747d --- /dev/null +++ b/gitlab/url_test.go @@ -0,0 +1,86 @@ +package gitlab + +import ( + "testing" + + forges "github.com/git-pkgs/forge" +) + +func TestParsePath(t *testing.T) { + tests := []struct { + name string + parts []string + wantOwner string + wantRepo string + wantResource forges.ResourceType + wantNumber int + wantErr bool + }{ + { + name: "repo only", + parts: []string{"group", "project"}, + wantOwner: "group", wantRepo: "project", + }, + { + name: "nested group", + parts: []string{"group", "subgroup", "project"}, + wantOwner: "group/subgroup", wantRepo: "project", + }, + { + name: "merge request", + parts: []string{"group", "project", "-", "merge_requests", "123"}, + wantOwner: "group", wantRepo: "project", + wantResource: forges.ResourceTypePR, wantNumber: 123, + }, + { + name: "nested group merge request", + parts: []string{"group", "subgroup", "project", "-", "merge_requests", "123"}, + wantOwner: "group/subgroup", wantRepo: "project", + wantResource: forges.ResourceTypePR, wantNumber: 123, + }, + { + name: "issue", + parts: []string{"group", "project", "-", "issues", "456"}, + wantOwner: "group", wantRepo: "project", + wantResource: forges.ResourceTypeIssue, wantNumber: 456, + }, + { + name: "missing repo", + parts: []string{"group"}, + wantErr: true, + }, + { + name: "invalid MR number", + parts: []string{"group", "project", "-", "merge_requests", "abc"}, + wantErr: true, + }, + } + + f := &gitLabForge{} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ref, err := f.ParsePath(tt.parts) + if tt.wantErr { + if err == nil { + t.Fatal("expected error") + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if ref.Owner != tt.wantOwner { + t.Errorf("owner: got %q, want %q", ref.Owner, tt.wantOwner) + } + if ref.Repo != tt.wantRepo { + t.Errorf("repo: got %q, want %q", ref.Repo, tt.wantRepo) + } + if ref.Type != tt.wantResource { + t.Errorf("resource: got %q, want %q", ref.Type, tt.wantResource) + } + if ref.Number != tt.wantNumber { + t.Errorf("number: got %d, want %d", ref.Number, tt.wantNumber) + } + }) + } +} diff --git a/internal/cli/collaborator.go b/internal/cli/collaborator.go index ded07d8..dda2be7 100644 --- a/internal/cli/collaborator.go +++ b/internal/cli/collaborator.go @@ -4,7 +4,7 @@ import ( "fmt" "os" - forges "github.com/git-pkgs/forge" + "github.com/git-pkgs/forge" "github.com/git-pkgs/forge/internal/output" "github.com/git-pkgs/forge/internal/resolve" "github.com/spf13/cobra" diff --git a/internal/cli/label.go b/internal/cli/label.go index 94d5eaa..517ad9a 100644 --- a/internal/cli/label.go +++ b/internal/cli/label.go @@ -5,7 +5,7 @@ import ( "fmt" "os" - forges "github.com/git-pkgs/forge" + "github.com/git-pkgs/forge" "github.com/git-pkgs/forge/internal/output" "github.com/git-pkgs/forge/internal/resolve" "github.com/spf13/cobra" diff --git a/internal/cli/notification.go b/internal/cli/notification.go index 1f1f9c9..655f583 100644 --- a/internal/cli/notification.go +++ b/internal/cli/notification.go @@ -4,7 +4,7 @@ import ( "fmt" "os" - forges "github.com/git-pkgs/forge" + "github.com/git-pkgs/forge" "github.com/git-pkgs/forge/internal/output" "github.com/git-pkgs/forge/internal/resolve" "github.com/spf13/cobra" diff --git a/internal/cli/pr.go b/internal/cli/pr.go index 0a962be..31c7da7 100644 --- a/internal/cli/pr.go +++ b/internal/cli/pr.go @@ -8,7 +8,7 @@ import ( "strconv" "strings" - forges "github.com/git-pkgs/forge" + "github.com/git-pkgs/forge" "github.com/git-pkgs/forge/internal/output" "github.com/git-pkgs/forge/internal/resolve" "github.com/spf13/cobra" @@ -551,7 +551,7 @@ func prCheckoutCmd() *cobra.Command { ) cmd := &cobra.Command{ - Use: "checkout ", + Use: "checkout ", Short: "Check out a pull request locally", Long: `Check out a pull request's head branch locally. @@ -559,17 +559,39 @@ If the PR is from a fork, the fork repository is added as a remote (named after the fork owner by default), and the branch is fetched and checked out with upstream tracking configured. -For same-repo PRs, the branch is fetched and checked out.`, +For same-repo PRs, the branch is fetched and checked out. + +The argument can be a PR number or a full URL: + forge pr checkout 123 + forge pr checkout https://github.com/owner/repo/pull/123`, 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]) - } - - forge, owner, repoName, domain, err := resolve.Repo(flagRepo, flagForgeType) - if err != nil { - return err + var ( + forge forges.Forge + owner, repoName string + domain string + number int + err error + ) + + // Try parsing as a number first + if n, parseErr := strconv.Atoi(args[0]); parseErr == nil { + number = n + forge, owner, repoName, domain, err = resolve.Repo(flagRepo, flagForgeType) + if err != nil { + return err + } + } else { + // Try parsing as a URL + var ref *forges.ResourceRef + forge, domain, ref, err = resolve.ResourceFromURL(args[0]) + if err != nil { + return fmt.Errorf("invalid PR number or URL %q: %w", args[0], err) + } + if ref.Type != forges.ResourceTypePR { + return fmt.Errorf("URL does not point to a pull request: %s", args[0]) + } + owner, repoName, number = ref.Owner, ref.Repo, ref.Number } ctx := cmd.Context() diff --git a/internal/cli/pr_checkout_test.go b/internal/cli/pr_checkout_test.go index cf77a77..f3b37ac 100644 --- a/internal/cli/pr_checkout_test.go +++ b/internal/cli/pr_checkout_test.go @@ -9,7 +9,7 @@ import ( "strings" "testing" - forges "github.com/git-pkgs/forge" + "github.com/git-pkgs/forge" "github.com/git-pkgs/forge/internal/resolve" ) @@ -95,6 +95,15 @@ func (m *mockForge) GetRateLimit(_ context.Context) (*forges.RateLimit, error) { return nil, forges.ErrNotSupported } +func (m *mockForge) ParsePath(_ []string) (*forges.ResourceRef, error) { + return &forges.ResourceRef{ + Owner: "testowner", + Repo: "testrepo", + Type: forges.ResourceTypePR, + Number: 42, + }, nil +} + // setupTestRepo creates a temporary git repository with an initial commit // and an origin remote pointing to a fake URL. func setupTestRepo(t *testing.T, originURL string) string { @@ -368,6 +377,19 @@ func TestPRCheckout(t *testing.T) { setupOrigin: true, wantBranch: "my-local-branch", }, + { + name: "checkout by URL", + pr: &forges.PullRequest{ + Number: 42, + Head: forges.PRBranch{ + Ref: "feature-branch", + SHA: "abc123", + }, + }, + args: []string{"pr", "checkout", "https://github.com/testowner/testrepo/pull/42"}, + setupOrigin: true, + wantBranch: "feature-branch", + }, { name: "invalid PR number", args: []string{"pr", "checkout", "notanumber"}, diff --git a/internal/cli/review.go b/internal/cli/review.go index a6efbda..09b4b2b 100644 --- a/internal/cli/review.go +++ b/internal/cli/review.go @@ -5,7 +5,7 @@ import ( "os" "strconv" - forges "github.com/git-pkgs/forge" + "github.com/git-pkgs/forge" "github.com/git-pkgs/forge/internal/output" "github.com/git-pkgs/forge/internal/resolve" "github.com/spf13/cobra" diff --git a/internal/cli/root.go b/internal/cli/root.go index e2d3558..431be8e 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -5,7 +5,7 @@ import ( "fmt" "os" - forges "github.com/git-pkgs/forge" + "github.com/git-pkgs/forge" "github.com/git-pkgs/forge/internal/config" "github.com/git-pkgs/forge/internal/output" "github.com/git-pkgs/forge/internal/resolve" diff --git a/internal/cli/root_test.go b/internal/cli/root_test.go index 6e8d034..a97a803 100644 --- a/internal/cli/root_test.go +++ b/internal/cli/root_test.go @@ -4,7 +4,7 @@ import ( "errors" "testing" - forges "github.com/git-pkgs/forge" + "github.com/git-pkgs/forge" ) func TestNotSupportedWrapsError(t *testing.T) { diff --git a/internal/cli/status.go b/internal/cli/status.go index 0cdb37e..3469460 100644 --- a/internal/cli/status.go +++ b/internal/cli/status.go @@ -3,7 +3,7 @@ package cli import ( "fmt" - forges "github.com/git-pkgs/forge" + "github.com/git-pkgs/forge" "github.com/git-pkgs/forge/internal/output" "github.com/git-pkgs/forge/internal/resolve" "github.com/spf13/cobra" diff --git a/internal/resolve/resolve.go b/internal/resolve/resolve.go index 953262e..106d6c0 100644 --- a/internal/resolve/resolve.go +++ b/internal/resolve/resolve.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "net/http" + "net/url" "os" "os/exec" "strings" @@ -137,6 +138,42 @@ func repoFromGitRemote(_ string) (forges.Forge, string, string, string, error) { return f, owner, repo, domain, nil } +// ResourceFromURL resolves a forge and resource details from a full forge URL. +func ResourceFromURL(rawURL string) (forge forges.Forge, domain string, ref *forges.ResourceRef, err error) { + u, err := url.Parse(rawURL) + if err != nil { + return nil, "", nil, fmt.Errorf("invalid URL: %w", err) + } + if u.Scheme == "" { + u, err = url.Parse("https://" + rawURL) + if err != nil { + return nil, "", nil, fmt.Errorf("invalid URL: %w", err) + } + } + + domain = u.Hostname() + path := strings.Trim(u.Path, "/") + + var f forges.Forge + if testForge != nil { + f = testForge + } else { + client := newClient(domain) + f, err = forgeForDomainMaybeConfig(context.Background(), client, domain) + if err != nil { + return nil, "", nil, err + } + } + + parts := strings.Split(path, "/") + ref, err = f.ParsePath(parts) + if err != nil { + return nil, "", nil, err + } + + return f, domain, ref, nil +} + func resolveRemote() (domain, owner, repo string, err error) { url, err := gitRemoteURL(remoteName) if err != nil {