diff --git a/pkg/cmd/repo/clone/clone.go b/pkg/cmd/repo/clone/clone.go index bac5727c8..005ed4b0c 100644 --- a/pkg/cmd/repo/clone/clone.go +++ b/pkg/cmd/repo/clone/clone.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "net/http" + "net/url" "strings" "github.com/MakeNowJust/heredoc" @@ -99,10 +100,12 @@ func cloneRun(opts *CloneOptions) error { var protocol string if repositoryIsURL { - repoURL, err := git.ParseURL(opts.Repository) + initialURL, err := git.ParseURL(opts.Repository) if err != nil { return err } + + repoURL := simplifyURL(initialURL) repo, err = ghrepo.FromURL(repoURL) if err != nil { return err @@ -198,3 +201,30 @@ func cloneRun(opts *CloneOptions) error { } return nil } + +// simplifyURL strips given URL of extra parts like extra path segments (i.e., +// anything beyond `/owner/repo`), query strings, or fragments. This function +// never returns an error. +// +// The rationale behind this function is to let users clone a repo with any +// URL related to the repo; like: +// - (Tree) github.com/owner/repo/blob/main/foo/bar +// - (Deep-link to line) github.com/owner/repo/blob/main/foo/bar#L168 +// - (Issue/PR comment) github.com/owner/repo/pull/999#issue-9999999999 +// - (Commit history) github.com/owner/repo/commits/main/?author=foo +func simplifyURL(u *url.URL) *url.URL { + result := &url.URL{ + Scheme: u.Scheme, + User: u.User, + Host: u.Host, + Path: u.Path, + } + + pathParts := strings.SplitN(strings.Trim(u.Path, "/"), "/", 3) + if len(pathParts) <= 2 { + return result + } + + result.Path = strings.Join(pathParts[0:2], "/") + return result +} diff --git a/pkg/cmd/repo/clone/clone_test.go b/pkg/cmd/repo/clone/clone_test.go index 9546b6f08..ebdc6351a 100644 --- a/pkg/cmd/repo/clone/clone_test.go +++ b/pkg/cmd/repo/clone/clone_test.go @@ -2,6 +2,7 @@ package clone import ( "net/http" + "net/url" "testing" "github.com/cli/cli/v2/git" @@ -163,6 +164,11 @@ func Test_RepoClone(t *testing.T) { args: "https://github.com/OWNER/REPO", want: "git clone https://github.com/OWNER/REPO.git", }, + { + name: "HTTPS URL with extra path parts", + args: "https://github.com/OWNER/REPO/extra/part?key=value#fragment", + want: "git clone https://github.com/OWNER/REPO.git", + }, { name: "SSH URL", args: "git@github.com:OWNER/REPO.git", @@ -183,6 +189,11 @@ func Test_RepoClone(t *testing.T) { args: "https://github.com/owner/repo.wiki", want: "git clone https://github.com/OWNER/REPO.wiki.git", }, + { + name: "wiki URL with extra path parts", + args: "https://github.com/owner/repo.wiki/extra/path?key=value#fragment", + want: "git clone https://github.com/OWNER/REPO.wiki.git", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -331,3 +342,56 @@ func Test_RepoClone_withoutUsername(t *testing.T) { assert.Equal(t, "", output.String()) assert.Equal(t, "", output.Stderr()) } + +func TestSimplifyURL(t *testing.T) { + tests := []struct { + name string + raw string + expectedRaw string + }{ + { + name: "empty", + raw: "", + expectedRaw: "", + }, + { + name: "no change, no path", + raw: "https://github.com", + expectedRaw: "https://github.com", + }, + { + name: "no change, single part path", + raw: "https://github.com/owner", + expectedRaw: "https://github.com/owner", + }, + { + name: "no change, two-part path", + raw: "https://github.com/owner/repo", + expectedRaw: "https://github.com/owner/repo", + }, + { + name: "no change, three-part path", + raw: "https://github.com/owner/repo/pulls", + expectedRaw: "https://github.com/owner/repo", + }, + { + name: "no change, two-part path, with query, with fragment", + raw: "https://github.com/owner/repo?key=value#fragment", + expectedRaw: "https://github.com/owner/repo", + }, + { + name: "no change, single part path, with query, with fragment", + raw: "https://github.com/owner?key=value#fragment", + expectedRaw: "https://github.com/owner", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + u, err := url.Parse(tt.raw) + require.NoError(t, err) + result := simplifyURL(u) + assert.Equal(t, tt.expectedRaw, result.String()) + }) + } +}