From 93a5a373d4c255c7efacb8898af269ba3cc375a7 Mon Sep 17 00:00:00 2001 From: elskow Date: Sun, 11 May 2025 12:16:37 +0700 Subject: [PATCH 1/4] feat: enhance checkout command to accept merge request URLs --- commands/mr/checkout/mr_checkout.go | 30 +++++++++++++------ commands/mr/mrutils/mrutils.go | 45 +++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 9 deletions(-) diff --git a/commands/mr/checkout/mr_checkout.go b/commands/mr/checkout/mr_checkout.go index 4d23632e0..2e6227bfd 100644 --- a/commands/mr/checkout/mr_checkout.go +++ b/commands/mr/checkout/mr_checkout.go @@ -23,23 +23,35 @@ var mrCheckoutCfg mrCheckoutConfig func NewCmdCheckout(f *cmdutils.Factory) *cobra.Command { mrCheckoutCmd := &cobra.Command{ - Use: "checkout [ | ]", + Use: "checkout [ | | ]", Short: "Check out an open merge request.", Long: ``, Example: heredoc.Doc(` - - glab mr checkout 1 - - glab mr checkout branch - - glab mr checkout 12 --branch todo-fix - - glab mr checkout new-feature --set-upstream-to=upstream/main - - Uses the checked-out branch - - glab mr checkout - `), + - glab mr checkout 1 + - glab mr checkout branch + - glab mr checkout 12 --branch todo-fix + - glab mr checkout new-feature --set-upstream-to=upstream/main + - glab mr checkout https://gitlab.com/owner/repo/-/merge_requests/123 + + Uses the checked-out branch + - glab mr checkout + `), Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { var err error var upstream string + // Process URL if provided + if strings.Contains(args[0], "gitlab.") && (strings.Contains(args[0], "/merge_requests/") || + strings.Contains(args[0], "/merge-requests/")) { + mrID, err := mrutils.ExtractMRIDFromURL(args[0]) + if err != nil { + return err + } + // Replace the URL with the extracted ID + args[0] = mrID + } + if mrCheckoutCfg.upstream != "" { upstream = mrCheckoutCfg.upstream diff --git a/commands/mr/mrutils/mrutils.go b/commands/mr/mrutils/mrutils.go index a946a6af4..73ef65749 100644 --- a/commands/mr/mrutils/mrutils.go +++ b/commands/mr/mrutils/mrutils.go @@ -7,6 +7,7 @@ import ( "sort" "strconv" "strings" + "unicode" "gitlab.com/gitlab-org/cli/pkg/dbg" "gitlab.com/gitlab-org/cli/pkg/iostreams" @@ -134,6 +135,50 @@ func DisplayAllMRs(streams *iostreams.IOStreams, mrs []*gitlab.BasicMergeRequest return table.Render() } +func ExtractMRIDFromURL(url string) (string, error) { + url = strings.TrimSpace(url) + + prefixes := []string{ + "/-/merge_requests/", + "/-/merge-requests/", + } + + var prefix string + for _, p := range prefixes { + if strings.Contains(url, p) { + prefix = p + break + } + } + + if prefix == "" { + return "", fmt.Errorf("not a valid merge request URL: %s", url) + } + + parts := strings.Split(url, prefix) + if len(parts) < 2 { + return "", fmt.Errorf("malformed merge request URL: %s", url) + } + + // Extract the ID part and clean up any trailing fragments + idPart := parts[1] + if idx := strings.IndexAny(idPart, "#?/"); idx != -1 { + idPart = idPart[:idx] + } + + if idPart == "" { + return "", fmt.Errorf("missing merge request ID in URL: %s", url) + } + + for _, r := range idPart { + if !unicode.IsDigit(r) { + return "", fmt.Errorf("invalid merge request ID in URL: %s", idPart) + } + } + + return idPart, nil +} + // MRFromArgs is wrapper around MRFromArgsWithOpts without any custom options func MRFromArgs(f *cmdutils.Factory, args []string, state string) (*gitlab.MergeRequest, glrepo.Interface, error) { return MRFromArgsWithOpts(f, args, &gitlab.GetMergeRequestsOptions{}, state) -- GitLab From 9ac8e523310f87153e6c32863ee9209ca33fdd5e Mon Sep 17 00:00:00 2001 From: elskow Date: Sun, 11 May 2025 12:18:56 +0700 Subject: [PATCH 2/4] test: extracting MR IDs from various URL formats and checkout tests for merge request URLs --- commands/mr/checkout/mr_checkout_test.go | 147 +++++++++++++++++++++++ commands/mr/mrutils/mrutils_test.go | 79 ++++++++++++ 2 files changed, 226 insertions(+) diff --git a/commands/mr/checkout/mr_checkout_test.go b/commands/mr/checkout/mr_checkout_test.go index 56c2cefd5..43cded6af 100644 --- a/commands/mr/checkout/mr_checkout_test.go +++ b/commands/mr/checkout/mr_checkout_test.go @@ -122,6 +122,104 @@ func TestMrCheckout(t *testing.T) { "git checkout feat-new-mr", }, }, + { + name: "when a valid MR is checked out using merge_requests URL format", + commandArgs: "https://gitlab.com/OWNER/REPO/-/merge_requests/123", + branch: "main", + httpMocks: []httpMock{ + { + http.MethodGet, + "/api/v4/projects/OWNER/REPO/merge_requests/123", + http.StatusOK, + `{ + "id": 123, + "iid": 123, + "project_id": 3, + "source_project_id": 3, + "title": "test mr title", + "description": "test mr description", + "allow_collaboration": false, + "state": "opened", + "source_branch":"feat-new-mr" + }`, + }, + { + http.MethodGet, + "/api/v4/projects/3", + http.StatusOK, + `{ + "id": 3, + "ssh_url_to_repo": "git@gitlab.com:OWNER/REPO.git" + }`, + }, + }, + shelloutStubs: []string{ + "HEAD branch: master\n", + "\n", + "\n", + heredoc.Doc(` + deadbeef HEAD + deadb00f refs/remotes/upstream/feat-new-mr + deadbeef refs/remotes/origin/feat-new-mr + `), + }, + + expectedShellouts: []string{ + "git fetch git@gitlab.com:OWNER/REPO.git refs/heads/feat-new-mr:feat-new-mr", + "git config branch.feat-new-mr.remote git@gitlab.com:OWNER/REPO.git", + "git config branch.feat-new-mr.merge refs/heads/feat-new-mr", + "git checkout feat-new-mr", + }, + }, + { + name: "when a valid MR is checked out using merge-requests URL format (hyphenated)", + commandArgs: "https://gitlab.com/OWNER/REPO/-/merge-requests/123", + branch: "main", + httpMocks: []httpMock{ + { + http.MethodGet, + "/api/v4/projects/OWNER/REPO/merge_requests/123", + http.StatusOK, + `{ + "id": 123, + "iid": 123, + "project_id": 3, + "source_project_id": 3, + "title": "test mr title", + "description": "test mr description", + "allow_collaboration": false, + "state": "opened", + "source_branch":"feat-new-mr" + }`, + }, + { + http.MethodGet, + "/api/v4/projects/3", + http.StatusOK, + `{ + "id": 3, + "ssh_url_to_repo": "git@gitlab.com:OWNER/REPO.git" + }`, + }, + }, + shelloutStubs: []string{ + "HEAD branch: master\n", + "\n", + "\n", + heredoc.Doc(` + deadbeef HEAD + deadb00f refs/remotes/upstream/feat-new-mr + deadbeef refs/remotes/origin/feat-new-mr + `), + }, + + expectedShellouts: []string{ + "git fetch git@gitlab.com:OWNER/REPO.git refs/heads/feat-new-mr:feat-new-mr", + "git config branch.feat-new-mr.remote git@gitlab.com:OWNER/REPO.git", + "git config branch.feat-new-mr.merge refs/heads/feat-new-mr", + "git checkout feat-new-mr", + }, + }, { name: "when a valid MR comes from a forked private project", commandArgs: "123", @@ -231,6 +329,55 @@ func TestMrCheckout(t *testing.T) { "git checkout foo", }, }, + { + name: "when a valid MR is checked out using URL and specifying branch", + commandArgs: "https://gitlab.com/OWNER/REPO/-/merge_requests/123 --branch custom-branch", + branch: "main", + httpMocks: []httpMock{ + { + http.MethodGet, + "/api/v4/projects/OWNER/REPO/merge_requests/123", + http.StatusOK, + `{ + "id": 123, + "iid": 123, + "project_id": 3, + "source_project_id": 3, + "title": "test mr title", + "description": "test mr description", + "allow_collaboration": false, + "state": "opened", + "source_branch":"feat-new-mr" + }`, + }, + { + http.MethodGet, + "/api/v4/projects/3", + http.StatusOK, + `{ + "id": 3, + "ssh_url_to_repo": "git@gitlab.com:OWNER/REPO.git" + }`, + }, + }, + shelloutStubs: []string{ + "HEAD branch: master\n", + "\n", + "\n", + heredoc.Doc(` + deadbeef HEAD + deadb00f refs/remotes/upstream/feat-new-mr + deadbeef refs/remotes/origin/feat-new-mr + `), + }, + + expectedShellouts: []string{ + "git fetch git@gitlab.com:OWNER/REPO.git refs/heads/feat-new-mr:custom-branch", + "git config branch.custom-branch.remote git@gitlab.com:OWNER/REPO.git", + "git config branch.custom-branch.merge refs/heads/feat-new-mr", + "git checkout custom-branch", + }, + }, } for _, tc := range tests { diff --git a/commands/mr/mrutils/mrutils_test.go b/commands/mr/mrutils/mrutils_test.go index 2c0e45c95..51ed76600 100644 --- a/commands/mr/mrutils/mrutils_test.go +++ b/commands/mr/mrutils/mrutils_test.go @@ -395,6 +395,85 @@ func Test_getMRForBranchPrompt(t *testing.T) { }) } +func TestExtractMRIDFromURL(t *testing.T) { + tests := []struct { + name string + url string + expectedID string + expectError bool + }{ + { + name: "Standard merge request URL", + url: "https://gitlab.com/OWNER/REPO/-/merge_requests/123", + expectedID: "123", + }, + { + name: "URL with alternative format", + url: "https://gitlab.com/OWNER/REPO/-/merge-requests/456", + expectedID: "456", + }, + { + name: "URL with trailing slash", + url: "https://gitlab.com/OWNER/REPO/-/merge_requests/789/", + expectedID: "789", + }, + { + name: "URL with query parameters", + url: "https://gitlab.com/OWNER/REPO/-/merge_requests/101?view=parallel", + expectedID: "101", + }, + { + name: "URL with hash fragment", + url: "https://gitlab.com/OWNER/REPO/-/merge_requests/202#note_123456", + expectedID: "202", + }, + { + name: "URL with spaces", + url: " https://gitlab.com/OWNER/REPO/-/merge_requests/303 ", + expectedID: "303", + }, + { + name: "URL with no MR ID", + url: "https://gitlab.com/OWNER/REPO/-/merge_requests/", + expectError: true, + }, + { + name: "URL with non-numeric MR ID", + url: "https://gitlab.com/OWNER/REPO/-/merge_requests/abc", + expectError: true, + }, + { + name: "URL with incorrect format", + url: "https://gitlab.com/OWNER/REPO/merge_requests/123", + expectError: true, + }, + { + name: "Not a URL", + url: "not-a-url", + expectError: true, + }, + { + name: "Empty URL", + url: "", + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + id, err := ExtractMRIDFromURL(tt.url) + + if tt.expectError { + assert.Error(t, err, "Expected an error for URL: %s", tt.url) + assert.Empty(t, id, "Expected empty ID for invalid URL") + } else { + assert.NoError(t, err, "Expected no error for URL: %s", tt.url) + assert.Equal(t, tt.expectedID, id, "Expected ID %s but got %s", tt.expectedID, id) + } + }) + } +} + func Test_MRFromArgsWithOpts(t *testing.T) { // Mock cmdutils.Factory object that can be modified as required to perform certain functions f := &cmdutils.Factory{ -- GitLab From 4feaa76653089d4a920ce6997318920c55b060b5 Mon Sep 17 00:00:00 2001 From: elskow Date: Sun, 11 May 2025 12:42:17 +0700 Subject: [PATCH 3/4] docs: update checkout command documentation to include support for merge request URLs --- docs/source/mr/checkout.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/source/mr/checkout.md b/docs/source/mr/checkout.md index 6b5ae8d3b..0cf1c2da9 100644 --- a/docs/source/mr/checkout.md +++ b/docs/source/mr/checkout.md @@ -14,7 +14,7 @@ Please do not edit this file directly. Run `make gen-docs` instead. Check out an open merge request. ```plaintext -glab mr checkout [ | ] [flags] +glab mr checkout [ | | ] [flags] ``` ## Examples @@ -24,6 +24,7 @@ glab mr checkout [ | ] [flags] - glab mr checkout branch - glab mr checkout 12 --branch todo-fix - glab mr checkout new-feature --set-upstream-to=upstream/main +- glab mr checkout https://gitlab.com/owner/repo/-/merge_requests/123 Uses the checked-out branch - glab mr checkout -- GitLab From 2aa0cd7917724891ceccaf6dcff5425a3d325ab0 Mon Sep 17 00:00:00 2001 From: avocrydo <> Date: Sun, 18 May 2025 16:40:52 +0700 Subject: [PATCH 4/4] feat: support checkout from different GitLab instances --- commands/mr/checkout/mr_checkout.go | 42 +++++++++++++++++++- commands/mr/checkout/mr_checkout_test.go | 49 ++++++++++++++++++++++++ docs/source/mr/checkout.md | 1 + 3 files changed, 90 insertions(+), 2 deletions(-) diff --git a/commands/mr/checkout/mr_checkout.go b/commands/mr/checkout/mr_checkout.go index 2e6227bfd..7b38246b2 100644 --- a/commands/mr/checkout/mr_checkout.go +++ b/commands/mr/checkout/mr_checkout.go @@ -2,6 +2,7 @@ package checkout import ( "fmt" + "net/url" "strings" "github.com/MakeNowJust/heredoc/v2" @@ -32,6 +33,7 @@ func NewCmdCheckout(f *cmdutils.Factory) *cobra.Command { - glab mr checkout 12 --branch todo-fix - glab mr checkout new-feature --set-upstream-to=upstream/main - glab mr checkout https://gitlab.com/owner/repo/-/merge_requests/123 + - glab mr checkout https://gitlab.example.com/owner/repo/-/merge_requests/123 Uses the checked-out branch - glab mr checkout @@ -42,8 +44,44 @@ func NewCmdCheckout(f *cmdutils.Factory) *cobra.Command { var upstream string // Process URL if provided - if strings.Contains(args[0], "gitlab.") && (strings.Contains(args[0], "/merge_requests/") || - strings.Contains(args[0], "/merge-requests/")) { + if strings.Contains(args[0], "/merge_requests/") || + strings.Contains(args[0], "/merge-requests/") { + func() { + parsedURL, err := url.Parse(args[0]) + if err != nil || parsedURL.Host == "" { + return // Skip host handling if URL is invalid + } + + cfg, err := f.Config() + if err != nil { + return + } + + configHost, err := cfg.Get("", "host") + if err != nil || configHost == "" { + return + } + + configURL, err := url.Parse(configHost) + if err != nil || configURL.Host == "" { + return + } + + // Only switch hosts if they're different + if parsedURL.Host != configURL.Host { + tempHost := fmt.Sprintf("https://%s", parsedURL.Host) + if err := cfg.Set("", "host", tempHost); err != nil { + fmt.Fprintf(f.IO.StdErr, "Warning: Could not set host for URL: %s\n", err) + return + } + + // Restore original host when done + defer func() { + _ = cfg.Set("", "host", configHost) + }() + } + }() + mrID, err := mrutils.ExtractMRIDFromURL(args[0]) if err != nil { return err diff --git a/commands/mr/checkout/mr_checkout_test.go b/commands/mr/checkout/mr_checkout_test.go index 43cded6af..f6b9997b1 100644 --- a/commands/mr/checkout/mr_checkout_test.go +++ b/commands/mr/checkout/mr_checkout_test.go @@ -220,6 +220,55 @@ func TestMrCheckout(t *testing.T) { "git checkout feat-new-mr", }, }, + { + name: "when a valid MR is checked out using URL from a different GitLab instance", + commandArgs: "https://gitlab.example.com/OWNER/REPO/-/merge_requests/123", + branch: "main", + httpMocks: []httpMock{ + { + http.MethodGet, + "/api/v4/projects/OWNER/REPO/merge_requests/123", + http.StatusOK, + `{ + "id": 123, + "iid": 123, + "project_id": 3, + "source_project_id": 3, + "title": "test mr title", + "description": "test mr description", + "allow_collaboration": false, + "state": "opened", + "source_branch":"feat-new-mr" + }`, + }, + { + http.MethodGet, + "/api/v4/projects/3", + http.StatusOK, + `{ + "id": 3, + "ssh_url_to_repo": "git@gitlab.example.com:OWNER/REPO.git" + }`, + }, + }, + shelloutStubs: []string{ + "HEAD branch: master\n", + "\n", + "\n", + heredoc.Doc(` + deadbeef HEAD + deadb00f refs/remotes/upstream/feat-new-mr + deadbeef refs/remotes/origin/feat-new-mr + `), + }, + + expectedShellouts: []string{ + "git fetch git@gitlab.example.com:OWNER/REPO.git refs/heads/feat-new-mr:feat-new-mr", + "git config branch.feat-new-mr.remote git@gitlab.example.com:OWNER/REPO.git", + "git config branch.feat-new-mr.merge refs/heads/feat-new-mr", + "git checkout feat-new-mr", + }, + }, { name: "when a valid MR comes from a forked private project", commandArgs: "123", diff --git a/docs/source/mr/checkout.md b/docs/source/mr/checkout.md index 0cf1c2da9..140d2dade 100644 --- a/docs/source/mr/checkout.md +++ b/docs/source/mr/checkout.md @@ -25,6 +25,7 @@ glab mr checkout [ | | ] [flags] - glab mr checkout 12 --branch todo-fix - glab mr checkout new-feature --set-upstream-to=upstream/main - glab mr checkout https://gitlab.com/owner/repo/-/merge_requests/123 +- glab mr checkout https://gitlab.example.com/owner/repo/-/merge_requests/123 Uses the checked-out branch - glab mr checkout -- GitLab