From 1ec47d8191ca37eede24fe34e588823c8a0e965f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 21 Jun 2021 16:06:03 +0200 Subject: [PATCH 1/4] Improvements to gh extensions - Extensions on Windows now enabled through the `sh.exe` interpreter - `sh.exe` now found on Windows when git was installed via scoop - `gh extensions list` command shows origin repo for the extension - `gh extensions upgrade --all` is required to upgrade all extensions - Added `gh extensions remove` - Shell completions now include aliases and extension names - `gh` help output now lists available extension names - Extensions are stored to XDG_DATA_HOME --- cmd/gh/main.go | 21 +- pkg/cmd/alias/expand/expand.go | 35 +--- pkg/cmd/alias/set/set.go | 20 ++ pkg/cmd/alias/set/set_test.go | 6 + pkg/cmd/extensions/command.go | 80 +++++-- pkg/cmd/extensions/command_test.go | 148 +++++++++++++ pkg/cmd/extensions/extension.go | 23 ++ pkg/cmd/extensions/manager.go | 97 +++++++-- pkg/cmd/extensions/manager_test.go | 139 +++++++++++++ pkg/cmd/factory/default.go | 3 + pkg/cmd/root/help.go | 15 +- pkg/cmd/root/root.go | 12 +- pkg/cmdutil/factory.go | 3 + pkg/extensions/extension.go | 22 ++ pkg/extensions/extension_mock.go | 138 ++++++++++++ pkg/extensions/manager_mock.go | 324 +++++++++++++++++++++++++++++ pkg/findsh/find.go | 10 + pkg/findsh/find_windows.go | 35 ++++ 18 files changed, 1064 insertions(+), 67 deletions(-) create mode 100644 pkg/cmd/extensions/command_test.go create mode 100644 pkg/cmd/extensions/extension.go create mode 100644 pkg/cmd/extensions/manager_test.go create mode 100644 pkg/extensions/extension.go create mode 100644 pkg/extensions/extension_mock.go create mode 100644 pkg/extensions/manager_mock.go create mode 100644 pkg/findsh/find.go create mode 100644 pkg/findsh/find_windows.go diff --git a/cmd/gh/main.go b/cmd/gh/main.go index 7ee00b061..d507c3d1c 100644 --- a/cmd/gh/main.go +++ b/cmd/gh/main.go @@ -21,7 +21,6 @@ import ( "github.com/cli/cli/internal/run" "github.com/cli/cli/internal/update" "github.com/cli/cli/pkg/cmd/alias/expand" - "github.com/cli/cli/pkg/cmd/extensions" "github.com/cli/cli/pkg/cmd/factory" "github.com/cli/cli/pkg/cmd/root" "github.com/cli/cli/pkg/cmdutil" @@ -143,7 +142,7 @@ func mainRun() exitCode { return exitOK } else if c, _, err := rootCmd.Traverse(expandedArgs); err == nil && c == rootCmd && len(expandedArgs) > 0 { - extensionManager := extensions.NewManager() + extensionManager := cmdFactory.ExtensionManager if found, err := extensionManager.Dispatch(expandedArgs, os.Stdin, os.Stdout, os.Stderr); err != nil { var execError *exec.ExitError if errors.As(err, &execError) { @@ -157,6 +156,24 @@ func mainRun() exitCode { } } + // provide completions for aliases and extensions + rootCmd.ValidArgsFunction = func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { + var results []string + if aliases, err := cfg.Aliases(); err == nil { + for aliasName := range aliases.All() { + if strings.HasPrefix(aliasName, toComplete) { + results = append(results, aliasName) + } + } + } + for _, ext := range cmdFactory.ExtensionManager.List() { + if strings.HasPrefix(ext.Name(), toComplete) { + results = append(results, ext.Name()) + } + } + return results, cobra.ShellCompDirectiveNoFileComp + } + cs := cmdFactory.IOStreams.ColorScheme() if cmd != nil && cmdutil.IsAuthCheckEnabled(cmd) && !cmdutil.CheckAuth(cfg) { diff --git a/pkg/cmd/alias/expand/expand.go b/pkg/cmd/alias/expand/expand.go index ec791df52..e48262298 100644 --- a/pkg/cmd/alias/expand/expand.go +++ b/pkg/cmd/alias/expand/expand.go @@ -3,14 +3,13 @@ package expand import ( "errors" "fmt" - "os" - "path/filepath" + "os/exec" "regexp" "runtime" "strings" "github.com/cli/cli/internal/config" - "github.com/cli/safeexec" + "github.com/cli/cli/pkg/findsh" "github.com/google/shlex" ) @@ -80,27 +79,15 @@ func ExpandAlias(cfg config.Config, args []string, findShFunc func() (string, er } func findSh() (string, error) { - shPath, err := safeexec.LookPath("sh") - if err == nil { - return shPath, nil - } - - if runtime.GOOS == "windows" { - winNotFoundErr := errors.New("unable to locate sh to execute the shell alias with. The sh.exe interpreter is typically distributed with Git for Windows.") - // We can try and find a sh executable in a Git for Windows install - gitPath, err := safeexec.LookPath("git") - if err != nil { - return "", winNotFoundErr + shPath, err := findsh.Find() + if err != nil { + if errors.Is(err, exec.ErrNotFound) { + if runtime.GOOS == "windows" { + return "", errors.New("unable to locate sh to execute the shell alias with. The sh.exe interpreter is typically distributed with Git for Windows.") + } + return "", errors.New("unable to locate sh to execute shell alias with") } - - shPath = filepath.Join(filepath.Dir(gitPath), "..", "bin", "sh.exe") - _, err = os.Stat(shPath) - if err != nil { - return "", winNotFoundErr - } - - return shPath, nil + return "", err } - - return "", errors.New("unable to locate sh to execute shell alias with") + return shPath, nil } diff --git a/pkg/cmd/alias/set/set.go b/pkg/cmd/alias/set/set.go index e7c4ea6d5..e50ca3f86 100644 --- a/pkg/cmd/alias/set/set.go +++ b/pkg/cmd/alias/set/set.go @@ -92,6 +92,26 @@ func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command cmd.Flags().BoolVarP(&opts.IsShell, "shell", "s", false, "Declare an alias to be passed through a shell interpreter") + opts.validCommand = func(args string) bool { + split, err := shlex.Split(args) + if err != nil { + return false + } + + rootCmd := cmd.Root() + cmd, _, err := rootCmd.Traverse(split) + if err == nil && cmd != rootCmd { + return true + } + + for _, ext := range f.ExtensionManager.List() { + if ext.Name() == split[0] { + return true + } + } + return false + } + return cmd } diff --git a/pkg/cmd/alias/set/set_test.go b/pkg/cmd/alias/set/set_test.go index 0e95a3c46..eaa5a9390 100644 --- a/pkg/cmd/alias/set/set_test.go +++ b/pkg/cmd/alias/set/set_test.go @@ -8,6 +8,7 @@ import ( "github.com/MakeNowJust/heredoc" "github.com/cli/cli/internal/config" "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/extensions" "github.com/cli/cli/pkg/iostreams" "github.com/cli/cli/test" "github.com/google/shlex" @@ -28,6 +29,11 @@ func runCommand(cfg config.Config, isTTY bool, cli string, in string) (*test.Cmd Config: func() (config.Config, error) { return cfg, nil }, + ExtensionManager: &extensions.ExtensionManagerMock{ + ListFunc: func() []extensions.Extension { + return []extensions.Extension{} + }, + }, } cmd := NewCmdSet(factory, nil) diff --git a/pkg/cmd/extensions/command.go b/pkg/cmd/extensions/command.go index 298787dcb..f764e22e5 100644 --- a/pkg/cmd/extensions/command.go +++ b/pkg/cmd/extensions/command.go @@ -4,21 +4,32 @@ import ( "errors" "fmt" "os" - "path/filepath" "strings" + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/git" "github.com/cli/cli/internal/ghrepo" "github.com/cli/cli/pkg/cmdutil" - "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/utils" "github.com/spf13/cobra" ) -func NewCmdExtensions(io *iostreams.IOStreams) *cobra.Command { - m := NewManager() +func NewCmdExtensions(f *cmdutil.Factory) *cobra.Command { + m := f.ExtensionManager + io := f.IOStreams extCmd := cobra.Command{ Use: "extensions", Short: "Manage gh extensions", + Long: heredoc.Docf(` + GitHub CLI extensions are repositories that provide additional gh commands. + + The name of the extension repository must start with "gh-" and it must contain an + executable of the same name. All arguments passed to the %[1]sgh %[1]s invocation + will be forwarded to the %[1]sgh-%[1]s executable of the extension. + + An extension cannot override any of the core gh commands. + `, "`"), } extCmd.AddCommand( @@ -31,12 +42,23 @@ func NewCmdExtensions(io *iostreams.IOStreams) *cobra.Command { if len(cmds) == 0 { return errors.New("no extensions installed") } + // cs := io.ColorScheme() + t := utils.NewTablePrinter(io) for _, c := range cmds { - name := filepath.Base(c) - parts := strings.SplitN(name, "-", 2) - fmt.Fprintf(io.Out, "%s %s\n", parts[0], parts[1]) + var repo string + if u, err := git.ParseURL(c.URL()); err == nil { + if r, err := ghrepo.FromURL(u); err == nil { + repo = ghrepo.FullName(r) + } + } + + t.AddField(fmt.Sprintf("gh %s", c.Name()), nil, nil) + t.AddField(repo, nil, nil) + // TODO: add notice about available update + //t.AddField("Update available", nil, cs.Green) + t.EndRow() } - return nil + return t.Render() }, }, &cobra.Command{ @@ -58,16 +80,48 @@ func NewCmdExtensions(io *iostreams.IOStreams) *cobra.Command { if !strings.HasPrefix(repo.RepoName(), "gh-") { return errors.New("the repository name must start with `gh-`") } - protocol := "https" // TODO: respect user's preferred protocol + cfg, err := f.Config() + if err != nil { + return err + } + protocol, _ := cfg.Get(repo.RepoHost(), "git_protocol") return m.Install(ghrepo.FormatRemoteURL(repo, protocol), io.Out, io.ErrOut) }, }, + func() *cobra.Command { + var flagAll bool + cmd := &cobra.Command{ + Use: "upgrade { | --all}", + Short: "Upgrade installed extensions", + Args: func(cmd *cobra.Command, args []string) error { + if len(args) == 0 && !flagAll { + return &cmdutil.FlagError{Err: errors.New("must specify an extension to upgrade")} + } + if len(args) > 0 && flagAll { + return &cmdutil.FlagError{Err: errors.New("cannot use `--all` with extension name")} + } + if len(args) > 1 { + return &cmdutil.FlagError{Err: errors.New("too many arguments")} + } + return nil + }, + RunE: func(cmd *cobra.Command, args []string) error { + var name string + if len(args) > 0 { + name = args[0] + } + return m.Upgrade(name, io.Out, io.ErrOut) + }, + } + cmd.Flags().BoolVar(&flagAll, "all", false, "Upgrade all extensions") + return cmd + }(), &cobra.Command{ - Use: "upgrade", - Short: "Upgrade installed extensions", - Args: cobra.NoArgs, + Use: "remove", + Short: "Remove an installed extension", + Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - return m.Upgrade(io.Out, io.ErrOut) + return m.Remove(args[0]) }, }, ) diff --git a/pkg/cmd/extensions/command_test.go b/pkg/cmd/extensions/command_test.go new file mode 100644 index 000000000..b9ce752ba --- /dev/null +++ b/pkg/cmd/extensions/command_test.go @@ -0,0 +1,148 @@ +package extensions + +import ( + "io" + "io/ioutil" + "os" + "strings" + "testing" + + "github.com/cli/cli/internal/config" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/extensions" + "github.com/cli/cli/pkg/iostreams" + "github.com/stretchr/testify/assert" +) + +func TestNewCmdExtensions(t *testing.T) { + tempDir := t.TempDir() + assert.NoError(t, os.Chdir(tempDir)) + + tests := []struct { + name string + args []string + managerStubs func(em *extensions.ExtensionManagerMock) func(*testing.T) + wantErr bool + wantStdout string + wantStderr string + }{ + { + name: "install an extension", + args: []string{"install", "owner/gh-some-ext"}, + managerStubs: func(em *extensions.ExtensionManagerMock) func(*testing.T) { + em.InstallFunc = func(s string, out, errOut io.Writer) error { + return nil + } + return func(t *testing.T) { + calls := em.InstallCalls() + assert.Equal(t, 1, len(calls)) + assert.Equal(t, "https://github.com/owner/gh-some-ext.git", calls[0].URL) + } + }, + }, + { + name: "install local extension", + args: []string{"install", "."}, + managerStubs: func(em *extensions.ExtensionManagerMock) func(*testing.T) { + em.InstallLocalFunc = func(dir string) error { + return nil + } + return func(t *testing.T) { + calls := em.InstallLocalCalls() + assert.Equal(t, 1, len(calls)) + assert.Equal(t, tempDir, normalizeDir(calls[0].Dir)) + } + }, + }, + { + name: "upgrade error", + args: []string{"upgrade"}, + wantErr: true, + }, + { + name: "upgrade an extension", + args: []string{"upgrade", "hello"}, + managerStubs: func(em *extensions.ExtensionManagerMock) func(*testing.T) { + em.UpgradeFunc = func(name string, out, errOut io.Writer) error { + return nil + } + return func(t *testing.T) { + calls := em.UpgradeCalls() + assert.Equal(t, 1, len(calls)) + assert.Equal(t, "hello", calls[0].Name) + } + }, + }, + { + name: "upgrade all", + args: []string{"upgrade", "--all"}, + managerStubs: func(em *extensions.ExtensionManagerMock) func(*testing.T) { + em.UpgradeFunc = func(name string, out, errOut io.Writer) error { + return nil + } + return func(t *testing.T) { + calls := em.UpgradeCalls() + assert.Equal(t, 1, len(calls)) + assert.Equal(t, "", calls[0].Name) + } + }, + }, + { + name: "remove extension", + args: []string{"remove", "hello"}, + managerStubs: func(em *extensions.ExtensionManagerMock) func(*testing.T) { + em.RemoveFunc = func(name string) error { + return nil + } + return func(t *testing.T) { + calls := em.RemoveCalls() + assert.Equal(t, 1, len(calls)) + assert.Equal(t, "hello", calls[0].Name) + } + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ios, _, stdout, stderr := iostreams.Test() + + var assertFunc func(*testing.T) + em := &extensions.ExtensionManagerMock{} + if tt.managerStubs != nil { + assertFunc = tt.managerStubs(em) + } + + f := cmdutil.Factory{ + Config: func() (config.Config, error) { + return config.NewBlankConfig(), nil + }, + IOStreams: ios, + ExtensionManager: em, + } + + cmd := NewCmdExtensions(&f) + cmd.SetArgs(tt.args) + cmd.SetOut(ioutil.Discard) + cmd.SetErr(ioutil.Discard) + + _, err := cmd.ExecuteC() + if tt.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + + if assertFunc != nil { + assertFunc(t) + } + + assert.Equal(t, tt.wantStdout, stdout.String()) + assert.Equal(t, tt.wantStderr, stderr.String()) + }) + } +} + +func normalizeDir(d string) string { + return strings.TrimPrefix(d, "/private") +} diff --git a/pkg/cmd/extensions/extension.go b/pkg/cmd/extensions/extension.go new file mode 100644 index 000000000..33b2f8d67 --- /dev/null +++ b/pkg/cmd/extensions/extension.go @@ -0,0 +1,23 @@ +package extensions + +import ( + "path/filepath" + "strings" +) + +type Extension struct { + path string + url string +} + +func (e *Extension) Name() string { + return strings.TrimPrefix(filepath.Base(e.path), "gh-") +} + +func (e *Extension) Path() string { + return e.path +} + +func (e *Extension) URL() string { + return e.url +} diff --git a/pkg/cmd/extensions/manager.go b/pkg/cmd/extensions/manager.go index c3988b483..09520a930 100644 --- a/pkg/cmd/extensions/manager.go +++ b/pkg/cmd/extensions/manager.go @@ -1,6 +1,7 @@ package extensions import ( + "bytes" "errors" "fmt" "io" @@ -9,21 +10,28 @@ import ( "os/exec" "path" "path/filepath" + "runtime" "strings" "github.com/cli/cli/internal/config" + "github.com/cli/cli/pkg/extensions" + "github.com/cli/cli/pkg/findsh" "github.com/cli/safeexec" ) type Manager struct { - dataDir func() string - lookPath func(string) (string, error) + dataDir func() string + lookPath func(string) (string, error) + findSh func() (string, error) + newCommand func(string, ...string) *exec.Cmd } func NewManager() *Manager { return &Manager{ - dataDir: config.ConfigDir, - lookPath: safeexec.LookPath, + dataDir: config.DataDir, + lookPath: safeexec.LookPath, + findSh: findsh.Find, + newCommand: exec.Command, } } @@ -33,12 +41,12 @@ func (m *Manager) Dispatch(args []string, stdin io.Reader, stdout, stderr io.Wri } var exe string - extName := "gh-" + args[0] + extName := args[0] forwardArgs := args[1:] - for _, e := range m.List() { - if filepath.Base(e) == extName { - exe = e + for _, e := range m.list(false) { + if e.Name() == extName { + exe = e.Path() break } } @@ -46,27 +54,63 @@ func (m *Manager) Dispatch(args []string, stdin io.Reader, stdout, stderr io.Wri return false, nil } - // TODO: parse the shebang on Windows and invoke the correct interpreter instead of invoking directly - externalCmd := exec.Command(exe, forwardArgs...) + var externalCmd *exec.Cmd + + if runtime.GOOS == "windows" { + // Dispatch all extension calls through the `sh` interpreter to support executable files with a + // shebang line on Windows. + shExe, err := m.findSh() + if err != nil { + if errors.Is(err, exec.ErrNotFound) { + return true, errors.New("the `sh.exe` interpreter is required. Please install Git for Windows and try again") + } + return true, err + } + forwardArgs = append([]string{"-c", `command "$@"`, "--", exe}, forwardArgs...) + externalCmd = m.newCommand(shExe, forwardArgs...) + } else { + externalCmd = m.newCommand(exe, forwardArgs...) + } externalCmd.Stdin = stdin externalCmd.Stdout = stdout externalCmd.Stderr = stderr return true, externalCmd.Run() } -func (m *Manager) List() []string { +func (m *Manager) List() []extensions.Extension { + return m.list(true) +} + +func (m *Manager) list(includeMetadata bool) []extensions.Extension { dir := m.installDir() entries, err := ioutil.ReadDir(dir) if err != nil { return nil } - var results []string + var gitExe string + if includeMetadata { + gitExe, _ = m.lookPath("git") + } + + var results []extensions.Extension for _, f := range entries { if !strings.HasPrefix(f.Name(), "gh-") || !(f.IsDir() || f.Mode()&os.ModeSymlink != 0) { continue } - results = append(results, filepath.Join(dir, f.Name(), f.Name())) + var remoteURL string + if gitExe != "" { + stdout := bytes.Buffer{} + cmd := m.newCommand(gitExe, "--git-dir="+filepath.Join(dir, f.Name(), ".git"), "config", "remote.origin.url") + cmd.Stdout = &stdout + if err := cmd.Run(); err == nil { + remoteURL = strings.TrimSpace(stdout.String()) + } + } + results = append(results, &Extension{ + path: filepath.Join(dir, f.Name(), f.Name()), + url: remoteURL, + }) } return results } @@ -86,13 +130,13 @@ func (m *Manager) Install(cloneURL string, stdout, stderr io.Writer) error { name := strings.TrimSuffix(path.Base(cloneURL), ".git") targetDir := filepath.Join(m.installDir(), name) - externalCmd := exec.Command(exe, "clone", cloneURL, targetDir) + externalCmd := m.newCommand(exe, "clone", cloneURL, targetDir) externalCmd.Stdout = stdout externalCmd.Stderr = stderr return externalCmd.Run() } -func (m *Manager) Upgrade(stdout, stderr io.Writer) error { +func (m *Manager) Upgrade(name string, stdout, stderr io.Writer) error { exe, err := m.lookPath("git") if err != nil { return err @@ -103,19 +147,36 @@ func (m *Manager) Upgrade(stdout, stderr io.Writer) error { return errors.New("no extensions installed") } + someUpgraded := false for _, f := range exts { - fmt.Fprintf(stdout, "[%s]: ", filepath.Base(f)) - dir := filepath.Dir(f) - externalCmd := exec.Command(exe, "-C", dir, "--git-dir="+filepath.Join(dir, ".git"), "pull", "--ff-only") + if name == "" { + fmt.Fprintf(stdout, "[%s]: ", f.Name()) + } else if f.Name() != name { + continue + } + dir := filepath.Dir(f.Path()) + externalCmd := m.newCommand(exe, "-C", dir, "--git-dir="+filepath.Join(dir, ".git"), "pull", "--ff-only") externalCmd.Stdout = stdout externalCmd.Stderr = stderr if e := externalCmd.Run(); e != nil { err = e } + someUpgraded = true + } + if err == nil && !someUpgraded { + err = fmt.Errorf("no extension matched %q", name) } return err } +func (m *Manager) Remove(name string) error { + targetDir := filepath.Join(m.installDir(), "gh-"+name) + if _, err := os.Stat(targetDir); os.IsNotExist(err) { + return fmt.Errorf("no extension found: %q", targetDir) + } + return os.RemoveAll(targetDir) +} + func (m *Manager) installDir() string { return filepath.Join(m.dataDir(), "extensions") } diff --git a/pkg/cmd/extensions/manager_test.go b/pkg/cmd/extensions/manager_test.go new file mode 100644 index 000000000..d829b156a --- /dev/null +++ b/pkg/cmd/extensions/manager_test.go @@ -0,0 +1,139 @@ +package extensions + +import ( + "bytes" + "fmt" + "io/ioutil" + "os" + "os/exec" + "path/filepath" + "runtime" + "testing" + + "github.com/MakeNowJust/heredoc" + "github.com/stretchr/testify/assert" +) + +func TestHelperProcess(t *testing.T) { + if os.Getenv("GH_WANT_HELPER_PROCESS") != "1" { + return + } + if err := func(args []string) error { + fmt.Fprintf(os.Stdout, "%v\n", args) + return nil + }(os.Args[3:]); err != nil { + fmt.Fprintln(os.Stderr, err) + os.Exit(1) + } + os.Exit(0) +} + +func newTestManager(dir string) *Manager { + return &Manager{ + dataDir: func() string { return dir }, + lookPath: func(exe string) (string, error) { return exe, nil }, + findSh: func() (string, error) { return "sh", nil }, + newCommand: func(exe string, args ...string) *exec.Cmd { + args = append([]string{os.Args[0], "-test.run=TestHelperProcess", "--", exe}, args...) + cmd := exec.Command(args[0], args[1:]...) + cmd.Env = []string{"GH_WANT_HELPER_PROCESS=1"} + return cmd + }, + } +} + +func TestManager_List(t *testing.T) { + tempDir := t.TempDir() + assert.NoError(t, stubExecutable(filepath.Join(tempDir, "extensions", "gh-hello", "gh-hello"))) + assert.NoError(t, stubExecutable(filepath.Join(tempDir, "extensions", "gh-two", "gh-two"))) + + m := newTestManager(tempDir) + exts := m.List() + assert.Equal(t, 2, len(exts)) + assert.Equal(t, "hello", exts[0].Name()) + assert.Equal(t, "two", exts[1].Name()) +} + +func TestManager_Dispatch(t *testing.T) { + tempDir := t.TempDir() + extPath := filepath.Join(tempDir, "extensions", "gh-hello", "gh-hello") + assert.NoError(t, stubExecutable(extPath)) + + m := newTestManager(tempDir) + + stdout := &bytes.Buffer{} + stderr := &bytes.Buffer{} + found, err := m.Dispatch([]string{"hello", "one", "two"}, nil, stdout, stderr) + assert.NoError(t, err) + assert.True(t, found) + + if runtime.GOOS == "windows" { + assert.Equal(t, fmt.Sprintf("[sh -c command \"$@\" -- %s one two]\n", extPath), stdout.String()) + } else { + assert.Equal(t, fmt.Sprintf("[%s one two]\n", extPath), stdout.String()) + } + assert.Equal(t, "", stderr.String()) +} + +func TestManager_Remove(t *testing.T) { + tempDir := t.TempDir() + assert.NoError(t, stubExecutable(filepath.Join(tempDir, "extensions", "gh-hello", "gh-hello"))) + assert.NoError(t, stubExecutable(filepath.Join(tempDir, "extensions", "gh-two", "gh-two"))) + + m := newTestManager(tempDir) + err := m.Remove("hello") + assert.NoError(t, err) + + items, err := ioutil.ReadDir(filepath.Join(tempDir, "extensions")) + assert.NoError(t, err) + assert.Equal(t, 1, len(items)) + assert.Equal(t, "gh-two", items[0].Name()) +} + +func TestManager_Upgrade(t *testing.T) { + tempDir := t.TempDir() + assert.NoError(t, stubExecutable(filepath.Join(tempDir, "extensions", "gh-hello", "gh-hello"))) + assert.NoError(t, stubExecutable(filepath.Join(tempDir, "extensions", "gh-two", "gh-two"))) + + m := newTestManager(tempDir) + + stdout := &bytes.Buffer{} + stderr := &bytes.Buffer{} + err := m.Upgrade("", stdout, stderr) + assert.NoError(t, err) + + assert.Equal(t, heredoc.Docf( + ` + [hello]: [git -C %s --git-dir=%s pull --ff-only] + [two]: [git -C %s --git-dir=%s pull --ff-only] + `, + filepath.Join(tempDir, "extensions", "gh-hello"), + filepath.Join(tempDir, "extensions", "gh-hello", ".git"), + filepath.Join(tempDir, "extensions", "gh-two"), + filepath.Join(tempDir, "extensions", "gh-two", ".git"), + ), stdout.String()) + assert.Equal(t, "", stderr.String()) +} + +func TestManager_Install(t *testing.T) { + tempDir := t.TempDir() + m := newTestManager(tempDir) + + stdout := &bytes.Buffer{} + stderr := &bytes.Buffer{} + err := m.Install("https://github.com/owner/gh-some-ext.git", stdout, stderr) + assert.NoError(t, err) + assert.Equal(t, fmt.Sprintf("[git clone https://github.com/owner/gh-some-ext.git %s]\n", filepath.Join(tempDir, "extensions", "gh-some-ext")), stdout.String()) + assert.Equal(t, "", stderr.String()) +} + +func stubExecutable(path string) error { + if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil { + return err + } + f, err := os.OpenFile(path, os.O_CREATE, 0755) + if err != nil { + return err + } + return f.Close() +} diff --git a/pkg/cmd/factory/default.go b/pkg/cmd/factory/default.go index eec60bc56..b7fb9dd13 100644 --- a/pkg/cmd/factory/default.go +++ b/pkg/cmd/factory/default.go @@ -11,6 +11,7 @@ import ( "github.com/cli/cli/git" "github.com/cli/cli/internal/config" "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/pkg/cmd/extensions" "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/iostreams" ) @@ -20,6 +21,8 @@ func New(appVersion string) *cmdutil.Factory { Config: configFunc(), // No factory dependencies Branch: branchFunc(), // No factory dependencies Executable: executable(), // No factory dependencies + + ExtensionManager: extensions.NewManager(), } f.IOStreams = ioStreams(f) // Depends on Config diff --git a/pkg/cmd/root/help.go b/pkg/cmd/root/help.go index f419002fb..a33088266 100644 --- a/pkg/cmd/root/help.go +++ b/pkg/cmd/root/help.go @@ -6,7 +6,6 @@ import ( "strings" "github.com/cli/cli/pkg/cmdutil" - "github.com/cli/cli/pkg/iostreams" "github.com/cli/cli/pkg/text" "github.com/spf13/cobra" "github.com/spf13/pflag" @@ -80,7 +79,9 @@ func isRootCmd(command *cobra.Command) bool { return command != nil && !command.HasParent() } -func rootHelpFunc(cs *iostreams.ColorScheme, command *cobra.Command, args []string) { +func rootHelpFunc(f *cmdutil.Factory, command *cobra.Command, args []string) { + cs := f.IOStreams.ColorScheme() + if isRootCmd(command.Parent()) && len(args) >= 2 && args[1] != "--help" && args[1] != "-h" { nestedSuggestFunc(command, args[1]) hasFailed = true @@ -143,6 +144,16 @@ func rootHelpFunc(cs *iostreams.ColorScheme, command *cobra.Command, args []stri helpEntries = append(helpEntries, helpEntry{"ADDITIONAL COMMANDS", strings.Join(additionalCommands, "\n")}) } + if isRootCmd(command) { + if exts := f.ExtensionManager.List(); len(exts) > 0 { + var names []string + for _, ext := range exts { + names = append(names, ext.Name()) + } + helpEntries = append(helpEntries, helpEntry{"EXTENSION COMMANDS", strings.Join(names, "\n")}) + } + } + flagUsages := command.LocalFlags().FlagUsages() if flagUsages != "" { helpEntries = append(helpEntries, helpEntry{"FLAGS", dedent(flagUsages)}) diff --git a/pkg/cmd/root/root.go b/pkg/cmd/root/root.go index 7876eb1f9..141f06db2 100644 --- a/pkg/cmd/root/root.go +++ b/pkg/cmd/root/root.go @@ -53,14 +53,10 @@ func NewCmdRoot(f *cmdutil.Factory, version, buildDate string) *cobra.Command { cmd.SetOut(f.IOStreams.Out) cmd.SetErr(f.IOStreams.ErrOut) - cs := f.IOStreams.ColorScheme() - - helpHelper := func(command *cobra.Command, args []string) { - rootHelpFunc(cs, command, args) - } - cmd.PersistentFlags().Bool("help", false, "Show help for command") - cmd.SetHelpFunc(helpHelper) + cmd.SetHelpFunc(func(cmd *cobra.Command, args []string) { + rootHelpFunc(f, cmd, args) + }) cmd.SetUsageFunc(rootUsageFunc) cmd.SetFlagErrorFunc(rootFlagErrorFunc) @@ -78,7 +74,7 @@ func NewCmdRoot(f *cmdutil.Factory, version, buildDate string) *cobra.Command { cmd.AddCommand(creditsCmd.NewCmdCredits(f, nil)) cmd.AddCommand(gistCmd.NewCmdGist(f)) cmd.AddCommand(completionCmd.NewCmdCompletion(f.IOStreams)) - cmd.AddCommand(extensionsCmd.NewCmdExtensions(f.IOStreams)) + cmd.AddCommand(extensionsCmd.NewCmdExtensions(f)) cmd.AddCommand(secretCmd.NewCmdSecret(f)) cmd.AddCommand(sshKeyCmd.NewCmdSSHKey(f)) diff --git a/pkg/cmdutil/factory.go b/pkg/cmdutil/factory.go index 96b6ec0b8..254d2f2df 100644 --- a/pkg/cmdutil/factory.go +++ b/pkg/cmdutil/factory.go @@ -6,6 +6,7 @@ import ( "github.com/cli/cli/context" "github.com/cli/cli/internal/config" "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/pkg/extensions" "github.com/cli/cli/pkg/iostreams" ) @@ -23,6 +24,8 @@ type Factory struct { Config func() (config.Config, error) Branch func() (string, error) + ExtensionManager extensions.ExtensionManager + // Executable is the path to the currently invoked gh binary Executable string } diff --git a/pkg/extensions/extension.go b/pkg/extensions/extension.go new file mode 100644 index 000000000..c0d65e297 --- /dev/null +++ b/pkg/extensions/extension.go @@ -0,0 +1,22 @@ +package extensions + +import ( + "io" +) + +//go:generate moq -out extension_mock.go . Extension +type Extension interface { + Name() string + Path() string + URL() string +} + +//go:generate moq -out manager_mock.go . ExtensionManager +type ExtensionManager interface { + List() []Extension + Install(url string, stdout, stderr io.Writer) error + InstallLocal(dir string) error + Upgrade(name string, stdout, stderr io.Writer) error + Remove(name string) error + Dispatch(args []string, stdin io.Reader, stdout, stderr io.Writer) (bool, error) +} diff --git a/pkg/extensions/extension_mock.go b/pkg/extensions/extension_mock.go new file mode 100644 index 000000000..fb32db5b1 --- /dev/null +++ b/pkg/extensions/extension_mock.go @@ -0,0 +1,138 @@ +// Code generated by moq; DO NOT EDIT. +// github.com/matryer/moq + +package extensions + +import ( + "sync" +) + +// Ensure, that ExtensionMock does implement Extension. +// If this is not the case, regenerate this file with moq. +var _ Extension = &ExtensionMock{} + +// ExtensionMock is a mock implementation of Extension. +// +// func TestSomethingThatUsesExtension(t *testing.T) { +// +// // make and configure a mocked Extension +// mockedExtension := &ExtensionMock{ +// NameFunc: func() string { +// panic("mock out the Name method") +// }, +// PathFunc: func() string { +// panic("mock out the Path method") +// }, +// URLFunc: func() string { +// panic("mock out the URL method") +// }, +// } +// +// // use mockedExtension in code that requires Extension +// // and then make assertions. +// +// } +type ExtensionMock struct { + // NameFunc mocks the Name method. + NameFunc func() string + + // PathFunc mocks the Path method. + PathFunc func() string + + // URLFunc mocks the URL method. + URLFunc func() string + + // calls tracks calls to the methods. + calls struct { + // Name holds details about calls to the Name method. + Name []struct { + } + // Path holds details about calls to the Path method. + Path []struct { + } + // URL holds details about calls to the URL method. + URL []struct { + } + } + lockName sync.RWMutex + lockPath sync.RWMutex + lockURL sync.RWMutex +} + +// Name calls NameFunc. +func (mock *ExtensionMock) Name() string { + if mock.NameFunc == nil { + panic("ExtensionMock.NameFunc: method is nil but Extension.Name was just called") + } + callInfo := struct { + }{} + mock.lockName.Lock() + mock.calls.Name = append(mock.calls.Name, callInfo) + mock.lockName.Unlock() + return mock.NameFunc() +} + +// NameCalls gets all the calls that were made to Name. +// Check the length with: +// len(mockedExtension.NameCalls()) +func (mock *ExtensionMock) NameCalls() []struct { +} { + var calls []struct { + } + mock.lockName.RLock() + calls = mock.calls.Name + mock.lockName.RUnlock() + return calls +} + +// Path calls PathFunc. +func (mock *ExtensionMock) Path() string { + if mock.PathFunc == nil { + panic("ExtensionMock.PathFunc: method is nil but Extension.Path was just called") + } + callInfo := struct { + }{} + mock.lockPath.Lock() + mock.calls.Path = append(mock.calls.Path, callInfo) + mock.lockPath.Unlock() + return mock.PathFunc() +} + +// PathCalls gets all the calls that were made to Path. +// Check the length with: +// len(mockedExtension.PathCalls()) +func (mock *ExtensionMock) PathCalls() []struct { +} { + var calls []struct { + } + mock.lockPath.RLock() + calls = mock.calls.Path + mock.lockPath.RUnlock() + return calls +} + +// URL calls URLFunc. +func (mock *ExtensionMock) URL() string { + if mock.URLFunc == nil { + panic("ExtensionMock.URLFunc: method is nil but Extension.URL was just called") + } + callInfo := struct { + }{} + mock.lockURL.Lock() + mock.calls.URL = append(mock.calls.URL, callInfo) + mock.lockURL.Unlock() + return mock.URLFunc() +} + +// URLCalls gets all the calls that were made to URL. +// Check the length with: +// len(mockedExtension.URLCalls()) +func (mock *ExtensionMock) URLCalls() []struct { +} { + var calls []struct { + } + mock.lockURL.RLock() + calls = mock.calls.URL + mock.lockURL.RUnlock() + return calls +} diff --git a/pkg/extensions/manager_mock.go b/pkg/extensions/manager_mock.go new file mode 100644 index 000000000..af71a7904 --- /dev/null +++ b/pkg/extensions/manager_mock.go @@ -0,0 +1,324 @@ +// Code generated by moq; DO NOT EDIT. +// github.com/matryer/moq + +package extensions + +import ( + "io" + "sync" +) + +// Ensure, that ExtensionManagerMock does implement ExtensionManager. +// If this is not the case, regenerate this file with moq. +var _ ExtensionManager = &ExtensionManagerMock{} + +// ExtensionManagerMock is a mock implementation of ExtensionManager. +// +// func TestSomethingThatUsesExtensionManager(t *testing.T) { +// +// // make and configure a mocked ExtensionManager +// mockedExtensionManager := &ExtensionManagerMock{ +// DispatchFunc: func(args []string, stdin io.Reader, stdout io.Writer, stderr io.Writer) (bool, error) { +// panic("mock out the Dispatch method") +// }, +// InstallFunc: func(url string, stdout io.Writer, stderr io.Writer) error { +// panic("mock out the Install method") +// }, +// InstallLocalFunc: func(dir string) error { +// panic("mock out the InstallLocal method") +// }, +// ListFunc: func() []Extension { +// panic("mock out the List method") +// }, +// RemoveFunc: func(name string) error { +// panic("mock out the Remove method") +// }, +// UpgradeFunc: func(name string, stdout io.Writer, stderr io.Writer) error { +// panic("mock out the Upgrade method") +// }, +// } +// +// // use mockedExtensionManager in code that requires ExtensionManager +// // and then make assertions. +// +// } +type ExtensionManagerMock struct { + // DispatchFunc mocks the Dispatch method. + DispatchFunc func(args []string, stdin io.Reader, stdout io.Writer, stderr io.Writer) (bool, error) + + // InstallFunc mocks the Install method. + InstallFunc func(url string, stdout io.Writer, stderr io.Writer) error + + // InstallLocalFunc mocks the InstallLocal method. + InstallLocalFunc func(dir string) error + + // ListFunc mocks the List method. + ListFunc func() []Extension + + // RemoveFunc mocks the Remove method. + RemoveFunc func(name string) error + + // UpgradeFunc mocks the Upgrade method. + UpgradeFunc func(name string, stdout io.Writer, stderr io.Writer) error + + // calls tracks calls to the methods. + calls struct { + // Dispatch holds details about calls to the Dispatch method. + Dispatch []struct { + // Args is the args argument value. + Args []string + // Stdin is the stdin argument value. + Stdin io.Reader + // Stdout is the stdout argument value. + Stdout io.Writer + // Stderr is the stderr argument value. + Stderr io.Writer + } + // Install holds details about calls to the Install method. + Install []struct { + // URL is the url argument value. + URL string + // Stdout is the stdout argument value. + Stdout io.Writer + // Stderr is the stderr argument value. + Stderr io.Writer + } + // InstallLocal holds details about calls to the InstallLocal method. + InstallLocal []struct { + // Dir is the dir argument value. + Dir string + } + // List holds details about calls to the List method. + List []struct { + } + // Remove holds details about calls to the Remove method. + Remove []struct { + // Name is the name argument value. + Name string + } + // Upgrade holds details about calls to the Upgrade method. + Upgrade []struct { + // Name is the name argument value. + Name string + // Stdout is the stdout argument value. + Stdout io.Writer + // Stderr is the stderr argument value. + Stderr io.Writer + } + } + lockDispatch sync.RWMutex + lockInstall sync.RWMutex + lockInstallLocal sync.RWMutex + lockList sync.RWMutex + lockRemove sync.RWMutex + lockUpgrade sync.RWMutex +} + +// Dispatch calls DispatchFunc. +func (mock *ExtensionManagerMock) Dispatch(args []string, stdin io.Reader, stdout io.Writer, stderr io.Writer) (bool, error) { + if mock.DispatchFunc == nil { + panic("ExtensionManagerMock.DispatchFunc: method is nil but ExtensionManager.Dispatch was just called") + } + callInfo := struct { + Args []string + Stdin io.Reader + Stdout io.Writer + Stderr io.Writer + }{ + Args: args, + Stdin: stdin, + Stdout: stdout, + Stderr: stderr, + } + mock.lockDispatch.Lock() + mock.calls.Dispatch = append(mock.calls.Dispatch, callInfo) + mock.lockDispatch.Unlock() + return mock.DispatchFunc(args, stdin, stdout, stderr) +} + +// DispatchCalls gets all the calls that were made to Dispatch. +// Check the length with: +// len(mockedExtensionManager.DispatchCalls()) +func (mock *ExtensionManagerMock) DispatchCalls() []struct { + Args []string + Stdin io.Reader + Stdout io.Writer + Stderr io.Writer +} { + var calls []struct { + Args []string + Stdin io.Reader + Stdout io.Writer + Stderr io.Writer + } + mock.lockDispatch.RLock() + calls = mock.calls.Dispatch + mock.lockDispatch.RUnlock() + return calls +} + +// Install calls InstallFunc. +func (mock *ExtensionManagerMock) Install(url string, stdout io.Writer, stderr io.Writer) error { + if mock.InstallFunc == nil { + panic("ExtensionManagerMock.InstallFunc: method is nil but ExtensionManager.Install was just called") + } + callInfo := struct { + URL string + Stdout io.Writer + Stderr io.Writer + }{ + URL: url, + Stdout: stdout, + Stderr: stderr, + } + mock.lockInstall.Lock() + mock.calls.Install = append(mock.calls.Install, callInfo) + mock.lockInstall.Unlock() + return mock.InstallFunc(url, stdout, stderr) +} + +// InstallCalls gets all the calls that were made to Install. +// Check the length with: +// len(mockedExtensionManager.InstallCalls()) +func (mock *ExtensionManagerMock) InstallCalls() []struct { + URL string + Stdout io.Writer + Stderr io.Writer +} { + var calls []struct { + URL string + Stdout io.Writer + Stderr io.Writer + } + mock.lockInstall.RLock() + calls = mock.calls.Install + mock.lockInstall.RUnlock() + return calls +} + +// InstallLocal calls InstallLocalFunc. +func (mock *ExtensionManagerMock) InstallLocal(dir string) error { + if mock.InstallLocalFunc == nil { + panic("ExtensionManagerMock.InstallLocalFunc: method is nil but ExtensionManager.InstallLocal was just called") + } + callInfo := struct { + Dir string + }{ + Dir: dir, + } + mock.lockInstallLocal.Lock() + mock.calls.InstallLocal = append(mock.calls.InstallLocal, callInfo) + mock.lockInstallLocal.Unlock() + return mock.InstallLocalFunc(dir) +} + +// InstallLocalCalls gets all the calls that were made to InstallLocal. +// Check the length with: +// len(mockedExtensionManager.InstallLocalCalls()) +func (mock *ExtensionManagerMock) InstallLocalCalls() []struct { + Dir string +} { + var calls []struct { + Dir string + } + mock.lockInstallLocal.RLock() + calls = mock.calls.InstallLocal + mock.lockInstallLocal.RUnlock() + return calls +} + +// List calls ListFunc. +func (mock *ExtensionManagerMock) List() []Extension { + if mock.ListFunc == nil { + panic("ExtensionManagerMock.ListFunc: method is nil but ExtensionManager.List was just called") + } + callInfo := struct { + }{} + mock.lockList.Lock() + mock.calls.List = append(mock.calls.List, callInfo) + mock.lockList.Unlock() + return mock.ListFunc() +} + +// ListCalls gets all the calls that were made to List. +// Check the length with: +// len(mockedExtensionManager.ListCalls()) +func (mock *ExtensionManagerMock) ListCalls() []struct { +} { + var calls []struct { + } + mock.lockList.RLock() + calls = mock.calls.List + mock.lockList.RUnlock() + return calls +} + +// Remove calls RemoveFunc. +func (mock *ExtensionManagerMock) Remove(name string) error { + if mock.RemoveFunc == nil { + panic("ExtensionManagerMock.RemoveFunc: method is nil but ExtensionManager.Remove was just called") + } + callInfo := struct { + Name string + }{ + Name: name, + } + mock.lockRemove.Lock() + mock.calls.Remove = append(mock.calls.Remove, callInfo) + mock.lockRemove.Unlock() + return mock.RemoveFunc(name) +} + +// RemoveCalls gets all the calls that were made to Remove. +// Check the length with: +// len(mockedExtensionManager.RemoveCalls()) +func (mock *ExtensionManagerMock) RemoveCalls() []struct { + Name string +} { + var calls []struct { + Name string + } + mock.lockRemove.RLock() + calls = mock.calls.Remove + mock.lockRemove.RUnlock() + return calls +} + +// Upgrade calls UpgradeFunc. +func (mock *ExtensionManagerMock) Upgrade(name string, stdout io.Writer, stderr io.Writer) error { + if mock.UpgradeFunc == nil { + panic("ExtensionManagerMock.UpgradeFunc: method is nil but ExtensionManager.Upgrade was just called") + } + callInfo := struct { + Name string + Stdout io.Writer + Stderr io.Writer + }{ + Name: name, + Stdout: stdout, + Stderr: stderr, + } + mock.lockUpgrade.Lock() + mock.calls.Upgrade = append(mock.calls.Upgrade, callInfo) + mock.lockUpgrade.Unlock() + return mock.UpgradeFunc(name, stdout, stderr) +} + +// UpgradeCalls gets all the calls that were made to Upgrade. +// Check the length with: +// len(mockedExtensionManager.UpgradeCalls()) +func (mock *ExtensionManagerMock) UpgradeCalls() []struct { + Name string + Stdout io.Writer + Stderr io.Writer +} { + var calls []struct { + Name string + Stdout io.Writer + Stderr io.Writer + } + mock.lockUpgrade.RLock() + calls = mock.calls.Upgrade + mock.lockUpgrade.RUnlock() + return calls +} diff --git a/pkg/findsh/find.go b/pkg/findsh/find.go new file mode 100644 index 000000000..b277f1aeb --- /dev/null +++ b/pkg/findsh/find.go @@ -0,0 +1,10 @@ +// +build !windows + +package findsh + +import "os/exec" + +// Find locates the `sh` interpreter on the system. +func Find() (string, error) { + return exec.LookPath("sh") +} diff --git a/pkg/findsh/find_windows.go b/pkg/findsh/find_windows.go new file mode 100644 index 000000000..b9bf19a19 --- /dev/null +++ b/pkg/findsh/find_windows.go @@ -0,0 +1,35 @@ +package findsh + +import ( + "os" + "path/filepath" + + "github.com/cli/safeexec" +) + +func Find() (string, error) { + shPath, shErr := safeexec.LookPath("sh") + if shErr == nil { + return shPath, nil + } + + gitPath, err := safeexec.LookPath("git") + if err != nil { + return "", shErr + } + gitDir := filepath.Dir(gitPath) + + // regular Git for Windows install + shPath = filepath.Join(gitDir, "..", "bin", "sh.exe") + if _, err := os.Stat(shPath); err == nil { + return filepath.Clean(shPath), nil + } + + // git as a scoop shim + shPath = filepath.Join(gitDir, "..", "apps", "git", "current", "bin", "sh.exe") + if _, err := os.Stat(shPath); err == nil { + return filepath.Clean(shPath), nil + } + + return "", shErr +} From f99191ea6f87e56559af9e02bf548509058058f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 21 Jun 2021 16:22:10 +0200 Subject: [PATCH 2/4] Enable setting an alias for an extension command --- pkg/cmd/alias/set/set.go | 39 ++++++++++++++++++++++++--------------- 1 file changed, 24 insertions(+), 15 deletions(-) diff --git a/pkg/cmd/alias/set/set.go b/pkg/cmd/alias/set/set.go index e50ca3f86..48b1231be 100644 --- a/pkg/cmd/alias/set/set.go +++ b/pkg/cmd/alias/set/set.go @@ -20,7 +20,8 @@ type SetOptions struct { Name string Expansion string IsShell bool - RootCmd *cobra.Command + + validCommand func(string) bool } func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command { @@ -78,11 +79,29 @@ func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command `), Args: cobra.ExactArgs(2), RunE: func(cmd *cobra.Command, args []string) error { - opts.RootCmd = cmd.Root() - opts.Name = args[0] opts.Expansion = args[1] + opts.validCommand = func(args string) bool { + split, err := shlex.Split(args) + if err != nil { + return false + } + + rootCmd := cmd.Root() + cmd, _, err := rootCmd.Traverse(split) + if err == nil && cmd != rootCmd { + return true + } + + for _, ext := range f.ExtensionManager.List() { + if ext.Name() == split[0] { + return true + } + } + return false + } + if runF != nil { return runF(opts) } @@ -143,11 +162,11 @@ func setRun(opts *SetOptions) error { } isShell = strings.HasPrefix(expansion, "!") - if validCommand(opts.RootCmd, opts.Name) { + if opts.validCommand(opts.Name) { return fmt.Errorf("could not create alias: %q is already a gh command", opts.Name) } - if !isShell && !validCommand(opts.RootCmd, expansion) { + if !isShell && !opts.validCommand(expansion) { return fmt.Errorf("could not create alias: %s does not correspond to a gh command", expansion) } @@ -173,16 +192,6 @@ func setRun(opts *SetOptions) error { return nil } -func validCommand(rootCmd *cobra.Command, expansion string) bool { - split, err := shlex.Split(expansion) - if err != nil { - return false - } - - cmd, _, err := rootCmd.Traverse(split) - return err == nil && cmd != rootCmd -} - func getExpansion(opts *SetOptions) (string, error) { if opts.Expansion == "-" { stdin, err := ioutil.ReadAll(opts.IO.In) From 42efc3f25a9fee1fc11089f9c1f18701a0ebc2be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 21 Jun 2021 17:22:17 +0200 Subject: [PATCH 3/4] Fix test cleanup on Windows --- pkg/cmd/extensions/command_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/cmd/extensions/command_test.go b/pkg/cmd/extensions/command_test.go index b9ce752ba..7bf0b4429 100644 --- a/pkg/cmd/extensions/command_test.go +++ b/pkg/cmd/extensions/command_test.go @@ -16,7 +16,9 @@ import ( func TestNewCmdExtensions(t *testing.T) { tempDir := t.TempDir() + oldWd, _ := os.Getwd() assert.NoError(t, os.Chdir(tempDir)) + t.Cleanup(func() { _ = os.Chdir(oldWd) }) tests := []struct { name string From 6c984f4512baad1182c0343d6f2225eb0828fa84 Mon Sep 17 00:00:00 2001 From: nate smith Date: Mon, 28 Jun 2021 14:36:51 -0500 Subject: [PATCH 4/4] remove dead code --- pkg/cmd/alias/set/set.go | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/pkg/cmd/alias/set/set.go b/pkg/cmd/alias/set/set.go index 48b1231be..8a452cca5 100644 --- a/pkg/cmd/alias/set/set.go +++ b/pkg/cmd/alias/set/set.go @@ -111,26 +111,6 @@ func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command cmd.Flags().BoolVarP(&opts.IsShell, "shell", "s", false, "Declare an alias to be passed through a shell interpreter") - opts.validCommand = func(args string) bool { - split, err := shlex.Split(args) - if err != nil { - return false - } - - rootCmd := cmd.Root() - cmd, _, err := rootCmd.Traverse(split) - if err == nil && cmd != rootCmd { - return true - } - - for _, ext := range f.ExtensionManager.List() { - if ext.Name() == split[0] { - return true - } - } - return false - } - return cmd }