Improved error message when "owner/repo" format not provided (cli#919)

Fixes cli#882
backwardn · May 18, 2020 · d440a95 · d440a95
1 parent c115d32
commit d440a95
Show file tree

Hide file tree

Showing 7 changed files with 31 additions and 17 deletions.
diff --git a/api/queries_issue_test.go b/api/queries_issue_test.go
@@ -39,7 +39,8 @@ func TestIssueList(t *testing.T) {
 	} } }
 	`))
 
-	_, err := IssueList(client, ghrepo.FromFullName("OWNER/REPO"), "open", []string{}, "", 251, "")
+	repo, _ := ghrepo.FromFullName("OWNER/REPO")
+	_, err := IssueList(client, repo, "open", []string{}, "", 251, "")
 	if err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}

diff --git a/api/queries_repo_test.go b/api/queries_repo_test.go
@@ -50,7 +50,7 @@ func Test_RepoMetadata(t *testing.T) {
 	http := &httpmock.Registry{}
 	client := NewClient(ReplaceTripper(http))
 
-	repo := ghrepo.FromFullName("OWNER/REPO")
+	repo, _ := ghrepo.FromFullName("OWNER/REPO")
 	input := RepoMetadataInput{
 		Assignees:  true,
 		Reviewers:  true,
@@ -181,7 +181,7 @@ func Test_RepoResolveMetadataIDs(t *testing.T) {
 	http := &httpmock.Registry{}
 	client := NewClient(ReplaceTripper(http))
 
-	repo := ghrepo.FromFullName("OWNER/REPO")
+	repo, _ := ghrepo.FromFullName("OWNER/REPO")
 	input := RepoResolveInput{
 		Assignees: []string{"monalisa", "hubot"},
 		Reviewers: []string{"monalisa", "octocat", "OWNER/core", "/robots"},

diff --git a/command/repo.go b/command/repo.go
@@ -189,7 +189,10 @@ func repoCreate(cmd *cobra.Command, args []string) error {
 	if len(args) > 0 {
 		name = args[0]
 		if strings.Contains(name, "/") {
-			newRepo := ghrepo.FromFullName(name)
+			newRepo, err := ghrepo.FromFullName(name)
+			if err != nil {
+				return fmt.Errorf("argument error: %w", err)
+			}
 			orgName = newRepo.RepoOwner()
 			name = newRepo.RepoName()
 		}
@@ -366,9 +369,9 @@ func repoFork(cmd *cobra.Command, args []string) error {
 				return fmt.Errorf("did not understand argument: %w", err)
 			}
 		} else {
-			repoToFork = ghrepo.FromFullName(repoArg)
-			if repoToFork.RepoName() == "" || repoToFork.RepoOwner() == "" {
-				return fmt.Errorf("could not parse owner or repo name from %s", repoArg)
+			repoToFork, err = ghrepo.FromFullName(repoArg)
+			if err != nil {
+				return fmt.Errorf("argument error: %w", err)
 			}
 		}
 	}
@@ -508,7 +511,11 @@ func repoView(cmd *cobra.Command, args []string) error {
 				return fmt.Errorf("did not understand argument: %w", err)
 			}
 		} else {
-			toView = ghrepo.FromFullName(repoArg)
+			var err error
+			toView, err = ghrepo.FromFullName(repoArg)
+			if err != nil {
+				return fmt.Errorf("argument error: %w", err)
+			}
 		}
 	}
 

diff --git a/command/root.go b/command/root.go
@@ -219,7 +219,11 @@ func changelogURL(version string) string {
 func determineBaseRepo(cmd *cobra.Command, ctx context.Context) (ghrepo.Interface, error) {
 	repo, err := cmd.Flags().GetString("repo")
 	if err == nil && repo != "" {
-		return ghrepo.FromFullName(repo), nil
+		baseRepo, err := ghrepo.FromFullName(repo)
+		if err != nil {
+			return nil, fmt.Errorf("argument error: %w", err)
+		}
+		return baseRepo, nil
 	}
 
 	apiClient, err := apiClientForContext(ctx)

diff --git a/context/blank_context.go b/context/blank_context.go
@@ -92,5 +92,6 @@ func (c *blankContext) BaseRepo() (ghrepo.Interface, error) {
 }
 
 func (c *blankContext) SetBaseRepo(nwo string) {
-	c.baseRepo = ghrepo.FromFullName(nwo)
+	repo, _ := ghrepo.FromFullName(nwo)
+	c.baseRepo = repo
 }
diff --git a/context/context.go b/context/context.go
@@ -39,7 +39,7 @@ func ResolveRemotesToRepos(remotes Remotes, client *api.Client, base string) (Re
 	}
 
 	hasBaseOverride := base != ""
-	baseOverride := ghrepo.FromFullName(base)
+	baseOverride, _ := ghrepo.FromFullName(base)
 	foundBaseOverride := false
 	repos := make([]ghrepo.Interface, 0, lenRemotesForLookup)
 	for _, r := range remotes[:lenRemotesForLookup] {
@@ -266,5 +266,5 @@ func (c *fsContext) BaseRepo() (ghrepo.Interface, error) {
 }
 
 func (c *fsContext) SetBaseRepo(nwo string) {
-	c.baseRepo = ghrepo.FromFullName(nwo)
+	c.baseRepo, _ = ghrepo.FromFullName(nwo)
 }
diff --git a/internal/ghrepo/repo.go b/internal/ghrepo/repo.go
@@ -28,14 +28,15 @@ func FullName(r Interface) string {
 	return fmt.Sprintf("%s/%s", r.RepoOwner(), r.RepoName())
 }
 
-// FromFullName extracts the GitHub repository inforation from an "OWNER/REPO" string
-func FromFullName(nwo string) Interface {
+// FromFullName extracts the GitHub repository information from an "OWNER/REPO" string
+func FromFullName(nwo string) (Interface, error) {
 	var r ghRepo
 	parts := strings.SplitN(nwo, "/", 2)
-	if len(parts) == 2 {
-		r.owner, r.name = parts[0], parts[1]
+	if len(parts) != 2 || parts[0] == "" || parts[1] == "" {
+		return &r, fmt.Errorf("expected OWNER/REPO format, got %q", nwo)
 	}
-	return &r
+	r.owner, r.name = parts[0], parts[1]
+	return &r, nil
 }
 
 // FromURL extracts the GitHub repository information from a URL