From a315e6865c266e5a907d2f79b52c502dfafeebee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 18 Feb 2022 20:27:40 +0100 Subject: [PATCH 01/30] run download: fix extracting to root path Our rudimentary check for whether a file path is entirely contained under a directory had a false negative when the parent directory is "/". --- pkg/cmd/run/download/zip.go | 21 ++++-- pkg/cmd/run/download/zip_test.go | 116 +++++++++++++++++++++++++++++++ 2 files changed, 130 insertions(+), 7 deletions(-) diff --git a/pkg/cmd/run/download/zip.go b/pkg/cmd/run/download/zip.go index d7a2613bc..bf56ea081 100644 --- a/pkg/cmd/run/download/zip.go +++ b/pkg/cmd/run/download/zip.go @@ -16,14 +16,9 @@ const ( ) func extractZip(zr *zip.Reader, destDir string) error { - destDirAbs, err := filepath.Abs(destDir) - if err != nil { - return err - } - pathPrefix := destDirAbs + string(filepath.Separator) for _, zf := range zr.File { - fpath := filepath.Join(destDirAbs, filepath.FromSlash(zf.Name)) - if !strings.HasPrefix(fpath, pathPrefix) { + fpath := filepath.Join(destDir, filepath.FromSlash(zf.Name)) + if !filepathDescendsFrom(fpath, destDir) { continue } if err := extractZipFile(zf, fpath); err != nil { @@ -67,3 +62,15 @@ func getPerm(m os.FileMode) os.FileMode { } return execMode } + +func filepathDescendsFrom(p, dir string) bool { + p = filepath.Clean(p) + dir = filepath.Clean(dir) + if dir == "." && !filepath.IsAbs(p) { + return !strings.HasPrefix(p, ".."+string(filepath.Separator)) + } + if !strings.HasSuffix(dir, string(filepath.Separator)) { + dir += string(filepath.Separator) + } + return strings.HasPrefix(p, dir) +} diff --git a/pkg/cmd/run/download/zip_test.go b/pkg/cmd/run/download/zip_test.go index f859511ee..97861b183 100644 --- a/pkg/cmd/run/download/zip_test.go +++ b/pkg/cmd/run/download/zip_test.go @@ -30,3 +30,119 @@ func Test_extractZip(t *testing.T) { _, err = os.Stat(filepath.Join("src", "main.go")) require.NoError(t, err) } + +func Test_filepathDescendsFrom(t *testing.T) { + type args struct { + p string + dir string + } + tests := []struct { + name string + args args + want bool + }{ + { + name: "root child", + args: args{ + p: filepath.FromSlash("/hoi.txt"), + dir: filepath.FromSlash("/"), + }, + want: true, + }, + { + name: "abs descendant", + args: args{ + p: filepath.FromSlash("/var/logs/hoi.txt"), + dir: filepath.FromSlash("/"), + }, + want: true, + }, + { + name: "abs trailing slash", + args: args{ + p: filepath.FromSlash("/var/logs/hoi.txt"), + dir: filepath.FromSlash("/var/logs/"), + }, + want: true, + }, + { + name: "abs mismatch", + args: args{ + p: filepath.FromSlash("/var/logs/hoi.txt"), + dir: filepath.FromSlash("/var/pids"), + }, + want: false, + }, + { + name: "abs partial prefix", + args: args{ + p: filepath.FromSlash("/var/logs/hoi.txt"), + dir: filepath.FromSlash("/var/log"), + }, + want: false, + }, + { + name: "rel child", + args: args{ + p: filepath.FromSlash("hoi.txt"), + dir: filepath.FromSlash("."), + }, + want: true, + }, + { + name: "rel descendant", + args: args{ + p: filepath.FromSlash("./log/hoi.txt"), + dir: filepath.FromSlash("."), + }, + want: true, + }, + { + name: "mixed rel styles", + args: args{ + p: filepath.FromSlash("./log/hoi.txt"), + dir: filepath.FromSlash("log"), + }, + want: true, + }, + { + name: "rel clean", + args: args{ + p: filepath.FromSlash("cats/../dogs/pug.txt"), + dir: filepath.FromSlash("dogs"), + }, + want: true, + }, + { + name: "rel mismatch", + args: args{ + p: filepath.FromSlash("dogs/pug.txt"), + dir: filepath.FromSlash("dog"), + }, + want: false, + }, + { + name: "rel breakout", + args: args{ + p: filepath.FromSlash("../escape.txt"), + dir: filepath.FromSlash("."), + }, + want: false, + }, + { + name: "rel sneaky breakout", + args: args{ + p: filepath.FromSlash("dogs/../../escape.txt"), + dir: filepath.FromSlash("dogs"), + }, + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := filepathDescendsFrom(tt.args.p, tt.args.dir); got != tt.want { + t.Errorf("filepathDescendsFrom() = %v, want %v", got, tt.want) + } + }) + } +} From fe5eca1dd5da2625abee56aaebed26ae1ecde7b4 Mon Sep 17 00:00:00 2001 From: meiji163 Date: Tue, 1 Mar 2022 15:01:04 -0800 Subject: [PATCH 02/30] pinning binary exts --- pkg/cmd/extension/command.go | 63 ++++++++++++++++++---------------- pkg/cmd/extension/http.go | 34 ++++++++++++++++++ pkg/cmd/extension/manager.go | 17 +++++---- pkg/extensions/extension.go | 2 +- pkg/extensions/manager_mock.go | 14 +++++--- 5 files changed, 90 insertions(+), 40 deletions(-) diff --git a/pkg/cmd/extension/command.go b/pkg/cmd/extension/command.go index de39d0287..c4a08152b 100644 --- a/pkg/cmd/extension/command.go +++ b/pkg/cmd/extension/command.go @@ -70,10 +70,12 @@ func NewCmdExtension(f *cmdutil.Factory) *cobra.Command { return t.Render() }, }, - &cobra.Command{ - Use: "install ", - Short: "Install a gh extension from a repository", - Long: heredoc.Doc(` + func() *cobra.Command { + var pinFlag string + cmd := &cobra.Command{ + Use: "install ", + Short: "Install a gh extension from a repository", + Long: heredoc.Doc(` Install a GitHub repository locally as a GitHub CLI extension. The repository argument can be specified in "owner/repo" format as well as a full URL. @@ -84,41 +86,44 @@ func NewCmdExtension(f *cmdutil.Factory) *cobra.Command { See the list of available extensions at `), - Example: heredoc.Doc(` + Example: heredoc.Doc(` $ gh extension install owner/gh-extension $ gh extension install https://git.example.com/owner/gh-extension $ gh extension install . `), - Args: cmdutil.MinimumArgs(1, "must specify a repository to install from"), - RunE: func(cmd *cobra.Command, args []string) error { - if args[0] == "." { - wd, err := os.Getwd() + Args: cmdutil.MinimumArgs(1, "must specify a repository to install from"), + RunE: func(cmd *cobra.Command, args []string) error { + if args[0] == "." { + wd, err := os.Getwd() + if err != nil { + return err + } + return m.InstallLocal(wd) + } + + repo, err := ghrepo.FromFullName(args[0]) if err != nil { return err } - return m.InstallLocal(wd) - } - repo, err := ghrepo.FromFullName(args[0]) - if err != nil { - return err - } + if err := checkValidExtension(cmd.Root(), m, repo.RepoName()); err != nil { + return err + } - if err := checkValidExtension(cmd.Root(), m, repo.RepoName()); err != nil { - return err - } + if err := m.Install(repo, pinFlag); err != nil { + return err + } - if err := m.Install(repo); err != nil { - return err - } - - if io.IsStdoutTTY() { - cs := io.ColorScheme() - fmt.Fprintf(io.Out, "%s Installed extension %s\n", cs.SuccessIcon(), args[0]) - } - return nil - }, - }, + if io.IsStdoutTTY() { + cs := io.ColorScheme() + fmt.Fprintf(io.Out, "%s Installed extension %s\n", cs.SuccessIcon(), args[0]) + } + return nil + }, + } + cmd.Flags().StringVar(&pinFlag, "pin", "", "pin extension to a release tag or commit sha") + return cmd + }(), func() *cobra.Command { var flagAll bool var flagForce bool diff --git a/pkg/cmd/extension/http.go b/pkg/cmd/extension/http.go index cfae2b738..d1bbe84e7 100644 --- a/pkg/cmd/extension/http.go +++ b/pkg/cmd/extension/http.go @@ -112,3 +112,37 @@ func fetchLatestRelease(httpClient *http.Client, baseRepo ghrepo.Interface) (*re return &r, nil } + +// fetchRelease finds release by tag name for a repository +func fetchReleaseFromTag(httpClient *http.Client, baseRepo ghrepo.Interface, tagName string) (*release, error) { + fullRepoName := fmt.Sprintf("%s/%s", baseRepo.RepoOwner(), baseRepo.RepoName()) + path := fmt.Sprintf("repos/%s/releases/tags/%s", fullRepoName, tagName) + url := ghinstance.RESTPrefix(baseRepo.RepoHost()) + path + req, err := http.NewRequest("GET", url, nil) + if err != nil { + return nil, err + } + + resp, err := httpClient.Do(req) + defer resp.Body.Close() + if err != nil { + return nil, err + } + + if resp.StatusCode > 299 { + return nil, api.HandleHTTPError(resp) + } + + b, err := ioutil.ReadAll(resp.Body) + if err != nil { + return nil, err + } + + var r release + err = json.Unmarshal(b, &r) + if err != nil { + return nil, err + } + + return &r, nil +} diff --git a/pkg/cmd/extension/manager.go b/pkg/cmd/extension/manager.go index 691110656..6ff5f461a 100644 --- a/pkg/cmd/extension/manager.go +++ b/pkg/cmd/extension/manager.go @@ -320,13 +320,13 @@ type binManifest struct { Path string } -func (m *Manager) Install(repo ghrepo.Interface) error { +func (m *Manager) Install(repo ghrepo.Interface, targetCommitish string) error { isBin, err := isBinExtension(m.client, repo) if err != nil { return fmt.Errorf("could not check for binary extension: %w", err) } if isBin { - return m.installBin(repo) + return m.installBin(repo, targetCommitish) } hs, err := hasScript(m.client, repo) @@ -341,9 +341,14 @@ func (m *Manager) Install(repo ghrepo.Interface) error { return m.installGit(ghrepo.FormatRemoteURL(repo, protocol), m.io.Out, m.io.ErrOut) } -func (m *Manager) installBin(repo ghrepo.Interface) error { +func (m *Manager) installBin(repo ghrepo.Interface, targetCommitish string) error { var r *release - r, err := fetchLatestRelease(m.client, repo) + var err error + if targetCommitish == "" { + r, err = fetchLatestRelease(m.client, repo) + } else { + r, err = fetchReleaseFromTag(m.client, repo, targetCommitish) + } if err != nil { return err } @@ -498,7 +503,7 @@ func (m *Manager) upgradeExtension(ext Extension, force bool) error { if err != nil { return fmt.Errorf("failed to migrate to new precompiled extension format: %w", err) } - return m.installBin(repo) + return m.installBin(repo, "") } err = m.upgradeGitExtension(ext, force) } @@ -525,7 +530,7 @@ func (m *Manager) upgradeBinExtension(ext Extension) error { if err != nil { return fmt.Errorf("failed to parse URL %s: %w", ext.url, err) } - return m.installBin(repo) + return m.installBin(repo, "") } func (m *Manager) Remove(name string) error { diff --git a/pkg/extensions/extension.go b/pkg/extensions/extension.go index 5826f4cd6..6b57a1cc5 100644 --- a/pkg/extensions/extension.go +++ b/pkg/extensions/extension.go @@ -27,7 +27,7 @@ type Extension interface { //go:generate moq -rm -out manager_mock.go . ExtensionManager type ExtensionManager interface { List(includeMetadata bool) []Extension - Install(ghrepo.Interface) error + Install(ghrepo.Interface, string) error InstallLocal(dir string) error Upgrade(name string, force bool) error Remove(name string) error diff --git a/pkg/extensions/manager_mock.go b/pkg/extensions/manager_mock.go index 80e1efd57..01eac92bb 100644 --- a/pkg/extensions/manager_mock.go +++ b/pkg/extensions/manager_mock.go @@ -25,7 +25,7 @@ var _ ExtensionManager = &ExtensionManagerMock{} // DispatchFunc: func(args []string, stdin io.Reader, stdout io.Writer, stderr io.Writer) (bool, error) { // panic("mock out the Dispatch method") // }, -// InstallFunc: func(interfaceMoqParam ghrepo.Interface) error { +// InstallFunc: func(interfaceMoqParam ghrepo.Interface, s string) error { // panic("mock out the Install method") // }, // InstallLocalFunc: func(dir string) error { @@ -54,7 +54,7 @@ type ExtensionManagerMock struct { DispatchFunc func(args []string, stdin io.Reader, stdout io.Writer, stderr io.Writer) (bool, error) // InstallFunc mocks the Install method. - InstallFunc func(interfaceMoqParam ghrepo.Interface) error + InstallFunc func(interfaceMoqParam ghrepo.Interface, s string) error // InstallLocalFunc mocks the InstallLocal method. InstallLocalFunc func(dir string) error @@ -92,6 +92,8 @@ type ExtensionManagerMock struct { Install []struct { // InterfaceMoqParam is the interfaceMoqParam argument value. InterfaceMoqParam ghrepo.Interface + // S is the s argument value. + S string } // InstallLocal holds details about calls to the InstallLocal method. InstallLocal []struct { @@ -204,19 +206,21 @@ func (mock *ExtensionManagerMock) DispatchCalls() []struct { } // Install calls InstallFunc. -func (mock *ExtensionManagerMock) Install(interfaceMoqParam ghrepo.Interface) error { +func (mock *ExtensionManagerMock) Install(interfaceMoqParam ghrepo.Interface, s string) error { if mock.InstallFunc == nil { panic("ExtensionManagerMock.InstallFunc: method is nil but ExtensionManager.Install was just called") } callInfo := struct { InterfaceMoqParam ghrepo.Interface + S string }{ InterfaceMoqParam: interfaceMoqParam, + S: s, } mock.lockInstall.Lock() mock.calls.Install = append(mock.calls.Install, callInfo) mock.lockInstall.Unlock() - return mock.InstallFunc(interfaceMoqParam) + return mock.InstallFunc(interfaceMoqParam, s) } // InstallCalls gets all the calls that were made to Install. @@ -224,9 +228,11 @@ func (mock *ExtensionManagerMock) Install(interfaceMoqParam ghrepo.Interface) er // len(mockedExtensionManager.InstallCalls()) func (mock *ExtensionManagerMock) InstallCalls() []struct { InterfaceMoqParam ghrepo.Interface + S string } { var calls []struct { InterfaceMoqParam ghrepo.Interface + S string } mock.lockInstall.RLock() calls = mock.calls.Install From 44334bbec65bea8b09852640e1a812c9704c6ab4 Mon Sep 17 00:00:00 2001 From: meiji163 Date: Wed, 2 Mar 2022 19:25:25 -0800 Subject: [PATCH 03/30] pinning script exts --- pkg/cmd/extension/command.go | 14 +++++++++++-- pkg/cmd/extension/http.go | 40 +++++++++++++++++++++++++++++++++++- pkg/cmd/extension/manager.go | 31 ++++++++++++++++++++++++---- 3 files changed, 78 insertions(+), 7 deletions(-) diff --git a/pkg/cmd/extension/command.go b/pkg/cmd/extension/command.go index 916940749..63f560710 100644 --- a/pkg/cmd/extension/command.go +++ b/pkg/cmd/extension/command.go @@ -111,18 +111,28 @@ func NewCmdExtension(f *cmdutil.Factory) *cobra.Command { return err } + cs := io.ColorScheme() if err := m.Install(repo, pinFlag); err != nil { + if errors.Is(err, releaseNotFoundErr) { + return fmt.Errorf("%s Could not find a release of %s for %s", + cs.FailureIcon(), args[0], cs.Cyan(pinFlag)) + } else if errors.Is(err, commitNotFoundErr) { + return fmt.Errorf("%s %s does not exist in %s", + cs.FailureIcon(), cs.Cyan(pinFlag), args[0]) + } return err } if io.IsStdoutTTY() { - cs := io.ColorScheme() fmt.Fprintf(io.Out, "%s Installed extension %s\n", cs.SuccessIcon(), args[0]) + if pinFlag != "" { + fmt.Fprintf(io.Out, "%s Pinned extension at %s\n", cs.SuccessIcon(), cs.Cyan(pinFlag)) + } } return nil }, } - cmd.Flags().StringVar(&pinFlag, "pin", "", "pin extension to a release tag or commit sha") + cmd.Flags().StringVar(&pinFlag, "pin", "", "pin extension to a release tag or commit ref") return cmd }(), func() *cobra.Command { diff --git a/pkg/cmd/extension/http.go b/pkg/cmd/extension/http.go index d1bbe84e7..38ba9c5e5 100644 --- a/pkg/cmd/extension/http.go +++ b/pkg/cmd/extension/http.go @@ -2,6 +2,7 @@ package extension import ( "encoding/json" + "errors" "fmt" "io" "io/ioutil" @@ -80,6 +81,9 @@ func downloadAsset(httpClient *http.Client, asset releaseAsset, destPath string) return err } +var releaseNotFoundErr = errors.New("release not found") +var commitNotFoundErr = errors.New("commit not found") + // fetchLatestRelease finds the latest published release for a repository. func fetchLatestRelease(httpClient *http.Client, baseRepo ghrepo.Interface) (*release, error) { path := fmt.Sprintf("repos/%s/%s/releases/latest", baseRepo.RepoOwner(), baseRepo.RepoName()) @@ -113,7 +117,7 @@ func fetchLatestRelease(httpClient *http.Client, baseRepo ghrepo.Interface) (*re return &r, nil } -// fetchRelease finds release by tag name for a repository +// fetchReleaseFromTag finds release by tag name for a repository func fetchReleaseFromTag(httpClient *http.Client, baseRepo ghrepo.Interface, tagName string) (*release, error) { fullRepoName := fmt.Sprintf("%s/%s", baseRepo.RepoOwner(), baseRepo.RepoName()) path := fmt.Sprintf("repos/%s/releases/tags/%s", fullRepoName, tagName) @@ -129,6 +133,9 @@ func fetchReleaseFromTag(httpClient *http.Client, baseRepo ghrepo.Interface, tag return nil, err } + if resp.StatusCode == 404 { + return nil, releaseNotFoundErr + } if resp.StatusCode > 299 { return nil, api.HandleHTTPError(resp) } @@ -146,3 +153,34 @@ func fetchReleaseFromTag(httpClient *http.Client, baseRepo ghrepo.Interface, tag return &r, nil } + +// fetchCommitSHA finds full commit SHA from a target ref in a repo +func fetchCommitSHA(httpClient *http.Client, baseRepo ghrepo.Interface, targetRef string) (string, error) { + path := fmt.Sprintf("repos/%s/%s/commits/%s", baseRepo.RepoOwner(), baseRepo.RepoName(), targetRef) + url := ghinstance.RESTPrefix(baseRepo.RepoHost()) + path + req, err := http.NewRequest("GET", url, nil) + if err != nil { + return "", err + } + + req.Header.Set("Accept", "application/vnd.github.VERSION.sha") + resp, err := httpClient.Do(req) + defer resp.Body.Close() + if err != nil { + return "", err + } + + if resp.StatusCode == 422 { + return "", commitNotFoundErr + } + if resp.StatusCode > 299 { + return "", api.HandleHTTPError(resp) + } + + body, err := ioutil.ReadAll(resp.Body) + if err != nil { + return "", err + } + + return string(body), nil +} diff --git a/pkg/cmd/extension/manager.go b/pkg/cmd/extension/manager.go index 41dab528d..aa45ee48c 100644 --- a/pkg/cmd/extension/manager.go +++ b/pkg/cmd/extension/manager.go @@ -8,6 +8,7 @@ import ( "io" "io/fs" "io/ioutil" + "log" "net/http" "os" "os/exec" @@ -320,6 +321,7 @@ type binManifest struct { Path string } +// Install an extension from repo, and pin to commitish if provided func (m *Manager) Install(repo ghrepo.Interface, targetCommitish string) error { isBin, err := isBinExtension(m.client, repo) if err != nil { @@ -337,11 +339,11 @@ func (m *Manager) Install(repo ghrepo.Interface, targetCommitish string) error { return errors.New("extension is not installable: missing executable") } - protocol, _ := m.config.GetOrDefault(repo.RepoHost(), "git_protocol") - return m.installGit(ghrepo.FormatRemoteURL(repo, protocol), m.io.Out, m.io.ErrOut) + return m.installGit(repo, targetCommitish, m.io.Out, m.io.ErrOut) } func (m *Manager) installBin(repo ghrepo.Interface, targetCommitish string) error { + log.Println("Installing binary extension") var r *release var err error if targetCommitish == "" { @@ -413,19 +415,40 @@ func (m *Manager) installBin(repo ghrepo.Interface, targetCommitish string) erro return nil } -func (m *Manager) installGit(cloneURL string, stdout, stderr io.Writer) error { +func (m *Manager) installGit(repo ghrepo.Interface, targetCommitish string, stdout, stderr io.Writer) error { + protocol, _ := m.config.GetOrDefault(repo.RepoHost(), "git_protocol") + cloneURL := ghrepo.FormatRemoteURL(repo, protocol) + exe, err := m.lookPath("git") if err != nil { return err } + var commitSHA string + if targetCommitish != "" { + commitSHA, err = fetchCommitSHA(m.client, repo, targetCommitish) + if err != nil { + return err + } + } + name := strings.TrimSuffix(path.Base(cloneURL), ".git") targetDir := filepath.Join(m.installDir(), name) externalCmd := m.newCommand(exe, "clone", cloneURL, targetDir) externalCmd.Stdout = stdout externalCmd.Stderr = stderr - return externalCmd.Run() + if err := externalCmd.Run(); err != nil { + return err + } + if commitSHA == "" { + return nil + } + + checkoutCmd := m.newCommand(exe, "-C", targetDir, "checkout", commitSHA) + checkoutCmd.Stdout = stdout + checkoutCmd.Stderr = stderr + return checkoutCmd.Run() } var localExtensionUpgradeError = errors.New("local extensions can not be upgraded") From bdab7de1d2b79641eb873cbb9b0b1b5c08017478 Mon Sep 17 00:00:00 2001 From: meiji163 Date: Wed, 2 Mar 2022 21:06:23 -0800 Subject: [PATCH 04/30] list pinned exts --- pkg/cmd/extension/command.go | 11 ++++--- pkg/cmd/extension/extension.go | 8 ++++- pkg/cmd/extension/manager.go | 55 ++++++++++++++++++++++++-------- pkg/extensions/extension.go | 3 +- pkg/extensions/extension_mock.go | 36 +++++++++++++++++++++ 5 files changed, 93 insertions(+), 20 deletions(-) diff --git a/pkg/cmd/extension/command.go b/pkg/cmd/extension/command.go index 63f560710..eb3a4306a 100644 --- a/pkg/cmd/extension/command.go +++ b/pkg/cmd/extension/command.go @@ -61,11 +61,14 @@ func NewCmdExtension(f *cmdutil.Factory) *cobra.Command { t.AddField(fmt.Sprintf("gh %s", c.Name()), nil, nil) t.AddField(repo, nil, nil) - var updateAvailable string - if c.UpdateAvailable() { - updateAvailable = "Upgrade available" + + if c.Pin() != "" { + t.AddField(c.Pin(), nil, cs.Cyan) + } else if c.UpdateAvailable() { + t.AddField("Upgrade available", nil, cs.Green) + } else { + t.AddField("", nil, nil) } - t.AddField(updateAvailable, nil, cs.Green) t.EndRow() } return t.Render() diff --git a/pkg/cmd/extension/extension.go b/pkg/cmd/extension/extension.go index b4106228f..87cd20c78 100644 --- a/pkg/cmd/extension/extension.go +++ b/pkg/cmd/extension/extension.go @@ -18,6 +18,7 @@ type Extension struct { path string url string isLocal bool + pin string currentVersion string latestVersion string kind ExtensionKind @@ -39,8 +40,13 @@ func (e *Extension) IsLocal() bool { return e.isLocal } +func (e *Extension) Pin() string { + return e.pin +} + func (e *Extension) UpdateAvailable() bool { - if e.isLocal || + if e.pin != "" || + e.isLocal || e.currentVersion == "" || e.latestVersion == "" || e.currentVersion == e.latestVersion { diff --git a/pkg/cmd/extension/manager.go b/pkg/cmd/extension/manager.go index aa45ee48c..34db89a10 100644 --- a/pkg/cmd/extension/manager.go +++ b/pkg/cmd/extension/manager.go @@ -8,7 +8,6 @@ import ( "io" "io/fs" "io/ioutil" - "log" "net/http" "os" "os/exec" @@ -199,6 +198,9 @@ func (m *Manager) parseBinaryExtensionDir(fi fs.FileInfo) (Extension, error) { remoteURL := ghrepo.GenerateRepoURL(repo, "") ext.url = remoteURL ext.currentVersion = bm.Tag + if bm.Pinned { + ext.pin = bm.Tag + } return ext, nil } @@ -207,12 +209,20 @@ func (m *Manager) parseGitExtensionDir(fi fs.FileInfo) (Extension, error) { exePath := filepath.Join(id, fi.Name(), fi.Name()) remoteUrl := m.getRemoteUrl(fi.Name()) currentVersion := m.getCurrentVersion(fi.Name()) + + var pinnedVersion string + pinPath := filepath.Join(id, fi.Name(), fmt.Sprintf(".pin-%s", currentVersion)) + if _, err := os.Stat(pinPath); err == nil { + pinnedVersion = currentVersion[:7] + } + return Extension{ path: exePath, url: remoteUrl, isLocal: false, currentVersion: currentVersion, kind: GitKind, + pin: pinnedVersion, }, nil } @@ -313,15 +323,16 @@ func (m *Manager) InstallLocal(dir string) error { } type binManifest struct { - Owner string - Name string - Host string - Tag string + Owner string + Name string + Host string + Tag string + Pinned bool // TODO I may end up not using this; just thinking ahead to local installs Path string } -// Install an extension from repo, and pin to commitish if provided +// Install installs an extension from repo, and pins to commitish if provided func (m *Manager) Install(repo ghrepo.Interface, targetCommitish string) error { isBin, err := isBinExtension(m.client, repo) if err != nil { @@ -343,10 +354,10 @@ func (m *Manager) Install(repo ghrepo.Interface, targetCommitish string) error { } func (m *Manager) installBin(repo ghrepo.Interface, targetCommitish string) error { - log.Println("Installing binary extension") var r *release var err error - if targetCommitish == "" { + isPinned := targetCommitish != "" + if isPinned { r, err = fetchLatestRelease(m.client, repo) } else { r, err = fetchReleaseFromTag(m.client, repo, targetCommitish) @@ -372,6 +383,7 @@ func (m *Manager) installBin(repo ghrepo.Interface, targetCommitish string) erro name := repo.RepoName() targetDir := filepath.Join(m.installDir(), name) + // TODO clean this up if function errs? err = os.MkdirAll(targetDir, 0755) if err != nil { @@ -387,11 +399,12 @@ func (m *Manager) installBin(repo ghrepo.Interface, targetCommitish string) erro } manifest := binManifest{ - Name: name, - Owner: repo.RepoOwner(), - Host: repo.RepoHost(), - Path: binPath, - Tag: r.Tag, + Name: name, + Owner: repo.RepoOwner(), + Host: repo.RepoHost(), + Path: binPath, + Tag: r.Tag, + Pinned: isPinned, } bs, err := yaml.Marshal(manifest) @@ -448,9 +461,20 @@ func (m *Manager) installGit(repo ghrepo.Interface, targetCommitish string, stdo checkoutCmd := m.newCommand(exe, "-C", targetDir, "checkout", commitSHA) checkoutCmd.Stdout = stdout checkoutCmd.Stderr = stderr - return checkoutCmd.Run() + if err := checkoutCmd.Run(); err != nil { + return err + } + + pinPath := filepath.Join(targetDir, fmt.Sprintf(".pin-%s", commitSHA)) + f, err := os.OpenFile(pinPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0600) + defer f.Close() + if err != nil { + return fmt.Errorf("failed to create pin in directory: %w", err) + } + return nil } +var pinnedExtensionUpgradeError = errors.New("pinned extensions can not be upgraded") var localExtensionUpgradeError = errors.New("local extensions can not be upgraded") var upToDateError = errors.New("already up to date") var noExtensionsInstalledError = errors.New("no extensions installed") @@ -508,6 +532,9 @@ func (m *Manager) upgradeExtension(ext Extension, force bool) error { if ext.isLocal { return localExtensionUpgradeError } + if ext.pin != "" { + return pinnedExtensionUpgradeError + } if !ext.UpdateAvailable() { return upToDateError } diff --git a/pkg/extensions/extension.go b/pkg/extensions/extension.go index 6b57a1cc5..2db8ab0fb 100644 --- a/pkg/extensions/extension.go +++ b/pkg/extensions/extension.go @@ -19,9 +19,10 @@ type Extension interface { Name() string // Extension Name without gh- Path() string // Path to executable URL() string - IsLocal() bool + Pin() string // Pinned version UpdateAvailable() bool IsBinary() bool + IsLocal() bool } //go:generate moq -rm -out manager_mock.go . ExtensionManager diff --git a/pkg/extensions/extension_mock.go b/pkg/extensions/extension_mock.go index b999ab955..9d47d1c4f 100644 --- a/pkg/extensions/extension_mock.go +++ b/pkg/extensions/extension_mock.go @@ -29,6 +29,9 @@ var _ Extension = &ExtensionMock{} // PathFunc: func() string { // panic("mock out the Path method") // }, +// PinFunc: func() string { +// panic("mock out the Pin method") +// }, // URLFunc: func() string { // panic("mock out the URL method") // }, @@ -54,6 +57,9 @@ type ExtensionMock struct { // PathFunc mocks the Path method. PathFunc func() string + // PinFunc mocks the Pin method. + PinFunc func() string + // URLFunc mocks the URL method. URLFunc func() string @@ -74,6 +80,9 @@ type ExtensionMock struct { // Path holds details about calls to the Path method. Path []struct { } + // Pin holds details about calls to the Pin method. + Pin []struct { + } // URL holds details about calls to the URL method. URL []struct { } @@ -85,6 +94,7 @@ type ExtensionMock struct { lockIsLocal sync.RWMutex lockName sync.RWMutex lockPath sync.RWMutex + lockPin sync.RWMutex lockURL sync.RWMutex lockUpdateAvailable sync.RWMutex } @@ -193,6 +203,32 @@ func (mock *ExtensionMock) PathCalls() []struct { return calls } +// Pin calls PinFunc. +func (mock *ExtensionMock) Pin() string { + if mock.PinFunc == nil { + panic("ExtensionMock.PinFunc: method is nil but Extension.Pin was just called") + } + callInfo := struct { + }{} + mock.lockPin.Lock() + mock.calls.Pin = append(mock.calls.Pin, callInfo) + mock.lockPin.Unlock() + return mock.PinFunc() +} + +// PinCalls gets all the calls that were made to Pin. +// Check the length with: +// len(mockedExtension.PinCalls()) +func (mock *ExtensionMock) PinCalls() []struct { +} { + var calls []struct { + } + mock.lockPin.RLock() + calls = mock.calls.Pin + mock.lockPin.RUnlock() + return calls +} + // URL calls URLFunc. func (mock *ExtensionMock) URL() string { if mock.URLFunc == nil { From f350b917e477280f89b8351331e2d55110399945 Mon Sep 17 00:00:00 2001 From: meiji163 Date: Thu, 3 Mar 2022 22:10:35 -0800 Subject: [PATCH 05/30] fix tests --- pkg/cmd/extension/command_test.go | 2 +- pkg/cmd/extension/http.go | 4 +- pkg/cmd/extension/manager.go | 31 +++++++------- pkg/cmd/extension/manager_test.go | 68 +++++++++++++++++++++++++++++-- 4 files changed, 84 insertions(+), 21 deletions(-) diff --git a/pkg/cmd/extension/command_test.go b/pkg/cmd/extension/command_test.go index 8f896eab0..1c7e4dd6d 100644 --- a/pkg/cmd/extension/command_test.go +++ b/pkg/cmd/extension/command_test.go @@ -44,7 +44,7 @@ func TestNewCmdExtension(t *testing.T) { em.ListFunc = func(bool) []extensions.Extension { return []extensions.Extension{} } - em.InstallFunc = func(_ ghrepo.Interface) error { + em.InstallFunc = func(_ ghrepo.Interface, _ string) error { return nil } return func(t *testing.T) { diff --git a/pkg/cmd/extension/http.go b/pkg/cmd/extension/http.go index 38ba9c5e5..7208b1569 100644 --- a/pkg/cmd/extension/http.go +++ b/pkg/cmd/extension/http.go @@ -128,11 +128,11 @@ func fetchReleaseFromTag(httpClient *http.Client, baseRepo ghrepo.Interface, tag } resp, err := httpClient.Do(req) - defer resp.Body.Close() if err != nil { return nil, err } + defer resp.Body.Close() if resp.StatusCode == 404 { return nil, releaseNotFoundErr } @@ -165,11 +165,11 @@ func fetchCommitSHA(httpClient *http.Client, baseRepo ghrepo.Interface, targetRe req.Header.Set("Accept", "application/vnd.github.VERSION.sha") resp, err := httpClient.Do(req) - defer resp.Body.Close() if err != nil { return "", err } + defer resp.Body.Close() if resp.StatusCode == 422 { return "", commitNotFoundErr } diff --git a/pkg/cmd/extension/manager.go b/pkg/cmd/extension/manager.go index 34db89a10..b6067fb7d 100644 --- a/pkg/cmd/extension/manager.go +++ b/pkg/cmd/extension/manager.go @@ -212,7 +212,7 @@ func (m *Manager) parseGitExtensionDir(fi fs.FileInfo) (Extension, error) { var pinnedVersion string pinPath := filepath.Join(id, fi.Name(), fmt.Sprintf(".pin-%s", currentVersion)) - if _, err := os.Stat(pinPath); err == nil { + if _, err := os.Stat(pinPath); err == nil && len(currentVersion) > 6 { pinnedVersion = currentVersion[:7] } @@ -235,6 +235,7 @@ func (m *Manager) getCurrentVersion(extension string) string { dir := m.installDir() gitDir := "--git-dir=" + filepath.Join(dir, extension, ".git") cmd := m.newCommand(gitExe, gitDir, "rev-parse", "HEAD") + localSha, err := cmd.Output() if err != nil { return "" @@ -333,13 +334,13 @@ type binManifest struct { } // Install installs an extension from repo, and pins to commitish if provided -func (m *Manager) Install(repo ghrepo.Interface, targetCommitish string) error { +func (m *Manager) Install(repo ghrepo.Interface, target string) error { isBin, err := isBinExtension(m.client, repo) if err != nil { return fmt.Errorf("could not check for binary extension: %w", err) } if isBin { - return m.installBin(repo, targetCommitish) + return m.installBin(repo, target) } hs, err := hasScript(m.client, repo) @@ -350,17 +351,17 @@ func (m *Manager) Install(repo ghrepo.Interface, targetCommitish string) error { return errors.New("extension is not installable: missing executable") } - return m.installGit(repo, targetCommitish, m.io.Out, m.io.ErrOut) + return m.installGit(repo, target, m.io.Out, m.io.ErrOut) } -func (m *Manager) installBin(repo ghrepo.Interface, targetCommitish string) error { +func (m *Manager) installBin(repo ghrepo.Interface, target string) error { var r *release var err error - isPinned := targetCommitish != "" + isPinned := target != "" if isPinned { - r, err = fetchLatestRelease(m.client, repo) + r, err = fetchReleaseFromTag(m.client, repo, target) } else { - r, err = fetchReleaseFromTag(m.client, repo, targetCommitish) + r, err = fetchLatestRelease(m.client, repo) } if err != nil { return err @@ -428,7 +429,7 @@ func (m *Manager) installBin(repo ghrepo.Interface, targetCommitish string) erro return nil } -func (m *Manager) installGit(repo ghrepo.Interface, targetCommitish string, stdout, stderr io.Writer) error { +func (m *Manager) installGit(repo ghrepo.Interface, target string, stdout, stderr io.Writer) error { protocol, _ := m.config.GetOrDefault(repo.RepoHost(), "git_protocol") cloneURL := ghrepo.FormatRemoteURL(repo, protocol) @@ -438,8 +439,8 @@ func (m *Manager) installGit(repo ghrepo.Interface, targetCommitish string, stdo } var commitSHA string - if targetCommitish != "" { - commitSHA, err = fetchCommitSHA(m.client, repo, targetCommitish) + if target != "" { + commitSHA, err = fetchCommitSHA(m.client, repo, target) if err != nil { return err } @@ -466,12 +467,11 @@ func (m *Manager) installGit(repo ghrepo.Interface, targetCommitish string, stdo } pinPath := filepath.Join(targetDir, fmt.Sprintf(".pin-%s", commitSHA)) - f, err := os.OpenFile(pinPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0600) - defer f.Close() + f, err := os.OpenFile(pinPath, os.O_WRONLY|os.O_CREATE, 0600) if err != nil { return fmt.Errorf("failed to create pin in directory: %w", err) } - return nil + return f.Close() } var pinnedExtensionUpgradeError = errors.New("pinned extensions can not be upgraded") @@ -514,7 +514,8 @@ func (m *Manager) upgradeExtensions(exts []Extension, force bool) error { err := m.upgradeExtension(f, force) if err != nil { if !errors.Is(err, localExtensionUpgradeError) && - !errors.Is(err, upToDateError) { + !errors.Is(err, upToDateError) && + !errors.Is(err, pinnedExtensionUpgradeError) { failed = true } fmt.Fprintf(m.io.Out, "%s\n", err) diff --git a/pkg/cmd/extension/manager_test.go b/pkg/cmd/extension/manager_test.go index 47f11d348..e1fc45f49 100644 --- a/pkg/cmd/extension/manager_test.go +++ b/pkg/cmd/extension/manager_test.go @@ -445,6 +445,50 @@ func TestManager_UpgradeExtension_BinaryExtension(t *testing.T) { assert.Equal(t, "", stderr.String()) } +func TestManager_UpgradeExtension_PinnedBinaryExtension(t *testing.T) { + tempDir := t.TempDir() + + assert.NoError(t, stubBinaryExtension( + filepath.Join(tempDir, "extensions", "gh-bin-ext"), + binManifest{ + Owner: "owner", + Name: "gh-bin-ext", + Host: "example.com", + Tag: "v1.6.3", + Pinned: true, + })) + + io, _, _, _ := iostreams.Test() + m := newTestManager(tempDir, nil, io) + exts, err := m.list(false) + assert.Nil(t, err) + assert.Equal(t, 1, len(exts)) + ext := exts[0] + + err = m.upgradeExtension(ext, false) + assert.NotNil(t, err) + assert.Equal(t, err, pinnedExtensionUpgradeError) +} + +func TestManager_UpgradeExtenion_PinnedGitExtension(t *testing.T) { + tempDir := t.TempDir() + extDir := filepath.Join(tempDir, "extensions", "gh-remote") + assert.NoError(t, stubPinnedExtension(filepath.Join(extDir, "gh-remote"), "abc1234")) + + io, _, _, _ := iostreams.Test() + m := newTestManager(tempDir, nil, io) + exts, err := m.list(false) + assert.NoError(t, err) + assert.Equal(t, 1, len(exts)) + ext := exts[0] + ext.latestVersion = "new version" + ext.pin = "abc1234" + + err = m.upgradeExtension(ext, false) + assert.NotNil(t, err) + assert.Equal(t, err, pinnedExtensionUpgradeError) +} + func TestManager_Install_git(t *testing.T) { tempDir := t.TempDir() @@ -472,7 +516,7 @@ func TestManager_Install_git(t *testing.T) { repo := ghrepo.New("owner", "gh-some-ext") - err := m.Install(repo) + err := m.Install(repo, "") 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()) @@ -514,7 +558,7 @@ func TestManager_Install_binary_unsupported(t *testing.T) { m := newTestManager(tempDir, &client, io) - err := m.Install(repo) + err := m.Install(repo, "") assert.EqualError(t, err, "gh-bin-ext unsupported for windows-amd64. Open an issue: `gh issue create -R owner/gh-bin-ext -t'Support windows-amd64'`") assert.Equal(t, "", stdout.String()) @@ -559,7 +603,7 @@ func TestManager_Install_binary(t *testing.T) { m := newTestManager(tempDir, &http.Client{Transport: ®}, io) - err := m.Install(repo) + err := m.Install(repo, "") assert.NoError(t, err) manifest, err := os.ReadFile(filepath.Join(tempDir, "extensions/gh-bin-ext", manifestName)) @@ -702,6 +746,24 @@ func stubExtension(path string) error { return f.Close() } +func stubPinnedExtension(path string, pinnedVersion 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 + } + f.Close() + + pinPath := filepath.Join(filepath.Dir(path), fmt.Sprintf(".pin-%s", pinnedVersion)) + f, err = os.OpenFile(pinPath, os.O_WRONLY|os.O_CREATE, 0600) + if err != nil { + return err + } + return f.Close() +} + func stubLocalExtension(tempDir, path string) error { extDir, err := ioutil.TempDir(tempDir, "local-ext") if err != nil { From d7277e396cfc31cd7bf9bce5c4e82f17c7ef9f25 Mon Sep 17 00:00:00 2001 From: meiji163 Date: Tue, 1 Mar 2022 15:01:04 -0800 Subject: [PATCH 06/30] pinning binary exts --- pkg/cmd/extension/command.go | 63 ++++++++++++++++++---------------- pkg/cmd/extension/http.go | 34 ++++++++++++++++++ pkg/cmd/extension/manager.go | 17 +++++---- pkg/extensions/extension.go | 2 +- pkg/extensions/manager_mock.go | 14 +++++--- 5 files changed, 90 insertions(+), 40 deletions(-) diff --git a/pkg/cmd/extension/command.go b/pkg/cmd/extension/command.go index 1c929482f..50aad7a40 100644 --- a/pkg/cmd/extension/command.go +++ b/pkg/cmd/extension/command.go @@ -76,10 +76,12 @@ func NewCmdExtension(f *cmdutil.Factory) *cobra.Command { return t.Render() }, }, - &cobra.Command{ - Use: "install ", - Short: "Install a gh extension from a repository", - Long: heredoc.Doc(` + func() *cobra.Command { + var pinFlag string + cmd := &cobra.Command{ + Use: "install ", + Short: "Install a gh extension from a repository", + Long: heredoc.Doc(` Install a GitHub repository locally as a GitHub CLI extension. The repository argument can be specified in "owner/repo" format as well as a full URL. @@ -90,41 +92,44 @@ func NewCmdExtension(f *cmdutil.Factory) *cobra.Command { See the list of available extensions at . `), - Example: heredoc.Doc(` + Example: heredoc.Doc(` $ gh extension install owner/gh-extension $ gh extension install https://git.example.com/owner/gh-extension $ gh extension install . `), - Args: cmdutil.MinimumArgs(1, "must specify a repository to install from"), - RunE: func(cmd *cobra.Command, args []string) error { - if args[0] == "." { - wd, err := os.Getwd() + Args: cmdutil.MinimumArgs(1, "must specify a repository to install from"), + RunE: func(cmd *cobra.Command, args []string) error { + if args[0] == "." { + wd, err := os.Getwd() + if err != nil { + return err + } + return m.InstallLocal(wd) + } + + repo, err := ghrepo.FromFullName(args[0]) if err != nil { return err } - return m.InstallLocal(wd) - } - repo, err := ghrepo.FromFullName(args[0]) - if err != nil { - return err - } + if err := checkValidExtension(cmd.Root(), m, repo.RepoName()); err != nil { + return err + } - if err := checkValidExtension(cmd.Root(), m, repo.RepoName()); err != nil { - return err - } + if err := m.Install(repo, pinFlag); err != nil { + return err + } - if err := m.Install(repo); err != nil { - return err - } - - if io.IsStdoutTTY() { - cs := io.ColorScheme() - fmt.Fprintf(io.Out, "%s Installed extension %s\n", cs.SuccessIcon(), args[0]) - } - return nil - }, - }, + if io.IsStdoutTTY() { + cs := io.ColorScheme() + fmt.Fprintf(io.Out, "%s Installed extension %s\n", cs.SuccessIcon(), args[0]) + } + return nil + }, + } + cmd.Flags().StringVar(&pinFlag, "pin", "", "pin extension to a release tag or commit sha") + return cmd + }(), func() *cobra.Command { var flagAll bool var flagForce bool diff --git a/pkg/cmd/extension/http.go b/pkg/cmd/extension/http.go index cfae2b738..d1bbe84e7 100644 --- a/pkg/cmd/extension/http.go +++ b/pkg/cmd/extension/http.go @@ -112,3 +112,37 @@ func fetchLatestRelease(httpClient *http.Client, baseRepo ghrepo.Interface) (*re return &r, nil } + +// fetchRelease finds release by tag name for a repository +func fetchReleaseFromTag(httpClient *http.Client, baseRepo ghrepo.Interface, tagName string) (*release, error) { + fullRepoName := fmt.Sprintf("%s/%s", baseRepo.RepoOwner(), baseRepo.RepoName()) + path := fmt.Sprintf("repos/%s/releases/tags/%s", fullRepoName, tagName) + url := ghinstance.RESTPrefix(baseRepo.RepoHost()) + path + req, err := http.NewRequest("GET", url, nil) + if err != nil { + return nil, err + } + + resp, err := httpClient.Do(req) + defer resp.Body.Close() + if err != nil { + return nil, err + } + + if resp.StatusCode > 299 { + return nil, api.HandleHTTPError(resp) + } + + b, err := ioutil.ReadAll(resp.Body) + if err != nil { + return nil, err + } + + var r release + err = json.Unmarshal(b, &r) + if err != nil { + return nil, err + } + + return &r, nil +} diff --git a/pkg/cmd/extension/manager.go b/pkg/cmd/extension/manager.go index 3449092cc..41dab528d 100644 --- a/pkg/cmd/extension/manager.go +++ b/pkg/cmd/extension/manager.go @@ -320,13 +320,13 @@ type binManifest struct { Path string } -func (m *Manager) Install(repo ghrepo.Interface) error { +func (m *Manager) Install(repo ghrepo.Interface, targetCommitish string) error { isBin, err := isBinExtension(m.client, repo) if err != nil { return fmt.Errorf("could not check for binary extension: %w", err) } if isBin { - return m.installBin(repo) + return m.installBin(repo, targetCommitish) } hs, err := hasScript(m.client, repo) @@ -341,9 +341,14 @@ func (m *Manager) Install(repo ghrepo.Interface) error { return m.installGit(ghrepo.FormatRemoteURL(repo, protocol), m.io.Out, m.io.ErrOut) } -func (m *Manager) installBin(repo ghrepo.Interface) error { +func (m *Manager) installBin(repo ghrepo.Interface, targetCommitish string) error { var r *release - r, err := fetchLatestRelease(m.client, repo) + var err error + if targetCommitish == "" { + r, err = fetchLatestRelease(m.client, repo) + } else { + r, err = fetchReleaseFromTag(m.client, repo, targetCommitish) + } if err != nil { return err } @@ -498,7 +503,7 @@ func (m *Manager) upgradeExtension(ext Extension, force bool) error { if err != nil { return fmt.Errorf("failed to migrate to new precompiled extension format: %w", err) } - return m.installBin(repo) + return m.installBin(repo, "") } err = m.upgradeGitExtension(ext, force) } @@ -525,7 +530,7 @@ func (m *Manager) upgradeBinExtension(ext Extension) error { if err != nil { return fmt.Errorf("failed to parse URL %s: %w", ext.url, err) } - return m.installBin(repo) + return m.installBin(repo, "") } func (m *Manager) Remove(name string) error { diff --git a/pkg/extensions/extension.go b/pkg/extensions/extension.go index a9e120dc9..fa1959ab7 100644 --- a/pkg/extensions/extension.go +++ b/pkg/extensions/extension.go @@ -28,7 +28,7 @@ type Extension interface { //go:generate moq -rm -out manager_mock.go . ExtensionManager type ExtensionManager interface { List(includeMetadata bool) []Extension - Install(ghrepo.Interface) error + Install(ghrepo.Interface, string) error InstallLocal(dir string) error Upgrade(name string, force bool) error Remove(name string) error diff --git a/pkg/extensions/manager_mock.go b/pkg/extensions/manager_mock.go index 80e1efd57..01eac92bb 100644 --- a/pkg/extensions/manager_mock.go +++ b/pkg/extensions/manager_mock.go @@ -25,7 +25,7 @@ var _ ExtensionManager = &ExtensionManagerMock{} // DispatchFunc: func(args []string, stdin io.Reader, stdout io.Writer, stderr io.Writer) (bool, error) { // panic("mock out the Dispatch method") // }, -// InstallFunc: func(interfaceMoqParam ghrepo.Interface) error { +// InstallFunc: func(interfaceMoqParam ghrepo.Interface, s string) error { // panic("mock out the Install method") // }, // InstallLocalFunc: func(dir string) error { @@ -54,7 +54,7 @@ type ExtensionManagerMock struct { DispatchFunc func(args []string, stdin io.Reader, stdout io.Writer, stderr io.Writer) (bool, error) // InstallFunc mocks the Install method. - InstallFunc func(interfaceMoqParam ghrepo.Interface) error + InstallFunc func(interfaceMoqParam ghrepo.Interface, s string) error // InstallLocalFunc mocks the InstallLocal method. InstallLocalFunc func(dir string) error @@ -92,6 +92,8 @@ type ExtensionManagerMock struct { Install []struct { // InterfaceMoqParam is the interfaceMoqParam argument value. InterfaceMoqParam ghrepo.Interface + // S is the s argument value. + S string } // InstallLocal holds details about calls to the InstallLocal method. InstallLocal []struct { @@ -204,19 +206,21 @@ func (mock *ExtensionManagerMock) DispatchCalls() []struct { } // Install calls InstallFunc. -func (mock *ExtensionManagerMock) Install(interfaceMoqParam ghrepo.Interface) error { +func (mock *ExtensionManagerMock) Install(interfaceMoqParam ghrepo.Interface, s string) error { if mock.InstallFunc == nil { panic("ExtensionManagerMock.InstallFunc: method is nil but ExtensionManager.Install was just called") } callInfo := struct { InterfaceMoqParam ghrepo.Interface + S string }{ InterfaceMoqParam: interfaceMoqParam, + S: s, } mock.lockInstall.Lock() mock.calls.Install = append(mock.calls.Install, callInfo) mock.lockInstall.Unlock() - return mock.InstallFunc(interfaceMoqParam) + return mock.InstallFunc(interfaceMoqParam, s) } // InstallCalls gets all the calls that were made to Install. @@ -224,9 +228,11 @@ func (mock *ExtensionManagerMock) Install(interfaceMoqParam ghrepo.Interface) er // len(mockedExtensionManager.InstallCalls()) func (mock *ExtensionManagerMock) InstallCalls() []struct { InterfaceMoqParam ghrepo.Interface + S string } { var calls []struct { InterfaceMoqParam ghrepo.Interface + S string } mock.lockInstall.RLock() calls = mock.calls.Install From db53df102c57ccea9f46bfa88cfc9456afa771e4 Mon Sep 17 00:00:00 2001 From: meiji163 Date: Wed, 2 Mar 2022 19:25:25 -0800 Subject: [PATCH 07/30] pinning script exts --- pkg/cmd/extension/command.go | 14 +++++++++++-- pkg/cmd/extension/http.go | 40 +++++++++++++++++++++++++++++++++++- pkg/cmd/extension/manager.go | 31 ++++++++++++++++++++++++---- 3 files changed, 78 insertions(+), 7 deletions(-) diff --git a/pkg/cmd/extension/command.go b/pkg/cmd/extension/command.go index 50aad7a40..ee0dbd512 100644 --- a/pkg/cmd/extension/command.go +++ b/pkg/cmd/extension/command.go @@ -116,18 +116,28 @@ func NewCmdExtension(f *cmdutil.Factory) *cobra.Command { return err } + cs := io.ColorScheme() if err := m.Install(repo, pinFlag); err != nil { + if errors.Is(err, releaseNotFoundErr) { + return fmt.Errorf("%s Could not find a release of %s for %s", + cs.FailureIcon(), args[0], cs.Cyan(pinFlag)) + } else if errors.Is(err, commitNotFoundErr) { + return fmt.Errorf("%s %s does not exist in %s", + cs.FailureIcon(), cs.Cyan(pinFlag), args[0]) + } return err } if io.IsStdoutTTY() { - cs := io.ColorScheme() fmt.Fprintf(io.Out, "%s Installed extension %s\n", cs.SuccessIcon(), args[0]) + if pinFlag != "" { + fmt.Fprintf(io.Out, "%s Pinned extension at %s\n", cs.SuccessIcon(), cs.Cyan(pinFlag)) + } } return nil }, } - cmd.Flags().StringVar(&pinFlag, "pin", "", "pin extension to a release tag or commit sha") + cmd.Flags().StringVar(&pinFlag, "pin", "", "pin extension to a release tag or commit ref") return cmd }(), func() *cobra.Command { diff --git a/pkg/cmd/extension/http.go b/pkg/cmd/extension/http.go index d1bbe84e7..38ba9c5e5 100644 --- a/pkg/cmd/extension/http.go +++ b/pkg/cmd/extension/http.go @@ -2,6 +2,7 @@ package extension import ( "encoding/json" + "errors" "fmt" "io" "io/ioutil" @@ -80,6 +81,9 @@ func downloadAsset(httpClient *http.Client, asset releaseAsset, destPath string) return err } +var releaseNotFoundErr = errors.New("release not found") +var commitNotFoundErr = errors.New("commit not found") + // fetchLatestRelease finds the latest published release for a repository. func fetchLatestRelease(httpClient *http.Client, baseRepo ghrepo.Interface) (*release, error) { path := fmt.Sprintf("repos/%s/%s/releases/latest", baseRepo.RepoOwner(), baseRepo.RepoName()) @@ -113,7 +117,7 @@ func fetchLatestRelease(httpClient *http.Client, baseRepo ghrepo.Interface) (*re return &r, nil } -// fetchRelease finds release by tag name for a repository +// fetchReleaseFromTag finds release by tag name for a repository func fetchReleaseFromTag(httpClient *http.Client, baseRepo ghrepo.Interface, tagName string) (*release, error) { fullRepoName := fmt.Sprintf("%s/%s", baseRepo.RepoOwner(), baseRepo.RepoName()) path := fmt.Sprintf("repos/%s/releases/tags/%s", fullRepoName, tagName) @@ -129,6 +133,9 @@ func fetchReleaseFromTag(httpClient *http.Client, baseRepo ghrepo.Interface, tag return nil, err } + if resp.StatusCode == 404 { + return nil, releaseNotFoundErr + } if resp.StatusCode > 299 { return nil, api.HandleHTTPError(resp) } @@ -146,3 +153,34 @@ func fetchReleaseFromTag(httpClient *http.Client, baseRepo ghrepo.Interface, tag return &r, nil } + +// fetchCommitSHA finds full commit SHA from a target ref in a repo +func fetchCommitSHA(httpClient *http.Client, baseRepo ghrepo.Interface, targetRef string) (string, error) { + path := fmt.Sprintf("repos/%s/%s/commits/%s", baseRepo.RepoOwner(), baseRepo.RepoName(), targetRef) + url := ghinstance.RESTPrefix(baseRepo.RepoHost()) + path + req, err := http.NewRequest("GET", url, nil) + if err != nil { + return "", err + } + + req.Header.Set("Accept", "application/vnd.github.VERSION.sha") + resp, err := httpClient.Do(req) + defer resp.Body.Close() + if err != nil { + return "", err + } + + if resp.StatusCode == 422 { + return "", commitNotFoundErr + } + if resp.StatusCode > 299 { + return "", api.HandleHTTPError(resp) + } + + body, err := ioutil.ReadAll(resp.Body) + if err != nil { + return "", err + } + + return string(body), nil +} diff --git a/pkg/cmd/extension/manager.go b/pkg/cmd/extension/manager.go index 41dab528d..aa45ee48c 100644 --- a/pkg/cmd/extension/manager.go +++ b/pkg/cmd/extension/manager.go @@ -8,6 +8,7 @@ import ( "io" "io/fs" "io/ioutil" + "log" "net/http" "os" "os/exec" @@ -320,6 +321,7 @@ type binManifest struct { Path string } +// Install an extension from repo, and pin to commitish if provided func (m *Manager) Install(repo ghrepo.Interface, targetCommitish string) error { isBin, err := isBinExtension(m.client, repo) if err != nil { @@ -337,11 +339,11 @@ func (m *Manager) Install(repo ghrepo.Interface, targetCommitish string) error { return errors.New("extension is not installable: missing executable") } - protocol, _ := m.config.GetOrDefault(repo.RepoHost(), "git_protocol") - return m.installGit(ghrepo.FormatRemoteURL(repo, protocol), m.io.Out, m.io.ErrOut) + return m.installGit(repo, targetCommitish, m.io.Out, m.io.ErrOut) } func (m *Manager) installBin(repo ghrepo.Interface, targetCommitish string) error { + log.Println("Installing binary extension") var r *release var err error if targetCommitish == "" { @@ -413,19 +415,40 @@ func (m *Manager) installBin(repo ghrepo.Interface, targetCommitish string) erro return nil } -func (m *Manager) installGit(cloneURL string, stdout, stderr io.Writer) error { +func (m *Manager) installGit(repo ghrepo.Interface, targetCommitish string, stdout, stderr io.Writer) error { + protocol, _ := m.config.GetOrDefault(repo.RepoHost(), "git_protocol") + cloneURL := ghrepo.FormatRemoteURL(repo, protocol) + exe, err := m.lookPath("git") if err != nil { return err } + var commitSHA string + if targetCommitish != "" { + commitSHA, err = fetchCommitSHA(m.client, repo, targetCommitish) + if err != nil { + return err + } + } + name := strings.TrimSuffix(path.Base(cloneURL), ".git") targetDir := filepath.Join(m.installDir(), name) externalCmd := m.newCommand(exe, "clone", cloneURL, targetDir) externalCmd.Stdout = stdout externalCmd.Stderr = stderr - return externalCmd.Run() + if err := externalCmd.Run(); err != nil { + return err + } + if commitSHA == "" { + return nil + } + + checkoutCmd := m.newCommand(exe, "-C", targetDir, "checkout", commitSHA) + checkoutCmd.Stdout = stdout + checkoutCmd.Stderr = stderr + return checkoutCmd.Run() } var localExtensionUpgradeError = errors.New("local extensions can not be upgraded") From bed630452bb2c29fbef3134479070f2e6d724b7b Mon Sep 17 00:00:00 2001 From: meiji163 Date: Wed, 2 Mar 2022 21:06:23 -0800 Subject: [PATCH 08/30] list pinned exts --- pkg/cmd/extension/command.go | 8 ++++- pkg/cmd/extension/extension.go | 8 ++++- pkg/cmd/extension/manager.go | 57 +++++++++++++++++++++++--------- pkg/extensions/extension.go | 3 +- pkg/extensions/extension_mock.go | 36 ++++++++++++++++++++ 5 files changed, 93 insertions(+), 19 deletions(-) diff --git a/pkg/cmd/extension/command.go b/pkg/cmd/extension/command.go index ee0dbd512..ec756edfa 100644 --- a/pkg/cmd/extension/command.go +++ b/pkg/cmd/extension/command.go @@ -65,7 +65,13 @@ func NewCmdExtension(f *cmdutil.Factory) *cobra.Command { if !c.IsBinary() && len(version) > 8 { version = version[:8] } - t.AddField(version, nil, nil) + + if c.IsPinned() { + t.AddField(version, nil, cs.Cyan) + } else { + t.AddField(version, nil, nil) + } + var updateAvailable string if c.UpdateAvailable() { updateAvailable = "Upgrade available" diff --git a/pkg/cmd/extension/extension.go b/pkg/cmd/extension/extension.go index d1b814c57..e4f109d83 100644 --- a/pkg/cmd/extension/extension.go +++ b/pkg/cmd/extension/extension.go @@ -18,6 +18,7 @@ type Extension struct { path string url string isLocal bool + isPinned bool currentVersion string latestVersion string kind ExtensionKind @@ -43,8 +44,13 @@ func (e *Extension) CurrentVersion() string { return e.currentVersion } +func (e *Extension) IsPinned() bool { + return e.isPinned +} + func (e *Extension) UpdateAvailable() bool { - if e.isLocal || + if e.isPinned || + e.isLocal || e.currentVersion == "" || e.latestVersion == "" || e.currentVersion == e.latestVersion { diff --git a/pkg/cmd/extension/manager.go b/pkg/cmd/extension/manager.go index aa45ee48c..031e6d3a7 100644 --- a/pkg/cmd/extension/manager.go +++ b/pkg/cmd/extension/manager.go @@ -8,7 +8,6 @@ import ( "io" "io/fs" "io/ioutil" - "log" "net/http" "os" "os/exec" @@ -199,6 +198,7 @@ func (m *Manager) parseBinaryExtensionDir(fi fs.FileInfo) (Extension, error) { remoteURL := ghrepo.GenerateRepoURL(repo, "") ext.url = remoteURL ext.currentVersion = bm.Tag + ext.isPinned = bm.IsPinned return ext, nil } @@ -207,12 +207,20 @@ func (m *Manager) parseGitExtensionDir(fi fs.FileInfo) (Extension, error) { exePath := filepath.Join(id, fi.Name(), fi.Name()) remoteUrl := m.getRemoteUrl(fi.Name()) currentVersion := m.getCurrentVersion(fi.Name()) + + var isPinned bool + pinPath := filepath.Join(id, fi.Name(), fmt.Sprintf(".pin-%s", currentVersion)) + if _, err := os.Stat(pinPath); err == nil { + isPinned = true + } + return Extension{ path: exePath, url: remoteUrl, isLocal: false, currentVersion: currentVersion, kind: GitKind, + isPinned: isPinned, }, nil } @@ -313,15 +321,16 @@ func (m *Manager) InstallLocal(dir string) error { } type binManifest struct { - Owner string - Name string - Host string - Tag string + Owner string + Name string + Host string + Tag string + IsPinned bool // TODO I may end up not using this; just thinking ahead to local installs Path string } -// Install an extension from repo, and pin to commitish if provided +// Install installs an extension from repo, and pins to commitish if provided func (m *Manager) Install(repo ghrepo.Interface, targetCommitish string) error { isBin, err := isBinExtension(m.client, repo) if err != nil { @@ -343,13 +352,13 @@ func (m *Manager) Install(repo ghrepo.Interface, targetCommitish string) error { } func (m *Manager) installBin(repo ghrepo.Interface, targetCommitish string) error { - log.Println("Installing binary extension") var r *release var err error - if targetCommitish == "" { - r, err = fetchLatestRelease(m.client, repo) - } else { + isPinned := targetCommitish != "" + if isPinned { r, err = fetchReleaseFromTag(m.client, repo, targetCommitish) + } else { + r, err = fetchLatestRelease(m.client, repo) } if err != nil { return err @@ -372,6 +381,7 @@ func (m *Manager) installBin(repo ghrepo.Interface, targetCommitish string) erro name := repo.RepoName() targetDir := filepath.Join(m.installDir(), name) + // TODO clean this up if function errs? err = os.MkdirAll(targetDir, 0755) if err != nil { @@ -387,11 +397,12 @@ func (m *Manager) installBin(repo ghrepo.Interface, targetCommitish string) erro } manifest := binManifest{ - Name: name, - Owner: repo.RepoOwner(), - Host: repo.RepoHost(), - Path: binPath, - Tag: r.Tag, + Name: name, + Owner: repo.RepoOwner(), + Host: repo.RepoHost(), + Path: binPath, + Tag: r.Tag, + IsPinned: isPinned, } bs, err := yaml.Marshal(manifest) @@ -448,9 +459,20 @@ func (m *Manager) installGit(repo ghrepo.Interface, targetCommitish string, stdo checkoutCmd := m.newCommand(exe, "-C", targetDir, "checkout", commitSHA) checkoutCmd.Stdout = stdout checkoutCmd.Stderr = stderr - return checkoutCmd.Run() + if err := checkoutCmd.Run(); err != nil { + return err + } + + pinPath := filepath.Join(targetDir, fmt.Sprintf(".pin-%s", commitSHA)) + f, err := os.OpenFile(pinPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0600) + defer f.Close() + if err != nil { + return fmt.Errorf("failed to create pin file in directory: %w", err) + } + return nil } +var pinnedExtensionUpgradeError = errors.New("pinned extensions can not be upgraded") var localExtensionUpgradeError = errors.New("local extensions can not be upgraded") var upToDateError = errors.New("already up to date") var noExtensionsInstalledError = errors.New("no extensions installed") @@ -508,6 +530,9 @@ func (m *Manager) upgradeExtension(ext Extension, force bool) error { if ext.isLocal { return localExtensionUpgradeError } + if ext.IsPinned() { + return pinnedExtensionUpgradeError + } if !ext.UpdateAvailable() { return upToDateError } diff --git a/pkg/extensions/extension.go b/pkg/extensions/extension.go index fa1959ab7..aa1019a70 100644 --- a/pkg/extensions/extension.go +++ b/pkg/extensions/extension.go @@ -19,10 +19,11 @@ type Extension interface { Name() string // Extension Name without gh- Path() string // Path to executable URL() string - IsLocal() bool CurrentVersion() string + IsPinned() bool UpdateAvailable() bool IsBinary() bool + IsLocal() bool } //go:generate moq -rm -out manager_mock.go . ExtensionManager diff --git a/pkg/extensions/extension_mock.go b/pkg/extensions/extension_mock.go index 5b418c3f1..e68c376f2 100644 --- a/pkg/extensions/extension_mock.go +++ b/pkg/extensions/extension_mock.go @@ -26,6 +26,9 @@ var _ Extension = &ExtensionMock{} // IsLocalFunc: func() bool { // panic("mock out the IsLocal method") // }, +// IsPinnedFunc: func() bool { +// panic("mock out the IsPinned method") +// }, // NameFunc: func() string { // panic("mock out the Name method") // }, @@ -54,6 +57,9 @@ type ExtensionMock struct { // IsLocalFunc mocks the IsLocal method. IsLocalFunc func() bool + // IsPinnedFunc mocks the IsPinned method. + IsPinnedFunc func() bool + // NameFunc mocks the Name method. NameFunc func() string @@ -77,6 +83,9 @@ type ExtensionMock struct { // IsLocal holds details about calls to the IsLocal method. IsLocal []struct { } + // IsPinned holds details about calls to the IsPinned method. + IsPinned []struct { + } // Name holds details about calls to the Name method. Name []struct { } @@ -93,6 +102,7 @@ type ExtensionMock struct { lockCurrentVersion sync.RWMutex lockIsBinary sync.RWMutex lockIsLocal sync.RWMutex + lockIsPinned sync.RWMutex lockName sync.RWMutex lockPath sync.RWMutex lockURL sync.RWMutex @@ -177,6 +187,32 @@ func (mock *ExtensionMock) IsLocalCalls() []struct { return calls } +// IsPinned calls IsPinnedFunc. +func (mock *ExtensionMock) IsPinned() bool { + if mock.IsPinnedFunc == nil { + panic("ExtensionMock.IsPinnedFunc: method is nil but Extension.IsPinned was just called") + } + callInfo := struct { + }{} + mock.lockIsPinned.Lock() + mock.calls.IsPinned = append(mock.calls.IsPinned, callInfo) + mock.lockIsPinned.Unlock() + return mock.IsPinnedFunc() +} + +// IsPinnedCalls gets all the calls that were made to IsPinned. +// Check the length with: +// len(mockedExtension.IsPinnedCalls()) +func (mock *ExtensionMock) IsPinnedCalls() []struct { +} { + var calls []struct { + } + mock.lockIsPinned.RLock() + calls = mock.calls.IsPinned + mock.lockIsPinned.RUnlock() + return calls +} + // Name calls NameFunc. func (mock *ExtensionMock) Name() string { if mock.NameFunc == nil { From 619774d460e192774b806189890ead7f890e0626 Mon Sep 17 00:00:00 2001 From: meiji163 Date: Thu, 3 Mar 2022 22:10:35 -0800 Subject: [PATCH 09/30] fix tests --- pkg/cmd/extension/command_test.go | 2 +- pkg/cmd/extension/http.go | 4 +- pkg/cmd/extension/manager.go | 27 ++++++------ pkg/cmd/extension/manager_test.go | 68 +++++++++++++++++++++++++++++-- 4 files changed, 82 insertions(+), 19 deletions(-) diff --git a/pkg/cmd/extension/command_test.go b/pkg/cmd/extension/command_test.go index 7e5520fa4..541b1b137 100644 --- a/pkg/cmd/extension/command_test.go +++ b/pkg/cmd/extension/command_test.go @@ -44,7 +44,7 @@ func TestNewCmdExtension(t *testing.T) { em.ListFunc = func(bool) []extensions.Extension { return []extensions.Extension{} } - em.InstallFunc = func(_ ghrepo.Interface) error { + em.InstallFunc = func(_ ghrepo.Interface, _ string) error { return nil } return func(t *testing.T) { diff --git a/pkg/cmd/extension/http.go b/pkg/cmd/extension/http.go index 38ba9c5e5..7208b1569 100644 --- a/pkg/cmd/extension/http.go +++ b/pkg/cmd/extension/http.go @@ -128,11 +128,11 @@ func fetchReleaseFromTag(httpClient *http.Client, baseRepo ghrepo.Interface, tag } resp, err := httpClient.Do(req) - defer resp.Body.Close() if err != nil { return nil, err } + defer resp.Body.Close() if resp.StatusCode == 404 { return nil, releaseNotFoundErr } @@ -165,11 +165,11 @@ func fetchCommitSHA(httpClient *http.Client, baseRepo ghrepo.Interface, targetRe req.Header.Set("Accept", "application/vnd.github.VERSION.sha") resp, err := httpClient.Do(req) - defer resp.Body.Close() if err != nil { return "", err } + defer resp.Body.Close() if resp.StatusCode == 422 { return "", commitNotFoundErr } diff --git a/pkg/cmd/extension/manager.go b/pkg/cmd/extension/manager.go index 031e6d3a7..156212909 100644 --- a/pkg/cmd/extension/manager.go +++ b/pkg/cmd/extension/manager.go @@ -233,6 +233,7 @@ func (m *Manager) getCurrentVersion(extension string) string { dir := m.installDir() gitDir := "--git-dir=" + filepath.Join(dir, extension, ".git") cmd := m.newCommand(gitExe, gitDir, "rev-parse", "HEAD") + localSha, err := cmd.Output() if err != nil { return "" @@ -331,13 +332,13 @@ type binManifest struct { } // Install installs an extension from repo, and pins to commitish if provided -func (m *Manager) Install(repo ghrepo.Interface, targetCommitish string) error { +func (m *Manager) Install(repo ghrepo.Interface, target string) error { isBin, err := isBinExtension(m.client, repo) if err != nil { return fmt.Errorf("could not check for binary extension: %w", err) } if isBin { - return m.installBin(repo, targetCommitish) + return m.installBin(repo, target) } hs, err := hasScript(m.client, repo) @@ -348,15 +349,15 @@ func (m *Manager) Install(repo ghrepo.Interface, targetCommitish string) error { return errors.New("extension is not installable: missing executable") } - return m.installGit(repo, targetCommitish, m.io.Out, m.io.ErrOut) + return m.installGit(repo, target, m.io.Out, m.io.ErrOut) } -func (m *Manager) installBin(repo ghrepo.Interface, targetCommitish string) error { +func (m *Manager) installBin(repo ghrepo.Interface, target string) error { var r *release var err error - isPinned := targetCommitish != "" + isPinned := target != "" if isPinned { - r, err = fetchReleaseFromTag(m.client, repo, targetCommitish) + r, err = fetchReleaseFromTag(m.client, repo, target) } else { r, err = fetchLatestRelease(m.client, repo) } @@ -426,7 +427,7 @@ func (m *Manager) installBin(repo ghrepo.Interface, targetCommitish string) erro return nil } -func (m *Manager) installGit(repo ghrepo.Interface, targetCommitish string, stdout, stderr io.Writer) error { +func (m *Manager) installGit(repo ghrepo.Interface, target string, stdout, stderr io.Writer) error { protocol, _ := m.config.GetOrDefault(repo.RepoHost(), "git_protocol") cloneURL := ghrepo.FormatRemoteURL(repo, protocol) @@ -436,8 +437,8 @@ func (m *Manager) installGit(repo ghrepo.Interface, targetCommitish string, stdo } var commitSHA string - if targetCommitish != "" { - commitSHA, err = fetchCommitSHA(m.client, repo, targetCommitish) + if target != "" { + commitSHA, err = fetchCommitSHA(m.client, repo, target) if err != nil { return err } @@ -464,12 +465,11 @@ func (m *Manager) installGit(repo ghrepo.Interface, targetCommitish string, stdo } pinPath := filepath.Join(targetDir, fmt.Sprintf(".pin-%s", commitSHA)) - f, err := os.OpenFile(pinPath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0600) - defer f.Close() + f, err := os.OpenFile(pinPath, os.O_WRONLY|os.O_CREATE, 0600) if err != nil { return fmt.Errorf("failed to create pin file in directory: %w", err) } - return nil + return f.Close() } var pinnedExtensionUpgradeError = errors.New("pinned extensions can not be upgraded") @@ -512,7 +512,8 @@ func (m *Manager) upgradeExtensions(exts []Extension, force bool) error { err := m.upgradeExtension(f, force) if err != nil { if !errors.Is(err, localExtensionUpgradeError) && - !errors.Is(err, upToDateError) { + !errors.Is(err, upToDateError) && + !errors.Is(err, pinnedExtensionUpgradeError) { failed = true } fmt.Fprintf(m.io.Out, "%s\n", err) diff --git a/pkg/cmd/extension/manager_test.go b/pkg/cmd/extension/manager_test.go index 47f11d348..e1fc45f49 100644 --- a/pkg/cmd/extension/manager_test.go +++ b/pkg/cmd/extension/manager_test.go @@ -445,6 +445,50 @@ func TestManager_UpgradeExtension_BinaryExtension(t *testing.T) { assert.Equal(t, "", stderr.String()) } +func TestManager_UpgradeExtension_PinnedBinaryExtension(t *testing.T) { + tempDir := t.TempDir() + + assert.NoError(t, stubBinaryExtension( + filepath.Join(tempDir, "extensions", "gh-bin-ext"), + binManifest{ + Owner: "owner", + Name: "gh-bin-ext", + Host: "example.com", + Tag: "v1.6.3", + Pinned: true, + })) + + io, _, _, _ := iostreams.Test() + m := newTestManager(tempDir, nil, io) + exts, err := m.list(false) + assert.Nil(t, err) + assert.Equal(t, 1, len(exts)) + ext := exts[0] + + err = m.upgradeExtension(ext, false) + assert.NotNil(t, err) + assert.Equal(t, err, pinnedExtensionUpgradeError) +} + +func TestManager_UpgradeExtenion_PinnedGitExtension(t *testing.T) { + tempDir := t.TempDir() + extDir := filepath.Join(tempDir, "extensions", "gh-remote") + assert.NoError(t, stubPinnedExtension(filepath.Join(extDir, "gh-remote"), "abc1234")) + + io, _, _, _ := iostreams.Test() + m := newTestManager(tempDir, nil, io) + exts, err := m.list(false) + assert.NoError(t, err) + assert.Equal(t, 1, len(exts)) + ext := exts[0] + ext.latestVersion = "new version" + ext.pin = "abc1234" + + err = m.upgradeExtension(ext, false) + assert.NotNil(t, err) + assert.Equal(t, err, pinnedExtensionUpgradeError) +} + func TestManager_Install_git(t *testing.T) { tempDir := t.TempDir() @@ -472,7 +516,7 @@ func TestManager_Install_git(t *testing.T) { repo := ghrepo.New("owner", "gh-some-ext") - err := m.Install(repo) + err := m.Install(repo, "") 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()) @@ -514,7 +558,7 @@ func TestManager_Install_binary_unsupported(t *testing.T) { m := newTestManager(tempDir, &client, io) - err := m.Install(repo) + err := m.Install(repo, "") assert.EqualError(t, err, "gh-bin-ext unsupported for windows-amd64. Open an issue: `gh issue create -R owner/gh-bin-ext -t'Support windows-amd64'`") assert.Equal(t, "", stdout.String()) @@ -559,7 +603,7 @@ func TestManager_Install_binary(t *testing.T) { m := newTestManager(tempDir, &http.Client{Transport: ®}, io) - err := m.Install(repo) + err := m.Install(repo, "") assert.NoError(t, err) manifest, err := os.ReadFile(filepath.Join(tempDir, "extensions/gh-bin-ext", manifestName)) @@ -702,6 +746,24 @@ func stubExtension(path string) error { return f.Close() } +func stubPinnedExtension(path string, pinnedVersion 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 + } + f.Close() + + pinPath := filepath.Join(filepath.Dir(path), fmt.Sprintf(".pin-%s", pinnedVersion)) + f, err = os.OpenFile(pinPath, os.O_WRONLY|os.O_CREATE, 0600) + if err != nil { + return err + } + return f.Close() +} + func stubLocalExtension(tempDir, path string) error { extDir, err := ioutil.TempDir(tempDir, "local-ext") if err != nil { From 65bc3acaa54c0b02fae7aab28cc6c650f886ad27 Mon Sep 17 00:00:00 2001 From: meiji163 Date: Mon, 7 Mar 2022 17:58:48 -0800 Subject: [PATCH 10/30] merge upstream --- pkg/cmd/extension/command.go | 1 + pkg/cmd/extension/manager.go | 42 ------------------------------------ 2 files changed, 1 insertion(+), 42 deletions(-) diff --git a/pkg/cmd/extension/command.go b/pkg/cmd/extension/command.go index 66370305c..ec756edfa 100644 --- a/pkg/cmd/extension/command.go +++ b/pkg/cmd/extension/command.go @@ -76,6 +76,7 @@ func NewCmdExtension(f *cmdutil.Factory) *cobra.Command { if c.UpdateAvailable() { updateAvailable = "Upgrade available" } + t.AddField(updateAvailable, nil, cs.Green) t.EndRow() } return t.Render() diff --git a/pkg/cmd/extension/manager.go b/pkg/cmd/extension/manager.go index 9452c2b53..156212909 100644 --- a/pkg/cmd/extension/manager.go +++ b/pkg/cmd/extension/manager.go @@ -198,13 +198,7 @@ func (m *Manager) parseBinaryExtensionDir(fi fs.FileInfo) (Extension, error) { remoteURL := ghrepo.GenerateRepoURL(repo, "") ext.url = remoteURL ext.currentVersion = bm.Tag -<<<<<<< HEAD - if bm.Pinned { - ext.pin = bm.Tag - } -======= ext.isPinned = bm.IsPinned ->>>>>>> tmp return ext, nil } @@ -214,17 +208,10 @@ func (m *Manager) parseGitExtensionDir(fi fs.FileInfo) (Extension, error) { remoteUrl := m.getRemoteUrl(fi.Name()) currentVersion := m.getCurrentVersion(fi.Name()) -<<<<<<< HEAD - var pinnedVersion string - pinPath := filepath.Join(id, fi.Name(), fmt.Sprintf(".pin-%s", currentVersion)) - if _, err := os.Stat(pinPath); err == nil && len(currentVersion) > 6 { - pinnedVersion = currentVersion[:7] -======= var isPinned bool pinPath := filepath.Join(id, fi.Name(), fmt.Sprintf(".pin-%s", currentVersion)) if _, err := os.Stat(pinPath); err == nil { isPinned = true ->>>>>>> tmp } return Extension{ @@ -233,11 +220,7 @@ func (m *Manager) parseGitExtensionDir(fi fs.FileInfo) (Extension, error) { isLocal: false, currentVersion: currentVersion, kind: GitKind, -<<<<<<< HEAD - pin: pinnedVersion, -======= isPinned: isPinned, ->>>>>>> tmp }, nil } @@ -339,19 +322,11 @@ func (m *Manager) InstallLocal(dir string) error { } type binManifest struct { -<<<<<<< HEAD - Owner string - Name string - Host string - Tag string - Pinned bool -======= Owner string Name string Host string Tag string IsPinned bool ->>>>>>> tmp // TODO I may end up not using this; just thinking ahead to local installs Path string } @@ -423,21 +398,12 @@ func (m *Manager) installBin(repo ghrepo.Interface, target string) error { } manifest := binManifest{ -<<<<<<< HEAD - Name: name, - Owner: repo.RepoOwner(), - Host: repo.RepoHost(), - Path: binPath, - Tag: r.Tag, - Pinned: isPinned, -======= Name: name, Owner: repo.RepoOwner(), Host: repo.RepoHost(), Path: binPath, Tag: r.Tag, IsPinned: isPinned, ->>>>>>> tmp } bs, err := yaml.Marshal(manifest) @@ -501,11 +467,7 @@ func (m *Manager) installGit(repo ghrepo.Interface, target string, stdout, stder pinPath := filepath.Join(targetDir, fmt.Sprintf(".pin-%s", commitSHA)) f, err := os.OpenFile(pinPath, os.O_WRONLY|os.O_CREATE, 0600) if err != nil { -<<<<<<< HEAD - return fmt.Errorf("failed to create pin in directory: %w", err) -======= return fmt.Errorf("failed to create pin file in directory: %w", err) ->>>>>>> tmp } return f.Close() } @@ -569,11 +531,7 @@ func (m *Manager) upgradeExtension(ext Extension, force bool) error { if ext.isLocal { return localExtensionUpgradeError } -<<<<<<< HEAD - if ext.pin != "" { -======= if ext.IsPinned() { ->>>>>>> tmp return pinnedExtensionUpgradeError } if !ext.UpdateAvailable() { From 8174640cda58b0f8858e0df2f55c1c2c59c87dfd Mon Sep 17 00:00:00 2001 From: meiji163 Date: Mon, 7 Mar 2022 19:47:21 -0800 Subject: [PATCH 11/30] install tests --- pkg/cmd/extension/manager_test.go | 119 +++++++++++++++++++++++++++--- 1 file changed, 110 insertions(+), 9 deletions(-) diff --git a/pkg/cmd/extension/manager_test.go b/pkg/cmd/extension/manager_test.go index e1fc45f49..717a0b906 100644 --- a/pkg/cmd/extension/manager_test.go +++ b/pkg/cmd/extension/manager_test.go @@ -445,17 +445,17 @@ func TestManager_UpgradeExtension_BinaryExtension(t *testing.T) { assert.Equal(t, "", stderr.String()) } -func TestManager_UpgradeExtension_PinnedBinaryExtension(t *testing.T) { +func TestManager_UpgradeExtension_BinaryExtension_Pinned(t *testing.T) { tempDir := t.TempDir() assert.NoError(t, stubBinaryExtension( filepath.Join(tempDir, "extensions", "gh-bin-ext"), binManifest{ - Owner: "owner", - Name: "gh-bin-ext", - Host: "example.com", - Tag: "v1.6.3", - Pinned: true, + Owner: "owner", + Name: "gh-bin-ext", + Host: "example.com", + Tag: "v1.6.3", + IsPinned: true, })) io, _, _, _ := iostreams.Test() @@ -470,10 +470,10 @@ func TestManager_UpgradeExtension_PinnedBinaryExtension(t *testing.T) { assert.Equal(t, err, pinnedExtensionUpgradeError) } -func TestManager_UpgradeExtenion_PinnedGitExtension(t *testing.T) { +func TestManager_UpgradeExtenion_GitExtension_Pinned(t *testing.T) { tempDir := t.TempDir() extDir := filepath.Join(tempDir, "extensions", "gh-remote") - assert.NoError(t, stubPinnedExtension(filepath.Join(extDir, "gh-remote"), "abc1234")) + assert.NoError(t, stubPinnedExtension(filepath.Join(extDir, "gh-remote"), "abcd1234")) io, _, _, _ := iostreams.Test() m := newTestManager(tempDir, nil, io) @@ -481,8 +481,8 @@ func TestManager_UpgradeExtenion_PinnedGitExtension(t *testing.T) { assert.NoError(t, err) assert.Equal(t, 1, len(exts)) ext := exts[0] + ext.isPinned = true ext.latestVersion = "new version" - ext.pin = "abc1234" err = m.upgradeExtension(ext, false) assert.NotNil(t, err) @@ -522,6 +522,107 @@ func TestManager_Install_git(t *testing.T) { assert.Equal(t, "", stderr.String()) } +func TestManager_Install_git_pinned(t *testing.T) { + tempDir := t.TempDir() + + reg := httpmock.Registry{} + defer reg.Verify(t) + client := http.Client{Transport: ®} + + io, _, _, stderr := iostreams.Test() + m := newTestManager(tempDir, &client, io) + + reg.Register( + httpmock.REST("GET", "repos/owner/gh-cool-ext/releases/latest"), + httpmock.JSONResponse( + release{ + Assets: []releaseAsset{ + { + Name: "not-a-binary", + APIURL: "https://example.com/release/cool", + }, + }, + })) + reg.Register( + httpmock.REST("GET", "repos/owner/gh-cool-ext/commits/some-ref"), + httpmock.StringResponse("abcd1234")) + reg.Register( + httpmock.REST("GET", "repos/owner/gh-cool-ext/contents/gh-cool-ext"), + httpmock.StringResponse("script")) + + _ = os.MkdirAll(filepath.Join(m.installDir(), "gh-cool-ext"), 0700) + repo := ghrepo.New("owner", "gh-cool-ext") + err := m.Install(repo, "some-ref") + assert.NoError(t, err) + assert.Equal(t, "", stderr.String()) +} + +func TestManager_Install_binary_pinned(t *testing.T) { + repo := ghrepo.NewWithHost("owner", "gh-bin-ext", "example.com") + + reg := httpmock.Registry{} + defer reg.Verify(t) + + reg.Register( + httpmock.REST("GET", "api/v3/repos/owner/gh-bin-ext/releases/latest"), + httpmock.JSONResponse( + release{ + Assets: []releaseAsset{ + { + Name: "gh-bin-ext-windows-amd64.exe", + APIURL: "https://example.com/release/cool", + }, + }, + })) + reg.Register( + httpmock.REST("GET", "api/v3/repos/owner/gh-bin-ext/releases/tags/v1.6.3-pre"), + httpmock.JSONResponse( + release{ + Tag: "v1.6.3-pre", + Assets: []releaseAsset{ + { + Name: "gh-bin-ext-windows-amd64.exe", + APIURL: "https://example.com/release/cool", + }, + }, + })) + reg.Register( + httpmock.REST("GET", "release/cool"), + httpmock.StringResponse("FAKE BINARY")) + + io, _, stdout, stderr := iostreams.Test() + tempDir := t.TempDir() + + m := newTestManager(tempDir, &http.Client{Transport: ®}, io) + + err := m.Install(repo, "v1.6.3-pre") + assert.NoError(t, err) + + manifest, err := os.ReadFile(filepath.Join(tempDir, "extensions/gh-bin-ext", manifestName)) + assert.NoError(t, err) + + var bm binManifest + err = yaml.Unmarshal(manifest, &bm) + assert.NoError(t, err) + + assert.Equal(t, binManifest{ + Name: "gh-bin-ext", + Owner: "owner", + Host: "example.com", + Tag: "v1.6.3-pre", + IsPinned: true, + Path: filepath.Join(tempDir, "extensions/gh-bin-ext/gh-bin-ext.exe"), + }, bm) + + fakeBin, err := os.ReadFile(filepath.Join(tempDir, "extensions/gh-bin-ext/gh-bin-ext.exe")) + assert.NoError(t, err) + assert.Equal(t, "FAKE BINARY", string(fakeBin)) + + assert.Equal(t, "", stdout.String()) + assert.Equal(t, "", stderr.String()) + +} + func TestManager_Install_binary_unsupported(t *testing.T) { repo := ghrepo.NewWithHost("owner", "gh-bin-ext", "example.com") From 45845bc999c0bd17b9cb2b02e7c5c85c33c892d9 Mon Sep 17 00:00:00 2001 From: meiji163 Date: Tue, 22 Mar 2022 11:02:16 -0700 Subject: [PATCH 12/30] cant pin local --- pkg/cmd/extension/command.go | 3 +++ pkg/cmd/extension/command_test.go | 7 +++++++ 2 files changed, 10 insertions(+) diff --git a/pkg/cmd/extension/command.go b/pkg/cmd/extension/command.go index ec756edfa..33306bb56 100644 --- a/pkg/cmd/extension/command.go +++ b/pkg/cmd/extension/command.go @@ -106,6 +106,9 @@ func NewCmdExtension(f *cmdutil.Factory) *cobra.Command { Args: cmdutil.MinimumArgs(1, "must specify a repository to install from"), RunE: func(cmd *cobra.Command, args []string) error { if args[0] == "." { + if pinFlag != "" { + return fmt.Errorf("local extensions cannot be pinned") + } wd, err := os.Getwd() if err != nil { return err diff --git a/pkg/cmd/extension/command_test.go b/pkg/cmd/extension/command_test.go index 541b1b137..c84bf0664 100644 --- a/pkg/cmd/extension/command_test.go +++ b/pkg/cmd/extension/command_test.go @@ -86,6 +86,13 @@ func TestNewCmdExtension(t *testing.T) { } }, }, + { + name: "install local extension with pin", + args: []string{"install", ".", "--pin", "v1.0.0"}, + wantErr: true, + errMsg: "local extensions cannot be pinned", + isTTY: true, + }, { name: "upgrade argument error", args: []string{"upgrade"}, From 17cb6346a5c7fee0b14eebcd8174595c9e67d4d0 Mon Sep 17 00:00:00 2001 From: Jason Lunz Date: Wed, 23 Mar 2022 16:54:30 -0600 Subject: [PATCH 13/30] ssh.go: use setup example that should work with any ssh config The `gh cs ssh` command suggests an example recipe for setting up openssh integration. The last step appends an `Include` statement to the user's `~/.ssh/config`. Unfortunately, this won't always work as intended. If the existing configuration ends with a `Host` block, the added `Include` statement will be conditional on whether that block matches. By preceding the `Include` statement with `Match all`, we can ensure that it is always evaluated. --- pkg/cmd/codespace/ssh.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/codespace/ssh.go b/pkg/cmd/codespace/ssh.go index 69c9c9ee1..52694551f 100644 --- a/pkg/cmd/codespace/ssh.go +++ b/pkg/cmd/codespace/ssh.go @@ -59,7 +59,7 @@ func newSSHCmd(app *App) *cobra.Command { $ gh codespace ssh $ gh codespace ssh --config > ~/.ssh/codespaces - $ echo 'include ~/.ssh/codespaces' >> ~/.ssh/config + $ printf 'Match all\nInclude ~/.ssh/codespaces\n' >> ~/.ssh/config `), PreRunE: func(c *cobra.Command, args []string) error { if opts.stdio { From f7ef503051ff7a3845738c4846d73a49f3c62f76 Mon Sep 17 00:00:00 2001 From: Callum Macrae Date: Fri, 25 Mar 2022 15:07:47 +0000 Subject: [PATCH 14/30] fix: respect `GH_FORCE_TTY` when running `gh pr view` `GH_FORCE_TTY` only affects stdout, not stderr, so this check was failing and the flag was being ignored. I also checked for similar problem in other files but everything else seemed stderr or stdin related. closes #5354 --- pkg/cmd/pr/view/view.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/pr/view/view.go b/pkg/cmd/pr/view/view.go index 67124b195..019ba53b5 100644 --- a/pkg/cmd/pr/view/view.go +++ b/pkg/cmd/pr/view/view.go @@ -98,7 +98,7 @@ func viewRun(opts *ViewOptions) error { return err } - connectedToTerminal := opts.IO.IsStdoutTTY() && opts.IO.IsStderrTTY() + connectedToTerminal := opts.IO.IsStdoutTTY() if opts.BrowserMode { openURL := pr.URL From 66abe7c1152783bf992731db76cf96fa570d2853 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 28 Mar 2022 14:29:18 +0000 Subject: [PATCH 15/30] Bump github.com/creack/pty from 1.1.17 to 1.1.18 Bumps [github.com/creack/pty](https://github.com/creack/pty) from 1.1.17 to 1.1.18. - [Release notes](https://github.com/creack/pty/releases) - [Commits](https://github.com/creack/pty/compare/v1.1.17...v1.1.18) --- updated-dependencies: - dependency-name: github.com/creack/pty dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] --- go.mod | 2 +- go.sum | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/go.mod b/go.mod index e04ff252e..854645adb 100644 --- a/go.mod +++ b/go.mod @@ -12,7 +12,7 @@ require ( github.com/cli/safeexec v1.0.0 github.com/cli/shurcooL-graphql v0.0.1 github.com/cpuguy83/go-md2man/v2 v2.0.1 - github.com/creack/pty v1.1.17 + github.com/creack/pty v1.1.18 github.com/gabriel-vasile/mimetype v1.4.0 github.com/google/go-cmp v0.5.7 github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 diff --git a/go.sum b/go.sum index 3c76c15a1..7adac44e6 100644 --- a/go.sum +++ b/go.sum @@ -66,8 +66,9 @@ github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDk github.com/cncf/udpa/go v0.0.0-20191209042840-269d4d468f6f/go.mod h1:M8M6+tZqaGXZJjfX53e64911xZQV5JYwmTeXPW+k8Sc= github.com/cpuguy83/go-md2man/v2 v2.0.1 h1:r/myEWzV9lfsM1tFLgDyu0atFtJ1fXn261LKYj/3DxU= github.com/cpuguy83/go-md2man/v2 v2.0.1/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= -github.com/creack/pty v1.1.17 h1:QeVUsEDNrLBW4tMgZHvxy18sKtr6VI492kBhUfhDJNI= github.com/creack/pty v1.1.17/go.mod h1:MOBLtS5ELjhRRrroQr9kyvTxUAFNvYEK993ew/Vr4O4= +github.com/creack/pty v1.1.18 h1:n56/Zwd5o6whRC5PMGretI4IdRLlmBXYNjScPaBgsbY= +github.com/creack/pty v1.1.18/go.mod h1:MOBLtS5ELjhRRrroQr9kyvTxUAFNvYEK993ew/Vr4O4= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= From f22c325ea44062acb26c3e6d6c4faaed5a42372a Mon Sep 17 00:00:00 2001 From: nate smith Date: Mon, 28 Mar 2022 11:30:37 -0500 Subject: [PATCH 16/30] gh status --- go.mod | 3 +- go.sum | 7 +- pkg/cmd/root/root.go | 2 + pkg/cmd/status/fixtures/events.json | 188 +++++++ pkg/cmd/status/fixtures/notifications.json | 138 +++++ pkg/cmd/status/fixtures/search.json | 188 +++++++ pkg/cmd/status/status.go | 618 +++++++++++++++++++++ pkg/cmd/status/status_test.go | 332 +++++++++++ pkg/httpmock/stub.go | 5 +- utils/table_printer.go | 24 +- 10 files changed, 1492 insertions(+), 13 deletions(-) create mode 100644 pkg/cmd/status/fixtures/events.json create mode 100644 pkg/cmd/status/fixtures/notifications.json create mode 100644 pkg/cmd/status/fixtures/search.json create mode 100644 pkg/cmd/status/status.go create mode 100644 pkg/cmd/status/status_test.go diff --git a/go.mod b/go.mod index e04ff252e..aebcd1a5e 100644 --- a/go.mod +++ b/go.mod @@ -7,6 +7,7 @@ require ( github.com/MakeNowJust/heredoc v1.0.0 github.com/briandowns/spinner v1.18.1 github.com/charmbracelet/glamour v0.4.0 + github.com/charmbracelet/lipgloss v0.5.0 github.com/cli/browser v1.1.0 github.com/cli/oauth v0.9.0 github.com/cli/safeexec v1.0.0 @@ -27,7 +28,7 @@ require ( github.com/mattn/go-isatty v0.0.14 github.com/mgutz/ansi v0.0.0-20200706080929-d51e80ef957d github.com/muesli/reflow v0.3.0 - github.com/muesli/termenv v0.9.0 + github.com/muesli/termenv v0.11.1-0.20220204035834-5ac8409525e0 github.com/muhammadmuzzammil1998/jsonc v0.0.0-20201229145248-615b0916ca38 github.com/opentracing/opentracing-go v1.1.0 github.com/shurcooL/githubv4 v0.0.0-20200928013246-d292edc3691b diff --git a/go.sum b/go.sum index 3c76c15a1..b0e941da5 100644 --- a/go.sum +++ b/go.sum @@ -48,6 +48,8 @@ github.com/briandowns/spinner v1.18.1/go.mod h1:mQak9GHqbspjC/5iUx3qMlIho8xBS/pp github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU= github.com/charmbracelet/glamour v0.4.0 h1:scR+smyB7WdmrlIaff6IVlm48P48JaNM7JypM/VGl4k= github.com/charmbracelet/glamour v0.4.0/go.mod h1:9ZRtG19AUIzcTm7FGLGbq3D5WKQ5UyZBbQsMQN0XIqc= +github.com/charmbracelet/lipgloss v0.5.0 h1:lulQHuVeodSgDez+3rGiuxlPVXSnhth442DATR2/8t8= +github.com/charmbracelet/lipgloss v0.5.0/go.mod h1:EZLha/HbzEt7cYqdFPovlqy5FZPj0xFhg5SaqxScmgs= github.com/chzyer/logex v1.1.10/go.mod h1:+Ywpsq7O8HXn0nuIou7OrIPyXbp3wmkHB+jjWRnGsAI= github.com/chzyer/readline v0.0.0-20180603132655-2972be24d48e/go.mod h1:nSuG5e5PlCu98SY8svDHJxuZscDgtXS6KTTbou5AhLI= github.com/chzyer/test v0.0.0-20180213035817-a1ea475d72b1/go.mod h1:Q3SI9o4m/ZMnBNeIyt5eFwwo7qiLfzFZmjNmxjkiQlU= @@ -181,6 +183,7 @@ github.com/mattn/go-isatty v0.0.13/go.mod h1:cbi8OIDigv2wuxKPP5vlRcQ1OAZbq2CE4Ky github.com/mattn/go-isatty v0.0.14 h1:yVuAays6BHfxijgZPzw+3Zlu5yQgKGP2/hcQbHb7S9Y= github.com/mattn/go-isatty v0.0.14/go.mod h1:7GGIvUiUoEMVVmxf/4nioHXj79iQHKdU27kJ6hsGG94= github.com/mattn/go-runewidth v0.0.9/go.mod h1:H031xJmbD/WCDINGzjvQ9THkh0rPKHF+m2gUSrubnMI= +github.com/mattn/go-runewidth v0.0.10/go.mod h1:RAqKPSqVFrSLVXbA8x7dzmKdmGzieGRCM46jaSJTDAk= github.com/mattn/go-runewidth v0.0.12/go.mod h1:RAqKPSqVFrSLVXbA8x7dzmKdmGzieGRCM46jaSJTDAk= github.com/mattn/go-runewidth v0.0.13 h1:lTGmDsbAYt5DmK6OnoV7EuIF1wEIFAcxld6ypU4OSgU= github.com/mattn/go-runewidth v0.0.13/go.mod h1:Jdepj2loyihRzMpdS35Xk/zdY8IAYHsh153qUoGf23w= @@ -189,10 +192,12 @@ github.com/mgutz/ansi v0.0.0-20200706080929-d51e80ef957d h1:5PJl274Y63IEHC+7izoQ github.com/mgutz/ansi v0.0.0-20200706080929-d51e80ef957d/go.mod h1:01TrycV0kFyexm33Z7vhZRXopbI8J3TDReVlkTgMUxE= github.com/microcosm-cc/bluemonday v1.0.17 h1:Z1a//hgsQ4yjC+8zEkV8IWySkXnsxmdSY642CTFQb5Y= github.com/microcosm-cc/bluemonday v1.0.17/go.mod h1:Z0r70sCuXHig8YpBzCc5eGHAap2K7e/u082ZUpDRRqM= +github.com/muesli/reflow v0.2.1-0.20210115123740-9e1d0d53df68/go.mod h1:Xk+z4oIWdQqJzsxyjgl3P22oYZnHdZ8FFTHAQQt5BMQ= github.com/muesli/reflow v0.3.0 h1:IFsN6K9NfGtjeggFP+68I4chLZV2yIKsXJFNZ+eWh6s= github.com/muesli/reflow v0.3.0/go.mod h1:pbwTDkVPibjO2kyvBQRBxTWEEGDGq0FlB1BIKtnHY/8= -github.com/muesli/termenv v0.9.0 h1:wnbOaGz+LUR3jNT0zOzinPnyDaCZUQRZj9GxK8eRVl8= github.com/muesli/termenv v0.9.0/go.mod h1:R/LzAKf+suGs4IsO95y7+7DpFHO0KABgnZqtlyx2mBw= +github.com/muesli/termenv v0.11.1-0.20220204035834-5ac8409525e0 h1:STjmj0uFfRryL9fzRA/OupNppeAID6QJYPMavTL7jtY= +github.com/muesli/termenv v0.11.1-0.20220204035834-5ac8409525e0/go.mod h1:Bd5NYQ7pd+SrtBSrSNoBBmXlcY8+Xj4BMJgh8qcZrvs= github.com/muhammadmuzzammil1998/jsonc v0.0.0-20201229145248-615b0916ca38 h1:0FrBxrkJ0hVembTb/e4EU5Ml6vLcOusAqymmYISg5Uo= github.com/muhammadmuzzammil1998/jsonc v0.0.0-20201229145248-615b0916ca38/go.mod h1:saF2fIVw4banK0H4+/EuqfFLpRnoy5S+ECwTOCcRcSU= github.com/olekukonko/tablewriter v0.0.5 h1:P2Ga83D34wi1o9J6Wh1mRuqd4mF/x/lgBS7N7AbDhec= diff --git a/pkg/cmd/root/root.go b/pkg/cmd/root/root.go index f77d17b55..2fd80ae1b 100644 --- a/pkg/cmd/root/root.go +++ b/pkg/cmd/root/root.go @@ -28,6 +28,7 @@ import ( searchCmd "github.com/cli/cli/v2/pkg/cmd/search" secretCmd "github.com/cli/cli/v2/pkg/cmd/secret" sshKeyCmd "github.com/cli/cli/v2/pkg/cmd/ssh-key" + statusCmd "github.com/cli/cli/v2/pkg/cmd/status" versionCmd "github.com/cli/cli/v2/pkg/cmd/version" workflowCmd "github.com/cli/cli/v2/pkg/cmd/workflow" "github.com/cli/cli/v2/pkg/cmdutil" @@ -83,6 +84,7 @@ func NewCmdRoot(f *cmdutil.Factory, version, buildDate string) *cobra.Command { cmd.AddCommand(searchCmd.NewCmdSearch(f)) cmd.AddCommand(secretCmd.NewCmdSecret(f)) cmd.AddCommand(sshKeyCmd.NewCmdSSHKey(f)) + cmd.AddCommand(statusCmd.NewCmdStatus(f, nil)) cmd.AddCommand(newCodespaceCmd(f)) // the `api` command should not inherit any extra HTTP headers diff --git a/pkg/cmd/status/fixtures/events.json b/pkg/cmd/status/fixtures/events.json new file mode 100644 index 000000000..84631269f --- /dev/null +++ b/pkg/cmd/status/fixtures/events.json @@ -0,0 +1,188 @@ +[ + { + "type": "PullRequestEvent", + "repo": { + "name": "rpd/todo" + }, + "payload": { + "action": "opened", + "number": 5326, + "pull_request": { + "number": 5326, + "title": "Only write UTF-8 BOM on Windows where it is needed" + } + }, + "created_at": "2022-03-17T15:42:28Z", + "org": { + "login": "rpd" + } + }, + { + "type": "IssueCommentEvent", + "repo": { + "name": "vilmibm/testing" + }, + "payload": { + "action": "created", + "issue": { + "number": 5325, + "title": "Ability to search \"not\" thing" + }, + "comment": { + "html_url": "https://github.com/vilmibm/testing/issues/5325#issuecomment-1070868636", + "body": "We are working on dedicated `search` functionality and might be exploring some syntax for denoting \"NOT\" qualifiers. However, until something like this is added, you can always make \"NOT\" searches by using the `--search` flag:\r\n```\r\ngh pr list --search \"-author:app/dependabot\"\r\n```\r\n/cc @samcoe " + } + }, + "created_at": "2022-03-17T12:36:31Z" + }, + { + "type": "DeleteEvent", + "repo": { + "name": "cli/cli" + }, + "payload": { + "ref": "dependabot/go_modules/github.com/stretchr/testify-1.7.1", + "ref_type": "branch", + "pusher_type": "user" + }, + "created_at": "2022-03-16T14:41:30Z", + "org": { + "login": "cli", + "avatar_url": "https://avatars.githubusercontent.com/u/59704711?" + } + }, + { + "type": "CreateEvent", + "repo": { + "name": "cli/cli" + }, + "payload": { + "ref": "dependabot/go_modules/github.com/stretchr/testify-1.7.1", + "ref_type": "branch", + "master_branch": "trunk", + "description": "GitHub’s official command line tool", + "pusher_type": "user" + }, + "created_at": "2022-03-16T14:25:19Z", + "org": { + "login": "cli", + "avatar_url": "https://avatars.githubusercontent.com/u/59704711?" + } + }, + { + "type": "PullRequestReviewCommentEvent", + "repo": { + "name": "cli/cli" + }, + "payload": { + "action": "created", + "comment": { + "body": "Wondering if we shouldn't name this something more generic and use it everywhere else to DRY things up a bit?", + "html_url": "https://github.com/cli/cli/pull/5319#discussion_r827935007" + }, + "pull_request": { + "number": 5319, + "title": "[Codespaces] Disallow some operations on codespaces that have a pending operation" + } + }, + "created_at": "2022-03-16T12:08:02Z", + "org": { + "login": "cli" + } + }, + { + "type": "CommitCommentEvent", + "repo": { + "name": "cli/cli" + }, + "payload": { + "comment": { + "html_url": "https://github.com/cli/cli/commit/1b50852b2dfecd17ca5d3a2a12eb5c16df9fe46b#r68794505", + "body": "spam" + } + }, + "created_at": "2022-03-16T03:32:59Z", + "org": { + "login": "cli" + } + }, + { + "type": "PushEvent", + "repo": { + "name": "cli/cli" + }, + "payload": { + "push_id": 9359833428, + "size": 1, + "distinct_size": 1, + "ref": "refs/heads/jg/event-handling", + "head": "7d07249150fe1a24b2b49b3ff7c55e2152446a5e", + "before": "06f1f6eb527a34af1222ca03807a9205a17d6e90", + "commits": [ + { + "sha": "7d07249150fe1a24b2b49b3ff7c55e2152446a5e", + "author": { + "email": "68619889+bchuecos@users.noreply.github.com", + "name": "Bernardo" + }, + "message": "review suggestions", + "distinct": true, + "url": "https://api.github.com/repos/cli/cli/commits/7d07249150fe1a24b2b49b3ff7c55e2152446a5e" + } + ] + }, + "created_at": "2022-03-15T18:22:41Z", + "org": { + "login": "cli" + } + }, + { + "type": "WatchEvent", + "repo": { + "name": "fengari-lua/fengari", + "url": "https://api.github.com/repos/fengari-lua/fengari" + }, + "payload": { + "action": "started" + }, + "created_at": "2022-03-14T13:03:51Z", + "org": { + "login": "fengari-lua", + "avatar_url": "https://avatars.githubusercontent.com/u/28658472?" + } + }, + { + "type": "IssuesEvent", + "repo": { + "name": "cli/cli" + }, + "payload": { + "action": "closed", + "issue": { + "number": 5301, + "title": "wheee" + } + }, + "created_at": "2022-03-14T12:21:59Z", + "org": { + "login": "cli" + } + }, + { + "type": "IssuesEvent", + "repo": { + "name": "cli/cli" + }, + "payload": { + "action": "opened", + "issue": { + "number": 5300, + "title": "Terminal bell when a running task is done" + } + }, + "created_at": "2022-03-14T12:21:59Z", + "org": { + "login": "cli" + } + } +] diff --git a/pkg/cmd/status/fixtures/notifications.json b/pkg/cmd/status/fixtures/notifications.json new file mode 100644 index 000000000..707078b70 --- /dev/null +++ b/pkg/cmd/status/fixtures/notifications.json @@ -0,0 +1,138 @@ +[ + { + "reason": "team_mention", + "subject": { + "title": "Tracking: Close an issue as either resolved or unresolved", + "url": "https://api.github.com/repos/vilmibm/testing/issues/594", + "latest_comment_url": "https://api.github.com/repos/vilmibm/testing/issues/comments/1070788734", + "type": "Issue" + }, + "repository": { + "full_name": "vilmibm/testing", + "owner": { + "login": "vilmibm" + } + } + }, + { + "reason": "comment", + "subject": { + "title": "Unclear message about base repository when creating a PR", + "url": "https://api.github.com/repos/cli/cli/issues/2090", + "latest_comment_url": "https://api.github.com/repos/cli/cli/issues/comments/1069309546", + "type": "Issue" + }, + "repository": { + "full_name": "cli/cli", + "owner": { + "login": "cli" + } + } + }, + { + "reason": "mention", + "subject": { + "title": "Good", + "url": "https://api.github.com/repos/rpd/todo/issues/110", + "latest_comment_url": "https://api.github.com/repos/rpd/todo/issues/110", + "type": "Issue" + }, + "repository": { + "full_name": "rpd/todo", + "owner": { + "login": "rpd" + } + } + }, + { + "reason": "mention", + "subject": { + "title": "Fire Irons", + "url": "https://api.github.com/repos/rpd/todo/issues/4113", + "latest_comment_url": "https://api.github.com/repos/rpd/todo/issues/4113", + "type": "Issue" + }, + "repository": { + "full_name": "rpd/todo", + "owner": { + "login": "rpd" + } + } + }, + { + "reason": "mention", + "subject": { + "title": "PR merge should switch to the base branch instead of the default branch.", + "url": "https://api.github.com/repos/cli/cli/issues/1096", + "latest_comment_url": "https://api.github.com/repos/cli/cli/issues/1096", + "type": "Issue" + }, + "repository": { + "full_name": "cli/cli", + "owner": { + "login": "cli" + } + } + }, + { + "reason": "mention", + "subject": { + "title": "Find bolt cutter", + "url": "https://api.github.com/repos/rpd/todo/issues/116", + "latest_comment_url": "https://api.github.com/repos/rpd/todo/issues/comments/1065", + "type": "Issue" + }, + "repository": { + "full_name": "rpd/todo", + "owner": { + "login": "rpd" + } + } + }, + { + "reason": "mention", + "subject": { + "title": "use the action to precompile", + "url": "https://api.github.com/repos/vilmibm/gh-screensaver/issues/15", + "latest_comment_url": "https://api.github.com/repos/vilmibm/gh-screensaver/issues/comments/10", + "type": "Issue" + }, + "repository": { + "full_name": "vilmibm/gh-screensaver", + "owner": { + "login": "vilmibm" + }, + "url": "https://api.github.com/repos/vilmibm/gh-screensaver" + } + }, + { + "reason": "author", + "subject": { + "title": "asodijasodji", + "url": "https://api.github.com/repos/vilmibm/testing/issues/150", + "latest_comment_url": "https://api.github.com/repos/vilmibm/testing/issues/comments/1064534165", + "type": "Issue" + }, + "repository": { + "full_name": "vilmibm/testing", + "owner": { + "login": "vilmibm" + } + } + }, + { + "reason": "review_requested", + "subject": { + "title": "Update LICENSE", + "url": "https://api.github.com/repos/cli/cli/pulls/5277", + "latest_comment_url": null, + "type": "PullRequest" + }, + "repository": { + "full_name": "cli/cli", + "owner": { + "login": "cli" + } + } + } +] diff --git a/pkg/cmd/status/fixtures/search.json b/pkg/cmd/status/fixtures/search.json new file mode 100644 index 000000000..e0367bdca --- /dev/null +++ b/pkg/cmd/status/fixtures/search.json @@ -0,0 +1,188 @@ +{ + "data": { + "assignments": { + "edges": [ + { + "node": { + "updatedAt": "2022-03-15T17:10:25Z", + "__typename": "PullRequest", + "title": "Pin extensions", + "number": 5272, + "repository": { + "nameWithOwner": "cli/cli" + } + } + }, + { + "node": { + "updatedAt": "2022-03-01T17:10:25Z", + "__typename": "PullRequest", + "title": "Board up RPD windows", + "number": 73, + "repository": { + "nameWithOwner": "rpd/todo" + } + } + }, + { + "node": { + "__typename": "Issue", + "updatedAt": "2022-03-10T21:33:57Z", + "title": "yolo", + "number": 157, + "repository": { + "nameWithOwner": "vilmibm/testing" + } + } + }, + { + "node": { + "updatedAt": "2022-01-06T05:05:12Z", + "__typename": "PullRequest", + "title": "Issue Frecency", + "number": 4768, + "repository": { + "nameWithOwner": "cli/cli" + } + } + }, + { + "node": { + "__typename": "Issue", + "updatedAt": "2021-08-05T20:03:07Z", + "title": "Reducing zombie threat in Raccoon City", + "number": 514, + "repository": { + "nameWithOwner": "rpd/todo" + } + } + }, + { + "node": { + "__typename": "Issue", + "updatedAt": "2021-10-16T23:20:35Z", + "title": "Repo garden for Windows", + "number": 3223, + "repository": { + "nameWithOwner": "cli/cli" + } + } + }, + { + "node": { + "__typename": "Issue", + "updatedAt": "2020-11-10T23:07:37Z", + "title": "welp", + "number": 74, + "repository": { + "nameWithOwner": "vilmibm/testing" + } + } + }, + { + "node": { + "__typename": "Issue", + "updatedAt": "2019-07-20T01:30:01Z", + "title": "Ability to send emails from helpdesk UI", + "number": 42, + "repository": { + "nameWithOwner": "tildetown/tildetown-admin" + } + } + }, + { + "node": { + "__typename": "Issue", + "updatedAt": "2019-07-22T19:37:45Z", + "title": "complete move to class based views", + "number": 22, + "repository": { + "nameWithOwner": "adreyer/arkestrator" + } + } + }, + { + "node": { + "__typename": "Issue", + "updatedAt": "2019-02-20T17:54:24Z", + "title": "`(every)` macro", + "number": 145, + "repository": { + "nameWithOwner": "vilmibm/tildemush" + } + } + }, + { + "node": { + "__typename": "Issue", + "updatedAt": "2019-02-20T18:10:53Z", + "title": "audit move rules", + "number": 79, + "repository": { + "nameWithOwner": "vilmibm/tildemush" + } + } + } + ] + }, + "reviewRequested": { + "edges": [ + { + "node": { + "updatedAt": "2022-03-15T17:10:25Z", + "__typename": "PullRequest", + "title": "Pin extensions", + "number": 5272, + "repository": { + "nameWithOwner": "cli/cli" + } + } + }, + { + "node": { + "updatedAt": "2022-02-18T19:38:20Z", + "__typename": "PullRequest", + "title": "Foobar", + "number": 1234, + "repository": { + "nameWithOwner": "vilmibm/testing" + } + } + }, + { + "node": { + "updatedAt": "2022-02-04T06:56:48Z", + "__typename": "PullRequest", + "title": "Welcome party for Leon", + "number": 50, + "repository": { + "nameWithOwner": "rpd/todo" + } + } + }, + { + "node": { + "updatedAt": "2021-12-20T10:30:39Z", + "__typename": "PullRequest", + "title": "Haircut for Leon", + "number": 49, + "repository": { + "nameWithOwner": "rpd/todo" + } + } + }, + { + "node": { + "updatedAt": "2021-12-20T16:43:15Z", + "__typename": "PullRequest", + "title": "This pull request adds support for json output the to `gh pr checks` …", + "number": 4671, + "repository": { + "nameWithOwner": "cli/cli" + } + } + } + ] + } + } +} diff --git a/pkg/cmd/status/status.go b/pkg/cmd/status/status.go new file mode 100644 index 000000000..87c8d55ee --- /dev/null +++ b/pkg/cmd/status/status.go @@ -0,0 +1,618 @@ +package status + +import ( + "bytes" + "errors" + "fmt" + "net/http" + "net/url" + "sort" + "strings" + "time" + + "github.com/MakeNowJust/heredoc" + "github.com/charmbracelet/lipgloss" + "github.com/cli/cli/v2/api" + "github.com/cli/cli/v2/internal/ghinstance" + "github.com/cli/cli/v2/pkg/cmdutil" + "github.com/cli/cli/v2/pkg/iostreams" + "github.com/cli/cli/v2/utils" + "github.com/spf13/cobra" +) + +type StatusOptions struct { + HttpClient func() (*http.Client, error) + CachedClient func(*http.Client, time.Duration) *http.Client + IO *iostreams.IOStreams + Org string + Exclude string +} + +func NewCmdStatus(f *cmdutil.Factory, runF func(*StatusOptions) error) *cobra.Command { + opts := &StatusOptions{ + CachedClient: func(c *http.Client, ttl time.Duration) *http.Client { + return api.NewCachedClient(c, ttl) + }, + } + opts.HttpClient = f.HttpClient + opts.IO = f.IOStreams + cmd := &cobra.Command{ + Use: "status", + Short: "Print information about relevant issues, pull requests, and notifications across repositories", + Long: heredoc.Doc(` + The status command prints information about your work on GitHub across all the repositories you're subscribed to, including: + + - Assigned Issues + - Assigned Pull Requests + - Review Requests + - Mentions + - Repository Activity (new issues/prs, comments) + + This data can be limited to a single org with -o; specific repositories can be excluded from this list with -e. + `), + Example: heredoc.Doc(` + Exclude some repositories: + + gh status -e"cli/cli,cli/go-gh" + + Limit information to a single organization's repositories: + + gh status -ocli + `), + RunE: func(cmd *cobra.Command, args []string) error { + if runF != nil { + return runF(opts) + } + + return statusRun(opts) + }, + } + + cmd.Flags().StringVarP(&opts.Org, "org", "o", "", "Report status within an organization") + cmd.Flags().StringVarP(&opts.Exclude, "exclude", "e", "", "Comma separated list of repos to exclude in owner/name format") + + return cmd +} + +type Notification struct { + Reason string + Subject struct { + Title string + LatestCommentURL string `json:"latest_comment_url"` + URL string + Type string + } + Repository struct { + Owner struct { + Login string + } + FullName string `json:"full_name"` + } +} + +type StatusItem struct { + Repository string // owner/repo + Identifier string // eg cli/cli#1234 or just 1234 + preview string // eg This is the truncated body of something... + Reason string // only used in repo activity +} + +func (s StatusItem) Preview() string { + return strings.ReplaceAll(strings.ReplaceAll(s.preview, "\r", ""), "\n", " ") +} + +type IssueOrPR struct { + Number int + Title string +} + +type Event struct { + Type string + Org struct { + Login string + } + CreatedAt time.Time `json:"created_at"` + Repo struct { + Name string // owner/repo + } + Payload struct { + Action string + Issue IssueOrPR + PullRequest IssueOrPR `json:"pull_request"` + Comment struct { + Body string + HTMLURL string `json:"html_url"` + } + } +} + +type SearchResult struct { + Type string `json:"__typename"` + UpdatedAt time.Time + Title string + Number int + Repository struct { + NameWithOwner string + } +} + +type Results []SearchResult + +func (rs Results) Len() int { + return len(rs) +} + +func (rs Results) Less(i, j int) bool { + return rs[i].UpdatedAt.After(rs[j].UpdatedAt) +} + +func (rs Results) Swap(i, j int) { + rs[i], rs[j] = rs[j], rs[i] +} + +type StatusGetter struct { + Client *http.Client + cachedClient func(*http.Client, time.Duration) *http.Client + Org string + Exclude string + AssignedPRs []StatusItem + AssignedIssues []StatusItem + Mentions []StatusItem + ReviewRequests []StatusItem + RepoActivity []StatusItem +} + +func NewStatusGetter(client *http.Client, opts *StatusOptions) *StatusGetter { + return &StatusGetter{ + Client: client, + Org: opts.Org, + Exclude: opts.Exclude, + cachedClient: opts.CachedClient, + } +} + +func (s *StatusGetter) CachedClient(ttl time.Duration) *http.Client { + return s.cachedClient(s.Client, ttl) +} + +func (s *StatusGetter) ShouldExclude(repo string) bool { + return strings.Contains(s.Exclude, repo) +} + +func (s *StatusGetter) CurrentUsername() (string, error) { + cachedClient := s.CachedClient(time.Hour * 48) + cachingAPIClient := api.NewClientFromHTTP(cachedClient) + currentUsername, err := api.CurrentLoginName(cachingAPIClient, ghinstance.Default()) + if err != nil { + return "", fmt.Errorf("failed to get current username: %w", err) + } + + return currentUsername, nil +} + +func (s *StatusGetter) ActualMention(n Notification) (string, error) { + currentUsername, err := s.CurrentUsername() + if err != nil { + return "", err + } + + // long cache period since once a comment is looked up, it never needs to be + // consulted again. + cachedClient := s.CachedClient(time.Hour * 24 * 30) + c := api.NewClientFromHTTP(cachedClient) + resp := struct { + Body string + }{} + if err := c.REST(ghinstance.Default(), "GET", n.Subject.LatestCommentURL, nil, &resp); err != nil { + return "", err + } + + var ret string + + if strings.Contains(resp.Body, "@"+currentUsername) { + ret = resp.Body + } + + return ret, nil +} + +// These are split up by endpoint since it is along that boundary we parallelize +// work + +// Populate .Mentions +func (s *StatusGetter) LoadNotifications() error { + perPage := 100 + c := api.NewClientFromHTTP(s.Client) + query := url.Values{} + query.Add("per_page", fmt.Sprintf("%d", perPage)) + query.Add("participating", "true") + query.Add("all", "true") + + // this sucks, having to fetch so much :/ but it was the only way in my + // testing to really get enough mentions. I would love to be able to just + // filter for mentions but it does not seem like the notifications API can + // do that. I'd switch to the GraphQL version, but to my knowledge that does + // not work with PATs right now. + var ns []Notification + var resp []Notification + pages := 3 + for page := 1; page <= pages; page++ { + query.Add("page", fmt.Sprintf("%d", page)) + p := fmt.Sprintf("notifications?%s", query.Encode()) + err := c.REST(ghinstance.Default(), "GET", p, nil, &resp) + if err != nil { + var httpErr api.HTTPError + if !errors.As(err, &httpErr) || httpErr.StatusCode != 404 { + return fmt.Errorf("could not get notifications: %w", err) + } + } + ns = append(ns, resp...) + if len(resp) == 0 || len(resp) < perPage { + break + } + } + + s.Mentions = []StatusItem{} + + for _, n := range ns { + if n.Reason != "mention" { + continue + } + + if s.Org != "" && n.Repository.Owner.Login != s.Org { + continue + } + + if s.ShouldExclude(n.Repository.FullName) { + continue + } + + if actual, err := s.ActualMention(n); actual != "" && err == nil { + // I'm so sorry + split := strings.Split(n.Subject.URL, "/") + s.Mentions = append(s.Mentions, StatusItem{ + Repository: n.Repository.FullName, + Identifier: fmt.Sprintf("%s#%s", n.Repository.FullName, split[len(split)-1]), + preview: actual, + }) + } else if err != nil { + return fmt.Errorf("could not fetch comment: %w", err) + } + } + + return nil +} + +func (s *StatusGetter) buildSearchQuery() string { + q := ` + query AssignedSearch { + assignments: search(first: 25, type: ISSUE, query:"%s") { + edges { + node { + ...on Issue { + __typename + updatedAt + title + number + repository { + nameWithOwner + } + } + ...on PullRequest { + updatedAt + __typename + title + number + repository { + nameWithOwner + } + } + } + } + } + reviewRequested: search(first: 25, type: ISSUE, query:"%s") { + edges { + node { + ...on PullRequest { + updatedAt + __typename + title + number + repository { + nameWithOwner + } + } + } + } + } + }` + assignmentsQ := `assignee:@me state:open%s%s` + requestedQ := `state:open review-requested:@me%s%s` + + orgFilter := "" + if s.Org != "" { + orgFilter = " org:" + s.Org + } + excludeFilter := "" + if s.Exclude != "" { + for _, repo := range strings.Split(s.Exclude, ",") { + excludeFilter += " -repo:" + repo + } + } + assignmentsQ = fmt.Sprintf(assignmentsQ, orgFilter, excludeFilter) + requestedQ = fmt.Sprintf(requestedQ, orgFilter, excludeFilter) + + return fmt.Sprintf(q, assignmentsQ, requestedQ) +} + +// Populate .AssignedPRs, .AssignedIssues, .ReviewRequests +func (s *StatusGetter) LoadSearchResults() error { + q := s.buildSearchQuery() + c := api.NewClientFromHTTP(s.Client) + + var resp struct { + Assignments struct { + Edges []struct { + Node SearchResult + } + } + ReviewRequested struct { + Edges []struct { + Node SearchResult + } + } + } + err := c.GraphQL(ghinstance.Default(), q, nil, &resp) + if err != nil { + return fmt.Errorf("could not search for assignments: %w", err) + } + + prs := []SearchResult{} + issues := []SearchResult{} + reviewRequested := []SearchResult{} + + for _, e := range resp.Assignments.Edges { + if e.Node.Type == "Issue" { + issues = append(issues, e.Node) + } else if e.Node.Type == "PullRequest" { + prs = append(prs, e.Node) + } else { + panic("you shouldn't be here") + } + } + + for _, e := range resp.ReviewRequested.Edges { + reviewRequested = append(reviewRequested, e.Node) + } + + sort.Sort(Results(issues)) + sort.Sort(Results(prs)) + sort.Sort(Results(reviewRequested)) + + s.AssignedIssues = []StatusItem{} + s.AssignedPRs = []StatusItem{} + s.ReviewRequests = []StatusItem{} + + for _, i := range issues { + s.AssignedIssues = append(s.AssignedIssues, StatusItem{ + Repository: i.Repository.NameWithOwner, + Identifier: fmt.Sprintf("%s#%d", i.Repository.NameWithOwner, i.Number), + preview: i.Title, + }) + } + + for _, pr := range prs { + s.AssignedPRs = append(s.AssignedPRs, StatusItem{ + Repository: pr.Repository.NameWithOwner, + Identifier: fmt.Sprintf("%s#%d", pr.Repository.NameWithOwner, pr.Number), + preview: pr.Title, + }) + } + + for _, r := range reviewRequested { + s.ReviewRequests = append(s.ReviewRequests, StatusItem{ + Repository: r.Repository.NameWithOwner, + Identifier: fmt.Sprintf("%s#%d", r.Repository.NameWithOwner, r.Number), + preview: r.Title, + }) + } + + return nil +} + +// Populate .RepoActivity +func (s *StatusGetter) LoadEvents() error { + perPage := 100 + c := api.NewClientFromHTTP(s.Client) + query := url.Values{} + query.Add("per_page", fmt.Sprintf("%d", perPage)) + + currentUsername, err := s.CurrentUsername() + if err != nil { + return err + } + + var events []Event + var resp []Event + pages := 2 + for page := 1; page <= pages; page++ { + query.Add("page", fmt.Sprintf("%d", page)) + p := fmt.Sprintf("users/%s/received_events?%s", currentUsername, query.Encode()) + err := c.REST(ghinstance.Default(), "GET", p, nil, &resp) + if err != nil { + var httpErr api.HTTPError + if !errors.As(err, &httpErr) || httpErr.StatusCode != 404 { + return fmt.Errorf("could not get events: %w", err) + } + } + events = append(events, resp...) + if len(resp) == 0 || len(resp) < perPage { + break + } + } + + s.RepoActivity = []StatusItem{} + + for _, e := range events { + if s.Org != "" && e.Org.Login != s.Org { + continue + } + if s.ShouldExclude(e.Repo.Name) { + continue + } + si := StatusItem{} + var number int + switch e.Type { + case "IssuesEvent": + if e.Payload.Action != "opened" { + continue + } + si.Reason = "new issue" + si.preview = e.Payload.Issue.Title + number = e.Payload.Issue.Number + case "PullRequestEvent": + if e.Payload.Action != "opened" { + continue + } + si.Reason = "new PR" + si.preview = e.Payload.PullRequest.Title + number = e.Payload.PullRequest.Number + case "PullRequestReviewCommentEvent": + si.Reason = "comment on " + e.Payload.PullRequest.Title + si.preview = e.Payload.Comment.Body + number = e.Payload.PullRequest.Number + case "IssueCommentEvent": + si.Reason = "comment on " + e.Payload.Issue.Title + si.preview = e.Payload.Comment.Body + number = e.Payload.Issue.Number + default: + continue + } + si.Repository = e.Repo.Name + si.Identifier = fmt.Sprintf("%s#%d", e.Repo.Name, number) + s.RepoActivity = append(s.RepoActivity, si) + } + + return nil +} + +func statusRun(opts *StatusOptions) error { + client, err := opts.HttpClient() + if err != nil { + return fmt.Errorf("could not create client: %w", err) + } + + sg := NewStatusGetter(client, opts) + errc := make(chan error) + + // TODO break out sections into individual subcommands + + opts.IO.StartProgressIndicator() + go func() { + err := sg.LoadNotifications() + if err != nil { + err = fmt.Errorf("could not load notifications: %w", err) + } + errc <- err + }() + + go func() { + err := sg.LoadEvents() + if err != nil { + err = fmt.Errorf("could not load events: %w", err) + } + errc <- err + }() + + go func() { + err := sg.LoadSearchResults() + if err != nil { + err = fmt.Errorf("failed to search: %w", err) + } + errc <- err + }() + + for i := 0; i < 3; i++ { + if err := <-errc; err != nil { + return err + } + } + opts.IO.StopProgressIndicator() + + cs := opts.IO.ColorScheme() + out := opts.IO.Out + fullWidth := opts.IO.TerminalWidth() + halfWidth := (fullWidth / 2) - 2 + + idStyle := cs.Cyan + leftHalfStyle := lipgloss.NewStyle().Width(halfWidth).Padding(0).BorderRight(true).BorderStyle(lipgloss.NormalBorder()) + rightHalfStyle := lipgloss.NewStyle().Width(halfWidth).Padding(0) + + section := func(header string, items []StatusItem, width, rowLimit int) (string, error) { + tableOut := &bytes.Buffer{} + fmt.Fprintln(tableOut, cs.Bold(header)) + tp := utils.NewTablePrinterWithOptions(opts.IO, utils.TablePrinterOptions{ + IsTTY: opts.IO.IsStdoutTTY(), + MaxWidth: width, + Out: tableOut, + }) + if len(items) == 0 { + tp.AddField("Nothing here ^_^", nil, nil) + tp.EndRow() + } else { + for i, si := range items { + if i == rowLimit { + break + } + tp.AddField(si.Identifier, nil, idStyle) + if si.Reason != "" { + tp.AddField(si.Reason, nil, nil) + } + tp.AddField(si.Preview(), nil, nil) + tp.EndRow() + } + } + + err := tp.Render() + if err != nil { + return "", err + } + + return tableOut.String(), nil + } + + mSection, err := section("Mentions", sg.Mentions, halfWidth, 5) + if err != nil { + return fmt.Errorf("failed to render 'Mentions': %w", err) + } + mSection = rightHalfStyle.Render(mSection) + + rrSection, err := section("Review Requests", sg.ReviewRequests, halfWidth, 5) + if err != nil { + return fmt.Errorf("failed to render 'Review Requests': %w", err) + } + rrSection = leftHalfStyle.Render(rrSection) + + prSection, err := section("Assigned PRs", sg.AssignedPRs, halfWidth, 5) + if err != nil { + return fmt.Errorf("failed to render 'Assigned PRs': %w", err) + } + prSection = rightHalfStyle.Render(prSection) + + issueSection, err := section("Assigned Issues", sg.AssignedIssues, halfWidth, 5) + if err != nil { + return fmt.Errorf("failed to render 'Assigned Issues': %w", err) + } + issueSection = leftHalfStyle.Render(issueSection) + + raSection, err := section("Repository Activity", sg.RepoActivity, fullWidth, 10) + if err != nil { + return fmt.Errorf("failed to render 'Repository Activity': %w", err) + } + + fmt.Fprintln(out, lipgloss.JoinHorizontal(lipgloss.Top, issueSection, prSection)) + fmt.Fprintln(out, lipgloss.JoinHorizontal(lipgloss.Top, rrSection, mSection)) + fmt.Fprintln(out, raSection) + + return nil +} diff --git a/pkg/cmd/status/status_test.go b/pkg/cmd/status/status_test.go new file mode 100644 index 000000000..8ae3e4f5d --- /dev/null +++ b/pkg/cmd/status/status_test.go @@ -0,0 +1,332 @@ +package status + +import ( + "bytes" + "net/http" + "testing" + "time" + + "github.com/cli/cli/v2/pkg/cmdutil" + "github.com/cli/cli/v2/pkg/httpmock" + "github.com/cli/cli/v2/pkg/iostreams" + "github.com/google/shlex" + "github.com/stretchr/testify/assert" +) + +func TestNewCmdStatus(t *testing.T) { + tests := []struct { + name string + cli string + wants StatusOptions + }{ + { + name: "defaults", + }, + { + name: "org", + cli: "-o cli", + wants: StatusOptions{ + Org: "cli", + }, + }, + { + name: "exclude", + cli: "-e cli/cli,cli/go-gh", + wants: StatusOptions{ + Exclude: "cli/cli,cli/go-gh", + }, + }, + } + + for _, tt := range tests { + io, _, _, _ := iostreams.Test() + io.SetStdinTTY(true) + io.SetStdoutTTY(true) + + f := &cmdutil.Factory{ + IOStreams: io, + } + t.Run(tt.name, func(t *testing.T) { + argv, err := shlex.Split(tt.cli) + assert.NoError(t, err) + + var gotOpts *StatusOptions + cmd := NewCmdStatus(f, func(opts *StatusOptions) error { + gotOpts = opts + return nil + }) + cmd.SetArgs(argv) + cmd.SetIn(&bytes.Buffer{}) + _, err = cmd.ExecuteC() + assert.NoError(t, err) + + assert.Equal(t, tt.wants.Org, gotOpts.Org) + assert.Equal(t, tt.wants.Exclude, gotOpts.Exclude) + }) + } +} + +func TestStatusRun(t *testing.T) { + tests := []struct { + name string + httpStubs func(*httpmock.Registry) + opts *StatusOptions + wantOut string + wantErrMsg string + }{ + { + name: "nothing", + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL("UserCurrent"), + httpmock.StringResponse(`{"data": {"viewer": {"login": "jillvalentine"}}}`)) + reg.Register( + httpmock.GraphQL("AssignedSearch"), + httpmock.StringResponse(`{"data": { "assignments": {"edges": [] }, "reviewRequested": {"edges": []}}}`)) + reg.Register( + httpmock.REST("GET", "notifications"), + httpmock.StringResponse(`[]`)) + reg.Register( + httpmock.REST("GET", "users/jillvalentine/received_events"), + httpmock.StringResponse(`[]`)) + }, + opts: &StatusOptions{}, + wantOut: "Assigned Issues │Assigned PRs \nNothing here ^_^ │Nothing here ^_^ \n │ \nReview Requests │Mentions \nNothing here ^_^ │Nothing here ^_^ \n │ \nRepository Activity\nNothing here ^_^\n\n", + }, + { + name: "something", + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL("UserCurrent"), + httpmock.StringResponse(`{"data": {"viewer": {"login": "jillvalentine"}}}`)) + reg.Register( + httpmock.GraphQL("UserCurrent"), + httpmock.StringResponse(`{"data": {"viewer": {"login": "jillvalentine"}}}`)) + reg.Register( + httpmock.GraphQL("UserCurrent"), + httpmock.StringResponse(`{"data": {"viewer": {"login": "jillvalentine"}}}`)) + reg.Register( + httpmock.GraphQL("UserCurrent"), + httpmock.StringResponse(`{"data": {"viewer": {"login": "jillvalentine"}}}`)) + reg.Register( + httpmock.GraphQL("UserCurrent"), + httpmock.StringResponse(`{"data": {"viewer": {"login": "jillvalentine"}}}`)) + reg.Register( + httpmock.REST("GET", "repos/rpd/todo/issues/110"), + httpmock.StringResponse(`{"body":"hello @jillvalentine how are you"}`)) + reg.Register( + httpmock.REST("GET", "repos/rpd/todo/issues/4113"), + httpmock.StringResponse(`{"body":"this is a comment"}`)) + reg.Register( + httpmock.REST("GET", "repos/cli/cli/issues/1096"), + httpmock.StringResponse(`{"body":"@jillvalentine hi"}`)) + reg.Register( + httpmock.REST("GET", "repos/rpd/todo/issues/comments/1065"), + httpmock.StringResponse(`{"body":"not a real mention"}`)) + reg.Register( + httpmock.REST("GET", "repos/vilmibm/gh-screensaver/issues/comments/10"), + httpmock.StringResponse(`{"body":"a message for @jillvalentine"}`)) + reg.Register( + httpmock.GraphQL("UserCurrent"), + httpmock.StringResponse(`{"data": {"viewer": {"login": "jillvalentine"}}}`)) + reg.Register( + httpmock.GraphQL("AssignedSearch"), + httpmock.FileResponse("./fixtures/search.json")) + reg.Register( + httpmock.REST("GET", "notifications"), + httpmock.FileResponse("./fixtures/notifications.json")) + reg.Register( + httpmock.REST("GET", "users/jillvalentine/received_events"), + httpmock.FileResponse("./fixtures/events.json")) + }, + opts: &StatusOptions{}, + wantOut: "Assigned Issues │Assigned PRs \nvilmibm/testing#157 yolo │cli/cli#5272 Pin extensions \ncli/cli#3223 Repo garden...│rpd/todo#73 Board up RPD windows \nrpd/todo#514 Reducing zo...│cli/cli#4768 Issue Frecency \nvilmibm/testing#74 welp │ \nadreyer/arkestrator#22 complete mo...│ \n │ \nReview Requests │Mentions \ncli/cli#5272 Pin extensions │rpd/todo#110 hello @j...\nvilmibm/testing#1234 Foobar │cli/cli#1096 @jillval...\nrpd/todo#50 Welcome party...│vilmibm/gh-screensaver#15 a messag...\ncli/cli#4671 This pull req...│ \nrpd/todo#49 Haircut for Leon│ \n │ \nRepository Activity\nrpd/todo#5326 new PR Only write UTF-8 BOM on W...\nvilmibm/testing#5325 comment on Ability to sea... We are working on dedicat...\ncli/cli#5319 comment on [Codespaces] D... Wondering if we shouldn't...\ncli/cli#5300 new issue Terminal bell when a runn...\n\n", + }, + { + name: "exclude a repository", + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL("UserCurrent"), + httpmock.StringResponse(`{"data": {"viewer": {"login": "jillvalentine"}}}`)) + reg.Register( + httpmock.GraphQL("UserCurrent"), + httpmock.StringResponse(`{"data": {"viewer": {"login": "jillvalentine"}}}`)) + reg.Register( + httpmock.GraphQL("UserCurrent"), + httpmock.StringResponse(`{"data": {"viewer": {"login": "jillvalentine"}}}`)) + reg.Register( + httpmock.GraphQL("UserCurrent"), + httpmock.StringResponse(`{"data": {"viewer": {"login": "jillvalentine"}}}`)) + reg.Register( + httpmock.REST("GET", "repos/rpd/todo/issues/110"), + httpmock.StringResponse(`{"body":"hello @jillvalentine how are you"}`)) + reg.Register( + httpmock.REST("GET", "repos/rpd/todo/issues/4113"), + httpmock.StringResponse(`{"body":"this is a comment"}`)) + reg.Register( + httpmock.REST("GET", "repos/rpd/todo/issues/comments/1065"), + httpmock.StringResponse(`{"body":"not a real mention"}`)) + reg.Register( + httpmock.REST("GET", "repos/vilmibm/gh-screensaver/issues/comments/10"), + httpmock.StringResponse(`{"body":"a message for @jillvalentine"}`)) + reg.Register( + httpmock.GraphQL("UserCurrent"), + httpmock.StringResponse(`{"data": {"viewer": {"login": "jillvalentine"}}}`)) + reg.Register( + httpmock.GraphQL("AssignedSearch"), + httpmock.FileResponse("./fixtures/search.json")) + reg.Register( + httpmock.REST("GET", "notifications"), + httpmock.FileResponse("./fixtures/notifications.json")) + reg.Register( + httpmock.REST("GET", "users/jillvalentine/received_events"), + httpmock.FileResponse("./fixtures/events.json")) + }, + opts: &StatusOptions{ + Exclude: "cli/cli", + }, + // NOTA BENE: you'll see cli/cli in search results because that happens + // server side and the fixture doesn't account for that + wantOut: "Assigned Issues │Assigned PRs \nvilmibm/testing#157 yolo │cli/cli#5272 Pin extensions \ncli/cli#3223 Repo garden...│rpd/todo#73 Board up RPD windows \nrpd/todo#514 Reducing zo...│cli/cli#4768 Issue Frecency \nvilmibm/testing#74 welp │ \nadreyer/arkestrator#22 complete mo...│ \n │ \nReview Requests │Mentions \ncli/cli#5272 Pin extensions │rpd/todo#110 hello @j...\nvilmibm/testing#1234 Foobar │vilmibm/gh-screensaver#15 a messag...\nrpd/todo#50 Welcome party...│ \ncli/cli#4671 This pull req...│ \nrpd/todo#49 Haircut for Leon│ \n │ \nRepository Activity\nrpd/todo#5326 new PR Only write UTF-8 BOM on W...\nvilmibm/testing#5325 comment on Ability to sea... We are working on dedicat...\n\n", + }, + { + name: "exclude repositories", + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL("UserCurrent"), + httpmock.StringResponse(`{"data": {"viewer": {"login": "jillvalentine"}}}`)) + reg.Register( + httpmock.REST("GET", "repos/vilmibm/gh-screensaver/issues/comments/10"), + httpmock.StringResponse(`{"body":"a message for @jillvalentine"}`)) + reg.Register( + httpmock.GraphQL("UserCurrent"), + httpmock.StringResponse(`{"data": {"viewer": {"login": "jillvalentine"}}}`)) + reg.Register( + httpmock.GraphQL("AssignedSearch"), + httpmock.FileResponse("./fixtures/search.json")) + reg.Register( + httpmock.REST("GET", "notifications"), + httpmock.FileResponse("./fixtures/notifications.json")) + reg.Register( + httpmock.REST("GET", "users/jillvalentine/received_events"), + httpmock.FileResponse("./fixtures/events.json")) + }, + opts: &StatusOptions{ + Exclude: "cli/cli,rpd/todo", + }, + // NOTA BENE: you'll see cli/cli in search results because that happens + // server side and the fixture doesn't account for that + wantOut: "Assigned Issues │Assigned PRs \nvilmibm/testing#157 yolo │cli/cli#5272 Pin extensions \ncli/cli#3223 Repo garden...│rpd/todo#73 Board up RPD windows \nrpd/todo#514 Reducing zo...│cli/cli#4768 Issue Frecency \nvilmibm/testing#74 welp │ \nadreyer/arkestrator#22 complete mo...│ \n │ \nReview Requests │Mentions \ncli/cli#5272 Pin extensions │vilmibm/gh-screensaver#15 a messag...\nvilmibm/testing#1234 Foobar │ \nrpd/todo#50 Welcome party...│ \ncli/cli#4671 This pull req...│ \nrpd/todo#49 Haircut for Leon│ \n │ \nRepository Activity\nvilmibm/testing#5325 comment on Ability to sea... We are working on dedicat...\n\n", + }, + { + name: "filter to an org", + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL("UserCurrent"), + httpmock.StringResponse(`{"data": {"viewer": {"login": "jillvalentine"}}}`)) + reg.Register( + httpmock.GraphQL("UserCurrent"), + httpmock.StringResponse(`{"data": {"viewer": {"login": "jillvalentine"}}}`)) + reg.Register( + httpmock.GraphQL("UserCurrent"), + httpmock.StringResponse(`{"data": {"viewer": {"login": "jillvalentine"}}}`)) + reg.Register( + httpmock.REST("GET", "repos/rpd/todo/issues/110"), + httpmock.StringResponse(`{"body":"hello @jillvalentine how are you"}`)) + reg.Register( + httpmock.REST("GET", "repos/rpd/todo/issues/4113"), + httpmock.StringResponse(`{"body":"this is a comment"}`)) + reg.Register( + httpmock.REST("GET", "repos/rpd/todo/issues/comments/1065"), + httpmock.StringResponse(`{"body":"not a real mention"}`)) + reg.Register( + httpmock.GraphQL("UserCurrent"), + httpmock.StringResponse(`{"data": {"viewer": {"login": "jillvalentine"}}}`)) + reg.Register( + httpmock.GraphQL("AssignedSearch"), + httpmock.FileResponse("./fixtures/search.json")) + reg.Register( + httpmock.REST("GET", "notifications"), + httpmock.FileResponse("./fixtures/notifications.json")) + reg.Register( + httpmock.REST("GET", "users/jillvalentine/received_events"), + httpmock.FileResponse("./fixtures/events.json")) + }, + opts: &StatusOptions{ + Org: "rpd", + }, + wantOut: "Assigned Issues │Assigned PRs \nvilmibm/testing#157 yolo │cli/cli#5272 Pin extensions \ncli/cli#3223 Repo garden...│rpd/todo#73 Board up RPD windows \nrpd/todo#514 Reducing zo...│cli/cli#4768 Issue Frecency \nvilmibm/testing#74 welp │ \nadreyer/arkestrator#22 complete mo...│ \n │ \nReview Requests │Mentions \ncli/cli#5272 Pin extensions │rpd/todo#110 hello @jillvalentine ...\nvilmibm/testing#1234 Foobar │ \nrpd/todo#50 Welcome party...│ \ncli/cli#4671 This pull req...│ \nrpd/todo#49 Haircut for Leon│ \n │ \nRepository Activity\nrpd/todo#5326 new PR Only write UTF-8 BOM on Windows where it is needed\n\n", + }, + } + + for _, tt := range tests { + reg := &httpmock.Registry{} + tt.httpStubs(reg) + tt.opts.HttpClient = func() (*http.Client, error) { + return &http.Client{Transport: reg}, nil + } + tt.opts.CachedClient = func(c *http.Client, _ time.Duration) *http.Client { + return c + } + io, _, stdout, _ := iostreams.Test() + io.SetStdoutTTY(true) + tt.opts.IO = io + + t.Run(tt.name, func(t *testing.T) { + err := statusRun(tt.opts) + if tt.wantErrMsg != "" { + assert.Equal(t, tt.wantErrMsg, err.Error()) + return + } + + assert.NoError(t, err) + + assert.Equal(t, tt.wantOut, stdout.String()) + reg.Verify(t) + }) + } +} + +func Test_buildSearchQuery(t *testing.T) { + tests := []struct { + name string + sg StatusGetter + wantReviewQ string + wantAssignedQ string + }{ + { + name: "nothing", + wantReviewQ: "first: 25, type: ISSUE, query:\"state:open review-requested:@me", + wantAssignedQ: "assignee:@me state:open", + }, + { + name: "exclude one", + sg: StatusGetter{ + Exclude: "cli/cli", + }, + wantReviewQ: "first: 25, type: ISSUE, query:\"state:open review-requested:@me -repo:cli/cli", + wantAssignedQ: "assignee:@me state:open", + }, + { + name: "exclude several", + sg: StatusGetter{ + Exclude: "cli/cli,vilmibm/testing", + }, + wantReviewQ: "first: 25, type: ISSUE, query:\"state:open review-requested:@me -repo:cli/cli -repo:vilmibm/testing", + wantAssignedQ: "assignee:@me state:open", + }, + { + name: "org filter", + sg: StatusGetter{ + Org: "cli", + }, + wantReviewQ: "first: 25, type: ISSUE, query:\"state:open review-requested:@me org:cli", + wantAssignedQ: "assignee:@me state:open org:cli", + }, + } + + for _, tt := range tests { + assert.Contains(t, tt.sg.buildSearchQuery(), tt.wantReviewQ) + assert.Contains(t, tt.sg.buildSearchQuery(), tt.wantAssignedQ) + } +} diff --git a/pkg/httpmock/stub.go b/pkg/httpmock/stub.go index 8970e8f49..be281fc0f 100644 --- a/pkg/httpmock/stub.go +++ b/pkg/httpmock/stub.go @@ -30,10 +30,7 @@ func REST(method, p string) Matcher { if !strings.EqualFold(req.Method, method) { return false } - if req.URL.Path != "/"+p { - return false - } - return true + return req.URL.Path == "/"+p } } diff --git a/utils/table_printer.go b/utils/table_printer.go index 6c937f35d..9a40cdbbc 100644 --- a/utils/table_printer.go +++ b/utils/table_printer.go @@ -18,7 +18,9 @@ type TablePrinter interface { } type TablePrinterOptions struct { - IsTTY bool + IsTTY bool + MaxWidth int + Out io.Writer } func NewTablePrinter(io *iostreams.IOStreams) TablePrinter { @@ -27,21 +29,29 @@ func NewTablePrinter(io *iostreams.IOStreams) TablePrinter { }) } -func NewTablePrinterWithOptions(io *iostreams.IOStreams, opts TablePrinterOptions) TablePrinter { +func NewTablePrinterWithOptions(ios *iostreams.IOStreams, opts TablePrinterOptions) TablePrinter { + var out io.Writer + if opts.Out != nil { + out = opts.Out + } else { + out = ios.Out + } if opts.IsTTY { var maxWidth int - if io.IsStdoutTTY() { - maxWidth = io.TerminalWidth() + if opts.MaxWidth > 0 { + maxWidth = opts.MaxWidth + } else if ios.IsStdoutTTY() { + maxWidth = ios.TerminalWidth() } else { - maxWidth = io.ProcessTerminalWidth() + maxWidth = ios.ProcessTerminalWidth() } return &ttyTablePrinter{ - out: io.Out, + out: out, maxWidth: maxWidth, } } return &tsvTablePrinter{ - out: io.Out, + out: out, } } From 058c2b4fe3938465662ecb5d8fdcb1060087cc51 Mon Sep 17 00:00:00 2001 From: nate smith Date: Mon, 28 Mar 2022 11:55:00 -0500 Subject: [PATCH 17/30] redundant --- pkg/cmd/status/status.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkg/cmd/status/status.go b/pkg/cmd/status/status.go index 87c8d55ee..655c12f4e 100644 --- a/pkg/cmd/status/status.go +++ b/pkg/cmd/status/status.go @@ -47,8 +47,6 @@ func NewCmdStatus(f *cmdutil.Factory, runF func(*StatusOptions) error) *cobra.Co - Review Requests - Mentions - Repository Activity (new issues/prs, comments) - - This data can be limited to a single org with -o; specific repositories can be excluded from this list with -e. `), Example: heredoc.Doc(` Exclude some repositories: From ce8f66b421213dfd3d719b223cb11082fc9335e5 Mon Sep 17 00:00:00 2001 From: Jimmy Keith Date: Tue, 29 Mar 2022 07:15:25 -0700 Subject: [PATCH 18/30] Have `extension upgrade --all` be non-fatal when no extensions installed (#5356) --- pkg/cmd/extension/command.go | 3 +++ pkg/cmd/extension/command_test.go | 16 ++++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/pkg/cmd/extension/command.go b/pkg/cmd/extension/command.go index 1c929482f..a650e69ab 100644 --- a/pkg/cmd/extension/command.go +++ b/pkg/cmd/extension/command.go @@ -153,6 +153,9 @@ func NewCmdExtension(f *cmdutil.Factory) *cobra.Command { if err != nil && !errors.Is(err, upToDateError) { if name != "" { fmt.Fprintf(io.ErrOut, "%s Failed upgrading extension %s: %s\n", cs.FailureIcon(), name, err) + } else if errors.Is(err, noExtensionsInstalledError) { + fmt.Fprintf(io.ErrOut, "%s No installed extensions found\n", cs.WarningIcon()) + return nil } else { fmt.Fprintf(io.ErrOut, "%s Failed upgrading extensions\n", cs.FailureIcon()) } diff --git a/pkg/cmd/extension/command_test.go b/pkg/cmd/extension/command_test.go index 7e5520fa4..438fd402e 100644 --- a/pkg/cmd/extension/command_test.go +++ b/pkg/cmd/extension/command_test.go @@ -207,6 +207,22 @@ func TestNewCmdExtension(t *testing.T) { isTTY: true, wantStdout: "✓ Successfully upgraded extensions\n", }, + { + name: "upgrade all none installed", + args: []string{"upgrade", "--all"}, + managerStubs: func(em *extensions.ExtensionManagerMock) func(*testing.T) { + em.UpgradeFunc = func(name string, force bool) error { + return noExtensionsInstalledError + } + return func(t *testing.T) { + calls := em.UpgradeCalls() + assert.Equal(t, 1, len(calls)) + assert.Equal(t, "", calls[0].Name) + } + }, + isTTY: true, + wantStderr: "! No installed extensions found\n", + }, { name: "upgrade all notty", args: []string{"upgrade", "--all"}, From 659a8ed1b90f3ce39314d902140d63ace1e0eb1b Mon Sep 17 00:00:00 2001 From: nate smith Date: Tue, 29 Mar 2022 09:47:56 -0500 Subject: [PATCH 19/30] use a StringSliceVar --- pkg/cmd/status/status.go | 28 +++++++++++++--------------- pkg/cmd/status/status_test.go | 14 +++++++++----- 2 files changed, 22 insertions(+), 20 deletions(-) diff --git a/pkg/cmd/status/status.go b/pkg/cmd/status/status.go index 655c12f4e..595f10826 100644 --- a/pkg/cmd/status/status.go +++ b/pkg/cmd/status/status.go @@ -25,7 +25,7 @@ type StatusOptions struct { CachedClient func(*http.Client, time.Duration) *http.Client IO *iostreams.IOStreams Org string - Exclude string + Exclude []string } func NewCmdStatus(f *cmdutil.Factory, runF func(*StatusOptions) error) *cobra.Command { @@ -49,13 +49,8 @@ func NewCmdStatus(f *cmdutil.Factory, runF func(*StatusOptions) error) *cobra.Co - Repository Activity (new issues/prs, comments) `), Example: heredoc.Doc(` - Exclude some repositories: - - gh status -e"cli/cli,cli/go-gh" - - Limit information to a single organization's repositories: - - gh status -ocli + $ gh status -e cli/cli -e cli/go-gh # Exclude multiple repositories + $ gh status -o cli # Limit results to a single organization `), RunE: func(cmd *cobra.Command, args []string) error { if runF != nil { @@ -67,7 +62,7 @@ func NewCmdStatus(f *cmdutil.Factory, runF func(*StatusOptions) error) *cobra.Co } cmd.Flags().StringVarP(&opts.Org, "org", "o", "", "Report status within an organization") - cmd.Flags().StringVarP(&opts.Exclude, "exclude", "e", "", "Comma separated list of repos to exclude in owner/name format") + cmd.Flags().StringSliceVarP(&opts.Exclude, "exclude", "e", []string{}, "Comma separated list of repos to exclude in owner/name format") return cmd } @@ -152,7 +147,7 @@ type StatusGetter struct { Client *http.Client cachedClient func(*http.Client, time.Duration) *http.Client Org string - Exclude string + Exclude []string AssignedPRs []StatusItem AssignedIssues []StatusItem Mentions []StatusItem @@ -174,7 +169,12 @@ func (s *StatusGetter) CachedClient(ttl time.Duration) *http.Client { } func (s *StatusGetter) ShouldExclude(repo string) bool { - return strings.Contains(s.Exclude, repo) + for _, exclude := range s.Exclude { + if repo == exclude { + return true + } + } + return false } func (s *StatusGetter) CurrentUsername() (string, error) { @@ -332,10 +332,8 @@ func (s *StatusGetter) buildSearchQuery() string { orgFilter = " org:" + s.Org } excludeFilter := "" - if s.Exclude != "" { - for _, repo := range strings.Split(s.Exclude, ",") { - excludeFilter += " -repo:" + repo - } + for _, repo := range s.Exclude { + excludeFilter += " -repo:" + repo } assignmentsQ = fmt.Sprintf(assignmentsQ, orgFilter, excludeFilter) requestedQ = fmt.Sprintf(requestedQ, orgFilter, excludeFilter) diff --git a/pkg/cmd/status/status_test.go b/pkg/cmd/status/status_test.go index 8ae3e4f5d..02df31696 100644 --- a/pkg/cmd/status/status_test.go +++ b/pkg/cmd/status/status_test.go @@ -33,7 +33,7 @@ func TestNewCmdStatus(t *testing.T) { name: "exclude", cli: "-e cli/cli,cli/go-gh", wants: StatusOptions{ - Exclude: "cli/cli,cli/go-gh", + Exclude: []string{"cli/cli", "cli/go-gh"}, }, }, } @@ -43,6 +43,10 @@ func TestNewCmdStatus(t *testing.T) { io.SetStdinTTY(true) io.SetStdoutTTY(true) + if tt.wants.Exclude == nil { + tt.wants.Exclude = []string{} + } + f := &cmdutil.Factory{ IOStreams: io, } @@ -183,7 +187,7 @@ func TestStatusRun(t *testing.T) { httpmock.FileResponse("./fixtures/events.json")) }, opts: &StatusOptions{ - Exclude: "cli/cli", + Exclude: []string{"cli/cli"}, }, // NOTA BENE: you'll see cli/cli in search results because that happens // server side and the fixture doesn't account for that @@ -212,7 +216,7 @@ func TestStatusRun(t *testing.T) { httpmock.FileResponse("./fixtures/events.json")) }, opts: &StatusOptions{ - Exclude: "cli/cli,rpd/todo", + Exclude: []string{"cli/cli", "rpd/todo"}, }, // NOTA BENE: you'll see cli/cli in search results because that happens // server side and the fixture doesn't account for that @@ -302,7 +306,7 @@ func Test_buildSearchQuery(t *testing.T) { { name: "exclude one", sg: StatusGetter{ - Exclude: "cli/cli", + Exclude: []string{"cli/cli"}, }, wantReviewQ: "first: 25, type: ISSUE, query:\"state:open review-requested:@me -repo:cli/cli", wantAssignedQ: "assignee:@me state:open", @@ -310,7 +314,7 @@ func Test_buildSearchQuery(t *testing.T) { { name: "exclude several", sg: StatusGetter{ - Exclude: "cli/cli,vilmibm/testing", + Exclude: []string{"cli/cli", "vilmibm/testing"}, }, wantReviewQ: "first: 25, type: ISSUE, query:\"state:open review-requested:@me -repo:cli/cli -repo:vilmibm/testing", wantAssignedQ: "assignee:@me state:open", From a3fba669bd238991fa6d9e03073feaa5836cbd96 Mon Sep 17 00:00:00 2001 From: nate smith Date: Tue, 29 Mar 2022 10:15:33 -0500 Subject: [PATCH 20/30] add and use RESTWithNext --- api/client.go | 28 ++++++++++++++++++++------- pkg/cmd/status/status.go | 41 ++++++++++++++++++++++++++++------------ 2 files changed, 50 insertions(+), 19 deletions(-) diff --git a/api/client.go b/api/client.go index bfd526fbc..9fd176006 100644 --- a/api/client.go +++ b/api/client.go @@ -316,41 +316,55 @@ func graphQLClient(h *http.Client, hostname string) *graphql.Client { // REST performs a REST request and parses the response. func (c Client) REST(hostname string, method string, p string, body io.Reader, data interface{}) error { + _, err := c.RESTWithNext(hostname, method, p, body, data) + return err +} + +func (c Client) RESTWithNext(hostname string, method string, p string, body io.Reader, data interface{}) (string, error) { req, err := http.NewRequest(method, restURL(hostname, p), body) if err != nil { - return err + return "", err } req.Header.Set("Content-Type", "application/json; charset=utf-8") resp, err := c.http.Do(req) if err != nil { - return err + return "", err } defer resp.Body.Close() success := resp.StatusCode >= 200 && resp.StatusCode < 300 if !success { - return HandleHTTPError(resp) + return "", HandleHTTPError(resp) } if resp.StatusCode == http.StatusNoContent { - return nil + return "", nil } b, err := ioutil.ReadAll(resp.Body) if err != nil { - return err + return "", err } err = json.Unmarshal(b, &data) if err != nil { - return err + return "", err } - return nil + var next string + for _, m := range linkRE.FindAllStringSubmatch(resp.Header.Get("Link"), -1) { + if len(m) > 2 && m[2] == "next" { + next = m[1] + } + } + + return next, nil } +var linkRE = regexp.MustCompile(`<([^>]+)>;\s*rel="([^"]+)"`) + func restURL(hostname string, pathOrURL string) string { if strings.HasPrefix(pathOrURL, "https://") || strings.HasPrefix(pathOrURL, "http://") { return pathOrURL diff --git a/pkg/cmd/status/status.go b/pkg/cmd/status/status.go index 595f10826..944eef2a8 100644 --- a/pkg/cmd/status/status.go +++ b/pkg/cmd/status/status.go @@ -6,6 +6,7 @@ import ( "fmt" "net/http" "net/url" + "regexp" "sort" "strings" "time" @@ -217,6 +218,17 @@ func (s *StatusGetter) ActualMention(n Notification) (string, error) { // These are split up by endpoint since it is along that boundary we parallelize // work +var linkRE = regexp.MustCompile(`<([^>]+)>;\s*rel="([^"]+)"`) + +func findNextPage(resp *http.Response) (string, bool) { + for _, m := range linkRE.FindAllStringSubmatch(resp.Header.Get("Link"), -1) { + if len(m) > 2 && m[2] == "next" { + return m[1], true + } + } + return "", false +} + // Populate .Mentions func (s *StatusGetter) LoadNotifications() error { perPage := 100 @@ -233,11 +245,10 @@ func (s *StatusGetter) LoadNotifications() error { // not work with PATs right now. var ns []Notification var resp []Notification - pages := 3 - for page := 1; page <= pages; page++ { - query.Add("page", fmt.Sprintf("%d", page)) - p := fmt.Sprintf("notifications?%s", query.Encode()) - err := c.REST(ghinstance.Default(), "GET", p, nil, &resp) + pages := 0 + p := fmt.Sprintf("notifications?%s", query.Encode()) + for pages < 3 { + next, err := c.RESTWithNext(ghinstance.Default(), "GET", p, nil, &resp) if err != nil { var httpErr api.HTTPError if !errors.As(err, &httpErr) || httpErr.StatusCode != 404 { @@ -245,9 +256,13 @@ func (s *StatusGetter) LoadNotifications() error { } } ns = append(ns, resp...) - if len(resp) == 0 || len(resp) < perPage { + + if next == "" || len(resp) < perPage { break } + + pages++ + p = next } s.Mentions = []StatusItem{} @@ -430,11 +445,10 @@ func (s *StatusGetter) LoadEvents() error { var events []Event var resp []Event - pages := 2 - for page := 1; page <= pages; page++ { - query.Add("page", fmt.Sprintf("%d", page)) - p := fmt.Sprintf("users/%s/received_events?%s", currentUsername, query.Encode()) - err := c.REST(ghinstance.Default(), "GET", p, nil, &resp) + pages := 0 + p := fmt.Sprintf("users/%s/received_events?%s", currentUsername, query.Encode()) + for pages < 2 { + next, err := c.RESTWithNext(ghinstance.Default(), "GET", p, nil, &resp) if err != nil { var httpErr api.HTTPError if !errors.As(err, &httpErr) || httpErr.StatusCode != 404 { @@ -442,9 +456,12 @@ func (s *StatusGetter) LoadEvents() error { } } events = append(events, resp...) - if len(resp) == 0 || len(resp) < perPage { + if next == "" || len(resp) < perPage { break } + + pages++ + p = next } s.RepoActivity = []StatusItem{} From 893f83367372c382e7f34879a04082240c9f7cbe Mon Sep 17 00:00:00 2001 From: nate smith Date: Tue, 29 Mar 2022 10:25:14 -0500 Subject: [PATCH 21/30] use errgroup --- pkg/cmd/status/status.go | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/pkg/cmd/status/status.go b/pkg/cmd/status/status.go index 944eef2a8..975a2329f 100644 --- a/pkg/cmd/status/status.go +++ b/pkg/cmd/status/status.go @@ -19,6 +19,7 @@ import ( "github.com/cli/cli/v2/pkg/iostreams" "github.com/cli/cli/v2/utils" "github.com/spf13/cobra" + "golang.org/x/sync/errgroup" ) type StatusOptions struct { @@ -516,39 +517,38 @@ func statusRun(opts *StatusOptions) error { } sg := NewStatusGetter(client, opts) - errc := make(chan error) // TODO break out sections into individual subcommands + g := new(errgroup.Group) opts.IO.StartProgressIndicator() - go func() { + g.Go(func() error { err := sg.LoadNotifications() if err != nil { err = fmt.Errorf("could not load notifications: %w", err) } - errc <- err - }() + return err + }) - go func() { + g.Go(func() error { err := sg.LoadEvents() if err != nil { err = fmt.Errorf("could not load events: %w", err) } - errc <- err - }() + return err + }) - go func() { + g.Go(func() error { err := sg.LoadSearchResults() if err != nil { err = fmt.Errorf("failed to search: %w", err) } - errc <- err - }() + return err + }) - for i := 0; i < 3; i++ { - if err := <-errc; err != nil { - return err - } + err = g.Wait() + if err != nil { + return err } opts.IO.StopProgressIndicator() From 0e941dca0af5a809d926d0a1cf6276135afbf7d0 Mon Sep 17 00:00:00 2001 From: nate smith Date: Tue, 29 Mar 2022 10:26:34 -0500 Subject: [PATCH 22/30] words --- pkg/cmd/status/status.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/status/status.go b/pkg/cmd/status/status.go index 975a2329f..70857966f 100644 --- a/pkg/cmd/status/status.go +++ b/pkg/cmd/status/status.go @@ -48,7 +48,7 @@ func NewCmdStatus(f *cmdutil.Factory, runF func(*StatusOptions) error) *cobra.Co - Assigned Pull Requests - Review Requests - Mentions - - Repository Activity (new issues/prs, comments) + - Repository Activity (new issues/pull requests, comments) `), Example: heredoc.Doc(` $ gh status -e cli/cli -e cli/go-gh # Exclude multiple repositories From 5569c0e94de24ce3a8f359ad69aeb6a749a0056e Mon Sep 17 00:00:00 2001 From: nate smith Date: Tue, 29 Mar 2022 10:29:05 -0500 Subject: [PATCH 23/30] oops --- pkg/cmd/status/status.go | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/pkg/cmd/status/status.go b/pkg/cmd/status/status.go index 70857966f..c04737960 100644 --- a/pkg/cmd/status/status.go +++ b/pkg/cmd/status/status.go @@ -6,7 +6,6 @@ import ( "fmt" "net/http" "net/url" - "regexp" "sort" "strings" "time" @@ -219,17 +218,6 @@ func (s *StatusGetter) ActualMention(n Notification) (string, error) { // These are split up by endpoint since it is along that boundary we parallelize // work -var linkRE = regexp.MustCompile(`<([^>]+)>;\s*rel="([^"]+)"`) - -func findNextPage(resp *http.Response) (string, bool) { - for _, m := range linkRE.FindAllStringSubmatch(resp.Header.Get("Link"), -1) { - if len(m) > 2 && m[2] == "next" { - return m[1], true - } - } - return "", false -} - // Populate .Mentions func (s *StatusGetter) LoadNotifications() error { perPage := 100 From fead25b56cd656343fd54eaf0f1136d74925b7b7 Mon Sep 17 00:00:00 2001 From: nate smith Date: Tue, 29 Mar 2022 10:34:20 -0500 Subject: [PATCH 24/30] some lipgloss tweaks --- pkg/cmd/status/status.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/status/status.go b/pkg/cmd/status/status.go index c04737960..2b542b1fd 100644 --- a/pkg/cmd/status/status.go +++ b/pkg/cmd/status/status.go @@ -546,8 +546,8 @@ func statusRun(opts *StatusOptions) error { halfWidth := (fullWidth / 2) - 2 idStyle := cs.Cyan - leftHalfStyle := lipgloss.NewStyle().Width(halfWidth).Padding(0).BorderRight(true).BorderStyle(lipgloss.NormalBorder()) - rightHalfStyle := lipgloss.NewStyle().Width(halfWidth).Padding(0) + leftHalfStyle := lipgloss.NewStyle().Width(halfWidth).Padding(0).MarginRight(1).BorderRight(true).BorderStyle(lipgloss.NormalBorder()) + rightHalfStyle := lipgloss.NewStyle().Width(halfWidth).Padding(0).BorderLeft(true) section := func(header string, items []StatusItem, width, rowLimit int) (string, error) { tableOut := &bytes.Buffer{} From dc034170423c059e71835e8dfccd670a8996cf71 Mon Sep 17 00:00:00 2001 From: nate smith Date: Tue, 29 Mar 2022 10:35:28 -0500 Subject: [PATCH 25/30] PR->Pull Request --- pkg/cmd/status/status.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/status/status.go b/pkg/cmd/status/status.go index 2b542b1fd..f73fa0e5c 100644 --- a/pkg/cmd/status/status.go +++ b/pkg/cmd/status/status.go @@ -594,9 +594,9 @@ func statusRun(opts *StatusOptions) error { } rrSection = leftHalfStyle.Render(rrSection) - prSection, err := section("Assigned PRs", sg.AssignedPRs, halfWidth, 5) + prSection, err := section("Assigned Pull Requests", sg.AssignedPRs, halfWidth, 5) if err != nil { - return fmt.Errorf("failed to render 'Assigned PRs': %w", err) + return fmt.Errorf("failed to render 'Assigned Pull Requests': %w", err) } prSection = rightHalfStyle.Render(prSection) From 2712ef05dfca29282917f9cbdb5912d3de329d6f Mon Sep 17 00:00:00 2001 From: nate smith Date: Tue, 29 Mar 2022 10:44:48 -0500 Subject: [PATCH 26/30] fix tests to account for padding --- pkg/cmd/status/status_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/cmd/status/status_test.go b/pkg/cmd/status/status_test.go index 02df31696..0e06207e8 100644 --- a/pkg/cmd/status/status_test.go +++ b/pkg/cmd/status/status_test.go @@ -95,7 +95,7 @@ func TestStatusRun(t *testing.T) { httpmock.StringResponse(`[]`)) }, opts: &StatusOptions{}, - wantOut: "Assigned Issues │Assigned PRs \nNothing here ^_^ │Nothing here ^_^ \n │ \nReview Requests │Mentions \nNothing here ^_^ │Nothing here ^_^ \n │ \nRepository Activity\nNothing here ^_^\n\n", + wantOut: "Assigned Issues │ Assigned Pull Requests \nNothing here ^_^ │ Nothing here ^_^ \n │ \nReview Requests │ Mentions \nNothing here ^_^ │ Nothing here ^_^ \n │ \nRepository Activity\nNothing here ^_^\n\n", }, { name: "something", @@ -144,7 +144,7 @@ func TestStatusRun(t *testing.T) { httpmock.FileResponse("./fixtures/events.json")) }, opts: &StatusOptions{}, - wantOut: "Assigned Issues │Assigned PRs \nvilmibm/testing#157 yolo │cli/cli#5272 Pin extensions \ncli/cli#3223 Repo garden...│rpd/todo#73 Board up RPD windows \nrpd/todo#514 Reducing zo...│cli/cli#4768 Issue Frecency \nvilmibm/testing#74 welp │ \nadreyer/arkestrator#22 complete mo...│ \n │ \nReview Requests │Mentions \ncli/cli#5272 Pin extensions │rpd/todo#110 hello @j...\nvilmibm/testing#1234 Foobar │cli/cli#1096 @jillval...\nrpd/todo#50 Welcome party...│vilmibm/gh-screensaver#15 a messag...\ncli/cli#4671 This pull req...│ \nrpd/todo#49 Haircut for Leon│ \n │ \nRepository Activity\nrpd/todo#5326 new PR Only write UTF-8 BOM on W...\nvilmibm/testing#5325 comment on Ability to sea... We are working on dedicat...\ncli/cli#5319 comment on [Codespaces] D... Wondering if we shouldn't...\ncli/cli#5300 new issue Terminal bell when a runn...\n\n", + wantOut: "Assigned Issues │ Assigned Pull Requests \nvilmibm/testing#157 yolo │ cli/cli#5272 Pin extensions \ncli/cli#3223 Repo garden...│ rpd/todo#73 Board up RPD windows \nrpd/todo#514 Reducing zo...│ cli/cli#4768 Issue Frecency \nvilmibm/testing#74 welp │ \nadreyer/arkestrator#22 complete mo...│ \n │ \nReview Requests │ Mentions \ncli/cli#5272 Pin extensions │ rpd/todo#110 hello @j...\nvilmibm/testing#1234 Foobar │ cli/cli#1096 @jillval...\nrpd/todo#50 Welcome party...│ vilmibm/gh-screensaver#15 a messag...\ncli/cli#4671 This pull req...│ \nrpd/todo#49 Haircut for Leon│ \n │ \nRepository Activity\nrpd/todo#5326 new PR Only write UTF-8 BOM on W...\nvilmibm/testing#5325 comment on Ability to sea... We are working on dedicat...\ncli/cli#5319 comment on [Codespaces] D... Wondering if we shouldn't...\ncli/cli#5300 new issue Terminal bell when a runn...\n\n", }, { name: "exclude a repository", @@ -191,7 +191,7 @@ func TestStatusRun(t *testing.T) { }, // NOTA BENE: you'll see cli/cli in search results because that happens // server side and the fixture doesn't account for that - wantOut: "Assigned Issues │Assigned PRs \nvilmibm/testing#157 yolo │cli/cli#5272 Pin extensions \ncli/cli#3223 Repo garden...│rpd/todo#73 Board up RPD windows \nrpd/todo#514 Reducing zo...│cli/cli#4768 Issue Frecency \nvilmibm/testing#74 welp │ \nadreyer/arkestrator#22 complete mo...│ \n │ \nReview Requests │Mentions \ncli/cli#5272 Pin extensions │rpd/todo#110 hello @j...\nvilmibm/testing#1234 Foobar │vilmibm/gh-screensaver#15 a messag...\nrpd/todo#50 Welcome party...│ \ncli/cli#4671 This pull req...│ \nrpd/todo#49 Haircut for Leon│ \n │ \nRepository Activity\nrpd/todo#5326 new PR Only write UTF-8 BOM on W...\nvilmibm/testing#5325 comment on Ability to sea... We are working on dedicat...\n\n", + wantOut: "Assigned Issues │ Assigned Pull Requests \nvilmibm/testing#157 yolo │ cli/cli#5272 Pin extensions \ncli/cli#3223 Repo garden...│ rpd/todo#73 Board up RPD windows \nrpd/todo#514 Reducing zo...│ cli/cli#4768 Issue Frecency \nvilmibm/testing#74 welp │ \nadreyer/arkestrator#22 complete mo...│ \n │ \nReview Requests │ Mentions \ncli/cli#5272 Pin extensions │ rpd/todo#110 hello @j...\nvilmibm/testing#1234 Foobar │ vilmibm/gh-screensaver#15 a messag...\nrpd/todo#50 Welcome party...│ \ncli/cli#4671 This pull req...│ \nrpd/todo#49 Haircut for Leon│ \n │ \nRepository Activity\nrpd/todo#5326 new PR Only write UTF-8 BOM on W...\nvilmibm/testing#5325 comment on Ability to sea... We are working on dedicat...\n\n", }, { name: "exclude repositories", @@ -220,7 +220,7 @@ func TestStatusRun(t *testing.T) { }, // NOTA BENE: you'll see cli/cli in search results because that happens // server side and the fixture doesn't account for that - wantOut: "Assigned Issues │Assigned PRs \nvilmibm/testing#157 yolo │cli/cli#5272 Pin extensions \ncli/cli#3223 Repo garden...│rpd/todo#73 Board up RPD windows \nrpd/todo#514 Reducing zo...│cli/cli#4768 Issue Frecency \nvilmibm/testing#74 welp │ \nadreyer/arkestrator#22 complete mo...│ \n │ \nReview Requests │Mentions \ncli/cli#5272 Pin extensions │vilmibm/gh-screensaver#15 a messag...\nvilmibm/testing#1234 Foobar │ \nrpd/todo#50 Welcome party...│ \ncli/cli#4671 This pull req...│ \nrpd/todo#49 Haircut for Leon│ \n │ \nRepository Activity\nvilmibm/testing#5325 comment on Ability to sea... We are working on dedicat...\n\n", + wantOut: "Assigned Issues │ Assigned Pull Requests \nvilmibm/testing#157 yolo │ cli/cli#5272 Pin extensions \ncli/cli#3223 Repo garden...│ rpd/todo#73 Board up RPD windows \nrpd/todo#514 Reducing zo...│ cli/cli#4768 Issue Frecency \nvilmibm/testing#74 welp │ \nadreyer/arkestrator#22 complete mo...│ \n │ \nReview Requests │ Mentions \ncli/cli#5272 Pin extensions │ vilmibm/gh-screensaver#15 a messag...\nvilmibm/testing#1234 Foobar │ \nrpd/todo#50 Welcome party...│ \ncli/cli#4671 This pull req...│ \nrpd/todo#49 Haircut for Leon│ \n │ \nRepository Activity\nvilmibm/testing#5325 comment on Ability to sea... We are working on dedicat...\n\n", }, { name: "filter to an org", @@ -259,7 +259,7 @@ func TestStatusRun(t *testing.T) { opts: &StatusOptions{ Org: "rpd", }, - wantOut: "Assigned Issues │Assigned PRs \nvilmibm/testing#157 yolo │cli/cli#5272 Pin extensions \ncli/cli#3223 Repo garden...│rpd/todo#73 Board up RPD windows \nrpd/todo#514 Reducing zo...│cli/cli#4768 Issue Frecency \nvilmibm/testing#74 welp │ \nadreyer/arkestrator#22 complete mo...│ \n │ \nReview Requests │Mentions \ncli/cli#5272 Pin extensions │rpd/todo#110 hello @jillvalentine ...\nvilmibm/testing#1234 Foobar │ \nrpd/todo#50 Welcome party...│ \ncli/cli#4671 This pull req...│ \nrpd/todo#49 Haircut for Leon│ \n │ \nRepository Activity\nrpd/todo#5326 new PR Only write UTF-8 BOM on Windows where it is needed\n\n", + wantOut: "Assigned Issues │ Assigned Pull Requests \nvilmibm/testing#157 yolo │ cli/cli#5272 Pin extensions \ncli/cli#3223 Repo garden...│ rpd/todo#73 Board up RPD windows \nrpd/todo#514 Reducing zo...│ cli/cli#4768 Issue Frecency \nvilmibm/testing#74 welp │ \nadreyer/arkestrator#22 complete mo...│ \n │ \nReview Requests │ Mentions \ncli/cli#5272 Pin extensions │ rpd/todo#110 hello @jillvalentine ...\nvilmibm/testing#1234 Foobar │ \nrpd/todo#50 Welcome party...│ \ncli/cli#4671 This pull req...│ \nrpd/todo#49 Haircut for Leon│ \n │ \nRepository Activity\nrpd/todo#5326 new PR Only write UTF-8 BOM on Windows where it is needed\n\n", }, } From 5b060a272d344614f83a5ee02872506f536065c9 Mon Sep 17 00:00:00 2001 From: nate smith Date: Tue, 29 Mar 2022 11:01:54 -0500 Subject: [PATCH 27/30] nvm --- pkg/cmd/status/status.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/status/status.go b/pkg/cmd/status/status.go index f73fa0e5c..4091c2290 100644 --- a/pkg/cmd/status/status.go +++ b/pkg/cmd/status/status.go @@ -547,7 +547,7 @@ func statusRun(opts *StatusOptions) error { idStyle := cs.Cyan leftHalfStyle := lipgloss.NewStyle().Width(halfWidth).Padding(0).MarginRight(1).BorderRight(true).BorderStyle(lipgloss.NormalBorder()) - rightHalfStyle := lipgloss.NewStyle().Width(halfWidth).Padding(0).BorderLeft(true) + rightHalfStyle := lipgloss.NewStyle().Width(halfWidth).Padding(0) section := func(header string, items []StatusItem, width, rowLimit int) (string, error) { tableOut := &bytes.Buffer{} From 56fda0f8c6880364539e76d500d95da83da81014 Mon Sep 17 00:00:00 2001 From: lylecantcode <95375350+lylecantcode@users.noreply.github.com> Date: Tue, 29 Mar 2022 17:05:35 +0100 Subject: [PATCH 28/30] Support GH_DEBUG to control verbosity, deprecate DEBUG (#5306) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The GH_DEBUG environment variable is a new gh-specific verbosity control. For backwards-compatibility, DEBUG will still be respected if it has values "1", "true", "yes", and "api", but any other values will be ignored. Finally, support for "oauth" debug value has been dropped in favor of "api". The "oauth" value only had limited, internal use. Co-authored-by: Mislav Marohnić --- cmd/gh/main.go | 14 ++++-------- internal/authflow/flow.go | 6 +++-- internal/run/run.go | 6 +++-- pkg/cmd/factory/http.go | 6 ++--- pkg/cmd/factory/http_test.go | 44 ++++++++++++++++++++++++++++++++++-- pkg/cmd/root/help_topic.go | 7 ++++-- utils/utils.go | 22 ++++++++++++++++++ 7 files changed, 85 insertions(+), 20 deletions(-) diff --git a/cmd/gh/main.go b/cmd/gh/main.go index a8f1ed141..9a2501f49 100644 --- a/cmd/gh/main.go +++ b/cmd/gh/main.go @@ -58,7 +58,7 @@ func mainRun() exitCode { updateMessageChan <- rel }() - hasDebug := os.Getenv("DEBUG") != "" + hasDebug, _ := utils.IsDebugEnabled() cmdFactory := factory.New(buildVersion) stderr := cmdFactory.IOStreams.ErrOut @@ -327,8 +327,10 @@ func checkForUpdate(currentVersion string) (*update.ReleaseInfo, error) { // does not depend on user configuration func basicClient(currentVersion string) (*api.Client, error) { var opts []api.ClientOption - if verbose := os.Getenv("DEBUG"); verbose != "" { - opts = append(opts, apiVerboseLog()) + if isVerbose, debugValue := utils.IsDebugEnabled(); isVerbose { + colorize := utils.IsTerminal(os.Stderr) + logTraffic := strings.Contains(debugValue, "api") + opts = append(opts, api.VerboseLog(colorable.NewColorable(os.Stderr), logTraffic, colorize)) } opts = append(opts, api.AddHeader("User-Agent", fmt.Sprintf("GitHub CLI %s", currentVersion))) @@ -344,12 +346,6 @@ func basicClient(currentVersion string) (*api.Client, error) { return api.NewClient(opts...), nil } -func apiVerboseLog() api.ClientOption { - logTraffic := strings.Contains(os.Getenv("DEBUG"), "api") - colorize := utils.IsTerminal(os.Stderr) - return api.VerboseLog(colorable.NewColorable(os.Stderr), logTraffic, colorize) -} - func isRecentRelease(publishedAt time.Time) bool { return !publishedAt.IsZero() && time.Since(publishedAt) < time.Hour*24 } diff --git a/internal/authflow/flow.go b/internal/authflow/flow.go index fbf0a9e34..c809e455a 100644 --- a/internal/authflow/flow.go +++ b/internal/authflow/flow.go @@ -12,6 +12,7 @@ import ( "github.com/cli/cli/v2/internal/ghinstance" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/iostreams" + "github.com/cli/cli/v2/utils" "github.com/cli/oauth" ) @@ -53,8 +54,9 @@ func authFlow(oauthHost string, IO *iostreams.IOStreams, notice string, addition cs := IO.ColorScheme() httpClient := http.DefaultClient - if envDebug := os.Getenv("DEBUG"); envDebug != "" { - logTraffic := strings.Contains(envDebug, "api") || strings.Contains(envDebug, "oauth") + debugEnabled, debugValue := utils.IsDebugEnabled() + if debugEnabled { + logTraffic := strings.Contains(debugValue, "api") httpClient.Transport = api.VerboseLog(IO.ErrOut, logTraffic, IO.ColorEnabled())(httpClient.Transport) } diff --git a/internal/run/run.go b/internal/run/run.go index 58fb189e3..d482e04cc 100644 --- a/internal/run/run.go +++ b/internal/run/run.go @@ -8,6 +8,8 @@ import ( "os/exec" "path/filepath" "strings" + + "github.com/cli/cli/v2/utils" ) // Runnable is typically an exec.Cmd or its stub in tests @@ -28,7 +30,7 @@ type cmdWithStderr struct { } func (c cmdWithStderr) Output() ([]byte, error) { - if os.Getenv("DEBUG") != "" { + if isVerbose, _ := utils.IsDebugEnabled(); isVerbose { _ = printArgs(os.Stderr, c.Cmd.Args) } if c.Cmd.Stderr != nil { @@ -44,7 +46,7 @@ func (c cmdWithStderr) Output() ([]byte, error) { } func (c cmdWithStderr) Run() error { - if os.Getenv("DEBUG") != "" { + if isVerbose, _ := utils.IsDebugEnabled(); isVerbose { _ = printArgs(os.Stderr, c.Cmd.Args) } if c.Cmd.Stderr != nil { diff --git a/pkg/cmd/factory/http.go b/pkg/cmd/factory/http.go index 7037b1558..f5c30582f 100644 --- a/pkg/cmd/factory/http.go +++ b/pkg/cmd/factory/http.go @@ -3,7 +3,6 @@ package factory import ( "fmt" "net/http" - "os" "regexp" "strings" "time" @@ -12,6 +11,7 @@ import ( "github.com/cli/cli/v2/internal/ghinstance" "github.com/cli/cli/v2/internal/httpunix" "github.com/cli/cli/v2/pkg/iostreams" + "github.com/cli/cli/v2/utils" ) var timezoneNames = map[int]string{ @@ -84,8 +84,8 @@ func NewHTTPClient(io *iostreams.IOStreams, cfg configGetter, appVersion string, })) } - if verbose := os.Getenv("DEBUG"); verbose != "" { - logTraffic := strings.Contains(verbose, "api") + if isVerbose, debugValue := utils.IsDebugEnabled(); isVerbose { + logTraffic := strings.Contains(debugValue, "api") opts = append(opts, api.VerboseLog(io.ErrOut, logTraffic, io.IsStderrTTY())) } diff --git a/pkg/cmd/factory/http_test.go b/pkg/cmd/factory/http_test.go index 0cb5ac15c..0686b855d 100644 --- a/pkg/cmd/factory/http_test.go +++ b/pkg/cmd/factory/http_test.go @@ -24,6 +24,8 @@ func TestNewHTTPClient(t *testing.T) { name string args args envDebug string + setGhDebug bool + envGhDebug string host string sso string wantHeader map[string]string @@ -82,8 +84,39 @@ func TestNewHTTPClient(t *testing.T) { appVersion: "v1.2.3", setAccept: true, }, - host: "github.com", - envDebug: "api", + host: "github.com", + envDebug: "api", + setGhDebug: false, + wantHeader: map[string]string{ + "authorization": "token MYTOKEN", + "user-agent": "GitHub CLI v1.2.3", + "accept": "application/vnd.github.merge-info-preview+json, application/vnd.github.nebula-preview", + }, + wantStderr: heredoc.Doc(` + * Request at