[pull-request] Improve push target detection for `push.default=upstream`

When `git config push.default` is "upstream" or "tracking", but the
current branch is pushed to a remote without having upstream
configuration set up (for example, via `git push <REMOTE> HEAD` without
`-u`), this change makes it so that the remote tracking branch (the push
target) is still discovered via the same mechanism as if `push.default`
wasn't set (i.e. iterating through all the remotes).

features/pull_request.feature

@@ -947,11 +947,12 @@ Feature: hub pull-request
     Then the output should contain exactly "the://url\n"
 
   Scenario: Current branch is pushed to remote without upstream configuration
-    Given git "push.default" is set to "upstream"
+    Given the "upstream" remote has url "git://github.com/lestephane/coral.git"
     And I am on the "feature" branch pushed to "origin/feature"
+    And git "push.default" is set to "upstream"
     Given the GitHub API server:
       """
-      post('/repos/mislav/coral/pulls') {
+      post('/repos/lestephane/coral/pulls') {
         assert :base  => 'master',
                :head  => 'mislav:feature'
         status 201

github/branch.go

@@ -23,45 +23,6 @@ func (b *Branch) LongName() string {
 	return reg.ReplaceAllString(b.Name, "")
 }
 
-// see also RemoteForBranch()
-func (b *Branch) PushTarget(owner string, preferUpstream bool) (branch *Branch) {
-	var err error
-	pushDefault, _ := git.Config("push.default")
-	if pushDefault == "upstream" || pushDefault == "tracking" {
-		branch, err = b.Upstream()
-		if err != nil {
-			return
-		}
-	} else {
-		shortName := b.ShortName()
-		remotes := b.Repo.remotesForPublish(owner)
-
-		var remotesInOrder []Remote
-		if preferUpstream {
-			// reverse the remote lookup order
-			// see OriginNamesInLookupOrder
-			for i := len(remotes) - 1; i >= 0; i-- {
-				remotesInOrder = append(remotesInOrder, remotes[i])
-			}
-		} else {
-			remotesInOrder = remotes
-		}
-
-		for _, remote := range remotesInOrder {
-			if _, err := remote.Project(); err != nil {
-				continue
-			}
-			if git.HasFile("refs", "remotes", remote.Name, shortName) {
-				name := fmt.Sprintf("refs/remotes/%s/%s", remote.Name, shortName)
-				branch = &Branch{b.Repo, name}
-				break
-			}
-		}
-	}
-
-	return
-}
-
 func (b *Branch) RemoteName() string {
 	reg := regexp.MustCompile("^refs/remotes/([^/]+)")
 	if reg.MatchString(b.Name) {

github/localrepo.go

@@ -140,23 +140,55 @@ func (r *GitHubRepo) RemoteBranchAndProject(owner string, preferUpstream bool) (
 		return
 	}
 
-	if project != nil {
-		branch = branch.PushTarget(owner, preferUpstream)
-		if branch != nil && branch.IsRemote() {
-			remote, e := r.RemoteByName(branch.RemoteName())
+	if project == nil {
+		return
+	}
+
+	pushDefault, _ := git.Config("push.default")
+	if pushDefault == "upstream" || pushDefault == "tracking" {
+		upstream, e := branch.Upstream()
+		if e == nil && upstream.IsRemote() {
+			remote, e := r.RemoteByName(upstream.RemoteName())
 			if e == nil {
-				project, err = remote.Project()
-				if err != nil {
+				p, e := remote.Project()
+				if e == nil {
+					branch = upstream
+					project = p
 					return
 				}
 			}
 		}
 	}
 
+	shortName := branch.ShortName()
+	remotes := r.remotesForPublish(owner)
+	if preferUpstream {
+		// reverse the remote lookup order; see OriginNamesInLookupOrder
+		remotesInOrder := []Remote{}
+		for i := len(remotes) - 1; i >= 0; i-- {
+			remotesInOrder = append(remotesInOrder, remotes[i])
+		}
+		remotes = remotesInOrder
+	}
+
+	for _, remote := range remotes {
+		p, e := remote.Project()
+		if e != nil {
+			continue
+		}
+		// NOTE: this is similar RemoteForBranch
+		if git.HasFile("refs", "remotes", remote.Name, shortName) {
+			name := fmt.Sprintf("refs/remotes/%s/%s", remote.Name, shortName)
+			branch = &Branch{r, name}
+			project = p
+			return
+		}
+	}
+
+	branch = nil
 	return
 }
 
-// duplicates logic from PushTarget()
 func (r *GitHubRepo) RemoteForBranch(branch *Branch, owner string) *Remote {
 	branchName := branch.ShortName()
 	for _, remote := range r.remotesForPublish(owner) {