Skip to content

Commit c1ca1c8

Browse files
Chris McDonnelljesseduffield
authored andcommitted
URL encode gitlab brackets to make consistent with branch names
Some operating systems 'open' implementations do not like when some special characters are unencoded, so they will double-enconde the branch name, which we already encode. This particularly matters since branch names with / are common
1 parent d423d2a commit c1ca1c8

File tree

2 files changed

+11
-11
lines changed

2 files changed

+11
-11
lines changed

pkg/commands/hosting_service/definitions.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,8 @@ var bitbucketServiceDef = ServiceDefinition{
3333

3434
var gitLabServiceDef = ServiceDefinition{
3535
provider: "gitlab",
36-
pullRequestURLIntoDefaultBranch: "/-/merge_requests/new?merge_request[source_branch]={{.From}}",
37-
pullRequestURLIntoTargetBranch: "/-/merge_requests/new?merge_request[source_branch]={{.From}}&merge_request[target_branch]={{.To}}",
36+
pullRequestURLIntoDefaultBranch: "/-/merge_requests/new?merge_request%5Bsource_branch%5D={{.From}}",
37+
pullRequestURLIntoTargetBranch: "/-/merge_requests/new?merge_request%5Bsource_branch%5D={{.From}}&merge_request%5Btarget_branch%5D={{.To}}",
3838
commitURL: "/-/commit/{{.CommitHash}}",
3939
regexStrings: defaultUrlRegexStrings,
4040
repoURLTemplate: defaultRepoURLTemplate,

pkg/commands/hosting_service/hosting_service_test.go

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ func TestGetPullRequestURL(t *testing.T) {
112112
remoteUrl: "git@gitlab.com:peter/calculator.git",
113113
test: func(url string, err error) {
114114
assert.NoError(t, err)
115-
assert.Equal(t, "https://gitlab.com/peter/calculator/-/merge_requests/new?merge_request[source_branch]=feature%2Fui", url)
115+
assert.Equal(t, "https://gitlab.com/peter/calculator/-/merge_requests/new?merge_request%5Bsource_branch%5D=feature%2Fui", url)
116116
},
117117
},
118118
{
@@ -121,7 +121,7 @@ func TestGetPullRequestURL(t *testing.T) {
121121
remoteUrl: "git@gitlab.com:peter/public/calculator.git",
122122
test: func(url string, err error) {
123123
assert.NoError(t, err)
124-
assert.Equal(t, "https://gitlab.com/peter/public/calculator/-/merge_requests/new?merge_request[source_branch]=feature%2Fui", url)
124+
assert.Equal(t, "https://gitlab.com/peter/public/calculator/-/merge_requests/new?merge_request%5Bsource_branch%5D=feature%2Fui", url)
125125
},
126126
},
127127
{
@@ -130,7 +130,7 @@ func TestGetPullRequestURL(t *testing.T) {
130130
remoteUrl: "https://gitlab.com/peter/public/calculator.git",
131131
test: func(url string, err error) {
132132
assert.NoError(t, err)
133-
assert.Equal(t, "https://gitlab.com/peter/public/calculator/-/merge_requests/new?merge_request[source_branch]=feature%2Fui", url)
133+
assert.Equal(t, "https://gitlab.com/peter/public/calculator/-/merge_requests/new?merge_request%5Bsource_branch%5D=feature%2Fui", url)
134134
},
135135
},
136136
{
@@ -140,7 +140,7 @@ func TestGetPullRequestURL(t *testing.T) {
140140
remoteUrl: "git@gitlab.com:peter/calculator.git",
141141
test: func(url string, err error) {
142142
assert.NoError(t, err)
143-
assert.Equal(t, "https://gitlab.com/peter/calculator/-/merge_requests/new?merge_request[source_branch]=feature%2Fcommit-ui&merge_request[target_branch]=epic%2Fui", url)
143+
assert.Equal(t, "https://gitlab.com/peter/calculator/-/merge_requests/new?merge_request%5Bsource_branch%5D=feature%2Fcommit-ui&merge_request%5Btarget_branch%5D=epic%2Fui", url)
144144
},
145145
},
146146
{
@@ -150,7 +150,7 @@ func TestGetPullRequestURL(t *testing.T) {
150150
remoteUrl: "git@gitlab.com:peter/public/calculator.git",
151151
test: func(url string, err error) {
152152
assert.NoError(t, err)
153-
assert.Equal(t, "https://gitlab.com/peter/public/calculator/-/merge_requests/new?merge_request[source_branch]=feature%2Fcommit-ui&merge_request[target_branch]=epic%2Fui", url)
153+
assert.Equal(t, "https://gitlab.com/peter/public/calculator/-/merge_requests/new?merge_request%5Bsource_branch%5D=feature%2Fcommit-ui&merge_request%5Btarget_branch%5D=epic%2Fui", url)
154154
},
155155
},
156156
{
@@ -160,7 +160,7 @@ func TestGetPullRequestURL(t *testing.T) {
160160
remoteUrl: "https://gitlab.com/peter/public/calculator.git",
161161
test: func(url string, err error) {
162162
assert.NoError(t, err)
163-
assert.Equal(t, "https://gitlab.com/peter/public/calculator/-/merge_requests/new?merge_request[source_branch]=feature%2Fcommit-ui&merge_request[target_branch]=epic%2Fui", url)
163+
assert.Equal(t, "https://gitlab.com/peter/public/calculator/-/merge_requests/new?merge_request%5Bsource_branch%5D=feature%2Fcommit-ui&merge_request%5Btarget_branch%5D=epic%2Fui", url)
164164
},
165165
},
166166
{
@@ -362,7 +362,7 @@ func TestGetPullRequestURL(t *testing.T) {
362362
},
363363
test: func(url string, err error) {
364364
assert.NoError(t, err)
365-
assert.Equal(t, "https://my.domain.test:1111/johndoe/social_network/-/merge_requests/new?merge_request[source_branch]=feature%2Fprofile-page", url)
365+
assert.Equal(t, "https://my.domain.test:1111/johndoe/social_network/-/merge_requests/new?merge_request%5Bsource_branch%5D=feature%2Fprofile-page", url)
366366
},
367367
},
368368
{
@@ -410,7 +410,7 @@ func TestGetPullRequestURL(t *testing.T) {
410410
remoteUrl: "git@gitlab.com:me/public/repo-with-issues.git",
411411
test: func(url string, err error) {
412412
assert.NoError(t, err)
413-
assert.Equal(t, "https://gitlab.com/me/public/repo-with-issues/-/merge_requests/new?merge_request[source_branch]=feature%2FsomeIssue%23123&merge_request[target_branch]=master", url)
413+
assert.Equal(t, "https://gitlab.com/me/public/repo-with-issues/-/merge_requests/new?merge_request%5Bsource_branch%5D=feature%2FsomeIssue%23123&merge_request%5Btarget_branch%5D=master", url)
414414
},
415415
},
416416
{
@@ -420,7 +420,7 @@ func TestGetPullRequestURL(t *testing.T) {
420420
remoteUrl: "git@gitlab.com:me/public/repo-with-issues.git",
421421
test: func(url string, err error) {
422422
assert.NoError(t, err)
423-
assert.Equal(t, "https://gitlab.com/me/public/repo-with-issues/-/merge_requests/new?merge_request[source_branch]=yolo&merge_request[target_branch]=archive%2Fnever-ending-feature%23666", url)
423+
assert.Equal(t, "https://gitlab.com/me/public/repo-with-issues/-/merge_requests/new?merge_request%5Bsource_branch%5D=yolo&merge_request%5Btarget_branch%5D=archive%2Fnever-ending-feature%23666", url)
424424
},
425425
},
426426
}

0 commit comments

Comments
 (0)