diff --git a/commands/mr/checkout/mr_checkout.go b/commands/mr/checkout/mr_checkout.go index 4d23632e0af32a72e64bc6a0f73152770866cabf..7b38246b2f2ecd6f0d41bbfe7c292a2e46e7558d 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" @@ -23,23 +24,72 @@ 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 + - glab mr checkout https://gitlab.example.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], "/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 + } + // Replace the URL with the extracted ID + args[0] = mrID + } + if mrCheckoutCfg.upstream != "" { upstream = mrCheckoutCfg.upstream diff --git a/commands/mr/checkout/mr_checkout_test.go b/commands/mr/checkout/mr_checkout_test.go index 56c2cefd556b91f9b09bbfe41e64384093c18fe8..f6b9997b13590ccfba927ef3fee830ac1f575ac4 100644 --- a/commands/mr/checkout/mr_checkout_test.go +++ b/commands/mr/checkout/mr_checkout_test.go @@ -122,6 +122,153 @@ 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 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", @@ -231,6 +378,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.go b/commands/mr/mrutils/mrutils.go index a946a6af4d80f342fa680576e206daf46ec472bd..73ef65749045365a7a25b85c1714a53a6443d323 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) diff --git a/commands/mr/mrutils/mrutils_test.go b/commands/mr/mrutils/mrutils_test.go index 2c0e45c9566e75dcd59959040f027fb2ccc85394..51ed766002d6f6be82e7dd2e4be6091eed814fe4 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{ diff --git a/docs/source/mr/checkout.md b/docs/source/mr/checkout.md index 6b5ae8d3b338e0d9a7d52b30708cc77099a7b688..140d2dade5e3b289d0c12a4a573e50a3e3f15ebe 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,8 @@ 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 +- glab mr checkout https://gitlab.example.com/owner/repo/-/merge_requests/123 Uses the checked-out branch - glab mr checkout