diff --git a/command/pr.go b/command/pr.go index bd7743f46..7ddaca56c 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 } @@ -331,16 +343,18 @@ 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) } 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": {