diff --git a/api/queries_pr.go b/api/queries_pr.go index be86dc679..b2e5f2f77 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -340,7 +340,7 @@ func PullRequestByNumber(client *Client, repo ghrepo.Interface, number int) (*Pu return &resp.Repository.PullRequest, nil } -func PullRequestForBranch(client *Client, repo ghrepo.Interface, branch string) (*PullRequest, error) { +func PullRequestForBranch(client *Client, repo ghrepo.Interface, baseBranch, headBranch string) (*PullRequest, error) { type response struct { Repository struct { PullRequests struct { @@ -376,9 +376,9 @@ func PullRequestForBranch(client *Client, repo ghrepo.Interface, branch string) } }` - branchWithoutOwner := branch - if idx := strings.Index(branch, ":"); idx >= 0 { - branchWithoutOwner = branch[idx+1:] + branchWithoutOwner := headBranch + if idx := strings.Index(headBranch, ":"); idx >= 0 { + branchWithoutOwner = headBranch[idx+1:] } variables := map[string]interface{}{ @@ -394,12 +394,17 @@ func PullRequestForBranch(client *Client, repo ghrepo.Interface, branch string) } for _, pr := range resp.Repository.PullRequests.Nodes { - if pr.HeadLabel() == branch { + if pr.HeadLabel() == headBranch { + if baseBranch != "" { + if pr.BaseRefName != baseBranch { + continue + } + } return &pr, nil } } - return nil, &NotFoundError{fmt.Errorf("no open pull requests found for branch %q", branch)} + return nil, &NotFoundError{fmt.Errorf("no open pull requests found for branch %q", headBranch)} } // CreatePullRequest creates a pull request in a GitHub repository diff --git a/api/queries_repo.go b/api/queries_repo.go index 048b72b50..ea2862500 100644 --- a/api/queries_repo.go +++ b/api/queries_repo.go @@ -154,7 +154,7 @@ func RepoNetwork(client *Client, repos []ghrepo.Interface) (RepoNetworkResult, e keys = append(keys, key) } // sort keys to ensure `repo_{N}` entries are processed in order - sort.Sort(sort.StringSlice(keys)) + sort.Strings(keys) // Iterate over keys of GraphQL response data and, based on its name, // dynamically allocate the target struct an individual message gets decoded to. diff --git a/command/pr.go b/command/pr.go index aa22464f4..03a9a5627 100644 --- a/command/pr.go +++ b/command/pr.go @@ -255,9 +255,21 @@ func prView(cmd *cobra.Command, args []string) error { return err } - baseRepo, err := determineBaseRepo(cmd, ctx) - if err != nil { - return err + var baseRepo ghrepo.Interface + var prArg string + if len(args) > 0 { + prArg = args[0] + if prNum, repo := prFromURL(prArg); repo != nil { + prArg = prNum + baseRepo = repo + } + } + + if baseRepo == nil { + baseRepo, err = determineBaseRepo(cmd, ctx) + if err != nil { + return err + } } preview, err := cmd.Flags().GetBool("preview") @@ -268,7 +280,7 @@ func prView(cmd *cobra.Command, args []string) error { var openURL string var pr *api.PullRequest if len(args) > 0 { - pr, err = prFromArg(apiClient, baseRepo, args[0]) + pr, err = prFromArg(apiClient, baseRepo, prArg) if err != nil { return err } @@ -288,7 +300,7 @@ func prView(cmd *cobra.Command, args []string) error { } } } else { - pr, err = api.PullRequestForBranch(apiClient, baseRepo, branchWithOwner) + pr, err = api.PullRequestForBranch(apiClient, baseRepo, "", branchWithOwner) if err != nil { return err } @@ -331,17 +343,19 @@ func printPrPreview(out io.Writer, pr *api.PullRequest) error { var prURLRE = regexp.MustCompile(`^https://github\.com/([^/]+)/([^/]+)/pull/(\d+)`) +func prFromURL(arg string) (string, ghrepo.Interface) { + if m := prURLRE.FindStringSubmatch(arg); m != nil { + return m[3], ghrepo.New(m[1], m[2]) + } + return "", nil +} + func prFromArg(apiClient *api.Client, baseRepo ghrepo.Interface, arg string) (*api.PullRequest, error) { if prNumber, err := strconv.Atoi(strings.TrimPrefix(arg, "#")); err == nil { return api.PullRequestByNumber(apiClient, baseRepo, prNumber) } - if m := prURLRE.FindStringSubmatch(arg); m != nil { - prNumber, _ := strconv.Atoi(m[3]) - return api.PullRequestByNumber(apiClient, baseRepo, prNumber) - } - - return api.PullRequestForBranch(apiClient, baseRepo, arg) + return api.PullRequestForBranch(apiClient, baseRepo, "", arg) } func prSelectorForCurrentBranch(ctx context.Context, baseRepo ghrepo.Interface) (prNumber int, prHeadRef string, err error) { diff --git a/command/pr_checkout.go b/command/pr_checkout.go index 08a37033d..336a5a8d1 100644 --- a/command/pr_checkout.go +++ b/command/pr_checkout.go @@ -7,6 +7,7 @@ import ( "os/exec" "github.com/cli/cli/git" + "github.com/cli/cli/internal/ghrepo" "github.com/cli/cli/utils" "github.com/spf13/cobra" ) @@ -18,21 +19,38 @@ func prCheckout(cmd *cobra.Command, args []string) error { if err != nil { return err } - // FIXME: duplicates logic from fsContext.BaseRepo - baseRemote, err := remotes.FindByName("upstream", "github", "origin", "*") - if err != nil { - return err - } + apiClient, err := apiClientForContext(ctx) if err != nil { return err } - pr, err := prFromArg(apiClient, baseRemote, args[0]) + var baseRepo ghrepo.Interface + prArg := args[0] + if prNum, repo := prFromURL(prArg); repo != nil { + prArg = prNum + baseRepo = repo + } + + if baseRepo == nil { + baseRepo, err = determineBaseRepo(cmd, ctx) + if err != nil { + return err + } + } + + pr, err := prFromArg(apiClient, baseRepo, prArg) if err != nil { return err } + baseRemote, _ := remotes.FindByRepo(baseRepo.RepoOwner(), baseRepo.RepoName()) + // baseRemoteSpec is a repository URL or a remote name to be used in git fetch + baseURLOrName := fmt.Sprintf("https://github.com/%s.git", ghrepo.FullName(baseRepo)) + if baseRemote != nil { + baseURLOrName = baseRemote.Name + } + headRemote := baseRemote if pr.IsCrossRepository { headRemote, _ = remotes.FindByRepo(pr.HeadRepositoryOwner.Login, pr.HeadRepository.Name) @@ -67,15 +85,15 @@ func prCheckout(cmd *cobra.Command, args []string) error { ref := fmt.Sprintf("refs/pull/%d/head", pr.Number) if newBranchName == currentBranch { // PR head matches currently checked out branch - cmdQueue = append(cmdQueue, []string{"git", "fetch", baseRemote.Name, ref}) + cmdQueue = append(cmdQueue, []string{"git", "fetch", baseURLOrName, ref}) cmdQueue = append(cmdQueue, []string{"git", "merge", "--ff-only", "FETCH_HEAD"}) } else { // create a new branch - cmdQueue = append(cmdQueue, []string{"git", "fetch", baseRemote.Name, fmt.Sprintf("%s:%s", ref, newBranchName)}) + cmdQueue = append(cmdQueue, []string{"git", "fetch", baseURLOrName, fmt.Sprintf("%s:%s", ref, newBranchName)}) cmdQueue = append(cmdQueue, []string{"git", "checkout", newBranchName}) } - remote := baseRemote.Name + remote := baseURLOrName mergeRef := ref if pr.MaintainerCanModify { remote = fmt.Sprintf("https://github.com/%s/%s.git", pr.HeadRepositoryOwner.Login, pr.HeadRepository.Name) diff --git a/command/pr_checkout_test.go b/command/pr_checkout_test.go index ce0266c5e..1a36756a7 100644 --- a/command/pr_checkout_test.go +++ b/command/pr_checkout_test.go @@ -2,6 +2,8 @@ package command import ( "bytes" + "encoding/json" + "io/ioutil" "os/exec" "strings" "testing" @@ -21,6 +23,7 @@ func TestPRCheckout_sameRepo(t *testing.T) { return ctx } http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { "pullRequest": { @@ -112,6 +115,68 @@ func TestPRCheckout_urlArg(t *testing.T) { eq(t, strings.Join(ranCommands[1], " "), "git checkout -b feature --no-track origin/feature") } +func TestPRCheckout_urlArg_differentBase(t *testing.T) { + ctx := context.NewBlank() + ctx.SetBranch("master") + ctx.SetRemotes(map[string]string{ + "origin": "OWNER/REPO", + }) + initContext = func() context.Context { + return ctx + } + http := initFakeHTTP() + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { "pullRequest": { + "number": 123, + "headRefName": "feature", + "headRepositoryOwner": { + "login": "hubot" + }, + "headRepository": { + "name": "POE", + "defaultBranchRef": { + "name": "master" + } + }, + "isCrossRepository": false, + "maintainerCanModify": false + } } } } + `)) + + ranCommands := [][]string{} + restoreCmd := utils.SetPrepareCmd(func(cmd *exec.Cmd) utils.Runnable { + switch strings.Join(cmd.Args, " ") { + case "git show-ref --verify --quiet refs/heads/feature": + return &errorStub{"exit status: 1"} + default: + ranCommands = append(ranCommands, cmd.Args) + return &test.OutputStub{} + } + }) + defer restoreCmd() + + output, err := RunCommand(prCheckoutCmd, `pr checkout https://github.com/OTHER/POE/pull/123/files`) + eq(t, err, nil) + eq(t, output.String(), "") + + bodyBytes, _ := ioutil.ReadAll(http.Requests[0].Body) + reqBody := struct { + Variables struct { + Owner string + Repo string + } + }{} + json.Unmarshal(bodyBytes, &reqBody) + + eq(t, reqBody.Variables.Owner, "OTHER") + eq(t, reqBody.Variables.Repo, "POE") + + eq(t, len(ranCommands), 5) + eq(t, strings.Join(ranCommands[1], " "), "git fetch https://github.com/OTHER/POE.git refs/pull/123/head:feature") + eq(t, strings.Join(ranCommands[3], " "), "git config branch.feature.remote https://github.com/OTHER/POE.git") +} + func TestPRCheckout_branchArg(t *testing.T) { ctx := context.NewBlank() ctx.SetBranch("master") @@ -122,6 +187,7 @@ func TestPRCheckout_branchArg(t *testing.T) { return ctx } http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { "pullRequests": { "nodes": [ @@ -171,6 +237,7 @@ func TestPRCheckout_existingBranch(t *testing.T) { return ctx } http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { "pullRequest": { @@ -223,6 +290,7 @@ func TestPRCheckout_differentRepo_remoteExists(t *testing.T) { return ctx } http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { "pullRequest": { @@ -275,6 +343,7 @@ func TestPRCheckout_differentRepo(t *testing.T) { return ctx } http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { "pullRequest": { @@ -327,6 +396,7 @@ func TestPRCheckout_differentRepo_existingBranch(t *testing.T) { return ctx } http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { "pullRequest": { @@ -377,6 +447,7 @@ func TestPRCheckout_differentRepo_currentBranch(t *testing.T) { return ctx } http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { "pullRequest": { @@ -427,6 +498,7 @@ func TestPRCheckout_maintainerCanModify(t *testing.T) { return ctx } http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { "pullRequest": { diff --git a/command/pr_create.go b/command/pr_create.go index ca7800a19..2be922987 100644 --- a/command/pr_create.go +++ b/command/pr_create.go @@ -132,13 +132,13 @@ func prCreate(cmd *cobra.Command, _ []string) error { if headRepo != nil && !ghrepo.IsSame(baseRepo, headRepo) { headBranchLabel = fmt.Sprintf("%s:%s", headRepo.RepoOwner(), headBranch) } - existingPR, err := api.PullRequestForBranch(client, baseRepo, headBranchLabel) + existingPR, err := api.PullRequestForBranch(client, baseRepo, baseBranch, headBranchLabel) var notFound *api.NotFoundError if err != nil && !errors.As(err, ¬Found) { return fmt.Errorf("error checking for existing pull request: %w", err) } if err == nil { - return fmt.Errorf("a pull request for branch %q already exists:\n%s", headBranchLabel, existingPR.URL) + return fmt.Errorf("a pull request for branch %q into branch %q already exists:\n%s", headBranchLabel, baseBranch, existingPR.URL) } } diff --git a/command/pr_create_test.go b/command/pr_create_test.go index 688dfd9c8..874b02a86 100644 --- a/command/pr_create_test.go +++ b/command/pr_create_test.go @@ -64,7 +64,8 @@ func TestPRCreate_alreadyExists(t *testing.T) { http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { "pullRequests": { "nodes": [ { "url": "https://github.com/OWNER/REPO/pull/123", - "headRefName": "feature" } + "headRefName": "feature", + "baseRefName": "master" } ] } } } } `)) @@ -78,11 +79,37 @@ func TestPRCreate_alreadyExists(t *testing.T) { if err == nil { t.Fatal("error expected, got nil") } - if err.Error() != "a pull request for branch \"feature\" already exists:\nhttps://github.com/OWNER/REPO/pull/123" { + if err.Error() != "a pull request for branch \"feature\" into branch \"master\" already exists:\nhttps://github.com/OWNER/REPO/pull/123" { t.Errorf("got error %q", err) } } +func TestPRCreate_alreadyExistsDifferentBase(t *testing.T) { + initBlankContext("OWNER/REPO", "feature") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { "pullRequests": { "nodes": [ + { "url": "https://github.com/OWNER/REPO/pull/123", + "headRefName": "feature", + "baseRefName": "master" } + ] } } } } + `)) + http.StubResponse(200, bytes.NewBufferString("{}")) + + cs, cmdTeardown := initCmdStubber() + defer cmdTeardown() + + cs.Stub("") // git status + cs.Stub("1234567890,commit 0\n2345678901,commit 1") // git log + cs.Stub("") // git rev-parse + + _, err := RunCommand(prCreateCmd, `pr create -BanotherBase -t"cool" -b"nah"`) + if err != nil { + t.Errorf("got unexpected error %q", err) + } +} + func TestPRCreate_web(t *testing.T) { initBlankContext("OWNER/REPO", "feature") http := initFakeHTTP() diff --git a/command/pr_test.go b/command/pr_test.go index 9fda8980c..505aff77d 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -635,7 +635,6 @@ func TestPRView_numberArgWithHash(t *testing.T) { func TestPRView_urlArg(t *testing.T) { initBlankContext("OWNER/REPO", "master") http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { "pullRequest": { diff --git a/command/repo.go b/command/repo.go index 1dc9b13a5..be930c0c8 100644 --- a/command/repo.go +++ b/command/repo.go @@ -295,7 +295,7 @@ func repoFork(cmd *cobra.Command, args []string) error { greenCheck := utils.Green("✓") out := colorableOut(cmd) - s := utils.Spinner() + s := utils.Spinner(out) loading := utils.Gray("Forking ") + utils.Bold(utils.Gray(ghrepo.FullName(toFork))) + utils.Gray("...") s.Suffix = " " + loading s.FinalMSG = utils.Gray(fmt.Sprintf("- %s\n", loading)) diff --git a/go.mod b/go.mod index 63a39c1bf..ee45df4ed 100644 --- a/go.mod +++ b/go.mod @@ -5,7 +5,7 @@ go 1.13 require ( github.com/AlecAivazis/survey/v2 v2.0.7 github.com/briandowns/spinner v1.9.0 - github.com/charmbracelet/glamour v0.1.1-0.20200304134224-7e5c90143acc + github.com/charmbracelet/glamour v0.1.1-0.20200320173916-301d3bcf3058 github.com/dlclark/regexp2 v1.2.0 // indirect github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 github.com/hashicorp/go-version v1.2.0 diff --git a/go.sum b/go.sum index 8d7665e96..b94236621 100644 --- a/go.sum +++ b/go.sum @@ -23,8 +23,8 @@ github.com/beorn7/perks v1.0.0/go.mod h1:KWe93zE9D1o94FZ5RNwFwVgaQK1VOXiVxmqh+Ce github.com/briandowns/spinner v1.9.0 h1:+OMAisemaHar1hjuJ3Z2hIvNhQl9Y7GLPWUwwz2Pxo8= github.com/briandowns/spinner v1.9.0/go.mod h1://Zf9tMcxfRUA36V23M6YGEAv+kECGfvpnLTnb8n4XQ= github.com/cespare/xxhash v1.1.0/go.mod h1:XrSqR1VqqWfGrhpAt58auRo0WTKS1nRRg3ghfAqPWnc= -github.com/charmbracelet/glamour v0.1.1-0.20200304134224-7e5c90143acc h1:WSZ06evYgfSZYqkcv0+jFVSVCJeYsIvU+J4ILZUrha4= -github.com/charmbracelet/glamour v0.1.1-0.20200304134224-7e5c90143acc/go.mod h1:sC1EP6T+3nFnl5vwf0TYEs1inMigQxZ7n912YKoxJow= +github.com/charmbracelet/glamour v0.1.1-0.20200320173916-301d3bcf3058 h1:Ks+RZ6s6UriHnL+yusm3OoaLwpV9WPvMV+FXQ6qMD7M= +github.com/charmbracelet/glamour v0.1.1-0.20200320173916-301d3bcf3058/go.mod h1:sC1EP6T+3nFnl5vwf0TYEs1inMigQxZ7n912YKoxJow= github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDkc90ppPyw= github.com/coreos/bbolt v1.3.2/go.mod h1:iRUV2dpdMOn7Bo10OQBFzIJO9kkE559Wcmn+qkEiiKk= github.com/coreos/etcd v3.3.10+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE= diff --git a/pkg/githubtemplate/github_template.go b/pkg/githubtemplate/github_template.go index f60878725..3aac35554 100644 --- a/pkg/githubtemplate/github_template.go +++ b/pkg/githubtemplate/github_template.go @@ -59,7 +59,7 @@ mainLoop: } } - sort.Sort(sort.StringSlice(results)) + sort.Strings(results) return results } diff --git a/utils/utils.go b/utils/utils.go index 6217871b7..77c643eba 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -2,6 +2,7 @@ package utils import ( "fmt" + "io" "strings" "time" @@ -68,6 +69,8 @@ func Humanize(s string) string { return strings.Map(h, s) } -func Spinner() *spinner.Spinner { - return spinner.New(spinner.CharSets[11], 400*time.Millisecond) +func Spinner(w io.Writer) *spinner.Spinner { + s := spinner.New(spinner.CharSets[11], 400*time.Millisecond) + s.Writer = w + return s }