Skip to content

Commit c735a5e

Browse files
jesseduffieldjesseduffield
authored
URL encode gitlab brackets to make consistent with branch names (#4336)
- **PR Description** Some operating systems 'open' implementations do not like when some special characters are unencoded, so they will double-encode the branch name, which we already encode. This particularly matters since branch names with / are common. Fixes #4321 - **Please check if the PR fulfills these requirements** * [X] Cheatsheets are up-to-date (run `go generate ./...`) * [X] Code has been formatted (see [here](https://github.com/jesseduffield/lazygit/blob/master/CONTRIBUTING.md#code-formatting)) * [X] Tests have been added/updated (see [here](https://github.com/jesseduffield/lazygit/blob/master/pkg/integration/README.md) for the integration test guide) * [ ] Text is internationalised (see [here](https://github.com/jesseduffield/lazygit/blob/master/CONTRIBUTING.md#internationalisation)) * [ ] If a new UserConfig entry was added, make sure it can be hot-reloaded (see [here](https://github.com/jesseduffield/lazygit/blob/master/docs/dev/Codebase_Guide.md#using-userconfig)) * [ ] Docs have been updated if necessary * [X] You've read through your own file changes for silly mistakes etc
2 parents d423d2a + c1ca1c8 commit c735a5e

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)