From ca2f2b18ca507db40c2b7a72f371be8e4fc98b32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 4 Sep 2020 21:32:58 +0200 Subject: [PATCH 01/67] Add support for CLICOLOR "standard" The CLICOLOR_FORCE environment variable can now be used to force color output even when stdout is redirected. https://bixense.com/clicolors/ --- pkg/cmd/root/root.go | 7 ++++++- pkg/iostreams/iostreams.go | 5 ++++- utils/color.go | 6 +++++- 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/pkg/cmd/root/root.go b/pkg/cmd/root/root.go index 252636e52..fe2d23d8c 100644 --- a/pkg/cmd/root/root.go +++ b/pkg/cmd/root/root.go @@ -61,7 +61,12 @@ func NewCmdRoot(f *cmdutil.Factory, version, buildDate string) *cobra.Command { GLAMOUR_STYLE: the style to use for rendering Markdown. See https://github.com/charmbracelet/glamour#styles - NO_COLOR: avoid printing ANSI escape sequences for color output. + NO_COLOR: set to any value to avoid printing ANSI escape sequences for color output. + + CLICOLOR: set to "0" to disable printing ANSI colors in output. + + CLICOLOR_FORCE: set to a value other than "0" to keep ANSI colors in output + even when the output is piped. `), }, } diff --git a/pkg/iostreams/iostreams.go b/pkg/iostreams/iostreams.go index e9b9c2220..bbaaecda4 100644 --- a/pkg/iostreams/iostreams.go +++ b/pkg/iostreams/iostreams.go @@ -106,11 +106,14 @@ func System() *IOStreams { stdoutIsTTY := isTerminal(os.Stdout) stderrIsTTY := isTerminal(os.Stderr) + envNoColor := os.Getenv("NO_COLOR") != "" || os.Getenv("CLICOLOR") == "0" + envForceColor := os.Getenv("CLICOLOR_FORCE") != "" && os.Getenv("CLICOLOR_FORCE") != "0" + io := &IOStreams{ In: os.Stdin, Out: colorable.NewColorable(os.Stdout), ErrOut: colorable.NewColorable(os.Stderr), - colorEnabled: os.Getenv("NO_COLOR") == "" && stdoutIsTTY, + colorEnabled: envForceColor || (!envNoColor && stdoutIsTTY), } // prevent duplicate isTerminal queries now that we know the answer diff --git a/utils/color.go b/utils/color.go index 123e0927c..034dd54d7 100644 --- a/utils/color.go +++ b/utils/color.go @@ -39,7 +39,11 @@ func makeColorFunc(color string) func(string) string { } func isColorEnabled() bool { - if os.Getenv("NO_COLOR") != "" { + if os.Getenv("CLICOLOR_FORCE") != "" && os.Getenv("CLICOLOR_FORCE") != "0" { + return true + } + + if os.Getenv("NO_COLOR") != "" || os.Getenv("CLICOLOR") == "0" { return false } From d01355e24bc881797c1b0a61d203c5d90d35dcf8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 4 Sep 2020 23:07:17 +0200 Subject: [PATCH 02/67] Add global PAGER support Extract the ad-hoc PAGER behavior from `pr diff` command and make it available opt-in to any command through IOStreams. --- pkg/cmd/pr/diff/diff.go | 26 +++++-------------- pkg/cmd/pr/diff/diff_test.go | 41 +---------------------------- pkg/iostreams/iostreams.go | 50 ++++++++++++++++++++++++++++++++++++ 3 files changed, 57 insertions(+), 60 deletions(-) diff --git a/pkg/cmd/pr/diff/diff.go b/pkg/cmd/pr/diff/diff.go index c6902e5b5..3c55e7db8 100644 --- a/pkg/cmd/pr/diff/diff.go +++ b/pkg/cmd/pr/diff/diff.go @@ -6,8 +6,6 @@ import ( "fmt" "io" "net/http" - "os" - "os/exec" "strings" "github.com/cli/cli/api" @@ -16,7 +14,6 @@ import ( "github.com/cli/cli/pkg/cmd/pr/shared" "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/iostreams" - "github.com/google/shlex" "github.com/spf13/cobra" ) @@ -93,17 +90,17 @@ func diffRun(opts *DiffOptions) error { } defer diff.Close() + err = opts.IO.StartPager() + if err != nil { + return err + } + defer opts.IO.StopPager() + if opts.UseColor == "never" { _, err = io.Copy(opts.IO.Out, diff) return err } - if opts.IO.IsStdoutTTY() { - if pager := os.Getenv("PAGER"); pager != "" { - return runPager(pager, diff, opts.IO.Out) - } - } - diffLines := bufio.NewScanner(diff) for diffLines.Scan() { diffLine := diffLines.Text() @@ -148,14 +145,3 @@ func isRemovalLine(dl string) bool { func validColorFlag(c string) bool { return c == "auto" || c == "always" || c == "never" } - -var runPager = func(pager string, diff io.Reader, out io.Writer) error { - args, err := shlex.Split(pager) - if err != nil { - return err - } - pagerCmd := exec.Command(args[0], args[1:]...) - pagerCmd.Stdin = diff - pagerCmd.Stdout = out - return pagerCmd.Run() -} diff --git a/pkg/cmd/pr/diff/diff_test.go b/pkg/cmd/pr/diff/diff_test.go index dc93970b6..5e362ac4d 100644 --- a/pkg/cmd/pr/diff/diff_test.go +++ b/pkg/cmd/pr/diff/diff_test.go @@ -2,10 +2,8 @@ package diff import ( "bytes" - "io" "io/ioutil" "net/http" - "os" "testing" "github.com/cli/cli/context" @@ -214,13 +212,8 @@ func TestPRDiff_notty(t *testing.T) { } func TestPRDiff_tty(t *testing.T) { - pager := os.Getenv("PAGER") http := &httpmock.Registry{} - defer func() { - os.Setenv("PAGER", pager) - http.Verify(t) - }() - os.Setenv("PAGER", "") + defer http.Verify(t) http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { "pullRequests": { "nodes": [ { "url": "https://github.com/OWNER/REPO/pull/123", @@ -237,38 +230,6 @@ func TestPRDiff_tty(t *testing.T) { assert.Contains(t, output.String(), "\x1b[32m+site: bin/gh\x1b[m") } -func TestPRDiff_pager(t *testing.T) { - realRunPager := runPager - pager := os.Getenv("PAGER") - http := &httpmock.Registry{} - defer func() { - runPager = realRunPager - os.Setenv("PAGER", pager) - http.Verify(t) - }() - runPager = func(pager string, diff io.Reader, out io.Writer) error { - _, err := io.Copy(out, diff) - return err - } - os.Setenv("PAGER", "fakepager") - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "pullRequests": { "nodes": [ - { "url": "https://github.com/OWNER/REPO/pull/123", - "number": 123, - "id": "foobar123", - "headRefName": "feature", - "baseRefName": "master" } - ] } } } }`)) - http.StubResponse(200, bytes.NewBufferString(testDiff)) - output, err := runCommand(http, nil, true, "") - if err != nil { - t.Fatalf("unexpected error: %s", err) - } - if diff := cmp.Diff(testDiff, output.String()); diff != "" { - t.Errorf("command output did not match:\n%s", diff) - } -} - const testDiff = `diff --git a/.github/workflows/releases.yml b/.github/workflows/releases.yml index 73974448..b7fc0154 100644 --- a/.github/workflows/releases.yml diff --git a/pkg/iostreams/iostreams.go b/pkg/iostreams/iostreams.go index e9b9c2220..d3f7e3c11 100644 --- a/pkg/iostreams/iostreams.go +++ b/pkg/iostreams/iostreams.go @@ -10,6 +10,7 @@ import ( "strconv" "strings" + "github.com/google/shlex" "github.com/mattn/go-colorable" "github.com/mattn/go-isatty" "golang.org/x/crypto/ssh/terminal" @@ -28,6 +29,9 @@ type IOStreams struct { stdoutIsTTY bool stderrTTYOverride bool stderrIsTTY bool + + pagerCommand string + pagerProcess *os.Process } func (s *IOStreams) ColorEnabled() bool { @@ -79,6 +83,51 @@ func (s *IOStreams) IsStderrTTY() bool { return false } +func (s *IOStreams) StartPager() error { + if s.pagerCommand == "" || !s.IsStdoutTTY() { + return nil + } + + pagerArgs, err := shlex.Split(s.pagerCommand) + if err != nil { + return err + } + + pagerEnv := os.Environ() + if _, ok := os.LookupEnv("LESS"); !ok { + pagerEnv = append(pagerEnv, "LESS=FRX") + } + if _, ok := os.LookupEnv("LV"); !ok { + pagerEnv = append(pagerEnv, "LV=-c") + } + + pagerCmd := exec.Command(pagerArgs[0], pagerArgs[1:]...) + pagerCmd.Env = pagerEnv + pagerCmd.Stdout = s.Out + pagerCmd.Stderr = s.ErrOut + pagedOut, err := pagerCmd.StdinPipe() + if err != nil { + return err + } + s.Out = pagedOut + err = pagerCmd.Start() + if err != nil { + return err + } + s.pagerProcess = pagerCmd.Process + return nil +} + +func (s *IOStreams) StopPager() { + if s.pagerProcess == nil { + return + } + + s.Out.(io.ReadCloser).Close() + _, _ = s.pagerProcess.Wait() + s.pagerProcess = nil +} + func (s *IOStreams) TerminalWidth() int { defaultWidth := 80 if s.stdoutTTYOverride { @@ -111,6 +160,7 @@ func System() *IOStreams { Out: colorable.NewColorable(os.Stdout), ErrOut: colorable.NewColorable(os.Stderr), colorEnabled: os.Getenv("NO_COLOR") == "" && stdoutIsTTY, + pagerCommand: os.Getenv("PAGER"), } // prevent duplicate isTerminal queries now that we know the answer From ccfe38de260f06bd399eaf6daa53d60980cf4397 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 4 Sep 2020 23:08:09 +0200 Subject: [PATCH 03/67] Add PAGER support to commands that produce significant output --- pkg/cmd/issue/list/fixtures/issueList.json | 3 ++ pkg/cmd/issue/list/list.go | 8 +++- pkg/cmd/issue/list/list_test.go | 24 ++++++----- pkg/cmd/issue/status/status.go | 6 +++ pkg/cmd/issue/view/view.go | 8 +++- pkg/cmd/pr/list/list.go | 8 +++- pkg/cmd/pr/list/list_test.go | 47 ++++++++-------------- pkg/cmd/pr/status/status.go | 6 +++ pkg/cmd/pr/view/view.go | 6 +++ pkg/cmd/repo/view/view.go | 14 ++++++- 10 files changed, 86 insertions(+), 44 deletions(-) diff --git a/pkg/cmd/issue/list/fixtures/issueList.json b/pkg/cmd/issue/list/fixtures/issueList.json index 4b19f3930..1f878f7a5 100644 --- a/pkg/cmd/issue/list/fixtures/issueList.json +++ b/pkg/cmd/issue/list/fixtures/issueList.json @@ -9,6 +9,7 @@ "number": 1, "title": "number won", "url": "https://wow.com", + "updatedAt": "2011-01-26T19:01:12Z", "labels": { "nodes": [ { @@ -22,6 +23,7 @@ "number": 2, "title": "number too", "url": "https://wow.com", + "updatedAt": "2011-01-26T19:01:12Z", "labels": { "nodes": [ { @@ -35,6 +37,7 @@ "number": 4, "title": "number fore", "url": "https://wow.com", + "updatedAt": "2011-01-26T19:01:12Z", "labels": { "nodes": [ { diff --git a/pkg/cmd/issue/list/list.go b/pkg/cmd/issue/list/list.go index 1065e61be..017556af6 100644 --- a/pkg/cmd/issue/list/list.go +++ b/pkg/cmd/issue/list/list.go @@ -116,10 +116,16 @@ func listRun(opts *ListOptions) error { return err } + err = opts.IO.StartPager() + if err != nil { + return err + } + defer opts.IO.StopPager() + if isTerminal { hasFilters := opts.State != "open" || len(opts.Labels) > 0 || opts.Assignee != "" || opts.Author != "" || opts.Mention != "" || opts.Milestone != "" title := prShared.ListHeader(ghrepo.FullName(baseRepo), "issue", len(listResult.Issues), listResult.TotalCount, hasFilters) - fmt.Fprintf(opts.IO.ErrOut, "\n%s\n\n", title) + fmt.Fprintf(opts.IO.Out, "\n%s\n\n", title) } issueShared.PrintIssues(opts.IO, "", len(listResult.Issues), listResult.Issues) diff --git a/pkg/cmd/issue/list/list_test.go b/pkg/cmd/issue/list/list_test.go index 6646a10f7..ff202b6f3 100644 --- a/pkg/cmd/issue/list/list_test.go +++ b/pkg/cmd/issue/list/list_test.go @@ -7,8 +7,10 @@ import ( "net/http" "os/exec" "reflect" + "regexp" "testing" + "github.com/MakeNowJust/heredoc" "github.com/cli/cli/internal/config" "github.com/cli/cli/internal/ghrepo" "github.com/cli/cli/internal/run" @@ -97,15 +99,19 @@ func TestIssueList_tty(t *testing.T) { t.Errorf("error running command `issue list`: %v", err) } - eq(t, output.Stderr(), ` -Showing 3 of 3 open issues in OWNER/REPO + out := output.String() + timeRE := regexp.MustCompile(`\d+ years`) + out = timeRE.ReplaceAllString(out, "X years") -`) + assert.Equal(t, heredoc.Doc(` - test.ExpectLines(t, output.String(), - "number won", - "number too", - "number fore") + Showing 3 of 3 open issues in OWNER/REPO + + #1 number won (label) about X years ago + #2 number too (label) about X years ago + #4 number fore (label) about X years ago + `), out) + assert.Equal(t, ``, output.Stderr()) } func TestIssueList_tty_withFlags(t *testing.T) { @@ -141,8 +147,8 @@ func TestIssueList_tty_withFlags(t *testing.T) { t.Errorf("error running command `issue list`: %v", err) } - eq(t, output.String(), "") - eq(t, output.Stderr(), ` + eq(t, output.Stderr(), "") + eq(t, output.String(), ` No issues match your search in OWNER/REPO `) diff --git a/pkg/cmd/issue/status/status.go b/pkg/cmd/issue/status/status.go index d1b68bc0d..42ba91823 100644 --- a/pkg/cmd/issue/status/status.go +++ b/pkg/cmd/issue/status/status.go @@ -68,6 +68,12 @@ func statusRun(opts *StatusOptions) error { return err } + err = opts.IO.StartPager() + if err != nil { + return err + } + defer opts.IO.StopPager() + out := opts.IO.Out fmt.Fprintln(out, "") diff --git a/pkg/cmd/issue/view/view.go b/pkg/cmd/issue/view/view.go index 6d2b0eeaa..7ff7a0e43 100644 --- a/pkg/cmd/issue/view/view.go +++ b/pkg/cmd/issue/view/view.go @@ -86,10 +86,16 @@ func viewRun(opts *ViewOptions) error { fmt.Fprintf(opts.IO.ErrOut, "Opening %s in your browser.\n", openURL) return utils.OpenInBrowser(openURL) } + + err = opts.IO.StartPager() + if err != nil { + return err + } + defer opts.IO.StopPager() + if opts.IO.IsStdoutTTY() { return printHumanIssuePreview(opts.IO.Out, issue) } - return printRawIssuePreview(opts.IO.Out, issue) } diff --git a/pkg/cmd/pr/list/list.go b/pkg/cmd/pr/list/list.go index fd1ebb2b9..e4fe318c8 100644 --- a/pkg/cmd/pr/list/list.go +++ b/pkg/cmd/pr/list/list.go @@ -133,10 +133,16 @@ func listRun(opts *ListOptions) error { return err } + err = opts.IO.StartPager() + if err != nil { + return err + } + defer opts.IO.StopPager() + if opts.IO.IsStdoutTTY() { hasFilters := opts.State != "open" || len(opts.Labels) > 0 || opts.BaseBranch != "" || opts.Assignee != "" title := shared.ListHeader(ghrepo.FullName(baseRepo), "pull request", len(listResult.PullRequests), listResult.TotalCount, hasFilters) - fmt.Fprintf(opts.IO.ErrOut, "\n%s\n\n", title) + fmt.Fprintf(opts.IO.Out, "\n%s\n\n", title) } table := utils.NewTablePrinter(opts.IO) diff --git a/pkg/cmd/pr/list/list_test.go b/pkg/cmd/pr/list/list_test.go index 5faf42b57..ab99e4aa9 100644 --- a/pkg/cmd/pr/list/list_test.go +++ b/pkg/cmd/pr/list/list_test.go @@ -6,10 +6,10 @@ import ( "net/http" "os/exec" "reflect" - "regexp" "strings" "testing" + "github.com/MakeNowJust/heredoc" "github.com/cli/cli/internal/ghrepo" "github.com/cli/cli/internal/run" "github.com/cli/cli/pkg/cmdutil" @@ -76,23 +76,15 @@ func TestPRList(t *testing.T) { t.Fatal(err) } - assert.Equal(t, ` -Showing 3 of 3 open pull requests in OWNER/REPO + assert.Equal(t, heredoc.Doc(` -`, output.Stderr()) - - lines := strings.Split(output.String(), "\n") - res := []*regexp.Regexp{ - regexp.MustCompile(`#32.*New feature.*feature`), - regexp.MustCompile(`#29.*Fixed bad bug.*hubot:bug-fix`), - regexp.MustCompile(`#28.*Improve documentation.*docs`), - } - - for i, r := range res { - if !r.MatchString(lines[i]) { - t.Errorf("%s did not match %s", lines[i], r) - } - } + Showing 3 of 3 open pull requests in OWNER/REPO + + #32 New feature feature + #29 Fixed bad bug hubot:bug-fix + #28 Improve documentation docs + `), output.String()) + assert.Equal(t, ``, output.Stderr()) } func TestPRList_nontty(t *testing.T) { @@ -130,8 +122,8 @@ func TestPRList_filtering(t *testing.T) { t.Fatal(err) } - eq(t, output.String(), "") - eq(t, output.Stderr(), ` + eq(t, output.Stderr(), "") + eq(t, output.String(), ` No pull requests match your search in OWNER/REPO `) @@ -150,19 +142,12 @@ func TestPRList_filteringRemoveDuplicate(t *testing.T) { t.Fatal(err) } - lines := strings.Split(output.String(), "\n") - - res := []*regexp.Regexp{ - regexp.MustCompile(`#32.*New feature.*feature`), - regexp.MustCompile(`#29.*Fixed bad bug.*hubot:bug-fix`), - regexp.MustCompile(`#28.*Improve documentation.*docs`), - } - - for i, r := range res { - if !r.MatchString(lines[i]) { - t.Errorf("%s did not match %s", lines[i], r) - } + out := output.String() + idx := strings.Index(out, "New feature") + if idx < 0 { + t.Fatalf("text %q not found in %q", "New feature", out) } + assert.Equal(t, idx, strings.LastIndex(out, "New feature")) } func TestPRList_filteringClosed(t *testing.T) { diff --git a/pkg/cmd/pr/status/status.go b/pkg/cmd/pr/status/status.go index dffd2b412..2c012d712 100644 --- a/pkg/cmd/pr/status/status.go +++ b/pkg/cmd/pr/status/status.go @@ -97,6 +97,12 @@ func statusRun(opts *StatusOptions) error { return err } + err = opts.IO.StartPager() + if err != nil { + return err + } + defer opts.IO.StopPager() + out := opts.IO.Out fmt.Fprintln(out, "") diff --git a/pkg/cmd/pr/view/view.go b/pkg/cmd/pr/view/view.go index b653e5ffe..e8b5398b2 100644 --- a/pkg/cmd/pr/view/view.go +++ b/pkg/cmd/pr/view/view.go @@ -99,6 +99,12 @@ func viewRun(opts *ViewOptions) error { return utils.OpenInBrowser(openURL) } + err = opts.IO.StartPager() + if err != nil { + return err + } + defer opts.IO.StopPager() + if connectedToTerminal { return printHumanPrPreview(opts.IO.Out, pr) } diff --git a/pkg/cmd/repo/view/view.go b/pkg/cmd/repo/view/view.go index adcf384a4..f5a70f54b 100644 --- a/pkg/cmd/repo/view/view.go +++ b/pkg/cmd/repo/view/view.go @@ -1,10 +1,13 @@ package view import ( + "errors" "fmt" "html/template" "net/http" + "os" "strings" + "syscall" "github.com/MakeNowJust/heredoc" "github.com/cli/cli/api" @@ -107,6 +110,12 @@ func viewRun(opts *ViewOptions) error { return err } + err = opts.IO.StartPager() + if err != nil { + return err + } + defer opts.IO.StopPager() + stdout := opts.IO.Out if !opts.IO.IsStdoutTTY() { @@ -167,7 +176,10 @@ func viewRun(opts *ViewOptions) error { err = tmpl.Execute(stdout, repoData) if err != nil { - return err + var pathError *os.PathError + if !errors.As(err, &pathError) || pathError.Op != "write" || pathError.Unwrap() != syscall.EPIPE { + return err + } } return nil From 84b1b3643328af13b4ee1c5b8f97d2cc2e0033b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 7 Sep 2020 12:41:28 +0200 Subject: [PATCH 04/67] Simplify handling the "broken pipe" error --- pkg/cmd/repo/view/view.go | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/pkg/cmd/repo/view/view.go b/pkg/cmd/repo/view/view.go index f5a70f54b..58eecbc01 100644 --- a/pkg/cmd/repo/view/view.go +++ b/pkg/cmd/repo/view/view.go @@ -5,7 +5,6 @@ import ( "fmt" "html/template" "net/http" - "os" "strings" "syscall" @@ -175,11 +174,8 @@ func viewRun(opts *ViewOptions) error { } err = tmpl.Execute(stdout, repoData) - if err != nil { - var pathError *os.PathError - if !errors.As(err, &pathError) || pathError.Op != "write" || pathError.Unwrap() != syscall.EPIPE { - return err - } + if err != nil && !errors.Is(err, syscall.EPIPE) { + return err } return nil From d38a0c5d842c78d6f17cbe8db29f4af4c231068b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 7 Sep 2020 12:50:12 +0200 Subject: [PATCH 05/67] Encapsulate checking color-controlling environment variables --- pkg/iostreams/color.go | 11 +++ pkg/iostreams/color_test.go | 145 ++++++++++++++++++++++++++++++++++++ pkg/iostreams/iostreams.go | 5 +- utils/color.go | 5 +- 4 files changed, 160 insertions(+), 6 deletions(-) create mode 100644 pkg/iostreams/color.go create mode 100644 pkg/iostreams/color_test.go diff --git a/pkg/iostreams/color.go b/pkg/iostreams/color.go new file mode 100644 index 000000000..c5e476791 --- /dev/null +++ b/pkg/iostreams/color.go @@ -0,0 +1,11 @@ +package iostreams + +import "os" + +func EnvColorDisabled() bool { + return os.Getenv("NO_COLOR") != "" || os.Getenv("CLICOLOR") == "0" +} + +func EnvColorForced() bool { + return os.Getenv("CLICOLOR_FORCE") != "" && os.Getenv("CLICOLOR_FORCE") != "0" +} diff --git a/pkg/iostreams/color_test.go b/pkg/iostreams/color_test.go new file mode 100644 index 000000000..90b3ae024 --- /dev/null +++ b/pkg/iostreams/color_test.go @@ -0,0 +1,145 @@ +package iostreams + +import ( + "os" + "testing" +) + +func TestEnvColorDisabled(t *testing.T) { + orig_NO_COLOR := os.Getenv("NO_COLOR") + orig_CLICOLOR := os.Getenv("CLICOLOR") + orig_CLICOLOR_FORCE := os.Getenv("CLICOLOR_FORCE") + t.Cleanup(func() { + os.Setenv("NO_COLOR", orig_NO_COLOR) + os.Setenv("CLICOLOR", orig_CLICOLOR) + os.Setenv("CLICOLOR_FORCE", orig_CLICOLOR_FORCE) + }) + + tests := []struct { + name string + NO_COLOR string + CLICOLOR string + CLICOLOR_FORCE string + want bool + }{ + { + name: "pristine env", + NO_COLOR: "", + CLICOLOR: "", + CLICOLOR_FORCE: "", + want: false, + }, + { + name: "NO_COLOR enabled", + NO_COLOR: "1", + CLICOLOR: "", + CLICOLOR_FORCE: "", + want: true, + }, + { + name: "CLICOLOR disabled", + NO_COLOR: "", + CLICOLOR: "0", + CLICOLOR_FORCE: "", + want: true, + }, + { + name: "CLICOLOR enabled", + NO_COLOR: "", + CLICOLOR: "1", + CLICOLOR_FORCE: "", + want: false, + }, + { + name: "CLICOLOR_FORCE has no effect", + NO_COLOR: "", + CLICOLOR: "", + CLICOLOR_FORCE: "1", + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + os.Setenv("NO_COLOR", tt.NO_COLOR) + os.Setenv("CLICOLOR", tt.CLICOLOR) + os.Setenv("CLICOLOR_FORCE", tt.CLICOLOR_FORCE) + + if got := EnvColorDisabled(); got != tt.want { + t.Errorf("EnvColorDisabled(): want %v, got %v", tt.want, got) + } + }) + } +} + +func TestEnvColorForced(t *testing.T) { + orig_NO_COLOR := os.Getenv("NO_COLOR") + orig_CLICOLOR := os.Getenv("CLICOLOR") + orig_CLICOLOR_FORCE := os.Getenv("CLICOLOR_FORCE") + t.Cleanup(func() { + os.Setenv("NO_COLOR", orig_NO_COLOR) + os.Setenv("CLICOLOR", orig_CLICOLOR) + os.Setenv("CLICOLOR_FORCE", orig_CLICOLOR_FORCE) + }) + + tests := []struct { + name string + NO_COLOR string + CLICOLOR string + CLICOLOR_FORCE string + want bool + }{ + { + name: "pristine env", + NO_COLOR: "", + CLICOLOR: "", + CLICOLOR_FORCE: "", + want: false, + }, + { + name: "NO_COLOR enabled", + NO_COLOR: "1", + CLICOLOR: "", + CLICOLOR_FORCE: "", + want: false, + }, + { + name: "CLICOLOR disabled", + NO_COLOR: "", + CLICOLOR: "0", + CLICOLOR_FORCE: "", + want: false, + }, + { + name: "CLICOLOR enabled", + NO_COLOR: "", + CLICOLOR: "1", + CLICOLOR_FORCE: "", + want: false, + }, + { + name: "CLICOLOR_FORCE enabled", + NO_COLOR: "", + CLICOLOR: "", + CLICOLOR_FORCE: "1", + want: true, + }, + { + name: "CLICOLOR_FORCE disabled", + NO_COLOR: "", + CLICOLOR: "", + CLICOLOR_FORCE: "0", + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + os.Setenv("NO_COLOR", tt.NO_COLOR) + os.Setenv("CLICOLOR", tt.CLICOLOR) + os.Setenv("CLICOLOR_FORCE", tt.CLICOLOR_FORCE) + + if got := EnvColorForced(); got != tt.want { + t.Errorf("EnvColorForced(): want %v, got %v", tt.want, got) + } + }) + } +} diff --git a/pkg/iostreams/iostreams.go b/pkg/iostreams/iostreams.go index bbaaecda4..6dc89cb57 100644 --- a/pkg/iostreams/iostreams.go +++ b/pkg/iostreams/iostreams.go @@ -106,14 +106,11 @@ func System() *IOStreams { stdoutIsTTY := isTerminal(os.Stdout) stderrIsTTY := isTerminal(os.Stderr) - envNoColor := os.Getenv("NO_COLOR") != "" || os.Getenv("CLICOLOR") == "0" - envForceColor := os.Getenv("CLICOLOR_FORCE") != "" && os.Getenv("CLICOLOR_FORCE") != "0" - io := &IOStreams{ In: os.Stdin, Out: colorable.NewColorable(os.Stdout), ErrOut: colorable.NewColorable(os.Stderr), - colorEnabled: envForceColor || (!envNoColor && stdoutIsTTY), + colorEnabled: EnvColorForced() || (!EnvColorDisabled() && stdoutIsTTY), } // prevent duplicate isTerminal queries now that we know the answer diff --git a/utils/color.go b/utils/color.go index 034dd54d7..0b276bf11 100644 --- a/utils/color.go +++ b/utils/color.go @@ -4,6 +4,7 @@ import ( "io" "os" + "github.com/cli/cli/pkg/iostreams" "github.com/mattn/go-colorable" "github.com/mgutz/ansi" ) @@ -39,11 +40,11 @@ func makeColorFunc(color string) func(string) string { } func isColorEnabled() bool { - if os.Getenv("CLICOLOR_FORCE") != "" && os.Getenv("CLICOLOR_FORCE") != "0" { + if iostreams.EnvColorForced() { return true } - if os.Getenv("NO_COLOR") != "" || os.Getenv("CLICOLOR") == "0" { + if iostreams.EnvColorDisabled() { return false } From a0e0f313634740d1d163d06cc8c13e60df2fe1a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 7 Sep 2020 12:57:02 +0200 Subject: [PATCH 06/67] Disable ANSI color in prompts when color is disabled --- cmd/gh/main.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/cmd/gh/main.go b/cmd/gh/main.go index 456195314..6920b8cc0 100644 --- a/cmd/gh/main.go +++ b/cmd/gh/main.go @@ -10,6 +10,7 @@ import ( "path" "strings" + surveyCore "github.com/AlecAivazis/survey/v2/core" "github.com/cli/cli/api" "github.com/cli/cli/command" "github.com/cli/cli/internal/config" @@ -43,6 +44,10 @@ func main() { cmdFactory := factory.New(command.Version) stderr := cmdFactory.IOStreams.ErrOut + if !cmdFactory.IOStreams.ColorEnabled() { + surveyCore.DisableColor = true + } + rootCmd := root.NewCmdRoot(cmdFactory, command.Version, command.BuildDate) expandedArgs := []string{} From 2345f49ccbd6f1e902b9eae305d93494db30ec38 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Thu, 10 Sep 2020 20:58:06 -0500 Subject: [PATCH 07/67] support auth login --web --- pkg/cmd/auth/login/login.go | 116 ++++++++++++++++++------------- pkg/cmd/auth/login/login_test.go | 49 +++++++++++-- 2 files changed, 113 insertions(+), 52 deletions(-) diff --git a/pkg/cmd/auth/login/login.go b/pkg/cmd/auth/login/login.go index 630677dff..10a79c685 100644 --- a/pkg/cmd/auth/login/login.go +++ b/pkg/cmd/auth/login/login.go @@ -24,8 +24,11 @@ type LoginOptions struct { IO *iostreams.IOStreams Config func() (config.Config, error) + Interactive bool + Hostname string Token string + Web bool } func NewCmdLogin(f *cmdutil.Factory, runF func(*LoginOptions) error) *cobra.Command { @@ -51,6 +54,9 @@ func NewCmdLogin(f *cmdutil.Factory, runF func(*LoginOptions) error) *cobra.Comm $ gh auth login # => do an interactive setup + $ gh auth login --web + # => open a browser to authenticate and do a non-interactive setup + $ gh auth login --with-token < mytoken.txt # => read token from mytoken.txt and authenticate against github.com @@ -58,6 +64,14 @@ func NewCmdLogin(f *cmdutil.Factory, runF func(*LoginOptions) error) *cobra.Comm # => read token from mytoken.txt and authenticate against a GitHub Enterprise Server instance `), RunE: func(cmd *cobra.Command, args []string) error { + if !opts.IO.CanPrompt() && !(tokenStdin || opts.Web) { + return &cmdutil.FlagError{Err: errors.New("--web or --with-token required when not running interactively")} + } + + if tokenStdin && opts.Web { + return &cmdutil.FlagError{Err: errors.New("specify only one of --web or --with-token")} + } + if tokenStdin { defer opts.IO.In.Close() token, err := ioutil.ReadAll(opts.IO.In) @@ -67,15 +81,8 @@ func NewCmdLogin(f *cmdutil.Factory, runF func(*LoginOptions) error) *cobra.Comm opts.Token = strings.TrimSpace(string(token)) } - if opts.Token != "" { - // Assume non-interactive if a token is specified - if opts.Hostname == "" { - opts.Hostname = ghinstance.Default() - } - } else { - if !opts.IO.CanPrompt() { - return &cmdutil.FlagError{Err: errors.New("--with-token required when not running interactively")} - } + if opts.IO.CanPrompt() && opts.Token == "" && !opts.Web { + opts.Interactive = true } if cmd.Flags().Changed("hostname") { @@ -84,6 +91,12 @@ func NewCmdLogin(f *cmdutil.Factory, runF func(*LoginOptions) error) *cobra.Comm } } + if !opts.Interactive { + if opts.Hostname == "" { + opts.Hostname = ghinstance.Default() + } + } + if runF != nil { return runF(opts) } @@ -94,6 +107,7 @@ func NewCmdLogin(f *cmdutil.Factory, runF func(*LoginOptions) error) *cobra.Comm cmd.Flags().StringVarP(&opts.Hostname, "hostname", "h", "", "The hostname of the GitHub instance to authenticate with") cmd.Flags().BoolVar(&tokenStdin, "with-token", false, "Read token from standard input") + cmd.Flags().BoolVarP(&opts.Web, "web", "w", false, "Open a browser to authenticate") return cmd } @@ -168,24 +182,26 @@ func loginRun(opts *LoginOptions) error { return err } - username, err := api.CurrentLoginName(apiClient, hostname) - if err != nil { - return fmt.Errorf("error using api: %w", err) - } - var keepGoing bool - err = prompt.SurveyAskOne(&survey.Confirm{ - Message: fmt.Sprintf( - "You're already logged into %s as %s. Do you want to re-authenticate?", - hostname, - username), - Default: false, - }, &keepGoing) - if err != nil { - return fmt.Errorf("could not prompt: %w", err) - } + if opts.Interactive { + username, err := api.CurrentLoginName(apiClient, hostname) + if err != nil { + return fmt.Errorf("error using api: %w", err) + } + var keepGoing bool + err = prompt.SurveyAskOne(&survey.Confirm{ + Message: fmt.Sprintf( + "You're already logged into %s as %s. Do you want to re-authenticate?", + hostname, + username), + Default: false, + }, &keepGoing) + if err != nil { + return fmt.Errorf("could not prompt: %w", err) + } - if !keepGoing { - return nil + if !keepGoing { + return nil + } } } } @@ -195,15 +211,19 @@ func loginRun(opts *LoginOptions) error { } var authMode int - err = prompt.SurveyAskOne(&survey.Select{ - Message: "How would you like to authenticate?", - Options: []string{ - "Login with a web browser", - "Paste an authentication token", - }, - }, &authMode) - if err != nil { - return fmt.Errorf("could not prompt: %w", err) + if opts.Web { + authMode = 0 + } else { + err = prompt.SurveyAskOne(&survey.Select{ + Message: "How would you like to authenticate?", + Options: []string{ + "Login with a web browser", + "Paste an authentication token", + }, + }, &authMode) + if err != nil { + return fmt.Errorf("could not prompt: %w", err) + } } if authMode == 0 { @@ -239,19 +259,21 @@ func loginRun(opts *LoginOptions) error { } } - var gitProtocol string - err = prompt.SurveyAskOne(&survey.Select{ - Message: "Choose default git protocol", - Options: []string{ - "HTTPS", - "SSH", - }, - }, &gitProtocol) - if err != nil { - return fmt.Errorf("could not prompt: %w", err) - } + gitProtocol := "https" + if opts.Interactive { + err = prompt.SurveyAskOne(&survey.Select{ + Message: "Choose default git protocol", + Options: []string{ + "HTTPS", + "SSH", + }, + }, &gitProtocol) + if err != nil { + return fmt.Errorf("could not prompt: %w", err) + } - gitProtocol = strings.ToLower(gitProtocol) + gitProtocol = strings.ToLower(gitProtocol) + } fmt.Fprintf(opts.IO.ErrOut, "- gh config set -h %s git_protocol %s\n", hostname, gitProtocol) err = cfg.Set(hostname, "git_protocol", gitProtocol) diff --git a/pkg/cmd/auth/login/login_test.go b/pkg/cmd/auth/login/login_test.go index 5d0ba1942..7166eac92 100644 --- a/pkg/cmd/auth/login/login_test.go +++ b/pkg/cmd/auth/login/login_test.go @@ -81,8 +81,9 @@ func Test_NewCmdLogin(t *testing.T) { stdinTTY: true, cli: "--hostname barry.burton", wants: LoginOptions{ - Hostname: "barry.burton", - Token: "", + Hostname: "barry.burton", + Token: "", + Interactive: true, }, }, { @@ -90,10 +91,33 @@ func Test_NewCmdLogin(t *testing.T) { stdinTTY: true, cli: "", wants: LoginOptions{ - Hostname: "", - Token: "", + Hostname: "", + Token: "", + Interactive: true, }, }, + { + name: "tty web", + stdinTTY: true, + cli: "--web", + wants: LoginOptions{ + Hostname: "github.com", + Web: true, + }, + }, + { + name: "nontty web", + cli: "--web", + wants: LoginOptions{ + Hostname: "github.com", + Web: true, + }, + }, + { + name: "web and with-token", + cli: "--web --with-token", + wantsErr: true, + }, } for _, tt := range tests { @@ -134,6 +158,8 @@ func Test_NewCmdLogin(t *testing.T) { assert.Equal(t, tt.wants.Token, gotOpts.Token) assert.Equal(t, tt.wants.Hostname, gotOpts.Hostname) + assert.Equal(t, tt.wants.Web, gotOpts.Web) + assert.Equal(t, tt.wants.Interactive, gotOpts.Interactive) }) } } @@ -262,6 +288,9 @@ func Test_loginRun_Survey(t *testing.T) { }{ { name: "already authenticated", + opts: &LoginOptions{ + Interactive: true, + }, cfg: func(cfg config.Config) { _ = cfg.Set("github.com", "oauth_token", "ghi789") }, @@ -280,7 +309,8 @@ func Test_loginRun_Survey(t *testing.T) { { name: "hostname set", opts: &LoginOptions{ - Hostname: "rebecca.chambers", + Hostname: "rebecca.chambers", + Interactive: true, }, wantHosts: "rebecca.chambers:\n oauth_token: def456\n git_protocol: https\n user: jillv\n", askStubs: func(as *prompt.AskStubber) { @@ -298,6 +328,9 @@ func Test_loginRun_Survey(t *testing.T) { { name: "choose enterprise", wantHosts: "brad.vickers:\n oauth_token: def456\n git_protocol: https\n user: jillv\n", + opts: &LoginOptions{ + Interactive: true, + }, askStubs: func(as *prompt.AskStubber) { as.StubOne(1) // host type enterprise as.StubOne("brad.vickers") // hostname @@ -315,6 +348,9 @@ func Test_loginRun_Survey(t *testing.T) { { name: "choose github.com", wantHosts: "github.com:\n oauth_token: def456\n git_protocol: https\n user: jillv\n", + opts: &LoginOptions{ + Interactive: true, + }, askStubs: func(as *prompt.AskStubber) { as.StubOne(0) // host type github.com as.StubOne(1) // auth mode: token @@ -325,6 +361,9 @@ func Test_loginRun_Survey(t *testing.T) { { name: "sets git_protocol", wantHosts: "github.com:\n oauth_token: def456\n git_protocol: ssh\n user: jillv\n", + opts: &LoginOptions{ + Interactive: true, + }, askStubs: func(as *prompt.AskStubber) { as.StubOne(0) // host type github.com as.StubOne(1) // auth mode: token From 6ad6784c4654d661a421ff6323cf641397eb73fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 14 Sep 2020 15:30:10 +0200 Subject: [PATCH 08/67] Ignore EPIPE when forwarding `pr diff` output to pager --- pkg/cmd/pr/diff/diff.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/cmd/pr/diff/diff.go b/pkg/cmd/pr/diff/diff.go index 3c55e7db8..2bea8072b 100644 --- a/pkg/cmd/pr/diff/diff.go +++ b/pkg/cmd/pr/diff/diff.go @@ -7,6 +7,7 @@ import ( "io" "net/http" "strings" + "syscall" "github.com/cli/cli/api" "github.com/cli/cli/context" @@ -98,6 +99,9 @@ func diffRun(opts *DiffOptions) error { if opts.UseColor == "never" { _, err = io.Copy(opts.IO.Out, diff) + if errors.Is(err, syscall.EPIPE) { + return nil + } return err } From 662f83fcb916f52d448abaadc9582ce5b91151c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 14 Sep 2020 15:31:31 +0200 Subject: [PATCH 09/67] Fix `release download` Only ever try to set an `Authorization` request header to hosts that we have OAuth credentials for; skip the header otherwise. --- pkg/cmd/factory/http.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/factory/http.go b/pkg/cmd/factory/http.go index 0adc87a12..47fbbefe8 100644 --- a/pkg/cmd/factory/http.go +++ b/pkg/cmd/factory/http.go @@ -24,7 +24,7 @@ func NewHTTPClient(io *iostreams.IOStreams, cfg config.Config, appVersion string 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()) - if token, err := cfg.Get(hostname, "oauth_token"); err == nil || token != "" { + if token, err := cfg.Get(hostname, "oauth_token"); err == nil && token != "" { return fmt.Sprintf("token %s", token), nil } return "", nil From aa7246311437facd235e6f1610632c89b02b3bb4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 14 Sep 2020 16:25:30 +0200 Subject: [PATCH 10/67] Extract `environment` as a separate help topic Co-authored-by: Sam Coe --- pkg/cmd/root/help.go | 5 +++++ pkg/cmd/root/help_topic.go | 41 ++++++++++++++++++++++++++++++++++++++ pkg/cmd/root/root.go | 26 +++--------------------- 3 files changed, 49 insertions(+), 23 deletions(-) create mode 100644 pkg/cmd/root/help_topic.go diff --git a/pkg/cmd/root/help.go b/pkg/cmd/root/help.go index 019ec53cd..b2841ba9f 100644 --- a/pkg/cmd/root/help.go +++ b/pkg/cmd/root/help.go @@ -78,6 +78,11 @@ func rootHelpFunc(command *cobra.Command, args []string) { return } + if helpTopic := command.Annotations["helpTopic"]; helpTopic == "true" { + fmt.Fprint(command.OutOrStdout(), command.Long) + return + } + coreCommands := []string{} additionalCommands := []string{} for _, c := range command.Commands() { diff --git a/pkg/cmd/root/help_topic.go b/pkg/cmd/root/help_topic.go new file mode 100644 index 000000000..8e17685ae --- /dev/null +++ b/pkg/cmd/root/help_topic.go @@ -0,0 +1,41 @@ +package root + +import ( + "github.com/MakeNowJust/heredoc" + "github.com/spf13/cobra" +) + +func NewHelpTopic(topic string) *cobra.Command { + return &cobra.Command{ + Use: "environment", + Long: heredoc.Doc(` + GITHUB_TOKEN: an authentication token for github.com API requests. Setting this avoids + being prompted to authenticate and takes precedence over previously stored credentials. + + GITHUB_ENTERPRISE_TOKEN: an authentication token for API requests to GitHub Enterprise. + + GH_REPO: specify the GitHub repository in the "[HOST/]OWNER/REPO" format for commands + that otherwise operate on a local repository. + + GH_HOST: specify the GitHub hostname for commands that would otherwise assume + the "github.com" host when not in a context of an existing repository. + + GH_EDITOR, GIT_EDITOR, VISUAL, EDITOR (in order of precedence): the editor tool to use + for authoring text. + + BROWSER: the web browser to use for opening links. + + DEBUG: set to any value to enable verbose output to standard error. Include values "api" + or "oauth" to print detailed information about HTTP requests or authentication flow. + + GLAMOUR_STYLE: the style to use for rendering Markdown. See + https://github.com/charmbracelet/glamour#styles + + NO_COLOR: avoid printing ANSI escape sequences for color output. + `), + Hidden: true, + Annotations: map[string]string{ + "helpTopic": "true", + }, + } +} diff --git a/pkg/cmd/root/root.go b/pkg/cmd/root/root.go index 0eadbd575..2c597352e 100644 --- a/pkg/cmd/root/root.go +++ b/pkg/cmd/root/root.go @@ -44,29 +44,7 @@ func NewCmdRoot(f *cmdutil.Factory, version, buildDate string) *cobra.Command { Open an issue using “gh issue create -R cli/cli” `), "help:environment": heredoc.Doc(` - GITHUB_TOKEN: an authentication token for github.com API requests. Setting this avoids - being prompted to authenticate and takes precedence over previously stored credentials. - - GITHUB_ENTERPRISE_TOKEN: an authentication token for API requests to GitHub Enterprise. - - GH_REPO: specify the GitHub repository in the "[HOST/]OWNER/REPO" format for commands - that otherwise operate on a local repository. - - GH_HOST: specify the GitHub hostname for commands that would otherwise assume - the "github.com" host when not in a context of an existing repository. - - GH_EDITOR, GIT_EDITOR, VISUAL, EDITOR (in order of precedence): the editor tool to use - for authoring text. - - BROWSER: the web browser to use for opening links. - - DEBUG: set to any value to enable verbose output to standard error. Include values "api" - or "oauth" to print detailed information about HTTP requests or authentication flow. - - GLAMOUR_STYLE: the style to use for rendering Markdown. See - https://github.com/charmbracelet/glamour#styles - - NO_COLOR: avoid printing ANSI escape sequences for color output. + See 'gh help environment' for the list of supported environment variables. `), }, } @@ -113,6 +91,8 @@ func NewCmdRoot(f *cmdutil.Factory, version, buildDate string) *cobra.Command { cmd.AddCommand(gistCmd.NewCmdGist(f)) cmd.AddCommand(NewCmdCompletion(f.IOStreams)) + cmd.AddCommand(NewHelpTopic("environment")) + // the `api` command should not inherit any extra HTTP headers bareHTTPCmdFactory := *f bareHTTPCmdFactory.HttpClient = func() (*http.Client, error) { From 3c32507a1316ad38e82374a8bfa8cc126e5bc312 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 14 Sep 2020 16:26:23 +0200 Subject: [PATCH 11/67] Consistent use of quotes to delineate commands to be run --- pkg/cmd/root/help.go | 2 +- pkg/cmd/root/root.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/root/help.go b/pkg/cmd/root/help.go index b2841ba9f..4f9a798fb 100644 --- a/pkg/cmd/root/help.go +++ b/pkg/cmd/root/help.go @@ -144,7 +144,7 @@ func rootHelpFunc(command *cobra.Command, args []string) { helpEntries = append(helpEntries, helpEntry{"ENVIRONMENT VARIABLES", command.Annotations["help:environment"]}) } helpEntries = append(helpEntries, helpEntry{"LEARN MORE", ` -Use "gh --help" for more information about a command. +Use 'gh --help' for more information about a command. Read the manual at https://cli.github.com/manual`}) if _, ok := command.Annotations["help:feedback"]; ok { helpEntries = append(helpEntries, helpEntry{"FEEDBACK", command.Annotations["help:feedback"]}) diff --git a/pkg/cmd/root/root.go b/pkg/cmd/root/root.go index 2c597352e..f3319e456 100644 --- a/pkg/cmd/root/root.go +++ b/pkg/cmd/root/root.go @@ -41,7 +41,7 @@ func NewCmdRoot(f *cmdutil.Factory, version, buildDate string) *cobra.Command { `), Annotations: map[string]string{ "help:feedback": heredoc.Doc(` - Open an issue using “gh issue create -R cli/cli” + Open an issue using 'gh issue create -R cli/cli' `), "help:environment": heredoc.Doc(` See 'gh help environment' for the list of supported environment variables. From 76181156ba1fd6ae8eb4b17dc933e7d0b53bcad7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Mon, 14 Sep 2020 16:49:30 +0200 Subject: [PATCH 12/67] Prevent endless recursive pager Unset PAGER environment variable when executing the command referenced in PAGER in case that command also has support for PAGER and would end up executing itself indefinitely. --- pkg/iostreams/iostreams.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pkg/iostreams/iostreams.go b/pkg/iostreams/iostreams.go index 9de8ab8d7..e3694dfe5 100644 --- a/pkg/iostreams/iostreams.go +++ b/pkg/iostreams/iostreams.go @@ -103,6 +103,11 @@ func (s *IOStreams) StartPager() error { } pagerEnv := os.Environ() + for i := len(pagerEnv) - 1; i >= 0; i-- { + if strings.HasPrefix(pagerEnv[i], "PAGER=") { + pagerEnv = append(pagerEnv[0:i], pagerEnv[i+1:]...) + } + } if _, ok := os.LookupEnv("LESS"); !ok { pagerEnv = append(pagerEnv, "LESS=FRX") } From 68a019cc3c9bae5609b29d6ff437624707ff5631 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Mon, 14 Sep 2020 10:16:50 -0500 Subject: [PATCH 13/67] review feedback --- pkg/cmd/auth/login/login.go | 55 +++++++++++++++++-------------------- 1 file changed, 25 insertions(+), 30 deletions(-) diff --git a/pkg/cmd/auth/login/login.go b/pkg/cmd/auth/login/login.go index 10a79c685..b6d2a2f99 100644 --- a/pkg/cmd/auth/login/login.go +++ b/pkg/cmd/auth/login/login.go @@ -54,9 +54,6 @@ func NewCmdLogin(f *cmdutil.Factory, runF func(*LoginOptions) error) *cobra.Comm $ gh auth login # => do an interactive setup - $ gh auth login --web - # => open a browser to authenticate and do a non-interactive setup - $ gh auth login --with-token < mytoken.txt # => read token from mytoken.txt and authenticate against github.com @@ -174,7 +171,7 @@ func loginRun(opts *LoginOptions) error { existingToken, _ := cfg.Get(hostname, "oauth_token") - if existingToken != "" { + if existingToken != "" && opts.Interactive { err := client.ValidateHostCfg(hostname, cfg) if err == nil { apiClient, err := client.ClientFromCfg(hostname, cfg) @@ -182,26 +179,24 @@ func loginRun(opts *LoginOptions) error { return err } - if opts.Interactive { - username, err := api.CurrentLoginName(apiClient, hostname) - if err != nil { - return fmt.Errorf("error using api: %w", err) - } - var keepGoing bool - err = prompt.SurveyAskOne(&survey.Confirm{ - Message: fmt.Sprintf( - "You're already logged into %s as %s. Do you want to re-authenticate?", - hostname, - username), - Default: false, - }, &keepGoing) - if err != nil { - return fmt.Errorf("could not prompt: %w", err) - } + username, err := api.CurrentLoginName(apiClient, hostname) + if err != nil { + return fmt.Errorf("error using api: %w", err) + } + var keepGoing bool + err = prompt.SurveyAskOne(&survey.Confirm{ + Message: fmt.Sprintf( + "You're already logged into %s as %s. Do you want to re-authenticate?", + hostname, + username), + Default: false, + }, &keepGoing) + if err != nil { + return fmt.Errorf("could not prompt: %w", err) + } - if !keepGoing { - return nil - } + if !keepGoing { + return nil } } } @@ -273,15 +268,15 @@ func loginRun(opts *LoginOptions) error { } gitProtocol = strings.ToLower(gitProtocol) - } - fmt.Fprintf(opts.IO.ErrOut, "- gh config set -h %s git_protocol %s\n", hostname, gitProtocol) - err = cfg.Set(hostname, "git_protocol", gitProtocol) - if err != nil { - return err - } + fmt.Fprintf(opts.IO.ErrOut, "- gh config set -h %s git_protocol %s\n", hostname, gitProtocol) + err = cfg.Set(hostname, "git_protocol", gitProtocol) + if err != nil { + return err + } - fmt.Fprintf(opts.IO.ErrOut, "%s Configured git protocol\n", utils.GreenCheck()) + fmt.Fprintf(opts.IO.ErrOut, "%s Configured git protocol\n", utils.GreenCheck()) + } apiClient, err := client.ClientFromCfg(hostname, cfg) if err != nil { From f7c4a0cf3f8436e24ebfc5fed2756da5bf580c2f Mon Sep 17 00:00:00 2001 From: vilmibm Date: Thu, 10 Sep 2020 20:14:29 -0500 Subject: [PATCH 14/67] gh gist list --- pkg/cmd/gist/gist.go | 2 + pkg/cmd/gist/list/http.go | 52 ++++++++++++++++++++ pkg/cmd/gist/list/list.go | 90 ++++++++++++++++++++++++++++++++++ pkg/cmd/gist/list/list_test.go | 84 +++++++++++++++++++++++++++++++ 4 files changed, 228 insertions(+) create mode 100644 pkg/cmd/gist/list/http.go create mode 100644 pkg/cmd/gist/list/list.go create mode 100644 pkg/cmd/gist/list/list_test.go diff --git a/pkg/cmd/gist/gist.go b/pkg/cmd/gist/gist.go index ea10a8c40..e47f055db 100644 --- a/pkg/cmd/gist/gist.go +++ b/pkg/cmd/gist/gist.go @@ -2,6 +2,7 @@ package gist import ( gistCreateCmd "github.com/cli/cli/pkg/cmd/gist/create" + gistListCmd "github.com/cli/cli/pkg/cmd/gist/list" "github.com/cli/cli/pkg/cmdutil" "github.com/spf13/cobra" ) @@ -14,6 +15,7 @@ func NewCmdGist(f *cmdutil.Factory) *cobra.Command { } cmd.AddCommand(gistCreateCmd.NewCmdCreate(f, nil)) + cmd.AddCommand(gistListCmd.NewCmdList(f, nil)) return cmd } diff --git a/pkg/cmd/gist/list/http.go b/pkg/cmd/gist/list/http.go new file mode 100644 index 000000000..e64243977 --- /dev/null +++ b/pkg/cmd/gist/list/http.go @@ -0,0 +1,52 @@ +package list + +import ( + "fmt" + "net/http" + "net/url" + "sort" + "time" + + "github.com/cli/cli/api" +) + +type Gist struct { + Description string `json:"description"` + HTMLURL string `json:"html_url"` + UpdatedAt time.Time `json:"updated_at"` + Public bool +} + +func listGists(client *http.Client, hostname string, limit int, visibility string) ([]Gist, error) { + result := []Gist{} + + query := url.Values{} + if visibility == "all" { + query.Add("per_page", fmt.Sprintf("%d", limit)) + } else { + query.Add("per_page", "100") + } + + apiClient := api.NewClientFromHTTP(client) + err := apiClient.REST(hostname, "GET", "gists?"+query.Encode(), nil, &result) + if err != nil { + return nil, err + } + + gists := []Gist{} + + for _, gist := range result { + if len(gists) == limit { + break + } + if visibility == "all" || (visibility == "private" && !gist.Public) || (visibility == "public" && gist.Public) { + gists = append(gists, gist) + } + } + + sort.SliceStable(gists, func(i, j int) bool { + return gists[i].UpdatedAt.After(gists[j].UpdatedAt) + }) + + return gists, nil +} diff --git a/pkg/cmd/gist/list/list.go b/pkg/cmd/gist/list/list.go new file mode 100644 index 000000000..bf59497f3 --- /dev/null +++ b/pkg/cmd/gist/list/list.go @@ -0,0 +1,90 @@ +package list + +import ( + "fmt" + "net/http" + + "github.com/cli/cli/internal/ghinstance" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/utils" + "github.com/spf13/cobra" +) + +type ListOptions struct { + IO *iostreams.IOStreams + HttpClient func() (*http.Client, error) + + Limit int + Visibility string // all, secret, public +} + +func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Command { + opts := &ListOptions{ + IO: f.IOStreams, + HttpClient: f.HttpClient, + } + + cmd := &cobra.Command{ + Use: "list", + Short: "List your gists", + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, args []string) error { + if opts.Limit < 1 { + return &cmdutil.FlagError{Err: fmt.Errorf("invalid limit: %v", opts.Limit)} + } + + pub := cmd.Flags().Changed("public") + priv := cmd.Flags().Changed("private") + + opts.Visibility = "all" + if pub && !priv { + opts.Visibility = "public" + } else if priv && !pub { + opts.Visibility = "private" + } + + if runF != nil { + return runF(opts) + } + + return listRun(opts) + }, + } + + cmd.Flags().IntVarP(&opts.Limit, "limit", "L", 10, "Maximum number of gists to fetch") + cmd.Flags().Bool("public", false, "Show only public gists") + cmd.Flags().Bool("private", false, "Show only private gists") + + return cmd +} + +func listRun(opts *ListOptions) error { + client, err := opts.HttpClient() + if err != nil { + return err + } + + gists, err := listGists(client, ghinstance.OverridableDefault(), opts.Limit, opts.Visibility) + if err != nil { + return err + } + + cs := opts.IO.ColorScheme() + + tp := utils.NewTablePrinter(opts.IO) + + for _, gist := range gists { + // TODO i was getting confusing results with table printer's truncation + description := gist.Description + if len(description) > 40 { + description = description[0:37] + "..." + } + + tp.AddField(description, nil, cs.Bold) + tp.AddField(gist.HTMLURL, nil, nil) + tp.EndRow() + } + + return tp.Render() +} diff --git a/pkg/cmd/gist/list/list_test.go b/pkg/cmd/gist/list/list_test.go new file mode 100644 index 000000000..03266cdf2 --- /dev/null +++ b/pkg/cmd/gist/list/list_test.go @@ -0,0 +1,84 @@ +package list + +import ( + "bytes" + "testing" + + "github.com/cli/cli/pkg/cmdutil" + "github.com/google/shlex" + "github.com/stretchr/testify/assert" +) + +func TestNewCmdList(t *testing.T) { + tests := []struct { + name string + cli string + wants ListOptions + }{ + { + name: "no arguments", + cli: "", + wants: ListOptions{ + Limit: 10, + Visibility: "all", + }, + }, + { + name: "public", + cli: "--public", + wants: ListOptions{ + Limit: 10, + Visibility: "public", + }, + }, + { + name: "private", + cli: "--private", + wants: ListOptions{ + Limit: 10, + Visibility: "private", + }, + }, + { + name: "public and private", + cli: "--private --public", + wants: ListOptions{ + Limit: 10, + Visibility: "all", + }, + }, + { + name: "limit", + cli: "--limit 30", + wants: ListOptions{ + Limit: 30, + Visibility: "all", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + f := &cmdutil.Factory{} + + argv, err := shlex.Split(tt.cli) + assert.NoError(t, err) + + var gotOpts *ListOptions + cmd := NewCmdList(f, func(opts *ListOptions) error { + gotOpts = opts + return nil + }) + cmd.SetArgs(argv) + cmd.SetIn(&bytes.Buffer{}) + cmd.SetOut(&bytes.Buffer{}) + cmd.SetErr(&bytes.Buffer{}) + + _, err = cmd.ExecuteC() + assert.NoError(t, err) + + assert.Equal(t, tt.wants.Visibility, gotOpts.Visibility) + assert.Equal(t, tt.wants.Limit, gotOpts.Limit) + }) + } +} From b17124157c062ed43268412767b82bd0ad4adc76 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Fri, 11 Sep 2020 13:42:05 -0500 Subject: [PATCH 15/67] start on gist view --- pkg/cmd/gist/gist.go | 2 + pkg/cmd/gist/list/list_test.go | 2 + pkg/cmd/gist/view/http.go | 33 ++++++++++++ pkg/cmd/gist/view/view.go | 94 ++++++++++++++++++++++++++++++++++ pkg/cmd/gist/view/view_test.go | 70 +++++++++++++++++++++++++ 5 files changed, 201 insertions(+) create mode 100644 pkg/cmd/gist/view/http.go create mode 100644 pkg/cmd/gist/view/view.go create mode 100644 pkg/cmd/gist/view/view_test.go diff --git a/pkg/cmd/gist/gist.go b/pkg/cmd/gist/gist.go index e47f055db..612ef0bcf 100644 --- a/pkg/cmd/gist/gist.go +++ b/pkg/cmd/gist/gist.go @@ -3,6 +3,7 @@ package gist import ( gistCreateCmd "github.com/cli/cli/pkg/cmd/gist/create" gistListCmd "github.com/cli/cli/pkg/cmd/gist/list" + gistViewCmd "github.com/cli/cli/pkg/cmd/gist/view" "github.com/cli/cli/pkg/cmdutil" "github.com/spf13/cobra" ) @@ -16,6 +17,7 @@ func NewCmdGist(f *cmdutil.Factory) *cobra.Command { cmd.AddCommand(gistCreateCmd.NewCmdCreate(f, nil)) cmd.AddCommand(gistListCmd.NewCmdList(f, nil)) + cmd.AddCommand(gistViewCmd.NewCmdView(f, nil)) return cmd } diff --git a/pkg/cmd/gist/list/list_test.go b/pkg/cmd/gist/list/list_test.go index 03266cdf2..5541a51bc 100644 --- a/pkg/cmd/gist/list/list_test.go +++ b/pkg/cmd/gist/list/list_test.go @@ -82,3 +82,5 @@ func TestNewCmdList(t *testing.T) { }) } } + +// TODO execution tests diff --git a/pkg/cmd/gist/view/http.go b/pkg/cmd/gist/view/http.go new file mode 100644 index 000000000..617fa2ec5 --- /dev/null +++ b/pkg/cmd/gist/view/http.go @@ -0,0 +1,33 @@ +package view + +import ( + "fmt" + "net/http" + + "github.com/cli/cli/api" +) + +type GistFile struct { + Filename string + Type string + Language string + Content string +} + +type Gist struct { + Description string + Files map[string]GistFile +} + +func getGist(client *http.Client, hostname, gistID string) (*Gist, error) { + gist := Gist{} + path := fmt.Sprintf("gists/%s", gistID) + + apiClient := api.NewClientFromHTTP(client) + err := apiClient.REST(hostname, "GET", path, nil, &gist) + if err != nil { + return nil, err + } + + return &gist, nil +} diff --git a/pkg/cmd/gist/view/view.go b/pkg/cmd/gist/view/view.go new file mode 100644 index 000000000..be9ac4bde --- /dev/null +++ b/pkg/cmd/gist/view/view.go @@ -0,0 +1,94 @@ +package view + +import ( + "fmt" + "net/http" + "net/url" + "strings" + + "github.com/cli/cli/internal/ghinstance" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/utils" + "github.com/spf13/cobra" +) + +type ViewOptions struct { + IO *iostreams.IOStreams + HttpClient func() (*http.Client, error) + + Selector string + Raw bool +} + +func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Command { + opts := &ViewOptions{ + IO: f.IOStreams, + HttpClient: f.HttpClient, + } + + cmd := &cobra.Command{ + Use: "view { | }", + Short: "View a gist", + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + opts.Selector = args[0] + + if !opts.IO.IsStdoutTTY() { + opts.Raw = true + } + + if runF != nil { + return runF(opts) + } + return viewRun(opts) + }, + } + + cmd.Flags().BoolVarP(&opts.Raw, "raw", "r", false, "do not try and render markdown") + + return cmd +} + +func viewRun(opts *ViewOptions) error { + gistID := opts.Selector + + u, err := url.Parse(opts.Selector) + if err == nil { + if strings.HasPrefix(u.Path, "/") { + gistID = u.Path[1:] + } + } + + client, err := opts.HttpClient() + if err != nil { + return err + } + + gist, err := getGist(client, ghinstance.OverridableDefault(), gistID) + if err != nil { + return err + } + + cs := opts.IO.ColorScheme() + if gist.Description != "" { + fmt.Fprintf(opts.IO.ErrOut, "%s\n", cs.Bold(gist.Description)) + } + + for filename, gistFile := range gist.Files { + fmt.Fprintf(opts.IO.ErrOut, "%s\n", cs.Gray(filename)) + fmt.Fprintln(opts.IO.ErrOut) + content := gistFile.Content + if strings.Contains(gistFile.Type, "markdown") && !opts.Raw { + rendered, err := utils.RenderMarkdown(gistFile.Content) + if err == nil { + content = rendered + } + } + fmt.Fprintf(opts.IO.Out, "%s\n", content) + fmt.Fprintln(opts.IO.Out) + } + + // TODO print gist files, possibly with rendered markdown + return nil +} diff --git a/pkg/cmd/gist/view/view_test.go b/pkg/cmd/gist/view/view_test.go new file mode 100644 index 000000000..d77ab946d --- /dev/null +++ b/pkg/cmd/gist/view/view_test.go @@ -0,0 +1,70 @@ +package view + +import ( + "bytes" + "testing" + + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/iostreams" + "github.com/google/shlex" + "github.com/stretchr/testify/assert" +) + +func TestNewCmdView(t *testing.T) { + tests := []struct { + name string + cli string + wants ViewOptions + tty bool + }{ + { + name: "tty no arguments", + tty: true, + cli: "123", + wants: ViewOptions{ + Raw: false, + Selector: "123", + }, + }, + { + name: "nontty no arguments", + cli: "123", + wants: ViewOptions{ + Raw: true, + Selector: "123", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + io, _, _, _ := iostreams.Test() + io.SetStdoutTTY(tt.tty) + + f := &cmdutil.Factory{ + IOStreams: io, + } + + argv, err := shlex.Split(tt.cli) + assert.NoError(t, err) + + var gotOpts *ViewOptions + cmd := NewCmdView(f, func(opts *ViewOptions) error { + gotOpts = opts + return nil + }) + cmd.SetArgs(argv) + cmd.SetIn(&bytes.Buffer{}) + cmd.SetOut(&bytes.Buffer{}) + cmd.SetErr(&bytes.Buffer{}) + + _, err = cmd.ExecuteC() + assert.NoError(t, err) + + assert.Equal(t, tt.wants.Raw, gotOpts.Raw) + assert.Equal(t, tt.wants.Selector, gotOpts.Selector) + }) + } +} + +// TODO execution tests From f3b4493448e5c6a6199e4287dff848f574aa8f62 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Fri, 11 Sep 2020 17:08:14 -0500 Subject: [PATCH 16/67] print aliases as valid yaml when piping --- pkg/cmd/alias/list/list.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/pkg/cmd/alias/list/list.go b/pkg/cmd/alias/list/list.go index b9d67c2ae..a8576db5c 100644 --- a/pkg/cmd/alias/list/list.go +++ b/pkg/cmd/alias/list/list.go @@ -69,12 +69,7 @@ func listRun(opts *ListOptions) error { sort.Strings(keys) for _, alias := range keys { - if tp.IsTTY() { - // ensure that screen readers pause - tp.AddField(alias+":", nil, nil) - } else { - tp.AddField(alias, nil, nil) - } + tp.AddField(alias+":", nil, nil) tp.AddField(aliasMap[alias], nil, nil) tp.EndRow() } From b6502deb24e5911121b754ce31107375f10e4f8b Mon Sep 17 00:00:00 2001 From: vilmibm Date: Fri, 11 Sep 2020 17:08:59 -0500 Subject: [PATCH 17/67] allow naming stdin gist files --- pkg/cmd/gist/create/create.go | 18 ++++++++++++------ pkg/cmd/gist/create/create_test.go | 2 +- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/pkg/cmd/gist/create/create.go b/pkg/cmd/gist/create/create.go index e2d6975b8..8024cfc24 100644 --- a/pkg/cmd/gist/create/create.go +++ b/pkg/cmd/gist/create/create.go @@ -23,9 +23,10 @@ import ( type CreateOptions struct { IO *iostreams.IOStreams - Description string - Public bool - Filenames []string + Description string + Public bool + Filenames []string + FilenameOverride string HttpClient func() (*http.Client, error) } @@ -84,6 +85,7 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co cmd.Flags().StringVarP(&opts.Description, "desc", "d", "", "A description for this gist") cmd.Flags().BoolVarP(&opts.Public, "public", "p", false, "List the gist publicly (default: private)") + cmd.Flags().StringVarP(&opts.FilenameOverride, "filename", "f", "", "Provide a filename to be used when reading from STDIN") return cmd } @@ -93,7 +95,7 @@ func createRun(opts *CreateOptions) error { fileArgs = []string{"-"} } - files, err := processFiles(opts.IO.In, fileArgs) + files, err := processFiles(opts.IO.In, opts.FilenameOverride, fileArgs) if err != nil { return fmt.Errorf("failed to collect files for posting: %w", err) } @@ -137,7 +139,7 @@ func createRun(opts *CreateOptions) error { return nil } -func processFiles(stdin io.ReadCloser, filenames []string) (map[string]string, error) { +func processFiles(stdin io.ReadCloser, filenameOverride string, filenames []string) (map[string]string, error) { fs := map[string]string{} if len(filenames) == 0 { @@ -149,7 +151,11 @@ func processFiles(stdin io.ReadCloser, filenames []string) (map[string]string, e var content []byte var err error if f == "-" { - filename = fmt.Sprintf("gistfile%d.txt", i) + if filenameOverride != "" { + filename = filenameOverride + } else { + filename = fmt.Sprintf("gistfile%d.txt", i) + } content, err = ioutil.ReadAll(stdin) if err != nil { return fs, fmt.Errorf("failed to read from stdin: %w", err) diff --git a/pkg/cmd/gist/create/create_test.go b/pkg/cmd/gist/create/create_test.go index bd57cfa2e..043b6f2d1 100644 --- a/pkg/cmd/gist/create/create_test.go +++ b/pkg/cmd/gist/create/create_test.go @@ -21,7 +21,7 @@ const ( func Test_processFiles(t *testing.T) { fakeStdin := strings.NewReader("hey cool how is it going") - files, err := processFiles(ioutil.NopCloser(fakeStdin), []string{"-"}) + files, err := processFiles(ioutil.NopCloser(fakeStdin), "", []string{"-"}) if err != nil { t.Fatalf("unexpected error processing files: %s", err) } From 39b6ec8aec41141e122a0f68fb2bc5b42642742c Mon Sep 17 00:00:00 2001 From: vilmibm Date: Fri, 11 Sep 2020 17:37:45 -0500 Subject: [PATCH 18/67] share gist getting --- pkg/cmd/gist/{view/http.go => shared/shared.go} | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) rename pkg/cmd/gist/{view/http.go => shared/shared.go} (85%) diff --git a/pkg/cmd/gist/view/http.go b/pkg/cmd/gist/shared/shared.go similarity index 85% rename from pkg/cmd/gist/view/http.go rename to pkg/cmd/gist/shared/shared.go index 617fa2ec5..7cfbc5866 100644 --- a/pkg/cmd/gist/view/http.go +++ b/pkg/cmd/gist/shared/shared.go @@ -1,4 +1,4 @@ -package view +package shared import ( "fmt" @@ -19,7 +19,7 @@ type Gist struct { Files map[string]GistFile } -func getGist(client *http.Client, hostname, gistID string) (*Gist, error) { +func GetGist(client *http.Client, hostname, gistID string) (*Gist, error) { gist := Gist{} path := fmt.Sprintf("gists/%s", gistID) From 0d3056d9a75d7d07a80b38e87c987626cd538880 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Fri, 11 Sep 2020 17:41:24 -0500 Subject: [PATCH 19/67] gh gist edit --- pkg/cmd/gist/edit/edit.go | 203 ++++++++++++++++++++++++++++++++++ pkg/cmd/gist/gist.go | 2 + pkg/cmd/gist/list/http.go | 2 +- pkg/cmd/gist/shared/shared.go | 15 ++- pkg/cmd/gist/view/view.go | 3 +- 5 files changed, 217 insertions(+), 8 deletions(-) create mode 100644 pkg/cmd/gist/edit/edit.go diff --git a/pkg/cmd/gist/edit/edit.go b/pkg/cmd/gist/edit/edit.go new file mode 100644 index 000000000..097cfad6b --- /dev/null +++ b/pkg/cmd/gist/edit/edit.go @@ -0,0 +1,203 @@ +package edit + +import ( + "bytes" + "encoding/json" + "errors" + "fmt" + "net/http" + "net/url" + "os" + "sort" + "strings" + + "github.com/AlecAivazis/survey/v2" + "github.com/cli/cli/api" + "github.com/cli/cli/internal/config" + "github.com/cli/cli/internal/ghinstance" + "github.com/cli/cli/pkg/cmd/gist/shared" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/pkg/prompt" + "github.com/cli/cli/pkg/surveyext" + "github.com/spf13/cobra" +) + +type EditOptions struct { + IO *iostreams.IOStreams + HttpClient func() (*http.Client, error) + Config func() (config.Config, error) + + Selector string + Filename string +} + +func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Command { + opts := EditOptions{ + IO: f.IOStreams, + HttpClient: f.HttpClient, + Config: f.Config, + } + + cmd := &cobra.Command{ + Use: "edit { | }", + Short: "Edit one of your gists", + Args: cobra.ExactArgs(1), + RunE: func(c *cobra.Command, args []string) error { + opts.Selector = args[0] + + if runF != nil { + return runF(&opts) + } + + return editRun(&opts) + }, + } + cmd.Flags().StringVarP(&opts.Filename, "filename", "f", "", "a specific file to edit") + + return cmd +} + +func editRun(opts *EditOptions) error { + gistID := opts.Selector + + u, err := url.Parse(opts.Selector) + if err == nil { + if strings.HasPrefix(u.Path, "/") { + gistID = u.Path[1:] + } + } + + client, err := opts.HttpClient() + if err != nil { + return err + } + + gist, err := shared.GetGist(client, ghinstance.OverridableDefault(), gistID) + if err != nil { + return err + } + + filesToUpdate := map[string]string{} + + for true { + filename := opts.Filename + candidates := []string{} + for filename, _ := range gist.Files { + candidates = append(candidates, filename) + } + + sort.Strings(candidates) + + if filename == "" { + if len(candidates) == 1 { + filename = candidates[0] + } else { + if !opts.IO.CanPrompt() { + return errors.New("unsure what file to edit; either specify --filename or run interactively") + } + err = prompt.SurveyAskOne(&survey.Select{ + Message: "Edit which file?", + Options: candidates, + }, &filename) + } + } + + if _, ok := gist.Files[filename]; !ok { + return fmt.Errorf("gist has no file %q", filename) + } + + editorCommand, err := cmdutil.DetermineEditor(opts.Config) + if err != nil { + return err + } + text, err := surveyext.Edit( + editorCommand, + "*."+filename, + gist.Files[filename].Content, + // TODO: consider using iostreams here + os.Stdin, os.Stdout, os.Stderr, nil) + + if err != nil { + return err + } + + if text != gist.Files[filename].Content { + gistFile := gist.Files[filename] + gistFile.Content = text // so it appears if they re-edit + filesToUpdate[filename] = text + } + + if !opts.IO.CanPrompt() { + break + } + + if len(candidates) == 1 { + break + } + + choice := "" + + err = prompt.SurveyAskOne(&survey.Select{ + Message: "What next?", + Options: []string{ + "Edit another file", + "Submit", + "Cancel", + }, + }, &choice) + + if err != nil { + return err + } + + stop := false + + switch choice { + case "Edit another file": + continue + case "Submit": + stop = true + case "Cancel": + return cmdutil.SilentError + } + + if stop { + break + } + } + + err = updateGist(client, ghinstance.OverridableDefault(), gist) + if err != nil { + return err + } + + return nil +} + +func updateGist(client *http.Client, hostname string, gist *shared.Gist) error { + body := shared.Gist{ + Description: gist.Description, + Files: gist.Files, + } + + path := "gists/" + gist.ID + + requestByte, err := json.Marshal(body) + if err != nil { + return err + } + + requestBody := bytes.NewReader(requestByte) + + result := shared.Gist{} + + apiClient := api.NewClientFromHTTP(client) + err = apiClient.REST(hostname, "POST", path, requestBody, &result) + + if err != nil { + return err + } + + return nil +} diff --git a/pkg/cmd/gist/gist.go b/pkg/cmd/gist/gist.go index 612ef0bcf..7496a72f0 100644 --- a/pkg/cmd/gist/gist.go +++ b/pkg/cmd/gist/gist.go @@ -2,6 +2,7 @@ package gist import ( gistCreateCmd "github.com/cli/cli/pkg/cmd/gist/create" + gistEditCmd "github.com/cli/cli/pkg/cmd/gist/edit" gistListCmd "github.com/cli/cli/pkg/cmd/gist/list" gistViewCmd "github.com/cli/cli/pkg/cmd/gist/view" "github.com/cli/cli/pkg/cmdutil" @@ -18,6 +19,7 @@ func NewCmdGist(f *cmdutil.Factory) *cobra.Command { cmd.AddCommand(gistCreateCmd.NewCmdCreate(f, nil)) cmd.AddCommand(gistListCmd.NewCmdList(f, nil)) cmd.AddCommand(gistViewCmd.NewCmdView(f, nil)) + cmd.AddCommand(gistEditCmd.NewCmdEdit(f, nil)) return cmd } diff --git a/pkg/cmd/gist/list/http.go b/pkg/cmd/gist/list/http.go index e64243977..4fcde88f8 100644 --- a/pkg/cmd/gist/list/http.go +++ b/pkg/cmd/gist/list/http.go @@ -11,7 +11,7 @@ import ( ) type Gist struct { - Description string `json:"description"` + Description string HTMLURL string `json:"html_url"` UpdatedAt time.Time `json:"updated_at"` Public bool diff --git a/pkg/cmd/gist/shared/shared.go b/pkg/cmd/gist/shared/shared.go index 7cfbc5866..af67e25af 100644 --- a/pkg/cmd/gist/shared/shared.go +++ b/pkg/cmd/gist/shared/shared.go @@ -7,16 +7,19 @@ import ( "github.com/cli/cli/api" ) +// TODO make gist create use this file + type GistFile struct { - Filename string - Type string - Language string - Content string + Filename string `json:"filename"` + Type string `json:"type,omitempty"` + Language string `json:"language,omitempty"` + Content string `json:"content"` } type Gist struct { - Description string - Files map[string]GistFile + ID string `json:"id,omitempty"` + Description string `json:"description"` + Files map[string]*GistFile `json:"files"` } func GetGist(client *http.Client, hostname, gistID string) (*Gist, error) { diff --git a/pkg/cmd/gist/view/view.go b/pkg/cmd/gist/view/view.go index be9ac4bde..313529d20 100644 --- a/pkg/cmd/gist/view/view.go +++ b/pkg/cmd/gist/view/view.go @@ -7,6 +7,7 @@ import ( "strings" "github.com/cli/cli/internal/ghinstance" + "github.com/cli/cli/pkg/cmd/gist/shared" "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/iostreams" "github.com/cli/cli/utils" @@ -65,7 +66,7 @@ func viewRun(opts *ViewOptions) error { return err } - gist, err := getGist(client, ghinstance.OverridableDefault(), gistID) + gist, err := shared.GetGist(client, ghinstance.OverridableDefault(), gistID) if err != nil { return err } From dc345a9f738145e147a47db6d9758ac4004cee22 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Fri, 11 Sep 2020 20:50:25 -0500 Subject: [PATCH 20/67] support --web for gist view --- internal/ghinstance/host.go | 7 +++++++ pkg/cmd/gist/view/view.go | 14 ++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/internal/ghinstance/host.go b/internal/ghinstance/host.go index 560cc9c5c..642dd0846 100644 --- a/internal/ghinstance/host.go +++ b/internal/ghinstance/host.go @@ -55,3 +55,10 @@ func RESTPrefix(hostname string) string { } return "https://api.github.com/" } + +func GistPrefix(hostname string) string { + if IsEnterprise(hostname) { + return fmt.Sprintf("https://%s/gist/", hostname) + } + return fmt.Sprintf("https://gist.%s/", hostname) +} diff --git a/pkg/cmd/gist/view/view.go b/pkg/cmd/gist/view/view.go index 313529d20..5e4da1e69 100644 --- a/pkg/cmd/gist/view/view.go +++ b/pkg/cmd/gist/view/view.go @@ -20,6 +20,7 @@ type ViewOptions struct { Selector string Raw bool + Web bool } func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Command { @@ -47,6 +48,7 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman } cmd.Flags().BoolVarP(&opts.Raw, "raw", "r", false, "do not try and render markdown") + cmd.Flags().BoolVarP(&opts.Web, "web", "w", false, "open gist in browser") return cmd } @@ -54,6 +56,18 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman func viewRun(opts *ViewOptions) error { gistID := opts.Selector + if opts.Web { + gistURL := gistID + if !strings.Contains(gistURL, "/") { + hostname := ghinstance.OverridableDefault() + gistURL = ghinstance.GistPrefix(hostname) + gistID + } + if opts.IO.IsStderrTTY() { + fmt.Fprintf(opts.IO.ErrOut, "Opening %s in your browser.\n", utils.DisplayURL(gistURL)) + } + return utils.OpenInBrowser(gistURL) + } + u, err := url.Parse(opts.Selector) if err == nil { if strings.HasPrefix(u.Path, "/") { From 02d94d6f93741f18095a684d9c3d378a34899462 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Sat, 12 Sep 2020 10:57:48 -0500 Subject: [PATCH 21/67] hmm --- pkg/cmd/gist/list/list.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/gist/list/list.go b/pkg/cmd/gist/list/list.go index bf59497f3..477ec3f9c 100644 --- a/pkg/cmd/gist/list/list.go +++ b/pkg/cmd/gist/list/list.go @@ -77,8 +77,8 @@ func listRun(opts *ListOptions) error { for _, gist := range gists { // TODO i was getting confusing results with table printer's truncation description := gist.Description - if len(description) > 40 { - description = description[0:37] + "..." + if len(description) > 30 { + description = description[0:27] + "..." } tp.AddField(description, nil, cs.Bold) From 269adab75a6a5c0cdc1a270ae53f3af8f574422f Mon Sep 17 00:00:00 2001 From: vilmibm Date: Mon, 14 Sep 2020 10:32:31 -0500 Subject: [PATCH 22/67] improve list output --- pkg/cmd/gist/list/http.go | 15 ++++----------- pkg/cmd/gist/list/list.go | 23 +++++++++++++++++------ pkg/cmd/gist/shared/shared.go | 3 +++ 3 files changed, 24 insertions(+), 17 deletions(-) diff --git a/pkg/cmd/gist/list/http.go b/pkg/cmd/gist/list/http.go index 4fcde88f8..9c462f456 100644 --- a/pkg/cmd/gist/list/http.go +++ b/pkg/cmd/gist/list/http.go @@ -5,20 +5,13 @@ import ( "net/http" "net/url" "sort" - "time" "github.com/cli/cli/api" + "github.com/cli/cli/pkg/cmd/gist/shared" ) -type Gist struct { - Description string - HTMLURL string `json:"html_url"` - UpdatedAt time.Time `json:"updated_at"` - Public bool -} - -func listGists(client *http.Client, hostname string, limit int, visibility string) ([]Gist, error) { - result := []Gist{} +func listGists(client *http.Client, hostname string, limit int, visibility string) ([]shared.Gist, error) { + result := []shared.Gist{} query := url.Values{} if visibility == "all" { @@ -33,7 +26,7 @@ func listGists(client *http.Client, hostname string, limit int, visibility strin return nil, err } - gists := []Gist{} + gists := []shared.Gist{} for _, gist := range result { if len(gists) == limit { diff --git a/pkg/cmd/gist/list/list.go b/pkg/cmd/gist/list/list.go index 477ec3f9c..8aba36fc4 100644 --- a/pkg/cmd/gist/list/list.go +++ b/pkg/cmd/gist/list/list.go @@ -3,6 +3,7 @@ package list import ( "fmt" "net/http" + "time" "github.com/cli/cli/internal/ghinstance" "github.com/cli/cli/pkg/cmdutil" @@ -75,14 +76,24 @@ func listRun(opts *ListOptions) error { tp := utils.NewTablePrinter(opts.IO) for _, gist := range gists { - // TODO i was getting confusing results with table printer's truncation - description := gist.Description - if len(description) > 30 { - description = description[0:27] + "..." + fileCount := 0 + for _, _ = range gist.Files { + fileCount++ } - tp.AddField(description, nil, cs.Bold) - tp.AddField(gist.HTMLURL, nil, nil) + visibility := "public" + visColor := cs.Green + if !gist.Public { + visibility = "secret" + visColor = cs.Red + } + + updatedAt := utils.FuzzyAgo(time.Since(gist.UpdatedAt)) + tp.AddField(gist.ID, nil, nil) + tp.AddField(gist.Description, nil, cs.Bold) + tp.AddField(utils.Pluralize(fileCount, "file"), nil, nil) + tp.AddField(visibility, nil, visColor) + tp.AddField(updatedAt, nil, utils.Gray) tp.EndRow() } diff --git a/pkg/cmd/gist/shared/shared.go b/pkg/cmd/gist/shared/shared.go index af67e25af..75e24c197 100644 --- a/pkg/cmd/gist/shared/shared.go +++ b/pkg/cmd/gist/shared/shared.go @@ -3,6 +3,7 @@ package shared import ( "fmt" "net/http" + "time" "github.com/cli/cli/api" ) @@ -20,6 +21,8 @@ type Gist struct { ID string `json:"id,omitempty"` Description string `json:"description"` Files map[string]*GistFile `json:"files"` + UpdatedAt time.Time `json:"updated_at"` + Public bool } func GetGist(client *http.Client, hostname, gistID string) (*Gist, error) { From 1edff18ad41f6fc369091acf93cab95c81071f1f Mon Sep 17 00:00:00 2001 From: vilmibm Date: Mon, 14 Sep 2020 10:38:35 -0500 Subject: [PATCH 23/67] s/private/secret --- pkg/cmd/gist/list/http.go | 2 +- pkg/cmd/gist/list/list.go | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/cmd/gist/list/http.go b/pkg/cmd/gist/list/http.go index 9c462f456..abc497423 100644 --- a/pkg/cmd/gist/list/http.go +++ b/pkg/cmd/gist/list/http.go @@ -32,7 +32,7 @@ func listGists(client *http.Client, hostname string, limit int, visibility strin if len(gists) == limit { break } - if visibility == "all" || (visibility == "private" && !gist.Public) || (visibility == "public" && gist.Public) { + if visibility == "all" || (visibility == "secret" && !gist.Public) || (visibility == "public" && gist.Public) { gists = append(gists, gist) } } diff --git a/pkg/cmd/gist/list/list.go b/pkg/cmd/gist/list/list.go index 8aba36fc4..1febf5c41 100644 --- a/pkg/cmd/gist/list/list.go +++ b/pkg/cmd/gist/list/list.go @@ -36,13 +36,13 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman } pub := cmd.Flags().Changed("public") - priv := cmd.Flags().Changed("private") + secret := cmd.Flags().Changed("secret") opts.Visibility = "all" - if pub && !priv { + if pub && !secret { opts.Visibility = "public" - } else if priv && !pub { - opts.Visibility = "private" + } else if secret && !pub { + opts.Visibility = "secret" } if runF != nil { @@ -55,7 +55,7 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman cmd.Flags().IntVarP(&opts.Limit, "limit", "L", 10, "Maximum number of gists to fetch") cmd.Flags().Bool("public", false, "Show only public gists") - cmd.Flags().Bool("private", false, "Show only private gists") + cmd.Flags().Bool("secret", false, "Show only secret gists") return cmd } From 415c2ac4829617818f5262213485d469353ac6b0 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Mon, 14 Sep 2020 10:43:34 -0500 Subject: [PATCH 24/67] put gist in core commands --- pkg/cmd/gist/gist.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/pkg/cmd/gist/gist.go b/pkg/cmd/gist/gist.go index 7496a72f0..6c86cc431 100644 --- a/pkg/cmd/gist/gist.go +++ b/pkg/cmd/gist/gist.go @@ -1,6 +1,7 @@ package gist import ( + "github.com/MakeNowJust/heredoc" gistCreateCmd "github.com/cli/cli/pkg/cmd/gist/create" gistEditCmd "github.com/cli/cli/pkg/cmd/gist/edit" gistListCmd "github.com/cli/cli/pkg/cmd/gist/list" @@ -14,6 +15,14 @@ func NewCmdGist(f *cmdutil.Factory) *cobra.Command { Use: "gist", Short: "Create gists", Long: `Work with GitHub gists.`, + Annotations: map[string]string{ + "IsCore": "true", + "help:arguments": heredoc.Doc(` + A gist can be supplied as argument in any of the following formats: + - by ID, e.g. 5b0e0062eb8e9654adad7bb1d81cc75f + - by URL, e.g. "https://gist.github.com/OWNER/5b0e0062eb8e9654adad7bb1d81cc75f"; or + `), + }, } cmd.AddCommand(gistCreateCmd.NewCmdCreate(f, nil)) From 21449213e52b07f341e2b42d3fec9b6d144fcceb Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Mon, 14 Sep 2020 17:50:50 +0200 Subject: [PATCH 25/67] Expand help topic functionality --- pkg/cmd/root/help.go | 5 --- pkg/cmd/root/help_topic.go | 79 +++++++++++++++++++++++++++----------- pkg/cmd/root/root.go | 4 +- 3 files changed, 59 insertions(+), 29 deletions(-) diff --git a/pkg/cmd/root/help.go b/pkg/cmd/root/help.go index 4f9a798fb..400be3834 100644 --- a/pkg/cmd/root/help.go +++ b/pkg/cmd/root/help.go @@ -78,11 +78,6 @@ func rootHelpFunc(command *cobra.Command, args []string) { return } - if helpTopic := command.Annotations["helpTopic"]; helpTopic == "true" { - fmt.Fprint(command.OutOrStdout(), command.Long) - return - } - coreCommands := []string{} additionalCommands := []string{} for _, c := range command.Commands() { diff --git a/pkg/cmd/root/help_topic.go b/pkg/cmd/root/help_topic.go index 8e17685ae..8a4e6f931 100644 --- a/pkg/cmd/root/help_topic.go +++ b/pkg/cmd/root/help_topic.go @@ -1,41 +1,76 @@ package root import ( + "fmt" + "github.com/MakeNowJust/heredoc" "github.com/spf13/cobra" ) func NewHelpTopic(topic string) *cobra.Command { - return &cobra.Command{ - Use: "environment", - Long: heredoc.Doc(` - GITHUB_TOKEN: an authentication token for github.com API requests. Setting this avoids - being prompted to authenticate and takes precedence over previously stored credentials. + topicContent := make(map[string]string) - GITHUB_ENTERPRISE_TOKEN: an authentication token for API requests to GitHub Enterprise. + topicContent["environment"] = heredoc.Doc(` + GITHUB_TOKEN: an authentication token for github.com API requests. Setting this avoids + being prompted to authenticate and takes precedence over previously stored credentials. - GH_REPO: specify the GitHub repository in the "[HOST/]OWNER/REPO" format for commands - that otherwise operate on a local repository. + GITHUB_ENTERPRISE_TOKEN: an authentication token for API requests to GitHub Enterprise. - GH_HOST: specify the GitHub hostname for commands that would otherwise assume - the "github.com" host when not in a context of an existing repository. + GH_REPO: specify the GitHub repository in the "[HOST/]OWNER/REPO" format for commands + that otherwise operate on a local repository. - GH_EDITOR, GIT_EDITOR, VISUAL, EDITOR (in order of precedence): the editor tool to use - for authoring text. + GH_HOST: specify the GitHub hostname for commands that would otherwise assume + the "github.com" host when not in a context of an existing repository. - BROWSER: the web browser to use for opening links. + GH_EDITOR, GIT_EDITOR, VISUAL, EDITOR (in order of precedence): the editor tool to use + for authoring text. - DEBUG: set to any value to enable verbose output to standard error. Include values "api" - or "oauth" to print detailed information about HTTP requests or authentication flow. + BROWSER: the web browser to use for opening links. - GLAMOUR_STYLE: the style to use for rendering Markdown. See - https://github.com/charmbracelet/glamour#styles + DEBUG: set to any value to enable verbose output to standard error. Include values "api" + or "oauth" to print detailed information about HTTP requests or authentication flow. - NO_COLOR: avoid printing ANSI escape sequences for color output. - `), + GLAMOUR_STYLE: the style to use for rendering Markdown. See + https://github.com/charmbracelet/glamour#styles + + NO_COLOR: avoid printing ANSI escape sequences for color output. + `) + + cmd := &cobra.Command{ + Use: topic, + Long: topicContent[topic], Hidden: true, - Annotations: map[string]string{ - "helpTopic": "true", - }, + Args: cobra.NoArgs, } + + cmd.SetHelpFunc(helpTopicHelpFunc) + cmd.SetUsageFunc(helpTopicUsageFunc) + + return cmd +} + +func helpTopicHelpFunc(command *cobra.Command, args []string) { + if len(args) >= 2 && args[1] != "--help" && args[1] != "-h" { + command.Printf("unknown command %q for %q\n", args[1], command.CommandPath()) + + if args[1] == "help" { + command.Print("\nDid you mean this?\n") + command.Printf("\t%s\n\n", "--help") + } else { + command.Printf("\n") + } + + helpTopicUsageFunc(command) + command.Printf("\n") + hasFailed = true + return + } + + fmt.Fprint(command.OutOrStdout(), command.Long) +} + +func helpTopicUsageFunc(command *cobra.Command) error { + command.Printf("Usage: gh help %s", command.Use) + + return nil } diff --git a/pkg/cmd/root/root.go b/pkg/cmd/root/root.go index f3319e456..7ad31fb68 100644 --- a/pkg/cmd/root/root.go +++ b/pkg/cmd/root/root.go @@ -82,8 +82,7 @@ func NewCmdRoot(f *cmdutil.Factory, version, buildDate string) *cobra.Command { cmdutil.DisableAuthCheck(cmd) - // CHILD COMMANDS - + // Child commands cmd.AddCommand(aliasCmd.NewCmdAlias(f)) cmd.AddCommand(authCmd.NewCmdAuth(f)) cmd.AddCommand(configCmd.NewCmdConfig(f)) @@ -91,6 +90,7 @@ func NewCmdRoot(f *cmdutil.Factory, version, buildDate string) *cobra.Command { cmd.AddCommand(gistCmd.NewCmdGist(f)) cmd.AddCommand(NewCmdCompletion(f.IOStreams)) + // Help Topics cmd.AddCommand(NewHelpTopic("environment")) // the `api` command should not inherit any extra HTTP headers From 2df6a6eb8cac7c195e0bc959e35f710ecb3e9dd8 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Mon, 14 Sep 2020 11:04:37 -0500 Subject: [PATCH 26/67] s/private/secret/ --- pkg/cmd/gist/list/list_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/cmd/gist/list/list_test.go b/pkg/cmd/gist/list/list_test.go index 5541a51bc..b2e50e99c 100644 --- a/pkg/cmd/gist/list/list_test.go +++ b/pkg/cmd/gist/list/list_test.go @@ -32,16 +32,16 @@ func TestNewCmdList(t *testing.T) { }, }, { - name: "private", - cli: "--private", + name: "secret", + cli: "--secret", wants: ListOptions{ Limit: 10, - Visibility: "private", + Visibility: "secret", }, }, { - name: "public and private", - cli: "--private --public", + name: "public and secret", + cli: "--secret --public", wants: ListOptions{ Limit: 10, Visibility: "all", From e7ab1b753eac35141f3d74cede3dc06397dbca74 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Mon, 14 Sep 2020 11:05:26 -0500 Subject: [PATCH 27/67] linter appeasement --- pkg/cmd/gist/edit/edit.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/gist/edit/edit.go b/pkg/cmd/gist/edit/edit.go index 097cfad6b..3480993f5 100644 --- a/pkg/cmd/gist/edit/edit.go +++ b/pkg/cmd/gist/edit/edit.go @@ -80,7 +80,7 @@ func editRun(opts *EditOptions) error { filesToUpdate := map[string]string{} - for true { + for { filename := opts.Filename candidates := []string{} for filename, _ := range gist.Files { From ecfbaaa31c05f82b9eac1f5b530a2ac5288fa464 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Mon, 14 Sep 2020 11:07:33 -0500 Subject: [PATCH 28/67] linter appeasement --- pkg/cmd/gist/edit/edit.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/cmd/gist/edit/edit.go b/pkg/cmd/gist/edit/edit.go index 3480993f5..76285ed6a 100644 --- a/pkg/cmd/gist/edit/edit.go +++ b/pkg/cmd/gist/edit/edit.go @@ -100,6 +100,10 @@ func editRun(opts *EditOptions) error { Message: "Edit which file?", Options: candidates, }, &filename) + + if err != nil { + return fmt.Errorf("could not prompt: %w", err) + } } } From 190d76abc5e665425f27024a94086408b157e51a Mon Sep 17 00:00:00 2001 From: vilmibm Date: Mon, 14 Sep 2020 11:08:32 -0500 Subject: [PATCH 29/67] linter appeasement --- pkg/cmd/gist/edit/edit.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/gist/edit/edit.go b/pkg/cmd/gist/edit/edit.go index 76285ed6a..de94d09c7 100644 --- a/pkg/cmd/gist/edit/edit.go +++ b/pkg/cmd/gist/edit/edit.go @@ -83,7 +83,7 @@ func editRun(opts *EditOptions) error { for { filename := opts.Filename candidates := []string{} - for filename, _ := range gist.Files { + for filename := range gist.Files { candidates = append(candidates, filename) } From 41382a558b94eb175a14ec7b51233225f4003218 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Mon, 14 Sep 2020 11:09:50 -0500 Subject: [PATCH 30/67] better err --- pkg/cmd/gist/edit/edit.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/gist/edit/edit.go b/pkg/cmd/gist/edit/edit.go index de94d09c7..3caf69b34 100644 --- a/pkg/cmd/gist/edit/edit.go +++ b/pkg/cmd/gist/edit/edit.go @@ -152,7 +152,7 @@ func editRun(opts *EditOptions) error { }, &choice) if err != nil { - return err + return fmt.Errorf("could not prompt: %w", err) } stop := false From b3266d94544fe3e3d8c9c58bc68f0f6b946a7983 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Mon, 14 Sep 2020 11:10:17 -0500 Subject: [PATCH 31/67] linter appeasement --- pkg/cmd/gist/list/list.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/gist/list/list.go b/pkg/cmd/gist/list/list.go index 1febf5c41..dbb9969fb 100644 --- a/pkg/cmd/gist/list/list.go +++ b/pkg/cmd/gist/list/list.go @@ -77,7 +77,7 @@ func listRun(opts *ListOptions) error { for _, gist := range gists { fileCount := 0 - for _, _ = range gist.Files { + for range gist.Files { fileCount++ } From 00b4eab573875f371ccd3e25a88610cb7730fae9 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Mon, 14 Sep 2020 11:29:11 -0500 Subject: [PATCH 32/67] include in readme --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index e3aa83aa4..c5a14ddc2 100644 --- a/README.md +++ b/README.md @@ -18,6 +18,7 @@ We'd love to hear your feedback about `gh`. If you spot bugs or have features th - `gh pr [status, list, view, checkout, create]` - `gh issue [status, list, view, create]` - `gh repo [view, create, clone, fork]` +- `gh gist [create, list, view, edit]` - `gh auth [login, logout, refresh, status]` - `gh config [get, set]` - `gh help` From 7ecb6a413fe9663614f46b6837ca71012762de47 Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Mon, 14 Sep 2020 22:21:05 +0200 Subject: [PATCH 33/67] Add tests for help topics --- pkg/cmd/root/help_topic.go | 24 ++-------- pkg/cmd/root/help_topic_test.go | 79 +++++++++++++++++++++++++++++++++ pkg/cmd/root/root.go | 2 +- 3 files changed, 83 insertions(+), 22 deletions(-) create mode 100644 pkg/cmd/root/help_topic_test.go diff --git a/pkg/cmd/root/help_topic.go b/pkg/cmd/root/help_topic.go index 8a4e6f931..1357825a2 100644 --- a/pkg/cmd/root/help_topic.go +++ b/pkg/cmd/root/help_topic.go @@ -1,8 +1,6 @@ package root import ( - "fmt" - "github.com/MakeNowJust/heredoc" "github.com/spf13/cobra" ) @@ -41,6 +39,7 @@ func NewHelpTopic(topic string) *cobra.Command { Long: topicContent[topic], Hidden: true, Args: cobra.NoArgs, + Run: helpTopicHelpFunc, } cmd.SetHelpFunc(helpTopicHelpFunc) @@ -50,27 +49,10 @@ func NewHelpTopic(topic string) *cobra.Command { } func helpTopicHelpFunc(command *cobra.Command, args []string) { - if len(args) >= 2 && args[1] != "--help" && args[1] != "-h" { - command.Printf("unknown command %q for %q\n", args[1], command.CommandPath()) - - if args[1] == "help" { - command.Print("\nDid you mean this?\n") - command.Printf("\t%s\n\n", "--help") - } else { - command.Printf("\n") - } - - helpTopicUsageFunc(command) - command.Printf("\n") - hasFailed = true - return - } - - fmt.Fprint(command.OutOrStdout(), command.Long) + command.Print(command.Long) } func helpTopicUsageFunc(command *cobra.Command) error { - command.Printf("Usage: gh help %s", command.Use) - + command.Printf("Usage: gh help %s", command.Use) return nil } diff --git a/pkg/cmd/root/help_topic_test.go b/pkg/cmd/root/help_topic_test.go new file mode 100644 index 000000000..f194541ac --- /dev/null +++ b/pkg/cmd/root/help_topic_test.go @@ -0,0 +1,79 @@ +package root + +import ( + "testing" + + "github.com/cli/cli/pkg/iostreams" + "github.com/stretchr/testify/assert" +) + +func TestNewHelpTopic(t *testing.T) { + tests := []struct { + name string + topic string + args []string + flags []string + wantsErr bool + }{ + { + name: "valid topic", + topic: "environment", + args: []string{}, + flags: []string{}, + wantsErr: false, + }, + { + name: "invalid topic", + topic: "invalid", + args: []string{}, + flags: []string{}, + wantsErr: false, + }, + { + name: "more than zero args", + topic: "environment", + args: []string{"invalid"}, + flags: []string{}, + wantsErr: true, + }, + { + name: "more than zero flags", + topic: "environment", + args: []string{}, + flags: []string{"--invalid"}, + wantsErr: true, + }, + { + name: "help arg", + topic: "environment", + args: []string{"help"}, + flags: []string{}, + wantsErr: true, + }, + { + name: "help flag", + topic: "environment", + args: []string{}, + flags: []string{"--help"}, + wantsErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, _, stdout, stderr := iostreams.Test() + + cmd := NewHelpTopic(tt.topic) + cmd.SetArgs(append(tt.args, tt.flags...)) + cmd.SetOut(stdout) + cmd.SetErr(stderr) + + _, err := cmd.ExecuteC() + if tt.wantsErr { + assert.Error(t, err) + return + } + assert.NoError(t, err) + }) + } +} diff --git a/pkg/cmd/root/root.go b/pkg/cmd/root/root.go index 7ad31fb68..e3257ed09 100644 --- a/pkg/cmd/root/root.go +++ b/pkg/cmd/root/root.go @@ -90,7 +90,7 @@ func NewCmdRoot(f *cmdutil.Factory, version, buildDate string) *cobra.Command { cmd.AddCommand(gistCmd.NewCmdGist(f)) cmd.AddCommand(NewCmdCompletion(f.IOStreams)) - // Help Topics + // Help topics cmd.AddCommand(NewHelpTopic("environment")) // the `api` command should not inherit any extra HTTP headers From 3d350e8707450d06036841c05223375002b03cd1 Mon Sep 17 00:00:00 2001 From: Cristian Dominguez <6853656+cristiand391@users.noreply.github.com> Date: Mon, 14 Sep 2020 20:31:22 +0000 Subject: [PATCH 34/67] Go 1.15+ is required to run the test suit --- .github/CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 8a1dc4849..09f60f318 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -27,7 +27,7 @@ Please avoid: Prerequisites: - Go 1.13+ for building the binary -- Go 1.14+ for running the test suite +- Go 1.15+ for running the test suite Build with: `make` or `go build -o bin/gh ./cmd/gh` From 50291aa6de313486cc498f7f6094192c41314703 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 15 Sep 2020 11:43:49 +0200 Subject: [PATCH 35/67] Add PAGER support to the `api` command --- pkg/cmd/api/api.go | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/pkg/cmd/api/api.go b/pkg/cmd/api/api.go index f051883a8..9ec4de435 100644 --- a/pkg/cmd/api/api.go +++ b/pkg/cmd/api/api.go @@ -13,6 +13,7 @@ import ( "sort" "strconv" "strings" + "syscall" "github.com/MakeNowJust/heredoc" "github.com/cli/cli/internal/ghinstance" @@ -196,6 +197,12 @@ func apiRun(opts *ApiOptions) error { headersOutputStream := opts.IO.Out if opts.Silent { opts.IO.Out = ioutil.Discard + } else { + err := opts.IO.StartPager() + if err != nil { + return err + } + defer opts.IO.StopPager() } host := ghinstance.OverridableDefault() @@ -265,12 +272,13 @@ func processResponse(resp *http.Response, opts *ApiOptions, headersOutputStream if isJSON && opts.IO.ColorEnabled() { err = jsoncolor.Write(opts.IO.Out, responseBody, " ") - if err != nil { - return - } } else { _, err = io.Copy(opts.IO.Out, responseBody) - if err != nil { + } + if err != nil { + if errors.Is(err, syscall.EPIPE) { + err = nil + } else { return } } From 1887fc07c9ae697004ff126f00cd4ffa4a0bebd9 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Tue, 15 Sep 2020 09:39:30 -0500 Subject: [PATCH 36/67] working on list tests, need to debug --- pkg/cmd/gist/list/http.go | 3 ++ pkg/cmd/gist/list/list.go | 7 +++- pkg/cmd/gist/list/list_test.go | 76 ++++++++++++++++++++++++++++++++++ pkg/cmd/gist/shared/shared.go | 2 +- pkg/httpmock/stub.go | 3 ++ 5 files changed, 89 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/gist/list/http.go b/pkg/cmd/gist/list/http.go index abc497423..4e5445ad8 100644 --- a/pkg/cmd/gist/list/http.go +++ b/pkg/cmd/gist/list/http.go @@ -26,6 +26,9 @@ func listGists(client *http.Client, hostname string, limit int, visibility strin return nil, err } + // TODO in tests the api call is matching properly and encoding json properly but i'm getting no + // result and no parse error, wtf? + gists := []shared.Gist{} for _, gist := range result { diff --git a/pkg/cmd/gist/list/list.go b/pkg/cmd/gist/list/list.go index dbb9969fb..ec9e45c0d 100644 --- a/pkg/cmd/gist/list/list.go +++ b/pkg/cmd/gist/list/list.go @@ -16,6 +16,8 @@ type ListOptions struct { IO *iostreams.IOStreams HttpClient func() (*http.Client, error) + Since func(t time.Time) time.Duration + Limit int Visibility string // all, secret, public } @@ -24,6 +26,9 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman opts := &ListOptions{ IO: f.IOStreams, HttpClient: f.HttpClient, + Since: func(t time.Time) time.Duration { + return time.Since(t) + }, } cmd := &cobra.Command{ @@ -88,7 +93,7 @@ func listRun(opts *ListOptions) error { visColor = cs.Red } - updatedAt := utils.FuzzyAgo(time.Since(gist.UpdatedAt)) + updatedAt := utils.FuzzyAgo(opts.Since(gist.UpdatedAt)) tp.AddField(gist.ID, nil, nil) tp.AddField(gist.Description, nil, cs.Bold) tp.AddField(utils.Pluralize(fileCount, "file"), nil, nil) diff --git a/pkg/cmd/gist/list/list_test.go b/pkg/cmd/gist/list/list_test.go index b2e50e99c..dcae4db60 100644 --- a/pkg/cmd/gist/list/list_test.go +++ b/pkg/cmd/gist/list/list_test.go @@ -2,9 +2,14 @@ package list import ( "bytes" + "net/http" "testing" + "time" + "github.com/cli/cli/pkg/cmd/gist/shared" "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/httpmock" + "github.com/cli/cli/pkg/iostreams" "github.com/google/shlex" "github.com/stretchr/testify/assert" ) @@ -84,3 +89,74 @@ func TestNewCmdList(t *testing.T) { } // TODO execution tests + +func Test_listRun(t *testing.T) { + tests := []struct { + name string + opts *ListOptions + wantOut string + stubs func(*httpmock.Registry) + }{ + { + name: "no gists", + opts: &ListOptions{}, + stubs: func(reg *httpmock.Registry) { + reg.Register(httpmock.REST("GET", "gists"), + httpmock.JSONResponse([]shared.Gist{})) + + }, + wantOut: "", + }, + + { + name: "default behavior", + opts: &ListOptions{}, + wantOut: "TODO", + }, + // TODO public filter + // TODO secret filter + // TODO limit specified + } + + for _, tt := range tests { + reg := &httpmock.Registry{} + if tt.stubs == nil { + reg.Register(httpmock.REST("GET", "gists"), + httpmock.JSONResponse([]shared.Gist{ + { + ID: "1234567890", + Description: "", + Files: map[string]*shared.GistFile{ + "cool.txt": { + Content: "lol", + }, + }, + Public: true, + UpdatedAt: time.Time{}, + }, + })) + } else { + tt.stubs(reg) + } + + tt.opts.HttpClient = func() (*http.Client, error) { + return &http.Client{Transport: reg}, nil + } + + tt.opts.Since = func(t time.Time) time.Duration { + d, _ := time.ParseDuration("6h") + return d + } + + io, _, stdout, _ := iostreams.Test() + tt.opts.IO = io + + t.Run(tt.name, func(t *testing.T) { + err := listRun(tt.opts) + assert.NoError(t, err) + + assert.Equal(t, tt.wantOut, stdout.String()) + reg.Verify(t) + }) + } +} diff --git a/pkg/cmd/gist/shared/shared.go b/pkg/cmd/gist/shared/shared.go index 75e24c197..95d8bae31 100644 --- a/pkg/cmd/gist/shared/shared.go +++ b/pkg/cmd/gist/shared/shared.go @@ -22,7 +22,7 @@ type Gist struct { Description string `json:"description"` Files map[string]*GistFile `json:"files"` UpdatedAt time.Time `json:"updated_at"` - Public bool + Public bool `json:"public"` } func GetGist(client *http.Client, hostname, gistID string) (*Gist, error) { diff --git a/pkg/httpmock/stub.go b/pkg/httpmock/stub.go index a1dcefaa3..e4572efe9 100644 --- a/pkg/httpmock/stub.go +++ b/pkg/httpmock/stub.go @@ -3,6 +3,7 @@ package httpmock import ( "bytes" "encoding/json" + "fmt" "io" "io/ioutil" "net/http" @@ -86,6 +87,8 @@ func StatusStringResponse(status int, body string) Responder { func JSONResponse(body interface{}) Responder { return func(req *http.Request) (*http.Response, error) { b, _ := json.Marshal(body) + fmt.Printf("DEBUG %#v\n", "COOOOOL") + fmt.Printf("DEBUG %#v\n", string(b)) return httpResponse(200, req, bytes.NewBuffer(b)), nil } } From 9f486efbc64ea338071fbf3633c74437f92bf024 Mon Sep 17 00:00:00 2001 From: Nate Smith Date: Tue, 15 Sep 2020 09:59:27 -0500 Subject: [PATCH 37/67] hackday: gh repo garden (#1049) * add gh repo garden * move file * oops * fixes * fix clearing * block windows sadly * broken wip * fix api thing * do not add to client * move helper as it does not work on windows * hide command * macos fix * Update pkg/cmd/repo/garden/garden.go Co-authored-by: Lee Reilly * default for key input loop * get redrawing working * clean up garden update, it all works * notes * fix arrow keys and just do wads/arrows/vi * this function is only called once now * support ghes * add a progress indicator * cap maxCommits Co-authored-by: Lee Reilly --- pkg/cmd/repo/garden/garden.go | 480 ++++++++++++++++++++++++++++++++++ pkg/cmd/repo/garden/http.go | 105 ++++++++ pkg/cmd/repo/repo.go | 2 + 3 files changed, 587 insertions(+) create mode 100644 pkg/cmd/repo/garden/garden.go create mode 100644 pkg/cmd/repo/garden/http.go diff --git a/pkg/cmd/repo/garden/garden.go b/pkg/cmd/repo/garden/garden.go new file mode 100644 index 000000000..46761b253 --- /dev/null +++ b/pkg/cmd/repo/garden/garden.go @@ -0,0 +1,480 @@ +package garden + +import ( + "bytes" + "errors" + "fmt" + "io" + "math/rand" + "net/http" + "os/exec" + "runtime" + "strconv" + "strings" + + "github.com/cli/cli/api" + "github.com/cli/cli/internal/ghinstance" + "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/utils" + "github.com/spf13/cobra" +) + +type Geometry struct { + Width int + Height int + Density float64 + Repository ghrepo.Interface +} + +type Player struct { + X int + Y int + Char string + Geo *Geometry + ShoeMoistureContent int +} + +type Commit struct { + Email string + Handle string + Sha string + Char string +} + +type Cell struct { + Char string + StatusLine string +} + +const ( + DirUp = iota + DirDown + DirLeft + DirRight +) + +type Direction = int + +func (p *Player) move(direction Direction) bool { + switch direction { + case DirUp: + if p.Y == 0 { + return false + } + p.Y-- + case DirDown: + if p.Y == p.Geo.Height-1 { + return false + } + p.Y++ + case DirLeft: + if p.X == 0 { + return false + } + p.X-- + case DirRight: + if p.X == p.Geo.Width-1 { + return false + } + p.X++ + } + + return true +} + +type GardenOptions struct { + HttpClient func() (*http.Client, error) + IO *iostreams.IOStreams + BaseRepo func() (ghrepo.Interface, error) + + RepoArg string +} + +func NewCmdGarden(f *cmdutil.Factory, runF func(*GardenOptions) error) *cobra.Command { + opts := GardenOptions{ + IO: f.IOStreams, + HttpClient: f.HttpClient, + BaseRepo: f.BaseRepo, + } + + cmd := &cobra.Command{ + Use: "garden []", + Short: "Explore a git repository as a garden", + Long: "Use arrow keys, WASD or vi keys to move. q to quit.", + Hidden: true, + RunE: func(c *cobra.Command, args []string) error { + if len(args) > 0 { + opts.RepoArg = args[0] + } + if runF != nil { + return runF(&opts) + } + return gardenRun(&opts) + }, + } + + return cmd +} + +func gardenRun(opts *GardenOptions) error { + out := opts.IO.Out + + if runtime.GOOS == "windows" { + return errors.New("sorry :( this command only works on linux and macos") + } + + if !opts.IO.IsStdoutTTY() { + return errors.New("must be connected to a terminal") + } + + httpClient, err := opts.HttpClient() + if err != nil { + return err + } + + var toView ghrepo.Interface + apiClient := api.NewClientFromHTTP(httpClient) + if opts.RepoArg == "" { + var err error + toView, err = opts.BaseRepo() + if err != nil { + return err + } + } else { + var err error + viewURL := opts.RepoArg + if !strings.Contains(viewURL, "/") { + currentUser, err := api.CurrentLoginName(apiClient, ghinstance.Default()) + if err != nil { + return err + } + viewURL = currentUser + "/" + viewURL + } + toView, err = ghrepo.FromFullName(viewURL) + if err != nil { + return fmt.Errorf("argument error: %w", err) + } + } + + seed := computeSeed(ghrepo.FullName(toView)) + rand.Seed(seed) + + termWidth, termHeight, err := utils.TerminalSize(out) + if err != nil { + return err + } + + termWidth -= 10 + termHeight -= 10 + + geo := &Geometry{ + Width: termWidth, + Height: termHeight, + Repository: toView, + // TODO based on number of commits/cells instead of just hardcoding + Density: 0.3, + } + + maxCommits := (geo.Width * geo.Height) / 2 + + opts.IO.StartProgressIndicator() + fmt.Fprintln(out, "gathering commits; this could take a minute...") + commits, err := getCommits(httpClient, toView, maxCommits) + opts.IO.StopProgressIndicator() + if err != nil { + return err + } + player := &Player{0, 0, utils.Bold("@"), geo, 0} + + garden := plantGarden(commits, geo) + clear(opts.IO) + drawGarden(out, garden, player) + + // thanks stackoverflow https://stackoverflow.com/a/17278776 + if runtime.GOOS == "darwin" { + _ = exec.Command("stty", "-f", "/dev/tty", "cbreak", "min", "1").Run() + _ = exec.Command("stty", "-f", "/dev/tty", "-echo").Run() + } else { + _ = exec.Command("stty", "-F", "/dev/tty", "cbreak", "min", "1").Run() + _ = exec.Command("stty", "-F", "/dev/tty", "-echo").Run() + } + + var b []byte = make([]byte, 3) + for { + _, _ = opts.IO.In.Read(b) + + oldX := player.X + oldY := player.Y + moved := false + quitting := false + continuing := false + + switch { + case isLeft(b): + moved = player.move(DirLeft) + case isRight(b): + moved = player.move(DirRight) + case isUp(b): + moved = player.move(DirUp) + case isDown(b): + moved = player.move(DirDown) + case isQuit(b): + quitting = true + default: + continuing = true + } + + if quitting { + break + } + + if !moved || continuing { + continue + } + + underPlayer := garden[player.Y][player.X] + previousCell := garden[oldY][oldX] + + // print whatever was just under player + + fmt.Fprint(out, "\033[;H") // move to top left + for x := 0; x < oldX && x < player.Geo.Width; x++ { + fmt.Fprint(out, "\033[C") + } + for y := 0; y < oldY && y < player.Geo.Height; y++ { + fmt.Fprint(out, "\033[B") + } + fmt.Fprint(out, previousCell.Char) + + // print player character + fmt.Fprint(out, "\033[;H") // move to top left + for x := 0; x < player.X && x < player.Geo.Width; x++ { + fmt.Fprint(out, "\033[C") + } + for y := 0; y < player.Y && y < player.Geo.Height; y++ { + fmt.Fprint(out, "\033[B") + } + fmt.Fprint(out, player.Char) + + // handle stream wettening + + if strings.Contains(underPlayer.StatusLine, "stream") { + player.ShoeMoistureContent = 5 + } else { + if player.ShoeMoistureContent > 0 { + player.ShoeMoistureContent-- + } + } + + // status line stuff + sl := statusLine(garden, player) + + fmt.Fprint(out, "\033[;H") // move to top left + for y := 0; y < player.Geo.Height-1; y++ { + fmt.Fprint(out, "\033[B") + } + fmt.Fprintln(out) + fmt.Fprintln(out) + + fmt.Fprint(out, utils.Bold(sl)) + } + + clear(opts.IO) + fmt.Fprint(out, "\033[?25h") + fmt.Fprintln(out) + fmt.Fprintln(out, utils.Bold("You turn and walk away from the wildflower garden...")) + + return nil +} + +func isLeft(b []byte) bool { + left := []byte{27, 91, 68} + r := rune(b[0]) + return bytes.EqualFold(b, left) || r == 'a' || r == 'h' +} + +func isRight(b []byte) bool { + right := []byte{27, 91, 67} + r := rune(b[0]) + return bytes.EqualFold(b, right) || r == 'd' || r == 'l' +} + +func isDown(b []byte) bool { + down := []byte{27, 91, 66} + r := rune(b[0]) + return bytes.EqualFold(b, down) || r == 's' || r == 'j' +} + +func isUp(b []byte) bool { + up := []byte{27, 91, 65} + r := rune(b[0]) + return bytes.EqualFold(b, up) || r == 'w' || r == 'k' +} + +func isQuit(b []byte) bool { + return rune(b[0]) == 'q' +} + +func plantGarden(commits []*Commit, geo *Geometry) [][]*Cell { + cellIx := 0 + grassCell := &Cell{RGB(0, 200, 0, ","), "You're standing on a patch of grass in a field of wildflowers."} + garden := [][]*Cell{} + streamIx := rand.Intn(geo.Width - 1) + if streamIx == geo.Width/2 { + streamIx-- + } + tint := 0 + for y := 0; y < geo.Height; y++ { + if cellIx == len(commits)-1 { + break + } + garden = append(garden, []*Cell{}) + for x := 0; x < geo.Width; x++ { + if (y > 0 && (x == 0 || x == geo.Width-1)) || y == geo.Height-1 { + garden[y] = append(garden[y], &Cell{ + Char: RGB(0, 150, 0, "^"), + StatusLine: "You're standing under a tall, leafy tree.", + }) + continue + } + if x == streamIx { + garden[y] = append(garden[y], &Cell{ + Char: RGB(tint, tint, 255, "#"), + StatusLine: "You're standing in a shallow stream. It's refreshing.", + }) + tint += 15 + streamIx-- + if rand.Float64() < 0.5 { + streamIx++ + } + if streamIx < 0 { + streamIx = 0 + } + if streamIx > geo.Width { + streamIx = geo.Width + } + continue + } + if y == 0 && (x < geo.Width/2 || x > geo.Width/2) { + garden[y] = append(garden[y], &Cell{ + Char: RGB(0, 200, 0, ","), + StatusLine: "You're standing by a wildflower garden. There is a light breeze.", + }) + continue + } else if y == 0 && x == geo.Width/2 { + garden[y] = append(garden[y], &Cell{ + Char: RGB(139, 69, 19, "+"), + StatusLine: fmt.Sprintf("You're standing in front of a weather-beaten sign that says %s.", ghrepo.FullName(geo.Repository)), + }) + continue + } + + if cellIx == len(commits)-1 { + garden[y] = append(garden[y], grassCell) + continue + } + + chance := rand.Float64() + if chance <= geo.Density { + commit := commits[cellIx] + garden[y] = append(garden[y], &Cell{ + Char: commits[cellIx].Char, + StatusLine: fmt.Sprintf("You're standing at a flower called %s planted by %s.", commit.Sha[0:6], commit.Handle), + }) + cellIx++ + } else { + garden[y] = append(garden[y], grassCell) + } + } + } + + return garden +} + +func drawGarden(out io.Writer, garden [][]*Cell, player *Player) { + fmt.Fprint(out, "\033[?25l") // hide cursor. it needs to be restored at command exit. + sl := "" + for y, gardenRow := range garden { + for x, gardenCell := range gardenRow { + char := "" + underPlayer := (player.X == x && player.Y == y) + if underPlayer { + sl = gardenCell.StatusLine + char = utils.Bold(player.Char) + + if strings.Contains(gardenCell.StatusLine, "stream") { + player.ShoeMoistureContent = 5 + } + } else { + char = gardenCell.Char + } + + fmt.Fprint(out, char) + } + fmt.Fprintln(out) + } + + fmt.Println() + fmt.Fprintln(out, utils.Bold(sl)) +} + +func statusLine(garden [][]*Cell, player *Player) string { + statusLine := garden[player.Y][player.X].StatusLine + " " + if player.ShoeMoistureContent > 1 { + statusLine += "\nYour shoes squish with water from the stream." + } else if player.ShoeMoistureContent == 1 { + statusLine += "\nYour shoes seem to have dried out." + } else { + statusLine += "\n " + } + + return statusLine +} + +func shaToColorFunc(sha string) func(string) string { + return func(c string) string { + red, err := strconv.ParseInt(sha[0:2], 16, 64) + if err != nil { + panic(err) + } + + green, err := strconv.ParseInt(sha[2:4], 16, 64) + if err != nil { + panic(err) + } + + blue, err := strconv.ParseInt(sha[4:6], 16, 64) + if err != nil { + panic(err) + } + + return fmt.Sprintf("\033[38;2;%d;%d;%dm%s\033[0m", red, green, blue, c) + } +} + +func computeSeed(seed string) int64 { + lol := "" + + for _, r := range seed { + lol += fmt.Sprintf("%d", int(r)) + } + + result, err := strconv.ParseInt(lol[0:10], 10, 64) + if err != nil { + panic(err) + } + + return result +} + +func clear(io *iostreams.IOStreams) { + cmd := exec.Command("clear") + cmd.Stdout = io.Out + _ = cmd.Run() +} + +func RGB(r, g, b int, x string) string { + return fmt.Sprintf("\033[38;2;%d;%d;%dm%s\033[0m", r, g, b, x) +} diff --git a/pkg/cmd/repo/garden/http.go b/pkg/cmd/repo/garden/http.go new file mode 100644 index 000000000..ff7f47fe0 --- /dev/null +++ b/pkg/cmd/repo/garden/http.go @@ -0,0 +1,105 @@ +package garden + +import ( + "encoding/json" + "errors" + "fmt" + "io/ioutil" + "net/http" + "strings" + "time" + + "github.com/cli/cli/internal/ghinstance" + "github.com/cli/cli/internal/ghrepo" +) + +func getCommits(client *http.Client, repo ghrepo.Interface, maxCommits int) ([]*Commit, error) { + type Item struct { + Author struct { + Login string + } + Sha string + } + + type Result []Item + + commits := []*Commit{} + + pathF := func(page int) string { + return fmt.Sprintf("repos/%s/%s/commits?per_page=100&page=%d", repo.RepoOwner(), repo.RepoName(), page) + } + + page := 1 + paginating := true + for paginating { + if len(commits) >= maxCommits { + break + } + result := Result{} + resp, err := getResponse(client, pathF(page), &result) + if err != nil { + return nil, err + } + for _, r := range result { + colorFunc := shaToColorFunc(r.Sha) + handle := r.Author.Login + if handle == "" { + handle = "a mysterious stranger" + } + commits = append(commits, &Commit{ + Handle: handle, + Sha: r.Sha, + Char: colorFunc(string(handle[0])), + }) + } + link := resp.Header["Link"] + if !strings.Contains(link[0], "last") { + paginating = false + } + page++ + time.Sleep(500) + } + + // reverse to get older commits first + for i, j := 0, len(commits)-1; i < j; i, j = i+1, j-1 { + commits[i], commits[j] = commits[j], commits[i] + } + + return commits, nil +} + +func getResponse(client *http.Client, path string, data interface{}) (*http.Response, error) { + url := ghinstance.RESTPrefix(ghinstance.OverridableDefault()) + path + req, err := http.NewRequest("GET", url, nil) + if err != nil { + return nil, err + } + + req.Header.Set("Content-Type", "application/json; charset=utf-8") + resp, err := client.Do(req) + if err != nil { + return nil, err + } + defer resp.Body.Close() + + success := resp.StatusCode >= 200 && resp.StatusCode < 300 + if !success { + return nil, errors.New("api call failed") + } + + if resp.StatusCode == http.StatusNoContent { + return resp, nil + } + + b, err := ioutil.ReadAll(resp.Body) + if err != nil { + return nil, err + } + + err = json.Unmarshal(b, &data) + if err != nil { + return nil, err + } + + return resp, nil +} diff --git a/pkg/cmd/repo/repo.go b/pkg/cmd/repo/repo.go index 551c96641..02e6c368b 100644 --- a/pkg/cmd/repo/repo.go +++ b/pkg/cmd/repo/repo.go @@ -6,6 +6,7 @@ import ( repoCreateCmd "github.com/cli/cli/pkg/cmd/repo/create" creditsCmd "github.com/cli/cli/pkg/cmd/repo/credits" repoForkCmd "github.com/cli/cli/pkg/cmd/repo/fork" + gardenCmd "github.com/cli/cli/pkg/cmd/repo/garden" repoViewCmd "github.com/cli/cli/pkg/cmd/repo/view" "github.com/cli/cli/pkg/cmdutil" "github.com/spf13/cobra" @@ -36,6 +37,7 @@ func NewCmdRepo(f *cmdutil.Factory) *cobra.Command { cmd.AddCommand(repoCloneCmd.NewCmdClone(f, nil)) cmd.AddCommand(repoCreateCmd.NewCmdCreate(f, nil)) cmd.AddCommand(creditsCmd.NewCmdRepoCredits(f, nil)) + cmd.AddCommand(gardenCmd.NewCmdGarden(f, nil)) return cmd } From 50211eade351a2bc5832a360b874294c8dbdb3be Mon Sep 17 00:00:00 2001 From: Nate Smith Date: Tue, 15 Sep 2020 12:00:07 -0500 Subject: [PATCH 38/67] add issue template for general feedback is this a good idea? --- .github/ISSUE_TEMPLATE/feedback.md | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 .github/ISSUE_TEMPLATE/feedback.md diff --git a/.github/ISSUE_TEMPLATE/feedback.md b/.github/ISSUE_TEMPLATE/feedback.md new file mode 100644 index 000000000..837c36632 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/feedback.md @@ -0,0 +1,28 @@ +--- +name: "\U0001F4E3 Feedback" +about: Give us general feedback about the GitHub CLI +title: '' +labels: feedback +assignees: '' + +--- + +# CLI Feedback + +You can use this template to give us structured feedback or just wipe it and leave us a note. Thank you! + +## What have you loved? + +_eg "the nice colors"_ + +## What was confusing or gave you pause? + +_eg "it did something unexpected"_ + +## Are there features you'd like to see added? + +_eg "gh cli needs mini-games"_ + +## Anything else? + +_eg "have a nice day"_ From 969321bff23b388571696ec3865e4a7b1da16fba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 15 Sep 2020 19:09:16 +0200 Subject: [PATCH 39/67] :fire: cleanup in `context` --- context/blank_context.go | 24 ------------------------ context/context.go | 30 ------------------------------ 2 files changed, 54 deletions(-) delete mode 100644 context/blank_context.go diff --git a/context/blank_context.go b/context/blank_context.go deleted file mode 100644 index 96f599981..000000000 --- a/context/blank_context.go +++ /dev/null @@ -1,24 +0,0 @@ -package context - -import ( - "fmt" - - "github.com/cli/cli/internal/config" -) - -// NewBlank initializes a blank Context suitable for testing -func NewBlank() *blankContext { - return &blankContext{} -} - -// A Context implementation that queries the filesystem -type blankContext struct { -} - -func (c *blankContext) Config() (config.Config, error) { - cfg, err := config.ParseConfig("config.yml") - if err != nil { - panic(fmt.Sprintf("failed to parse config during tests. did you remember to stub? error: %s", err)) - } - return cfg, nil -} diff --git a/context/context.go b/context/context.go index 9b2fc47c1..29ea74995 100644 --- a/context/context.go +++ b/context/context.go @@ -3,20 +3,13 @@ package context import ( "errors" "fmt" - "os" "sort" "strings" "github.com/cli/cli/api" - "github.com/cli/cli/internal/config" "github.com/cli/cli/internal/ghrepo" ) -// Context represents the interface for querying information about the current environment -type Context interface { - Config() (config.Config, error) -} - // cap the number of git remotes looked up, since the user might have an // unusually large number of git remotes const maxRemotesForLookup = 5 @@ -150,26 +143,3 @@ func (r ResolvedRemotes) RemoteForRepo(repo ghrepo.Interface) (*Remote, error) { } return nil, errors.New("not found") } - -// New initializes a Context that reads from the filesystem -func New() Context { - return &fsContext{} -} - -// A Context implementation that queries the filesystem -type fsContext struct { - config config.Config -} - -func (c *fsContext) Config() (config.Config, error) { - if c.config == nil { - cfg, err := config.ParseDefaultConfig() - if errors.Is(err, os.ErrNotExist) { - cfg = config.NewBlankConfig() - } else if err != nil { - return nil, err - } - c.config = cfg - } - return c.config, nil -} From 425f707c7d7d258ba99b1458b41fb4ec52c5df37 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Tue, 15 Sep 2020 13:36:32 -0500 Subject: [PATCH 40/67] fix tests --- pkg/cmd/gist/list/http.go | 3 --- pkg/cmd/gist/list/list_test.go | 12 ++++++++++-- pkg/httpmock/stub.go | 3 --- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/pkg/cmd/gist/list/http.go b/pkg/cmd/gist/list/http.go index 4e5445ad8..abc497423 100644 --- a/pkg/cmd/gist/list/http.go +++ b/pkg/cmd/gist/list/http.go @@ -26,9 +26,6 @@ func listGists(client *http.Client, hostname string, limit int, visibility strin return nil, err } - // TODO in tests the api call is matching properly and encoding json properly but i'm getting no - // result and no parse error, wtf? - gists := []shared.Gist{} for _, gist := range result { diff --git a/pkg/cmd/gist/list/list_test.go b/pkg/cmd/gist/list/list_test.go index dcae4db60..d6f707e71 100644 --- a/pkg/cmd/gist/list/list_test.go +++ b/pkg/cmd/gist/list/list_test.go @@ -88,14 +88,13 @@ func TestNewCmdList(t *testing.T) { } } -// TODO execution tests - func Test_listRun(t *testing.T) { tests := []struct { name string opts *ListOptions wantOut string stubs func(*httpmock.Registry) + nontty bool }{ { name: "no gists", @@ -116,6 +115,7 @@ func Test_listRun(t *testing.T) { // TODO public filter // TODO secret filter // TODO limit specified + // TODO nontty output } for _, tt := range tests { @@ -149,8 +149,16 @@ func Test_listRun(t *testing.T) { } io, _, stdout, _ := iostreams.Test() + io.SetStdoutTTY(!tt.nontty) tt.opts.IO = io + if tt.opts.Limit == 0 { + tt.opts.Limit = 10 + } + + if tt.opts.Visibility == "" { + tt.opts.Visibility = "all" + } t.Run(tt.name, func(t *testing.T) { err := listRun(tt.opts) assert.NoError(t, err) diff --git a/pkg/httpmock/stub.go b/pkg/httpmock/stub.go index e4572efe9..a1dcefaa3 100644 --- a/pkg/httpmock/stub.go +++ b/pkg/httpmock/stub.go @@ -3,7 +3,6 @@ package httpmock import ( "bytes" "encoding/json" - "fmt" "io" "io/ioutil" "net/http" @@ -87,8 +86,6 @@ func StatusStringResponse(status int, body string) Responder { func JSONResponse(body interface{}) Responder { return func(req *http.Request) (*http.Response, error) { b, _ := json.Marshal(body) - fmt.Printf("DEBUG %#v\n", "COOOOOL") - fmt.Printf("DEBUG %#v\n", string(b)) return httpResponse(200, req, bytes.NewBuffer(b)), nil } } From 9fd87faadc223e9a1ab50c6172eaf83401974eaf Mon Sep 17 00:00:00 2001 From: vilmibm Date: Tue, 15 Sep 2020 14:15:44 -0500 Subject: [PATCH 41/67] wip tests --- pkg/cmd/gist/list/http.go | 1 + pkg/cmd/gist/list/list_test.go | 62 +++++++++++++++++++++++++++++----- 2 files changed, 55 insertions(+), 8 deletions(-) diff --git a/pkg/cmd/gist/list/http.go b/pkg/cmd/gist/list/http.go index abc497423..aa75eef25 100644 --- a/pkg/cmd/gist/list/http.go +++ b/pkg/cmd/gist/list/http.go @@ -20,6 +20,7 @@ func listGists(client *http.Client, hostname string, limit int, visibility strin query.Add("per_page", "100") } + // TODO switch to graphql apiClient := api.NewClientFromHTTP(client) err := apiClient.REST(hostname, "GET", "gists?"+query.Encode(), nil, &result) if err != nil { diff --git a/pkg/cmd/gist/list/list_test.go b/pkg/cmd/gist/list/list_test.go index d6f707e71..d08973532 100644 --- a/pkg/cmd/gist/list/list_test.go +++ b/pkg/cmd/gist/list/list_test.go @@ -106,16 +106,32 @@ func Test_listRun(t *testing.T) { }, wantOut: "", }, - { name: "default behavior", opts: &ListOptions{}, - wantOut: "TODO", + wantOut: "1234567890 1 file public about 6 hours ago\n2345678901 tea leaves thwart... 2 files secret about 6 hours ago\n3456789012 short desc 11 files secret about 6 hours ago\n", + }, + { + name: "with public filter", + opts: &ListOptions{Visibility: "public"}, + wantOut: "1234567890 1 file public about 6 hours ago\n", + }, + { + name: "with secret filter", + opts: &ListOptions{Visibility: "secret"}, + wantOut: "2345678901 tea leaves thwart... 2 files secret about 6 hours ago\n3456789012 short desc 11 files secret about 6 hours ago\n", + }, + { + name: "with limit", + opts: &ListOptions{Limit: 1}, + wantOut: "1234567890 1 file public about 6 hours ago\n", + }, + { + name: "nontty output", + opts: &ListOptions{}, + wantOut: "", + nontty: true, }, - // TODO public filter - // TODO secret filter - // TODO limit specified - // TODO nontty output } for _, tt := range tests { @@ -131,8 +147,38 @@ func Test_listRun(t *testing.T) { Content: "lol", }, }, - Public: true, - UpdatedAt: time.Time{}, + Public: true, + }, + { + ID: "2345678901", + Description: "tea leaves thwart those who court catastrophe", + Files: map[string]*shared.GistFile{ + "gistfile0.txt": { + Content: "lolol", + }, + "gistfile1.txt": { + Content: "lololol", + }, + }, + Public: false, + }, + { + ID: "3456789012", + Description: "short desc", + Files: map[string]*shared.GistFile{ + "gistfile0.txt": {}, + "gistfile1.txt": {}, + "gistfile2.txt": {}, + "gistfile3.txt": {}, + "gistfile4.txt": {}, + "gistfile5.txt": {}, + "gistfile6.txt": {}, + "gistfile7.txt": {}, + "gistfile8.txt": {}, + "gistfile9.txt": {}, + "gistfile10.txt": {}, + }, + Public: false, }, })) } else { From d534a94d1bf4ca001ebff8088fddb3b9a2480d7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 15 Sep 2020 21:27:12 +0200 Subject: [PATCH 42/67] Change how base repository is resolved On first run in a git repository, `BaseRepo()` will now prompt the user which repository should be queried as base repository if there are multiple git remotes or when we are in the context of a fork. In non-interactive mode, the prompt is skipped and we default to the first git remote instead. After the base repo is resolved, the result is cached in the local repository using `git config` so that RepositoryNetwork API lookups can be avoided in the future. --- context/context.go | 163 ++++++++++++++++++++++-------------- context/remote_test.go | 163 ------------------------------------ git/remote.go | 32 ++++++- pkg/cmd/pr/create/create.go | 16 +++- pkg/cmd/root/root.go | 2 +- 5 files changed, 145 insertions(+), 231 deletions(-) diff --git a/context/context.go b/context/context.go index 29ea74995..746c866a2 100644 --- a/context/context.go +++ b/context/context.go @@ -1,26 +1,27 @@ +// TODO: rename this package to avoid clash with stdlib package context import ( "errors" - "fmt" "sort" - "strings" + "github.com/AlecAivazis/survey/v2" "github.com/cli/cli/api" + "github.com/cli/cli/git" "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/pkg/prompt" ) // cap the number of git remotes looked up, since the user might have an // unusually large number of git remotes const maxRemotesForLookup = 5 -// ResolveRemotesToRepos takes in a list of git remotes and fetches more information about the repositories they map to. -// Only the git remotes belonging to the same hostname are ever looked up; all others are ignored. -func ResolveRemotesToRepos(remotes Remotes, client *api.Client, base string) (ResolvedRemotes, error) { +func ResolveRemotesToRepos(remotes Remotes, client *api.Client, base string) (*ResolvedRemotes, error) { sort.Stable(remotes) - result := ResolvedRemotes{ - Remotes: remotes, + result := &ResolvedRemotes{ + remotes: remotes, apiClient: client, } @@ -31,84 +32,123 @@ func ResolveRemotesToRepos(remotes Remotes, client *api.Client, base string) (Re if err != nil { return result, err } - result.BaseOverride = baseOverride + result.baseOverride = baseOverride } - foundBaseOverride := false - var hostname string + return result, nil +} + +func resolveNetwork(result *ResolvedRemotes) error { var repos []ghrepo.Interface - for i, r := range remotes { - if i == 0 { - hostname = r.RepoHost() - } else if !strings.EqualFold(r.RepoHost(), hostname) { - // ignore all remotes for a hostname different to that of the 1st remote - continue - } + for _, r := range result.remotes { repos = append(repos, r) - if baseOverride != nil && ghrepo.IsSame(r, baseOverride) { - foundBaseOverride = true - } if len(repos) == maxRemotesForLookup { break } } - if baseOverride != nil && !foundBaseOverride { - // additionally, look up the explicitly specified base repo if it's not - // already covered by git remotes - repos = append(repos, baseOverride) - } - networkResult, err := api.RepoNetwork(client, repos) - if err != nil { - return result, err - } - result.Network = networkResult - return result, nil + networkResult, err := api.RepoNetwork(result.apiClient, repos) + result.network = &networkResult + return err } type ResolvedRemotes struct { - BaseOverride ghrepo.Interface - Remotes Remotes - Network api.RepoNetworkResult + baseOverride ghrepo.Interface + remotes Remotes + network *api.RepoNetworkResult apiClient *api.Client } -// BaseRepo is the first found repository in the "upstream", "github", "origin" -// git remote order, resolved to the parent repo if the git remote points to a fork -func (r ResolvedRemotes) BaseRepo() (*api.Repository, error) { - if r.BaseOverride != nil { - for _, repo := range r.Network.Repositories { - if repo != nil && ghrepo.IsSame(repo, r.BaseOverride) { - return repo, nil - } - } - return nil, fmt.Errorf("failed looking up information about the '%s' repository", - ghrepo.FullName(r.BaseOverride)) +func (r *ResolvedRemotes) BaseRepo(io *iostreams.IOStreams) (ghrepo.Interface, error) { + if r.baseOverride != nil { + return r.baseOverride, nil } - for _, repo := range r.Network.Repositories { + // if any of the remotes already has a resolution, respect that + for _, r := range r.remotes { + if r.Resolved == "base" { + return r, nil + } else if r.Resolved != "" { + repo, err := ghrepo.FromFullName(r.Resolved) + if err != nil { + return nil, err + } + return ghrepo.NewWithHost(repo.RepoOwner(), repo.RepoName(), r.RepoHost()), nil + } + } + + if !io.CanPrompt() { + // we cannot prompt, so just resort to the 1st remote + return r.remotes[0], nil + } + + // from here on, consult the API + if r.network == nil { + err := resolveNetwork(r) + if err != nil { + return nil, err + } + } + + var repoNames []string + repoMap := map[string]*api.Repository{} + add := func(r *api.Repository) { + fn := ghrepo.FullName(r) + if _, ok := repoMap[fn]; !ok { + repoMap[fn] = r + repoNames = append(repoNames, fn) + } + } + + for _, repo := range r.network.Repositories { if repo == nil { continue } if repo.IsFork() { - return repo.Parent, nil + add(repo.Parent) } - return repo, nil + add(repo) } - return nil, errors.New("not found") + if len(repoNames) == 0 { + return r.remotes[0], nil + } + + baseName := repoNames[0] + if len(repoNames) > 1 { + err := prompt.SurveyAskOne(&survey.Select{ + Message: "Which should be the base repository (used for e.g. querying issues) for this directory?", + Options: repoNames, + }, &baseName) + if err != nil { + return nil, err + } + } + + // determine corresponding git remote + selectedRepo := repoMap[baseName] + resolution := "base" + remote, _ := r.RemoteForRepo(selectedRepo) + if remote == nil { + remote = r.remotes[0] + resolution = ghrepo.FullName(selectedRepo) + } + + // cache the result to git config + err := git.SetRemoteResolution(remote.Name, resolution) + return selectedRepo, err } -// HeadRepo is a fork of base repo (if any), or the first found repository that -// has push access -func (r ResolvedRemotes) HeadRepo() (*api.Repository, error) { - baseRepo, err := r.BaseRepo() - if err != nil { - return nil, err +func (r *ResolvedRemotes) HeadRepo(baseRepo ghrepo.Interface) (ghrepo.Interface, error) { + if r.network == nil { + err := resolveNetwork(r) + if err != nil { + return nil, err + } } // try to find a pushable fork among existing remotes - for _, repo := range r.Network.Repositories { + for _, repo := range r.network.Repositories { if repo != nil && repo.Parent != nil && repo.ViewerCanPush() && ghrepo.IsSame(repo.Parent, baseRepo) { return repo, nil } @@ -123,7 +163,7 @@ func (r ResolvedRemotes) HeadRepo() (*api.Repository, error) { } // fall back to any listed repository that has push access - for _, repo := range r.Network.Repositories { + for _, repo := range r.network.Repositories { if repo != nil && repo.ViewerCanPush() { return repo, nil } @@ -132,12 +172,9 @@ func (r ResolvedRemotes) HeadRepo() (*api.Repository, error) { } // RemoteForRepo finds the git remote that points to a repository -func (r ResolvedRemotes) RemoteForRepo(repo ghrepo.Interface) (*Remote, error) { - for i, remote := range r.Remotes { - if ghrepo.IsSame(remote, repo) || - // additionally, look up the resolved repository name in case this - // git remote points to this repository via a redirect - (r.Network.Repositories[i] != nil && ghrepo.IsSame(r.Network.Repositories[i], repo)) { +func (r *ResolvedRemotes) RemoteForRepo(repo ghrepo.Interface) (*Remote, error) { + for _, remote := range r.remotes { + if ghrepo.IsSame(remote, repo) { return remote, nil } } diff --git a/context/remote_test.go b/context/remote_test.go index 98326b3aa..ab3f7e2e2 100644 --- a/context/remote_test.go +++ b/context/remote_test.go @@ -1,16 +1,13 @@ package context import ( - "bytes" "errors" "net/url" "reflect" "testing" - "github.com/cli/cli/api" "github.com/cli/cli/git" "github.com/cli/cli/internal/ghrepo" - "github.com/cli/cli/pkg/httpmock" ) func eq(t *testing.T, got interface{}, expected interface{}) { @@ -69,163 +66,3 @@ func Test_translateRemotes(t *testing.T) { t.Errorf("got %q", result[0].RepoName()) } } - -func Test_resolvedRemotes_triangularSetup(t *testing.T) { - http := &httpmock.Registry{} - apiClient := api.NewClient(api.ReplaceTripper(http)) - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "forks": { "nodes": [ - ] } } } } - `)) - - resolved := ResolvedRemotes{ - BaseOverride: nil, - Remotes: Remotes{ - &Remote{ - Remote: &git.Remote{Name: "origin"}, - Repo: ghrepo.New("OWNER", "REPO"), - }, - &Remote{ - Remote: &git.Remote{Name: "fork"}, - Repo: ghrepo.New("MYSELF", "REPO"), - }, - }, - Network: api.RepoNetworkResult{ - Repositories: []*api.Repository{ - { - Name: "NEWNAME", - Owner: api.RepositoryOwner{Login: "NEWOWNER"}, - ViewerPermission: "READ", - }, - { - Name: "REPO", - Owner: api.RepositoryOwner{Login: "MYSELF"}, - ViewerPermission: "ADMIN", - }, - }, - }, - apiClient: apiClient, - } - - baseRepo, err := resolved.BaseRepo() - if err != nil { - t.Fatalf("got %v", err) - } - eq(t, ghrepo.FullName(baseRepo), "NEWOWNER/NEWNAME") - baseRemote, err := resolved.RemoteForRepo(baseRepo) - if err != nil { - t.Fatalf("got %v", err) - } - if baseRemote.Name != "origin" { - t.Errorf("got remote %q", baseRemote.Name) - } - - headRepo, err := resolved.HeadRepo() - if err != nil { - t.Fatalf("got %v", err) - } - eq(t, ghrepo.FullName(headRepo), "MYSELF/REPO") - headRemote, err := resolved.RemoteForRepo(headRepo) - if err != nil { - t.Fatalf("got %v", err) - } - if headRemote.Name != "fork" { - t.Errorf("got remote %q", headRemote.Name) - } -} - -func Test_resolvedRemotes_forkLookup(t *testing.T) { - http := &httpmock.Registry{} - apiClient := api.NewClient(api.ReplaceTripper(http)) - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "forks": { "nodes": [ - { "id": "FORKID", - "url": "https://github.com/FORKOWNER/REPO", - "name": "REPO", - "owner": { "login": "FORKOWNER" }, - "viewerPermission": "WRITE" - } - ] } } } } - `)) - - resolved := ResolvedRemotes{ - BaseOverride: nil, - Remotes: Remotes{ - &Remote{ - Remote: &git.Remote{Name: "origin"}, - Repo: ghrepo.New("OWNER", "REPO"), - }, - }, - Network: api.RepoNetworkResult{ - Repositories: []*api.Repository{ - { - Name: "NEWNAME", - Owner: api.RepositoryOwner{Login: "NEWOWNER"}, - ViewerPermission: "READ", - }, - }, - }, - apiClient: apiClient, - } - - headRepo, err := resolved.HeadRepo() - if err != nil { - t.Fatalf("got %v", err) - } - eq(t, ghrepo.FullName(headRepo), "FORKOWNER/REPO") - _, err = resolved.RemoteForRepo(headRepo) - if err == nil { - t.Fatal("expected to not find a matching remote") - } -} - -func Test_resolvedRemotes_clonedFork(t *testing.T) { - resolved := ResolvedRemotes{ - BaseOverride: nil, - Remotes: Remotes{ - &Remote{ - Remote: &git.Remote{Name: "origin"}, - Repo: ghrepo.New("OWNER", "REPO"), - }, - }, - Network: api.RepoNetworkResult{ - Repositories: []*api.Repository{ - { - Name: "REPO", - Owner: api.RepositoryOwner{Login: "OWNER"}, - ViewerPermission: "ADMIN", - Parent: &api.Repository{ - Name: "REPO", - Owner: api.RepositoryOwner{Login: "PARENTOWNER"}, - ViewerPermission: "READ", - }, - }, - }, - }, - } - - baseRepo, err := resolved.BaseRepo() - if err != nil { - t.Fatalf("got %v", err) - } - eq(t, ghrepo.FullName(baseRepo), "PARENTOWNER/REPO") - baseRemote, err := resolved.RemoteForRepo(baseRepo) - if baseRemote != nil || err == nil { - t.Error("did not expect any remote for base") - } - - headRepo, err := resolved.HeadRepo() - if err != nil { - t.Fatalf("got %v", err) - } - eq(t, ghrepo.FullName(headRepo), "OWNER/REPO") - headRemote, err := resolved.RemoteForRepo(headRepo) - if err != nil { - t.Fatalf("got %v", err) - } - if headRemote.Name != "origin" { - t.Errorf("got remote %q", headRemote.Name) - } -} diff --git a/git/remote.go b/git/remote.go index e808c1492..77e550c37 100644 --- a/git/remote.go +++ b/git/remote.go @@ -1,6 +1,7 @@ package git import ( + "fmt" "net/url" "os/exec" "regexp" @@ -26,6 +27,7 @@ func NewRemote(name string, u string) *Remote { // Remote is a parsed git remote type Remote struct { Name string + Resolved string FetchURL *url.URL PushURL *url.URL } @@ -40,7 +42,30 @@ func Remotes() (RemoteSet, error) { if err != nil { return nil, err } - return parseRemotes(list), nil + remotes := parseRemotes(list) + + // this is affected by SetRemoteResolution + remoteCmd := exec.Command("git", "config", "--get-regexp", `^remote\..*\.gh-resolved$`) + output, _ := run.PrepareCmd(remoteCmd).Output() + for _, l := range outputLines(output) { + parts := strings.SplitN(l, " ", 2) + if len(parts) < 2 { + continue + } + rp := strings.SplitN(parts[0], ".", 3) + if len(rp) < 2 { + continue + } + name := rp[1] + for _, r := range remotes { + if r.Name == name { + r.Resolved = parts[1] + break + } + } + } + + return remotes, nil } func parseRemotes(gitRemotes []string) (remotes RemoteSet) { @@ -109,3 +134,8 @@ func AddRemote(name, u string) (*Remote, error) { PushURL: urlParsed, }, nil } + +func SetRemoteResolution(name, resolution string) error { + addCmd := exec.Command("git", "config", "--add", fmt.Sprintf("remote.%s.gh-resolved", name), resolution) + return run.PrepareCmd(addCmd).Run() +} diff --git a/pkg/cmd/pr/create/create.go b/pkg/cmd/pr/create/create.go index d14be560c..655fca9ba 100644 --- a/pkg/cmd/pr/create/create.go +++ b/pkg/cmd/pr/create/create.go @@ -127,8 +127,18 @@ func createRun(opts *CreateOptions) error { return err } - baseRepo, err := repoContext.BaseRepo() - if err != nil { + var baseRepo *api.Repository + if br, err := repoContext.BaseRepo(opts.IO); err == nil { + if r, ok := br.(*api.Repository); ok { + baseRepo = r + } else { + var err error + baseRepo, err = api.GitHubRepo(client, br) + if err != nil { + return err + } + } + } else { return fmt.Errorf("could not determine base repository: %w", err) } @@ -155,7 +165,7 @@ func createRun(opts *CreateOptions) error { // otherwise, determine the head repository with info obtained from the API if headRepo == nil { - if r, err := repoContext.HeadRepo(); err == nil { + if r, err := repoContext.HeadRepo(baseRepo); err == nil { headRepo = r } } diff --git a/pkg/cmd/root/root.go b/pkg/cmd/root/root.go index e3257ed09..334ee418d 100644 --- a/pkg/cmd/root/root.go +++ b/pkg/cmd/root/root.go @@ -134,7 +134,7 @@ func resolvedBaseRepo(f *cmdutil.Factory) func() (ghrepo.Interface, error) { if err != nil { return nil, err } - baseRepo, err := repoContext.BaseRepo() + baseRepo, err := repoContext.BaseRepo(f.IOStreams) if err != nil { return nil, err } From ba5b639be437ecd9bead6fb4d8763d0e9c37caf7 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Tue, 15 Sep 2020 14:33:14 -0500 Subject: [PATCH 43/67] finish list tests --- pkg/cmd/gist/list/list.go | 8 ++++++-- pkg/cmd/gist/list/list_test.go | 3 +-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/pkg/cmd/gist/list/list.go b/pkg/cmd/gist/list/list.go index ec9e45c0d..03882152d 100644 --- a/pkg/cmd/gist/list/list.go +++ b/pkg/cmd/gist/list/list.go @@ -93,12 +93,16 @@ func listRun(opts *ListOptions) error { visColor = cs.Red } - updatedAt := utils.FuzzyAgo(opts.Since(gist.UpdatedAt)) tp.AddField(gist.ID, nil, nil) tp.AddField(gist.Description, nil, cs.Bold) tp.AddField(utils.Pluralize(fileCount, "file"), nil, nil) tp.AddField(visibility, nil, visColor) - tp.AddField(updatedAt, nil, utils.Gray) + if tp.IsTTY() { + updatedAt := utils.FuzzyAgo(opts.Since(gist.UpdatedAt)) + tp.AddField(updatedAt, nil, cs.Gray) + } else { + tp.AddField(gist.UpdatedAt.String(), nil, nil) + } tp.EndRow() } diff --git a/pkg/cmd/gist/list/list_test.go b/pkg/cmd/gist/list/list_test.go index d08973532..2507f3e6b 100644 --- a/pkg/cmd/gist/list/list_test.go +++ b/pkg/cmd/gist/list/list_test.go @@ -22,7 +22,6 @@ func TestNewCmdList(t *testing.T) { }{ { name: "no arguments", - cli: "", wants: ListOptions{ Limit: 10, Visibility: "all", @@ -129,7 +128,7 @@ func Test_listRun(t *testing.T) { { name: "nontty output", opts: &ListOptions{}, - wantOut: "", + wantOut: "1234567890\t\t1 file\tpublic\t0001-01-01 00:00:00 +0000 UTC\n2345678901\ttea leaves thwart those who court catastrophe\t2 files\tsecret\t0001-01-01 00:00:00 +0000 UTC\n3456789012\tshort desc\t11 files\tsecret\t0001-01-01 00:00:00 +0000 UTC\n", nontty: true, }, } From a61c897e4ce2777ee3eb866891554f6513808aa7 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Tue, 15 Sep 2020 14:55:55 -0500 Subject: [PATCH 44/67] show filename if no description --- pkg/cmd/gist/list/list.go | 13 ++++++++++++- pkg/cmd/gist/list/list_test.go | 28 +++++++++++++++------------- 2 files changed, 27 insertions(+), 14 deletions(-) diff --git a/pkg/cmd/gist/list/list.go b/pkg/cmd/gist/list/list.go index 03882152d..1040d5f87 100644 --- a/pkg/cmd/gist/list/list.go +++ b/pkg/cmd/gist/list/list.go @@ -3,6 +3,7 @@ package list import ( "fmt" "net/http" + "strings" "time" "github.com/cli/cli/internal/ghinstance" @@ -93,8 +94,18 @@ func listRun(opts *ListOptions) error { visColor = cs.Red } + description := gist.Description + if description == "" { + for filename, _ := range gist.Files { + if !strings.HasPrefix(filename, "gistfile") { + description = filename + break + } + } + } + tp.AddField(gist.ID, nil, nil) - tp.AddField(gist.Description, nil, cs.Bold) + tp.AddField(description, nil, cs.Bold) tp.AddField(utils.Pluralize(fileCount, "file"), nil, nil) tp.AddField(visibility, nil, visColor) if tp.IsTTY() { diff --git a/pkg/cmd/gist/list/list_test.go b/pkg/cmd/gist/list/list_test.go index 2507f3e6b..26fa46114 100644 --- a/pkg/cmd/gist/list/list_test.go +++ b/pkg/cmd/gist/list/list_test.go @@ -108,12 +108,12 @@ func Test_listRun(t *testing.T) { { name: "default behavior", opts: &ListOptions{}, - wantOut: "1234567890 1 file public about 6 hours ago\n2345678901 tea leaves thwart... 2 files secret about 6 hours ago\n3456789012 short desc 11 files secret about 6 hours ago\n", + wantOut: "1234567890 cool.txt 1 file public about 6 hours ago\n4567890123 1 file public about 6 hours ago\n2345678901 tea leaves thwart... 2 files secret about 6 hours ago\n3456789012 short desc 11 files secret about 6 hours ago\n", }, { name: "with public filter", opts: &ListOptions{Visibility: "public"}, - wantOut: "1234567890 1 file public about 6 hours ago\n", + wantOut: "1234567890 cool.txt 1 file public about 6 hours ago\n4567890123 1 file public about 6 hours ago\n", }, { name: "with secret filter", @@ -123,12 +123,12 @@ func Test_listRun(t *testing.T) { { name: "with limit", opts: &ListOptions{Limit: 1}, - wantOut: "1234567890 1 file public about 6 hours ago\n", + wantOut: "1234567890 cool.txt 1 file public about 6 hours ago\n", }, { name: "nontty output", opts: &ListOptions{}, - wantOut: "1234567890\t\t1 file\tpublic\t0001-01-01 00:00:00 +0000 UTC\n2345678901\ttea leaves thwart those who court catastrophe\t2 files\tsecret\t0001-01-01 00:00:00 +0000 UTC\n3456789012\tshort desc\t11 files\tsecret\t0001-01-01 00:00:00 +0000 UTC\n", + wantOut: "1234567890\tcool.txt\t1 file\tpublic\t0001-01-01 00:00:00 +0000 UTC\n4567890123\t\t1 file\tpublic\t0001-01-01 00:00:00 +0000 UTC\n2345678901\ttea leaves thwart those who court catastrophe\t2 files\tsecret\t0001-01-01 00:00:00 +0000 UTC\n3456789012\tshort desc\t11 files\tsecret\t0001-01-01 00:00:00 +0000 UTC\n", nontty: true, }, } @@ -142,9 +142,15 @@ func Test_listRun(t *testing.T) { ID: "1234567890", Description: "", Files: map[string]*shared.GistFile{ - "cool.txt": { - Content: "lol", - }, + "cool.txt": {}, + }, + Public: true, + }, + { + ID: "4567890123", + Description: "", + Files: map[string]*shared.GistFile{ + "gistfile0.txt": {}, }, Public: true, }, @@ -152,12 +158,8 @@ func Test_listRun(t *testing.T) { ID: "2345678901", Description: "tea leaves thwart those who court catastrophe", Files: map[string]*shared.GistFile{ - "gistfile0.txt": { - Content: "lolol", - }, - "gistfile1.txt": { - Content: "lololol", - }, + "gistfile0.txt": {}, + "gistfile1.txt": {}, }, Public: false, }, From f124b426fe36a7ab11b7da0596c0e9d055ed77c9 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Tue, 15 Sep 2020 14:56:13 -0500 Subject: [PATCH 45/67] tweak gist view; support --filename --- pkg/cmd/gist/view/view.go | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/pkg/cmd/gist/view/view.go b/pkg/cmd/gist/view/view.go index 5e4da1e69..cbe735675 100644 --- a/pkg/cmd/gist/view/view.go +++ b/pkg/cmd/gist/view/view.go @@ -19,6 +19,7 @@ type ViewOptions struct { HttpClient func() (*http.Client, error) Selector string + Filename string Raw bool Web bool } @@ -49,6 +50,7 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman cmd.Flags().BoolVarP(&opts.Raw, "raw", "r", false, "do not try and render markdown") cmd.Flags().BoolVarP(&opts.Web, "web", "w", false, "open gist in browser") + cmd.Flags().StringVarP(&opts.Filename, "filename", "f", "", "display a single file of the gist") return cmd } @@ -87,12 +89,27 @@ func viewRun(opts *ViewOptions) error { cs := opts.IO.ColorScheme() if gist.Description != "" { - fmt.Fprintf(opts.IO.ErrOut, "%s\n", cs.Bold(gist.Description)) + fmt.Fprintf(opts.IO.Out, "%s\n", cs.Bold(gist.Description)) } + if opts.Filename != "" { + gistFile, ok := gist.Files[opts.Filename] + if !ok { + return fmt.Errorf("gist has no such file %q", opts.Filename) + } + + gist.Files = map[string]*shared.GistFile{ + opts.Filename: gistFile, + } + } + + showFilenames := len(gist.Files) > 1 + for filename, gistFile := range gist.Files { - fmt.Fprintf(opts.IO.ErrOut, "%s\n", cs.Gray(filename)) - fmt.Fprintln(opts.IO.ErrOut) + if showFilenames { + fmt.Fprintf(opts.IO.Out, "%s\n", cs.Gray(filename)) + fmt.Fprintln(opts.IO.Out) + } content := gistFile.Content if strings.Contains(gistFile.Type, "markdown") && !opts.Raw { rendered, err := utils.RenderMarkdown(gistFile.Content) @@ -104,6 +121,5 @@ func viewRun(opts *ViewOptions) error { fmt.Fprintln(opts.IO.Out) } - // TODO print gist files, possibly with rendered markdown return nil } From 4a467864d544fc5aec06acdbb641ee58fa0c8c37 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Tue, 15 Sep 2020 15:22:28 -0500 Subject: [PATCH 46/67] linter appeasement --- pkg/cmd/gist/list/list.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/gist/list/list.go b/pkg/cmd/gist/list/list.go index 1040d5f87..5e22e267b 100644 --- a/pkg/cmd/gist/list/list.go +++ b/pkg/cmd/gist/list/list.go @@ -96,7 +96,7 @@ func listRun(opts *ListOptions) error { description := gist.Description if description == "" { - for filename, _ := range gist.Files { + for filename := range gist.Files { if !strings.HasPrefix(filename, "gistfile") { description = filename break From ada2c566067b64db942c1de32578e74c6ea3c005 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Tue, 15 Sep 2020 16:05:30 -0500 Subject: [PATCH 47/67] test gist view --- pkg/cmd/gist/view/view.go | 18 +++- pkg/cmd/gist/view/view_test.go | 149 ++++++++++++++++++++++++++++++++- 2 files changed, 162 insertions(+), 5 deletions(-) diff --git a/pkg/cmd/gist/view/view.go b/pkg/cmd/gist/view/view.go index cbe735675..fecb69adb 100644 --- a/pkg/cmd/gist/view/view.go +++ b/pkg/cmd/gist/view/view.go @@ -4,6 +4,7 @@ import ( "fmt" "net/http" "net/url" + "sort" "strings" "github.com/cli/cli/internal/ghinstance" @@ -105,10 +106,12 @@ func viewRun(opts *ViewOptions) error { showFilenames := len(gist.Files) > 1 + outs := []string{} // to ensure consistent ordering + for filename, gistFile := range gist.Files { + out := "" if showFilenames { - fmt.Fprintf(opts.IO.Out, "%s\n", cs.Gray(filename)) - fmt.Fprintln(opts.IO.Out) + out += fmt.Sprintf("%s\n\n", cs.Gray(filename)) } content := gistFile.Content if strings.Contains(gistFile.Type, "markdown") && !opts.Raw { @@ -117,8 +120,15 @@ func viewRun(opts *ViewOptions) error { content = rendered } } - fmt.Fprintf(opts.IO.Out, "%s\n", content) - fmt.Fprintln(opts.IO.Out) + out += fmt.Sprintf("%s\n\n", content) + + outs = append(outs, out) + } + + sort.Strings(outs) + + for _, out := range outs { + fmt.Fprint(opts.IO.Out, out) } return nil diff --git a/pkg/cmd/gist/view/view_test.go b/pkg/cmd/gist/view/view_test.go index d77ab946d..0ddc33181 100644 --- a/pkg/cmd/gist/view/view_test.go +++ b/pkg/cmd/gist/view/view_test.go @@ -2,9 +2,12 @@ package view import ( "bytes" + "net/http" "testing" + "github.com/cli/cli/pkg/cmd/gist/shared" "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/httpmock" "github.com/cli/cli/pkg/iostreams" "github.com/google/shlex" "github.com/stretchr/testify/assert" @@ -34,6 +37,16 @@ func TestNewCmdView(t *testing.T) { Selector: "123", }, }, + { + name: "filename passed", + cli: "-fcool.txt 123", + tty: true, + wants: ViewOptions{ + Raw: false, + Selector: "123", + Filename: "cool.txt", + }, + }, } for _, tt := range tests { @@ -63,8 +76,142 @@ func TestNewCmdView(t *testing.T) { assert.Equal(t, tt.wants.Raw, gotOpts.Raw) assert.Equal(t, tt.wants.Selector, gotOpts.Selector) + assert.Equal(t, tt.wants.Filename, gotOpts.Filename) }) } } -// TODO execution tests +func Test_viewRun(t *testing.T) { + tests := []struct { + name string + opts *ViewOptions + wantOut string + gist *shared.Gist + wantErr bool + }{ + { + name: "no such gist", + wantErr: true, + }, + { + name: "one file", + gist: &shared.Gist{ + Files: map[string]*shared.GistFile{ + "cicada.txt": { + Content: "bwhiizzzbwhuiiizzzz", + Type: "text/plain", + }, + }, + }, + wantOut: "bwhiizzzbwhuiiizzzz\n\n", + }, + { + name: "filename selected", + opts: &ViewOptions{ + Filename: "cicada.txt", + }, + gist: &shared.Gist{ + Files: map[string]*shared.GistFile{ + "cicada.txt": { + Content: "bwhiizzzbwhuiiizzzz", + Type: "text/plain", + }, + "foo.md": { + Content: "# foo", + Type: "application/markdown", + }, + }, + }, + wantOut: "bwhiizzzbwhuiiizzzz\n\n", + }, + { + name: "multiple files, no description", + gist: &shared.Gist{ + Files: map[string]*shared.GistFile{ + "cicada.txt": { + Content: "bwhiizzzbwhuiiizzzz", + Type: "text/plain", + }, + "foo.md": { + Content: "# foo", + Type: "application/markdown", + }, + }, + }, + wantOut: "cicada.txt\n\nbwhiizzzbwhuiiizzzz\n\nfoo.md\n\n\n # foo \n\n\n\n", + }, + { + name: "multiple files, description", + gist: &shared.Gist{ + Description: "some files", + Files: map[string]*shared.GistFile{ + "cicada.txt": { + Content: "bwhiizzzbwhuiiizzzz", + Type: "text/plain", + }, + "foo.md": { + Content: "- foo", + Type: "application/markdown", + }, + }, + }, + wantOut: "some files\ncicada.txt\n\nbwhiizzzbwhuiiizzzz\n\nfoo.md\n\n\n \n • foo \n\n\n\n", + }, + { + name: "raw", + opts: &ViewOptions{ + Raw: true, + }, + gist: &shared.Gist{ + Description: "some files", + Files: map[string]*shared.GistFile{ + "cicada.txt": { + Content: "bwhiizzzbwhuiiizzzz", + Type: "text/plain", + }, + "foo.md": { + Content: "- foo", + Type: "application/markdown", + }, + }, + }, + wantOut: "some files\ncicada.txt\n\nbwhiizzzbwhuiiizzzz\n\nfoo.md\n\n- foo\n\n", + }, + } + + for _, tt := range tests { + reg := &httpmock.Registry{} + if tt.gist == nil { + reg.Register(httpmock.REST("GET", "gists/1234"), + httpmock.StatusStringResponse(404, "Not Found")) + } else { + reg.Register(httpmock.REST("GET", "gists/1234"), + httpmock.JSONResponse(tt.gist)) + } + + if tt.opts == nil { + tt.opts = &ViewOptions{} + } + + tt.opts.HttpClient = func() (*http.Client, error) { + return &http.Client{Transport: reg}, nil + } + io, _, stdout, _ := iostreams.Test() + io.SetStdoutTTY(true) + tt.opts.IO = io + + tt.opts.Selector = "1234" + + t.Run(tt.name, func(t *testing.T) { + err := viewRun(tt.opts) + if tt.wantErr { + assert.Error(t, err) + return + } + assert.NoError(t, err) + + assert.Equal(t, tt.wantOut, stdout.String()) + reg.Verify(t) + }) + } +} From 62f54f0f029b546c34bc50de1e866966c183079c Mon Sep 17 00:00:00 2001 From: vilmibm Date: Tue, 15 Sep 2020 16:18:26 -0500 Subject: [PATCH 48/67] start on edit tests --- pkg/cmd/gist/edit/edit_test.go | 65 ++++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) create mode 100644 pkg/cmd/gist/edit/edit_test.go diff --git a/pkg/cmd/gist/edit/edit_test.go b/pkg/cmd/gist/edit/edit_test.go new file mode 100644 index 000000000..eab8c214e --- /dev/null +++ b/pkg/cmd/gist/edit/edit_test.go @@ -0,0 +1,65 @@ +package edit + +import ( + "bytes" + "testing" + + "github.com/cli/cli/pkg/cmdutil" + "github.com/google/shlex" + "github.com/stretchr/testify/assert" +) + +func TestNewCmdEdit(t *testing.T) { + tests := []struct { + name string + cli string + wants EditOptions + }{ + { + name: "no flags", + cli: "123", + wants: EditOptions{ + Selector: "123", + }, + }, + { + name: "filename", + cli: "123 --filename cool.md", + wants: EditOptions{ + Selector: "123", + Filename: "cool.md", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + f := &cmdutil.Factory{} + + argv, err := shlex.Split(tt.cli) + assert.NoError(t, err) + + var gotOpts *EditOptions + cmd := NewCmdEdit(f, func(opts *EditOptions) error { + gotOpts = opts + return nil + }) + cmd.SetArgs(argv) + cmd.SetIn(&bytes.Buffer{}) + cmd.SetOut(&bytes.Buffer{}) + cmd.SetErr(&bytes.Buffer{}) + + _, err = cmd.ExecuteC() + assert.NoError(t, err) + + assert.Equal(t, tt.wants.Filename, gotOpts.Filename) + assert.Equal(t, tt.wants.Selector, gotOpts.Selector) + }) + } +} + +// TODO execution tests +// TODO no such gist +// TODO one files +// TODO multiple files, submit +// TODO multiple files, cancel From a9ab2a98fca42ee99fa74a44e4e7b8a2c47608c2 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Tue, 15 Sep 2020 17:04:43 -0500 Subject: [PATCH 49/67] move Edit to opts for testing --- pkg/cmd/gist/edit/edit.go | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/pkg/cmd/gist/edit/edit.go b/pkg/cmd/gist/edit/edit.go index 3caf69b34..554e9363d 100644 --- a/pkg/cmd/gist/edit/edit.go +++ b/pkg/cmd/gist/edit/edit.go @@ -7,7 +7,6 @@ import ( "fmt" "net/http" "net/url" - "os" "sort" "strings" @@ -28,6 +27,8 @@ type EditOptions struct { HttpClient func() (*http.Client, error) Config func() (config.Config, error) + Edit func(string, string, string, *iostreams.IOStreams) (string, error) + Selector string Filename string } @@ -37,6 +38,13 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman IO: f.IOStreams, HttpClient: f.HttpClient, Config: f.Config, + Edit: func(editorCmd, filename, defaultContent string, io *iostreams.IOStreams) (string, error) { + return surveyext.Edit( + editorCmd, + "*."+filename, + defaultContent, + io.In, io.Out, io.ErrOut, nil) + }, } cmd := &cobra.Command{ @@ -115,12 +123,7 @@ func editRun(opts *EditOptions) error { if err != nil { return err } - text, err := surveyext.Edit( - editorCommand, - "*."+filename, - gist.Files[filename].Content, - // TODO: consider using iostreams here - os.Stdin, os.Stdout, os.Stderr, nil) + text, err := opts.Edit(editorCommand, filename, gist.Files[filename].Content, opts.IO) if err != nil { return err From 15cf786c5a253601ace15885b8380e0273931a0e Mon Sep 17 00:00:00 2001 From: vilmibm Date: Tue, 15 Sep 2020 17:20:36 -0500 Subject: [PATCH 50/67] wip tests --- pkg/cmd/gist/edit/edit_test.go | 126 +++++++++++++++++++++++++++++++-- 1 file changed, 121 insertions(+), 5 deletions(-) diff --git a/pkg/cmd/gist/edit/edit_test.go b/pkg/cmd/gist/edit/edit_test.go index eab8c214e..8b39e95cd 100644 --- a/pkg/cmd/gist/edit/edit_test.go +++ b/pkg/cmd/gist/edit/edit_test.go @@ -2,9 +2,17 @@ package edit import ( "bytes" + "encoding/json" + "io/ioutil" + "net/http" "testing" + "github.com/cli/cli/internal/config" + "github.com/cli/cli/pkg/cmd/gist/shared" "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/httpmock" + "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/pkg/prompt" "github.com/google/shlex" "github.com/stretchr/testify/assert" ) @@ -58,8 +66,116 @@ func TestNewCmdEdit(t *testing.T) { } } -// TODO execution tests -// TODO no such gist -// TODO one files -// TODO multiple files, submit -// TODO multiple files, cancel +func Test_editRun(t *testing.T) { + tests := []struct { + name string + opts *EditOptions + gist *shared.Gist + httpStubs func(*httpmock.Registry) + askStubs func(*prompt.AskStubber) + nontty bool + wantErr bool + wantParams map[string]interface{} + }{ + { + name: "no such gist", + wantErr: true, + }, + { + name: "one file", + gist: &shared.Gist{ + ID: "1234", + Files: map[string]*shared.GistFile{ + "cicada.txt": { + Filename: "cicada.txt", + Content: "bwhiizzzbwhuiiizzzz", + Type: "text/plain", + }, + }, + }, + //askStubs: func(as *prompt.AskStubber) { + // as.StubOne("new file content") + //}, + httpStubs: func(reg *httpmock.Registry) { + reg.Register(httpmock.REST("POST", "gists/1234"), + httpmock.StatusStringResponse(201, "{}")) + }, + wantParams: map[string]interface{}{ + "description": "", + "updated_at": "0001-01-01T00:00:00Z", + "public": false, + "files": map[string]interface{}{ + "cicada.txt": map[string]interface{}{ + "content": "new file content", + "filename": "cicada.txt", + "type": "text/plain", + }, + }, + }, + }, + // TODO multiple files, submit + // TODO multiple files, cancel + } + + for _, tt := range tests { + reg := &httpmock.Registry{} + if tt.gist == nil { + reg.Register(httpmock.REST("GET", "gists/1234"), + httpmock.StatusStringResponse(404, "Not Found")) + } else { + reg.Register(httpmock.REST("GET", "gists/1234"), + httpmock.JSONResponse(tt.gist)) + } + + if tt.httpStubs != nil { + tt.httpStubs(reg) + } + + as, teardown := prompt.InitAskStubber() + defer teardown() + if tt.askStubs != nil { + tt.askStubs(as) + } + + if tt.opts == nil { + tt.opts = &EditOptions{} + } + + tt.opts.Edit = func(_, _, _ string, _ *iostreams.IOStreams) (string, error) { + return "new file content", nil + } + + tt.opts.HttpClient = func() (*http.Client, error) { + return &http.Client{Transport: reg}, nil + } + io, _, _, _ := iostreams.Test() + io.SetStdoutTTY(!tt.nontty) + io.SetStdinTTY(!tt.nontty) + tt.opts.IO = io + + tt.opts.Selector = "1234" + + tt.opts.Config = func() (config.Config, error) { + return config.NewBlankConfig(), nil + } + + t.Run(tt.name, func(t *testing.T) { + err := editRun(tt.opts) + if tt.wantErr { + assert.Error(t, err) + return + } + assert.NoError(t, err) + + reg.Verify(t) + + bodyBytes, _ := ioutil.ReadAll(reg.Requests[1].Body) + reqBody := make(map[string]interface{}) + err = json.Unmarshal(bodyBytes, &reqBody) + if err != nil { + t.Fatalf("error decoding JSON: %v", err) + } + assert.Equal(t, tt.wantParams, reqBody) + }) + } +} From 0d45dd82f347e30f560ddadfd49f3f14fc3ef464 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Tue, 15 Sep 2020 17:29:54 -0500 Subject: [PATCH 51/67] finish edit tests --- pkg/cmd/gist/edit/edit_test.go | 89 +++++++++++++++++++++++++++++----- 1 file changed, 76 insertions(+), 13 deletions(-) diff --git a/pkg/cmd/gist/edit/edit_test.go b/pkg/cmd/gist/edit/edit_test.go index 8b39e95cd..d7fffd958 100644 --- a/pkg/cmd/gist/edit/edit_test.go +++ b/pkg/cmd/gist/edit/edit_test.go @@ -93,9 +93,6 @@ func Test_editRun(t *testing.T) { }, }, }, - //askStubs: func(as *prompt.AskStubber) { - // as.StubOne("new file content") - //}, httpStubs: func(reg *httpmock.Registry) { reg.Register(httpmock.REST("POST", "gists/1234"), httpmock.StatusStringResponse(201, "{}")) @@ -113,8 +110,73 @@ func Test_editRun(t *testing.T) { }, }, }, - // TODO multiple files, submit - // TODO multiple files, cancel + { + name: "multiple files, submit", + askStubs: func(as *prompt.AskStubber) { + as.StubOne("unix.md") + as.StubOne("Submit") + }, + gist: &shared.Gist{ + ID: "1234", + Description: "catbug", + Files: map[string]*shared.GistFile{ + "cicada.txt": { + Filename: "cicada.txt", + Content: "bwhiizzzbwhuiiizzzz", + Type: "text/plain", + }, + "unix.md": { + Filename: "unix.md", + Content: "meow", + Type: "application/markdown", + }, + }, + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register(httpmock.REST("POST", "gists/1234"), + httpmock.StatusStringResponse(201, "{}")) + }, + wantParams: map[string]interface{}{ + "description": "catbug", + "updated_at": "0001-01-01T00:00:00Z", + "public": false, + "files": map[string]interface{}{ + "cicada.txt": map[string]interface{}{ + "content": "bwhiizzzbwhuiiizzzz", + "filename": "cicada.txt", + "type": "text/plain", + }, + "unix.md": map[string]interface{}{ + "content": "new file content", + "filename": "unix.md", + "type": "application/markdown", + }, + }, + }, + }, + { + name: "multiple files, cancel", + askStubs: func(as *prompt.AskStubber) { + as.StubOne("unix.md") + as.StubOne("Cancel") + }, + wantErr: true, + gist: &shared.Gist{ + ID: "1234", + Files: map[string]*shared.GistFile{ + "cicada.txt": { + Filename: "cicada.txt", + Content: "bwhiizzzbwhuiiizzzz", + Type: "text/plain", + }, + "unix.md": { + Filename: "unix.md", + Content: "meow", + Type: "application/markdown", + }, + }, + }, + }, } for _, tt := range tests { @@ -161,21 +223,22 @@ func Test_editRun(t *testing.T) { t.Run(tt.name, func(t *testing.T) { err := editRun(tt.opts) + reg.Verify(t) if tt.wantErr { assert.Error(t, err) return } assert.NoError(t, err) - reg.Verify(t) - - bodyBytes, _ := ioutil.ReadAll(reg.Requests[1].Body) - reqBody := make(map[string]interface{}) - err = json.Unmarshal(bodyBytes, &reqBody) - if err != nil { - t.Fatalf("error decoding JSON: %v", err) + if tt.wantParams != nil { + bodyBytes, _ := ioutil.ReadAll(reg.Requests[1].Body) + reqBody := make(map[string]interface{}) + err = json.Unmarshal(bodyBytes, &reqBody) + if err != nil { + t.Fatalf("error decoding JSON: %v", err) + } + assert.Equal(t, tt.wantParams, reqBody) } - assert.Equal(t, tt.wantParams, reqBody) }) } } From 3f3c781ab649ae0c13fa1ff5ad8d92d4b920720f Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Wed, 16 Sep 2020 11:47:15 +0200 Subject: [PATCH 52/67] Remove square brackets from persistent flag description to fix zsh autocomplete --- pkg/cmdutil/repo_override.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmdutil/repo_override.go b/pkg/cmdutil/repo_override.go index 8b3d36489..aede57ea6 100644 --- a/pkg/cmdutil/repo_override.go +++ b/pkg/cmdutil/repo_override.go @@ -8,7 +8,7 @@ import ( ) func EnableRepoOverride(cmd *cobra.Command, f *Factory) { - cmd.PersistentFlags().StringP("repo", "R", "", "Select another repository using the `[HOST/]OWNER/REPO` format") + cmd.PersistentFlags().StringP("repo", "R", "", "Select another repository using the `OWNER/REPO` format") cmd.PersistentPreRun = func(cmd *cobra.Command, args []string) { repoOverride, _ := cmd.Flags().GetString("repo") From 6d111b2458638b92f911a177b467350c3fdb0b28 Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Wed, 16 Sep 2020 14:45:06 +0200 Subject: [PATCH 53/67] Use WSL2 detection to pick browser --- pkg/browser/browser.go | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/pkg/browser/browser.go b/pkg/browser/browser.go index 67ebca92f..a91bae302 100644 --- a/pkg/browser/browser.go +++ b/pkg/browser/browser.go @@ -1,6 +1,7 @@ package browser import ( + "bufio" "os" "os/exec" "runtime" @@ -30,7 +31,11 @@ func ForOS(goos, url string) *exec.Cmd { r := strings.NewReplacer("&", "^&") args = append(args, "/c", "start", r.Replace(url)) default: - exe = "xdg-open" + if inWsl() { + exe = "wslview" + } else { + exe = "xdg-open" + } args = append(args, url) } @@ -51,3 +56,21 @@ func FromLauncher(launcher, url string) (*exec.Cmd, error) { cmd.Stderr = os.Stderr return cmd, nil } + +// Determine if gh is running inside of WSL2 +func inWsl() bool { + file, err := os.Open("/proc/sys/kernel/osrelease") + if err != nil { + return false + } + defer file.Close() + + scanner := bufio.NewScanner(file) + scanner.Scan() // Read single line + if err := scanner.Err(); err != nil { + return false + } + + os := scanner.Text() + return strings.Contains(os, "microsoft-WSL2") +} From 7a8db80420893d3201e1904ae0712697271fc100 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 15 Sep 2020 22:39:30 +0200 Subject: [PATCH 54/67] Prompt for push target during `pr create` We no longer guess the head repository using heuristics; instead, we present the user with the choice of pushable repositories and an additional option to create a new fork. The new `pr create --head` flag is available for the user to specify the head branch in `branch` or `owner:branch` format and completely skip any forking or auto-pushing checks. --- api/queries_repo.go | 29 +- context/context.go | 23 +- pkg/cmd/pr/create/create.go | 154 ++++-- pkg/cmd/pr/create/create_test.go | 848 +++++-------------------------- pkg/httpmock/legacy.go | 16 + 5 files changed, 269 insertions(+), 801 deletions(-) diff --git a/api/queries_repo.go b/api/queries_repo.go index e9d8534a5..fdbd13e92 100644 --- a/api/queries_repo.go +++ b/api/queries_repo.go @@ -91,6 +91,8 @@ func GitHubRepo(client *Client, repo ghrepo.Interface) (*Repository, error) { query RepositoryInfo($owner: String!, $name: String!) { repository(owner: $owner, name: $name) { id + name + owner { login } hasIssuesEnabled description viewerPermission @@ -317,8 +319,8 @@ func ForkRepo(client *Client, repo ghrepo.Interface) (*Repository, error) { }, nil } -// RepoFindFork finds a fork of repo affiliated with the viewer -func RepoFindFork(client *Client, repo ghrepo.Interface) (*Repository, error) { +// RepoFindForks finds forks of the repo that are affiliated with the viewer +func RepoFindForks(client *Client, repo ghrepo.Interface, limit int) ([]*Repository, error) { result := struct { Repository struct { Forks struct { @@ -330,12 +332,13 @@ func RepoFindFork(client *Client, repo ghrepo.Interface) (*Repository, error) { variables := map[string]interface{}{ "owner": repo.RepoOwner(), "repo": repo.RepoName(), + "limit": limit, } if err := client.GraphQL(repo.RepoHost(), ` - query RepositoryFindFork($owner: String!, $repo: String!) { + query RepositoryFindFork($owner: String!, $repo: String!, $limit: Int!) { repository(owner: $owner, name: $repo) { - forks(first: 1, affiliations: [OWNER, COLLABORATOR]) { + forks(first: $limit, affiliations: [OWNER, COLLABORATOR]) { nodes { id name @@ -350,14 +353,18 @@ func RepoFindFork(client *Client, repo ghrepo.Interface) (*Repository, error) { return nil, err } - forks := result.Repository.Forks.Nodes - // we check ViewerCanPush, even though we expect it to always be true per - // `affiliations` condition, to guard against versions of GitHub with a - // faulty `affiliations` implementation - if len(forks) > 0 && forks[0].ViewerCanPush() { - return InitRepoHostname(&forks[0], repo.RepoHost()), nil + var results []*Repository + for _, r := range result.Repository.Forks.Nodes { + // we check ViewerCanPush, even though we expect it to always be true per + // `affiliations` condition, to guard against versions of GitHub with a + // faulty `affiliations` implementation + if !r.ViewerCanPush() { + continue + } + results = append(results, InitRepoHostname(&r, repo.RepoHost())) } - return nil, &NotFoundError{errors.New("no fork found")} + + return results, nil } type RepoMetadataResult struct { diff --git a/context/context.go b/context/context.go index 746c866a2..babd51f55 100644 --- a/context/context.go +++ b/context/context.go @@ -139,7 +139,7 @@ func (r *ResolvedRemotes) BaseRepo(io *iostreams.IOStreams) (ghrepo.Interface, e return selectedRepo, err } -func (r *ResolvedRemotes) HeadRepo(baseRepo ghrepo.Interface) (ghrepo.Interface, error) { +func (r *ResolvedRemotes) HeadRepos() ([]*api.Repository, error) { if r.network == nil { err := resolveNetwork(r) if err != nil { @@ -147,28 +147,13 @@ func (r *ResolvedRemotes) HeadRepo(baseRepo ghrepo.Interface) (ghrepo.Interface, } } - // try to find a pushable fork among existing remotes - for _, repo := range r.network.Repositories { - if repo != nil && repo.Parent != nil && repo.ViewerCanPush() && ghrepo.IsSame(repo.Parent, baseRepo) { - return repo, nil - } - } - - // a fork might still exist on GitHub, so let's query for it - var notFound *api.NotFoundError - if repo, err := api.RepoFindFork(r.apiClient, baseRepo); err == nil { - return repo, nil - } else if !errors.As(err, ¬Found) { - return nil, err - } - - // fall back to any listed repository that has push access + var results []*api.Repository for _, repo := range r.network.Repositories { if repo != nil && repo.ViewerCanPush() { - return repo, nil + results = append(results, repo) } } - return nil, errors.New("none of the repositories have push access") + return results, nil } // RemoteForRepo finds the git remote that points to a repository diff --git a/pkg/cmd/pr/create/create.go b/pkg/cmd/pr/create/create.go index 655fca9ba..3bffbb1d7 100644 --- a/pkg/cmd/pr/create/create.go +++ b/pkg/cmd/pr/create/create.go @@ -7,6 +7,7 @@ import ( "strings" "time" + "github.com/AlecAivazis/survey/v2" "github.com/MakeNowJust/heredoc" "github.com/cli/cli/api" "github.com/cli/cli/context" @@ -17,6 +18,7 @@ import ( "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/githubtemplate" "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/pkg/prompt" "github.com/cli/cli/utils" "github.com/spf13/cobra" ) @@ -40,6 +42,7 @@ type CreateOptions struct { Title string Body string BaseBranch string + HeadBranch string Reviewers []string Assignees []string @@ -60,13 +63,23 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co cmd := &cobra.Command{ Use: "create", Short: "Create a pull request", + Long: heredoc.Doc(` + Create a pull request on GitHub. + + When the current branch isn't fully pushed to a git remote, a prompt will ask where + to push the branch and offer an option to fork the base repository. Use '--head' to + explicitly skip any forking or pushing behavior. + + A prompt will also ask for the title and the body of the pull request. Use '--title' + and '--body' to skip this, or use '--fill' to autofill these values from git commits. + `), Example: heredoc.Doc(` $ gh pr create --title "The bug is fixed" --body "Everything works again" $ gh issue create --label "bug,help wanted" $ gh issue create --label bug --label "help wanted" $ gh pr create --reviewer monalisa,hubot $ gh pr create --project "Roadmap" - $ gh pr create --base develop + $ gh pr create --base develop --head monalisa:feature `), Args: cmdutil.NoArgsQuoteReminder, RunE: func(cmd *cobra.Command, args []string) error { @@ -96,9 +109,10 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co fl := cmd.Flags() fl.BoolVarP(&opts.IsDraft, "draft", "d", false, "Mark pull request as a draft") - fl.StringVarP(&opts.Title, "title", "t", "", "Supply a title. Will prompt for one otherwise.") - fl.StringVarP(&opts.Body, "body", "b", "", "Supply a body. Will prompt for one otherwise.") - fl.StringVarP(&opts.BaseBranch, "base", "B", "", "The branch into which you want your code merged") + fl.StringVarP(&opts.Title, "title", "t", "", "Title for the pull request") + fl.StringVarP(&opts.Body, "body", "b", "", "Body for the pull request") + fl.StringVarP(&opts.BaseBranch, "base", "B", "", "The `branch` into which you want your code merged") + fl.StringVarP(&opts.HeadBranch, "head", "H", "", "The `branch` that contains commits for your pull request (default: current branch)") fl.BoolVarP(&opts.WebMode, "web", "w", false, "Open the web browser to create a pull request") fl.BoolVarP(&opts.Autofill, "fill", "f", false, "Do not prompt for title/body and just use commit info") fl.StringSliceVarP(&opts.Reviewers, "reviewer", "r", nil, "Request reviews from people by their `login`") @@ -132,6 +146,8 @@ func createRun(opts *CreateOptions) error { if r, ok := br.(*api.Repository); ok { baseRepo = r } else { + // TODO: if RepoNetwork is going to be requested anyway in `repoContext.HeadRepos()`, + // consider piggybacking on that result instead of performing a separate lookup var err error baseRepo, err = api.GitHubRepo(client, br) if err != nil { @@ -142,32 +158,106 @@ func createRun(opts *CreateOptions) error { return fmt.Errorf("could not determine base repository: %w", err) } - headBranch, err := opts.Branch() - if err != nil { - return fmt.Errorf("could not determine the current branch: %w", err) + isPushEnabled := false + headBranch := opts.HeadBranch + headBranchLabel := opts.HeadBranch + if headBranch == "" { + headBranch, err = opts.Branch() + if err != nil { + return fmt.Errorf("could not determine the current branch: %w", err) + } + headBranchLabel = headBranch + isPushEnabled = true + } else if idx := strings.IndexRune(headBranch, ':'); idx >= 0 { + headBranch = headBranch[idx+1:] + } + + if ucc, err := git.UncommittedChangeCount(); err == nil && ucc > 0 { + fmt.Fprintf(opts.IO.ErrOut, "Warning: %s\n", utils.Pluralize(ucc, "uncommitted change")) } var headRepo ghrepo.Interface var headRemote *context.Remote - // determine whether the head branch is already pushed to a remote - headBranchPushedTo := determineTrackingBranch(remotes, headBranch) - if headBranchPushedTo != nil { - for _, r := range remotes { - if r.Name != headBranchPushedTo.RemoteName { - continue + if isPushEnabled { + // determine whether the head branch is already pushed to a remote + if pushedTo := determineTrackingBranch(remotes, headBranch); pushedTo != nil { + isPushEnabled = false + for _, r := range remotes { + if r.Name != pushedTo.RemoteName { + continue + } + headRepo = r + headRemote = r + break } - headRepo = r - headRemote = r - break } } - // otherwise, determine the head repository with info obtained from the API - if headRepo == nil { - if r, err := repoContext.HeadRepo(baseRepo); err == nil { - headRepo = r + // otherwise, ask the user for the head repository using info obtained from the API + if headRepo == nil && isPushEnabled && opts.IO.CanPrompt() { + pushableRepos, err := repoContext.HeadRepos() + if err != nil { + return err } + + if len(pushableRepos) == 0 { + pushableRepos, err = api.RepoFindForks(client, baseRepo, 3) + if err != nil { + return err + } + } + + currentLogin, err := api.CurrentLoginName(client, baseRepo.RepoHost()) + if err != nil { + return err + } + + hasOwnFork := false + var pushOptions []string + for _, r := range pushableRepos { + pushOptions = append(pushOptions, ghrepo.FullName(r)) + if r.RepoOwner() == currentLogin { + hasOwnFork = true + } + } + + if !hasOwnFork { + pushOptions = append(pushOptions, "Create a fork of "+ghrepo.FullName(baseRepo)) + } + pushOptions = append(pushOptions, "Skip pushing the branch") + pushOptions = append(pushOptions, "Cancel") + + var selectedOption int + err = prompt.SurveyAskOne(&survey.Select{ + Message: fmt.Sprintf("Where should we push the '%s' branch?", headBranch), + Options: pushOptions, + }, &selectedOption) + if err != nil { + return err + } + + if selectedOption < len(pushableRepos) { + headRepo = pushableRepos[selectedOption] + if !ghrepo.IsSame(baseRepo, headRepo) { + headBranchLabel = fmt.Sprintf("%s:%s", headRepo.RepoOwner(), headBranch) + } + } else if pushOptions[selectedOption] == "Skip pushing the branch" { + isPushEnabled = false + } else if pushOptions[selectedOption] == "Cancel" { + return cmdutil.SilentError + } else { + // "Create a fork of ..." + if baseRepo.IsPrivate { + return fmt.Errorf("cannot fork private repository %s", ghrepo.FullName(baseRepo)) + } + headBranchLabel = fmt.Sprintf("%s:%s", currentLogin, headBranch) + } + } + + if headRepo == nil && isPushEnabled && !opts.IO.CanPrompt() { + fmt.Fprintf(opts.IO.ErrOut, "aborted: you must first push the current branch to a remote, or use the --head flag") + return cmdutil.SilentError } baseBranch := opts.BaseBranch @@ -178,10 +268,6 @@ func createRun(opts *CreateOptions) error { return fmt.Errorf("must be on a branch named differently than %q", baseBranch) } - if ucc, err := git.UncommittedChangeCount(); err == nil && ucc > 0 { - fmt.Fprintf(opts.IO.ErrOut, "Warning: %s\n", utils.Pluralize(ucc, "uncommitted change")) - } - var milestoneTitles []string if opts.Milestone != "" { milestoneTitles = []string{opts.Milestone} @@ -211,10 +297,6 @@ func createRun(opts *CreateOptions) error { } if !opts.WebMode { - headBranchLabel := headBranch - if headRepo != nil && !ghrepo.IsSame(baseRepo, headRepo) { - headBranchLabel = fmt.Sprintf("%s:%s", headRepo.RepoOwner(), headBranch) - } existingPR, err := api.PullRequestForBranch(client, baseRepo, baseBranch, headBranchLabel) var notFound *api.NotFoundError if err != nil && !errors.As(err, ¬Found) { @@ -297,10 +379,7 @@ func createRun(opts *CreateOptions) error { didForkRepo := false // if a head repository could not be determined so far, automatically create // one by forking the base repository - if headRepo == nil { - if baseRepo.IsPrivate { - return fmt.Errorf("cannot fork private repository '%s'", ghrepo.FullName(baseRepo)) - } + if headRepo == nil && isPushEnabled { headRepo, err = api.ForkRepo(client, baseRepo) if err != nil { return fmt.Errorf("error forking repo: %w", err) @@ -308,12 +387,7 @@ func createRun(opts *CreateOptions) error { didForkRepo = true } - headBranchLabel := headBranch - if !ghrepo.IsSame(baseRepo, headRepo) { - headBranchLabel = fmt.Sprintf("%s:%s", headRepo.RepoOwner(), headBranch) - } - - if headRemote == nil { + if headRemote == nil && headRepo != nil { headRemote, _ = repoContext.RemoteForRepo(headRepo) } @@ -324,7 +398,7 @@ func createRun(opts *CreateOptions) error { // // In either case, we want to add the head repo as a new git remote so we // can push to it. - if headRemote == nil { + if headRemote == nil && isPushEnabled { cfg, err := opts.Config() if err != nil { return err @@ -345,7 +419,7 @@ func createRun(opts *CreateOptions) error { } // automatically push the branch if it hasn't been pushed anywhere yet - if headBranchPushedTo == nil { + if isPushEnabled { pushTries := 0 maxPushTries := 3 for { diff --git a/pkg/cmd/pr/create/create_test.go b/pkg/cmd/pr/create/create_test.go index 1eecab69b..935675f5d 100644 --- a/pkg/cmd/pr/create/create_test.go +++ b/pkg/cmd/pr/create/create_test.go @@ -5,8 +5,6 @@ import ( "encoding/json" "io/ioutil" "net/http" - "os" - "path" "reflect" "strings" "testing" @@ -56,8 +54,11 @@ func runCommandWithRootDirOverridden(rt http.RoundTripper, remotes context.Remot } return context.Remotes{ { - Remote: &git.Remote{Name: "origin"}, - Repo: ghrepo.New("OWNER", "REPO"), + Remote: &git.Remote{ + Name: "origin", + Resolved: "base", + }, + Repo: ghrepo.New("OWNER", "REPO"), }, }, nil }, @@ -97,31 +98,23 @@ func TestPRCreate_nontty_web(t *testing.T) { http := initFakeHTTP() defer http.Verify(t) - http.StubRepoResponse("OWNER", "REPO") - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "forks": { "nodes": [ - ] } } } } - `)) + http.StubRepoInfoResponse("OWNER", "REPO", "master") cs, cmdTeardown := test.InitCmdStubber() defer cmdTeardown() - cs.Stub("") // git config --get-regexp (determineTrackingBranch) - cs.Stub("") // git show-ref --verify (determineTrackingBranch) cs.Stub("") // git status cs.Stub("1234567890,commit 0\n2345678901,commit 1") // git log - cs.Stub("") // git push cs.Stub("") // browser - output, err := runCommand(http, nil, "feature", false, `--web`) + output, err := runCommand(http, nil, "feature", false, `--web --head=feature`) require.NoError(t, err) eq(t, output.String(), "") eq(t, output.Stderr(), "") - eq(t, len(cs.Calls), 6) - eq(t, strings.Join(cs.Calls[4].Args, " "), "git push --set-upstream origin HEAD:feature") - browserCall := cs.Calls[5].Args + eq(t, len(cs.Calls), 3) + browserCall := cs.Calls[2].Args eq(t, browserCall[len(browserCall)-1], "https://github.com/OWNER/REPO/compare/master...feature?expand=1") } @@ -144,11 +137,7 @@ func TestPRCreate_nontty(t *testing.T) { http := initFakeHTTP() defer http.Verify(t) - http.StubRepoResponse("OWNER", "REPO") - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "forks": { "nodes": [ - ] } } } } - `)) + http.StubRepoInfoResponse("OWNER", "REPO", "master") http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { "pullRequests": { "nodes" : [ ] } } } } @@ -162,16 +151,13 @@ func TestPRCreate_nontty(t *testing.T) { cs, cmdTeardown := test.InitCmdStubber() defer cmdTeardown() - cs.Stub("") // git config --get-regexp (determineTrackingBranch) - cs.Stub("") // git show-ref --verify (determineTrackingBranch) cs.Stub("") // git status cs.Stub("1234567890,commit 0\n2345678901,commit 1") // git log - cs.Stub("") // git push - output, err := runCommand(http, nil, "feature", false, `-t "my title" -b "my body"`) + output, err := runCommand(http, nil, "feature", false, `-t "my title" -b "my body" -H feature`) require.NoError(t, err) - bodyBytes, _ := ioutil.ReadAll(http.Requests[3].Body) + bodyBytes, _ := ioutil.ReadAll(http.Requests[2].Body) reqBody := struct { Variables struct { Input struct { @@ -199,20 +185,30 @@ func TestPRCreate(t *testing.T) { http := initFakeHTTP() defer http.Verify(t) + http.StubRepoInfoResponse("OWNER", "REPO", "master") http.StubRepoResponse("OWNER", "REPO") - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "forks": { "nodes": [ - ] } } } } - `)) - http.StubResponse(200, bytes.NewBufferString(` + http.Register( + httpmock.GraphQL(`query UserCurrent\b`), + httpmock.StringResponse(`{"data": {"viewer": {"login": "OWNER"} } }`)) + http.Register( + httpmock.GraphQL(`query PullRequestForBranch\b`), + httpmock.StringResponse(` { "data": { "repository": { "pullRequests": { "nodes" : [ ] } } } } - `)) - http.StubResponse(200, bytes.NewBufferString(` + `)) + http.Register( + httpmock.GraphQL(`mutation PullRequestCreate\b`), + httpmock.GraphQLMutation(` { "data": { "createPullRequest": { "pullRequest": { "URL": "https://github.com/OWNER/REPO/pull/12" } } } } - `)) + `, func(input map[string]interface{}) { + assert.Equal(t, "REPOID", input["repositoryId"].(string)) + assert.Equal(t, "my title", input["title"].(string)) + assert.Equal(t, "my body", input["body"].(string)) + assert.Equal(t, "master", input["baseRefName"].(string)) + assert.Equal(t, "feature", input["headRefName"].(string)) + })) cs, cmdTeardown := test.InitCmdStubber() defer cmdTeardown() @@ -223,49 +219,51 @@ func TestPRCreate(t *testing.T) { cs.Stub("1234567890,commit 0\n2345678901,commit 1") // git log cs.Stub("") // git push + ask, cleanupAsk := prompt.InitAskStubber() + defer cleanupAsk() + ask.StubOne(0) + output, err := runCommand(http, nil, "feature", true, `-t "my title" -b "my body"`) require.NoError(t, err) - bodyBytes, _ := ioutil.ReadAll(http.Requests[3].Body) - reqBody := struct { - Variables struct { - Input struct { - RepositoryID string - Title string - Body string - BaseRefName string - HeadRefName string - } - } - }{} - _ = json.Unmarshal(bodyBytes, &reqBody) - - eq(t, reqBody.Variables.Input.RepositoryID, "REPOID") - eq(t, reqBody.Variables.Input.Title, "my title") - eq(t, reqBody.Variables.Input.Body, "my body") - eq(t, reqBody.Variables.Input.BaseRefName, "master") - eq(t, reqBody.Variables.Input.HeadRefName, "feature") - - eq(t, output.String(), "https://github.com/OWNER/REPO/pull/12\n") + assert.Equal(t, "https://github.com/OWNER/REPO/pull/12\n", output.String()) + assert.Equal(t, "\nCreating pull request for feature into master in OWNER/REPO\n\n", output.Stderr()) } -func TestPRCreate_nonLegacyTemplate(t *testing.T) { + +func TestPRCreate_createFork(t *testing.T) { http := initFakeHTTP() defer http.Verify(t) + http.StubRepoInfoResponse("OWNER", "REPO", "master") http.StubRepoResponse("OWNER", "REPO") - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "forks": { "nodes": [ - ] } } } } - `)) - http.StubResponse(200, bytes.NewBufferString(` + http.Register( + httpmock.GraphQL(`query UserCurrent\b`), + httpmock.StringResponse(`{"data": {"viewer": {"login": "monalisa"} } }`)) + http.Register( + httpmock.GraphQL(`query PullRequestForBranch\b`), + httpmock.StringResponse(` { "data": { "repository": { "pullRequests": { "nodes" : [ ] } } } } - `)) - http.StubResponse(200, bytes.NewBufferString(` + `)) + http.Register( + httpmock.REST("POST", "repos/OWNER/REPO/forks"), + httpmock.StatusStringResponse(201, ` + { "node_id": "NODEID", + "name": "REPO", + "owner": {"login": "monalisa"} + } + `)) + http.Register( + httpmock.GraphQL(`mutation PullRequestCreate\b`), + httpmock.GraphQLMutation(` { "data": { "createPullRequest": { "pullRequest": { "URL": "https://github.com/OWNER/REPO/pull/12" } } } } - `)) + `, func(input map[string]interface{}) { + assert.Equal(t, "REPOID", input["repositoryId"].(string)) + assert.Equal(t, "master", input["baseRefName"].(string)) + assert.Equal(t, "monalisa:feature", input["headRefName"].(string)) + })) cs, cmdTeardown := test.InitCmdStubber() defer cmdTeardown() @@ -274,8 +272,50 @@ func TestPRCreate_nonLegacyTemplate(t *testing.T) { cs.Stub("") // git show-ref --verify (determineTrackingBranch) cs.Stub("") // git status cs.Stub("1234567890,commit 0\n2345678901,commit 1") // git log + cs.Stub("") // git remote add cs.Stub("") // git push + ask, cleanupAsk := prompt.InitAskStubber() + defer cleanupAsk() + ask.StubOne(1) + + output, err := runCommand(http, nil, "feature", true, `-t title -b body`) + require.NoError(t, err) + + assert.Equal(t, []string{"git", "remote", "add", "-f", "fork", "https://github.com/monalisa/REPO.git"}, cs.Calls[4].Args) + assert.Equal(t, []string{"git", "push", "--set-upstream", "fork", "HEAD:feature"}, cs.Calls[5].Args) + + assert.Equal(t, "https://github.com/OWNER/REPO/pull/12\n", output.String()) +} + +func TestPRCreate_nonLegacyTemplate(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + + http.StubRepoInfoResponse("OWNER", "REPO", "master") + http.Register( + httpmock.GraphQL(`query PullRequestForBranch\b`), + httpmock.StringResponse(` + { "data": { "repository": { "pullRequests": { "nodes" : [ + ] } } } } + `)) + http.Register( + httpmock.GraphQL(`mutation PullRequestCreate\b`), + httpmock.GraphQLMutation(` + { "data": { "createPullRequest": { "pullRequest": { + "URL": "https://github.com/OWNER/REPO/pull/12" + } } } } + `, func(input map[string]interface{}) { + assert.Equal(t, "my title", input["title"].(string)) + assert.Equal(t, "- commit 1\n- commit 0\n\nFixes a bug and Closes an issue", input["body"].(string)) + })) + + cs, cmdTeardown := test.InitCmdStubber() + defer cmdTeardown() + + cs.Stub("") // git status + cs.Stub("1234567890,commit 0\n2345678901,commit 1") // git log + as, teardown := prompt.InitAskStubber() defer teardown() as.Stub([]*prompt.QuestionStub{ @@ -297,29 +337,9 @@ func TestPRCreate_nonLegacyTemplate(t *testing.T) { }, }) - output, err := runCommandWithRootDirOverridden(http, nil, "feature", true, `-t "my title"`, "./fixtures/repoWithNonLegacyPRTemplates") + output, err := runCommandWithRootDirOverridden(http, nil, "feature", true, `-t "my title" -H feature`, "./fixtures/repoWithNonLegacyPRTemplates") require.NoError(t, err) - bodyBytes, _ := ioutil.ReadAll(http.Requests[3].Body) - reqBody := struct { - Variables struct { - Input struct { - RepositoryID string - Title string - Body string - BaseRefName string - HeadRefName string - } - } - }{} - _ = json.Unmarshal(bodyBytes, &reqBody) - - eq(t, reqBody.Variables.Input.RepositoryID, "REPOID") - eq(t, reqBody.Variables.Input.Title, "my title") - eq(t, reqBody.Variables.Input.Body, "- commit 1\n- commit 0\n\nFixes a bug and Closes an issue") - eq(t, reqBody.Variables.Input.BaseRefName, "master") - eq(t, reqBody.Variables.Input.HeadRefName, "feature") - eq(t, output.String(), "https://github.com/OWNER/REPO/pull/12\n") } @@ -327,15 +347,7 @@ func TestPRCreate_metadata(t *testing.T) { http := initFakeHTTP() defer http.Verify(t) - http.Register( - httpmock.GraphQL(`query RepositoryNetwork\b`), - httpmock.StringResponse(httpmock.RepoNetworkStubResponse("OWNER", "REPO", "master", "WRITE"))) - http.Register( - httpmock.GraphQL(`query RepositoryFindFork\b`), - httpmock.StringResponse(` - { "data": { "repository": { "forks": { "nodes": [ - ] } } } } - `)) + http.StubRepoInfoResponse("OWNER", "REPO", "master") http.Register( httpmock.GraphQL(`query PullRequestForBranch\b`), httpmock.StringResponse(` @@ -434,71 +446,20 @@ func TestPRCreate_metadata(t *testing.T) { cs, cmdTeardown := test.InitCmdStubber() defer cmdTeardown() - cs.Stub("") // git config --get-regexp (determineTrackingBranch) - cs.Stub("") // git show-ref --verify (determineTrackingBranch) cs.Stub("") // git status cs.Stub("1234567890,commit 0\n2345678901,commit 1") // git log - cs.Stub("") // git push - output, err := runCommand(http, nil, "feature", true, `-t TITLE -b BODY -a monalisa -l bug -l todo -p roadmap -m 'big one.oh' -r hubot -r monalisa -r /core -r /robots`) + output, err := runCommand(http, nil, "feature", true, `-t TITLE -b BODY -H feature -a monalisa -l bug -l todo -p roadmap -m 'big one.oh' -r hubot -r monalisa -r /core -r /robots`) eq(t, err, nil) eq(t, output.String(), "https://github.com/OWNER/REPO/pull/12\n") } -func TestPRCreate_withForking(t *testing.T) { - http := initFakeHTTP() - defer http.Verify(t) - - http.StubRepoResponseWithPermission("OWNER", "REPO", "READ") - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "forks": { "nodes": [ - ] } } } } - `)) - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "pullRequests": { "nodes" : [ - ] } } } } - `)) - http.StubResponse(200, bytes.NewBufferString(` - { "node_id": "NODEID", - "name": "REPO", - "owner": {"login": "myself"}, - "clone_url": "http://example.com", - "created_at": "2008-02-25T20:21:40Z" - } - `)) - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "createPullRequest": { "pullRequest": { - "URL": "https://github.com/OWNER/REPO/pull/12" - } } } } - `)) - - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() - - cs.Stub("") // git config --get-regexp (determineTrackingBranch) - cs.Stub("") // git show-ref --verify (determineTrackingBranch) - cs.Stub("") // git status - cs.Stub("1234567890,commit 0\n2345678901,commit 1") // git log - cs.Stub("") // git remote add - cs.Stub("") // git push - - output, err := runCommand(http, nil, "feature", true, `-t title -b body`) - require.NoError(t, err) - - eq(t, http.Requests[3].URL.Path, "/repos/OWNER/REPO/forks") - eq(t, output.String(), "https://github.com/OWNER/REPO/pull/12\n") -} - func TestPRCreate_alreadyExists(t *testing.T) { http := initFakeHTTP() defer http.Verify(t) - http.StubRepoResponse("OWNER", "REPO") - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "forks": { "nodes": [ - ] } } } } - `)) + http.StubRepoInfoResponse("OWNER", "REPO", "master") http.StubResponse(200, bytes.NewBufferString(` { "data": { "repository": { "pullRequests": { "nodes": [ { "url": "https://github.com/OWNER/REPO/pull/123", @@ -515,7 +476,7 @@ func TestPRCreate_alreadyExists(t *testing.T) { cs.Stub("") // git status cs.Stub("1234567890,commit 0\n2345678901,commit 1") // git log - _, err := runCommand(http, nil, "feature", true, ``) + _, err := runCommand(http, nil, "feature", true, `-H feature`) if err == nil { t.Fatal("error expected, got nil") } @@ -524,48 +485,15 @@ func TestPRCreate_alreadyExists(t *testing.T) { } } -func TestPRCreate_alreadyExistsDifferentBase(t *testing.T) { - http := initFakeHTTP() - defer http.Verify(t) - - http.StubRepoResponse("OWNER", "REPO") - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "forks": { "nodes": [ - ] } } } } - `)) - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "pullRequests": { "nodes": [ - { "url": "https://github.com/OWNER/REPO/pull/123", - "headRefName": "feature", - "baseRefName": "master" } - ] } } } } - `)) - http.StubResponse(200, bytes.NewBufferString("{}")) - - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() - - cs.Stub("") // git config --get-regexp (determineTrackingBranch) - cs.Stub("") // git show-ref --verify (determineTrackingBranch) - cs.Stub("") // git status - cs.Stub("1234567890,commit 0\n2345678901,commit 1") // git log - cs.Stub("") // git rev-parse - - _, err := runCommand(http, nil, "feature", true, `-BanotherBase -t"cool" -b"nah"`) - if err != nil { - t.Errorf("got unexpected error %q", err) - } -} - func TestPRCreate_web(t *testing.T) { http := initFakeHTTP() defer http.Verify(t) + http.StubRepoInfoResponse("OWNER", "REPO", "master") http.StubRepoResponse("OWNER", "REPO") - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "forks": { "nodes": [ - ] } } } } - `)) + http.Register( + httpmock.GraphQL(`query UserCurrent\b`), + httpmock.StringResponse(`{"data": {"viewer": {"login": "OWNER"} } }`)) cs, cmdTeardown := test.InitCmdStubber() defer cmdTeardown() @@ -577,6 +505,10 @@ func TestPRCreate_web(t *testing.T) { cs.Stub("") // git push cs.Stub("") // browser + ask, cleanupAsk := prompt.InitAskStubber() + defer cleanupAsk() + ask.StubOne(0) + output, err := runCommand(http, nil, "feature", true, `--web`) require.NoError(t, err) @@ -589,552 +521,6 @@ func TestPRCreate_web(t *testing.T) { eq(t, browserCall[len(browserCall)-1], "https://github.com/OWNER/REPO/compare/master...feature?expand=1") } -func TestPRCreate_ReportsUncommittedChanges(t *testing.T) { - http := initFakeHTTP() - defer http.Verify(t) - - http.StubRepoResponse("OWNER", "REPO") - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "forks": { "nodes": [ - ] } } } } - `)) - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "pullRequests": { "nodes" : [ - ] } } } } - `)) - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "createPullRequest": { "pullRequest": { - "URL": "https://github.com/OWNER/REPO/pull/12" - } } } } - `)) - - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() - - cs.Stub("") // git config --get-regexp (determineTrackingBranch) - cs.Stub("") // git show-ref --verify (determineTrackingBranch) - cs.Stub(" M git/git.go") // git status - cs.Stub("1234567890,commit 0\n2345678901,commit 1") // git log - cs.Stub("") // git push - - output, err := runCommand(http, nil, "feature", true, `-t "my title" -b "my body"`) - eq(t, err, nil) - - eq(t, output.String(), "https://github.com/OWNER/REPO/pull/12\n") - test.ExpectLines(t, output.Stderr(), `Warning: 1 uncommitted change`, `Creating pull request for.*feature.*into.*master.*in OWNER/REPO`) -} - -func TestPRCreate_cross_repo_same_branch(t *testing.T) { - remotes := context.Remotes{ - { - Remote: &git.Remote{Name: "origin"}, - Repo: ghrepo.New("OWNER", "REPO"), - }, - { - Remote: &git.Remote{Name: "fork"}, - Repo: ghrepo.New("MYSELF", "REPO"), - }, - } - - http := initFakeHTTP() - defer http.Verify(t) - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repo_000": { - "id": "REPOID0", - "name": "REPO", - "owner": {"login": "OWNER"}, - "defaultBranchRef": { - "name": "default" - }, - "viewerPermission": "READ" - }, - "repo_001" : { - "parent": { - "id": "REPOID0", - "name": "REPO", - "owner": {"login": "OWNER"}, - "defaultBranchRef": { - "name": "default" - }, - "viewerPermission": "READ" - }, - "id": "REPOID1", - "name": "REPO", - "owner": {"login": "MYSELF"}, - "defaultBranchRef": { - "name": "default" - }, - "viewerPermission": "WRITE" - } } } - `)) - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "pullRequests": { "nodes" : [ - ] } } } } - `)) - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "createPullRequest": { "pullRequest": { - "URL": "https://github.com/OWNER/REPO/pull/12" - } } } } - `)) - - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() - - cs.Stub("") // git config --get-regexp (determineTrackingBranch) - cs.Stub("") // git show-ref --verify (determineTrackingBranch) - cs.Stub("") // git status - cs.Stub("1234567890,commit 0\n2345678901,commit 1") // git log - cs.Stub("") // git push - - output, err := runCommand(http, remotes, "default", true, `-t "cross repo" -b "same branch"`) - require.NoError(t, err) - - bodyBytes, _ := ioutil.ReadAll(http.Requests[2].Body) - reqBody := struct { - Variables struct { - Input struct { - RepositoryID string - Title string - Body string - BaseRefName string - HeadRefName string - } - } - }{} - _ = json.Unmarshal(bodyBytes, &reqBody) - - eq(t, reqBody.Variables.Input.RepositoryID, "REPOID0") - eq(t, reqBody.Variables.Input.Title, "cross repo") - eq(t, reqBody.Variables.Input.Body, "same branch") - eq(t, reqBody.Variables.Input.BaseRefName, "default") - eq(t, reqBody.Variables.Input.HeadRefName, "MYSELF:default") - - eq(t, output.String(), "https://github.com/OWNER/REPO/pull/12\n") - - // goal: only care that gql is formatted properly -} - -func TestPRCreate_survey_defaults_multicommit(t *testing.T) { - http := initFakeHTTP() - defer http.Verify(t) - - http.StubRepoResponse("OWNER", "REPO") - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "forks": { "nodes": [ - ] } } } } - `)) - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "pullRequests": { "nodes" : [ - ] } } } } - `)) - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "createPullRequest": { "pullRequest": { - "URL": "https://github.com/OWNER/REPO/pull/12" - } } } } - `)) - - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() - - cs.Stub("") // git config --get-regexp (determineTrackingBranch) - cs.Stub("") // git show-ref --verify (determineTrackingBranch) - cs.Stub("") // git status - cs.Stub("1234567890,commit 0\n2345678901,commit 1") // git log - cs.Stub("") // git rev-parse - cs.Stub("") // git push - - as, surveyTeardown := prompt.InitAskStubber() - defer surveyTeardown() - - as.Stub([]*prompt.QuestionStub{ - { - Name: "title", - Default: true, - }, - { - Name: "body", - Default: true, - }, - }) - as.Stub([]*prompt.QuestionStub{ - { - Name: "confirmation", - Value: 0, - }, - }) - - output, err := runCommand(http, nil, "cool_bug-fixes", true, ``) - require.NoError(t, err) - - bodyBytes, _ := ioutil.ReadAll(http.Requests[3].Body) - reqBody := struct { - Variables struct { - Input struct { - RepositoryID string - Title string - Body string - BaseRefName string - HeadRefName string - } - } - }{} - _ = json.Unmarshal(bodyBytes, &reqBody) - - expectedBody := "- commit 1\n- commit 0\n" - - eq(t, reqBody.Variables.Input.RepositoryID, "REPOID") - eq(t, reqBody.Variables.Input.Title, "cool bug fixes") - eq(t, reqBody.Variables.Input.Body, expectedBody) - eq(t, reqBody.Variables.Input.BaseRefName, "master") - eq(t, reqBody.Variables.Input.HeadRefName, "cool_bug-fixes") - - eq(t, output.String(), "https://github.com/OWNER/REPO/pull/12\n") -} - -func TestPRCreate_survey_defaults_monocommit(t *testing.T) { - http := initFakeHTTP() - defer http.Verify(t) - - http.Register(httpmock.GraphQL(`query RepositoryNetwork\b`), httpmock.StringResponse(httpmock.RepoNetworkStubResponse("OWNER", "REPO", "master", "WRITE"))) - http.Register(httpmock.GraphQL(`query RepositoryFindFork\b`), httpmock.StringResponse(` - { "data": { "repository": { "forks": { "nodes": [ - ] } } } } - `)) - http.Register(httpmock.GraphQL(`query PullRequestForBranch\b`), httpmock.StringResponse(` - { "data": { "repository": { "pullRequests": { "nodes" : [ - ] } } } } - `)) - http.Register(httpmock.GraphQL(`mutation PullRequestCreate\b`), httpmock.GraphQLMutation(` - { "data": { "createPullRequest": { "pullRequest": { - "URL": "https://github.com/OWNER/REPO/pull/12" - } } } } - `, func(inputs map[string]interface{}) { - eq(t, inputs["repositoryId"], "REPOID") - eq(t, inputs["title"], "the sky above the port") - eq(t, inputs["body"], "was the color of a television, turned to a dead channel") - eq(t, inputs["baseRefName"], "master") - eq(t, inputs["headRefName"], "feature") - })) - - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() - - cs.Stub("") // git config --get-regexp (determineTrackingBranch) - cs.Stub("") // git show-ref --verify (determineTrackingBranch) - cs.Stub("") // git status - cs.Stub("1234567890,the sky above the port") // git log - cs.Stub("was the color of a television, turned to a dead channel") // git show - cs.Stub("") // git rev-parse - cs.Stub("") // git push - - as, surveyTeardown := prompt.InitAskStubber() - defer surveyTeardown() - - as.Stub([]*prompt.QuestionStub{ - { - Name: "title", - Default: true, - }, - { - Name: "body", - Default: true, - }, - }) - as.Stub([]*prompt.QuestionStub{ - { - Name: "confirmation", - Value: 0, - }, - }) - - output, err := runCommand(http, nil, "feature", true, ``) - eq(t, err, nil) - eq(t, output.String(), "https://github.com/OWNER/REPO/pull/12\n") -} - -func TestPRCreate_survey_defaults_monocommit_template(t *testing.T) { - http := initFakeHTTP() - defer http.Verify(t) - - http.Register(httpmock.GraphQL(`query RepositoryNetwork\b`), httpmock.StringResponse(httpmock.RepoNetworkStubResponse("OWNER", "REPO", "master", "WRITE"))) - http.Register(httpmock.GraphQL(`query RepositoryFindFork\b`), httpmock.StringResponse(` - { "data": { "repository": { "forks": { "nodes": [ - ] } } } } - `)) - http.Register(httpmock.GraphQL(`query PullRequestForBranch\b`), httpmock.StringResponse(` - { "data": { "repository": { "pullRequests": { "nodes" : [ - ] } } } } - `)) - http.Register(httpmock.GraphQL(`mutation PullRequestCreate\b`), httpmock.GraphQLMutation(` - { "data": { "createPullRequest": { "pullRequest": { - "URL": "https://github.com/OWNER/REPO/pull/12" - } } } } - `, func(inputs map[string]interface{}) { - eq(t, inputs["repositoryId"], "REPOID") - eq(t, inputs["title"], "the sky above the port") - eq(t, inputs["body"], "was the color of a television\n\n... turned to a dead channel") - eq(t, inputs["baseRefName"], "master") - eq(t, inputs["headRefName"], "feature") - })) - - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() - - tmpdir, err := ioutil.TempDir("", "gh-cli") - if err != nil { - t.Fatal(err) - } - defer os.RemoveAll(tmpdir) - - templateFp := path.Join(tmpdir, ".github/PULL_REQUEST_TEMPLATE.md") - _ = os.MkdirAll(path.Dir(templateFp), 0700) - _ = ioutil.WriteFile(templateFp, []byte("... turned to a dead channel"), 0700) - - cs.Stub("") // git config --get-regexp (determineTrackingBranch) - cs.Stub("") // git show-ref --verify (determineTrackingBranch) - cs.Stub("") // git status - cs.Stub("1234567890,the sky above the port") // git log - cs.Stub("was the color of a television") // git show - cs.Stub(tmpdir) // git rev-parse - cs.Stub("") // git push - - as, surveyTeardown := prompt.InitAskStubber() - defer surveyTeardown() - - as.Stub([]*prompt.QuestionStub{ - { - Name: "title", - Default: true, - }, - { - Name: "body", - Default: true, - }, - }) - as.Stub([]*prompt.QuestionStub{ - { - Name: "confirmation", - Value: 0, - }, - }) - - output, err := runCommand(http, nil, "feature", true, ``) - eq(t, err, nil) - eq(t, output.String(), "https://github.com/OWNER/REPO/pull/12\n") -} - -func TestPRCreate_survey_autofill_nontty(t *testing.T) { - http := initFakeHTTP() - defer http.Verify(t) - - http.StubRepoResponse("OWNER", "REPO") - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "forks": { "nodes": [ - ] } } } } - `)) - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "pullRequests": { "nodes" : [ - ] } } } } - `)) - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "createPullRequest": { "pullRequest": { - "URL": "https://github.com/OWNER/REPO/pull/12" - } } } } - `)) - - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() - - cs.Stub("") // git config --get-regexp (determineTrackingBranch) - cs.Stub("") // git show-ref --verify (determineTrackingBranch) - cs.Stub("") // git status - cs.Stub("1234567890,the sky above the port") // git log - cs.Stub("was the color of a television, turned to a dead channel") // git show - cs.Stub("") // git rev-parse - cs.Stub("") // git push - cs.Stub("") // browser open - - output, err := runCommand(http, nil, "feature", false, `-f`) - require.NoError(t, err) - - bodyBytes, _ := ioutil.ReadAll(http.Requests[3].Body) - reqBody := struct { - Variables struct { - Input struct { - RepositoryID string - Title string - Body string - BaseRefName string - HeadRefName string - } - } - }{} - _ = json.Unmarshal(bodyBytes, &reqBody) - - expectedBody := "was the color of a television, turned to a dead channel" - - assert.Equal(t, "REPOID", reqBody.Variables.Input.RepositoryID) - assert.Equal(t, "the sky above the port", reqBody.Variables.Input.Title) - assert.Equal(t, expectedBody, reqBody.Variables.Input.Body) - assert.Equal(t, "master", reqBody.Variables.Input.BaseRefName) - assert.Equal(t, "feature", reqBody.Variables.Input.HeadRefName) - - assert.Equal(t, "https://github.com/OWNER/REPO/pull/12\n", output.String()) - - assert.Equal(t, "", output.Stderr()) -} - -func TestPRCreate_survey_autofill(t *testing.T) { - http := initFakeHTTP() - defer http.Verify(t) - - http.StubRepoResponse("OWNER", "REPO") - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "forks": { "nodes": [ - ] } } } } - `)) - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "pullRequests": { "nodes" : [ - ] } } } } - `)) - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "createPullRequest": { "pullRequest": { - "URL": "https://github.com/OWNER/REPO/pull/12" - } } } } - `)) - - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() - - cs.Stub("") // git config --get-regexp (determineTrackingBranch) - cs.Stub("") // git show-ref --verify (determineTrackingBranch) - cs.Stub("") // git status - cs.Stub("1234567890,the sky above the port") // git log - cs.Stub("was the color of a television, turned to a dead channel") // git show - cs.Stub("") // git rev-parse - cs.Stub("") // git push - cs.Stub("") // browser open - - output, err := runCommand(http, nil, "feature", true, `-f`) - require.NoError(t, err) - - bodyBytes, _ := ioutil.ReadAll(http.Requests[3].Body) - reqBody := struct { - Variables struct { - Input struct { - RepositoryID string - Title string - Body string - BaseRefName string - HeadRefName string - } - } - }{} - _ = json.Unmarshal(bodyBytes, &reqBody) - - expectedBody := "was the color of a television, turned to a dead channel" - - eq(t, reqBody.Variables.Input.RepositoryID, "REPOID") - eq(t, reqBody.Variables.Input.Title, "the sky above the port") - eq(t, reqBody.Variables.Input.Body, expectedBody) - eq(t, reqBody.Variables.Input.BaseRefName, "master") - eq(t, reqBody.Variables.Input.HeadRefName, "feature") - - eq(t, output.String(), "https://github.com/OWNER/REPO/pull/12\n") -} - -func TestPRCreate_defaults_error_autofill(t *testing.T) { - http := initFakeHTTP() - defer http.Verify(t) - - http.StubRepoResponse("OWNER", "REPO") - - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() - - cs.Stub("") // git config --get-regexp (determineTrackingBranch) - cs.Stub("") // git show-ref --verify (determineTrackingBranch) - cs.Stub("") // git status - cs.Stub("") // git log - - _, err := runCommand(http, nil, "feature", true, "-f") - - eq(t, err.Error(), "could not compute title or body defaults: could not find any commits between origin/master and feature") -} - -func TestPRCreate_defaults_error_web(t *testing.T) { - http := initFakeHTTP() - defer http.Verify(t) - - http.StubRepoResponse("OWNER", "REPO") - - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() - - cs.Stub("") // git config --get-regexp (determineTrackingBranch) - cs.Stub("") // git show-ref --verify (determineTrackingBranch) - cs.Stub("") // git status - cs.Stub("") // git log - - _, err := runCommand(http, nil, "feature", true, "-w") - - eq(t, err.Error(), "could not compute title or body defaults: could not find any commits between origin/master and feature") -} - -func TestPRCreate_defaults_error_interactive(t *testing.T) { - http := initFakeHTTP() - defer http.Verify(t) - - http.StubRepoResponse("OWNER", "REPO") - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "forks": { "nodes": [ - ] } } } } - `)) - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "createPullRequest": { "pullRequest": { - "URL": "https://github.com/OWNER/REPO/pull/12" - } } } } - `)) - - cs, cmdTeardown := test.InitCmdStubber() - defer cmdTeardown() - - cs.Stub("") // git config --get-regexp (determineTrackingBranch) - cs.Stub("") // git show-ref --verify (determineTrackingBranch) - cs.Stub("") // git status - cs.Stub("") // git log - cs.Stub("") // git rev-parse - cs.Stub("") // git push - cs.Stub("") // browser open - - as, surveyTeardown := prompt.InitAskStubber() - defer surveyTeardown() - - as.Stub([]*prompt.QuestionStub{ - { - Name: "title", - Default: true, - }, - { - Name: "body", - Value: "social distancing", - }, - }) - as.Stub([]*prompt.QuestionStub{ - { - Name: "confirmation", - Value: 1, - }, - }) - - output, err := runCommand(http, nil, "feature", true, ``) - require.NoError(t, err) - - stderr := string(output.Stderr()) - eq(t, strings.Contains(stderr, "warning: could not compute title or body defaults: could not find any commits"), true) -} - func Test_determineTrackingBranch_empty(t *testing.T) { cs, cmdTeardown := test.InitCmdStubber() defer cmdTeardown() diff --git a/pkg/httpmock/legacy.go b/pkg/httpmock/legacy.go index 0d42005d6..9b5d5afef 100644 --- a/pkg/httpmock/legacy.go +++ b/pkg/httpmock/legacy.go @@ -48,6 +48,22 @@ func (r *Registry) StubWithFixturePath(status int, fixturePath string) func() { } } +func (r *Registry) StubRepoInfoResponse(owner, repo, branch string) { + r.Register( + GraphQL(`query RepositoryInfo\b`), + StringResponse(fmt.Sprintf(` + { "data": { "repository": { + "id": "REPOID", + "name": "%s", + "owner": {"login": "%s"}, + "description": "", + "defaultBranchRef": {"name": "%s"}, + "hasIssuesEnabled": true, + "viewerPermission": "WRITE" + } } } + `, repo, owner, branch))) +} + func (r *Registry) StubRepoResponse(owner, repo string) { r.StubRepoResponseWithPermission(owner, repo, "WRITE") } From f9239661f2083c4785e37a6ac543bf7782b40629 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 16 Sep 2020 14:48:53 +0200 Subject: [PATCH 55/67] Reuse the StubRepoInfoReponse test helper --- pkg/cmd/issue/create/create_test.go | 10 +--------- pkg/cmd/pr/checkout/checkout_test.go | 6 +----- pkg/cmd/repo/create/create_test.go | 9 +-------- pkg/cmd/repo/view/view_test.go | 4 +--- 4 files changed, 4 insertions(+), 25 deletions(-) diff --git a/pkg/cmd/issue/create/create_test.go b/pkg/cmd/issue/create/create_test.go index ffa5d3365..2216f5b1d 100644 --- a/pkg/cmd/issue/create/create_test.go +++ b/pkg/cmd/issue/create/create_test.go @@ -191,15 +191,7 @@ func TestIssueCreate_metadata(t *testing.T) { http := &httpmock.Registry{} defer http.Verify(t) - http.Register( - httpmock.GraphQL(`query RepositoryInfo\b`), - httpmock.StringResponse(` - { "data": { "repository": { - "id": "REPOID", - "hasIssuesEnabled": true, - "viewerPermission": "WRITE" - } } } - `)) + http.StubRepoInfoResponse("OWNER", "REPO", "main") http.Register( httpmock.GraphQL(`query RepositoryResolveMetadataIDs\b`), httpmock.StringResponse(` diff --git a/pkg/cmd/pr/checkout/checkout_test.go b/pkg/cmd/pr/checkout/checkout_test.go index c49fef7e0..ea9926018 100644 --- a/pkg/cmd/pr/checkout/checkout_test.go +++ b/pkg/cmd/pr/checkout/checkout_test.go @@ -198,11 +198,7 @@ func TestPRCheckout_urlArg_differentBase(t *testing.T) { "maintainerCanModify": false } } } } `)) - http.Register(httpmock.GraphQL(`query RepositoryInfo\b`), httpmock.StringResponse(` - { "data": { "repository": { - "defaultBranchRef": {"name": "master"} - } } } - `)) + http.StubRepoInfoResponse("OWNER", "REPO", "master") ranCommands := [][]string{} restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { diff --git a/pkg/cmd/repo/create/create_test.go b/pkg/cmd/repo/create/create_test.go index 1fd99c035..303119f36 100644 --- a/pkg/cmd/repo/create/create_test.go +++ b/pkg/cmd/repo/create/create_test.go @@ -322,14 +322,7 @@ func TestRepoCreate_template(t *testing.T) { } } } }`)) - reg.Register( - httpmock.GraphQL(`query RepositoryInfo\b`), - httpmock.StringResponse(` - { "data": { - "repository": { - "id": "REPOID", - "description": "DESCRIPTION" - } } }`)) + reg.StubRepoInfoResponse("OWNER", "REPO", "main") reg.Register( httpmock.GraphQL(`query UserCurrent\b`), diff --git a/pkg/cmd/repo/view/view_test.go b/pkg/cmd/repo/view/view_test.go index 626ff62ed..55e8ace6b 100644 --- a/pkg/cmd/repo/view/view_test.go +++ b/pkg/cmd/repo/view/view_test.go @@ -104,9 +104,7 @@ func Test_RepoView_Web(t *testing.T) { for _, tt := range tests { reg := &httpmock.Registry{} - reg.Register( - httpmock.GraphQL(`query RepositoryInfo\b`), - httpmock.StringResponse(`{}`)) + reg.StubRepoInfoResponse("OWNER", "REPO", "main") opts := &ViewOptions{ Web: true, From f99a55438fcd47af2a143e3974d02a08a396aaac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 16 Sep 2020 15:48:13 +0200 Subject: [PATCH 56/67] Remove `issue create` examples from `pr create` --- pkg/cmd/pr/create/create.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkg/cmd/pr/create/create.go b/pkg/cmd/pr/create/create.go index 3bffbb1d7..3259b5561 100644 --- a/pkg/cmd/pr/create/create.go +++ b/pkg/cmd/pr/create/create.go @@ -75,8 +75,6 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co `), Example: heredoc.Doc(` $ gh pr create --title "The bug is fixed" --body "Everything works again" - $ gh issue create --label "bug,help wanted" - $ gh issue create --label bug --label "help wanted" $ gh pr create --reviewer monalisa,hubot $ gh pr create --project "Roadmap" $ gh pr create --base develop --head monalisa:feature From dd1c24a20aa04ad94df62807bc78c7768012d171 Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Wed, 16 Sep 2020 15:05:42 +0200 Subject: [PATCH 57/67] Use `LookPath` to determine if `xdg-open` exists --- pkg/browser/browser.go | 24 +++++++----------------- pkg/browser/browser_test.go | 20 ++++++++++++++++---- 2 files changed, 23 insertions(+), 21 deletions(-) diff --git a/pkg/browser/browser.go b/pkg/browser/browser.go index a91bae302..6496966a4 100644 --- a/pkg/browser/browser.go +++ b/pkg/browser/browser.go @@ -1,7 +1,6 @@ package browser import ( - "bufio" "os" "os/exec" "runtime" @@ -31,10 +30,10 @@ func ForOS(goos, url string) *exec.Cmd { r := strings.NewReplacer("&", "^&") args = append(args, "/c", "start", r.Replace(url)) default: - if inWsl() { - exe = "wslview" - } else { + if findExe("xdg-open") { exe = "xdg-open" + } else { + exe = "wslview" } args = append(args, url) } @@ -57,20 +56,11 @@ func FromLauncher(launcher, url string) (*exec.Cmd, error) { return cmd, nil } -// Determine if gh is running inside of WSL2 -func inWsl() bool { - file, err := os.Open("/proc/sys/kernel/osrelease") +var findExe = func(command string) bool { + _, err := exec.LookPath(command) if err != nil { return false + } else { + return true } - defer file.Close() - - scanner := bufio.NewScanner(file) - scanner.Scan() // Read single line - if err := scanner.Err(); err != nil { - return false - } - - os := scanner.Text() - return strings.Contains(os, "microsoft-WSL2") } diff --git a/pkg/browser/browser_test.go b/pkg/browser/browser_test.go index 749d0693e..dd007f950 100644 --- a/pkg/browser/browser_test.go +++ b/pkg/browser/browser_test.go @@ -11,9 +11,10 @@ func TestForOS(t *testing.T) { url string } tests := []struct { - name string - args args - want []string + name string + args args + findExe bool + want []string }{ { name: "macOS", @@ -29,7 +30,17 @@ func TestForOS(t *testing.T) { goos: "linux", url: "https://example.com/path?a=1&b=2", }, - want: []string{"xdg-open", "https://example.com/path?a=1&b=2"}, + findExe: true, + want: []string{"xdg-open", "https://example.com/path?a=1&b=2"}, + }, + { + name: "WSL", + args: args{ + goos: "linux", + url: "https://example.com/path?a=1&b=2", + }, + findExe: false, + want: []string{"wslview", "https://example.com/path?a=1&b=2"}, }, { name: "Windows", @@ -42,6 +53,7 @@ func TestForOS(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + findExe = func(string) bool { return tt.findExe } if cmd := ForOS(tt.args.goos, tt.args.url); !reflect.DeepEqual(cmd.Args, tt.want) { t.Errorf("ForOS() = %v, want %v", cmd.Args, tt.want) } From 7551139caf20e7dbfb8f0b55061ad1bf7d65e766 Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Wed, 16 Sep 2020 16:04:58 +0200 Subject: [PATCH 58/67] Address PR comments --- pkg/browser/browser.go | 12 ++++-------- pkg/browser/browser_test.go | 4 ++-- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/pkg/browser/browser.go b/pkg/browser/browser.go index 6496966a4..d972542f6 100644 --- a/pkg/browser/browser.go +++ b/pkg/browser/browser.go @@ -30,10 +30,10 @@ func ForOS(goos, url string) *exec.Cmd { r := strings.NewReplacer("&", "^&") args = append(args, "/c", "start", r.Replace(url)) default: - if findExe("xdg-open") { - exe = "xdg-open" - } else { + if findExe("wslview") { exe = "wslview" + } else { + exe = "xdg-open" } args = append(args, url) } @@ -58,9 +58,5 @@ func FromLauncher(launcher, url string) (*exec.Cmd, error) { var findExe = func(command string) bool { _, err := exec.LookPath(command) - if err != nil { - return false - } else { - return true - } + return err == nil } diff --git a/pkg/browser/browser_test.go b/pkg/browser/browser_test.go index dd007f950..97e6b1a13 100644 --- a/pkg/browser/browser_test.go +++ b/pkg/browser/browser_test.go @@ -30,7 +30,7 @@ func TestForOS(t *testing.T) { goos: "linux", url: "https://example.com/path?a=1&b=2", }, - findExe: true, + findExe: false, // wslview does not exist on standard Linux want: []string{"xdg-open", "https://example.com/path?a=1&b=2"}, }, { @@ -39,7 +39,7 @@ func TestForOS(t *testing.T) { goos: "linux", url: "https://example.com/path?a=1&b=2", }, - findExe: false, + findExe: true, // wslview exists on WSL want: []string{"wslview", "https://example.com/path?a=1&b=2"}, }, { From c643778701240f7ab56b598cf1916419721b8f0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 16 Sep 2020 16:10:53 +0200 Subject: [PATCH 59/67] Have only one test assert default config rendering --- internal/config/config_file_test.go | 4 ++-- internal/config/config_type_test.go | 17 ++++++++++++++--- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/internal/config/config_file_test.go b/internal/config/config_file_test.go index 1eb97ab10..f40cb9097 100644 --- a/internal/config/config_file_test.go +++ b/internal/config/config_file_test.go @@ -110,14 +110,14 @@ github.com: _, err := ParseConfig("config.yml") assert.Nil(t, err) - expectedMain := "# What protocol to use when performing git operations. Supported values: ssh, https\ngit_protocol: https\n# What editor gh should run when creating issues, pull requests, etc. If blank, will refer to environment.\neditor:\n# When to interactively prompt. This is a global config that cannot be overriden by hostname. Supported values: enabled, disabled\nprompt: enabled\n# Aliases allow you to create nicknames for gh commands\naliases:\n co: pr checkout\n" expectedHosts := `github.com: user: keiyuri oauth_token: "123456" ` - assert.Equal(t, expectedMain, mainBuf.String()) assert.Equal(t, expectedHosts, hostsBuf.String()) + assert.NotContains(t, mainBuf.String(), "github.com") + assert.NotContains(t, mainBuf.String(), "oauth_token") } func Test_parseConfigFile(t *testing.T) { diff --git a/internal/config/config_type_test.go b/internal/config/config_type_test.go index 6e561f634..1e1122c78 100644 --- a/internal/config/config_type_test.go +++ b/internal/config/config_type_test.go @@ -4,6 +4,7 @@ import ( "bytes" "testing" + "github.com/MakeNowJust/heredoc" "github.com/stretchr/testify/assert" ) @@ -19,8 +20,8 @@ func Test_fileConfig_Set(t *testing.T) { assert.NoError(t, c.Set("github.com", "user", "hubot")) assert.NoError(t, c.Write()) - expected := "# What protocol to use when performing git operations. Supported values: ssh, https\ngit_protocol: https\n# What editor gh should run when creating issues, pull requests, etc. If blank, will refer to environment.\neditor: nano\n# When to interactively prompt. This is a global config that cannot be overriden by hostname. Supported values: enabled, disabled\nprompt: enabled\n# Aliases allow you to create nicknames for gh commands\naliases:\n co: pr checkout\n" - assert.Equal(t, expected, mainBuf.String()) + assert.Contains(t, mainBuf.String(), "editor: nano") + assert.Contains(t, mainBuf.String(), "git_protocol: https") assert.Equal(t, `github.com: git_protocol: ssh user: hubot @@ -37,7 +38,17 @@ func Test_defaultConfig(t *testing.T) { cfg := NewBlankConfig() assert.NoError(t, cfg.Write()) - expected := "# What protocol to use when performing git operations. Supported values: ssh, https\ngit_protocol: https\n# What editor gh should run when creating issues, pull requests, etc. If blank, will refer to environment.\neditor:\n# When to interactively prompt. This is a global config that cannot be overriden by hostname. Supported values: enabled, disabled\nprompt: enabled\n# Aliases allow you to create nicknames for gh commands\naliases:\n co: pr checkout\n" + expected := heredoc.Doc(` + # What protocol to use when performing git operations. Supported values: ssh, https + git_protocol: https + # What editor gh should run when creating issues, pull requests, etc. If blank, will refer to environment. + editor: + # When to interactively prompt. This is a global config that cannot be overriden by hostname. Supported values: enabled, disabled + prompt: enabled + # Aliases allow you to create nicknames for gh commands + aliases: + co: pr checkout + `) assert.Equal(t, expected, mainBuf.String()) assert.Equal(t, "", hostsBuf.String()) From f6dd1bcd0af163eff61ca7002965e9f0126b5def Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 16 Sep 2020 16:15:37 +0200 Subject: [PATCH 60/67] Add the `pager` config option --- cmd/gh/main.go | 8 +++++--- internal/config/config_type.go | 9 +++++++++ internal/config/config_type_test.go | 2 ++ pkg/iostreams/iostreams.go | 4 ++++ 4 files changed, 20 insertions(+), 3 deletions(-) diff --git a/cmd/gh/main.go b/cmd/gh/main.go index fdde7acd3..73b153ffa 100644 --- a/cmd/gh/main.go +++ b/cmd/gh/main.go @@ -51,12 +51,14 @@ func main() { os.Exit(2) } - prompt, _ := cfg.Get("", "prompt") - - if prompt == config.PromptsDisabled { + if prompt, _ := cfg.Get("", "prompt"); prompt == config.PromptsDisabled { cmdFactory.IOStreams.SetNeverPrompt(true) } + if pager, _ := cfg.Get("", "pager"); pager != "" { + cmdFactory.IOStreams.SetPager(pager) + } + expandedArgs := []string{} if len(os.Args) > 0 { expandedArgs = os.Args[1:] diff --git a/internal/config/config_type.go b/internal/config/config_type.go index 740955539..2ff23e58c 100644 --- a/internal/config/config_type.go +++ b/internal/config/config_type.go @@ -180,6 +180,15 @@ func NewBlankRoot() *yaml.Node { Kind: yaml.ScalarNode, Value: PromptsEnabled, }, + { + HeadComment: "A pager program to send command output to. Example value: less", + Kind: yaml.ScalarNode, + Value: "pager", + }, + { + Kind: yaml.ScalarNode, + Value: "", + }, { HeadComment: "Aliases allow you to create nicknames for gh commands", Kind: yaml.ScalarNode, diff --git a/internal/config/config_type_test.go b/internal/config/config_type_test.go index 1e1122c78..50e2b9f5a 100644 --- a/internal/config/config_type_test.go +++ b/internal/config/config_type_test.go @@ -45,6 +45,8 @@ func Test_defaultConfig(t *testing.T) { editor: # When to interactively prompt. This is a global config that cannot be overriden by hostname. Supported values: enabled, disabled prompt: enabled + # A pager program to send command output to. Example value: less + pager: # Aliases allow you to create nicknames for gh commands aliases: co: pr checkout diff --git a/pkg/iostreams/iostreams.go b/pkg/iostreams/iostreams.go index e3694dfe5..d6c8ae48f 100644 --- a/pkg/iostreams/iostreams.go +++ b/pkg/iostreams/iostreams.go @@ -92,6 +92,10 @@ func (s *IOStreams) IsStderrTTY() bool { return false } +func (s *IOStreams) SetPager(cmd string) { + s.pagerCommand = cmd +} + func (s *IOStreams) StartPager() error { if s.pagerCommand == "" || !s.IsStdoutTTY() { return nil From fa2513dea6230b6eb244b2c95029db51012a9c7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 16 Sep 2020 16:15:57 +0200 Subject: [PATCH 61/67] Document PAGER environment variable --- pkg/cmd/root/help_topic.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/cmd/root/help_topic.go b/pkg/cmd/root/help_topic.go index 1357825a2..a1acf5a40 100644 --- a/pkg/cmd/root/help_topic.go +++ b/pkg/cmd/root/help_topic.go @@ -28,6 +28,8 @@ func NewHelpTopic(topic string) *cobra.Command { DEBUG: set to any value to enable verbose output to standard error. Include values "api" or "oauth" to print detailed information about HTTP requests or authentication flow. + PAGER: a paging program to send standard output to, e.g. "less". + GLAMOUR_STYLE: the style to use for rendering Markdown. See https://github.com/charmbracelet/glamour#styles From fd1d09dfc21512e5eb69d762de4d61f2afac96ae Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Wed, 16 Sep 2020 16:56:47 +0200 Subject: [PATCH 62/67] Clean up linux logic to have better default logic --- pkg/browser/browser.go | 16 ++++++++++------ pkg/browser/browser_test.go | 18 +++++++++--------- 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/pkg/browser/browser.go b/pkg/browser/browser.go index d972542f6..2408a2e08 100644 --- a/pkg/browser/browser.go +++ b/pkg/browser/browser.go @@ -30,11 +30,7 @@ func ForOS(goos, url string) *exec.Cmd { r := strings.NewReplacer("&", "^&") args = append(args, "/c", "start", r.Replace(url)) default: - if findExe("wslview") { - exe = "wslview" - } else { - exe = "xdg-open" - } + exe = linuxExe() args = append(args, url) } @@ -56,7 +52,15 @@ func FromLauncher(launcher, url string) (*exec.Cmd, error) { return cmd, nil } -var findExe = func(command string) bool { +var linuxExe = func() string { + exe := "xdg-open" + if !findExe("xdg-open") && findExe("wslview") { + exe = "wslview" + } + return exe +} + +func findExe(command string) bool { _, err := exec.LookPath(command) return err == nil } diff --git a/pkg/browser/browser_test.go b/pkg/browser/browser_test.go index 97e6b1a13..11dd80372 100644 --- a/pkg/browser/browser_test.go +++ b/pkg/browser/browser_test.go @@ -11,10 +11,10 @@ func TestForOS(t *testing.T) { url string } tests := []struct { - name string - args args - findExe bool - want []string + name string + args args + exe string + want []string }{ { name: "macOS", @@ -30,8 +30,8 @@ func TestForOS(t *testing.T) { goos: "linux", url: "https://example.com/path?a=1&b=2", }, - findExe: false, // wslview does not exist on standard Linux - want: []string{"xdg-open", "https://example.com/path?a=1&b=2"}, + exe: "xdg-open", + want: []string{"xdg-open", "https://example.com/path?a=1&b=2"}, }, { name: "WSL", @@ -39,8 +39,8 @@ func TestForOS(t *testing.T) { goos: "linux", url: "https://example.com/path?a=1&b=2", }, - findExe: true, // wslview exists on WSL - want: []string{"wslview", "https://example.com/path?a=1&b=2"}, + exe: "wslview", + want: []string{"wslview", "https://example.com/path?a=1&b=2"}, }, { name: "Windows", @@ -53,7 +53,7 @@ func TestForOS(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - findExe = func(string) bool { return tt.findExe } + linuxExe = func() string { return tt.exe } if cmd := ForOS(tt.args.goos, tt.args.url); !reflect.DeepEqual(cmd.Args, tt.want) { t.Errorf("ForOS() = %v, want %v", cmd.Args, tt.want) } From 9058feebec47d343a42d45e29f058ad4c6b9a63b Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Wed, 16 Sep 2020 17:19:11 +0200 Subject: [PATCH 63/67] Fix up structure for better testing --- pkg/browser/browser.go | 17 ++++++++++------- pkg/browser/browser_test.go | 10 +++++++++- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/pkg/browser/browser.go b/pkg/browser/browser.go index 2408a2e08..c710a3b38 100644 --- a/pkg/browser/browser.go +++ b/pkg/browser/browser.go @@ -52,15 +52,18 @@ func FromLauncher(launcher, url string) (*exec.Cmd, error) { return cmd, nil } -var linuxExe = func() string { +func linuxExe() string { exe := "xdg-open" - if !findExe("xdg-open") && findExe("wslview") { - exe = "wslview" + + _, err := lookPath(exe) + if err != nil { + _, err := lookPath("wslview") + if err == nil { + exe = "wslview" + } } + return exe } -func findExe(command string) bool { - _, err := exec.LookPath(command) - return err == nil -} +var lookPath = exec.LookPath diff --git a/pkg/browser/browser_test.go b/pkg/browser/browser_test.go index 11dd80372..48b91f7c1 100644 --- a/pkg/browser/browser_test.go +++ b/pkg/browser/browser_test.go @@ -1,6 +1,7 @@ package browser import ( + "errors" "reflect" "testing" ) @@ -52,8 +53,15 @@ func TestForOS(t *testing.T) { }, } for _, tt := range tests { + lookPath = func(file string) (string, error) { + if file == tt.exe { + return file, nil + } else { + return "", errors.New("not found") + } + } + t.Run(tt.name, func(t *testing.T) { - linuxExe = func() string { return tt.exe } if cmd := ForOS(tt.args.goos, tt.args.url); !reflect.DeepEqual(cmd.Args, tt.want) { t.Errorf("ForOS() = %v, want %v", cmd.Args, tt.want) } From 88aacc14f0e05a448e018bb9d00082c535d6871b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 16 Sep 2020 17:50:31 +0200 Subject: [PATCH 64/67] Fix gray color presentation across terminals We used to send the ANSI sequence for "bright black" when we wanted gray, but this color turns out to not be visible in some popular color schemes. Instead, when we detect a 256-color terminal, switch to displaying a color sequence for gray that is consistent and does not depend on terminal color scheme. --- pkg/iostreams/color.go | 31 ++++++++++++++++++++++++++++--- pkg/iostreams/iostreams.go | 8 +++++++- utils/color.go | 4 ++++ 3 files changed, 39 insertions(+), 4 deletions(-) diff --git a/pkg/iostreams/color.go b/pkg/iostreams/color.go index 92f4f31ae..2efe93c8b 100644 --- a/pkg/iostreams/color.go +++ b/pkg/iostreams/color.go @@ -1,7 +1,9 @@ package iostreams import ( + "fmt" "os" + "strings" "github.com/mgutz/ansi" ) @@ -15,6 +17,10 @@ var ( green = ansi.ColorFunc("green") gray = ansi.ColorFunc("black+h") bold = ansi.ColorFunc("default+b") + + gray256 = func(t string) string { + return fmt.Sprintf("\x1b[%d;5;%dm%s\x1b[m", 38, 242, t) + } ) func EnvColorDisabled() bool { @@ -25,12 +31,28 @@ func EnvColorForced() bool { return os.Getenv("CLICOLOR_FORCE") != "" && os.Getenv("CLICOLOR_FORCE") != "0" } -func NewColorScheme(enabled bool) *ColorScheme { - return &ColorScheme{enabled: enabled} +func Is256ColorSupported() bool { + term := os.Getenv("TERM") + colorterm := os.Getenv("COLORTERM") + + return strings.Contains(term, "256") || + strings.Contains(term, "24bit") || + strings.Contains(term, "truecolor") || + strings.Contains(colorterm, "256") || + strings.Contains(colorterm, "24bit") || + strings.Contains(colorterm, "truecolor") +} + +func NewColorScheme(enabled, is256enabled bool) *ColorScheme { + return &ColorScheme{ + enabled: enabled, + is256enabled: is256enabled, + } } type ColorScheme struct { - enabled bool + enabled bool + is256enabled bool } func (c *ColorScheme) Bold(t string) string { @@ -65,6 +87,9 @@ func (c *ColorScheme) Gray(t string) string { if !c.enabled { return t } + if c.is256enabled { + return gray256(t) + } return gray(t) } diff --git a/pkg/iostreams/iostreams.go b/pkg/iostreams/iostreams.go index f4ed4b032..968f06e06 100644 --- a/pkg/iostreams/iostreams.go +++ b/pkg/iostreams/iostreams.go @@ -26,6 +26,7 @@ type IOStreams struct { // the original (non-colorable) output stream originalOut io.Writer colorEnabled bool + is256enabled bool progressIndicatorEnabled bool progressIndicator *spinner.Spinner @@ -47,6 +48,10 @@ func (s *IOStreams) ColorEnabled() bool { return s.colorEnabled } +func (s *IOStreams) ColorSupport256() bool { + return s.is256enabled +} + func (s *IOStreams) SetStdinTTY(isTTY bool) { s.stdinTTYOverride = true s.stdinIsTTY = isTTY @@ -200,7 +205,7 @@ func (s *IOStreams) TerminalWidth() int { } func (s *IOStreams) ColorScheme() *ColorScheme { - return NewColorScheme(s.ColorEnabled()) + return NewColorScheme(s.ColorEnabled(), s.ColorSupport256()) } func System() *IOStreams { @@ -213,6 +218,7 @@ func System() *IOStreams { Out: colorable.NewColorable(os.Stdout), ErrOut: colorable.NewColorable(os.Stderr), colorEnabled: EnvColorForced() || (!EnvColorDisabled() && stdoutIsTTY), + is256enabled: Is256ColorSupported(), pagerCommand: os.Getenv("PAGER"), } diff --git a/utils/color.go b/utils/color.go index 0b276bf11..3e875acc4 100644 --- a/utils/color.go +++ b/utils/color.go @@ -1,6 +1,7 @@ package utils import ( + "fmt" "io" "os" @@ -33,6 +34,9 @@ func makeColorFunc(color string) func(string) string { cf := ansi.ColorFunc(color) return func(arg string) string { if isColorEnabled() { + if color == "black+h" && iostreams.Is256ColorSupported() { + return fmt.Sprintf("\x1b[%d;5;%dm%s\x1b[m", 38, 242, arg) + } return cf(arg) } return arg From b2de27c6241cba458c6177478ca5b3af5833fbc5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 16 Sep 2020 17:52:25 +0200 Subject: [PATCH 65/67] Fix Survey's presentation of default values For default values for e.g. `Input` prompts, Survey uses the literal "white" color, which makes no sense on dark terminals and is literally invisible on light backgrounds. This overrides Survey to output a gray color for 256-color terminals and "default" for basic terminals. --- cmd/gh/main.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/cmd/gh/main.go b/cmd/gh/main.go index 71b5fd149..db329c3cd 100644 --- a/cmd/gh/main.go +++ b/cmd/gh/main.go @@ -46,6 +46,19 @@ func main() { stderr := cmdFactory.IOStreams.ErrOut if !cmdFactory.IOStreams.ColorEnabled() { surveyCore.DisableColor = true + } else { + // override survey's poor choice of color + surveyCore.TemplateFuncsWithColor["color"] = func(style string) string { + switch style { + case "white": + if cmdFactory.IOStreams.ColorSupport256() { + return fmt.Sprintf("\x1b[%d;5;%dm", 38, 242) + } + return ansi.ColorCode("default") + default: + return ansi.ColorCode(style) + } + } } rootCmd := root.NewCmdRoot(cmdFactory, command.Version, command.BuildDate) From 2b70e8266a7d20e3e2f14db24bd4203ad06b43b8 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Wed, 16 Sep 2020 10:57:20 -0500 Subject: [PATCH 66/67] better time stub --- pkg/cmd/gist/list/list.go | 7 +------ pkg/cmd/gist/list/list_test.go | 35 ++++++++++++++++++++-------------- 2 files changed, 22 insertions(+), 20 deletions(-) diff --git a/pkg/cmd/gist/list/list.go b/pkg/cmd/gist/list/list.go index 5e22e267b..e1142bdb7 100644 --- a/pkg/cmd/gist/list/list.go +++ b/pkg/cmd/gist/list/list.go @@ -17,8 +17,6 @@ type ListOptions struct { IO *iostreams.IOStreams HttpClient func() (*http.Client, error) - Since func(t time.Time) time.Duration - Limit int Visibility string // all, secret, public } @@ -27,9 +25,6 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman opts := &ListOptions{ IO: f.IOStreams, HttpClient: f.HttpClient, - Since: func(t time.Time) time.Duration { - return time.Since(t) - }, } cmd := &cobra.Command{ @@ -109,7 +104,7 @@ func listRun(opts *ListOptions) error { tp.AddField(utils.Pluralize(fileCount, "file"), nil, nil) tp.AddField(visibility, nil, visColor) if tp.IsTTY() { - updatedAt := utils.FuzzyAgo(opts.Since(gist.UpdatedAt)) + updatedAt := utils.FuzzyAgo(time.Since(gist.UpdatedAt)) tp.AddField(updatedAt, nil, cs.Gray) } else { tp.AddField(gist.UpdatedAt.String(), nil, nil) diff --git a/pkg/cmd/gist/list/list_test.go b/pkg/cmd/gist/list/list_test.go index 26fa46114..497f68edc 100644 --- a/pkg/cmd/gist/list/list_test.go +++ b/pkg/cmd/gist/list/list_test.go @@ -89,11 +89,12 @@ func TestNewCmdList(t *testing.T) { func Test_listRun(t *testing.T) { tests := []struct { - name string - opts *ListOptions - wantOut string - stubs func(*httpmock.Registry) - nontty bool + name string + opts *ListOptions + wantOut string + stubs func(*httpmock.Registry) + nontty bool + updatedAt *time.Time }{ { name: "no gists", @@ -126,20 +127,28 @@ func Test_listRun(t *testing.T) { wantOut: "1234567890 cool.txt 1 file public about 6 hours ago\n", }, { - name: "nontty output", - opts: &ListOptions{}, - wantOut: "1234567890\tcool.txt\t1 file\tpublic\t0001-01-01 00:00:00 +0000 UTC\n4567890123\t\t1 file\tpublic\t0001-01-01 00:00:00 +0000 UTC\n2345678901\ttea leaves thwart those who court catastrophe\t2 files\tsecret\t0001-01-01 00:00:00 +0000 UTC\n3456789012\tshort desc\t11 files\tsecret\t0001-01-01 00:00:00 +0000 UTC\n", - nontty: true, + name: "nontty output", + opts: &ListOptions{}, + updatedAt: &time.Time{}, + wantOut: "1234567890\tcool.txt\t1 file\tpublic\t0001-01-01 00:00:00 +0000 UTC\n4567890123\t\t1 file\tpublic\t0001-01-01 00:00:00 +0000 UTC\n2345678901\ttea leaves thwart those who court catastrophe\t2 files\tsecret\t0001-01-01 00:00:00 +0000 UTC\n3456789012\tshort desc\t11 files\tsecret\t0001-01-01 00:00:00 +0000 UTC\n", + nontty: true, }, } for _, tt := range tests { + sixHoursAgo, _ := time.ParseDuration("-6h") + updatedAt := time.Now().Add(sixHoursAgo) + if tt.updatedAt != nil { + updatedAt = *tt.updatedAt + } + reg := &httpmock.Registry{} if tt.stubs == nil { reg.Register(httpmock.REST("GET", "gists"), httpmock.JSONResponse([]shared.Gist{ { ID: "1234567890", + UpdatedAt: updatedAt, Description: "", Files: map[string]*shared.GistFile{ "cool.txt": {}, @@ -148,6 +157,7 @@ func Test_listRun(t *testing.T) { }, { ID: "4567890123", + UpdatedAt: updatedAt, Description: "", Files: map[string]*shared.GistFile{ "gistfile0.txt": {}, @@ -156,6 +166,7 @@ func Test_listRun(t *testing.T) { }, { ID: "2345678901", + UpdatedAt: updatedAt, Description: "tea leaves thwart those who court catastrophe", Files: map[string]*shared.GistFile{ "gistfile0.txt": {}, @@ -165,6 +176,7 @@ func Test_listRun(t *testing.T) { }, { ID: "3456789012", + UpdatedAt: updatedAt, Description: "short desc", Files: map[string]*shared.GistFile{ "gistfile0.txt": {}, @@ -190,11 +202,6 @@ func Test_listRun(t *testing.T) { return &http.Client{Transport: reg}, nil } - tt.opts.Since = func(t time.Time) time.Duration { - d, _ := time.ParseDuration("6h") - return d - } - io, _, stdout, _ := iostreams.Test() io.SetStdoutTTY(!tt.nontty) tt.opts.IO = io From 7c986c0454690616b207313f7c2f58fd8343bad9 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Wed, 16 Sep 2020 11:23:14 -0500 Subject: [PATCH 67/67] help typo --- pkg/cmd/gist/gist.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/gist/gist.go b/pkg/cmd/gist/gist.go index 6c86cc431..029fab588 100644 --- a/pkg/cmd/gist/gist.go +++ b/pkg/cmd/gist/gist.go @@ -18,9 +18,9 @@ func NewCmdGist(f *cmdutil.Factory) *cobra.Command { Annotations: map[string]string{ "IsCore": "true", "help:arguments": heredoc.Doc(` - A gist can be supplied as argument in any of the following formats: + A gist can be supplied as argument in either of the following formats: - by ID, e.g. 5b0e0062eb8e9654adad7bb1d81cc75f - - by URL, e.g. "https://gist.github.com/OWNER/5b0e0062eb8e9654adad7bb1d81cc75f"; or + - by URL, e.g. "https://gist.github.com/OWNER/5b0e0062eb8e9654adad7bb1d81cc75f" `), }, }