From 5656296adec1fc8856cd59eb08adaa34f299c84a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 11 Jul 2022 13:54:58 +0200 Subject: [PATCH] repo fork: directly fork under the desired name A new GitHub feature landed where the API client can specify the desired name of the new fork. This avoids the necessity of subsequently having to rename the forked repo after the fork operation has created one. For backwards compatibility, the renaming logic is still here, but activates only if the resulting repo name is not the desired name. --- api/queries_repo.go | 5 +++- pkg/cmd/pr/create/create.go | 2 +- pkg/cmd/repo/create/create.go | 11 ++----- pkg/cmd/repo/create/create_test.go | 41 ------------------------- pkg/cmd/repo/fork/fork.go | 7 +++-- pkg/cmd/repo/shared/repo.go | 14 +++++++++ pkg/cmd/repo/shared/repo_test.go | 48 ++++++++++++++++++++++++++++++ 7 files changed, 74 insertions(+), 54 deletions(-) create mode 100644 pkg/cmd/repo/shared/repo.go create mode 100644 pkg/cmd/repo/shared/repo_test.go diff --git a/api/queries_repo.go b/api/queries_repo.go index a688f9fd1..d63888cef 100644 --- a/api/queries_repo.go +++ b/api/queries_repo.go @@ -500,13 +500,16 @@ type repositoryV3 struct { } // ForkRepo forks the repository on GitHub and returns the new repository -func ForkRepo(client *Client, repo ghrepo.Interface, org string) (*Repository, error) { +func ForkRepo(client *Client, repo ghrepo.Interface, org, newName string) (*Repository, error) { path := fmt.Sprintf("repos/%s/forks", ghrepo.FullName(repo)) params := map[string]interface{}{} if org != "" { params["organization"] = org } + if newName != "" { + params["name"] = newName + } body := &bytes.Buffer{} enc := json.NewEncoder(body) diff --git a/pkg/cmd/pr/create/create.go b/pkg/cmd/pr/create/create.go index f5691a563..9f8e34fd4 100644 --- a/pkg/cmd/pr/create/create.go +++ b/pkg/cmd/pr/create/create.go @@ -680,7 +680,7 @@ func handlePush(opts CreateOptions, ctx CreateContext) error { // one by forking the base repository if headRepo == nil && ctx.IsPushEnabled { opts.IO.StartProgressIndicator() - headRepo, err = api.ForkRepo(client, ctx.BaseRepo, "") + headRepo, err = api.ForkRepo(client, ctx.BaseRepo, "", "") opts.IO.StopProgressIndicator() if err != nil { return fmt.Errorf("error forking repo: %w", err) diff --git a/pkg/cmd/repo/create/create.go b/pkg/cmd/repo/create/create.go index a1ca50aa7..241453ed1 100644 --- a/pkg/cmd/repo/create/create.go +++ b/pkg/cmd/repo/create/create.go @@ -6,7 +6,6 @@ import ( "net/http" "os/exec" "path/filepath" - "regexp" "strings" "github.com/AlecAivazis/survey/v2" @@ -16,6 +15,7 @@ import ( "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/internal/run" + "github.com/cli/cli/v2/pkg/cmd/repo/shared" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" "github.com/cli/cli/v2/pkg/prompt" @@ -816,9 +816,9 @@ func interactiveSource() (string, error) { } func confirmSubmission(repoWithOwner, visibility string) error { - targetRepo := normalizeRepoName(repoWithOwner) + targetRepo := shared.NormalizeRepoName(repoWithOwner) if idx := strings.IndexRune(repoWithOwner, '/'); idx > 0 { - targetRepo = repoWithOwner[0:idx+1] + normalizeRepoName(repoWithOwner[idx+1:]) + targetRepo = repoWithOwner[0:idx+1] + shared.NormalizeRepoName(repoWithOwner[idx+1:]) } var answer struct { ConfirmSubmit bool @@ -838,8 +838,3 @@ func confirmSubmission(repoWithOwner, visibility string) error { } return nil } - -// normalizeRepoName takes in the repo name the user inputted and normalizes it using the same logic as GitHub (GitHub.com/new) -func normalizeRepoName(repoName string) string { - return strings.TrimSuffix(regexp.MustCompile(`[^\w._-]+`).ReplaceAllString(repoName, "-"), ".git") -} diff --git a/pkg/cmd/repo/create/create_test.go b/pkg/cmd/repo/create/create_test.go index f7fb1797b..75dd3a0d1 100644 --- a/pkg/cmd/repo/create/create_test.go +++ b/pkg/cmd/repo/create/create_test.go @@ -495,44 +495,3 @@ func Test_createRun(t *testing.T) { }) } } - -func Test_getModifiedNormalizedName(t *testing.T) { - // confirmed using GitHub.com/new - tests := []struct { - LocalName string - NormalizedName string - }{ - { - LocalName: "cli", - NormalizedName: "cli", - }, - { - LocalName: "cli.git", - NormalizedName: "cli", - }, - { - LocalName: "@-#$^", - NormalizedName: "---", - }, - { - LocalName: "[cli]", - NormalizedName: "-cli-", - }, - { - LocalName: "Hello World, I'm a new repo!", - NormalizedName: "Hello-World-I-m-a-new-repo-", - }, - { - LocalName: " @E3H*(#$#_$-ZVp,n.7lGq*_eMa-(-zAZSJYg!", - NormalizedName: "-E3H-_--ZVp-n.7lGq-_eMa---zAZSJYg-", - }, - { - LocalName: "I'm a crazy .git repo name .git.git .git", - NormalizedName: "I-m-a-crazy-.git-repo-name-.git.git-", - }, - } - for _, tt := range tests { - output := normalizeRepoName(tt.LocalName) - assert.Equal(t, tt.NormalizedName, output) - } -} diff --git a/pkg/cmd/repo/fork/fork.go b/pkg/cmd/repo/fork/fork.go index 1752e6272..d0b0f9c7a 100644 --- a/pkg/cmd/repo/fork/fork.go +++ b/pkg/cmd/repo/fork/fork.go @@ -14,6 +14,7 @@ import ( "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/ghrepo" "github.com/cli/cli/v2/internal/run" + "github.com/cli/cli/v2/pkg/cmd/repo/shared" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" "github.com/cli/cli/v2/pkg/prompt" @@ -179,7 +180,7 @@ func forkRun(opts *ForkOptions) error { apiClient := api.NewClientFromHTTP(httpClient) opts.IO.StartProgressIndicator() - forkedRepo, err := api.ForkRepo(apiClient, repoToFork, opts.Organization) + forkedRepo, err := api.ForkRepo(apiClient, repoToFork, opts.Organization, opts.ForkName) opts.IO.StopProgressIndicator() if err != nil { return fmt.Errorf("failed to fork: %w", err) @@ -206,8 +207,8 @@ func forkRun(opts *ForkOptions) error { } } - // Rename the forked repo if ForkName is specified in opts. - if opts.ForkName != "" { + // Rename the new repo if necessary + if opts.ForkName != "" && !strings.EqualFold(forkedRepo.RepoName(), shared.NormalizeRepoName(opts.ForkName)) { forkedRepo, err = api.RenameRepo(apiClient, forkedRepo, opts.ForkName) if err != nil { return fmt.Errorf("could not rename fork: %w", err) diff --git a/pkg/cmd/repo/shared/repo.go b/pkg/cmd/repo/shared/repo.go new file mode 100644 index 000000000..b980098b8 --- /dev/null +++ b/pkg/cmd/repo/shared/repo.go @@ -0,0 +1,14 @@ +package shared + +import ( + "regexp" + "strings" +) + +var invalidCharactersRE = regexp.MustCompile(`[^\w._-]+`) + +// NormalizeRepoName takes in the repo name the user inputted and normalizes it using the same logic as GitHub (GitHub.com/new) +func NormalizeRepoName(repoName string) string { + newName := invalidCharactersRE.ReplaceAllString(repoName, "-") + return strings.TrimSuffix(newName, ".git") +} diff --git a/pkg/cmd/repo/shared/repo_test.go b/pkg/cmd/repo/shared/repo_test.go new file mode 100644 index 000000000..2e9251840 --- /dev/null +++ b/pkg/cmd/repo/shared/repo_test.go @@ -0,0 +1,48 @@ +package shared + +import ( + "testing" +) + +func TestNormalizeRepoName(t *testing.T) { + // confirmed using GitHub.com/new + tests := []struct { + LocalName string + NormalizedName string + }{ + { + LocalName: "cli", + NormalizedName: "cli", + }, + { + LocalName: "cli.git", + NormalizedName: "cli", + }, + { + LocalName: "@-#$^", + NormalizedName: "---", + }, + { + LocalName: "[cli]", + NormalizedName: "-cli-", + }, + { + LocalName: "Hello World, I'm a new repo!", + NormalizedName: "Hello-World-I-m-a-new-repo-", + }, + { + LocalName: " @E3H*(#$#_$-ZVp,n.7lGq*_eMa-(-zAZSJYg!", + NormalizedName: "-E3H-_--ZVp-n.7lGq-_eMa---zAZSJYg-", + }, + { + LocalName: "I'm a crazy .git repo name .git.git .git", + NormalizedName: "I-m-a-crazy-.git-repo-name-.git.git-", + }, + } + for _, tt := range tests { + output := NormalizeRepoName(tt.LocalName) + if output != tt.NormalizedName { + t.Errorf("Expected %q, got %q", tt.NormalizedName, output) + } + } +}