Merge remote-tracking branch 'origin/master' into tomasv-request-reviewers-2

.gitignore

@@ -11,3 +11,4 @@ target
 .vagrant
 tags
 /site
+/hub

Makefile

@@ -8,6 +8,7 @@ HELP_CMD = \
 	share/man/man1/hub-compare.1 \
 	share/man/man1/hub-create.1 \
 	share/man/man1/hub-fork.1 \
+	share/man/man1/hub-pr.1 \
 	share/man/man1/hub-pull-request.1 \
 	share/man/man1/hub-release.1 \
 	share/man/man1/hub-issue.1 \

commands/checkout.go

@@ -31,17 +31,10 @@ func init() {
 }
 
 func checkout(command *Command, args *Args) {
-	if !args.IsParamsEmpty() {
-		err := transformCheckoutArgs(args)
-		utils.Check(err)
-	}
-}
-
-func transformCheckoutArgs(args *Args) error {
 	words := args.Words()
 
 	if len(words) == 0 {
-		return nil
+		return
 	}
 
 	checkoutURL := words[0]
@@ -53,40 +46,42 @@ func transformCheckoutArgs(args *Args) error {
 	url, err := github.ParseURL(checkoutURL)
 	if err != nil {
 		// not a valid GitHub URL
-		return nil
+		return
 	}
 
 	pullURLRegex := regexp.MustCompile("^pull/(\\d+)")
 	projectPath := url.ProjectPath()
 	if !pullURLRegex.MatchString(projectPath) {
 		// not a valid PR URL
-		return nil
+		return
 	}
 
 	err = sanitizeCheckoutFlags(args)
-	if err != nil {
-		return err
-	}
+	utils.Check(err)
 
 	id := pullURLRegex.FindStringSubmatch(projectPath)[1]
 	gh := github.NewClient(url.Project.Host)
 	pullRequest, err := gh.PullRequest(url.Project, id)
-	if err != nil {
-		return err
-	}
+	utils.Check(err)
+
+	newArgs, err := transformCheckoutArgs(args, pullRequest, newBranchName)
+	utils.Check(err)
 
 	if idx := args.IndexOfParam(newBranchName); idx >= 0 {
 		args.RemoveParam(idx)
 	}
+	replaceCheckoutParam(args, checkoutURL, newArgs...)
+}
 
+func transformCheckoutArgs(args *Args, pullRequest *github.PullRequest, newBranchName string) (newArgs []string, err error) {
 	repo, err := github.LocalRepo()
 	if err != nil {
-		return err
+		return
 	}
 
 	baseRemote, err := repo.RemoteForRepo(pullRequest.Base.Repo)
 	if err != nil {
-		return err
+		return
 	}
 
 	var headRemote *github.Remote
@@ -96,8 +91,6 @@ func transformCheckoutArgs(args *Args) error {
 		headRemote, _ = repo.RemoteForRepo(pullRequest.Head.Repo)
 	}
 
-	var newArgs []string
-
 	if headRemote != nil {
 		if newBranchName == "" {
 			newBranchName = pullRequest.Head.Ref
@@ -114,22 +107,23 @@ func transformCheckoutArgs(args *Args) error {
 	} else {
 		if newBranchName == "" {
 			if pullRequest.Head.Repo == nil {
-				newBranchName = fmt.Sprintf("pr-%s", id)
+				newBranchName = fmt.Sprintf("pr-%d", pullRequest.Number)
 			} else {
 				newBranchName = fmt.Sprintf("%s-%s", pullRequest.Head.Repo.Owner.Login, pullRequest.Head.Ref)
 			}
 		}
 		newArgs = append(newArgs, newBranchName)
 
-		ref := fmt.Sprintf("refs/pull/%s/head", id)
+		ref := fmt.Sprintf("refs/pull/%d/head", pullRequest.Number)
 		args.Before("git", "fetch", baseRemote.Name, fmt.Sprintf("%s:%s", ref, newBranchName))
 
 		remote := baseRemote.Name
 		mergeRef := ref
 		if pullRequest.MaintainerCanModify && pullRequest.Head.Repo != nil {
-			project, err := github.NewProjectFromRepo(pullRequest.Head.Repo)
+			var project *github.Project
+			project, err = github.NewProjectFromRepo(pullRequest.Head.Repo)
 			if err != nil {
-				return err
+				return
 			}
 
 			remote = project.GitURL("", "", true)
@@ -138,8 +132,7 @@ func transformCheckoutArgs(args *Args) error {
 		args.Before("git", "config", fmt.Sprintf("branch.%s.remote", newBranchName), remote)
 		args.Before("git", "config", fmt.Sprintf("branch.%s.merge", newBranchName), mergeRef)
 	}
-	replaceCheckoutParam(args, checkoutURL, newArgs...)
-	return nil
+	return
 }
 
 func sanitizeCheckoutFlags(args *Args) error {

commands/checkout_test.go

-package commands
-
-import (
-	"testing"
-
-	"github.com/bmizerany/assert"
-)
-
-func TestReplaceCheckoutParam(t *testing.T) {
-	checkoutURL := "https://github.com/github/hub/pull/12"
-	args := NewArgs([]string{"checkout", checkoutURL})
-	replaceCheckoutParam(args, checkoutURL, "--track", "-B", "jingweno", "origin/master")
-
-	cmd := args.ToCmd()
-	assert.Equal(t, "git checkout --track -B jingweno origin/master", cmd.String())
-}
-
-func TestTransformCheckoutArgs(t *testing.T) {
-	args := NewArgs([]string{"checkout", "-b", "https://github.com/github/hub/pull/12"})
-	err := transformCheckoutArgs(args)
-
-	assert.Equal(t, "Unsupported flag -b when checking out pull request", err.Error())
-
-	args = NewArgs([]string{"checkout", "--orphan", "https://github.com/github/hub/pull/12"})
-	err = transformCheckoutArgs(args)
-
-	assert.Equal(t, "Unsupported flag --orphan when checking out pull request", err.Error())
-}

commands/help.go

@@ -166,6 +166,7 @@ var helpText = `
 These GitHub commands are provided by hub:
 
    pull-request   Open a pull request on GitHub
+   pr             Work with pull requests
    fork           Make a fork of a remote repository on GitHub and add as remote
    create         Create this repository on GitHub and add GitHub as origin
    browse         Open a GitHub page in the default browser

commands/pr.go

+package commands
+
+import (
+	"fmt"
+	"os"
+	"strconv"
+
+	"github.com/github/hub/github"
+	"github.com/github/hub/utils"
+)
+
+var (
+	cmdPr = &Command{
+		Run:   printHelp,
+		Usage: "pr checkout <PULLREQ-NUMBER> [<BRANCH>]",
+		Long: `Check out the head of a pull request as a local branch.
+
+## Examples:
+	$ hub pr checkout 73
+	> git fetch origin pull/73/head:jingweno-feature
+	> git checkout jingweno-feature
+
+## See also:
+
+hub-merge(1), hub(1), hub-checkout(1)
+	`,
+	}
+
+	cmdCheckoutPr = &Command{
+		Key: "checkout",
+		Run: checkoutPr,
+	}
+)
+
+func init() {
+	cmdPr.Use(cmdCheckoutPr)
+	CmdRunner.Use(cmdPr)
+}
+
+func printHelp(command *Command, args *Args) {
+	fmt.Print(command.HelpText())
+	os.Exit(0)
+}
+
+func checkoutPr(command *Command, args *Args) {
+	words := args.Words()
+	var newBranchName string
+
+	if len(words) == 0 {
+		utils.Check(fmt.Errorf("Error: No pull request number given"))
+	} else if len(words) > 1 {
+		newBranchName = words[1]
+	}
+
+	prNumberString := words[0]
+	_, err := strconv.Atoi(prNumberString)
+	utils.Check(err)
+
+	// Figure out the PR URL
+	localRepo, err := github.LocalRepo()
+	utils.Check(err)
+	baseProject, err := localRepo.MainProject()
+	utils.Check(err)
+	host, err := github.CurrentConfig().PromptForHost(baseProject.Host)
+	utils.Check(err)
+	client := github.NewClientWithHost(host)
+	pr, err := client.PullRequest(baseProject, prNumberString)
+	utils.Check(err)
+
+	newArgs, err := transformCheckoutArgs(args, pr, newBranchName)
+	utils.Check(err)
+
+	args.Replace(args.Executable, "checkout", newArgs...)
+}

features/checkout.feature

@@ -12,7 +12,7 @@ Feature: hub checkout <PULLREQ-URL>
       """
       get('/repos/mojombo/jekyll/pulls/77') {
         halt 415 unless request.accept?('application/vnd.github.v3+json')
-        json :head => {
+        json :number => 77, :head => {
           :ref => "fixes",
           :repo => {
             :owner => { :login => "mislav" },
@@ -37,7 +37,7 @@ Feature: hub checkout <PULLREQ-URL>
     Given the GitHub API server:
       """
       get('/repos/mislav/jekyll/pulls/77') {
-        json :base => {
+        json :number => 77, :base => {
           :repo => {
             :name => 'jekyll',
             :html_url => 'https://github.com/mislav/jekyll',
@@ -57,7 +57,7 @@ Feature: hub checkout <PULLREQ-URL>
     Given the GitHub API server:
       """
       get('/repos/mojombo/jekyll/pulls/77') {
-        json :head => {
+        json :number => 77, :head => {
           :ref => "fixes",
           :repo => {
             :name => "jekyll",
@@ -81,7 +81,7 @@ Feature: hub checkout <PULLREQ-URL>
     Given the GitHub API server:
       """
       get('/repos/mojombo/jekyll/pulls/77') {
-        json :head => {
+        json :number => 77, :head => {
           :ref => "fixes",
           :repo => {
             :name => "jekyll",
@@ -104,7 +104,7 @@ Feature: hub checkout <PULLREQ-URL>
     Given the GitHub API server:
       """
       get('/repos/mojombo/jekyll/pulls/77') {
-        json :head => {
+        json :number => 77, :head => {
           :ref => "fixes",
           :repo => {
             :name => "jekyll",
@@ -127,7 +127,7 @@ Feature: hub checkout <PULLREQ-URL>
     Given the GitHub API server:
       """
       get('/repos/mojombo/jekyll/pulls/77') {
-        json :head => {
+        json :number => 77, :head => {
           :ref => "fixes",
           :repo => nil
         }, :base => {
@@ -148,7 +148,7 @@ Feature: hub checkout <PULLREQ-URL>
     Given the GitHub API server:
       """
       get('/repos/mojombo/jekyll/pulls/77') {
-        json :head => {
+        json :number => 77, :head => {
           :ref => "fixes",
           :repo => {
             :owner => { :login => "mislav" },
@@ -173,7 +173,7 @@ Feature: hub checkout <PULLREQ-URL>
     Given the GitHub API server:
       """
       get('/repos/mojombo/jekyll/pulls/77') {
-        json :head => {
+        json :number => 77, :head => {
           :ref => "fixes",
           :repo => {
             :owner => { :login => "mislav" },
@@ -200,7 +200,7 @@ Feature: hub checkout <PULLREQ-URL>
     Given the GitHub API server:
       """
       get('/repos/mojombo/jekyll/pulls/77') {
-        json :head => {
+        json :number => 77, :head => {
           :ref => "fixes",
           :repo => {
             :owner => { :login => "mislav" },
@@ -226,7 +226,7 @@ Feature: hub checkout <PULLREQ-URL>
     Given the GitHub API server:
       """
       get('/repos/mojombo/jekyll/pulls/77') {
-        json :head => {
+        json :number => 77, :head => {
           :ref => "fixes",
           :repo => {
             :owner => { :login => "mislav" },

features/pr.feature

+Feature: hub pr checkout <PULLREQ-NUMBER>
+  Background:
+    Given I am in "git://github.com/mojombo/jekyll.git" git repo
+    And I am "mojombo" on github.com with OAuth token "OTOKEN"
+
+  Scenario: Checkout a pull request
+    Given the GitHub API server:
+      """
+      get('/repos/mojombo/jekyll/pulls/77') {
+        json :number => 77, :head => {
+          :ref => "fixes",
+          :repo => {
+            :owner => { :login => "mislav" },
+            :name => "jekyll",
+            :private => false
+          }
+        }, :base => {
+          :repo => {
+            :name => 'jekyll',
+            :html_url => 'https://github.com/mojombo/jekyll',
+            :owner => { :login => "mojombo" },
+          }
+        },
+        :maintainer_can_modify => false,
+        :html_url => 'https://github.com/mojombo/jekyll/pull/77'
+      }
+      """
+    When I run `hub pr checkout 77`
+    Then "git fetch origin refs/pull/77/head:mislav-fixes" should be run
+    And "git checkout mislav-fixes" should be run
+    And "mislav-fixes" should merge "refs/pull/77/head" from remote "origin"
+
+  Scenario: Custom name for new branch
+    Given the GitHub API server:
+      """
+      get('/repos/mojombo/jekyll/pulls/77') {
+        json :number => 77, :head => {
+          :ref => "fixes",
+          :repo => {
+            :name => "jekyll",
+            :owner => { :login => "mislav" },
+          }
+        }, :base => {
+          :repo => {
+            :name => 'jekyll',
+            :html_url => 'https://github.com/mojombo/jekyll',
+            :owner => { :login => "mojombo" },
+          }
+        },
+        :maintainer_can_modify => false,
+        :html_url => 'https://github.com/mojombo/jekyll/pull/77'
+      }
+      """
+    When I run `hub pr checkout 77 fixes-from-mislav`
+    Then "git fetch origin refs/pull/77/head:fixes-from-mislav" should be run
+    And "git checkout fixes-from-mislav" should be run
+    And "fixes-from-mislav" should merge "refs/pull/77/head" from remote "origin"
+
+  Scenario: Same-repo
+    Given the GitHub API server:
+      """
+      get('/repos/mojombo/jekyll/pulls/77') {
+        json :number => 77, :head => {
+          :ref => "fixes",
+          :repo => {
+            :name => "jekyll",
+            :owner => { :login => "mojombo" },
+          }
+        }, :base => {
+          :repo => {
+            :name => "jekyll",
+            :html_url => "https://github.com/mojombo/jekyll",
+            :owner => { :login => "mojombo" },
+          }
+        },
+        :html_url => 'https://github.com/mojombo/jekyll/pull/77'
+      }
+      """
+    When I run `hub pr checkout 77`
+    Then "git fetch origin +refs/heads/fixes:refs/remotes/origin/fixes" should be run
+    And "git checkout -b fixes --track origin/fixes" should be run

github/client.go

@@ -91,13 +91,13 @@ func (client *Client) PullRequestPatch(project *Project, id string) (patch io.Re
 }
 
 type PullRequest struct {
-	ApiUrl    	    string           `json:"url"`
-	Number    	    int              `json:"number"`
-	HtmlUrl   	    string           `json:"html_url"`
-	Title     	    string           `json:"title"`
+	ApiUrl              string           `json:"url"`
+	Number              int              `json:"number"`
+	HtmlUrl             string           `json:"html_url"`
+	Title               string           `json:"title"`
 	MaintainerCanModify bool             `json:"maintainer_can_modify"`
-	Head      	    *PullRequestSpec `json:"head"`
-	Base      	    *PullRequestSpec `json:"base"`
+	Head                *PullRequestSpec `json:"head"`
+	Base                *PullRequestSpec `json:"base"`
 }
 
 type PullRequestSpec struct {

github/http.go

@@ -159,6 +159,7 @@ func newHttpClient(testHost string, verbose bool) *http.Client {
 			if len(via) > 2 {
 				return fmt.Errorf("too many redirects")
 			} else {
+				// replicate rudimentary functionality of copyHeaders from Go 1.8
 				for key, vals := range via[0].Header {
 					lkey := strings.ToLower(key)
 					if !strings.HasPrefix(lkey, "x-original-") && via[0].Host == req.URL.Host || lkey != "authorization" {
@@ -248,6 +249,7 @@ func (c *simpleClient) performRequestUrl(method string, url *url.URL, body io.Re
 	}
 
 	res = &simpleResponse{httpResponse}
+	// TODO: remove after upgrade to Go 1.8
 	if res.StatusCode == 307 && redirectsRemaining > 0 {
 		url, err = url.Parse(res.Header.Get("Location"))
 		if err != nil || url.Host != req.URL.Host || url.Scheme != req.URL.Scheme {