Respect base repository when passing URL argument to pr checkout/view
Previously, the repository owner+name component of the URL was ignored and only the pull request number was read. Now, the URL dictates which base repository will be used.
This commit is contained in:
Mislav Marohnić
parent
2660561ed9
commit
9786498a25
3 changed files with 122 additions and 18 deletions
|
|
@ -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)
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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": {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue