From 583e74d70c22356f7dc80e71d10a4bb4ec397efc Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Tue, 25 May 2021 16:01:37 -0400 Subject: [PATCH 01/54] Add support for XDG_STATE_HOME --- cmd/gh/main.go | 2 +- internal/config/config_file.go | 73 ++++++++-- internal/config/config_file_test.go | 207 +++++++++++++++++++++++----- internal/config/testing.go | 8 -- 4 files changed, 237 insertions(+), 53 deletions(-) diff --git a/cmd/gh/main.go b/cmd/gh/main.go index b9dc52858..d4459dd0c 100644 --- a/cmd/gh/main.go +++ b/cmd/gh/main.go @@ -264,7 +264,7 @@ func checkForUpdate(currentVersion string) (*update.ReleaseInfo, error) { } repo := updaterEnabled - stateFilePath := filepath.Join(config.ConfigDir(), "state.yml") + stateFilePath := filepath.Join(config.StateDir(), "state.yml") return update.CheckForUpdate(client, stateFilePath, repo, currentVersion) } diff --git a/internal/config/config_file.go b/internal/config/config_file.go index bcf475f64..b4c611af8 100644 --- a/internal/config/config_file.go +++ b/internal/config/config_file.go @@ -16,6 +16,7 @@ import ( const ( GH_CONFIG_DIR = "GH_CONFIG_DIR" XDG_CONFIG_HOME = "XDG_CONFIG_HOME" + XDG_STATE_HOME = "XDG_STATE_HOME" APP_DATA = "AppData" ) @@ -51,28 +52,82 @@ func ConfigDir() string { func autoMigrateConfigDir(newPath string) { path, err := os.UserHomeDir() if oldPath := filepath.Join(path, ".config", "gh"); err == nil && dirExists(oldPath) { - migrateConfigDir(oldPath, newPath) + migrateDir(oldPath, newPath) return } path, err = homedir.Dir() if oldPath := filepath.Join(path, ".config", "gh"); err == nil && dirExists(oldPath) { - migrateConfigDir(oldPath, newPath) + migrateDir(oldPath, newPath) } } +func StateDir() string { + if path := os.Getenv(XDG_STATE_HOME); path != "" { + path = filepath.Join(path, "gh") + if _, err := os.Stat(path); errors.Is(err, os.ErrNotExist) { + _ = os.MkdirAll(path, 0755) + autoMigrateStateDir(path) + } + return path + } + + return ConfigDir() +} + +// Check default paths (os.UserHomeDir, and homedir.Dir) for existing state file (state.yml) +// If state file exist then move it to newPath +// TODO: Remove support for homedir.Dir location in v2 +func autoMigrateStateDir(newPath string) { + path, err := os.UserHomeDir() + if oldPath := filepath.Join(path, ".config", "gh"); err == nil && dirExists(oldPath) { + migrateFile(oldPath, newPath, "state.yml") + return + } + + path, err = homedir.Dir() + if oldPath := filepath.Join(path, ".config", "gh"); err == nil && dirExists(oldPath) { + migrateFile(oldPath, newPath, "state.yml") + } +} + +func migrateFile(oldPath, newPath, file string) { + if oldPath == newPath { + return + } + + oldFile := filepath.Join(oldPath, file) + newFile := filepath.Join(newPath, file) + + if !fileExists(oldFile) { + return + } + + _ = os.MkdirAll(filepath.Dir(newFile), 0755) + _ = os.Rename(oldFile, newFile) +} + +func migrateDir(oldPath, newPath string) { + if oldPath == newPath { + return + } + + if !dirExists(oldPath) { + return + } + + _ = os.MkdirAll(filepath.Dir(newPath), 0755) + _ = os.Rename(oldPath, newPath) +} + func dirExists(path string) bool { f, err := os.Stat(path) return err == nil && f.IsDir() } -var migrateConfigDir = func(oldPath, newPath string) { - if oldPath == newPath { - return - } - - _ = os.MkdirAll(filepath.Dir(newPath), 0755) - _ = os.Rename(oldPath, newPath) +func fileExists(path string) bool { + f, err := os.Stat(path) + return err == nil && !f.IsDir() } func ConfigFile() string { diff --git a/internal/config/config_file_test.go b/internal/config/config_file_test.go index 188f10ff0..c54727332 100644 --- a/internal/config/config_file_test.go +++ b/internal/config/config_file_test.go @@ -152,6 +152,8 @@ func Test_parseConfigFile(t *testing.T) { } func Test_ConfigDir(t *testing.T) { + tempDir := t.TempDir() + tests := []struct { name string onlyWindows bool @@ -159,63 +161,63 @@ func Test_ConfigDir(t *testing.T) { output string }{ { - name: "no envVars", + name: "HOME/USERPROFILE specified", env: map[string]string{ "GH_CONFIG_DIR": "", "XDG_CONFIG_HOME": "", "AppData": "", - "USERPROFILE": "", - "HOME": "", + "USERPROFILE": tempDir, + "HOME": tempDir, }, - output: ".config/gh", + output: filepath.Join(tempDir, ".config", "gh"), }, { name: "GH_CONFIG_DIR specified", env: map[string]string{ - "GH_CONFIG_DIR": "/tmp/gh_config_dir", + "GH_CONFIG_DIR": filepath.Join(tempDir, "gh_config_dir"), }, - output: "/tmp/gh_config_dir", + output: filepath.Join(tempDir, "gh_config_dir"), }, { name: "XDG_CONFIG_HOME specified", env: map[string]string{ - "XDG_CONFIG_HOME": "/tmp", + "XDG_CONFIG_HOME": tempDir, }, - output: "/tmp/gh", + output: filepath.Join(tempDir, "gh"), }, { name: "GH_CONFIG_DIR and XDG_CONFIG_HOME specified", env: map[string]string{ - "GH_CONFIG_DIR": "/tmp/gh_config_dir", - "XDG_CONFIG_HOME": "/tmp", + "GH_CONFIG_DIR": filepath.Join(tempDir, "gh_config_dir"), + "XDG_CONFIG_HOME": tempDir, }, - output: "/tmp/gh_config_dir", + output: filepath.Join(tempDir, "gh_config_dir"), }, { name: "AppData specified", onlyWindows: true, env: map[string]string{ - "AppData": "/tmp/", + "AppData": tempDir, }, - output: "/tmp/GitHub CLI", + output: filepath.Join(tempDir, "GitHub CLI"), }, { name: "GH_CONFIG_DIR and AppData specified", onlyWindows: true, env: map[string]string{ - "GH_CONFIG_DIR": "/tmp/gh_config_dir", - "AppData": "/tmp", + "GH_CONFIG_DIR": filepath.Join(tempDir, "gh_config_dir"), + "AppData": tempDir, }, - output: "/tmp/gh_config_dir", + output: filepath.Join(tempDir, "gh_config_dir"), }, { name: "XDG_CONFIG_HOME and AppData specified", onlyWindows: true, env: map[string]string{ - "XDG_CONFIG_HOME": "/tmp", - "AppData": "/tmp", + "XDG_CONFIG_HOME": tempDir, + "AppData": tempDir, }, - output: "/tmp/gh", + output: filepath.Join(tempDir, "gh"), }, } @@ -227,22 +229,25 @@ func Test_ConfigDir(t *testing.T) { if tt.env != nil { for k, v := range tt.env { old := os.Getenv(k) - os.Setenv(k, filepath.FromSlash(v)) + os.Setenv(k, v) defer os.Setenv(k, old) } } - defer stubMigrateConfigDir()() - assert.Equal(t, filepath.FromSlash(tt.output), ConfigDir()) + // Create directory to skip auto migration code + // which gets run when target directory does not exist + _ = os.MkdirAll(tt.output, 0755) + + assert.Equal(t, tt.output, ConfigDir()) }) } } func Test_configFile_Write_toDisk(t *testing.T) { configDir := filepath.Join(t.TempDir(), ".config", "gh") + _ = os.MkdirAll(configDir, 0755) os.Setenv(GH_CONFIG_DIR, configDir) defer os.Unsetenv(GH_CONFIG_DIR) - defer stubMigrateConfigDir()() cfg := NewFromString(`pager: less`) err := cfg.Write() @@ -265,6 +270,8 @@ func Test_configFile_Write_toDisk(t *testing.T) { } func Test_autoMigrateConfigDir_noMigration(t *testing.T) { + // homeDir does not have any config files + homeDir := t.TempDir() migrateDir := t.TempDir() homeEnvVar := "HOME" @@ -272,7 +279,7 @@ func Test_autoMigrateConfigDir_noMigration(t *testing.T) { homeEnvVar = "USERPROFILE" } old := os.Getenv(homeEnvVar) - os.Setenv(homeEnvVar, "/nonexistent-dir") + os.Setenv(homeEnvVar, homeDir) defer os.Setenv(homeEnvVar, old) autoMigrateConfigDir(migrateDir) @@ -283,14 +290,17 @@ func Test_autoMigrateConfigDir_noMigration(t *testing.T) { } func Test_autoMigrateConfigDir_noMigration_samePath(t *testing.T) { - migrateDir := t.TempDir() + homeDir := t.TempDir() + migrateDir := filepath.Join(homeDir, ".config", "gh") + err := os.MkdirAll(migrateDir, 0755) + assert.NoError(t, err) homeEnvVar := "HOME" if runtime.GOOS == "windows" { homeEnvVar = "USERPROFILE" } old := os.Getenv(homeEnvVar) - os.Setenv(homeEnvVar, migrateDir) + os.Setenv(homeEnvVar, homeDir) defer os.Setenv(homeEnvVar, old) autoMigrateConfigDir(migrateDir) @@ -301,31 +311,158 @@ func Test_autoMigrateConfigDir_noMigration_samePath(t *testing.T) { } func Test_autoMigrateConfigDir_migration(t *testing.T) { - defaultDir := t.TempDir() - dd := filepath.Join(defaultDir, ".config", "gh") + homeDir := t.TempDir() migrateDir := t.TempDir() - md := filepath.Join(migrateDir, ".config", "gh") + homeConfigDir := filepath.Join(homeDir, ".config", "gh") + migrateConfigDir := filepath.Join(migrateDir, ".config", "gh") homeEnvVar := "HOME" if runtime.GOOS == "windows" { homeEnvVar = "USERPROFILE" } old := os.Getenv(homeEnvVar) - os.Setenv(homeEnvVar, defaultDir) + os.Setenv(homeEnvVar, homeDir) defer os.Setenv(homeEnvVar, old) - err := os.MkdirAll(dd, 0777) + err := os.MkdirAll(homeConfigDir, 0755) assert.NoError(t, err) - f, err := ioutil.TempFile(dd, "") + f, err := ioutil.TempFile(homeConfigDir, "") assert.NoError(t, err) f.Close() - autoMigrateConfigDir(md) + autoMigrateConfigDir(migrateConfigDir) - _, err = ioutil.ReadDir(dd) + _, err = ioutil.ReadDir(homeConfigDir) assert.True(t, os.IsNotExist(err)) - files, err := ioutil.ReadDir(md) + files, err := ioutil.ReadDir(migrateConfigDir) assert.NoError(t, err) assert.Equal(t, 1, len(files)) } + +func Test_StateDir(t *testing.T) { + tempDir := t.TempDir() + + tests := []struct { + name string + env map[string]string + output string + }{ + { + name: "HOME/USERPROFILE specified", + env: map[string]string{ + "XDG_STATE_HOME": "", + "GH_CONFIG_DIR": "", + "XDG_CONFIG_HOME": "", + "AppData": "", + "USERPROFILE": tempDir, + "HOME": tempDir, + }, + output: filepath.Join(tempDir, ".config", "gh"), + }, + { + name: "XDG_STATE_HOME specified", + env: map[string]string{ + "XDG_STATE_HOME": tempDir, + }, + output: filepath.Join(tempDir, "gh"), + }, + { + name: "GH_CONFIG_DIR specified", + env: map[string]string{ + "GH_CONFIG_DIR": filepath.Join(tempDir, "gh_config_dir"), + }, + output: filepath.Join(tempDir, "gh_config_dir"), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.env != nil { + for k, v := range tt.env { + old := os.Getenv(k) + os.Setenv(k, v) + defer os.Setenv(k, old) + } + } + + // Create directory to skip auto migration code + // which gets run when target directory does not exist + _ = os.MkdirAll(tt.output, 0755) + + assert.Equal(t, tt.output, StateDir()) + }) + } +} + +func Test_autoMigrateStateDir_noMigration(t *testing.T) { + // homeDir does not have any config files + homeDir := t.TempDir() + migrateDir := t.TempDir() + + homeEnvVar := "HOME" + if runtime.GOOS == "windows" { + homeEnvVar = "USERPROFILE" + } + old := os.Getenv(homeEnvVar) + os.Setenv(homeEnvVar, homeDir) + defer os.Setenv(homeEnvVar, old) + + autoMigrateStateDir(migrateDir) + + files, err := ioutil.ReadDir(migrateDir) + assert.NoError(t, err) + assert.Equal(t, 0, len(files)) +} + +func Test_autoMigrateStateDir_noMigration_samePath(t *testing.T) { + homeDir := t.TempDir() + migrateDir := filepath.Join(homeDir, ".config", "gh") + err := os.MkdirAll(migrateDir, 0755) + assert.NoError(t, err) + + homeEnvVar := "HOME" + if runtime.GOOS == "windows" { + homeEnvVar = "USERPROFILE" + } + old := os.Getenv(homeEnvVar) + os.Setenv(homeEnvVar, homeDir) + defer os.Setenv(homeEnvVar, old) + + autoMigrateStateDir(migrateDir) + + files, err := ioutil.ReadDir(migrateDir) + assert.NoError(t, err) + assert.Equal(t, 0, len(files)) +} + +func Test_autoMigrateStateDir_migration(t *testing.T) { + homeDir := t.TempDir() + migrateDir := t.TempDir() + homeConfigDir := filepath.Join(homeDir, ".config", "gh") + migrateConfigDir := filepath.Join(migrateDir, ".config", "gh") + + homeEnvVar := "HOME" + if runtime.GOOS == "windows" { + homeEnvVar = "USERPROFILE" + } + old := os.Getenv(homeEnvVar) + os.Setenv(homeEnvVar, homeDir) + defer os.Setenv(homeEnvVar, old) + + err := os.MkdirAll(homeConfigDir, 0755) + assert.NoError(t, err) + err = ioutil.WriteFile(filepath.Join(homeConfigDir, "state.yml"), nil, 0755) + assert.NoError(t, err) + + autoMigrateStateDir(migrateConfigDir) + + files, err := ioutil.ReadDir(homeConfigDir) + assert.NoError(t, err) + assert.Equal(t, 0, len(files)) + + files, err = ioutil.ReadDir(migrateConfigDir) + assert.NoError(t, err) + assert.Equal(t, 1, len(files)) + assert.Equal(t, "state.yml", files[0].Name()) +} diff --git a/internal/config/testing.go b/internal/config/testing.go index 1908ac30e..31a5fb2a8 100644 --- a/internal/config/testing.go +++ b/internal/config/testing.go @@ -62,11 +62,3 @@ func stubConfig(main, hosts string) func() { ReadConfigFile = orig } } - -func stubMigrateConfigDir() func() { - orig := migrateConfigDir - migrateConfigDir = func(_, _ string) {} - return func() { - migrateConfigDir = orig - } -} From 602167c0c7345f28148d74fb6d674d3a65938bf5 Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Wed, 26 May 2021 11:28:58 -0400 Subject: [PATCH 02/54] Address PR comments --- internal/config/config_file.go | 71 +++++++++++++++-------------- internal/config/config_file_test.go | 24 ++++++---- 2 files changed, 52 insertions(+), 43 deletions(-) diff --git a/internal/config/config_file.go b/internal/config/config_file.go index b4c611af8..74fe77502 100644 --- a/internal/config/config_file.go +++ b/internal/config/config_file.go @@ -40,34 +40,18 @@ func ConfigDir() string { // If the path does not exist try migrating config from default paths if _, err := os.Stat(path); errors.Is(err, os.ErrNotExist) { - autoMigrateConfigDir(path) + _ = autoMigrateConfigDir(path) } return path } -// Check default paths (os.UserHomeDir, and homedir.Dir) for existing configs -// If configs exist then move them to newPath -// TODO: Remove support for homedir.Dir location in v2 -func autoMigrateConfigDir(newPath string) { - path, err := os.UserHomeDir() - if oldPath := filepath.Join(path, ".config", "gh"); err == nil && dirExists(oldPath) { - migrateDir(oldPath, newPath) - return - } - - path, err = homedir.Dir() - if oldPath := filepath.Join(path, ".config", "gh"); err == nil && dirExists(oldPath) { - migrateDir(oldPath, newPath) - } -} - func StateDir() string { if path := os.Getenv(XDG_STATE_HOME); path != "" { path = filepath.Join(path, "gh") - if _, err := os.Stat(path); errors.Is(err, os.ErrNotExist) { + if !dirExists(path) { _ = os.MkdirAll(path, 0755) - autoMigrateStateDir(path) + _ = autoMigrateStateDir(path) } return path } @@ -75,49 +59,70 @@ func StateDir() string { return ConfigDir() } -// Check default paths (os.UserHomeDir, and homedir.Dir) for existing state file (state.yml) -// If state file exist then move it to newPath +var errSamePath = errors.New("same path") +var errNotExist = errors.New("not exist") + +// Check default paths (os.UserHomeDir, and homedir.Dir) for existing configs +// If configs exist then move them to newPath // TODO: Remove support for homedir.Dir location in v2 -func autoMigrateStateDir(newPath string) { +func autoMigrateConfigDir(newPath string) error { path, err := os.UserHomeDir() if oldPath := filepath.Join(path, ".config", "gh"); err == nil && dirExists(oldPath) { - migrateFile(oldPath, newPath, "state.yml") - return + return migrateDir(oldPath, newPath) } path, err = homedir.Dir() if oldPath := filepath.Join(path, ".config", "gh"); err == nil && dirExists(oldPath) { - migrateFile(oldPath, newPath, "state.yml") + return migrateDir(oldPath, newPath) } + + return errNotExist } -func migrateFile(oldPath, newPath, file string) { +// Check default paths (os.UserHomeDir, and homedir.Dir) for existing state file (state.yml) +// If state file exist then move it to newPath +// TODO: Remove support for homedir.Dir location in v2 +func autoMigrateStateDir(newPath string) error { + path, err := os.UserHomeDir() + if oldPath := filepath.Join(path, ".config", "gh"); err == nil && dirExists(oldPath) { + return migrateFile(oldPath, newPath, "state.yml") + } + + path, err = homedir.Dir() + if oldPath := filepath.Join(path, ".config", "gh"); err == nil && dirExists(oldPath) { + return migrateFile(oldPath, newPath, "state.yml") + } + + return errNotExist +} + +func migrateFile(oldPath, newPath, file string) error { if oldPath == newPath { - return + return errSamePath } oldFile := filepath.Join(oldPath, file) newFile := filepath.Join(newPath, file) if !fileExists(oldFile) { - return + return errNotExist } _ = os.MkdirAll(filepath.Dir(newFile), 0755) - _ = os.Rename(oldFile, newFile) + return os.Rename(oldFile, newFile) } -func migrateDir(oldPath, newPath string) { +func migrateDir(oldPath, newPath string) error { if oldPath == newPath { - return + return errSamePath } if !dirExists(oldPath) { - return + return errNotExist } _ = os.MkdirAll(filepath.Dir(newPath), 0755) - _ = os.Rename(oldPath, newPath) + return os.Rename(oldPath, newPath) } func dirExists(path string) bool { diff --git a/internal/config/config_file_test.go b/internal/config/config_file_test.go index c54727332..ff4cd1b04 100644 --- a/internal/config/config_file_test.go +++ b/internal/config/config_file_test.go @@ -269,8 +269,7 @@ func Test_configFile_Write_toDisk(t *testing.T) { } } -func Test_autoMigrateConfigDir_noMigration(t *testing.T) { - // homeDir does not have any config files +func Test_autoMigrateConfigDir_noMigration_notExist(t *testing.T) { homeDir := t.TempDir() migrateDir := t.TempDir() @@ -282,7 +281,8 @@ func Test_autoMigrateConfigDir_noMigration(t *testing.T) { os.Setenv(homeEnvVar, homeDir) defer os.Setenv(homeEnvVar, old) - autoMigrateConfigDir(migrateDir) + err := autoMigrateConfigDir(migrateDir) + assert.Equal(t, errNotExist, err) files, err := ioutil.ReadDir(migrateDir) assert.NoError(t, err) @@ -303,7 +303,8 @@ func Test_autoMigrateConfigDir_noMigration_samePath(t *testing.T) { os.Setenv(homeEnvVar, homeDir) defer os.Setenv(homeEnvVar, old) - autoMigrateConfigDir(migrateDir) + err = autoMigrateConfigDir(migrateDir) + assert.Equal(t, errSamePath, err) files, err := ioutil.ReadDir(migrateDir) assert.NoError(t, err) @@ -330,7 +331,8 @@ func Test_autoMigrateConfigDir_migration(t *testing.T) { assert.NoError(t, err) f.Close() - autoMigrateConfigDir(migrateConfigDir) + err = autoMigrateConfigDir(migrateConfigDir) + assert.NoError(t, err) _, err = ioutil.ReadDir(homeConfigDir) assert.True(t, os.IsNotExist(err)) @@ -395,8 +397,7 @@ func Test_StateDir(t *testing.T) { } } -func Test_autoMigrateStateDir_noMigration(t *testing.T) { - // homeDir does not have any config files +func Test_autoMigrateStateDir_noMigration_notExist(t *testing.T) { homeDir := t.TempDir() migrateDir := t.TempDir() @@ -408,7 +409,8 @@ func Test_autoMigrateStateDir_noMigration(t *testing.T) { os.Setenv(homeEnvVar, homeDir) defer os.Setenv(homeEnvVar, old) - autoMigrateStateDir(migrateDir) + err := autoMigrateStateDir(migrateDir) + assert.Equal(t, errNotExist, err) files, err := ioutil.ReadDir(migrateDir) assert.NoError(t, err) @@ -429,7 +431,8 @@ func Test_autoMigrateStateDir_noMigration_samePath(t *testing.T) { os.Setenv(homeEnvVar, homeDir) defer os.Setenv(homeEnvVar, old) - autoMigrateStateDir(migrateDir) + err = autoMigrateStateDir(migrateDir) + assert.Equal(t, errSamePath, err) files, err := ioutil.ReadDir(migrateDir) assert.NoError(t, err) @@ -455,7 +458,8 @@ func Test_autoMigrateStateDir_migration(t *testing.T) { err = ioutil.WriteFile(filepath.Join(homeConfigDir, "state.yml"), nil, 0755) assert.NoError(t, err) - autoMigrateStateDir(migrateConfigDir) + err = autoMigrateStateDir(migrateConfigDir) + assert.NoError(t, err) files, err := ioutil.ReadDir(homeConfigDir) assert.NoError(t, err) From 1a980e768cdad166b30e8c3ae706c99c6c709bd6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 28 May 2021 14:32:31 +0200 Subject: [PATCH 03/54] Fix how teams are displayed in requested reviewers 1. The `--json` export now only renders the `login` field for User types and `name` and `slug` fields for Team types. 2. The `pr view` command now renders team reviewers in the format of `ORG/SLUG` instead of the team name. This is so that the same value can be used in the `pr create -r` flag. --- api/export_pr.go | 16 +++++++-- api/queries_pr.go | 35 +++++++++++-------- .../prViewPreviewWithReviewersByNumber.json | 10 +++--- pkg/cmd/pr/view/view.go | 7 +--- pkg/cmd/pr/view/view_test.go | 4 +-- 5 files changed, 42 insertions(+), 30 deletions(-) diff --git a/api/export_pr.go b/api/export_pr.go index 4bd0aabc7..29a5c4a63 100644 --- a/api/export_pr.go +++ b/api/export_pr.go @@ -80,10 +80,20 @@ func (pr *PullRequest) ExportData(fields []string) *map[string]interface{} { case "reviewRequests": requests := make([]interface{}, 0, len(pr.ReviewRequests.Nodes)) for _, req := range pr.ReviewRequests.Nodes { - if req.RequestedReviewer.TypeName == "" { - continue + r := req.RequestedReviewer + switch r.TypeName { + case "User": + requests = append(requests, map[string]string{ + "__typename": r.TypeName, + "login": r.Login, + }) + case "Team": + requests = append(requests, map[string]string{ + "__typename": r.TypeName, + "name": r.Name, + "slug": r.LoginOrSlug(), + }) } - requests = append(requests, req.RequestedReviewer) } data[f] = &requests default: diff --git a/api/queries_pr.go b/api/queries_pr.go index 364784f9f..3032f6364 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -150,28 +150,33 @@ type PullRequestFile struct { type ReviewRequests struct { Nodes []struct { - RequestedReviewer struct { - TypeName string `json:"__typename"` - Login string `json:"login"` - Name string `json:"name"` - Slug string `json:"slug"` - Organization struct { - Login string `json:"login"` - } - } + RequestedReviewer RequestedReviewer } } +type RequestedReviewer struct { + TypeName string `json:"__typename"` + Login string `json:"login"` + Name string `json:"name"` + Slug string `json:"slug"` + Organization struct { + Login string `json:"login"` + } `json:"organization"` +} + +func (r RequestedReviewer) LoginOrSlug() string { + if r.TypeName == teamTypeName { + return fmt.Sprintf("%s/%s", r.Organization.Login, r.Slug) + } + return r.Login +} + const teamTypeName = "Team" func (r ReviewRequests) Logins() []string { logins := make([]string, len(r.Nodes)) - for i, a := range r.Nodes { - if a.RequestedReviewer.TypeName == teamTypeName { - logins[i] = fmt.Sprintf("%s/%s", a.RequestedReviewer.Organization.Login, a.RequestedReviewer.Slug) - } else { - logins[i] = a.RequestedReviewer.Login - } + for i, r := range r.Nodes { + logins[i] = r.RequestedReviewer.LoginOrSlug() } return logins } diff --git a/pkg/cmd/pr/view/fixtures/prViewPreviewWithReviewersByNumber.json b/pkg/cmd/pr/view/fixtures/prViewPreviewWithReviewersByNumber.json index 59b1ed1a7..1b1a1e304 100644 --- a/pkg/cmd/pr/view/fixtures/prViewPreviewWithReviewersByNumber.json +++ b/pkg/cmd/pr/view/fixtures/prViewPreviewWithReviewersByNumber.json @@ -21,10 +21,12 @@ } }, { - "requestedReviewer": { - "__typename": "Team", - "name": "Team 1" - } + "requestedReviewer": { + "__typename": "Team", + "name": "Team 1", + "slug": "team-1", + "organization": {"login": "my-org"} + } }, { "requestedReviewer": { diff --git a/pkg/cmd/pr/view/view.go b/pkg/cmd/pr/view/view.go index 91087d74b..702763bef 100644 --- a/pkg/cmd/pr/view/view.go +++ b/pkg/cmd/pr/view/view.go @@ -294,8 +294,6 @@ func prReviewerList(pr api.PullRequest, cs *iostreams.ColorScheme) string { return reviewerList } -const teamTypeName = "Team" - const ghostName = "ghost" // parseReviewers parses given Reviews and ReviewRequests @@ -317,10 +315,7 @@ func parseReviewers(pr api.PullRequest) []*reviewerState { // Overwrite reviewer's state if a review request for the same reviewer exists. for _, reviewRequest := range pr.ReviewRequests.Nodes { - name := reviewRequest.RequestedReviewer.Login - if reviewRequest.RequestedReviewer.TypeName == teamTypeName { - name = reviewRequest.RequestedReviewer.Name - } + name := reviewRequest.RequestedReviewer.LoginOrSlug() reviewerStates[name] = &reviewerState{ Name: name, State: requestedReviewState, diff --git a/pkg/cmd/pr/view/view_test.go b/pkg/cmd/pr/view/view_test.go index 78d588f1e..ed844db50 100644 --- a/pkg/cmd/pr/view/view_test.go +++ b/pkg/cmd/pr/view/view_test.go @@ -250,7 +250,7 @@ func TestPRView_Preview_nontty(t *testing.T) { `milestone:\t\n`, `additions:\t100\n`, `deletions:\t10\n`, - `reviewers:\tDEF \(Commented\), def \(Changes requested\), ghost \(Approved\), hubot \(Commented\), xyz \(Approved\), 123 \(Requested\), Team 1 \(Requested\), abc \(Requested\)\n`, + `reviewers:\tDEF \(Commented\), def \(Changes requested\), ghost \(Approved\), hubot \(Commented\), xyz \(Approved\), 123 \(Requested\), abc \(Requested\), my-org\/team-1 \(Requested\)\n`, `\*\*blueberries taste good\*\*`, }, }, @@ -383,7 +383,7 @@ func TestPRView_Preview(t *testing.T) { }, expectedOutputs: []string{ `Blueberries are from a fork`, - `Reviewers:.*DEF \(.*Commented.*\), def \(.*Changes requested.*\), ghost \(.*Approved.*\), hubot \(Commented\), xyz \(.*Approved.*\), 123 \(.*Requested.*\), Team 1 \(.*Requested.*\), abc \(.*Requested.*\)\n`, + `Reviewers: DEF \(Commented\), def \(Changes requested\), ghost \(Approved\), hubot \(Commented\), xyz \(Approved\), 123 \(Requested\), abc \(Requested\), my-org\/team-1 \(Requested\)`, `blueberries taste good`, `View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12`, }, From 3943a8bb1fca0c2356f145e381e2dc4bf179120c Mon Sep 17 00:00:00 2001 From: jack1142 <6032823+jack1142@users.noreply.github.com> Date: Fri, 28 May 2021 18:53:31 +0200 Subject: [PATCH 04/54] Update PR tty view formatting and its tests --- pkg/cmd/pr/view/view.go | 2 +- pkg/cmd/pr/view/view_test.go | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/pkg/cmd/pr/view/view.go b/pkg/cmd/pr/view/view.go index 91087d74b..a37568741 100644 --- a/pkg/cmd/pr/view/view.go +++ b/pkg/cmd/pr/view/view.go @@ -170,7 +170,7 @@ func printHumanPrPreview(opts *ViewOptions, pr *api.PullRequest) error { cs := opts.IO.ColorScheme() // Header (Title and State) - fmt.Fprintln(out, cs.Bold(pr.Title)) + fmt.Fprintf(out, "%s #%d", cs.Bold(pr.Title), pr.Number) fmt.Fprintf(out, "%s • %s wants to merge %s into %s from %s • %s %s \n", shared.StateTitleWithColor(cs, *pr), diff --git a/pkg/cmd/pr/view/view_test.go b/pkg/cmd/pr/view/view_test.go index 78d588f1e..82a4a4e95 100644 --- a/pkg/cmd/pr/view/view_test.go +++ b/pkg/cmd/pr/view/view_test.go @@ -350,7 +350,7 @@ func TestPRView_Preview(t *testing.T) { "PullRequestByNumber": "./fixtures/prViewPreview.json", }, expectedOutputs: []string{ - `Blueberries are from a fork`, + `Blueberries are from a fork #12`, `Open.*nobody wants to merge 12 commits into master from blueberries.+100.-10`, `blueberries taste good`, `View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12`, @@ -363,7 +363,7 @@ func TestPRView_Preview(t *testing.T) { "PullRequestByNumber": "./fixtures/prViewPreviewWithMetadataByNumber.json", }, expectedOutputs: []string{ - `Blueberries are from a fork`, + `Blueberries are from a fork #12`, `Open.*nobody wants to merge 12 commits into master from blueberries.+100.-10`, `Reviewers:.*1 \(.*Requested.*\)\n`, `Assignees:.*marseilles, monaco\n`, @@ -382,7 +382,7 @@ func TestPRView_Preview(t *testing.T) { "ReviewsForPullRequest": "./fixtures/prViewPreviewManyReviews.json", }, expectedOutputs: []string{ - `Blueberries are from a fork`, + `Blueberries are from a fork #12`, `Reviewers:.*DEF \(.*Commented.*\), def \(.*Changes requested.*\), ghost \(.*Approved.*\), hubot \(Commented\), xyz \(.*Approved.*\), 123 \(.*Requested.*\), Team 1 \(.*Requested.*\), abc \(.*Requested.*\)\n`, `blueberries taste good`, `View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12`, @@ -395,7 +395,7 @@ func TestPRView_Preview(t *testing.T) { "PullRequestByNumber": "./fixtures/prViewPreviewClosedState.json", }, expectedOutputs: []string{ - `Blueberries are from a fork`, + `Blueberries are from a fork #12`, `Closed.*nobody wants to merge 12 commits into master from blueberries.+100.-10`, `blueberries taste good`, `View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12`, @@ -408,7 +408,7 @@ func TestPRView_Preview(t *testing.T) { "PullRequestByNumber": "./fixtures/prViewPreviewMergedState.json", }, expectedOutputs: []string{ - `Blueberries are from a fork`, + `Blueberries are from a fork #12`, `Merged.*nobody wants to merge 12 commits into master from blueberries.+100.-10`, `blueberries taste good`, `View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12`, @@ -421,7 +421,7 @@ func TestPRView_Preview(t *testing.T) { "PullRequestByNumber": "./fixtures/prViewPreviewDraftState.json", }, expectedOutputs: []string{ - `Blueberries are from a fork`, + `Blueberries are from a fork #12`, `Draft.*nobody wants to merge 12 commits into master from blueberries.+100.-10`, `blueberries taste good`, `View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12`, @@ -501,7 +501,7 @@ func TestPRView_tty_Comments(t *testing.T) { "ReviewsForPullRequest": "./fixtures/prViewPreviewReviews.json", }, expectedOutputs: []string{ - `some title`, + `some title #12`, `1 \x{1f615} • 2 \x{1f440} • 3 \x{2764}\x{fe0f}`, `some body`, `———————— Not showing 9 comments ————————`, @@ -521,7 +521,7 @@ func TestPRView_tty_Comments(t *testing.T) { "CommentsForPullRequest": "./fixtures/prViewPreviewFullComments.json", }, expectedOutputs: []string{ - `some title`, + `some title #12`, `some body`, `monalisa • Jan 1, 2020 • Edited`, `1 \x{1f615} • 2 \x{1f440} • 3 \x{2764}\x{fe0f} • 4 \x{1f389} • 5 \x{1f604} • 6 \x{1f680} • 7 \x{1f44e} • 8 \x{1f44d}`, From 979ec9298d7d88f84ac84b1bf753a4b296262923 Mon Sep 17 00:00:00 2001 From: jack1142 <6032823+jack1142@users.noreply.github.com> Date: Sat, 29 May 2021 04:43:34 +0200 Subject: [PATCH 05/54] Update issue tty view formatting and its tests --- pkg/cmd/issue/view/view.go | 2 +- pkg/cmd/issue/view/view_test.go | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/pkg/cmd/issue/view/view.go b/pkg/cmd/issue/view/view.go index 78be7df09..21fa76001 100644 --- a/pkg/cmd/issue/view/view.go +++ b/pkg/cmd/issue/view/view.go @@ -172,7 +172,7 @@ func printHumanIssuePreview(opts *ViewOptions, issue *api.Issue) error { cs := opts.IO.ColorScheme() // Header (Title and State) - fmt.Fprintln(out, cs.Bold(issue.Title)) + fmt.Fprintf(out, "%s #%d", cs.Bold(issue.Title), issue.Number) fmt.Fprintf(out, "%s • %s opened %s • %s\n", issueStateTitleWithColor(cs, issue.State), diff --git a/pkg/cmd/issue/view/view_test.go b/pkg/cmd/issue/view/view_test.go index dc5c5dfb5..f10ab1677 100644 --- a/pkg/cmd/issue/view/view_test.go +++ b/pkg/cmd/issue/view/view_test.go @@ -178,7 +178,7 @@ func TestIssueView_tty_Preview(t *testing.T) { "Open issue without metadata": { fixture: "./fixtures/issueView_preview.json", expectedOutputs: []string{ - `ix of coins`, + `ix of coins #123`, `Open.*marseilles opened about 9 years ago.*9 comments`, `bold story`, `View this issue on GitHub: https://github.com/OWNER/REPO/issues/123`, @@ -187,7 +187,7 @@ func TestIssueView_tty_Preview(t *testing.T) { "Open issue with metadata": { fixture: "./fixtures/issueView_previewWithMetadata.json", expectedOutputs: []string{ - `ix of coins`, + `ix of coins #123`, `Open.*marseilles opened about 9 years ago.*9 comments`, `8 \x{1f615} • 7 \x{1f440} • 6 \x{2764}\x{fe0f} • 5 \x{1f389} • 4 \x{1f604} • 3 \x{1f680} • 2 \x{1f44e} • 1 \x{1f44d}`, `Assignees:.*marseilles, monaco\n`, @@ -201,7 +201,7 @@ func TestIssueView_tty_Preview(t *testing.T) { "Open issue with empty body": { fixture: "./fixtures/issueView_previewWithEmptyBody.json", expectedOutputs: []string{ - `ix of coins`, + `ix of coins #123`, `Open.*marseilles opened about 9 years ago.*9 comments`, `No description provided`, `View this issue on GitHub: https://github.com/OWNER/REPO/issues/123`, @@ -210,7 +210,7 @@ func TestIssueView_tty_Preview(t *testing.T) { "Closed issue": { fixture: "./fixtures/issueView_previewClosedState.json", expectedOutputs: []string{ - `ix of coins`, + `ix of coins #123`, `Closed.*marseilles opened about 9 years ago.*9 comments`, `bold story`, `View this issue on GitHub: https://github.com/OWNER/REPO/issues/123`, @@ -310,7 +310,7 @@ func TestIssueView_tty_Comments(t *testing.T) { "IssueByNumber": "./fixtures/issueView_previewSingleComment.json", }, expectedOutputs: []string{ - `some title`, + `some title #123`, `some body`, `———————— Not showing 5 comments ————————`, `marseilles \(Collaborator\) • Jan 1, 2020 • Newest comment`, @@ -326,7 +326,7 @@ func TestIssueView_tty_Comments(t *testing.T) { "CommentsForIssue": "./fixtures/issueView_previewFullComments.json", }, expectedOutputs: []string{ - `some title`, + `some title #123`, `some body`, `monalisa • Jan 1, 2020 • Edited`, `1 \x{1f615} • 2 \x{1f440} • 3 \x{2764}\x{fe0f} • 4 \x{1f389} • 5 \x{1f604} • 6 \x{1f680} • 7 \x{1f44e} • 8 \x{1f44d}`, From 42333bb2d1cb11a3d224d11aa1c15bf298382ad2 Mon Sep 17 00:00:00 2001 From: jack1142 <6032823+jack1142@users.noreply.github.com> Date: Sat, 29 May 2021 04:43:58 +0200 Subject: [PATCH 06/54] Update issue non-tty view formatting and its tests --- pkg/cmd/issue/view/view.go | 1 + pkg/cmd/issue/view/view_test.go | 5 +++++ 2 files changed, 6 insertions(+) diff --git a/pkg/cmd/issue/view/view.go b/pkg/cmd/issue/view/view.go index 21fa76001..87c8e3143 100644 --- a/pkg/cmd/issue/view/view.go +++ b/pkg/cmd/issue/view/view.go @@ -160,6 +160,7 @@ func printRawIssuePreview(out io.Writer, issue *api.Issue) error { milestoneTitle = issue.Milestone.Title } fmt.Fprintf(out, "milestone:\t%s\n", milestoneTitle) + fmt.Fprintf(out, "number:\t%d\n", issue.Number) fmt.Fprintln(out, "--") fmt.Fprintln(out, issue.Body) return nil diff --git a/pkg/cmd/issue/view/view_test.go b/pkg/cmd/issue/view/view_test.go index f10ab1677..01ec824fc 100644 --- a/pkg/cmd/issue/view/view_test.go +++ b/pkg/cmd/issue/view/view_test.go @@ -112,6 +112,7 @@ func TestIssueView_nontty_Preview(t *testing.T) { `comments:\t9`, `author:\tmarseilles`, `assignees:`, + `number:\t123\n`, `\*\*bold story\*\*`, }, }, @@ -126,6 +127,7 @@ func TestIssueView_nontty_Preview(t *testing.T) { `labels:\tone, two, three, four, five`, `projects:\tProject 1 \(column A\), Project 2 \(column B\), Project 3 \(column C\), Project 4 \(Awaiting triage\)\n`, `milestone:\tuluru\n`, + `number:\t123\n`, `\*\*bold story\*\*`, }, }, @@ -136,6 +138,7 @@ func TestIssueView_nontty_Preview(t *testing.T) { `state:\tOPEN`, `author:\tmarseilles`, `labels:\ttarot`, + `number:\t123\n`, }, }, "Closed issue": { @@ -146,6 +149,7 @@ func TestIssueView_nontty_Preview(t *testing.T) { `\*\*bold story\*\*`, `author:\tmarseilles`, `labels:\ttarot`, + `number:\t123\n`, `\*\*bold story\*\*`, }, }, @@ -386,6 +390,7 @@ func TestIssueView_nontty_Comments(t *testing.T) { `state:\tOPEN`, `author:\tmarseilles`, `comments:\t6`, + `number:\t123`, `some body`, }, }, From b51403391eeac9f95f519dc80bce90c98500bfff Mon Sep 17 00:00:00 2001 From: itsme-alan <75578443+itsme-alan@users.noreply.github.com> Date: Mon, 31 May 2021 13:22:35 +0530 Subject: [PATCH 07/54] Update upgrade docs for winget --- README.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/README.md b/README.md index 6255b5d81..2886e3471 100644 --- a/README.md +++ b/README.md @@ -49,9 +49,7 @@ For more information and distro-specific instructions, see the [Linux installati | Install: | Upgrade: | | ------------------- | --------------------| -| `winget install gh` | `winget install gh` | - -WinGet does not have a specialized `upgrade` command yet, but the `install` command should work for upgrading to a newer version of GitHub CLI. +| `winget install gh` | `winget upgrade gh` | #### scoop From cb605387091ab70dd50cdc250f6ed460f439c7fc Mon Sep 17 00:00:00 2001 From: Gowtham Munukutla Date: Sat, 22 May 2021 13:57:40 +0530 Subject: [PATCH 08/54] paginate to get all secrets at once --- pkg/cmd/secret/list/list.go | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/pkg/cmd/secret/list/list.go b/pkg/cmd/secret/list/list.go index e8694c62c..e512295ac 100644 --- a/pkg/cmd/secret/list/list.go +++ b/pkg/cmd/secret/list/list.go @@ -179,12 +179,24 @@ type secretsPayload struct { } func getSecrets(client *api.Client, host, path string) ([]*Secret, error) { - result := secretsPayload{} + results := secretsPayload{} - err := client.REST(host, "GET", path, nil, &result) - if err != nil { - return nil, err + perPage := 100 + page := 1 + + for { + result := secretsPayload{} + err := client.REST(host, "GET", fmt.Sprintf("%s?per_page=%d&page=%d", path, perPage, page), nil, &result) + if err != nil { + return nil, err + } + results.Secrets = append(results.Secrets, result.Secrets...) + if len(result.Secrets) == 0 || len(result.Secrets) < 100 { + break + } + + page++ } - return result.Secrets, nil + return results.Secrets, nil } From 260f720c0738c403132f10477a1565cd57477c5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 31 May 2021 19:00:44 +0200 Subject: [PATCH 09/54] :nail_care: refactor and add tests for Secrets pagination --- pkg/cmd/secret/list/list.go | 72 +++++++++++++++++++++++++------- pkg/cmd/secret/list/list_test.go | 31 ++++++++++++++ 2 files changed, 87 insertions(+), 16 deletions(-) diff --git a/pkg/cmd/secret/list/list.go b/pkg/cmd/secret/list/list.go index e512295ac..97c3ac27a 100644 --- a/pkg/cmd/secret/list/list.go +++ b/pkg/cmd/secret/list/list.go @@ -1,13 +1,16 @@ package list import ( + "encoding/json" "fmt" "net/http" + "regexp" "strings" "time" "github.com/cli/cli/api" "github.com/cli/cli/internal/config" + "github.com/cli/cli/internal/ghinstance" "github.com/cli/cli/internal/ghrepo" "github.com/cli/cli/pkg/cmd/secret/shared" "github.com/cli/cli/pkg/cmdutil" @@ -55,11 +58,10 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman } func listRun(opts *ListOptions) error { - c, err := opts.HttpClient() + client, err := opts.HttpClient() if err != nil { return fmt.Errorf("could not create http client: %w", err) } - client := api.NewClientFromHTTP(c) orgName := opts.OrgName @@ -145,7 +147,7 @@ func fmtVisibility(s Secret) string { return "" } -func getOrgSecrets(client *api.Client, host, orgName string) ([]*Secret, error) { +func getOrgSecrets(client httpClient, host, orgName string) ([]*Secret, error) { secrets, err := getSecrets(client, host, fmt.Sprintf("orgs/%s/actions/secrets", orgName)) if err != nil { return nil, err @@ -160,7 +162,7 @@ func getOrgSecrets(client *api.Client, host, orgName string) ([]*Secret, error) continue } var result responseData - if err := client.REST(host, "GET", secret.SelectedReposURL, nil, &result); err != nil { + if _, err := apiGet(client, secret.SelectedReposURL, &result); err != nil { return nil, fmt.Errorf("failed determining selected repositories for %s: %w", secret.Name, err) } secret.NumSelectedRepos = result.TotalCount @@ -169,7 +171,7 @@ func getOrgSecrets(client *api.Client, host, orgName string) ([]*Secret, error) return secrets, nil } -func getRepoSecrets(client *api.Client, repo ghrepo.Interface) ([]*Secret, error) { +func getRepoSecrets(client httpClient, repo ghrepo.Interface) ([]*Secret, error) { return getSecrets(client, repo.RepoHost(), fmt.Sprintf("repos/%s/actions/secrets", ghrepo.FullName(repo))) } @@ -178,25 +180,63 @@ type secretsPayload struct { Secrets []*Secret } -func getSecrets(client *api.Client, host, path string) ([]*Secret, error) { - results := secretsPayload{} +type httpClient interface { + Do(*http.Request) (*http.Response, error) +} - perPage := 100 - page := 1 +func getSecrets(client httpClient, host, path string) ([]*Secret, error) { + var results []*Secret + url := fmt.Sprintf("%s%s?per_page=100", ghinstance.RESTPrefix(host), path) for { - result := secretsPayload{} - err := client.REST(host, "GET", fmt.Sprintf("%s?per_page=%d&page=%d", path, perPage, page), nil, &result) + var payload secretsPayload + nextURL, err := apiGet(client, url, &payload) if err != nil { return nil, err } - results.Secrets = append(results.Secrets, result.Secrets...) - if len(result.Secrets) == 0 || len(result.Secrets) < 100 { + results = append(results, payload.Secrets...) + + if nextURL == "" { break } - - page++ + url = nextURL } - return results.Secrets, nil + return results, nil +} + +func apiGet(client httpClient, url string, data interface{}) (string, error) { + req, err := http.NewRequest("GET", url, nil) + if err != nil { + return "", err + } + req.Header.Set("Content-Type", "application/json; charset=utf-8") + + resp, err := client.Do(req) + if err != nil { + return "", err + } + defer resp.Body.Close() + + if resp.StatusCode > 299 { + return "", api.HandleHTTPError(resp) + } + + dec := json.NewDecoder(resp.Body) + if err := dec.Decode(data); err != nil { + return "", err + } + + return findNextPage(resp.Header.Get("Link")), nil +} + +var linkRE = regexp.MustCompile(`<([^>]+)>;\s*rel="([^"]+)"`) + +func findNextPage(link string) string { + for _, m := range linkRE.FindAllStringSubmatch(link, -1) { + if len(m) >= 2 && m[2] == "next" { + return m[1] + } + } + return "" } diff --git a/pkg/cmd/secret/list/list_test.go b/pkg/cmd/secret/list/list_test.go index 8620ca012..e3975b1e6 100644 --- a/pkg/cmd/secret/list/list_test.go +++ b/pkg/cmd/secret/list/list_test.go @@ -3,7 +3,9 @@ package list import ( "bytes" "fmt" + "io/ioutil" "net/http" + "strings" "testing" "time" @@ -200,3 +202,32 @@ func Test_listRun(t *testing.T) { }) } } + +func Test_getSecrets_pagination(t *testing.T) { + var requests []*http.Request + var client testClient = func(req *http.Request) (*http.Response, error) { + header := make(map[string][]string) + if len(requests) == 0 { + header["Link"] = []string{`; rel="previous", ; rel="next"`} + } + requests = append(requests, req) + return &http.Response{ + Request: req, + Body: ioutil.NopCloser(strings.NewReader(`{"secrets":[{},{}]}`)), + Header: header, + }, nil + } + + secrets, err := getSecrets(client, "github.com", "path/to") + assert.NoError(t, err) + assert.Equal(t, 2, len(requests)) + assert.Equal(t, 4, len(secrets)) + assert.Equal(t, "https://api.github.com/path/to?per_page=100", requests[0].URL.String()) + assert.Equal(t, "http://example.com/page/2", requests[1].URL.String()) +} + +type testClient func(*http.Request) (*http.Response, error) + +func (c testClient) Do(req *http.Request) (*http.Response, error) { + return c(req) +} From e5bdaaab2c9a7e5efbe39c24e356aace40c60a8b Mon Sep 17 00:00:00 2001 From: Tyler Wright Date: Mon, 31 May 2021 21:13:13 +0200 Subject: [PATCH 10/54] Add ability to list environment secrets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Mislav Marohnić --- pkg/cmd/secret/list/list.go | 20 ++++++++++++++-- pkg/cmd/secret/list/list_test.go | 39 ++++++++++++++++++++++++++++++-- pkg/cmd/secret/secret.go | 4 ++-- 3 files changed, 57 insertions(+), 6 deletions(-) diff --git a/pkg/cmd/secret/list/list.go b/pkg/cmd/secret/list/list.go index 97c3ac27a..35d8db675 100644 --- a/pkg/cmd/secret/list/list.go +++ b/pkg/cmd/secret/list/list.go @@ -26,6 +26,7 @@ type ListOptions struct { BaseRepo func() (ghrepo.Interface, error) OrgName string + EnvName string } func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Command { @@ -38,12 +39,16 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman cmd := &cobra.Command{ Use: "list", Short: "List secrets", - Long: "List secrets for a repository or organization", + Long: "List secrets for a repository, environment, or organization", Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { // support `-R, --repo` override opts.BaseRepo = f.BaseRepo + if err := cmdutil.MutuallyExclusive("specify only one of `--org` or `--env`", opts.OrgName != "", opts.EnvName != ""); err != nil { + return err + } + if runF != nil { return runF(opts) } @@ -53,6 +58,7 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman } cmd.Flags().StringVarP(&opts.OrgName, "org", "o", "", "List secrets for an organization") + cmd.Flags().StringVarP(&opts.EnvName, "env", "e", "", "List secrets for an environment") return cmd } @@ -64,6 +70,7 @@ func listRun(opts *ListOptions) error { } orgName := opts.OrgName + envName := opts.EnvName var baseRepo ghrepo.Interface if orgName == "" { @@ -75,7 +82,11 @@ func listRun(opts *ListOptions) error { var secrets []*Secret if orgName == "" { - secrets, err = getRepoSecrets(client, baseRepo) + if envName == "" { + secrets, err = getRepoSecrets(client, baseRepo) + } else { + secrets, err = getEnvSecrets(client, baseRepo, envName) + } } else { var cfg config.Config var host string @@ -171,6 +182,11 @@ func getOrgSecrets(client httpClient, host, orgName string) ([]*Secret, error) { return secrets, nil } +func getEnvSecrets(client httpClient, repo ghrepo.Interface, envName string) ([]*Secret, error) { + path := fmt.Sprintf("repos/%s/environments/%s/secrets", ghrepo.FullName(repo), envName) + return getSecrets(client, repo.RepoHost(), path) +} + func getRepoSecrets(client httpClient, repo ghrepo.Interface) ([]*Secret, error) { return getSecrets(client, repo.RepoHost(), fmt.Sprintf("repos/%s/actions/secrets", ghrepo.FullName(repo))) diff --git a/pkg/cmd/secret/list/list_test.go b/pkg/cmd/secret/list/list_test.go index e3975b1e6..42504a961 100644 --- a/pkg/cmd/secret/list/list_test.go +++ b/pkg/cmd/secret/list/list_test.go @@ -40,6 +40,13 @@ func Test_NewCmdList(t *testing.T) { OrgName: "UmbrellaCorporation", }, }, + { + name: "env", + cli: "-eDevelopment", + wants: ListOptions{ + EnvName: "Development", + }, + }, } for _, tt := range tests { @@ -66,7 +73,7 @@ func Test_NewCmdList(t *testing.T) { assert.NoError(t, err) assert.Equal(t, tt.wants.OrgName, gotOpts.OrgName) - + assert.Equal(t, tt.wants.EnvName, gotOpts.EnvName) }) } } @@ -122,16 +129,44 @@ func Test_listRun(t *testing.T) { "SECRET_THREE\t1975-11-30\tSELECTED", }, }, + { + name: "env tty", + tty: true, + opts: &ListOptions{ + EnvName: "Development", + }, + wantOut: []string{ + "SECRET_ONE.*Updated 1988-10-11", + "SECRET_TWO.*Updated 2020-12-04", + "SECRET_THREE.*Updated 1975-11-30", + }, + }, + { + name: "env not tty", + tty: false, + opts: &ListOptions{ + EnvName: "Development", + }, + wantOut: []string{ + "SECRET_ONE\t1988-10-11", + "SECRET_TWO\t2020-12-04", + "SECRET_THREE\t1975-11-30", + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { reg := &httpmock.Registry{} + path := "repos/owner/repo/actions/secrets" + if tt.opts.EnvName != "" { + path = fmt.Sprintf("repos/owner/repo/environments/%s/secrets", tt.opts.EnvName) + } + t0, _ := time.Parse("2006-01-02", "1988-10-11") t1, _ := time.Parse("2006-01-02", "2020-12-04") t2, _ := time.Parse("2006-01-02", "1975-11-30") - path := "repos/owner/repo/actions/secrets" payload := secretsPayload{} payload.Secrets = []*Secret{ { diff --git a/pkg/cmd/secret/secret.go b/pkg/cmd/secret/secret.go index e8b82a41d..dc04fc85c 100644 --- a/pkg/cmd/secret/secret.go +++ b/pkg/cmd/secret/secret.go @@ -15,8 +15,8 @@ func NewCmdSecret(f *cmdutil.Factory) *cobra.Command { Use: "secret ", Short: "Manage GitHub secrets", Long: heredoc.Doc(` - Secrets can be set at the repository or organization level for use in GitHub Actions. - Run "gh help secret set" to learn how to get started. + Secrets can be set at the repository, environment, or organization level for use in + GitHub Actions. Run "gh help secret set" to learn how to get started. `), } From fce93d60809ba97487b1268de1cd3977b59034fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 28 May 2021 19:54:22 +0200 Subject: [PATCH 11/54] Experimental command extensions support Extensions are looked up as `~/.config/gh/extensions/gh-*`. Additionally, any executables found in PATH named `gh-*` are available as `gh `. --- cmd/gh/main.go | 19 ++++- pkg/cmd/extensions/command.go | 69 ++++++++++++++++ pkg/cmd/extensions/manager.go | 150 ++++++++++++++++++++++++++++++++++ pkg/cmd/root/root.go | 2 + 4 files changed, 237 insertions(+), 3 deletions(-) create mode 100644 pkg/cmd/extensions/command.go create mode 100644 pkg/cmd/extensions/manager.go diff --git a/cmd/gh/main.go b/cmd/gh/main.go index b9dc52858..874c2e715 100644 --- a/cmd/gh/main.go +++ b/cmd/gh/main.go @@ -21,6 +21,7 @@ import ( "github.com/cli/cli/internal/run" "github.com/cli/cli/internal/update" "github.com/cli/cli/pkg/cmd/alias/expand" + "github.com/cli/cli/pkg/cmd/extensions" "github.com/cli/cli/pkg/cmd/factory" "github.com/cli/cli/pkg/cmd/root" "github.com/cli/cli/pkg/cmdutil" @@ -140,15 +141,27 @@ func mainRun() exitCode { err = preparedCmd.Run() if err != nil { - if ee, ok := err.(*exec.ExitError); ok { - return exitCode(ee.ExitCode()) + var execError *exec.ExitError + if errors.As(err, &execError) { + return exitCode(execError.ExitCode()) } - fmt.Fprintf(stderr, "failed to run external command: %s", err) return exitError } return exitOK + } else if c, _, err := rootCmd.Traverse(expandedArgs); err == nil && c == rootCmd && len(expandedArgs) > 0 { + extensionManager := extensions.NewManager() + if found, err := extensionManager.Dispatch(expandedArgs, os.Stdin, os.Stdout, os.Stderr); err != nil { + var execError *exec.ExitError + if errors.As(err, &execError) { + return exitCode(execError.ExitCode()) + } + fmt.Fprintf(stderr, "failed to run extension: %s", err) + return exitError + } else if found { + return exitOK + } } } diff --git a/pkg/cmd/extensions/command.go b/pkg/cmd/extensions/command.go new file mode 100644 index 000000000..5794a3806 --- /dev/null +++ b/pkg/cmd/extensions/command.go @@ -0,0 +1,69 @@ +package extensions + +import ( + "errors" + "fmt" + "path/filepath" + "strings" + + "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/iostreams" + "github.com/spf13/cobra" +) + +func NewCmdExtensions(io *iostreams.IOStreams) *cobra.Command { + m := NewManager() + + extCmd := cobra.Command{ + Use: "extensions", + Short: "Manage gh extensions", + } + + extCmd.AddCommand( + &cobra.Command{ + Use: "list", + Short: "List installed extension commands", + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, args []string) error { + cmds := m.List() + if len(cmds) == 0 { + return errors.New("no extensions installed") + } + for _, c := range cmds { + name := filepath.Base(c) + parts := strings.SplitN(name, "-", 2) + fmt.Fprintf(io.Out, "%s %s\n", parts[0], parts[1]) + } + return nil + }, + }, + &cobra.Command{ + Use: "install ", + Short: "Install a gh extension from a repository", + Args: cmdutil.MinimumArgs(1, "must specify a repository to install from"), + RunE: func(cmd *cobra.Command, args []string) error { + repo, err := ghrepo.FromFullName(args[0]) + if err != nil { + return err + } + if !strings.HasPrefix(repo.RepoName(), "gh-") { + return errors.New("the repository name must start with `gh-`") + } + protocol := "https" // TODO: respect user's preferred protocol + return m.Install(ghrepo.FormatRemoteURL(repo, protocol), io.Out, io.ErrOut) + }, + }, + &cobra.Command{ + Use: "upgrade", + Short: "Upgrade installed extensions", + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, args []string) error { + return m.Upgrade(io.Out, io.ErrOut) + }, + }, + ) + + extCmd.Hidden = true + return &extCmd +} diff --git a/pkg/cmd/extensions/manager.go b/pkg/cmd/extensions/manager.go new file mode 100644 index 000000000..d4fc39916 --- /dev/null +++ b/pkg/cmd/extensions/manager.go @@ -0,0 +1,150 @@ +package extensions + +import ( + "errors" + "io" + "io/ioutil" + "os" + "os/exec" + "path" + "path/filepath" + "strings" + + "github.com/cli/cli/internal/config" + "github.com/cli/safeexec" +) + +type Manager struct { + dataDir func() string + lookPath func(string) (string, error) + pathEnv string +} + +func NewManager() *Manager { + return &Manager{ + dataDir: config.ConfigDir, + lookPath: safeexec.LookPath, + pathEnv: os.Getenv("PATH"), + } +} + +func (m *Manager) Dispatch(args []string, stdin io.Reader, stdout, stderr io.Writer) (bool, error) { + if len(args) == 0 { + return false, errors.New("too few arguments in list") + } + + var exe string + extName := "gh-" + args[0] + forwardArgs := args[1:] + + for _, e := range m.listInstalled() { + if filepath.Base(e) == extName { + exe = e + break + } + } + if exe == "" { + var err error + exe, err = m.lookPath(extName) + if err != nil { + return false, nil + } + } + + // TODO: parse the shebang on Windows and invoke the correct interpreter instead of invoking directly + externalCmd := exec.Command(exe, forwardArgs...) + externalCmd.Stdin = stdin + externalCmd.Stdout = stdout + externalCmd.Stderr = stderr + return true, externalCmd.Run() +} + +func (m *Manager) listInstalled() []string { + dir := m.installDir() + entries, err := ioutil.ReadDir(dir) + if err != nil { + return nil + } + + var results []string + for _, f := range entries { + if !strings.HasPrefix(f.Name(), "gh-") || !f.IsDir() { + continue + } + results = append(results, filepath.Join(dir, f.Name(), f.Name())) + } + return results +} + +func (m *Manager) List() []string { + results := m.listInstalled() + seen := make(map[string]struct{}) + for _, f := range results { + seen[filepath.Base(f)] = struct{}{} + } + + for _, p := range filepath.SplitList(m.pathEnv) { + entries, err := ioutil.ReadDir(p) + if err != nil { + continue + } + for _, f := range entries { + if _, ok := seen[f.Name()]; ok { + continue + } + if !strings.HasPrefix(f.Name(), "gh-") || !isExecutable(f) { + continue + } + results = append(results, filepath.Join(p, f.Name())) + seen[f.Name()] = struct{}{} + } + } + + return results +} + +func (m *Manager) Install(cloneURL string, stdout, stderr io.Writer) error { + exe, err := m.lookPath("git") + if err != nil { + return err + } + + name := strings.TrimSuffix(path.Base(cloneURL), ".git") + targetDir := filepath.Join(m.installDir(), name) + + externalCmd := exec.Command(exe, "clone", cloneURL, targetDir) + externalCmd.Stdout = stdout + externalCmd.Stderr = stderr + return externalCmd.Run() +} + +func (m *Manager) Upgrade(stdout, stderr io.Writer) error { + exe, err := m.lookPath("git") + if err != nil { + return err + } + + exts := m.listInstalled() + if len(exts) == 0 { + return errors.New("no extensions installed") + } + + for _, f := range exts { + externalCmd := exec.Command(exe, "-C", filepath.Dir(f), "pull", "--ff-only") + externalCmd.Stdout = stdout + externalCmd.Stderr = stderr + if e := externalCmd.Run(); e != nil { + err = e + } + } + return err +} + +func (m *Manager) installDir() string { + return filepath.Join(m.dataDir(), "extensions") +} + +// TODO: ignore file mode on Windows +func isExecutable(f os.FileInfo) bool { + return !f.IsDir() && f.Mode()&0111 != 0 +} diff --git a/pkg/cmd/root/root.go b/pkg/cmd/root/root.go index b60d16012..cc971a0d4 100644 --- a/pkg/cmd/root/root.go +++ b/pkg/cmd/root/root.go @@ -13,6 +13,7 @@ import ( authCmd "github.com/cli/cli/pkg/cmd/auth" completionCmd "github.com/cli/cli/pkg/cmd/completion" configCmd "github.com/cli/cli/pkg/cmd/config" + extensionsCmd "github.com/cli/cli/pkg/cmd/extensions" "github.com/cli/cli/pkg/cmd/factory" gistCmd "github.com/cli/cli/pkg/cmd/gist" issueCmd "github.com/cli/cli/pkg/cmd/issue" @@ -80,6 +81,7 @@ func NewCmdRoot(f *cmdutil.Factory, version, buildDate string) *cobra.Command { cmd.AddCommand(creditsCmd.NewCmdCredits(f, nil)) cmd.AddCommand(gistCmd.NewCmdGist(f)) cmd.AddCommand(completionCmd.NewCmdCompletion(f.IOStreams)) + cmd.AddCommand(extensionsCmd.NewCmdExtensions(f.IOStreams)) cmd.AddCommand(secretCmd.NewCmdSecret(f)) cmd.AddCommand(sshKeyCmd.NewCmdSSHKey(f)) From 1d7ffc20133c49bcd8cba6ba68c77a637d7364fd Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Tue, 1 Jun 2021 15:40:08 -0400 Subject: [PATCH 12/54] Add support for LocalAppData and .local/state/ fallback --- internal/config/config_file.go | 29 +++++++++++++++-------- internal/config/config_file_test.go | 36 ++++++++++++++++++++--------- internal/update/update.go | 11 +++++++-- 3 files changed, 54 insertions(+), 22 deletions(-) diff --git a/internal/config/config_file.go b/internal/config/config_file.go index 74fe77502..e90d40ab0 100644 --- a/internal/config/config_file.go +++ b/internal/config/config_file.go @@ -18,6 +18,7 @@ const ( XDG_CONFIG_HOME = "XDG_CONFIG_HOME" XDG_STATE_HOME = "XDG_STATE_HOME" APP_DATA = "AppData" + LOCAL_APP_DATA = "LocalAppData" ) // Config path precedence @@ -39,24 +40,34 @@ func ConfigDir() string { } // If the path does not exist try migrating config from default paths - if _, err := os.Stat(path); errors.Is(err, os.ErrNotExist) { + if !dirExists(path) { _ = autoMigrateConfigDir(path) } return path } +// State path precedence +// 1. XDG_CONFIG_HOME +// 2. LocalAppData (windows only) +// 3. HOME func StateDir() string { - if path := os.Getenv(XDG_STATE_HOME); path != "" { - path = filepath.Join(path, "gh") - if !dirExists(path) { - _ = os.MkdirAll(path, 0755) - _ = autoMigrateStateDir(path) - } - return path + var path string + if a := os.Getenv(XDG_STATE_HOME); a != "" { + path = filepath.Join(a, "gh") + } else if b := os.Getenv(LOCAL_APP_DATA); runtime.GOOS == "windows" && b != "" { + path = filepath.Join(b, "GitHub CLI") + } else { + c, _ := os.UserHomeDir() + path = filepath.Join(c, ".local", "state", "gh") } - return ConfigDir() + // If the path does not exist try migrating state from default paths + if !dirExists(path) { + _ = autoMigrateStateDir(path) + } + + return path } var errSamePath = errors.New("same path") diff --git a/internal/config/config_file_test.go b/internal/config/config_file_test.go index ff4cd1b04..e6150aae3 100644 --- a/internal/config/config_file_test.go +++ b/internal/config/config_file_test.go @@ -346,9 +346,10 @@ func Test_StateDir(t *testing.T) { tempDir := t.TempDir() tests := []struct { - name string - env map[string]string - output string + name string + onlyWindows bool + env map[string]string + output string }{ { name: "HOME/USERPROFILE specified", @@ -356,11 +357,11 @@ func Test_StateDir(t *testing.T) { "XDG_STATE_HOME": "", "GH_CONFIG_DIR": "", "XDG_CONFIG_HOME": "", - "AppData": "", + "LocalAppData": "", "USERPROFILE": tempDir, "HOME": tempDir, }, - output: filepath.Join(tempDir, ".config", "gh"), + output: filepath.Join(tempDir, ".local", "state", "gh"), }, { name: "XDG_STATE_HOME specified", @@ -370,15 +371,28 @@ func Test_StateDir(t *testing.T) { output: filepath.Join(tempDir, "gh"), }, { - name: "GH_CONFIG_DIR specified", + name: "LocalAppData specified", + onlyWindows: true, env: map[string]string{ - "GH_CONFIG_DIR": filepath.Join(tempDir, "gh_config_dir"), + "LocalAppData": tempDir, }, - output: filepath.Join(tempDir, "gh_config_dir"), + output: filepath.Join(tempDir, "GitHub CLI"), + }, + { + name: "XDG_STATE_HOME and LocalAppData specified", + onlyWindows: true, + env: map[string]string{ + "XDG_STATE_HOME": tempDir, + "LocalAppData": tempDir, + }, + output: filepath.Join(tempDir, "gh"), }, } for _, tt := range tests { + if tt.onlyWindows && runtime.GOOS != "windows" { + continue + } t.Run(tt.name, func(t *testing.T) { if tt.env != nil { for k, v := range tt.env { @@ -443,7 +457,7 @@ func Test_autoMigrateStateDir_migration(t *testing.T) { homeDir := t.TempDir() migrateDir := t.TempDir() homeConfigDir := filepath.Join(homeDir, ".config", "gh") - migrateConfigDir := filepath.Join(migrateDir, ".config", "gh") + migrateStateDir := filepath.Join(migrateDir, ".local", "state", "gh") homeEnvVar := "HOME" if runtime.GOOS == "windows" { @@ -458,14 +472,14 @@ func Test_autoMigrateStateDir_migration(t *testing.T) { err = ioutil.WriteFile(filepath.Join(homeConfigDir, "state.yml"), nil, 0755) assert.NoError(t, err) - err = autoMigrateStateDir(migrateConfigDir) + err = autoMigrateStateDir(migrateStateDir) assert.NoError(t, err) files, err := ioutil.ReadDir(homeConfigDir) assert.NoError(t, err) assert.Equal(t, 0, len(files)) - files, err = ioutil.ReadDir(migrateConfigDir) + files, err = ioutil.ReadDir(migrateStateDir) assert.NoError(t, err) assert.Equal(t, 1, len(files)) assert.Equal(t, "state.yml", files[0].Name()) diff --git a/internal/update/update.go b/internal/update/update.go index 024fd2377..f525544e9 100644 --- a/internal/update/update.go +++ b/internal/update/update.go @@ -3,6 +3,8 @@ package update import ( "fmt" "io/ioutil" + "os" + "path/filepath" "regexp" "strconv" "strings" @@ -83,9 +85,14 @@ func setStateEntry(stateFilePath string, t time.Time, r ReleaseInfo) error { if err != nil { return err } - _ = ioutil.WriteFile(stateFilePath, content, 0600) - return nil + err = os.MkdirAll(filepath.Dir(stateFilePath), 0755) + if err != nil { + return err + } + + err = ioutil.WriteFile(stateFilePath, content, 0600) + return err } func versionGreaterThan(v, w string) bool { From 184149b8440ec013a10ea90ce71879017c0f453a Mon Sep 17 00:00:00 2001 From: jack1142 <6032823+jack1142@users.noreply.github.com> Date: Tue, 1 Jun 2021 23:53:58 +0200 Subject: [PATCH 13/54] Add missing new line --- pkg/cmd/issue/view/view.go | 2 +- pkg/cmd/pr/view/view.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/issue/view/view.go b/pkg/cmd/issue/view/view.go index 87c8e3143..d4f96bbd8 100644 --- a/pkg/cmd/issue/view/view.go +++ b/pkg/cmd/issue/view/view.go @@ -173,7 +173,7 @@ func printHumanIssuePreview(opts *ViewOptions, issue *api.Issue) error { cs := opts.IO.ColorScheme() // Header (Title and State) - fmt.Fprintf(out, "%s #%d", cs.Bold(issue.Title), issue.Number) + fmt.Fprintf(out, "%s #%d\n", cs.Bold(issue.Title), issue.Number) fmt.Fprintf(out, "%s • %s opened %s • %s\n", issueStateTitleWithColor(cs, issue.State), diff --git a/pkg/cmd/pr/view/view.go b/pkg/cmd/pr/view/view.go index a37568741..c733909c5 100644 --- a/pkg/cmd/pr/view/view.go +++ b/pkg/cmd/pr/view/view.go @@ -170,7 +170,7 @@ func printHumanPrPreview(opts *ViewOptions, pr *api.PullRequest) error { cs := opts.IO.ColorScheme() // Header (Title and State) - fmt.Fprintf(out, "%s #%d", cs.Bold(pr.Title), pr.Number) + fmt.Fprintf(out, "%s #%d\n", cs.Bold(pr.Title), pr.Number) fmt.Fprintf(out, "%s • %s wants to merge %s into %s from %s • %s %s \n", shared.StateTitleWithColor(cs, *pr), From c34d017a04947a6eca1facfbd90f1f75ea588109 Mon Sep 17 00:00:00 2001 From: Param Patidar Date: Wed, 2 Jun 2021 12:33:40 +0530 Subject: [PATCH 14/54] fix project layout link --- .github/CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index d339a0685..d48b17ae2 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -36,7 +36,7 @@ Run the new binary as: Run tests with: `go test ./...` -See [project layout documentation](../project-layout.md) for information on where to find specific source files. +See [project layout documentation](../docs/project-layout.md) for information on where to find specific source files. ## Submitting a pull request From e6ad8a10d0bd576d105f99fab318dbd382d7a03a Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Wed, 2 Jun 2021 09:46:14 -0400 Subject: [PATCH 15/54] Add support for XDG_DATA_HOME --- internal/config/config_file.go | 19 +++++++++ internal/config/config_file_test.go | 65 +++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+) diff --git a/internal/config/config_file.go b/internal/config/config_file.go index e90d40ab0..304ea3327 100644 --- a/internal/config/config_file.go +++ b/internal/config/config_file.go @@ -17,6 +17,7 @@ const ( GH_CONFIG_DIR = "GH_CONFIG_DIR" XDG_CONFIG_HOME = "XDG_CONFIG_HOME" XDG_STATE_HOME = "XDG_STATE_HOME" + XDG_DATA_HOME = "XDG_DATA_HOME" APP_DATA = "AppData" LOCAL_APP_DATA = "LocalAppData" ) @@ -70,6 +71,24 @@ func StateDir() string { return path } +// Data path precedence +// 1. XDG_DATA_HOME +// 2. LocalAppData (windows only) +// 3. HOME +func DataDir() string { + var path string + if a := os.Getenv(XDG_DATA_HOME); a != "" { + path = filepath.Join(a, "gh") + } else if b := os.Getenv(LOCAL_APP_DATA); runtime.GOOS == "windows" && b != "" { + path = filepath.Join(b, "GitHub CLI") + } else { + c, _ := os.UserHomeDir() + path = filepath.Join(c, ".local", "share", "gh") + } + + return path +} + var errSamePath = errors.New("same path") var errNotExist = errors.New("not exist") diff --git a/internal/config/config_file_test.go b/internal/config/config_file_test.go index e6150aae3..4c35f24f9 100644 --- a/internal/config/config_file_test.go +++ b/internal/config/config_file_test.go @@ -484,3 +484,68 @@ func Test_autoMigrateStateDir_migration(t *testing.T) { assert.Equal(t, 1, len(files)) assert.Equal(t, "state.yml", files[0].Name()) } + +func Test_DataDir(t *testing.T) { + tempDir := t.TempDir() + + tests := []struct { + name string + onlyWindows bool + env map[string]string + output string + }{ + { + name: "HOME/USERPROFILE specified", + env: map[string]string{ + "XDG_DATA_HOME": "", + "GH_CONFIG_DIR": "", + "XDG_CONFIG_HOME": "", + "LocalAppData": "", + "USERPROFILE": tempDir, + "HOME": tempDir, + }, + output: filepath.Join(tempDir, ".local", "share", "gh"), + }, + { + name: "XDG_DATA_HOME specified", + env: map[string]string{ + "XDG_DATA_HOME": tempDir, + }, + output: filepath.Join(tempDir, "gh"), + }, + { + name: "LocalAppData specified", + onlyWindows: true, + env: map[string]string{ + "LocalAppData": tempDir, + }, + output: filepath.Join(tempDir, "GitHub CLI"), + }, + { + name: "XDG_DATA_HOME and LocalAppData specified", + onlyWindows: true, + env: map[string]string{ + "XDG_DATA_HOME": tempDir, + "LocalAppData": tempDir, + }, + output: filepath.Join(tempDir, "gh"), + }, + } + + for _, tt := range tests { + if tt.onlyWindows && runtime.GOOS != "windows" { + continue + } + t.Run(tt.name, func(t *testing.T) { + if tt.env != nil { + for k, v := range tt.env { + old := os.Getenv(k) + os.Setenv(k, v) + defer os.Setenv(k, old) + } + } + + assert.Equal(t, tt.output, DataDir()) + }) + } +} From 389fdb7f99ffe445b8b5b881cd8dae1c428e249b Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Wed, 2 Jun 2021 09:56:22 -0400 Subject: [PATCH 16/54] Add XDG env variables to environment help topic --- pkg/cmd/root/help_topic.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pkg/cmd/root/help_topic.go b/pkg/cmd/root/help_topic.go index 968b8a617..040c9fa12 100644 --- a/pkg/cmd/root/help_topic.go +++ b/pkg/cmd/root/help_topic.go @@ -64,6 +64,12 @@ var HelpTopics = map[string]map[string]string{ GH_NO_UPDATE_NOTIFIER: set to any value to disable update notifications. By default, gh checks for new releases once every 24 hours and displays an upgrade notice on standard error if a newer version was found. + + GH_CONFIG_DIR, XDG_CONFIG_HOME (in order of precedence): the directory where gh will store configuration files. + + XDG_STATE_HOME: the directory where gh will store state files. + + XDG_DATA_HOME: the directory where gh will store data files. `), }, "reference": { From d396674e12e1fdaba930043eae2f208b751c635c Mon Sep 17 00:00:00 2001 From: Bruno Alla Date: Wed, 2 Jun 2021 15:53:03 +0100 Subject: [PATCH 17/54] Fix a typo in the docs --- docs/project-layout.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/project-layout.md b/docs/project-layout.md index cf594a2e7..cf9758b46 100644 --- a/docs/project-layout.md +++ b/docs/project-layout.md @@ -75,7 +75,7 @@ and talk through which code gets run in order. This task might be tricky. Typically, gh commands do things like look up information from the git repository in the current directory, query the GitHub API, scan the user's `~/.ssh/config` file, clone or fetch git repositories, etc. Naturally, none of these things should ever happen for real when running tests, unless -you are sure that any filesystem operations are stricly scoped to a location made for and maintained by the +you are sure that any filesystem operations are strictly scoped to a location made for and maintained by the test itself. To avoid actually running things like making real API requests or shelling out to `git` commands, we stub them. You should look at how that's done within some existing tests. From 32856c987dc8b4ab688f7cbb23d16bb9313c1eba Mon Sep 17 00:00:00 2001 From: Bruno Alla Date: Wed, 2 Jun 2021 15:53:40 +0100 Subject: [PATCH 18/54] Add ability to set environments secrets --- pkg/cmd/secret/set/http.go | 9 ++++++ pkg/cmd/secret/set/set.go | 20 +++++++++++-- pkg/cmd/secret/set/set_test.go | 52 ++++++++++++++++++++++++++++++++++ 3 files changed, 78 insertions(+), 3 deletions(-) diff --git a/pkg/cmd/secret/set/http.go b/pkg/cmd/secret/set/http.go index 8c589adec..3ecee37c0 100644 --- a/pkg/cmd/secret/set/http.go +++ b/pkg/cmd/secret/set/http.go @@ -90,6 +90,15 @@ func putOrgSecret(client *api.Client, host string, pk *PubKey, opts SetOptions, return putSecret(client, host, path, payload) } +func putEnvSecret(client *api.Client, pk *PubKey, repo ghrepo.Interface, envName string, secretName, eValue string) error { + payload := SecretPayload{ + EncryptedValue: eValue, + KeyID: pk.ID, + } + path := fmt.Sprintf("repos/%s/environments/%s/secrets/%s", ghrepo.FullName(repo), envName, secretName) + return putSecret(client, repo.RepoHost(), path, payload) +} + func putRepoSecret(client *api.Client, pk *PubKey, repo ghrepo.Interface, secretName, eValue string) error { payload := SecretPayload{ EncryptedValue: eValue, diff --git a/pkg/cmd/secret/set/set.go b/pkg/cmd/secret/set/set.go index cb1975ad3..9bb664e5a 100644 --- a/pkg/cmd/secret/set/set.go +++ b/pkg/cmd/secret/set/set.go @@ -33,6 +33,7 @@ type SetOptions struct { SecretName string OrgName string + EnvName string Body string Visibility string RepositoryNames []string @@ -48,7 +49,7 @@ func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command cmd := &cobra.Command{ Use: "set ", Short: "Create or update secrets", - Long: "Locally encrypt a new or updated secret at either the repository or organization level and send it to GitHub for storage.", + Long: "Locally encrypt a new or updated secret at either the repository, environment, or organization level and send it to GitHub for storage.", Example: heredoc.Doc(` Paste secret in prompt $ gh secret set MYSECRET @@ -59,6 +60,9 @@ func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command Use file as secret value $ gh secret set MYSECRET < file.json + Set environment level secret + $ gh secret set MYSECRET -bval --env=anEnv + Set organization level secret visible to entire organization $ gh secret set MYSECRET -bval --org=anOrg --visibility=all @@ -75,6 +79,10 @@ func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command // support `-R, --repo` override opts.BaseRepo = f.BaseRepo + if err := cmdutil.MutuallyExclusive("specify only one of `--org` or `--env`", opts.OrgName != "", opts.EnvName != ""); err != nil { + return err + } + opts.SecretName = args[0] err := validSecretName(opts.SecretName) @@ -115,7 +123,8 @@ func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command return setRun(opts) }, } - cmd.Flags().StringVarP(&opts.OrgName, "org", "o", "", "List secrets for an organization") + cmd.Flags().StringVarP(&opts.OrgName, "org", "o", "", "Set a secret for an organization") + cmd.Flags().StringVarP(&opts.EnvName, "env", "e", "", "Set a secret for an organization") cmd.Flags().StringVarP(&opts.Visibility, "visibility", "v", "private", "Set visibility for an organization secret: `all`, `private`, or `selected`") cmd.Flags().StringSliceVarP(&opts.RepositoryNames, "repos", "r", []string{}, "List of repository names for `selected` visibility") cmd.Flags().StringVarP(&opts.Body, "body", "b", "", "A value for the secret. Reads from STDIN if not specified.") @@ -136,6 +145,7 @@ func setRun(opts *SetOptions) error { client := api.NewClientFromHTTP(c) orgName := opts.OrgName + envName := opts.EnvName var baseRepo ghrepo.Interface if orgName == "" { @@ -175,7 +185,11 @@ func setRun(opts *SetOptions) error { if orgName != "" { err = putOrgSecret(client, host, pk, *opts, encoded) } else { - err = putRepoSecret(client, pk, baseRepo, opts.SecretName, encoded) + if envName != "" { + err = putEnvSecret(client, pk, baseRepo, envName, opts.SecretName, encoded) + } else { + err = putRepoSecret(client, pk, baseRepo, opts.SecretName, encoded) + } } if err != nil { return fmt.Errorf("failed to set secret: %w", err) diff --git a/pkg/cmd/secret/set/set_test.go b/pkg/cmd/secret/set/set_test.go index 7f9273eae..f1c2b7d76 100644 --- a/pkg/cmd/secret/set/set_test.go +++ b/pkg/cmd/secret/set/set_test.go @@ -100,6 +100,17 @@ func TestNewCmdSet(t *testing.T) { OrgName: "", }, }, + { + name: "env", + cli: `cool_secret -b"a secret" -eRelease`, + wants: SetOptions{ + SecretName: "cool_secret", + Visibility: shared.Private, + Body: "a secret", + OrgName: "", + EnvName: "Release", + }, + }, { name: "vis all", cli: `cool_secret --org coolOrg -b"cool" -vall`, @@ -160,6 +171,7 @@ func TestNewCmdSet(t *testing.T) { assert.Equal(t, tt.wants.Body, gotOpts.Body) assert.Equal(t, tt.wants.Visibility, gotOpts.Visibility) assert.Equal(t, tt.wants.OrgName, gotOpts.OrgName) + assert.Equal(t, tt.wants.EnvName, gotOpts.EnvName) assert.ElementsMatch(t, tt.wants.RepositoryNames, gotOpts.RepositoryNames) }) } @@ -204,6 +216,46 @@ func Test_setRun_repo(t *testing.T) { assert.Equal(t, payload.EncryptedValue, "UKYUCbHd0DJemxa3AOcZ6XcsBwALG9d4bpB8ZT0gSV39vl3BHiGSgj8zJapDxgB2BwqNqRhpjC4=") } +func Test_setRun_env(t *testing.T) { + reg := &httpmock.Registry{} + + reg.Register(httpmock.REST("GET", "repos/owner/repo/actions/secrets/public-key"), + httpmock.JSONResponse(PubKey{ID: "123", Key: "CDjXqf7AJBXWhMczcy+Fs7JlACEptgceysutztHaFQI="})) + + reg.Register(httpmock.REST("PUT", "repos/owner/repo/environments/development/secrets/cool_secret"), httpmock.StatusStringResponse(201, `{}`)) + + io, _, _, _ := iostreams.Test() + + opts := &SetOptions{ + HttpClient: func() (*http.Client, error) { + return &http.Client{Transport: reg}, nil + }, + Config: func() (config.Config, error) { return config.NewBlankConfig(), nil }, + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.FromFullName("owner/repo") + }, + EnvName: "development", + IO: io, + SecretName: "cool_secret", + Body: "a secret", + // Cribbed from https://github.com/golang/crypto/commit/becbf705a91575484002d598f87d74f0002801e7 + RandomOverride: bytes.NewReader([]byte{5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5, 5}), + } + + err := setRun(opts) + assert.NoError(t, err) + + reg.Verify(t) + + data, err := ioutil.ReadAll(reg.Requests[1].Body) + assert.NoError(t, err) + var payload SecretPayload + err = json.Unmarshal(data, &payload) + assert.NoError(t, err) + assert.Equal(t, payload.KeyID, "123") + assert.Equal(t, payload.EncryptedValue, "UKYUCbHd0DJemxa3AOcZ6XcsBwALG9d4bpB8ZT0gSV39vl3BHiGSgj8zJapDxgB2BwqNqRhpjC4=") +} + func Test_setRun_org(t *testing.T) { tests := []struct { name string From 20f915d5ba89141f2f26f05e87a27e9c247dac4f Mon Sep 17 00:00:00 2001 From: Param Patidar Date: Wed, 2 Jun 2021 17:20:31 +0000 Subject: [PATCH 19/54] escape metacharacters in job name --- pkg/cmd/run/view/view.go | 2 +- pkg/cmd/run/view/view_test.go | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/run/view/view.go b/pkg/cmd/run/view/view.go index 17a8525f3..5dba471fc 100644 --- a/pkg/cmd/run/view/view.go +++ b/pkg/cmd/run/view/view.go @@ -462,7 +462,7 @@ func promptForJob(cs *iostreams.ColorScheme, jobs []shared.Job) (*shared.Job, er } func logFilenameRegexp(job shared.Job, step shared.Step) *regexp.Regexp { - re := fmt.Sprintf(`%s\/%d_.*\.txt`, job.Name, step.Number) + re := fmt.Sprintf(`%s\/%d_.*\.txt`, regexp.QuoteMeta(job.Name), step.Number) return regexp.MustCompile(re) } diff --git a/pkg/cmd/run/view/view_test.go b/pkg/cmd/run/view/view_test.go index 13ef80b7e..4e284b354 100644 --- a/pkg/cmd/run/view/view_test.go +++ b/pkg/cmd/run/view/view_test.go @@ -926,6 +926,17 @@ func Test_attachRunLog(t *testing.T) { }, wantMatch: false, }, + { + name: "escape metacharacters in job name", + job: shared.Job{ + Name: "metacharacters .+*?()|[]{}^$ job", + Steps: []shared.Step{{ + Name: "fob the barz", + Number: 0, + }}, + }, + wantMatch: false, + }, { name: "mismatching job name", job: shared.Job{ From 0931531e2fac2322f37ed28a106183188b87a547 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Wed, 2 Jun 2021 13:27:19 -0500 Subject: [PATCH 20/54] collapse conditional --- pkg/cmd/secret/set/set.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/pkg/cmd/secret/set/set.go b/pkg/cmd/secret/set/set.go index 9bb664e5a..8f6de703d 100644 --- a/pkg/cmd/secret/set/set.go +++ b/pkg/cmd/secret/set/set.go @@ -184,12 +184,10 @@ func setRun(opts *SetOptions) error { if orgName != "" { err = putOrgSecret(client, host, pk, *opts, encoded) + } else if envName != "" { + err = putEnvSecret(client, pk, baseRepo, envName, opts.SecretName, encoded) } else { - if envName != "" { - err = putEnvSecret(client, pk, baseRepo, envName, opts.SecretName, encoded) - } else { - err = putRepoSecret(client, pk, baseRepo, opts.SecretName, encoded) - } + err = putRepoSecret(client, pk, baseRepo, opts.SecretName, encoded) } if err != nil { return fmt.Errorf("failed to set secret: %w", err) From 4b79edf603044928c8bb5a2bef9070209de0ee52 Mon Sep 17 00:00:00 2001 From: Bruno Alla Date: Thu, 3 Jun 2021 08:51:39 +0100 Subject: [PATCH 21/54] Add support for removing environment secrets --- pkg/cmd/secret/remove/remove.go | 25 +++++++++++++----- pkg/cmd/secret/remove/remove_test.go | 39 ++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 6 deletions(-) diff --git a/pkg/cmd/secret/remove/remove.go b/pkg/cmd/secret/remove/remove.go index d52269d73..718e09050 100644 --- a/pkg/cmd/secret/remove/remove.go +++ b/pkg/cmd/secret/remove/remove.go @@ -20,6 +20,7 @@ type RemoveOptions struct { SecretName string OrgName string + EnvName string } func NewCmdRemove(f *cmdutil.Factory, runF func(*RemoveOptions) error) *cobra.Command { @@ -31,12 +32,16 @@ func NewCmdRemove(f *cmdutil.Factory, runF func(*RemoveOptions) error) *cobra.Co cmd := &cobra.Command{ Use: "remove ", - Short: "Remove an organization or repository secret", + Short: "Remove an organization, environment, or repository secret", Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { // support `-R, --repo` override opts.BaseRepo = f.BaseRepo + if err := cmdutil.MutuallyExclusive("specify only one of `--org` or `--env`", opts.OrgName != "", opts.EnvName != ""); err != nil { + return err + } + opts.SecretName = args[0] if runF != nil { @@ -46,7 +51,8 @@ func NewCmdRemove(f *cmdutil.Factory, runF func(*RemoveOptions) error) *cobra.Co return removeRun(opts) }, } - cmd.Flags().StringVarP(&opts.OrgName, "org", "o", "", "List secrets for an organization") + cmd.Flags().StringVarP(&opts.OrgName, "org", "o", "", "Remove a secret for an organization") + cmd.Flags().StringVarP(&opts.EnvName, "env", "e", "", "Remove a secret for an environment") return cmd } @@ -59,6 +65,7 @@ func removeRun(opts *RemoveOptions) error { client := api.NewClientFromHTTP(c) orgName := opts.OrgName + envName := opts.EnvName var baseRepo ghrepo.Interface if orgName == "" { @@ -69,10 +76,12 @@ func removeRun(opts *RemoveOptions) error { } var path string - if orgName == "" { - path = fmt.Sprintf("repos/%s/actions/secrets/%s", ghrepo.FullName(baseRepo), opts.SecretName) - } else { + if orgName != "" { path = fmt.Sprintf("orgs/%s/actions/secrets/%s", orgName, opts.SecretName) + } else if envName != "" { + path = fmt.Sprintf("repos/%s/environments/%s/secrets/%s", ghrepo.FullName(baseRepo), envName, opts.SecretName) + } else { + path = fmt.Sprintf("repos/%s/actions/secrets/%s", ghrepo.FullName(baseRepo), opts.SecretName) } cfg, err := opts.Config() @@ -96,7 +105,11 @@ func removeRun(opts *RemoveOptions) error { target = ghrepo.FullName(baseRepo) } cs := opts.IO.ColorScheme() - fmt.Fprintf(opts.IO.Out, "%s Removed secret %s from %s\n", cs.SuccessIconWithColor(cs.Red), opts.SecretName, target) + if envName != "" { + fmt.Fprintf(opts.IO.Out, "%s Removed secret %s from %s environment on %s\n", cs.SuccessIconWithColor(cs.Red), opts.SecretName, envName, target) + } else { + fmt.Fprintf(opts.IO.Out, "%s Removed secret %s from %s\n", cs.SuccessIconWithColor(cs.Red), opts.SecretName, target) + } } return nil diff --git a/pkg/cmd/secret/remove/remove_test.go b/pkg/cmd/secret/remove/remove_test.go index 7cec1030c..b47703049 100644 --- a/pkg/cmd/secret/remove/remove_test.go +++ b/pkg/cmd/secret/remove/remove_test.go @@ -41,6 +41,14 @@ func TestNewCmdRemove(t *testing.T) { OrgName: "anOrg", }, }, + { + name: "env", + cli: "cool --env anEnv", + wants: RemoveOptions{ + SecretName: "cool", + EnvName: "anEnv", + }, + }, } for _, tt := range tests { @@ -72,6 +80,7 @@ func TestNewCmdRemove(t *testing.T) { assert.Equal(t, tt.wants.SecretName, gotOpts.SecretName) assert.Equal(t, tt.wants.OrgName, gotOpts.OrgName) + assert.Equal(t, tt.wants.EnvName, gotOpts.EnvName) }) } @@ -106,6 +115,36 @@ func Test_removeRun_repo(t *testing.T) { reg.Verify(t) } +func Test_removeRun_env(t *testing.T) { + reg := &httpmock.Registry{} + + reg.Register( + httpmock.REST("DELETE", "repos/owner/repo/environments/development/secrets/cool_secret"), + httpmock.StatusStringResponse(204, "No Content")) + + io, _, _, _ := iostreams.Test() + + opts := &RemoveOptions{ + IO: io, + HttpClient: func() (*http.Client, error) { + return &http.Client{Transport: reg}, nil + }, + Config: func() (config.Config, error) { + return config.NewBlankConfig(), nil + }, + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.FromFullName("owner/repo") + }, + SecretName: "cool_secret", + EnvName: "development", + } + + err := removeRun(opts) + assert.NoError(t, err) + + reg.Verify(t) +} + func Test_removeRun_org(t *testing.T) { tests := []struct { name string From d974dbd338893027432a078248d35a78dbe765a2 Mon Sep 17 00:00:00 2001 From: chemotaxis Date: Thu, 20 May 2021 03:32:34 -0400 Subject: [PATCH 22/54] Return default text if skipping the text editor when prompted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If we are allowed to skip the editor _and_ we want to append the default text to the editor if we'd opened it, we just return the default text. Co-Authored-By: Mislav Marohnić --- pkg/surveyext/editor.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/surveyext/editor.go b/pkg/surveyext/editor.go index 21a358aa0..f868ed671 100644 --- a/pkg/surveyext/editor.go +++ b/pkg/surveyext/editor.go @@ -105,7 +105,7 @@ func (e *GhEditor) prompt(initialValue string, config *survey.PromptConfig) (int } if r == '\r' || r == '\n' { if e.BlankAllowed { - return "", nil + return initialValue, nil } else { continue } From c2c691f44419cc1b1ece7f1771450d43269080a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 3 Jun 2021 16:19:53 +0200 Subject: [PATCH 23/54] Add test for our survey editor extension --- go.mod | 1 + go.sum | 2 + pkg/surveyext/editor_test.go | 132 +++++++++++++++++++++++++++++++++++ 3 files changed, 135 insertions(+) create mode 100644 pkg/surveyext/editor_test.go diff --git a/go.mod b/go.mod index 80b4b523a..080876810 100644 --- a/go.mod +++ b/go.mod @@ -11,6 +11,7 @@ require ( github.com/cli/oauth v0.8.0 github.com/cli/safeexec v1.0.0 github.com/cpuguy83/go-md2man/v2 v2.0.0 + github.com/creack/pty v1.1.13 github.com/gabriel-vasile/mimetype v1.1.2 github.com/google/go-cmp v0.5.2 github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 diff --git a/go.sum b/go.sum index 7f89f8dd8..aad4401e7 100644 --- a/go.sum +++ b/go.sum @@ -62,6 +62,8 @@ github.com/coreos/go-systemd v0.0.0-20190321100706-95778dfbb74e/go.mod h1:F5haX7 github.com/coreos/pkg v0.0.0-20180928190104-399ea9e2e55f/go.mod h1:E3G3o1h8I7cfcXa63jLwjI0eiQQMgzzUDFVpN/nH/eA= github.com/cpuguy83/go-md2man/v2 v2.0.0 h1:EoUDS0afbrsXAZ9YQ9jdu/mZ2sXgT1/2yyNng4PGlyM= github.com/cpuguy83/go-md2man/v2 v2.0.0/go.mod h1:maD7wRr/U5Z6m/iR4s+kqSMx2CaBsrgA7czyZG/E6dU= +github.com/creack/pty v1.1.13 h1:rTPnd/xocYRjutMfqide2zle1u96upp1gm6eUHKi7us= +github.com/creack/pty v1.1.13/go.mod h1:MOBLtS5ELjhRRrroQr9kyvTxUAFNvYEK993ew/Vr4O4= github.com/danwakefield/fnmatch v0.0.0-20160403171240-cbb64ac3d964 h1:y5HC9v93H5EPKqaS1UYVg1uYah5Xf51mBfIoWehClUQ= github.com/danwakefield/fnmatch v0.0.0-20160403171240-cbb64ac3d964/go.mod h1:Xd9hchkHSWYkEqJwUGisez3G1QY8Ryz0sdWrLPMGjLk= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= diff --git a/pkg/surveyext/editor_test.go b/pkg/surveyext/editor_test.go new file mode 100644 index 000000000..03ad27989 --- /dev/null +++ b/pkg/surveyext/editor_test.go @@ -0,0 +1,132 @@ +package surveyext + +import ( + "bytes" + "errors" + "fmt" + "os" + "strings" + "sync" + "testing" + "time" + + "github.com/AlecAivazis/survey/v2" + "github.com/AlecAivazis/survey/v2/terminal" + pseudotty "github.com/creack/pty" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func Test_GhEditor_Prompt(t *testing.T) { + e := &GhEditor{ + BlankAllowed: true, + EditorCommand: "false", + Editor: &survey.Editor{ + Message: "Body", + FileName: "*.md", + Default: "initial value", + HideDefault: true, + AppendDefault: true, + }, + } + + pty, tty, err := pseudotty.Open() + if errors.Is(err, pseudotty.ErrUnsupported) { + return + } + require.NoError(t, err) + defer pty.Close() + defer tty.Close() + + err = pseudotty.Setsize(tty, &pseudotty.Winsize{Cols: 72, Rows: 30}) + require.NoError(t, err) + + out := teeWriter{File: tty} + e.WithStdio(terminal.Stdio{ + In: tty, + Out: &out, + Err: tty, + }) + + var res string + errc := make(chan error) + + go func() { + r, err := e.Prompt(defaultPromptConfig()) + if r != nil { + res = r.(string) + } + errc <- err + }() + + time.Sleep(5 * time.Millisecond) + assert.Equal(t, "\x1b[0G\x1b[2K\x1b[0;1;92m? \x1b[0m\x1b[0;1;99mBody \x1b[0m\x1b[0;36m[(e) to launch false, enter to skip] \x1b[0m\x1b[?25l", out.String()) + fmt.Fprint(pty, "\n") // send Enter key + + err = <-errc + assert.NoError(t, err) + assert.Equal(t, "initial value", res) + assert.Equal(t, "\x1b[?25h", out.String()) +} + +// survey doesn't expose this +func defaultPromptConfig() *survey.PromptConfig { + return &survey.PromptConfig{ + PageSize: 7, + HelpInput: "?", + SuggestInput: "tab", + Icons: survey.IconSet{ + Error: survey.Icon{ + Text: "X", + Format: "red", + }, + Help: survey.Icon{ + Text: "?", + Format: "cyan", + }, + Question: survey.Icon{ + Text: "?", + Format: "green+hb", + }, + MarkedOption: survey.Icon{ + Text: "[x]", + Format: "green", + }, + UnmarkedOption: survey.Icon{ + Text: "[ ]", + Format: "default+hb", + }, + SelectFocus: survey.Icon{ + Text: ">", + Format: "cyan+b", + }, + }, + Filter: func(filter string, value string, index int) (include bool) { + filter = strings.ToLower(filter) + return strings.Contains(strings.ToLower(value), filter) + }, + KeepFilter: false, + } +} + +// teeWriter is a writer that duplicates all writes to a file into a buffer +type teeWriter struct { + *os.File + buf bytes.Buffer + mu sync.Mutex +} + +func (f *teeWriter) Write(p []byte) (n int, err error) { + f.mu.Lock() + defer f.mu.Unlock() + _, _ = f.buf.Write(p) + return f.File.Write(p) +} + +func (f *teeWriter) String() string { + f.mu.Lock() + s := f.buf.String() + f.buf.Reset() + f.mu.Unlock() + return s +} From 4bdddd72d34b61c391397ab3332989a6f85f6b60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 3 Jun 2021 19:06:28 +0200 Subject: [PATCH 24/54] Allow installing local extensions via symlinks This also quits searching for local extensions in PATH. --- pkg/cmd/extensions/command.go | 8 +++++ pkg/cmd/extensions/manager.go | 55 +++++++++-------------------------- 2 files changed, 21 insertions(+), 42 deletions(-) diff --git a/pkg/cmd/extensions/command.go b/pkg/cmd/extensions/command.go index 5794a3806..298787dcb 100644 --- a/pkg/cmd/extensions/command.go +++ b/pkg/cmd/extensions/command.go @@ -3,6 +3,7 @@ package extensions import ( "errors" "fmt" + "os" "path/filepath" "strings" @@ -43,6 +44,13 @@ func NewCmdExtensions(io *iostreams.IOStreams) *cobra.Command { Short: "Install a gh extension from a repository", 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 diff --git a/pkg/cmd/extensions/manager.go b/pkg/cmd/extensions/manager.go index d4fc39916..c3988b483 100644 --- a/pkg/cmd/extensions/manager.go +++ b/pkg/cmd/extensions/manager.go @@ -2,6 +2,7 @@ package extensions import ( "errors" + "fmt" "io" "io/ioutil" "os" @@ -17,14 +18,12 @@ import ( type Manager struct { dataDir func() string lookPath func(string) (string, error) - pathEnv string } func NewManager() *Manager { return &Manager{ dataDir: config.ConfigDir, lookPath: safeexec.LookPath, - pathEnv: os.Getenv("PATH"), } } @@ -37,18 +36,14 @@ func (m *Manager) Dispatch(args []string, stdin io.Reader, stdout, stderr io.Wri extName := "gh-" + args[0] forwardArgs := args[1:] - for _, e := range m.listInstalled() { + for _, e := range m.List() { if filepath.Base(e) == extName { exe = e break } } if exe == "" { - var err error - exe, err = m.lookPath(extName) - if err != nil { - return false, nil - } + return false, nil } // TODO: parse the shebang on Windows and invoke the correct interpreter instead of invoking directly @@ -59,7 +54,7 @@ func (m *Manager) Dispatch(args []string, stdin io.Reader, stdout, stderr io.Wri return true, externalCmd.Run() } -func (m *Manager) listInstalled() []string { +func (m *Manager) List() []string { dir := m.installDir() entries, err := ioutil.ReadDir(dir) if err != nil { @@ -68,7 +63,7 @@ func (m *Manager) listInstalled() []string { var results []string for _, f := range entries { - if !strings.HasPrefix(f.Name(), "gh-") || !f.IsDir() { + if !strings.HasPrefix(f.Name(), "gh-") || !(f.IsDir() || f.Mode()&os.ModeSymlink != 0) { continue } results = append(results, filepath.Join(dir, f.Name(), f.Name())) @@ -76,31 +71,10 @@ func (m *Manager) listInstalled() []string { return results } -func (m *Manager) List() []string { - results := m.listInstalled() - seen := make(map[string]struct{}) - for _, f := range results { - seen[filepath.Base(f)] = struct{}{} - } - - for _, p := range filepath.SplitList(m.pathEnv) { - entries, err := ioutil.ReadDir(p) - if err != nil { - continue - } - for _, f := range entries { - if _, ok := seen[f.Name()]; ok { - continue - } - if !strings.HasPrefix(f.Name(), "gh-") || !isExecutable(f) { - continue - } - results = append(results, filepath.Join(p, f.Name())) - seen[f.Name()] = struct{}{} - } - } - - return results +func (m *Manager) InstallLocal(dir string) error { + name := filepath.Base(dir) + targetDir := filepath.Join(m.installDir(), name) + return os.Symlink(dir, targetDir) } func (m *Manager) Install(cloneURL string, stdout, stderr io.Writer) error { @@ -124,13 +98,15 @@ func (m *Manager) Upgrade(stdout, stderr io.Writer) error { return err } - exts := m.listInstalled() + exts := m.List() if len(exts) == 0 { return errors.New("no extensions installed") } for _, f := range exts { - externalCmd := exec.Command(exe, "-C", filepath.Dir(f), "pull", "--ff-only") + fmt.Fprintf(stdout, "[%s]: ", filepath.Base(f)) + dir := filepath.Dir(f) + externalCmd := exec.Command(exe, "-C", dir, "--git-dir="+filepath.Join(dir, ".git"), "pull", "--ff-only") externalCmd.Stdout = stdout externalCmd.Stderr = stderr if e := externalCmd.Run(); e != nil { @@ -143,8 +119,3 @@ func (m *Manager) Upgrade(stdout, stderr io.Writer) error { func (m *Manager) installDir() string { return filepath.Join(m.dataDir(), "extensions") } - -// TODO: ignore file mode on Windows -func isExecutable(f os.FileInfo) bool { - return !f.IsDir() && f.Mode()&0111 != 0 -} From 4d46447eb3ebbb34f123b6e31e1485e0c7b3b0d5 Mon Sep 17 00:00:00 2001 From: Bruno Alla Date: Fri, 4 Jun 2021 15:29:01 +0100 Subject: [PATCH 25/54] Fix description for gh secret set --env option --- pkg/cmd/secret/set/set.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/secret/set/set.go b/pkg/cmd/secret/set/set.go index 8f6de703d..c1c75ced7 100644 --- a/pkg/cmd/secret/set/set.go +++ b/pkg/cmd/secret/set/set.go @@ -124,7 +124,7 @@ func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command }, } cmd.Flags().StringVarP(&opts.OrgName, "org", "o", "", "Set a secret for an organization") - cmd.Flags().StringVarP(&opts.EnvName, "env", "e", "", "Set a secret for an organization") + cmd.Flags().StringVarP(&opts.EnvName, "env", "e", "", "Set a secret for an environment") cmd.Flags().StringVarP(&opts.Visibility, "visibility", "v", "private", "Set visibility for an organization secret: `all`, `private`, or `selected`") cmd.Flags().StringSliceVarP(&opts.RepositoryNames, "repos", "r", []string{}, "List of repository names for `selected` visibility") cmd.Flags().StringVarP(&opts.Body, "body", "b", "", "A value for the secret. Reads from STDIN if not specified.") From 051520afe10c7e0219288db63cf8b691e37f950e Mon Sep 17 00:00:00 2001 From: Bruno Alla Date: Fri, 4 Jun 2021 16:32:21 +0100 Subject: [PATCH 26/54] Add a long command description for secrets remove --- pkg/cmd/secret/remove/remove.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/secret/remove/remove.go b/pkg/cmd/secret/remove/remove.go index 718e09050..fea4415ee 100644 --- a/pkg/cmd/secret/remove/remove.go +++ b/pkg/cmd/secret/remove/remove.go @@ -32,7 +32,8 @@ func NewCmdRemove(f *cmdutil.Factory, runF func(*RemoveOptions) error) *cobra.Co cmd := &cobra.Command{ Use: "remove ", - Short: "Remove an organization, environment, or repository secret", + Short: "Remove secrets", + Long: "Remove a secret for a repository, environment, or organization", Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { // support `-R, --repo` override From bcfe176594fcd7dbab1d42e7a9b8ccf723f9bea1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 4 Jun 2021 20:06:21 +0200 Subject: [PATCH 27/54] Fix flaky editor test There was a race condition wherein the test didn't wait enough time for the prompt to get rendered before testing the terminal output. --- pkg/surveyext/editor_test.go | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/pkg/surveyext/editor_test.go b/pkg/surveyext/editor_test.go index 03ad27989..c189c641b 100644 --- a/pkg/surveyext/editor_test.go +++ b/pkg/surveyext/editor_test.go @@ -59,8 +59,15 @@ func Test_GhEditor_Prompt(t *testing.T) { errc <- err }() - time.Sleep(5 * time.Millisecond) + for { + time.Sleep(time.Millisecond) + if strings.Contains(out.String(), "Body") { + break + } + } + assert.Equal(t, "\x1b[0G\x1b[2K\x1b[0;1;92m? \x1b[0m\x1b[0;1;99mBody \x1b[0m\x1b[0;36m[(e) to launch false, enter to skip] \x1b[0m\x1b[?25l", out.String()) + out.Reset() fmt.Fprint(pty, "\n") // send Enter key err = <-errc @@ -126,7 +133,12 @@ func (f *teeWriter) Write(p []byte) (n int, err error) { func (f *teeWriter) String() string { f.mu.Lock() s := f.buf.String() - f.buf.Reset() f.mu.Unlock() return s } + +func (f *teeWriter) Reset() { + f.mu.Lock() + f.buf.Reset() + f.mu.Unlock() +} From f570deb11859a0da7e956124e1800b8272616ff0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 4 Jun 2021 21:24:17 +0200 Subject: [PATCH 28/54] Add tests for opening the editor program --- pkg/surveyext/editor.go | 8 +- pkg/surveyext/editor_manual.go | 18 ++- pkg/surveyext/editor_test.go | 217 ++++++++++++++++++++++++++------- 3 files changed, 198 insertions(+), 45 deletions(-) diff --git a/pkg/surveyext/editor.go b/pkg/surveyext/editor.go index f868ed671..3a824d025 100644 --- a/pkg/surveyext/editor.go +++ b/pkg/surveyext/editor.go @@ -35,6 +35,8 @@ type GhEditor struct { *survey.Editor EditorCommand string BlankAllowed bool + + lookPath func(string) ([]string, []string, error) } func (e *GhEditor) editorCommand() string { @@ -136,7 +138,11 @@ func (e *GhEditor) prompt(initialValue string, config *survey.PromptConfig) (int } stdio := e.Stdio() - text, err := Edit(e.editorCommand(), e.FileName, initialValue, stdio.In, stdio.Out, stdio.Err, cursor) + lookPath := e.lookPath + if lookPath == nil { + lookPath = defaultLookPath + } + text, err := edit(e.editorCommand(), e.FileName, initialValue, stdio.In, stdio.Out, stdio.Err, cursor, lookPath) if err != nil { return "", err } diff --git a/pkg/surveyext/editor_manual.go b/pkg/surveyext/editor_manual.go index d5faeb063..93572ba70 100644 --- a/pkg/surveyext/editor_manual.go +++ b/pkg/surveyext/editor_manual.go @@ -16,6 +16,18 @@ type showable interface { } func Edit(editorCommand, fn, initialValue string, stdin io.Reader, stdout io.Writer, stderr io.Writer, cursor showable) (string, error) { + return edit(editorCommand, fn, initialValue, stdin, stdout, stderr, cursor, defaultLookPath) +} + +func defaultLookPath(name string) ([]string, []string, error) { + exe, err := safeexec.LookPath(name) + if err != nil { + return nil, nil, err + } + return []string{exe}, nil, nil +} + +func edit(editorCommand, fn, initialValue string, stdin io.Reader, stdout io.Writer, stderr io.Writer, cursor showable, lookPath func(string) ([]string, []string, error)) (string, error) { // prepare the temp file pattern := fn if pattern == "" { @@ -56,12 +68,14 @@ func Edit(editorCommand, fn, initialValue string, stdin io.Reader, stdout io.Wri } args = append(args, f.Name()) - editorExe, err := safeexec.LookPath(args[0]) + editorExe, env, err := lookPath(args[0]) if err != nil { return "", err } + args = append(editorExe, args[1:]...) - cmd := exec.Command(editorExe, args[1:]...) + cmd := exec.Command(args[0], args[1:]...) + cmd.Env = env cmd.Stdin = stdin cmd.Stdout = stdout cmd.Stderr = stderr diff --git a/pkg/surveyext/editor_test.go b/pkg/surveyext/editor_test.go index c189c641b..f98a98567 100644 --- a/pkg/surveyext/editor_test.go +++ b/pkg/surveyext/editor_test.go @@ -14,13 +14,50 @@ import ( "github.com/AlecAivazis/survey/v2/terminal" pseudotty "github.com/creack/pty" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) -func Test_GhEditor_Prompt(t *testing.T) { +func testLookPath(s string) ([]string, []string, error) { + return []string{os.Args[0], "-test.run=TestHelperProcess", "--", s}, []string{"GH_WANT_HELPER_PROCESS=1"}, nil +} + +func TestHelperProcess(t *testing.T) { + if os.Getenv("GH_WANT_HELPER_PROCESS") != "1" { + return + } + if err := func(args []string) error { + switch args[0] { + // "vim" appends a message to the file + case "vim": + f, err := os.OpenFile(args[1], os.O_APPEND|os.O_WRONLY, 0) + if err != nil { + return err + } + defer f.Close() + _, err = f.WriteString(" - added by vim") + return err + // "nano" truncates the contents of the file + case "nano": + f, err := os.OpenFile(args[1], os.O_TRUNC|os.O_WRONLY, 0) + if err != nil { + return err + } + return f.Close() + default: + return fmt.Errorf("unrecognized arguments: %#v\n", args) + } + }(os.Args[3:]); err != nil { + fmt.Fprintln(os.Stderr, err) + os.Exit(1) + } + os.Exit(0) +} + +func Test_GhEditor_Prompt_skip(t *testing.T) { + pty := newTerminal(t) + e := &GhEditor{ BlankAllowed: true, - EditorCommand: "false", + EditorCommand: "vim", Editor: &survey.Editor{ Message: "Body", FileName: "*.md", @@ -28,52 +65,86 @@ func Test_GhEditor_Prompt(t *testing.T) { HideDefault: true, AppendDefault: true, }, + lookPath: func(s string) ([]string, []string, error) { + return nil, nil, errors.New("no editor allowed") + }, } + e.WithStdio(pty.Stdio()) - pty, tty, err := pseudotty.Open() - if errors.Is(err, pseudotty.ErrUnsupported) { - return - } - require.NoError(t, err) - defer pty.Close() - defer tty.Close() - - err = pseudotty.Setsize(tty, &pseudotty.Winsize{Cols: 72, Rows: 30}) - require.NoError(t, err) - - out := teeWriter{File: tty} - e.WithStdio(terminal.Stdio{ - In: tty, - Out: &out, - Err: tty, - }) - - var res string - errc := make(chan error) - + // wait until the prompt is rendered and send the Enter key go func() { - r, err := e.Prompt(defaultPromptConfig()) - if r != nil { - res = r.(string) - } - errc <- err + pty.WaitForOutput("Body") + assert.Equal(t, "\x1b[0G\x1b[2K\x1b[0;1;92m? \x1b[0m\x1b[0;1;99mBody \x1b[0m\x1b[0;36m[(e) to launch vim, enter to skip] \x1b[0m\x1b[?25l", pty.Output()) + pty.ResetOutput() + assert.NoError(t, pty.SendKey('\n')) }() - for { - time.Sleep(time.Millisecond) - if strings.Contains(out.String(), "Body") { - break - } - } - - assert.Equal(t, "\x1b[0G\x1b[2K\x1b[0;1;92m? \x1b[0m\x1b[0;1;99mBody \x1b[0m\x1b[0;36m[(e) to launch false, enter to skip] \x1b[0m\x1b[?25l", out.String()) - out.Reset() - fmt.Fprint(pty, "\n") // send Enter key - - err = <-errc + res, err := e.Prompt(defaultPromptConfig()) assert.NoError(t, err) assert.Equal(t, "initial value", res) - assert.Equal(t, "\x1b[?25h", out.String()) + assert.Equal(t, "\x1b[?25h", pty.Output()) +} + +func Test_GhEditor_Prompt_editorAppend(t *testing.T) { + pty := newTerminal(t) + + e := &GhEditor{ + BlankAllowed: true, + EditorCommand: "vim", + Editor: &survey.Editor{ + Message: "Body", + FileName: "*.md", + Default: "initial value", + HideDefault: true, + AppendDefault: true, + }, + lookPath: testLookPath, + } + e.WithStdio(pty.Stdio()) + + // wait until the prompt is rendered and send the 'e' key + go func() { + pty.WaitForOutput("Body") + assert.Equal(t, "\x1b[0G\x1b[2K\x1b[0;1;92m? \x1b[0m\x1b[0;1;99mBody \x1b[0m\x1b[0;36m[(e) to launch vim, enter to skip] \x1b[0m\x1b[?25l", pty.Output()) + pty.ResetOutput() + assert.NoError(t, pty.SendKey('e')) + }() + + res, err := e.Prompt(defaultPromptConfig()) + assert.NoError(t, err) + assert.Equal(t, "initial value - added by vim", res) + assert.Equal(t, "\x1b[?25h\x1b[?25h", pty.Output()) +} + +func Test_GhEditor_Prompt_editorTruncate(t *testing.T) { + pty := newTerminal(t) + + e := &GhEditor{ + BlankAllowed: true, + EditorCommand: "nano", + Editor: &survey.Editor{ + Message: "Body", + FileName: "*.md", + Default: "initial value", + HideDefault: true, + AppendDefault: true, + }, + lookPath: testLookPath, + } + e.WithStdio(pty.Stdio()) + + // wait until the prompt is rendered and send the 'e' key + go func() { + pty.WaitForOutput("Body") + assert.Equal(t, "\x1b[0G\x1b[2K\x1b[0;1;92m? \x1b[0m\x1b[0;1;99mBody \x1b[0m\x1b[0;36m[(e) to launch nano, enter to skip] \x1b[0m\x1b[?25l", pty.Output()) + pty.ResetOutput() + assert.NoError(t, pty.SendKey('e')) + }() + + res, err := e.Prompt(defaultPromptConfig()) + assert.NoError(t, err) + assert.Equal(t, "", res) + assert.Equal(t, "\x1b[?25h\x1b[?25h", pty.Output()) } // survey doesn't expose this @@ -116,6 +187,68 @@ func defaultPromptConfig() *survey.PromptConfig { } } +type testTerminal struct { + pty *os.File + tty *os.File + stdout *teeWriter + stderr *teeWriter +} + +func newTerminal(t *testing.T) *testTerminal { + t.Helper() + + pty, tty, err := pseudotty.Open() + if errors.Is(err, pseudotty.ErrUnsupported) { + t.SkipNow() + return nil + } + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + pty.Close() + tty.Close() + }) + + if err := pseudotty.Setsize(tty, &pseudotty.Winsize{Cols: 72, Rows: 30}); err != nil { + t.Fatal(err) + } + + return &testTerminal{pty: pty, tty: tty} +} + +func (t *testTerminal) SendKey(c rune) error { + _, err := t.pty.WriteString(string(c)) + return err +} + +func (t *testTerminal) WaitForOutput(s string) { + for { + time.Sleep(time.Millisecond) + if strings.Contains(t.stdout.String(), s) { + return + } + } +} + +func (t *testTerminal) Output() string { + return t.stdout.String() +} + +func (t *testTerminal) ResetOutput() { + t.stdout.Reset() +} + +func (t *testTerminal) Stdio() terminal.Stdio { + t.stdout = &teeWriter{File: t.tty} + t.stderr = t.stdout + return terminal.Stdio{ + In: t.tty, + Out: t.stdout, + Err: t.stderr, + } +} + // teeWriter is a writer that duplicates all writes to a file into a buffer type teeWriter struct { *os.File From 606deaf134dc7c7eabeb2914b1e3302e2968b589 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 4 Jun 2021 21:32:54 +0200 Subject: [PATCH 29/54] Allow setting empty body via editor in `issue/pr create` --- pkg/cmd/issue/create/create.go | 4 --- pkg/cmd/issue/create/create_test.go | 8 +++-- pkg/cmd/pr/create/create.go | 4 --- pkg/cmd/pr/create/create_test.go | 12 +++---- pkg/cmd/pr/shared/params.go | 9 ++--- pkg/cmd/pr/shared/params_test.go | 55 ++++++++++++++++++++++++++++- pkg/cmd/pr/shared/survey.go | 2 +- 7 files changed, 71 insertions(+), 23 deletions(-) diff --git a/pkg/cmd/issue/create/create.go b/pkg/cmd/issue/create/create.go index a974d8d67..4ef2ab12f 100644 --- a/pkg/cmd/issue/create/create.go +++ b/pkg/cmd/issue/create/create.go @@ -234,10 +234,6 @@ func createRun(opts *CreateOptions) (err error) { if err != nil { return } - - if tb.Body == "" { - tb.Body = templateContent - } } openURL, err = generatePreviewURL(apiClient, baseRepo, tb) diff --git a/pkg/cmd/issue/create/create_test.go b/pkg/cmd/issue/create/create_test.go index 3e69f386c..f3d70b575 100644 --- a/pkg/cmd/issue/create/create_test.go +++ b/pkg/cmd/issue/create/create_test.go @@ -114,6 +114,8 @@ func TestNewCmdCreate(t *testing.T) { args, err := shlex.Split(tt.cli) require.NoError(t, err) cmd.SetArgs(args) + cmd.SetOut(ioutil.Discard) + cmd.SetErr(ioutil.Discard) _, err = cmd.ExecuteC() if tt.wantsErr { assert.Error(t, err) @@ -168,7 +170,7 @@ func Test_createRun(t *testing.T) { WebMode: true, Assignees: []string{"monalisa"}, }, - wantsBrowse: "https://github.com/OWNER/REPO/issues/new?assignees=monalisa", + wantsBrowse: "https://github.com/OWNER/REPO/issues/new?assignees=monalisa&body=", wantsStderr: "Opening github.com/OWNER/REPO/issues/new in your browser.\n", }, { @@ -185,7 +187,7 @@ func Test_createRun(t *testing.T) { "viewer": { "login": "MonaLisa" } } }`)) }, - wantsBrowse: "https://github.com/OWNER/REPO/issues/new?assignees=MonaLisa", + wantsBrowse: "https://github.com/OWNER/REPO/issues/new?assignees=MonaLisa&body=", wantsStderr: "Opening github.com/OWNER/REPO/issues/new in your browser.\n", }, { @@ -214,7 +216,7 @@ func Test_createRun(t *testing.T) { "pageInfo": { "hasNextPage": false } } } } }`)) }, - wantsBrowse: "https://github.com/OWNER/REPO/issues/new?projects=OWNER%2FREPO%2F1", + wantsBrowse: "https://github.com/OWNER/REPO/issues/new?body=&projects=OWNER%2FREPO%2F1", wantsStderr: "Opening github.com/OWNER/REPO/issues/new in your browser.\n", }, { diff --git a/pkg/cmd/pr/create/create.go b/pkg/cmd/pr/create/create.go index 7ab899e85..b2b40c65a 100644 --- a/pkg/cmd/pr/create/create.go +++ b/pkg/cmd/pr/create/create.go @@ -302,10 +302,6 @@ func createRun(opts *CreateOptions) (err error) { if err != nil { return } - - if state.Body == "" { - state.Body = templateContent - } } openURL, err = generateCompareURL(*ctx, *state) diff --git a/pkg/cmd/pr/create/create_test.go b/pkg/cmd/pr/create/create_test.go index a3d24c4d6..a19208892 100644 --- a/pkg/cmd/pr/create/create_test.go +++ b/pkg/cmd/pr/create/create_test.go @@ -239,7 +239,7 @@ func TestPRCreate_nontty_web(t *testing.T) { assert.Equal(t, "", output.String()) assert.Equal(t, "", output.Stderr()) - assert.Equal(t, "https://github.com/OWNER/REPO/compare/master...feature?expand=1", output.BrowsedURL) + assert.Equal(t, "https://github.com/OWNER/REPO/compare/master...feature?body=&expand=1", output.BrowsedURL) } func TestPRCreate_recover(t *testing.T) { @@ -780,7 +780,7 @@ func TestPRCreate_web(t *testing.T) { assert.Equal(t, "", output.String()) assert.Equal(t, "Opening github.com/OWNER/REPO/compare/master...feature in your browser.\n", output.Stderr()) - assert.Equal(t, "https://github.com/OWNER/REPO/compare/master...feature?expand=1", output.BrowsedURL) + assert.Equal(t, "https://github.com/OWNER/REPO/compare/master...feature?body=&expand=1", output.BrowsedURL) } func TestPRCreate_webLongURL(t *testing.T) { @@ -851,7 +851,7 @@ func TestPRCreate_webProject(t *testing.T) { assert.Equal(t, "", output.String()) assert.Equal(t, "Opening github.com/OWNER/REPO/compare/master...feature in your browser.\n", output.Stderr()) - assert.Equal(t, "https://github.com/OWNER/REPO/compare/master...feature?expand=1&projects=ORG%2F1", output.BrowsedURL) + assert.Equal(t, "https://github.com/OWNER/REPO/compare/master...feature?body=&expand=1&projects=ORG%2F1", output.BrowsedURL) } func Test_determineTrackingBranch_empty(t *testing.T) { @@ -965,7 +965,7 @@ func Test_generateCompareURL(t *testing.T) { BaseBranch: "main", HeadBranchLabel: "feature", }, - want: "https://github.com/OWNER/REPO/compare/main...feature?expand=1", + want: "https://github.com/OWNER/REPO/compare/main...feature?body=&expand=1", wantErr: false, }, { @@ -978,7 +978,7 @@ func Test_generateCompareURL(t *testing.T) { state: prShared.IssueMetadataState{ Labels: []string{"one", "two three"}, }, - want: "https://github.com/OWNER/REPO/compare/a...b?expand=1&labels=one%2Ctwo+three", + want: "https://github.com/OWNER/REPO/compare/a...b?body=&expand=1&labels=one%2Ctwo+three", wantErr: false, }, { @@ -988,7 +988,7 @@ func Test_generateCompareURL(t *testing.T) { BaseBranch: "main/trunk", HeadBranchLabel: "owner:feature", }, - want: "https://github.com/OWNER/REPO/compare/main%2Ftrunk...owner%3Afeature?expand=1", + want: "https://github.com/OWNER/REPO/compare/main%2Ftrunk...owner%3Afeature?body=&expand=1", wantErr: false, }, } diff --git a/pkg/cmd/pr/shared/params.go b/pkg/cmd/pr/shared/params.go index 93a5f2f58..ecd260645 100644 --- a/pkg/cmd/pr/shared/params.go +++ b/pkg/cmd/pr/shared/params.go @@ -2,13 +2,13 @@ package shared import ( "fmt" - "github.com/google/shlex" "net/url" "strings" "github.com/cli/cli/api" "github.com/cli/cli/internal/ghrepo" "github.com/cli/cli/pkg/githubsearch" + "github.com/google/shlex" ) func WithPrAndIssueQueryParams(client *api.Client, baseRepo ghrepo.Interface, baseURL string, state IssueMetadataState) (string, error) { @@ -20,9 +20,10 @@ func WithPrAndIssueQueryParams(client *api.Client, baseRepo ghrepo.Interface, ba if state.Title != "" { q.Set("title", state.Title) } - if state.Body != "" { - q.Set("body", state.Body) - } + // We always want to set body, even if it's empty, to prevent the web interface to apply the default + // template. Since the user has the option to select a template in the terminal, assume that empty body + // here means that the user either skipped it or erased its contents. + q.Set("body", state.Body) if len(state.Assignees) > 0 { q.Set("assignees", strings.Join(state.Assignees, ",")) } diff --git a/pkg/cmd/pr/shared/params_test.go b/pkg/cmd/pr/shared/params_test.go index 314218754..85dd9fa9e 100644 --- a/pkg/cmd/pr/shared/params_test.go +++ b/pkg/cmd/pr/shared/params_test.go @@ -1,7 +1,6 @@ package shared import ( - "github.com/stretchr/testify/assert" "net/http" "reflect" "testing" @@ -9,6 +8,7 @@ import ( "github.com/cli/cli/api" "github.com/cli/cli/internal/ghrepo" "github.com/cli/cli/pkg/httpmock" + "github.com/stretchr/testify/assert" ) func Test_listURLWithQuery(t *testing.T) { @@ -192,3 +192,56 @@ func Test_QueryHasStateClause(t *testing.T) { assert.Equal(t, tt.hasState, gotState) } } + +func Test_WithPrAndIssueQueryParams(t *testing.T) { + type args struct { + baseURL string + state IssueMetadataState + } + tests := []struct { + name string + args args + want string + wantErr bool + }{ + { + name: "blank", + args: args{ + baseURL: "", + state: IssueMetadataState{}, + }, + want: "?body=", + }, + { + name: "no values", + args: args{ + baseURL: "http://example.com/hey", + state: IssueMetadataState{}, + }, + want: "http://example.com/hey?body=", + }, + { + name: "title and body", + args: args{ + baseURL: "http://example.com/hey", + state: IssueMetadataState{ + Title: "my title", + Body: "my bodeh", + }, + }, + want: "http://example.com/hey?body=my+bodeh&title=my+title", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := WithPrAndIssueQueryParams(nil, nil, tt.args.baseURL, tt.args.state) + if (err != nil) != tt.wantErr { + t.Errorf("WithPrAndIssueQueryParams() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != tt.want { + t.Errorf("WithPrAndIssueQueryParams() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/pkg/cmd/pr/shared/survey.go b/pkg/cmd/pr/shared/survey.go index 5d8f13a37..15c930002 100644 --- a/pkg/cmd/pr/shared/survey.go +++ b/pkg/cmd/pr/shared/survey.go @@ -110,7 +110,7 @@ func BodySurvey(state *IssueMetadataState, templateContent, editorCommand string return err } - if state.Body != "" && preBody != state.Body { + if preBody != state.Body { state.MarkDirty() } From b3c2318e09023820b0235984ac449499fc5325f4 Mon Sep 17 00:00:00 2001 From: Cristian Dominguez Date: Fri, 4 Jun 2021 23:22:37 -0300 Subject: [PATCH 30/54] Increase `GH_PAGER` precedence If `GH_PAGER` is exists, set it as the pager even if one is already set in config. This allows a user to change/disable the pager per single invocation. --- cmd/gh/main.go | 5 +++-- pkg/iostreams/iostreams.go | 7 +------ 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/cmd/gh/main.go b/cmd/gh/main.go index 72b635810..891d4b98f 100644 --- a/cmd/gh/main.go +++ b/cmd/gh/main.go @@ -96,8 +96,9 @@ func mainRun() exitCode { if prompt, _ := cfg.Get("", "prompt"); prompt == "disabled" { cmdFactory.IOStreams.SetNeverPrompt(true) } - - if pager, _ := cfg.Get("", "pager"); pager != "" { + if ghPager, ghPagerExists := os.LookupEnv("GH_PAGER"); ghPagerExists { + cmdFactory.IOStreams.SetPager(ghPager) + } else if pager, _ := cfg.Get("", "pager"); pager != "" { cmdFactory.IOStreams.SetPager(pager) } diff --git a/pkg/iostreams/iostreams.go b/pkg/iostreams/iostreams.go index d6717ca8a..ec2503151 100644 --- a/pkg/iostreams/iostreams.go +++ b/pkg/iostreams/iostreams.go @@ -281,12 +281,7 @@ func System() *IOStreams { stdoutIsTTY := isTerminal(os.Stdout) stderrIsTTY := isTerminal(os.Stderr) - var pagerCommand string - if ghPager, ghPagerExists := os.LookupEnv("GH_PAGER"); ghPagerExists { - pagerCommand = ghPager - } else { - pagerCommand = os.Getenv("PAGER") - } + pagerCommand := os.Getenv("PAGER") io := &IOStreams{ In: os.Stdin, From 3caba02f3c222be27449b70b8c83f26017b3acc8 Mon Sep 17 00:00:00 2001 From: chemotaxis Date: Sat, 5 Jun 2021 00:35:30 -0400 Subject: [PATCH 31/54] Add documentation for installing via Conda Conda is a cross-platform package and environment manager, primarily associated with the scientific computing and data science communities. --- README.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/README.md b/README.md index 2886e3471..39856ec36 100644 --- a/README.md +++ b/README.md @@ -67,6 +67,18 @@ For more information and distro-specific instructions, see the [Linux installati MSI installers are available for download on the [releases page][]. +### Other package managers + +#### Conda + +`gh` is available via [Conda][], a cross-platform package and environment manager. + +| Install: | Upgrade: | +|------------------------------------------|-----------------------------------------| +| `conda install gh --channel conda-forge` | `conda update gh --channel conda-forge` | + +Additional installation options available on the [gh-feedstock page](https://github.com/conda-forge/gh-feedstock#installing-gh). + ### GitHub Actions GitHub CLI comes pre-installed in all [GitHub-Hosted Runners](https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners). @@ -93,6 +105,7 @@ tool. Check out our [more detailed explanation][gh-vs-hub] to learn more. [winget]: https://github.com/microsoft/winget-cli [scoop]: https://scoop.sh [Chocolatey]: https://chocolatey.org +[Conda]: https://docs.conda.io/en/latest/ [releases page]: https://github.com/cli/cli/releases/latest [hub]: https://github.com/github/hub [contributing]: ./.github/CONTRIBUTING.md From 71b738b553ff22f17fc7c00e83bb66d54d315a64 Mon Sep 17 00:00:00 2001 From: chemotaxis Date: Sat, 5 Jun 2021 00:42:39 -0400 Subject: [PATCH 32/54] Make whitespace consistent --- README.md | 3 --- 1 file changed, 3 deletions(-) diff --git a/README.md b/README.md index 39856ec36..6279f182c 100644 --- a/README.md +++ b/README.md @@ -14,7 +14,6 @@ GitHub CLI is available for repositories hosted on GitHub.com and GitHub Enterpr If anything feels off, or if you feel that some functionality is missing, please check out the [contributing page][contributing]. There you will find instructions for sharing your feedback, building the tool locally, and submitting pull requests to the project. - ## Installation @@ -44,7 +43,6 @@ For more information and distro-specific instructions, see the [Linux installati `gh` is available via [WinGet][], [scoop][], [Chocolatey][], and as downloadable MSI. - #### WinGet | Install: | Upgrade: | @@ -98,7 +96,6 @@ what an official GitHub CLI tool can look like with a fundamentally different de tools bring GitHub to the terminal, `hub` behaves as a proxy to `git`, and `gh` is a standalone tool. Check out our [more detailed explanation][gh-vs-hub] to learn more. - [manual]: https://cli.github.com/manual/ [Homebrew]: https://brew.sh [MacPorts]: https://www.macports.org From 71c6b8f43ab8fde443ce1ef2146246fae8ff07a1 Mon Sep 17 00:00:00 2001 From: chemotaxis Date: Sat, 5 Jun 2021 00:48:36 -0400 Subject: [PATCH 33/54] Add links to Conda section within each OS section --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 6279f182c..d9c96cf89 100644 --- a/README.md +++ b/README.md @@ -19,7 +19,7 @@ If anything feels off, or if you feel that some functionality is missing, please ### macOS -`gh` is available via [Homebrew][], [MacPorts][], and as a downloadable binary from the [releases page][]. +`gh` is available via [Homebrew][], [MacPorts][], [Conda](#Conda), and as a downloadable binary from the [releases page][]. #### Homebrew @@ -35,13 +35,13 @@ If anything feels off, or if you feel that some functionality is missing, please ### Linux -`gh` is available via [Homebrew](#homebrew), and as downloadable binaries from the [releases page][]. +`gh` is available via [Homebrew](#homebrew), [Conda](#Conda), and as downloadable binaries from the [releases page][]. For more information and distro-specific instructions, see the [Linux installation docs](./docs/install_linux.md). ### Windows -`gh` is available via [WinGet][], [scoop][], [Chocolatey][], and as downloadable MSI. +`gh` is available via [WinGet][], [scoop][], [Chocolatey][], [Conda](#Conda), and as downloadable MSI. #### WinGet From a72f6346dd9dba218156759047ecc4a98b95fff6 Mon Sep 17 00:00:00 2001 From: chemotaxis Date: Sat, 5 Jun 2021 12:37:55 -0400 Subject: [PATCH 34/54] Rearrange Conda installation instructions Originally, I was thinking of putting Conda in a separate section after the Windows section, since Conda probably isn't as well known or used. But reading through the readme again, it seems like arranging it like the other instructions makes more sense. I found myself trying to look for the instructions when I first read it in the MacOS section, but couldn't find the instructions. --- README.md | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index d9c96cf89..49d11ff70 100644 --- a/README.md +++ b/README.md @@ -19,7 +19,7 @@ If anything feels off, or if you feel that some functionality is missing, please ### macOS -`gh` is available via [Homebrew][], [MacPorts][], [Conda](#Conda), and as a downloadable binary from the [releases page][]. +`gh` is available via [Homebrew][], [MacPorts][], [Conda][], and as a downloadable binary from the [releases page][]. #### Homebrew @@ -33,6 +33,14 @@ If anything feels off, or if you feel that some functionality is missing, please | ---------------------- | ---------------------------------------------- | | `sudo port install gh` | `sudo port selfupdate && sudo port upgrade gh` | +#### Conda + +| Install: | Upgrade: | +|------------------------------------------|-----------------------------------------| +| `conda install gh --channel conda-forge` | `conda update gh --channel conda-forge` | + +Additional Conda installation options available on the [gh-feedstock page](https://github.com/conda-forge/gh-feedstock#installing-gh). + ### Linux `gh` is available via [Homebrew](#homebrew), [Conda](#Conda), and as downloadable binaries from the [releases page][]. @@ -65,18 +73,6 @@ For more information and distro-specific instructions, see the [Linux installati MSI installers are available for download on the [releases page][]. -### Other package managers - -#### Conda - -`gh` is available via [Conda][], a cross-platform package and environment manager. - -| Install: | Upgrade: | -|------------------------------------------|-----------------------------------------| -| `conda install gh --channel conda-forge` | `conda update gh --channel conda-forge` | - -Additional installation options available on the [gh-feedstock page](https://github.com/conda-forge/gh-feedstock#installing-gh). - ### GitHub Actions GitHub CLI comes pre-installed in all [GitHub-Hosted Runners](https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners). From 8a221bb766e5841dd733167c1395e3ff6accfed8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 8 Jun 2021 19:21:48 +0200 Subject: [PATCH 35/54] Add tests for our default HTTP client --- pkg/cmd/factory/http.go | 18 +++- pkg/cmd/factory/http_test.go | 158 +++++++++++++++++++++++++++++++++++ 2 files changed, 172 insertions(+), 4 deletions(-) create mode 100644 pkg/cmd/factory/http_test.go diff --git a/pkg/cmd/factory/http.go b/pkg/cmd/factory/http.go index 2e7619540..0005c6022 100644 --- a/pkg/cmd/factory/http.go +++ b/pkg/cmd/factory/http.go @@ -8,7 +8,6 @@ import ( "time" "github.com/cli/cli/api" - "github.com/cli/cli/internal/config" "github.com/cli/cli/internal/ghinstance" "github.com/cli/cli/pkg/iostreams" ) @@ -53,8 +52,12 @@ var timezoneNames = map[int]string{ 50400: "Pacific/Kiritimati", } +type configGetter interface { + Get(string, string) (string, error) +} + // generic authenticated HTTP client for commands -func NewHTTPClient(io *iostreams.IOStreams, cfg config.Config, appVersion string, setAccept bool) *http.Client { +func NewHTTPClient(io *iostreams.IOStreams, cfg configGetter, appVersion string, setAccept bool) *http.Client { var opts []api.ClientOption if verbose := os.Getenv("DEBUG"); verbose != "" { logTraffic := strings.Contains(verbose, "api") @@ -64,7 +67,7 @@ func NewHTTPClient(io *iostreams.IOStreams, cfg config.Config, appVersion string opts = append(opts, api.AddHeader("User-Agent", fmt.Sprintf("GitHub CLI %s", appVersion)), api.AddHeaderFunc("Authorization", func(req *http.Request) (string, error) { - hostname := ghinstance.NormalizeHostname(req.URL.Hostname()) + hostname := ghinstance.NormalizeHostname(getHost(req)) if token, err := cfg.Get(hostname, "oauth_token"); err == nil && token != "" { return fmt.Sprintf("token %s", token), nil } @@ -89,7 +92,7 @@ func NewHTTPClient(io *iostreams.IOStreams, cfg config.Config, appVersion string accept := "application/vnd.github.antiope-preview+json" // introduced for #2952: pr branch up to date status accept += ", application/vnd.github.merge-info-preview+json" - if ghinstance.IsEnterprise(req.URL.Hostname()) { + if ghinstance.IsEnterprise(getHost(req)) { // shadow-cat-preview: Draft pull requests accept += ", application/vnd.github.shadow-cat-preview" } @@ -100,3 +103,10 @@ func NewHTTPClient(io *iostreams.IOStreams, cfg config.Config, appVersion string return api.NewHTTPClient(opts...) } + +func getHost(r *http.Request) string { + if r.Host != "" { + return r.Host + } + return r.URL.Hostname() +} diff --git a/pkg/cmd/factory/http_test.go b/pkg/cmd/factory/http_test.go new file mode 100644 index 000000000..f52a8704f --- /dev/null +++ b/pkg/cmd/factory/http_test.go @@ -0,0 +1,158 @@ +package factory + +import ( + "fmt" + "net/http" + "net/http/httptest" + "os" + "regexp" + "testing" + + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/pkg/iostreams" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestNewHTTPClient(t *testing.T) { + type args struct { + config configGetter + appVersion string + setAccept bool + } + tests := []struct { + name string + args args + envDebug string + host string + wantHeader map[string]string + wantStderr string + }{ + { + name: "github.com with Accept header", + args: args{ + config: tinyConfig{"github.com:oauth_token": "MYTOKEN"}, + appVersion: "v1.2.3", + setAccept: true, + }, + host: "github.com", + wantHeader: map[string]string{ + "authorization": "token MYTOKEN", + "user-agent": "GitHub CLI v1.2.3", + "accept": "application/vnd.github.antiope-preview+json, application/vnd.github.merge-info-preview+json", + }, + wantStderr: "", + }, + { + name: "github.com no Accept header", + args: args{ + config: tinyConfig{"example.com:oauth_token": "MYTOKEN"}, + appVersion: "v1.2.3", + setAccept: false, + }, + host: "github.com", + wantHeader: map[string]string{ + "authorization": "", + "user-agent": "GitHub CLI v1.2.3", + "accept": "", + }, + wantStderr: "", + }, + { + name: "github.com in verbose mode", + args: args{ + config: tinyConfig{"github.com:oauth_token": "MYTOKEN"}, + appVersion: "v1.2.3", + setAccept: false, + }, + host: "github.com", + envDebug: "api", + wantHeader: map[string]string{ + "authorization": "token MYTOKEN", + "user-agent": "GitHub CLI v1.2.3", + "accept": "", + }, + wantStderr: heredoc.Doc(` + * Request at