From 3caba02f3c222be27449b70b8c83f26017b3acc8 Mon Sep 17 00:00:00 2001 From: chemotaxis Date: Sat, 5 Jun 2021 00:35:30 -0400 Subject: [PATCH 1/9] Add documentation for installing via Conda Conda is a cross-platform package and environment manager, primarily associated with the scientific computing and data science communities. --- README.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/README.md b/README.md index 2886e3471..39856ec36 100644 --- a/README.md +++ b/README.md @@ -67,6 +67,18 @@ For more information and distro-specific instructions, see the [Linux installati MSI installers are available for download on the [releases page][]. +### Other package managers + +#### Conda + +`gh` is available via [Conda][], a cross-platform package and environment manager. + +| Install: | Upgrade: | +|------------------------------------------|-----------------------------------------| +| `conda install gh --channel conda-forge` | `conda update gh --channel conda-forge` | + +Additional installation options available on the [gh-feedstock page](https://github.com/conda-forge/gh-feedstock#installing-gh). + ### GitHub Actions GitHub CLI comes pre-installed in all [GitHub-Hosted Runners](https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners). @@ -93,6 +105,7 @@ tool. Check out our [more detailed explanation][gh-vs-hub] to learn more. [winget]: https://github.com/microsoft/winget-cli [scoop]: https://scoop.sh [Chocolatey]: https://chocolatey.org +[Conda]: https://docs.conda.io/en/latest/ [releases page]: https://github.com/cli/cli/releases/latest [hub]: https://github.com/github/hub [contributing]: ./.github/CONTRIBUTING.md From 71b738b553ff22f17fc7c00e83bb66d54d315a64 Mon Sep 17 00:00:00 2001 From: chemotaxis Date: Sat, 5 Jun 2021 00:42:39 -0400 Subject: [PATCH 2/9] Make whitespace consistent --- README.md | 3 --- 1 file changed, 3 deletions(-) diff --git a/README.md b/README.md index 39856ec36..6279f182c 100644 --- a/README.md +++ b/README.md @@ -14,7 +14,6 @@ GitHub CLI is available for repositories hosted on GitHub.com and GitHub Enterpr If anything feels off, or if you feel that some functionality is missing, please check out the [contributing page][contributing]. There you will find instructions for sharing your feedback, building the tool locally, and submitting pull requests to the project. - ## Installation @@ -44,7 +43,6 @@ For more information and distro-specific instructions, see the [Linux installati `gh` is available via [WinGet][], [scoop][], [Chocolatey][], and as downloadable MSI. - #### WinGet | Install: | Upgrade: | @@ -98,7 +96,6 @@ what an official GitHub CLI tool can look like with a fundamentally different de tools bring GitHub to the terminal, `hub` behaves as a proxy to `git`, and `gh` is a standalone tool. Check out our [more detailed explanation][gh-vs-hub] to learn more. - [manual]: https://cli.github.com/manual/ [Homebrew]: https://brew.sh [MacPorts]: https://www.macports.org From 71c6b8f43ab8fde443ce1ef2146246fae8ff07a1 Mon Sep 17 00:00:00 2001 From: chemotaxis Date: Sat, 5 Jun 2021 00:48:36 -0400 Subject: [PATCH 3/9] Add links to Conda section within each OS section --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 6279f182c..d9c96cf89 100644 --- a/README.md +++ b/README.md @@ -19,7 +19,7 @@ If anything feels off, or if you feel that some functionality is missing, please ### macOS -`gh` is available via [Homebrew][], [MacPorts][], and as a downloadable binary from the [releases page][]. +`gh` is available via [Homebrew][], [MacPorts][], [Conda](#Conda), and as a downloadable binary from the [releases page][]. #### Homebrew @@ -35,13 +35,13 @@ If anything feels off, or if you feel that some functionality is missing, please ### Linux -`gh` is available via [Homebrew](#homebrew), and as downloadable binaries from the [releases page][]. +`gh` is available via [Homebrew](#homebrew), [Conda](#Conda), and as downloadable binaries from the [releases page][]. For more information and distro-specific instructions, see the [Linux installation docs](./docs/install_linux.md). ### Windows -`gh` is available via [WinGet][], [scoop][], [Chocolatey][], and as downloadable MSI. +`gh` is available via [WinGet][], [scoop][], [Chocolatey][], [Conda](#Conda), and as downloadable MSI. #### WinGet From a72f6346dd9dba218156759047ecc4a98b95fff6 Mon Sep 17 00:00:00 2001 From: chemotaxis Date: Sat, 5 Jun 2021 12:37:55 -0400 Subject: [PATCH 4/9] Rearrange Conda installation instructions Originally, I was thinking of putting Conda in a separate section after the Windows section, since Conda probably isn't as well known or used. But reading through the readme again, it seems like arranging it like the other instructions makes more sense. I found myself trying to look for the instructions when I first read it in the MacOS section, but couldn't find the instructions. --- README.md | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index d9c96cf89..49d11ff70 100644 --- a/README.md +++ b/README.md @@ -19,7 +19,7 @@ If anything feels off, or if you feel that some functionality is missing, please ### macOS -`gh` is available via [Homebrew][], [MacPorts][], [Conda](#Conda), and as a downloadable binary from the [releases page][]. +`gh` is available via [Homebrew][], [MacPorts][], [Conda][], and as a downloadable binary from the [releases page][]. #### Homebrew @@ -33,6 +33,14 @@ If anything feels off, or if you feel that some functionality is missing, please | ---------------------- | ---------------------------------------------- | | `sudo port install gh` | `sudo port selfupdate && sudo port upgrade gh` | +#### Conda + +| Install: | Upgrade: | +|------------------------------------------|-----------------------------------------| +| `conda install gh --channel conda-forge` | `conda update gh --channel conda-forge` | + +Additional Conda installation options available on the [gh-feedstock page](https://github.com/conda-forge/gh-feedstock#installing-gh). + ### Linux `gh` is available via [Homebrew](#homebrew), [Conda](#Conda), and as downloadable binaries from the [releases page][]. @@ -65,18 +73,6 @@ For more information and distro-specific instructions, see the [Linux installati MSI installers are available for download on the [releases page][]. -### Other package managers - -#### Conda - -`gh` is available via [Conda][], a cross-platform package and environment manager. - -| Install: | Upgrade: | -|------------------------------------------|-----------------------------------------| -| `conda install gh --channel conda-forge` | `conda update gh --channel conda-forge` | - -Additional installation options available on the [gh-feedstock page](https://github.com/conda-forge/gh-feedstock#installing-gh). - ### GitHub Actions GitHub CLI comes pre-installed in all [GitHub-Hosted Runners](https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners). From 53fac59ef92d691fa265375df70d96a42c400fb6 Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Mon, 14 Jun 2021 15:56:11 -0400 Subject: [PATCH 5/9] Cleanup factory/default and add tests --- pkg/cmd/factory/default.go | 135 ++++++++++----- pkg/cmd/factory/default_test.go | 281 ++++++++++++++++++++++++++++++++ pkg/cmd/root/root.go | 31 +--- pkg/cmdutil/repo_override.go | 22 ++- 4 files changed, 389 insertions(+), 80 deletions(-) create mode 100644 pkg/cmd/factory/default_test.go diff --git a/pkg/cmd/factory/default.go b/pkg/cmd/factory/default.go index e9023c852..a5a71f579 100644 --- a/pkg/cmd/factory/default.go +++ b/pkg/cmd/factory/default.go @@ -6,6 +6,8 @@ import ( "net/http" "os" + "github.com/cli/cli/api" + "github.com/cli/cli/context" "github.com/cli/cli/git" "github.com/cli/cli/internal/config" "github.com/cli/cli/internal/ghrepo" @@ -14,11 +16,93 @@ import ( ) func New(appVersion string) *cmdutil.Factory { - io := iostreams.System() + f := &cmdutil.Factory{ + IOStreams: iostreams.System(), // No factory dependencies + Config: configFunc(), // No factory dependencies + Branch: branchFunc(), // No factory dependencies + Executable: executable(), // No factory dependencies + } + f.HttpClient = httpClientFunc(f, appVersion) // Depends on Config, IOStreams, and appVersion + f.Remotes = remotesFunc(f) // Depends on Config + f.BaseRepo = BaseRepoFunc(f) // Depends on Remotes + f.Browser = browser(f) // Depends on IOStreams + + return f +} + +func BaseRepoFunc(f *cmdutil.Factory) func() (ghrepo.Interface, error) { + return func() (ghrepo.Interface, error) { + remotes, err := f.Remotes() + if err != nil { + return nil, err + } + return remotes[0], nil + } +} + +func SmartBaseRepoFunc(f *cmdutil.Factory) func() (ghrepo.Interface, error) { + return func() (ghrepo.Interface, error) { + httpClient, err := f.HttpClient() + if err != nil { + return nil, err + } + + apiClient := api.NewClientFromHTTP(httpClient) + + remotes, err := f.Remotes() + if err != nil { + return nil, err + } + repoContext, err := context.ResolveRemotesToRepos(remotes, apiClient, "") + if err != nil { + return nil, err + } + baseRepo, err := repoContext.BaseRepo(f.IOStreams) + if err != nil { + return nil, err + } + + return baseRepo, nil + } +} + +func remotesFunc(f *cmdutil.Factory) func() (context.Remotes, error) { + rr := &remoteResolver{ + readRemotes: git.Remotes, + getConfig: f.Config, + } + return rr.Resolver() +} + +func httpClientFunc(f *cmdutil.Factory, appVersion string) func() (*http.Client, error) { + return func() (*http.Client, error) { + io := f.IOStreams + cfg, err := f.Config() + if err != nil { + return nil, err + } + return NewHTTPClient(io, cfg, appVersion, true), nil + } +} + +func browser(f *cmdutil.Factory) cmdutil.Browser { + io := f.IOStreams + return cmdutil.NewBrowser(os.Getenv("BROWSER"), io.Out, io.ErrOut) +} + +func executable() string { + gh := "gh" + if exe, err := os.Executable(); err == nil { + gh = exe + } + return gh +} + +func configFunc() func() (config.Config, error) { var cachedConfig config.Config var configError error - configFunc := func() (config.Config, error) { + return func() (config.Config, error) { if cachedConfig != nil || configError != nil { return cachedConfig, configError } @@ -30,45 +114,14 @@ func New(appVersion string) *cmdutil.Factory { cachedConfig = config.InheritEnv(cachedConfig) return cachedConfig, configError } +} - rr := &remoteResolver{ - readRemotes: git.Remotes, - getConfig: configFunc, - } - remotesFunc := rr.Resolver() - - ghExecutable := "gh" - if exe, err := os.Executable(); err == nil { - ghExecutable = exe - } - - return &cmdutil.Factory{ - IOStreams: io, - Config: configFunc, - Remotes: remotesFunc, - HttpClient: func() (*http.Client, error) { - cfg, err := configFunc() - if err != nil { - return nil, err - } - - return NewHTTPClient(io, cfg, appVersion, true), nil - }, - BaseRepo: func() (ghrepo.Interface, error) { - remotes, err := remotesFunc() - if err != nil { - return nil, err - } - return remotes[0], nil - }, - Branch: func() (string, error) { - currentBranch, err := git.CurrentBranch() - if err != nil { - return "", fmt.Errorf("could not determine current branch: %w", err) - } - return currentBranch, nil - }, - Executable: ghExecutable, - Browser: cmdutil.NewBrowser(os.Getenv("BROWSER"), io.Out, io.ErrOut), +func branchFunc() func() (string, error) { + return func() (string, error) { + currentBranch, err := git.CurrentBranch() + if err != nil { + return "", fmt.Errorf("could not determine current branch: %w", err) + } + return currentBranch, nil } } diff --git a/pkg/cmd/factory/default_test.go b/pkg/cmd/factory/default_test.go new file mode 100644 index 000000000..1f60b6b23 --- /dev/null +++ b/pkg/cmd/factory/default_test.go @@ -0,0 +1,281 @@ +package factory + +import ( + "net/url" + "os" + "testing" + + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/git" + "github.com/cli/cli/internal/config" + "github.com/cli/cli/pkg/cmdutil" + "github.com/stretchr/testify/assert" +) + +func Test_BaseRepo(t *testing.T) { + orig_GH_HOST := os.Getenv("GH_HOST") + t.Cleanup(func() { + os.Setenv("GH_HOST", orig_GH_HOST) + }) + + tests := []struct { + name string + remotes git.RemoteSet + config config.Config + override string + wantsErr bool + wantsName string + wantsOwner string + wantsHost string + }{ + { + name: "matching remote", + remotes: git.RemoteSet{ + git.NewRemote("origin", "https://nonsense.com/owner/repo.git"), + }, + config: defaultConfig(), + wantsName: "repo", + wantsOwner: "owner", + wantsHost: "nonsense.com", + }, + { + name: "no matching remote", + remotes: git.RemoteSet{ + git.NewRemote("origin", "https://test.com/owner/repo.git"), + }, + config: defaultConfig(), + wantsErr: true, + }, + { + name: "override with matching remote", + remotes: git.RemoteSet{ + git.NewRemote("origin", "https://test.com/owner/repo.git"), + }, + config: defaultConfig(), + override: "test.com", + wantsName: "repo", + wantsOwner: "owner", + wantsHost: "test.com", + }, + { + name: "override with no matching remote", + remotes: git.RemoteSet{ + git.NewRemote("origin", "https://nonsense.com/owner/repo.git"), + }, + config: defaultConfig(), + override: "test.com", + wantsErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.override != "" { + os.Setenv("GH_HOST", tt.override) + } else { + os.Unsetenv("GH_HOST") + } + f := New("1") + rr := &remoteResolver{ + readRemotes: func() (git.RemoteSet, error) { + return tt.remotes, nil + }, + getConfig: func() (config.Config, error) { + return tt.config, nil + }, + } + f.Remotes = rr.Resolver() + f.BaseRepo = BaseRepoFunc(f) + repo, err := f.BaseRepo() + if tt.wantsErr { + assert.Error(t, err) + return + } + assert.NoError(t, err) + assert.Equal(t, tt.wantsName, repo.RepoName()) + assert.Equal(t, tt.wantsOwner, repo.RepoOwner()) + assert.Equal(t, tt.wantsHost, repo.RepoHost()) + }) + } +} + +func Test_SmartBaseRepo(t *testing.T) { + pu, _ := url.Parse("https://test.com/newowner/newrepo.git") + orig_GH_HOST := os.Getenv("GH_HOST") + t.Cleanup(func() { + os.Setenv("GH_HOST", orig_GH_HOST) + }) + + tests := []struct { + name string + remotes git.RemoteSet + config config.Config + override string + wantsErr bool + wantsName string + wantsOwner string + wantsHost string + }{ + { + name: "override with matching remote", + remotes: git.RemoteSet{ + git.NewRemote("origin", "https://test.com/owner/repo.git"), + }, + config: defaultConfig(), + override: "test.com", + wantsName: "repo", + wantsOwner: "owner", + wantsHost: "test.com", + }, + { + name: "override with matching remote and base resolution", + remotes: git.RemoteSet{ + &git.Remote{Name: "origin", + Resolved: "base", + FetchURL: pu, + PushURL: pu}, + }, + config: defaultConfig(), + override: "test.com", + wantsName: "newrepo", + wantsOwner: "newowner", + wantsHost: "test.com", + }, + { + name: "override with matching remote and nonbase resolution", + remotes: git.RemoteSet{ + &git.Remote{Name: "origin", + Resolved: "johnny/test", + FetchURL: pu, + PushURL: pu}, + }, + config: defaultConfig(), + override: "test.com", + wantsName: "test", + wantsOwner: "johnny", + wantsHost: "test.com", + }, + { + name: "override with no matching remote", + remotes: git.RemoteSet{ + git.NewRemote("origin", "https://example.com/owner/repo.git"), + }, + config: defaultConfig(), + override: "test.com", + wantsErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.override != "" { + os.Setenv("GH_HOST", tt.override) + } else { + os.Unsetenv("GH_HOST") + } + f := New("1") + rr := &remoteResolver{ + readRemotes: func() (git.RemoteSet, error) { + return tt.remotes, nil + }, + getConfig: func() (config.Config, error) { + return tt.config, nil + }, + } + f.Remotes = rr.Resolver() + f.BaseRepo = SmartBaseRepoFunc(f) + repo, err := f.BaseRepo() + if tt.wantsErr { + assert.Error(t, err) + return + } + assert.NoError(t, err) + assert.Equal(t, tt.wantsName, repo.RepoName()) + assert.Equal(t, tt.wantsOwner, repo.RepoOwner()) + assert.Equal(t, tt.wantsHost, repo.RepoHost()) + }) + } +} + +// Defined in pkg/cmdutil/repo_override.go but test it along with other BaseRepo functions +func Test_OverrideBaseRepo(t *testing.T) { + orig_GH_HOST := os.Getenv("GH_REPO") + t.Cleanup(func() { + os.Setenv("GH_REPO", orig_GH_HOST) + }) + + tests := []struct { + name string + remotes git.RemoteSet + config config.Config + envOverride string + argOverride string + wantsErr bool + wantsName string + wantsOwner string + wantsHost string + }{ + { + name: "override from argument", + argOverride: "override/test", + wantsHost: "github.com", + wantsOwner: "override", + wantsName: "test", + }, + { + name: "override from environment", + envOverride: "somehost.com/override/test", + wantsHost: "somehost.com", + wantsOwner: "override", + wantsName: "test", + }, + { + name: "no override", + remotes: git.RemoteSet{ + git.NewRemote("origin", "https://nonsense.com/owner/repo.git"), + }, + config: defaultConfig(), + wantsHost: "nonsense.com", + wantsOwner: "owner", + wantsName: "repo", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.envOverride != "" { + os.Setenv("GH_REPO", tt.envOverride) + } else { + os.Unsetenv("GH_REPO") + } + f := New("1") + rr := &remoteResolver{ + readRemotes: func() (git.RemoteSet, error) { + return tt.remotes, nil + }, + getConfig: func() (config.Config, error) { + return tt.config, nil + }, + } + f.Remotes = rr.Resolver() + f.BaseRepo = cmdutil.OverrideBaseRepoFunc(f, tt.argOverride) + repo, err := f.BaseRepo() + if tt.wantsErr { + assert.Error(t, err) + return + } + assert.NoError(t, err) + assert.Equal(t, tt.wantsName, repo.RepoName()) + assert.Equal(t, tt.wantsOwner, repo.RepoOwner()) + assert.Equal(t, tt.wantsHost, repo.RepoHost()) + }) + } +} + +func defaultConfig() config.Config { + return config.InheritEnv(config.NewFromString(heredoc.Doc(` + hosts: + nonsense.com: + oauth_token: BLAH + `))) +} diff --git a/pkg/cmd/root/root.go b/pkg/cmd/root/root.go index cc971a0d4..7876eb1f9 100644 --- a/pkg/cmd/root/root.go +++ b/pkg/cmd/root/root.go @@ -4,9 +4,6 @@ import ( "net/http" "github.com/MakeNowJust/heredoc" - "github.com/cli/cli/api" - "github.com/cli/cli/context" - "github.com/cli/cli/internal/ghrepo" actionsCmd "github.com/cli/cli/pkg/cmd/actions" aliasCmd "github.com/cli/cli/pkg/cmd/alias" apiCmd "github.com/cli/cli/pkg/cmd/api" @@ -93,7 +90,7 @@ func NewCmdRoot(f *cmdutil.Factory, version, buildDate string) *cobra.Command { // below here at the commands that require the "intelligent" BaseRepo resolver repoResolvingCmdFactory := *f - repoResolvingCmdFactory.BaseRepo = resolvedBaseRepo(f) + repoResolvingCmdFactory.BaseRepo = factory.SmartBaseRepoFunc(f) cmd.AddCommand(prCmd.NewCmdPR(&repoResolvingCmdFactory)) cmd.AddCommand(issueCmd.NewCmdIssue(&repoResolvingCmdFactory)) @@ -126,29 +123,3 @@ func bareHTTPClient(f *cmdutil.Factory, version string) func() (*http.Client, er return factory.NewHTTPClient(f.IOStreams, cfg, version, false), nil } } - -func resolvedBaseRepo(f *cmdutil.Factory) func() (ghrepo.Interface, error) { - return func() (ghrepo.Interface, error) { - httpClient, err := f.HttpClient() - if err != nil { - return nil, err - } - - apiClient := api.NewClientFromHTTP(httpClient) - - remotes, err := f.Remotes() - if err != nil { - return nil, err - } - repoContext, err := context.ResolveRemotesToRepos(remotes, apiClient, "") - if err != nil { - return nil, err - } - baseRepo, err := repoContext.BaseRepo(f.IOStreams) - if err != nil { - return nil, err - } - - return baseRepo, nil - } -} diff --git a/pkg/cmdutil/repo_override.go b/pkg/cmdutil/repo_override.go index 8b3d36489..c5d996c70 100644 --- a/pkg/cmdutil/repo_override.go +++ b/pkg/cmdutil/repo_override.go @@ -12,14 +12,18 @@ func EnableRepoOverride(cmd *cobra.Command, f *Factory) { cmd.PersistentPreRun = func(cmd *cobra.Command, args []string) { repoOverride, _ := cmd.Flags().GetString("repo") - if repoFromEnv := os.Getenv("GH_REPO"); repoOverride == "" && repoFromEnv != "" { - repoOverride = repoFromEnv - } - if repoOverride != "" { - // NOTE: this mutates the factory - f.BaseRepo = func() (ghrepo.Interface, error) { - return ghrepo.FromFullName(repoOverride) - } - } + f.BaseRepo = OverrideBaseRepoFunc(f, repoOverride) } } + +func OverrideBaseRepoFunc(f *Factory, override string) func() (ghrepo.Interface, error) { + if override == "" { + override = os.Getenv("GH_REPO") + } + if override != "" { + return func() (ghrepo.Interface, error) { + return ghrepo.FromFullName(override) + } + } + return f.BaseRepo +} From edfac4238408673be932208970fc153458408419 Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Mon, 14 Jun 2021 16:17:33 -0400 Subject: [PATCH 6/9] Set up iostreams in factory default --- cmd/gh/main.go | 9 --- pkg/cmd/factory/default.go | 32 ++++++++-- pkg/cmd/factory/default_test.go | 110 ++++++++++++++++++++++++++++++++ pkg/iostreams/iostreams.go | 12 +++- 4 files changed, 147 insertions(+), 16 deletions(-) diff --git a/cmd/gh/main.go b/cmd/gh/main.go index 891d4b98f..7ee00b061 100644 --- a/cmd/gh/main.go +++ b/cmd/gh/main.go @@ -93,15 +93,6 @@ func mainRun() exitCode { return exitError } - if prompt, _ := cfg.Get("", "prompt"); prompt == "disabled" { - cmdFactory.IOStreams.SetNeverPrompt(true) - } - if ghPager, ghPagerExists := os.LookupEnv("GH_PAGER"); ghPagerExists { - cmdFactory.IOStreams.SetPager(ghPager) - } else if pager, _ := cfg.Get("", "pager"); pager != "" { - cmdFactory.IOStreams.SetPager(pager) - } - // TODO: remove after FromFullName has been revisited if host, err := cfg.DefaultHost(); err == nil { ghrepo.SetDefaultHost(host) diff --git a/pkg/cmd/factory/default.go b/pkg/cmd/factory/default.go index a5a71f579..eec60bc56 100644 --- a/pkg/cmd/factory/default.go +++ b/pkg/cmd/factory/default.go @@ -17,12 +17,12 @@ import ( func New(appVersion string) *cmdutil.Factory { f := &cmdutil.Factory{ - IOStreams: iostreams.System(), // No factory dependencies - Config: configFunc(), // No factory dependencies - Branch: branchFunc(), // No factory dependencies - Executable: executable(), // No factory dependencies + Config: configFunc(), // No factory dependencies + Branch: branchFunc(), // No factory dependencies + Executable: executable(), // No factory dependencies } + f.IOStreams = ioStreams(f) // Depends on Config f.HttpClient = httpClientFunc(f, appVersion) // Depends on Config, IOStreams, and appVersion f.Remotes = remotesFunc(f) // Depends on Config f.BaseRepo = BaseRepoFunc(f) // Depends on Remotes @@ -125,3 +125,27 @@ func branchFunc() func() (string, error) { return currentBranch, nil } } + +func ioStreams(f *cmdutil.Factory) *iostreams.IOStreams { + io := iostreams.System() + cfg, err := f.Config() + if err != nil { + return io + } + + if prompt, _ := cfg.Get("", "prompt"); prompt == "disabled" { + io.SetNeverPrompt(true) + } + + // Pager precedence + // 1. GH_PAGER + // 2. pager from config + // 3. PAGER + if ghPager, ghPagerExists := os.LookupEnv("GH_PAGER"); ghPagerExists { + io.SetPager(ghPager) + } else if pager, _ := cfg.Get("", "pager"); pager != "" { + io.SetPager(pager) + } + + return io +} diff --git a/pkg/cmd/factory/default_test.go b/pkg/cmd/factory/default_test.go index 1f60b6b23..1bf8702c8 100644 --- a/pkg/cmd/factory/default_test.go +++ b/pkg/cmd/factory/default_test.go @@ -272,6 +272,108 @@ func Test_OverrideBaseRepo(t *testing.T) { } } +func Test_ioStreams_pager(t *testing.T) { + tests := []struct { + name string + env map[string]string + config config.Config + wantPager string + }{ + { + name: "GH_PAGER and PAGER set", + env: map[string]string{ + "GH_PAGER": "GH_PAGER", + "PAGER": "PAGER", + }, + wantPager: "GH_PAGER", + }, + { + name: "GH_PAGER and config pager set", + env: map[string]string{ + "GH_PAGER": "GH_PAGER", + }, + config: pagerConfig(), + wantPager: "GH_PAGER", + }, + { + name: "config pager and PAGER set", + env: map[string]string{ + "PAGER": "PAGER", + }, + config: pagerConfig(), + wantPager: "CONFIG_PAGER", + }, + { + name: "only PAGER set", + env: map[string]string{ + "PAGER": "PAGER", + }, + wantPager: "PAGER", + }, + { + name: "GH_PAGER set to blank string", + env: map[string]string{ + "GH_PAGER": "", + "PAGER": "PAGER", + }, + wantPager: "", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.env != nil { + for k, v := range tt.env { + old := os.Getenv(k) + os.Setenv(k, v) + if k == "GH_PAGER" { + defer os.Unsetenv(k) + } else { + defer os.Setenv(k, old) + } + } + } + f := New("1") + if tt.config != nil { + f.Config = func() (config.Config, error) { + return tt.config, nil + } + } + io := ioStreams(f) + assert.Equal(t, tt.wantPager, io.GetPager()) + }) + } +} + +func Test_ioStreams_prompt(t *testing.T) { + tests := []struct { + name string + config config.Config + promptDisabled bool + }{ + { + name: "default config", + promptDisabled: false, + }, + { + name: "config with prompt disabled", + config: disablePromptConfig(), + promptDisabled: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + f := New("1") + if tt.config != nil { + f.Config = func() (config.Config, error) { + return tt.config, nil + } + } + io := ioStreams(f) + assert.Equal(t, tt.promptDisabled, io.GetNeverPrompt()) + }) + } +} + func defaultConfig() config.Config { return config.InheritEnv(config.NewFromString(heredoc.Doc(` hosts: @@ -279,3 +381,11 @@ func defaultConfig() config.Config { oauth_token: BLAH `))) } + +func pagerConfig() config.Config { + return config.NewFromString("pager: CONFIG_PAGER") +} + +func disablePromptConfig() config.Config { + return config.NewFromString("prompt: disabled") +} diff --git a/pkg/iostreams/iostreams.go b/pkg/iostreams/iostreams.go index ec2503151..7e5aa0023 100644 --- a/pkg/iostreams/iostreams.go +++ b/pkg/iostreams/iostreams.go @@ -140,6 +140,10 @@ func (s *IOStreams) SetPager(cmd string) { s.pagerCommand = cmd } +func (s *IOStreams) GetPager() string { + return s.pagerCommand +} + func (s *IOStreams) StartPager() error { if s.pagerCommand == "" || s.pagerCommand == "cat" || !s.IsStdoutTTY() { return nil @@ -202,6 +206,10 @@ func (s *IOStreams) CanPrompt() bool { return s.IsStdinTTY() && s.IsStdoutTTY() } +func (s *IOStreams) GetNeverPrompt() bool { + return s.neverPrompt +} + func (s *IOStreams) SetNeverPrompt(v bool) { s.neverPrompt = v } @@ -281,8 +289,6 @@ func System() *IOStreams { stdoutIsTTY := isTerminal(os.Stdout) stderrIsTTY := isTerminal(os.Stderr) - pagerCommand := os.Getenv("PAGER") - io := &IOStreams{ In: os.Stdin, originalOut: os.Stdout, @@ -290,7 +296,7 @@ func System() *IOStreams { ErrOut: colorable.NewColorable(os.Stderr), colorEnabled: EnvColorForced() || (!EnvColorDisabled() && stdoutIsTTY), is256enabled: Is256ColorSupported(), - pagerCommand: pagerCommand, + pagerCommand: os.Getenv("PAGER"), } if stdoutIsTTY && stderrIsTTY { From cda406f495e564aac3596f0aa8c79e00a303ab1a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 15 Jun 2021 17:31:01 +0200 Subject: [PATCH 7/9] Better error handling in build script on Windows `script/build.go` could encounter an "Access is denied" error when the project contains a symlink that could not be followed. This ignores such errors with a warning and allows the build to resume. --- script/build.go | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/script/build.go b/script/build.go index 2c92a5f20..ee0ac4652 100644 --- a/script/build.go +++ b/script/build.go @@ -23,6 +23,7 @@ package main import ( + "errors" "fmt" "io/ioutil" "os" @@ -136,8 +137,14 @@ func date() string { func sourceFilesLaterThan(t time.Time) bool { foundLater := false - _ = filepath.Walk(".", func(path string, info os.FileInfo, err error) error { + err := filepath.Walk(".", func(path string, info os.FileInfo, err error) error { if err != nil { + // Ignore errors that occur when the project contains a symlink to a filesystem or volume that + // Windows doesn't have access to. + if path != "." && isAccessDenied(err) { + fmt.Fprintf(os.Stderr, "%s: %v\n", path, err) + return nil + } return err } if foundLater { @@ -160,9 +167,18 @@ func sourceFilesLaterThan(t time.Time) bool { } return nil }) + if err != nil { + panic(err) + } return foundLater } +func isAccessDenied(err error) bool { + var pe *os.PathError + // we would use `syscall.ERROR_ACCESS_DENIED` if this script supported build tags + return errors.As(err, &pe) && strings.Contains(pe.Err.Error(), "Access is denied") +} + func rmrf(targets ...string) error { args := append([]string{"rm", "-rf"}, targets...) announce(args...) From 32f9a462a8a5be23b133272a14b18e6c4552da46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 15 Jun 2021 17:32:43 +0200 Subject: [PATCH 8/9] Speed up build script by avoiding recursing into 3rd-party directories --- script/build.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/script/build.go b/script/build.go index ee0ac4652..3d04fd6b2 100644 --- a/script/build.go +++ b/script/build.go @@ -158,6 +158,9 @@ func sourceFilesLaterThan(t time.Time) bool { } } if info.IsDir() { + if name := filepath.Base(path); name == "vendor" || name == "node_modules" { + return filepath.SkipDir + } return nil } if path == "go.mod" || path == "go.sum" || (strings.HasSuffix(path, ".go") && !strings.HasSuffix(path, "_test.go")) { From bd0156625137f3490d38dd6dbcc149e2726e7af1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 15 Jun 2021 17:33:33 +0200 Subject: [PATCH 9/9] Allow `script\build` as shorthand for `go run script\build.go` on Windows --- script/build.bat | 1 + 1 file changed, 1 insertion(+) create mode 100644 script/build.bat diff --git a/script/build.bat b/script/build.bat new file mode 100644 index 000000000..c88d5b22b --- /dev/null +++ b/script/build.bat @@ -0,0 +1 @@ +go run script\build.go %*