From 4385cc9114888426bcbff1eb9e0be47f919392fe Mon Sep 17 00:00:00 2001 From: DiegoPerezGarcia Date: Tue, 9 Jul 2024 20:46:35 +0100 Subject: [PATCH 1/9] Refactor EnableRepoOverride function to use ghowner.RepoToOwnerRepo This commit refactors the EnableRepoOverride function in the repo_override.go file to use the ghowner.RepoToOwnerRepo function. By using this function, the default owner is retrieved from the configuration and formatted correctly for the repository override. This change improves code readability and ensures consistency with other parts of the codebase. --- pkg/cmdutil/repo_override.go | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/pkg/cmdutil/repo_override.go b/pkg/cmdutil/repo_override.go index 309bdabcaad..fc885159a62 100644 --- a/pkg/cmdutil/repo_override.go +++ b/pkg/cmdutil/repo_override.go @@ -53,15 +53,6 @@ func EnableRepoOverride(cmd *cobra.Command, f *Factory) { return err } repoOverride, _ := cmd.Flags().GetString("repo") - defaultOwner, err := f.DefaultOwner() - if err != nil { - return err - } - - repoOverride, err = ghowner.RepoToOwnerRepo(defaultOwner, repoOverride) - if err != nil { - return err - } f.BaseRepo = OverrideBaseRepoFunc(f, repoOverride) return nil } @@ -73,7 +64,17 @@ func OverrideBaseRepoFunc(f *Factory, override string) func() (ghrepo.Interface, } if override != "" { return func() (ghrepo.Interface, error) { - return ghrepo.FromFullName(override) + defaultOwner, err := f.DefaultOwner() + if err != nil { + return nil, err + } + + repository, err := ghowner.RepoToOwnerRepo(defaultOwner, override) + if err != nil { + return nil, err + } + + return ghrepo.FromFullName(repository) } } return f.BaseRepo From 85eb13a99ebf8937b1e6af10c5762f4f6b08dd84 Mon Sep 17 00:00:00 2001 From: DiegoPerezGarcia Date: Tue, 9 Jul 2024 22:33:51 +0100 Subject: [PATCH 2/9] Refactor NewCmdList function to use DefaultOwner field This commit refactors the NewCmdList function in the list.go file to use the DefaultOwner field from the ListOptions struct. The DefaultOwner field is a function that retrieves the default owner from the configuration. By using this field, the code ensures that the owner is set to the default value when no owner is provided as an argument. This change improves flexibility and convenience when listing repositories. --- pkg/cmd/repo/list/list.go | 16 ++++++++++------ pkg/cmd/repo/list/list_test.go | 12 ++++++++++++ 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/pkg/cmd/repo/list/list.go b/pkg/cmd/repo/list/list.go index 63ef7f2ad0a..d66c6f4f061 100644 --- a/pkg/cmd/repo/list/list.go +++ b/pkg/cmd/repo/list/list.go @@ -89,12 +89,6 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman if len(args) > 0 { opts.Owner = args[0] - } else { - defaultOwner, err := opts.DefaultOwner() - if err != nil { - return err - } - opts.Owner = defaultOwner } if runF != nil { @@ -165,6 +159,16 @@ func listRun(opts *ListOptions) error { filter.Fields = opts.Exporter.Fields() } + if opts.Owner == "" { + defaultOwner, err := opts.DefaultOwner() + if err != nil { + return err + } + if defaultOwner != "" { + opts.Owner = defaultOwner + } + } + listResult, err := listRepos(httpClient, host, opts.Limit, opts.Owner, filter) if err != nil { return err diff --git a/pkg/cmd/repo/list/list_test.go b/pkg/cmd/repo/list/list_test.go index bd51cadfc03..8f28f62019a 100644 --- a/pkg/cmd/repo/list/list_test.go +++ b/pkg/cmd/repo/list/list_test.go @@ -286,6 +286,9 @@ func runCommand(rt http.RoundTripper, isTTY bool, cli string) (*test.CmdOut, err Config: func() (gh.Config, error) { return config.NewBlankConfig(), nil }, + DefaultOwner: func() (string, error) { + return "", nil + }, } cmd := NewCmdList(factory, nil) @@ -334,6 +337,9 @@ func TestRepoList_nontty(t *testing.T) { return t }, Limit: 30, + DefaultOwner: func() (string, error) { + return "octocat", nil + }, } err := listRun(&opts) @@ -375,6 +381,9 @@ func TestRepoList_tty(t *testing.T) { return t }, Limit: 30, + DefaultOwner: func() (string, error) { + return "octocat", nil + }, } err := listRun(&opts) @@ -446,6 +455,9 @@ func TestRepoList_noVisibilityField(t *testing.T) { }, Limit: 30, Detector: &fd.DisabledDetectorMock{}, + DefaultOwner: func() (string, error) { + return "octocat", nil + }, } err := listRun(&opts) From 14fa6d97696e1c3e1293316ef71e8af9d2416a25 Mon Sep 17 00:00:00 2001 From: DiegoPerezGarcia Date: Tue, 9 Jul 2024 22:34:57 +0100 Subject: [PATCH 3/9] Refactor archive command to use DefaultOwner field This commit refactors the archive command in the archive.go file to use the DefaultOwner field from the ArchiveOptions struct. The DefaultOwner field is a function that retrieves the default owner from the configuration. By using this field, the code ensures that the owner is set to the default value when no owner is provided as an argument. This change improves flexibility and convenience when archiving repositories. --- pkg/cmd/repo/archive/archive.go | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/pkg/cmd/repo/archive/archive.go b/pkg/cmd/repo/archive/archive.go index b7b47340f61..7c1715aff07 100644 --- a/pkg/cmd/repo/archive/archive.go +++ b/pkg/cmd/repo/archive/archive.go @@ -47,15 +47,7 @@ With no argument, archives the current repository.`), Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { if len(args) > 0 { - defaultOwner, err := opts.DefaultOwner() - if err != nil { - return err - } - repository, err := ghowner.RepoToOwnerRepo(defaultOwner, args[0]) - if err != nil { - return err - } - opts.RepoArg = repository + opts.RepoArg = args[0] } if !opts.Confirmed && !opts.IO.CanPrompt() { @@ -105,7 +97,14 @@ func archiveRun(opts *ArchiveOptions) error { if err != nil { return err } - repoSelector = currentUser + "/" + repoSelector + if defaultOwner, _ := opts.DefaultOwner(); defaultOwner != "" { + repoSelector, err = ghowner.RepoToOwnerRepo(defaultOwner, repoSelector) + if err != nil { + return err + } + } else { + repoSelector = currentUser + "/" + repoSelector + } } toArchive, err = ghrepo.FromFullName(repoSelector) From 2e9335cc2efe89f6157e95a0f8e703c91016de2d Mon Sep 17 00:00:00 2001 From: DiegoPerezGarcia Date: Tue, 9 Jul 2024 22:35:32 +0100 Subject: [PATCH 4/9] Refactor cloneRun function to use DefaultOwner field This commit refactors the cloneRun function in the clone.go file to use the DefaultOwner field from the CloneOptions struct. The DefaultOwner field is a function that retrieves the default owner from the configuration. By using this field, the code ensures that the owner is set to the default value when no owner is provided as an argument. This change improves flexibility and convenience when cloning repositories. --- pkg/cmd/repo/clone/clone.go | 14 +++++--------- pkg/cmd/repo/clone/clone_test.go | 3 +++ 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/pkg/cmd/repo/clone/clone.go b/pkg/cmd/repo/clone/clone.go index 9c7e0c0e173..417d394fac1 100644 --- a/pkg/cmd/repo/clone/clone.go +++ b/pkg/cmd/repo/clone/clone.go @@ -143,22 +143,18 @@ func cloneRun(opts *CloneOptions) error { if repositoryIsFullName { fullName = opts.Repository } else { - defaultOwner, err := opts.DefaultOwner() + host, _ := cfg.Authentication().DefaultHost() + currentUser, err := api.CurrentLoginName(apiClient, host) if err != nil { return err } - if defaultOwner != "" { - repository, err := ghowner.RepoToOwnerRepo(defaultOwner, opts.Repository) + if defaultOwner, _ := opts.DefaultOwner(); defaultOwner != "" { + // TODO: Find a way to test this + fullName, err = ghowner.RepoToOwnerRepo(defaultOwner, fullName) if err != nil { return err } - fullName = repository } else { - host, _ := cfg.Authentication().DefaultHost() - currentUser, err := api.CurrentLoginName(apiClient, host) - if err != nil { - return err - } fullName = currentUser + "/" + opts.Repository } } diff --git a/pkg/cmd/repo/clone/clone_test.go b/pkg/cmd/repo/clone/clone_test.go index 471ed05ddc2..691d0612c0f 100644 --- a/pkg/cmd/repo/clone/clone_test.go +++ b/pkg/cmd/repo/clone/clone_test.go @@ -110,6 +110,9 @@ func runCloneCommand(httpClient *http.Client, cli string) (*test.CmdOut, error) GhPath: "some/path/gh", GitPath: "some/path/git", }, + DefaultOwner: func() (string, error) { + return "", nil + }, } cmd := NewCmdClone(fac, nil) From d99b617e73c8cece2caf1a8c3a0a262c85337ca3 Mon Sep 17 00:00:00 2001 From: DiegoPerezGarcia Date: Tue, 9 Jul 2024 22:41:53 +0100 Subject: [PATCH 5/9] Refactor deleteRun function to use DefaultOwner field This commit refactors the deleteRun function in the delete.go file to use the DefaultOwner field from the DeleteOptions struct. The DefaultOwner field is a function that retrieves the default owner from the configuration. By using this field, the code ensures that the owner is set to the default value when no owner is provided as an argument. This change improves flexibility and convenience when deleting repositories. --- pkg/cmd/repo/delete/delete.go | 19 +++++++++---------- pkg/cmd/repo/delete/delete_test.go | 4 ++++ 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/pkg/cmd/repo/delete/delete.go b/pkg/cmd/repo/delete/delete.go index 6898f47d32e..7032adca41a 100644 --- a/pkg/cmd/repo/delete/delete.go +++ b/pkg/cmd/repo/delete/delete.go @@ -50,15 +50,7 @@ To authorize, run "gh auth refresh -s delete_repo"`, Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { if len(args) > 0 { - defaultOwner, err := opts.DefaultOwner() - if err != nil { - return err - } - repository, err := ghowner.RepoToOwnerRepo(defaultOwner, args[0]) - if err != nil { - return err - } - opts.RepoArg = repository + opts.RepoArg = args[0] } if !opts.IO.CanPrompt() && !opts.Confirmed { @@ -101,7 +93,14 @@ func deleteRun(opts *DeleteOptions) error { if err != nil { return err } - repoSelector = currentUser + "/" + repoSelector + if defaultOwner, _ := opts.DefaultOwner(); defaultOwner != "" { + repoSelector, err = ghowner.RepoToOwnerRepo(defaultOwner, repoSelector) + if err != nil { + return err + } + } else { + repoSelector = currentUser + "/" + repoSelector + } } toDelete, err = ghrepo.FromFullName(repoSelector) if err != nil { diff --git a/pkg/cmd/repo/delete/delete_test.go b/pkg/cmd/repo/delete/delete_test.go index b52ac1de9d7..bffa60428ae 100644 --- a/pkg/cmd/repo/delete/delete_test.go +++ b/pkg/cmd/repo/delete/delete_test.go @@ -180,6 +180,10 @@ func Test_deleteRun(t *testing.T) { return ghrepo.New("OWNER", "REPO"), nil } + tt.opts.DefaultOwner = func() (string, error) { + return "", nil + } + reg := &httpmock.Registry{} if tt.httpStubs != nil { tt.httpStubs(reg) From 5f0b5038975b645e762ead7210248441b6835389 Mon Sep 17 00:00:00 2001 From: DiegoPerezGarcia Date: Tue, 9 Jul 2024 22:53:58 +0100 Subject: [PATCH 6/9] Refactor forkRun function to use DefaultOwner field This commit refactors the forkRun function in the fork.go file to use the DefaultOwner field from the ForkOptions struct. The DefaultOwner field is a function that retrieves the default owner from the configuration. By using this field, the code ensures that the owner is set to the default value when no owner is provided as an argument. This change improves flexibility and convenience when forking repositories. --- pkg/cmd/repo/fork/fork.go | 16 +++++++--------- pkg/cmd/repo/fork/fork_test.go | 4 ++++ 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/pkg/cmd/repo/fork/fork.go b/pkg/cmd/repo/fork/fork.go index 1046337e44f..cddc6f9e996 100644 --- a/pkg/cmd/repo/fork/fork.go +++ b/pkg/cmd/repo/fork/fork.go @@ -102,15 +102,7 @@ func NewCmdFork(f *cmdutil.Factory, runF func(*ForkOptions) error) *cobra.Comman RunE: func(cmd *cobra.Command, args []string) error { promptOk := opts.IO.CanPrompt() if len(args) > 0 { - defaultOwner, err := opts.DefaultOwner() - if err != nil { - return err - } - repository, err := ghowner.RepoToOwnerRepo(defaultOwner, args[0]) - if err != nil { - return err - } - opts.Repository = repository + opts.Repository = args[0] opts.GitArgs = args[1:] } @@ -188,6 +180,12 @@ func forkRun(opts *ForkOptions) error { return fmt.Errorf("did not understand argument: %w", err) } } else { + if defaultOwner, _ := opts.DefaultOwner(); defaultOwner != "" { + repoArg, err = ghowner.RepoToOwnerRepo(defaultOwner, repoArg) + if err != nil { + return err + } + } repoToFork, err = ghrepo.FromFullName(repoArg) if err != nil { return fmt.Errorf("argument error: %w", err) diff --git a/pkg/cmd/repo/fork/fork_test.go b/pkg/cmd/repo/fork/fork_test.go index 0f94496f0c4..9d17462443b 100644 --- a/pkg/cmd/repo/fork/fork_test.go +++ b/pkg/cmd/repo/fork/fork_test.go @@ -724,6 +724,10 @@ func TestRepoFork(t *testing.T) { return ghrepo.New("OWNER", "REPO"), nil } + tt.opts.DefaultOwner = func() (string, error) { + return "", nil + } + reg := &httpmock.Registry{} if tt.httpStubs != nil { tt.httpStubs(reg) From bf301d729008c4a68930d07da32a131186277bc3 Mon Sep 17 00:00:00 2001 From: DiegoPerezGarcia Date: Tue, 9 Jul 2024 23:19:37 +0100 Subject: [PATCH 7/9] Refactor SetDefaultOptions struct to include DefaultOwner function --- pkg/cmd/repo/setdefault/setdefault_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/cmd/repo/setdefault/setdefault_test.go b/pkg/cmd/repo/setdefault/setdefault_test.go index a89effa1499..a0b558f5197 100644 --- a/pkg/cmd/repo/setdefault/setdefault_test.go +++ b/pkg/cmd/repo/setdefault/setdefault_test.go @@ -84,6 +84,9 @@ func TestNewCmdSetDefault(t *testing.T) { f := &cmdutil.Factory{ IOStreams: io, GitClient: &git.Client{GitPath: "/fake/path/to/git"}, + DefaultOwner: func() (string, error) { + return "", nil + }, } var gotOpts *SetDefaultOptions From b7650e703b964f7e5794d8fd308e21a80dba6da1 Mon Sep 17 00:00:00 2001 From: DiegoPerezGarcia Date: Tue, 9 Jul 2024 23:21:51 +0100 Subject: [PATCH 8/9] Refactor unarchive command to use DefaultOwner field --- pkg/cmd/repo/unarchive/unarchive.go | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/pkg/cmd/repo/unarchive/unarchive.go b/pkg/cmd/repo/unarchive/unarchive.go index 066d22b4e86..55c929f6fc3 100644 --- a/pkg/cmd/repo/unarchive/unarchive.go +++ b/pkg/cmd/repo/unarchive/unarchive.go @@ -46,15 +46,7 @@ With no argument, unarchives the current repository.`), Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { if len(args) > 0 { - defaultOwner, err := opts.DefaultOwner() - if err != nil { - return err - } - repository, err := ghowner.RepoToOwnerRepo(defaultOwner, args[0]) - if err != nil { - return err - } - opts.RepoArg = repository + opts.RepoArg = args[0] } if !opts.Confirmed && !opts.IO.CanPrompt() { @@ -104,7 +96,14 @@ func unarchiveRun(opts *UnarchiveOptions) error { if err != nil { return err } - repoSelector = currentUser + "/" + repoSelector + if defaultOwner, _ := opts.DefaultOwner(); defaultOwner != "" { + repoSelector, err = ghowner.RepoToOwnerRepo(defaultOwner, repoSelector) + if err != nil { + return err + } + } else { + repoSelector = currentUser + "/" + repoSelector + } } toUnarchive, err = ghrepo.FromFullName(repoSelector) From fd6ece1a21a9b7910a2aa2b71f11f3cccc2a9661 Mon Sep 17 00:00:00 2001 From: DiegoPerezGarcia Date: Tue, 9 Jul 2024 23:23:22 +0100 Subject: [PATCH 9/9] chore: Refactor viewRun function to use DefaultOwner field This commit refactors the viewRun function in the view.go file to use the DefaultOwner field from the ViewOptions struct. The DefaultOwner field is a function that retrieves the default owner from the configuration. By using this field, the code ensures that the owner is set to the default value when no owner is provided as an argument. This change improves flexibility and convenience when viewing repositories. --- pkg/cmd/repo/view/view.go | 19 +++++++++---------- pkg/cmd/repo/view/view_test.go | 3 +++ 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/pkg/cmd/repo/view/view.go b/pkg/cmd/repo/view/view.go index 6ec762cc3f2..dd94a070888 100644 --- a/pkg/cmd/repo/view/view.go +++ b/pkg/cmd/repo/view/view.go @@ -58,15 +58,7 @@ With '--branch', view a specific branch of the repository.`, Args: cobra.MaximumNArgs(1), RunE: func(c *cobra.Command, args []string) error { if len(args) > 0 { - defaultOwner, err := opts.DefaultOwner() - if err != nil { - return err - } - repository, err := ghowner.RepoToOwnerRepo(defaultOwner, args[0]) - if err != nil { - return err - } - opts.RepoArg = repository + opts.RepoArg = args[0] } if runF != nil { return runF(&opts) @@ -112,7 +104,14 @@ func viewRun(opts *ViewOptions) error { if err != nil { return err } - viewURL = currentUser + "/" + viewURL + if defaultOwner, _ := opts.DefaultOwner(); defaultOwner != "" { + viewURL, err = ghowner.RepoToOwnerRepo(defaultOwner, viewURL) + if err != nil { + return err + } + } else { + viewURL = currentUser + "/" + viewURL + } } toView, err = ghrepo.FromFullName(viewURL) if err != nil { diff --git a/pkg/cmd/repo/view/view_test.go b/pkg/cmd/repo/view/view_test.go index a8361e9ef69..21cb3225589 100644 --- a/pkg/cmd/repo/view/view_test.go +++ b/pkg/cmd/repo/view/view_test.go @@ -534,6 +534,9 @@ func Test_ViewRun_WithoutUsername(t *testing.T) { Config: func() (gh.Config, error) { return config.NewBlankConfig(), nil }, + DefaultOwner: func() (string, error) { + return "", nil + }, } if err := viewRun(opts); err != nil {