Query escape branch names to allow branch/cli#123 names

Leonardo · web-flow · commit d0bee19eafcf · 2020-10-21T12:09:46.000Z
diff --git a/pkg/cmd/pr/create/create.go b/pkg/cmd/pr/create/create.go
@@ -4,6 +4,7 @@ import (
 	"errors"
 	"fmt"
 	"net/http"
+	"net/url"
 	"strings"
 	"time"
 
@@ -553,7 +554,11 @@ func determineTrackingBranch(remotes context.Remotes, headBranch string) *git.Tr
 }
 
 func generateCompareURL(r ghrepo.Interface, base, head, title, body string, assignees, labels, projects []string, milestones []string) (string, error) {
-	u := ghrepo.GenerateRepoURL(r, "compare/%s...%s?expand=1", base, head)
+	// This is to fix issues with names like "branch/#123"
+	b := url.QueryEscape(base)
+	h := url.QueryEscape(head)
+
+	u := ghrepo.GenerateRepoURL(r, "compare/%s...%s?expand=1", b, h)
 	url, err := shared.WithPrAndIssueQueryParams(u, title, body, assignees, labels, projects, milestones)
 	if err != nil {
 		return "", err
diff --git a/pkg/cmd/pr/create/create_test.go b/pkg/cmd/pr/create/create_test.go
@@ -636,6 +636,42 @@ func TestPRCreate_web(t *testing.T) {
 	eq(t, browserCall[len(browserCall)-1], "https://github.com/OWNER/REPO/compare/master...feature?expand=1")
 }
 
+func TestPRCreate_web_uncommon_branch_names(t *testing.T) {
+	http := initFakeHTTP()
+	defer http.Verify(t)
+
+	http.StubRepoInfoResponse("OWNER", "REPO", "master")
+	http.StubRepoResponse("OWNER", "REPO")
+	http.Register(
+		httpmock.GraphQL(`query UserCurrent\b`),
+		httpmock.StringResponse(`{"data": {"viewer": {"login": "OWNER"} } }`))
+
+	cs, cmdTeardown := test.InitCmdStubber()
+	defer cmdTeardown()
+
+	cs.Stub("")                                         // git config --get-regexp (determineTrackingBranch)
+	cs.Stub("")                                         // git show-ref --verify   (determineTrackingBranch)
+	cs.Stub("")                                         // git status
+	cs.Stub("1234567890,commit 0\n2345678901,commit 1") // git log
+	cs.Stub("")                                         // git push
+	cs.Stub("")                                         // browser
+
+	ask, cleanupAsk := prompt.InitAskStubber()
+	defer cleanupAsk()
+	ask.StubOne(0)
+
+	output, err := runCommand(http, nil, "feature/#123", true, `--web`)
+	require.NoError(t, err)
+
+	eq(t, output.String(), "")
+	eq(t, output.Stderr(), "Opening github.com/OWNER/REPO/compare/master...feature/#123 in your browser.\n")
+
+	eq(t, len(cs.Calls), 6)
+	eq(t, strings.Join(cs.Calls[4].Args, " "), "git push --set-upstream origin HEAD:feature/#123")
+	browserCall := cs.Calls[5].Args
+	eq(t, browserCall[len(browserCall)-1], "https://github.com/OWNER/REPO/compare/master...feature%2F%23123?expand=1")
+}
+
 func Test_determineTrackingBranch_empty(t *testing.T) {
 	cs, cmdTeardown := test.InitCmdStubber()
 	defer cmdTeardown()
