From 05a1a252712e529a385c850bab501590a0156526 Mon Sep 17 00:00:00 2001 From: Nate Smith Date: Fri, 20 Nov 2020 12:00:49 -0600 Subject: [PATCH 01/43] match parent repo protocol when forking (#2434) * match parent repo protocol when forking * guard against nil and prefer PushURL --- pkg/cmd/repo/fork/fork.go | 15 +++++++++ pkg/cmd/repo/fork/fork_test.go | 56 +++++++++++++++++++++++++++++++--- 2 files changed, 67 insertions(+), 4 deletions(-) diff --git a/pkg/cmd/repo/fork/fork.go b/pkg/cmd/repo/fork/fork.go index ebaf6c400..760bb6537 100644 --- a/pkg/cmd/repo/fork/fork.go +++ b/pkg/cmd/repo/fork/fork.go @@ -195,6 +195,21 @@ func forkRun(opts *ForkOptions) error { if err != nil { return err } + + if remote, err := remotes.FindByRepo(repoToFork.RepoOwner(), repoToFork.RepoName()); err == nil { + + scheme := "" + if remote.FetchURL != nil { + scheme = remote.FetchURL.Scheme + } + if remote.PushURL != nil { + scheme = remote.PushURL.Scheme + } + if scheme != "" { + protocol = scheme + } + } + if remote, err := remotes.FindByRepo(forkedRepo.RepoOwner(), forkedRepo.RepoName()); err == nil { if connectedToTerminal { fmt.Fprintf(stderr, "%s Using existing remote %s\n", cs.SuccessIcon(), cs.Bold(remote.Name)) diff --git a/pkg/cmd/repo/fork/fork_test.go b/pkg/cmd/repo/fork/fork_test.go index 0a92b77e5..20f396394 100644 --- a/pkg/cmd/repo/fork/fork_test.go +++ b/pkg/cmd/repo/fork/fork_test.go @@ -2,6 +2,7 @@ package fork import ( "net/http" + "net/url" "os/exec" "regexp" "strings" @@ -44,8 +45,11 @@ func runCommand(httpClient *http.Client, remotes []*context.Remote, isTTY bool, if remotes == nil { return []*context.Remote{ { - Remote: &git.Remote{Name: "origin"}, - Repo: ghrepo.New("OWNER", "REPO"), + Remote: &git.Remote{ + Name: "origin", + FetchURL: &url.URL{}, + }, + Repo: ghrepo.New("OWNER", "REPO"), }, }, nil } @@ -179,11 +183,11 @@ func TestRepoFork_reuseRemote(t *testing.T) { stubSpinner() remotes := []*context.Remote{ { - Remote: &git.Remote{Name: "origin"}, + Remote: &git.Remote{Name: "origin", FetchURL: &url.URL{}}, Repo: ghrepo.New("someone", "REPO"), }, { - Remote: &git.Remote{Name: "upstream"}, + Remote: &git.Remote{Name: "upstream", FetchURL: &url.URL{}}, Repo: ghrepo.New("OWNER", "REPO"), }, } @@ -465,6 +469,50 @@ func TestRepoFork_in_parent_survey_no(t *testing.T) { reg.Verify(t) } +func TestRepoFork_in_parent_match_protocol(t *testing.T) { + stubSpinner() + defer stubSince(2 * time.Second)() + reg := &httpmock.Registry{} + defer reg.StubWithFixturePath(200, "./forkResult.json")() + httpClient := &http.Client{Transport: reg} + + var seenCmds []*exec.Cmd + defer run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { + seenCmds = append(seenCmds, cmd) + return &test.OutputStub{} + })() + + remotes := []*context.Remote{ + { + Remote: &git.Remote{Name: "origin", PushURL: &url.URL{ + Scheme: "ssh", + }}, + Repo: ghrepo.New("OWNER", "REPO"), + }, + } + + output, err := runCommand(httpClient, remotes, true, "--remote") + if err != nil { + t.Errorf("error running command `repo fork`: %v", err) + } + + expectedCmds := []string{ + "git remote rename origin upstream", + "git remote add -f origin git@github.com:someone/REPO.git", + } + + for x, cmd := range seenCmds { + assert.Equal(t, expectedCmds[x], strings.Join(cmd.Args, " ")) + } + + assert.Equal(t, "", output.String()) + + test.ExpectLines(t, output.Stderr(), + "Created fork.*someone/REPO", + "Added remote.*origin") + reg.Verify(t) +} + func stubSpinner() { // not bothering with teardown since we never want spinners when doing tests utils.StartSpinner = func(_ *spinner.Spinner) { From e16bf094bdca3c8e5d0486f6e1a9f6e2c3f701c5 Mon Sep 17 00:00:00 2001 From: Ismael Luceno Date: Sat, 21 Nov 2020 23:00:14 +0100 Subject: [PATCH 02/43] Simplify CGO flags setup --- Makefile | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/Makefile b/Makefile index 859461984..05e792609 100644 --- a/Makefile +++ b/Makefile @@ -9,15 +9,12 @@ else BUILD_DATE ?= $(shell date "$(DATE_FMT)") endif -ifndef CGO_CPPFLAGS - export CGO_CPPFLAGS := $(CPPFLAGS) -endif -ifndef CGO_CFLAGS - export CGO_CFLAGS := $(CFLAGS) -endif -ifndef CGO_LDFLAGS - export CGO_LDFLAGS := $(LDFLAGS) -endif +CGO_CPPFLAGS ?= ${CPPFLAGS} +export CGO_CPPFLAGS +CGO_CFLAGS ?= ${CFLAGS} +export CGO_CFLAGS +CGO_LDFLAGS ?= ${LDFLAGS} +export CGO_LDFLAGS GO_LDFLAGS := -X github.com/cli/cli/internal/build.Version=$(GH_VERSION) $(GO_LDFLAGS) GO_LDFLAGS := -X github.com/cli/cli/internal/build.Date=$(BUILD_DATE) $(GO_LDFLAGS) From a4daf96a694b56e7945748e1d392dd746ec0de71 Mon Sep 17 00:00:00 2001 From: Ismael Luceno Date: Sat, 21 Nov 2020 23:00:45 +0100 Subject: [PATCH 03/43] Make bin/gh rule verbose --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 05e792609..dbafea1e4 100644 --- a/Makefile +++ b/Makefile @@ -24,7 +24,7 @@ ifdef GH_OAUTH_CLIENT_SECRET endif bin/gh: $(BUILD_FILES) - @go build -trimpath -ldflags "$(GO_LDFLAGS)" -o "$@" ./cmd/gh + go build -trimpath -ldflags "${GO_LDFLAGS}" -o "$@" ./cmd/gh clean: rm -rf ./bin ./share From 0e681ca6c4a43c6f33532917ef995d591375c348 Mon Sep 17 00:00:00 2001 From: Josh Soref Date: Sat, 21 Nov 2020 21:18:50 -0500 Subject: [PATCH 04/43] spelling: beginning --- pkg/cmd/pr/create/regexp_writer_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/cmd/pr/create/regexp_writer_test.go b/pkg/cmd/pr/create/regexp_writer_test.go index fd772a760..f9ec0b863 100644 --- a/pkg/cmd/pr/create/regexp_writer_test.go +++ b/pkg/cmd/pr/create/regexp_writer_test.go @@ -93,14 +93,14 @@ func Test_Write(t *testing.T) { { name: "multiple lines removed", input: input{ - in: []string{"begining line\nremove this whole line\nremove this one also\nnot this one"}, + in: []string{"beginning line\nremove this whole line\nremove this one also\nnot this one"}, re: regexp.MustCompile("(?s)^remove.*$"), repl: "", }, output: output{ wantsErr: false, - out: "begining line\nnot this one", - length: 70, + out: "beginning line\nnot this one", + length: 71, }, }, { From e58b2dbe92815222858af49ec0e168a2aae4a253 Mon Sep 17 00:00:00 2001 From: Josh Soref Date: Sat, 21 Nov 2020 21:18:50 -0500 Subject: [PATCH 05/43] spelling: chestnuts --- pkg/cmd/repo/create/http_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/cmd/repo/create/http_test.go b/pkg/cmd/repo/create/http_test.go index c8e2e7a2a..360403538 100644 --- a/pkg/cmd/repo/create/http_test.go +++ b/pkg/cmd/repo/create/http_test.go @@ -17,7 +17,7 @@ func Test_RepoCreate(t *testing.T) { reg.StubResponse(200, bytes.NewBufferString(`{}`)) input := repoCreateInput{ - Description: "roasted chesnuts", + Description: "roasted chestnuts", HomepageURL: "http://example.com", } @@ -39,8 +39,8 @@ func Test_RepoCreate(t *testing.T) { bodyBytes, _ := ioutil.ReadAll(reg.Requests[0].Body) _ = json.Unmarshal(bodyBytes, &reqBody) - if description := reqBody.Variables.Input["description"].(string); description != "roasted chesnuts" { - t.Errorf("expected description to be %q, got %q", "roasted chesnuts", description) + if description := reqBody.Variables.Input["description"].(string); description != "roasted chestnuts" { + t.Errorf("expected description to be %q, got %q", "roasted chestnuts", description) } if homepage := reqBody.Variables.Input["homepageUrl"].(string); homepage != "http://example.com" { t.Errorf("expected homepageUrl to be %q, got %q", "http://example.com", homepage) From 8ba68fc68ada852d8468753dd0a2ffb3c357a110 Mon Sep 17 00:00:00 2001 From: Josh Soref Date: Sat, 21 Nov 2020 21:18:50 -0500 Subject: [PATCH 06/43] spelling: deprecated --- internal/docs/man_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/docs/man_test.go b/internal/docs/man_test.go index 4e844ad61..58591e19e 100644 --- a/internal/docs/man_test.go +++ b/internal/docs/man_test.go @@ -106,7 +106,7 @@ func TestGenManSeeAlso(t *testing.T) { } } -func TestManPrintFlagsHidesShortDeperecated(t *testing.T) { +func TestManPrintFlagsHidesShortDeprecated(t *testing.T) { c := &cobra.Command{} c.Flags().StringP("foo", "f", "default", "Foo flag") _ = c.Flags().MarkShorthandDeprecated("foo", "don't use it no more") From ddd438d5e14ce696298f684e24e6933b3fe49548 Mon Sep 17 00:00:00 2001 From: Josh Soref Date: Sat, 21 Nov 2020 21:18:50 -0500 Subject: [PATCH 07/43] spelling: dismissed --- pkg/cmd/pr/view/view.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/pr/view/view.go b/pkg/cmd/pr/view/view.go index 1bb0eff1b..9dcdb8f8b 100644 --- a/pkg/cmd/pr/view/view.go +++ b/pkg/cmd/pr/view/view.go @@ -214,7 +214,7 @@ type reviewerState struct { func formattedReviewerState(cs *iostreams.ColorScheme, reviewer *reviewerState) string { state := reviewer.State if state == dismissedReviewState { - // Show "DISMISSED" review as "COMMENTED", since "dimissed" only makes + // Show "DISMISSED" review as "COMMENTED", since "dismissed" only makes // sense when displayed in an events timeline but not in the final tally. state = commentedReviewState } From 861d350440ca0ba866c4651351b29afb939124a2 Mon Sep 17 00:00:00 2001 From: Josh Soref Date: Sat, 21 Nov 2020 21:18:50 -0500 Subject: [PATCH 08/43] spelling: dunno --- pkg/cmd/pr/review/review_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/pr/review/review_test.go b/pkg/cmd/pr/review/review_test.go index b96ab8089..2184c264c 100644 --- a/pkg/cmd/pr/review/review_test.go +++ b/pkg/cmd/pr/review/review_test.go @@ -319,7 +319,7 @@ func TestPRReview(t *testing.T) { {`--request-changes -b"bad"`, "REQUEST_CHANGES", "bad"}, {`--approve`, "APPROVE", ""}, {`--approve -b"hot damn"`, "APPROVE", "hot damn"}, - {`--comment --body "i donno"`, "COMMENT", "i donno"}, + {`--comment --body "i dunno"`, "COMMENT", "i dunno"}, } for _, kase := range cases { From 76bd3772530f09de9b2d40f480c63a6d6427f410 Mon Sep 17 00:00:00 2001 From: Josh Soref Date: Sat, 21 Nov 2020 21:18:51 -0500 Subject: [PATCH 09/43] spelling: error --- 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 a3be5a27a..80b61341e 100644 --- a/pkg/cmd/root/help.go +++ b/pkg/cmd/root/help.go @@ -35,7 +35,7 @@ func rootUsageFunc(command *cobra.Command) error { return nil } -func rootFlagErrrorFunc(cmd *cobra.Command, err error) error { +func rootFlagErrorFunc(cmd *cobra.Command, err error) error { if err == pflag.ErrHelp { return err } diff --git a/pkg/cmd/root/root.go b/pkg/cmd/root/root.go index 93cb4431d..42c1b627f 100644 --- a/pkg/cmd/root/root.go +++ b/pkg/cmd/root/root.go @@ -59,7 +59,7 @@ func NewCmdRoot(f *cmdutil.Factory, version, buildDate string) *cobra.Command { cmd.PersistentFlags().Bool("help", false, "Show help for command") cmd.SetHelpFunc(helpHelper) cmd.SetUsageFunc(rootUsageFunc) - cmd.SetFlagErrorFunc(rootFlagErrrorFunc) + cmd.SetFlagErrorFunc(rootFlagErrorFunc) formattedVersion := versionCmd.Format(version, buildDate) cmd.SetVersionTemplate(formattedVersion) From c8b9486fd3ac6a1692965013d1685e66cb197bfe Mon Sep 17 00:00:00 2001 From: Josh Soref Date: Sat, 21 Nov 2020 21:18:51 -0500 Subject: [PATCH 10/43] spelling: nonexistent --- context/remote_test.go | 4 ++-- git/ssh_config_test.go | 2 +- internal/config/config_file_test.go | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/context/remote_test.go b/context/remote_test.go index ab3f7e2e2..1d2140c90 100644 --- a/context/remote_test.go +++ b/context/remote_test.go @@ -28,11 +28,11 @@ func Test_Remotes_FindByName(t *testing.T) { eq(t, err, nil) eq(t, r.Name, "upstream") - r, err = list.FindByName("nonexist", "*") + r, err = list.FindByName("nonexistent", "*") eq(t, err, nil) eq(t, r.Name, "mona") - _, err = list.FindByName("nonexist") + _, err = list.FindByName("nonexistent") eq(t, err, errors.New(`no GitHub remotes found`)) } diff --git a/git/ssh_config_test.go b/git/ssh_config_test.go index 28f339aa6..7aafc5b21 100644 --- a/git/ssh_config_test.go +++ b/git/ssh_config_test.go @@ -25,7 +25,7 @@ func Test_sshParse(t *testing.T) { `)) eq(t, m["foo"], "example.com") eq(t, m["bar"], "%bar.net%") - eq(t, m["nonexist"], "") + eq(t, m["nonexistent"], "") } func Test_Translator(t *testing.T) { diff --git a/internal/config/config_file_test.go b/internal/config/config_file_test.go index f40cb9097..cc6ba287f 100644 --- a/internal/config/config_file_test.go +++ b/internal/config/config_file_test.go @@ -90,7 +90,7 @@ example.com: val, err = config.Get("github.com", "git_protocol") eq(t, err, nil) eq(t, val, "ssh") - val, err = config.Get("nonexist.io", "git_protocol") + val, err = config.Get("nonexistent.io", "git_protocol") eq(t, err, nil) eq(t, val, "ssh") } From e5f59a15fe6de03de0c17b8ec1ed004bf8c5afc6 Mon Sep 17 00:00:00 2001 From: Josh Soref Date: Sat, 21 Nov 2020 21:18:51 -0500 Subject: [PATCH 11/43] spelling: response --- api/cache.go | 6 +++--- api/cache_test.go | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/api/cache.go b/api/cache.go index 620660c15..1f6d8896b 100644 --- a/api/cache.go +++ b/api/cache.go @@ -19,7 +19,7 @@ import ( func makeCachedClient(httpClient *http.Client, cacheTTL time.Duration) *http.Client { cacheDir := filepath.Join(os.TempDir(), "gh-cli-cache") return &http.Client{ - Transport: CacheReponse(cacheTTL, cacheDir)(httpClient.Transport), + Transport: CacheResponse(cacheTTL, cacheDir)(httpClient.Transport), } } @@ -39,8 +39,8 @@ func isCacheableResponse(res *http.Response) bool { return res.StatusCode < 500 && res.StatusCode != 403 } -// CacheReponse produces a RoundTripper that caches HTTP responses to disk for a specified amount of time -func CacheReponse(ttl time.Duration, dir string) ClientOption { +// CacheResponse produces a RoundTripper that caches HTTP responses to disk for a specified amount of time +func CacheResponse(ttl time.Duration, dir string) ClientOption { fs := fileStorage{ dir: dir, ttl: ttl, diff --git a/api/cache_test.go b/api/cache_test.go index d1039d71b..f4a6a756e 100644 --- a/api/cache_test.go +++ b/api/cache_test.go @@ -14,7 +14,7 @@ import ( "github.com/stretchr/testify/require" ) -func Test_CacheReponse(t *testing.T) { +func Test_CacheResponse(t *testing.T) { counter := 0 fakeHTTP := funcTripper{ roundTrip: func(req *http.Request) (*http.Response, error) { @@ -32,7 +32,7 @@ func Test_CacheReponse(t *testing.T) { } cacheDir := filepath.Join(t.TempDir(), "gh-cli-cache") - httpClient := NewHTTPClient(ReplaceTripper(fakeHTTP), CacheReponse(time.Minute, cacheDir)) + httpClient := NewHTTPClient(ReplaceTripper(fakeHTTP), CacheResponse(time.Minute, cacheDir)) do := func(method, url string, body io.Reader) (string, error) { req, err := http.NewRequest(method, url, body) From ec82d3c47e5b2340df8ccbda074ec2120b0c25fe Mon Sep 17 00:00:00 2001 From: Josh Soref Date: Sat, 21 Nov 2020 21:18:51 -0500 Subject: [PATCH 12/43] spelling: settings --- pkg/cmd/repo/garden/garden.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/repo/garden/garden.go b/pkg/cmd/repo/garden/garden.go index 359d8d898..c7ee048a7 100644 --- a/pkg/cmd/repo/garden/garden.go +++ b/pkg/cmd/repo/garden/garden.go @@ -190,7 +190,7 @@ func gardenRun(opts *GardenOptions) error { oldTTYCommand := exec.Command("stty", sttyFileArg, "/dev/tty", "-g") oldTTYSettings, err := oldTTYCommand.CombinedOutput() if err != nil { - fmt.Fprintln(out, "getting TTY setings failed:", string(oldTTYSettings)) + fmt.Fprintln(out, "getting TTY settings failed:", string(oldTTYSettings)) return err } From ded92972cd684f95f2e35ad3ee20f3f28a71bd86 Mon Sep 17 00:00:00 2001 From: Josh Soref Date: Sat, 21 Nov 2020 21:18:51 -0500 Subject: [PATCH 13/43] spelling: template --- pkg/cmd/issue/create/create_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/issue/create/create_test.go b/pkg/cmd/issue/create/create_test.go index c89a349a0..0ced19b62 100644 --- a/pkg/cmd/issue/create/create_test.go +++ b/pkg/cmd/issue/create/create_test.go @@ -145,7 +145,7 @@ func TestIssueCreate_nonLegacyTemplate(t *testing.T) { as, teardown := prompt.InitAskStubber() defer teardown() - // tmeplate + // template as.Stub([]*prompt.QuestionStub{ { Name: "index", From a66a65d4220b970af61b27fbe7ce9b4b62c0c7ba Mon Sep 17 00:00:00 2001 From: Josh Soref Date: Sat, 21 Nov 2020 21:18:52 -0500 Subject: [PATCH 14/43] spelling: unmatched --- internal/run/stub.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/run/stub.go b/internal/run/stub.go index 9bd6e279b..f11834c19 100644 --- a/internal/run/stub.go +++ b/internal/run/stub.go @@ -39,7 +39,7 @@ func Stub() (*CommandStubber, func(T)) { return } t.Helper() - t.Errorf("umatched stubs (%d): %s", len(unmatched), strings.Join(unmatched, ", ")) + t.Errorf("unmatched stubs (%d): %s", len(unmatched), strings.Join(unmatched, ", ")) } } From e92cd432598775998211267eed0c7ef79f825e88 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Tue, 3 Nov 2020 13:08:37 -0800 Subject: [PATCH 15/43] add IOStreams.ReadUserFile --- pkg/cmd/api/api.go | 17 +---------------- pkg/iostreams/iostreams.go | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/pkg/cmd/api/api.go b/pkg/cmd/api/api.go index 16e61e7e1..63437937c 100644 --- a/pkg/cmd/api/api.go +++ b/pkg/cmd/api/api.go @@ -403,7 +403,7 @@ func parseField(f string) (string, string, error) { func magicFieldValue(v string, opts *ApiOptions) (interface{}, error) { if strings.HasPrefix(v, "@") { - return readUserFile(v[1:], opts.IO.In) + return opts.IO.ReadUserFile(v[1:]) } if n, err := strconv.Atoi(v); err == nil { @@ -422,21 +422,6 @@ func magicFieldValue(v string, opts *ApiOptions) (interface{}, error) { } } -func readUserFile(fn string, stdin io.ReadCloser) ([]byte, error) { - var r io.ReadCloser - if fn == "-" { - r = stdin - } else { - var err error - r, err = os.Open(fn) - if err != nil { - return nil, err - } - } - defer r.Close() - return ioutil.ReadAll(r) -} - func openUserFile(fn string, stdin io.ReadCloser) (io.ReadCloser, int64, error) { if fn == "-" { return stdin, -1, nil diff --git a/pkg/iostreams/iostreams.go b/pkg/iostreams/iostreams.go index 99ae0cfdc..a90222fdd 100644 --- a/pkg/iostreams/iostreams.go +++ b/pkg/iostreams/iostreams.go @@ -253,6 +253,21 @@ func (s *IOStreams) ColorScheme() *ColorScheme { return NewColorScheme(s.ColorEnabled(), s.ColorSupport256()) } +func (s *IOStreams) ReadUserFile(fn string) ([]byte, error) { + var r io.ReadCloser + if fn == "-" { + r = s.In + } else { + var err error + r, err = os.Open(fn) + if err != nil { + return nil, err + } + } + defer r.Close() + return ioutil.ReadAll(r) +} + func System() *IOStreams { stdoutIsTTY := isTerminal(os.Stdout) stderrIsTTY := isTerminal(os.Stderr) From d300526318bd7589ed1527a7a9b376336e8c4e32 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Fri, 13 Nov 2020 10:11:39 -0800 Subject: [PATCH 16/43] preserve and restore issue/pr input on failure --- pkg/cmd/issue/create/create.go | 72 ++++++++++++------ pkg/cmd/issue/create/create_test.go | 40 ++++++++++ pkg/cmd/pr/create/create.go | 57 ++++++++++---- pkg/cmd/pr/create/create_test.go | 48 ++++++++++++ pkg/cmd/pr/shared/preserve.go | 66 ++++++++++++++++ pkg/cmd/pr/shared/preserve_test.go | 114 ++++++++++++++++++++++++++++ pkg/cmd/pr/shared/state.go | 73 ++++++++++++++++++ pkg/cmd/pr/shared/survey.go | 44 +++-------- pkg/iostreams/color.go | 4 + pkg/iostreams/iostreams.go | 10 +++ 10 files changed, 457 insertions(+), 71 deletions(-) create mode 100644 pkg/cmd/pr/shared/preserve.go create mode 100644 pkg/cmd/pr/shared/preserve_test.go create mode 100644 pkg/cmd/pr/shared/state.go diff --git a/pkg/cmd/issue/create/create.go b/pkg/cmd/issue/create/create.go index 28c8f0148..6681af62a 100644 --- a/pkg/cmd/issue/create/create.go +++ b/pkg/cmd/issue/create/create.go @@ -26,6 +26,8 @@ type CreateOptions struct { RepoOverride string WebMode bool + JSONFill bool + JSONInput string Title string Body string @@ -62,6 +64,7 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co titleProvided := cmd.Flags().Changed("title") bodyProvided := cmd.Flags().Changed("body") opts.RepoOverride, _ = cmd.Flags().GetString("repo") + opts.JSONFill = cmd.Flags().Changed("json") opts.Interactive = !(titleProvided && bodyProvided) @@ -69,6 +72,14 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co return &cmdutil.FlagError{Err: errors.New("must provide --title and --body when not running interactively")} } + if opts.JSONFill { + opts.Interactive = false + + if opts.WebMode { + return errors.New("--web and --json are mutually exclusive") + } + } + if runF != nil { return runF(opts) } @@ -83,20 +94,21 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co cmd.Flags().StringSliceVarP(&opts.Labels, "label", "l", nil, "Add labels by `name`") cmd.Flags().StringSliceVarP(&opts.Projects, "project", "p", nil, "Add the issue to projects by `name`") cmd.Flags().StringVarP(&opts.Milestone, "milestone", "m", "", "Add the issue to a milestone by `name`") + cmd.Flags().StringVarP(&opts.JSONInput, "json", "j", "", "Use JSON to populate and submit issue") return cmd } -func createRun(opts *CreateOptions) error { +func createRun(opts *CreateOptions) (err error) { httpClient, err := opts.HttpClient() if err != nil { - return err + return } apiClient := api.NewClientFromHTTP(httpClient) baseRepo, err := opts.BaseRepo() if err != nil { - return err + return } templateFiles, legacyTemplate := prShared.FindTemplates(opts.RootDirOverride, "ISSUE_TEMPLATE") @@ -123,7 +135,7 @@ func createRun(opts *CreateOptions) error { if opts.Title != "" || opts.Body != "" { openURL, err = prShared.WithPrAndIssueQueryParams(openURL, tb) if err != nil { - return err + return } } else if len(templateFiles) > 1 { openURL += "/choose" @@ -140,24 +152,28 @@ func createRun(opts *CreateOptions) error { repo, err := api.GitHubRepo(apiClient, baseRepo) if err != nil { - return err + return } if !repo.HasIssuesEnabled { - return fmt.Errorf("the '%s' repository has disabled issues", ghrepo.FullName(baseRepo)) + err = fmt.Errorf("the '%s' repository has disabled issues", ghrepo.FullName(baseRepo)) + return } action := prShared.SubmitAction if opts.Interactive { - editorCommand, err := cmdutil.DetermineEditor(opts.Config) + var editorCommand string + editorCommand, err = cmdutil.DetermineEditor(opts.Config) if err != nil { - return err + return } + defer prShared.PreserveInput(opts.IO, &tb, &err)() + if tb.Title == "" { err = prShared.TitleSurvey(&tb) if err != nil { - return err + return } } @@ -166,12 +182,12 @@ func createRun(opts *CreateOptions) error { templateContent, err = prShared.TemplateSurvey(templateFiles, legacyTemplate, tb) if err != nil { - return err + return } err = prShared.BodySurvey(&tb, templateContent, editorCommand) if err != nil { - return err + return } if tb.Body == "" { @@ -179,31 +195,40 @@ func createRun(opts *CreateOptions) error { } } - action, err := prShared.ConfirmSubmission(!tb.HasMetadata(), repo.ViewerCanTriage()) + var action prShared.Action + action, err = prShared.ConfirmSubmission(!tb.HasMetadata(), repo.ViewerCanTriage()) if err != nil { - return fmt.Errorf("unable to confirm: %w", err) + err = fmt.Errorf("unable to confirm: %w", err) + return } if action == prShared.MetadataAction { err = prShared.MetadataSurvey(opts.IO, apiClient, baseRepo, &tb) if err != nil { - return err + return } action, err = prShared.ConfirmSubmission(!tb.HasMetadata(), false) if err != nil { - return err + return } } if action == prShared.CancelAction { fmt.Fprintln(opts.IO.ErrOut, "Discarding.") - - return nil + return } + } else if opts.JSONFill { + err = prShared.FillFromJSON(opts.IO, opts.JSONInput, &tb) + if err != nil { + return + } + + action = prShared.SubmitAction } else { if tb.Title == "" { - return fmt.Errorf("title can't be blank") + err = fmt.Errorf("title can't be blank") + return } } @@ -211,7 +236,7 @@ func createRun(opts *CreateOptions) error { openURL := ghrepo.GenerateRepoURL(baseRepo, "issues/new") openURL, err = prShared.WithPrAndIssueQueryParams(openURL, tb) if err != nil { - return err + return } if isTerminal { fmt.Fprintf(opts.IO.ErrOut, "Opening %s in your browser.\n", utils.DisplayURL(openURL)) @@ -225,12 +250,13 @@ func createRun(opts *CreateOptions) error { err = prShared.AddMetadataToIssueParams(apiClient, baseRepo, params, &tb) if err != nil { - return err + return } - newIssue, err := api.IssueCreate(apiClient, repo, params) + var newIssue *api.Issue + newIssue, err = api.IssueCreate(apiClient, repo, params) if err != nil { - return err + return } fmt.Fprintln(opts.IO.Out, newIssue.URL) @@ -238,5 +264,5 @@ func createRun(opts *CreateOptions) error { panic("Unreachable state") } - return nil + return } diff --git a/pkg/cmd/issue/create/create_test.go b/pkg/cmd/issue/create/create_test.go index 0ced19b62..cd05cf6c4 100644 --- a/pkg/cmd/issue/create/create_test.go +++ b/pkg/cmd/issue/create/create_test.go @@ -126,6 +126,46 @@ func TestIssueCreate(t *testing.T) { eq(t, output.String(), "https://github.com/OWNER/REPO/issues/12\n") } +func TestIssueCreate_JSON(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { + "id": "REPOID", + "hasIssuesEnabled": true + } } } + `)) + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "createIssue": { "issue": { + "URL": "https://github.com/OWNER/REPO/issues/12" + } } } } + `)) + + output, err := runCommand(http, true, `-j'{"title":"cool", "body":"issue"}'`) + if err != nil { + t.Errorf("error running command `issue create`: %v", err) + } + + bodyBytes, _ := ioutil.ReadAll(http.Requests[1].Body) + reqBody := struct { + Variables struct { + Input struct { + RepositoryID string + Title string + Body string + } + } + }{} + _ = json.Unmarshal(bodyBytes, &reqBody) + + eq(t, reqBody.Variables.Input.RepositoryID, "REPOID") + eq(t, reqBody.Variables.Input.Title, "cool") + eq(t, reqBody.Variables.Input.Body, "issue") + + eq(t, output.String(), "https://github.com/OWNER/REPO/issues/12\n") +} + func TestIssueCreate_nonLegacyTemplate(t *testing.T) { http := &httpmock.Registry{} defer http.Verify(t) diff --git a/pkg/cmd/pr/create/create.go b/pkg/cmd/pr/create/create.go index 9e0180965..39d97dcd6 100644 --- a/pkg/cmd/pr/create/create.go +++ b/pkg/cmd/pr/create/create.go @@ -38,8 +38,10 @@ type CreateOptions struct { RootDirOverride string RepoOverride string - Autofill bool - WebMode bool + Autofill bool + WebMode bool + JSONFill bool + JSONInput string IsDraft bool Title string @@ -99,11 +101,12 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co `), Args: cmdutil.NoArgsQuoteReminder, RunE: func(cmd *cobra.Command, args []string) error { + opts.JSONFill = cmd.Flags().Changed("json") opts.TitleProvided = cmd.Flags().Changed("title") opts.BodyProvided = cmd.Flags().Changed("body") opts.RepoOverride, _ = cmd.Flags().GetString("repo") - if !opts.IO.CanPrompt() && !opts.WebMode && !opts.TitleProvided && !opts.Autofill { + if !opts.IO.CanPrompt() && !opts.JSONFill && !opts.WebMode && !opts.TitleProvided && !opts.Autofill { return &cmdutil.FlagError{Err: errors.New("--title or --fill required when not running interactively")} } @@ -114,6 +117,10 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co return errors.New("the --reviewer flag is not supported with --web") } + if opts.JSONFill && opts.WebMode { + return errors.New("--web and --json are mutually exclusive") + } + if runF != nil { return runF(opts) } @@ -134,6 +141,7 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co fl.StringSliceVarP(&opts.Labels, "label", "l", nil, "Add labels by `name`") fl.StringSliceVarP(&opts.Projects, "project", "p", nil, "Add the pull request to projects by `name`") fl.StringVarP(&opts.Milestone, "milestone", "m", "", "Add the pull request to a milestone by `name`") + fl.StringVarP(&opts.JSONInput, "json", "j", "", "Use JSON to populate and submit PR") return cmd } @@ -141,14 +149,14 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co func createRun(opts *CreateOptions) (err error) { ctx, err := NewCreateContext(opts) if err != nil { - return err + return } client := ctx.Client state, err := NewIssueState(*ctx, *opts) if err != nil { - return err + return } if opts.WebMode { @@ -156,9 +164,9 @@ func createRun(opts *CreateOptions) (err error) { state.Title = opts.Title state.Body = opts.Body } - err := handlePush(*opts, *ctx) + err = handlePush(*opts, *ctx) if err != nil { - return err + return } return previewPR(*opts, *ctx, *state) } @@ -199,35 +207,51 @@ func createRun(opts *CreateOptions) (err error) { if opts.Autofill || (opts.TitleProvided && opts.BodyProvided) { err = handlePush(*opts, *ctx) if err != nil { - return err + return } return submitPR(*opts, *ctx, *state) } + if opts.JSONFill { + err = shared.FillFromJSON(opts.IO, opts.JSONInput, state) + if err != nil { + return fmt.Errorf("could not use JSON input: %w", err) + } + + err = handlePush(*opts, *ctx) + if err != nil { + return + } + + return submitPR(*opts, *ctx, *state) + } + if !opts.TitleProvided { err = shared.TitleSurvey(state) if err != nil { - return err + return } } editorCommand, err := cmdutil.DetermineEditor(opts.Config) if err != nil { - return err + return } + defer shared.PreserveInput(opts.IO, state, &err)() + templateContent := "" if !opts.BodyProvided { templateFiles, legacyTemplate := shared.FindTemplates(opts.RootDirOverride, "PULL_REQUEST_TEMPLATE") templateContent, err = shared.TemplateSurvey(templateFiles, legacyTemplate, *state) if err != nil { - return err + return } err = shared.BodySurvey(state, templateContent, editorCommand) if err != nil { - return err + return } if state.Body == "" { @@ -244,12 +268,12 @@ func createRun(opts *CreateOptions) (err error) { if action == shared.MetadataAction { err = shared.MetadataSurvey(opts.IO, client, ctx.BaseRepo, state) if err != nil { - return err + return } action, err = shared.ConfirmSubmission(!state.HasMetadata(), false) if err != nil { - return err + return } } @@ -260,7 +284,7 @@ func createRun(opts *CreateOptions) (err error) { err = handlePush(*opts, *ctx) if err != nil { - return err + return } if action == shared.PreviewAction { @@ -271,7 +295,8 @@ func createRun(opts *CreateOptions) (err error) { return submitPR(*opts, *ctx, *state) } - return errors.New("expected to cancel, preview, or submit") + err = errors.New("expected to cancel, preview, or submit") + return } func initDefaultTitleBody(ctx CreateContext, state *shared.IssueMetadataState) error { diff --git a/pkg/cmd/pr/create/create_test.go b/pkg/cmd/pr/create/create_test.go index 0c9a91c1a..3228d51f8 100644 --- a/pkg/cmd/pr/create/create_test.go +++ b/pkg/cmd/pr/create/create_test.go @@ -136,6 +136,54 @@ func TestPRCreate_nontty_insufficient_flags(t *testing.T) { assert.Equal(t, "", output.String()) } +func TestPRCreate_json(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + + http.StubRepoInfoResponse("OWNER", "REPO", "master") + 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 status + cs.Stub("1234567890,commit 0\n2345678901,commit 1") // git log + + output, err := runCommand(http, nil, "feature", false, `-j'{"title":"cool", "body":"pr"}' -H feature`) + 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) + + assert.Equal(t, "REPOID", reqBody.Variables.Input.RepositoryID) + assert.Equal(t, "cool", reqBody.Variables.Input.Title) + assert.Equal(t, "pr", reqBody.Variables.Input.Body) + assert.Equal(t, "master", reqBody.Variables.Input.BaseRefName) + assert.Equal(t, "feature", reqBody.Variables.Input.HeadRefName) + + assert.Equal(t, "", output.Stderr()) + assert.Equal(t, "https://github.com/OWNER/REPO/pull/12\n", output.String()) +} + func TestPRCreate_nontty(t *testing.T) { http := initFakeHTTP() defer http.Verify(t) diff --git a/pkg/cmd/pr/shared/preserve.go b/pkg/cmd/pr/shared/preserve.go new file mode 100644 index 000000000..ba8c8cc46 --- /dev/null +++ b/pkg/cmd/pr/shared/preserve.go @@ -0,0 +1,66 @@ +package shared + +import ( + "encoding/json" + "fmt" + "os" + "path/filepath" + "time" + + "github.com/cli/cli/pkg/iostreams" +) + +func dumpPath(random int64) string { + r := fmt.Sprintf("%x", random) + r = r[len(r)-5:] + dumpFilename := fmt.Sprintf("gh%s.json", r) + return filepath.Join(os.TempDir(), dumpFilename) +} + +func PreserveInput(io *iostreams.IOStreams, state *IssueMetadataState, createErr *error) func() { + return func() { + if !state.IsDirty() { + return + } + + if *createErr == nil { + return + } + + out := io.ErrOut + + // this extra newline guards against appending to the end of a survey line + fmt.Fprintln(out) + + data, err := json.Marshal(state) + if err != nil { + fmt.Fprintf(out, "failed to save input to file: %s\n", err) + fmt.Fprintln(out, "would have saved:") + fmt.Fprintf(out, "%v\n", state) + return + } + + dp := dumpPath(time.Now().UnixNano()) + + err = io.WriteFile(dp, data) + if err != nil { + fmt.Fprintf(out, "failed to save input to file: %s\n", err) + fmt.Fprintln(out, "would have saved:") + fmt.Fprintln(out, string(data)) + return + } + + cs := io.ColorScheme() + + issueType := "pr" + if state.Type == IssueMetadata { + issueType = "issue" + } + + fmt.Fprintf(out, "%s operation failed. input saved to: %s\n", cs.FailureIcon(), dp) + fmt.Fprintf(out, "resubmit with: gh %s create -j@%s\n", issueType, dp) + + // some whitespace before the actual error + fmt.Fprintln(out) + } +} diff --git a/pkg/cmd/pr/shared/preserve_test.go b/pkg/cmd/pr/shared/preserve_test.go new file mode 100644 index 000000000..706544023 --- /dev/null +++ b/pkg/cmd/pr/shared/preserve_test.go @@ -0,0 +1,114 @@ +package shared + +import ( + "encoding/json" + "errors" + "os" + "testing" + + "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/test" + "github.com/stretchr/testify/assert" +) + +func Test_dumpPath(t *testing.T) { + // mostly pointless test + var random int64 = 1234567890 + tempDir := os.TempDir() + assert.Equal(t, dumpPath(random), tempDir+"/gh602d2.json") +} + +func Test_PreserveInput(t *testing.T) { + tests := []struct { + name string + state *IssueMetadataState + err bool + wantErrLines []string + wantPreservation bool + }{ + { + name: "err, no changes to state", + err: true, + }, + { + name: "no err, no changes to state", + err: false, + }, + { + name: "no err, changes to state", + state: &IssueMetadataState{ + dirty: true, + }, + }, + { + name: "err, title/body input received", + state: &IssueMetadataState{ + dirty: true, + Title: "almost a", + Body: "jill sandwich", + Reviewers: []string{"barry", "chris"}, + Labels: []string{"sandwich"}, + }, + wantErrLines: []string{ + `X operation failed. input saved to:.*\.json`, + `resubmit with: gh issue create -j@.*\.json`, + }, + err: true, + wantPreservation: true, + }, + { + name: "err, metadata received", + state: &IssueMetadataState{ + Reviewers: []string{"barry", "chris"}, + Labels: []string{"sandwich"}, + }, + wantErrLines: []string{ + `X operation failed. input saved to:.*\.json`, + `resubmit with: gh issue create -j@.*\.json`, + }, + err: true, + wantPreservation: true, + }, + { + name: "err, dirty, pull request", + state: &IssueMetadataState{ + dirty: true, + Title: "a pull request", + Type: PRMetadata, + }, + wantErrLines: []string{ + `X operation failed. input saved to:.*\.json`, + `resubmit with: gh pr create -j@.*\.json`, + }, + err: true, + wantPreservation: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.state == nil { + tt.state = &IssueMetadataState{} + } + io, _, _, errOut := iostreams.Test() + io.WriteOverride = []byte{} + var err error + if tt.err { + err = errors.New("error during creation") + } + + PreserveInput(io, tt.state, &err)() + + if tt.wantPreservation { + test.ExpectLines(t, errOut.String(), tt.wantErrLines...) + preserved := &IssueMetadataState{} + assert.NoError(t, json.Unmarshal(io.WriteOverride, preserved)) + preserved.dirty = tt.state.dirty + assert.Equal(t, preserved, tt.state) + } else { + assert.Equal(t, errOut.String(), "") + assert.Equal(t, string(io.WriteOverride), "") + } + }) + } +} diff --git a/pkg/cmd/pr/shared/state.go b/pkg/cmd/pr/shared/state.go new file mode 100644 index 000000000..6c8e3531e --- /dev/null +++ b/pkg/cmd/pr/shared/state.go @@ -0,0 +1,73 @@ +package shared + +import ( + "encoding/json" + "fmt" + "strings" + + "github.com/cli/cli/api" + "github.com/cli/cli/pkg/iostreams" +) + +type metadataStateType int + +const ( + IssueMetadata metadataStateType = iota + PRMetadata +) + +type IssueMetadataState struct { + Type metadataStateType + + Draft bool + + Body string + Title string + + Metadata []string + Reviewers []string + Assignees []string + Labels []string + Projects []string + Milestones []string + + MetadataResult *api.RepoMetadataResult + + dirty bool // whether user i/o has modified this +} + +func (tb *IssueMetadataState) MarkDirty() { + tb.dirty = true +} + +func (tb *IssueMetadataState) IsDirty() bool { + return tb.dirty || tb.HasMetadata() +} + +func (tb *IssueMetadataState) HasMetadata() bool { + return len(tb.Reviewers) > 0 || + len(tb.Assignees) > 0 || + len(tb.Labels) > 0 || + len(tb.Projects) > 0 || + len(tb.Milestones) > 0 +} + +func FillFromJSON(io *iostreams.IOStreams, JSONInput string, state *IssueMetadataState) error { + var data []byte + var err error + if strings.HasPrefix(JSONInput, "@") { + data, err = io.ReadUserFile(JSONInput[1:]) + if err != nil { + return fmt.Errorf("failed to read file %s: %w", JSONInput[1:], err) + } + } else { + data = []byte(JSONInput) + } + + err = json.Unmarshal(data, state) + if err != nil { + return fmt.Errorf("JSON parsing failure: %w", err) + } + + return nil +} diff --git a/pkg/cmd/pr/shared/survey.go b/pkg/cmd/pr/shared/survey.go index 03bddb1f8..4e4a72855 100644 --- a/pkg/cmd/pr/shared/survey.go +++ b/pkg/cmd/pr/shared/survey.go @@ -16,38 +16,6 @@ import ( ) type Action int -type metadataStateType int - -const ( - IssueMetadata metadataStateType = iota - PRMetadata -) - -type IssueMetadataState struct { - Type metadataStateType - - Draft bool - - Body string - Title string - - Metadata []string - Reviewers []string - Assignees []string - Labels []string - Projects []string - Milestones []string - - MetadataResult *api.RepoMetadataResult -} - -func (tb *IssueMetadataState) HasMetadata() bool { - return len(tb.Reviewers) > 0 || - len(tb.Assignees) > 0 || - len(tb.Labels) > 0 || - len(tb.Projects) > 0 || - len(tb.Milestones) > 0 -} const ( SubmitAction Action = iota @@ -170,6 +138,8 @@ func BodySurvey(state *IssueMetadataState, templateContent, editorCommand string state.Body += templateContent } + preBody := state.Body + // TODO should just be an AskOne but ran into problems with the stubber qs := []*survey.Question{ { @@ -193,10 +163,16 @@ func BodySurvey(state *IssueMetadataState, templateContent, editorCommand string return err } + if state.Body != "" && preBody != state.Body { + state.MarkDirty() + } + return nil } func TitleSurvey(state *IssueMetadataState) error { + preTitle := state.Title + // TODO should just be an AskOne but ran into problems with the stubber qs := []*survey.Question{ { @@ -213,6 +189,10 @@ func TitleSurvey(state *IssueMetadataState) error { return err } + if preTitle != state.Title { + state.MarkDirty() + } + return nil } diff --git a/pkg/iostreams/color.go b/pkg/iostreams/color.go index 7e429e407..6fc2bd023 100644 --- a/pkg/iostreams/color.go +++ b/pkg/iostreams/color.go @@ -122,6 +122,10 @@ func (c *ColorScheme) WarningIcon() string { return c.Yellow("!") } +func (c *ColorScheme) FailureIcon() string { + return c.Red("X") +} + func (c *ColorScheme) ColorFromString(s string) func(string) string { s = strings.ToLower(s) var fn func(string) string diff --git a/pkg/iostreams/iostreams.go b/pkg/iostreams/iostreams.go index a90222fdd..87615b614 100644 --- a/pkg/iostreams/iostreams.go +++ b/pkg/iostreams/iostreams.go @@ -45,6 +45,8 @@ type IOStreams struct { pagerProcess *os.Process neverPrompt bool + + WriteOverride []byte } func (s *IOStreams) ColorEnabled() bool { @@ -268,6 +270,14 @@ func (s *IOStreams) ReadUserFile(fn string) ([]byte, error) { return ioutil.ReadAll(r) } +func (s *IOStreams) WriteFile(fn string, data []byte) error { + if s.WriteOverride != nil { + s.WriteOverride = data + return nil + } + return ioutil.WriteFile(fn, data, 0660) +} + func System() *IOStreams { stdoutIsTTY := isTerminal(os.Stdout) stderrIsTTY := isTerminal(os.Stderr) From fffd315a7e3c977825ef921af8f7ceb997f9f1d3 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Mon, 16 Nov 2020 14:08:14 -0800 Subject: [PATCH 17/43] fix dumb test --- pkg/cmd/pr/shared/preserve_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/pr/shared/preserve_test.go b/pkg/cmd/pr/shared/preserve_test.go index 706544023..b69154113 100644 --- a/pkg/cmd/pr/shared/preserve_test.go +++ b/pkg/cmd/pr/shared/preserve_test.go @@ -4,6 +4,7 @@ import ( "encoding/json" "errors" "os" + "path/filepath" "testing" "github.com/cli/cli/pkg/iostreams" @@ -14,8 +15,7 @@ import ( func Test_dumpPath(t *testing.T) { // mostly pointless test var random int64 = 1234567890 - tempDir := os.TempDir() - assert.Equal(t, dumpPath(random), tempDir+"/gh602d2.json") + assert.Equal(t, dumpPath(random), filepath.Join(os.TempDir(), "gh602d2.json")) } func Test_PreserveInput(t *testing.T) { From f68909b7a840d08a542d86939c799d4e45fcf35a Mon Sep 17 00:00:00 2001 From: vilmibm Date: Fri, 20 Nov 2020 10:57:35 -0800 Subject: [PATCH 18/43] use TempFile though the testing is gross --- pkg/cmd/pr/shared/preserve.go | 17 +++++++---- pkg/cmd/pr/shared/preserve_test.go | 47 ++++++++++++++++++++---------- pkg/iostreams/iostreams.go | 12 ++++---- 3 files changed, 50 insertions(+), 26 deletions(-) diff --git a/pkg/cmd/pr/shared/preserve.go b/pkg/cmd/pr/shared/preserve.go index ba8c8cc46..6d0b7303c 100644 --- a/pkg/cmd/pr/shared/preserve.go +++ b/pkg/cmd/pr/shared/preserve.go @@ -5,7 +5,6 @@ import ( "fmt" "os" "path/filepath" - "time" "github.com/cli/cli/pkg/iostreams" ) @@ -40,9 +39,17 @@ func PreserveInput(io *iostreams.IOStreams, state *IssueMetadataState, createErr return } - dp := dumpPath(time.Now().UnixNano()) + tmpfile, err := io.TempFile(os.TempDir(), "gh*.json") + if err != nil { + fmt.Fprintf(out, "failed to save input to file: %s\n", err) + fmt.Fprintln(out, "would have saved:") + fmt.Fprintf(out, "%v\n", state) + return + } - err = io.WriteFile(dp, data) + tmpfilePath := filepath.Join(os.TempDir(), tmpfile.Name()) + + _, err = tmpfile.Write(data) if err != nil { fmt.Fprintf(out, "failed to save input to file: %s\n", err) fmt.Fprintln(out, "would have saved:") @@ -57,8 +64,8 @@ func PreserveInput(io *iostreams.IOStreams, state *IssueMetadataState, createErr issueType = "issue" } - fmt.Fprintf(out, "%s operation failed. input saved to: %s\n", cs.FailureIcon(), dp) - fmt.Fprintf(out, "resubmit with: gh %s create -j@%s\n", issueType, dp) + fmt.Fprintf(out, "%s operation failed. input saved to: %s\n", cs.FailureIcon(), tmpfilePath) + fmt.Fprintf(out, "resubmit with: gh %s create -j@%s\n", issueType, tmpfilePath) // some whitespace before the actual error fmt.Fprintln(out) diff --git a/pkg/cmd/pr/shared/preserve_test.go b/pkg/cmd/pr/shared/preserve_test.go index b69154113..8dc574676 100644 --- a/pkg/cmd/pr/shared/preserve_test.go +++ b/pkg/cmd/pr/shared/preserve_test.go @@ -3,6 +3,7 @@ package shared import ( "encoding/json" "errors" + "io/ioutil" "os" "path/filepath" "testing" @@ -12,12 +13,6 @@ import ( "github.com/stretchr/testify/assert" ) -func Test_dumpPath(t *testing.T) { - // mostly pointless test - var random int64 = 1234567890 - assert.Equal(t, dumpPath(random), filepath.Join(os.TempDir(), "gh602d2.json")) -} - func Test_PreserveInput(t *testing.T) { tests := []struct { name string @@ -50,8 +45,8 @@ func Test_PreserveInput(t *testing.T) { Labels: []string{"sandwich"}, }, wantErrLines: []string{ - `X operation failed. input saved to:.*\.json`, - `resubmit with: gh issue create -j@.*\.json`, + `X operation failed. input saved to:.*testfile.*`, + `resubmit with: gh issue create -j@.*testfile.*`, }, err: true, wantPreservation: true, @@ -63,8 +58,8 @@ func Test_PreserveInput(t *testing.T) { Labels: []string{"sandwich"}, }, wantErrLines: []string{ - `X operation failed. input saved to:.*\.json`, - `resubmit with: gh issue create -j@.*\.json`, + `X operation failed. input saved to:.*testfile.*`, + `resubmit with: gh issue create -j@.*testfile.*`, }, err: true, wantPreservation: true, @@ -77,8 +72,8 @@ func Test_PreserveInput(t *testing.T) { Type: PRMetadata, }, wantErrLines: []string{ - `X operation failed. input saved to:.*\.json`, - `resubmit with: gh pr create -j@.*\.json`, + `X operation failed. input saved to:.*testfile.*`, + `resubmit with: gh pr create -j@.*testfile.*`, }, err: true, wantPreservation: true, @@ -90,8 +85,15 @@ func Test_PreserveInput(t *testing.T) { if tt.state == nil { tt.state = &IssueMetadataState{} } + io, _, _, errOut := iostreams.Test() - io.WriteOverride = []byte{} + + tfPath, tf, tferr := tmpfile() + assert.NoError(t, tferr) + defer os.Remove(tfPath) + + io.TempFileOverride = tf + var err error if tt.err { err = errors.New("error during creation") @@ -99,16 +101,31 @@ func Test_PreserveInput(t *testing.T) { PreserveInput(io, tt.state, &err)() + tf.Seek(0, 0) + + data, err := ioutil.ReadAll(tf) + assert.NoError(t, err) + if tt.wantPreservation { test.ExpectLines(t, errOut.String(), tt.wantErrLines...) preserved := &IssueMetadataState{} - assert.NoError(t, json.Unmarshal(io.WriteOverride, preserved)) + assert.NoError(t, json.Unmarshal(data, preserved)) preserved.dirty = tt.state.dirty assert.Equal(t, preserved, tt.state) } else { assert.Equal(t, errOut.String(), "") - assert.Equal(t, string(io.WriteOverride), "") + assert.Equal(t, string(data), "") } }) } } + +func tmpfile() (string, *os.File, error) { + dir := os.TempDir() + tmpfile, err := ioutil.TempFile(dir, "testfile*") + if err != nil { + return "", nil, err + } + + return filepath.Join(dir, tmpfile.Name()), tmpfile, nil +} diff --git a/pkg/iostreams/iostreams.go b/pkg/iostreams/iostreams.go index 87615b614..ae200e3d1 100644 --- a/pkg/iostreams/iostreams.go +++ b/pkg/iostreams/iostreams.go @@ -46,7 +46,7 @@ type IOStreams struct { neverPrompt bool - WriteOverride []byte + TempFileOverride *os.File } func (s *IOStreams) ColorEnabled() bool { @@ -270,12 +270,12 @@ func (s *IOStreams) ReadUserFile(fn string) ([]byte, error) { return ioutil.ReadAll(r) } -func (s *IOStreams) WriteFile(fn string, data []byte) error { - if s.WriteOverride != nil { - s.WriteOverride = data - return nil +func (s *IOStreams) TempFile(dir, pattern string) (*os.File, error) { + if s.TempFileOverride != nil { + fmt.Printf("DBG %#v\n", s.TempFileOverride) + return s.TempFileOverride, nil } - return ioutil.WriteFile(fn, data, 0660) + return ioutil.TempFile(dir, pattern) } func System() *IOStreams { From 1d408eb30de84538cea441dcb45ffd9c9cf416c3 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Fri, 20 Nov 2020 11:00:25 -0800 Subject: [PATCH 19/43] linter appeasement --- pkg/cmd/pr/shared/preserve.go | 7 ------- pkg/cmd/pr/shared/preserve_test.go | 3 ++- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/pkg/cmd/pr/shared/preserve.go b/pkg/cmd/pr/shared/preserve.go index 6d0b7303c..8002e4a59 100644 --- a/pkg/cmd/pr/shared/preserve.go +++ b/pkg/cmd/pr/shared/preserve.go @@ -9,13 +9,6 @@ import ( "github.com/cli/cli/pkg/iostreams" ) -func dumpPath(random int64) string { - r := fmt.Sprintf("%x", random) - r = r[len(r)-5:] - dumpFilename := fmt.Sprintf("gh%s.json", r) - return filepath.Join(os.TempDir(), dumpFilename) -} - func PreserveInput(io *iostreams.IOStreams, state *IssueMetadataState, createErr *error) func() { return func() { if !state.IsDirty() { diff --git a/pkg/cmd/pr/shared/preserve_test.go b/pkg/cmd/pr/shared/preserve_test.go index 8dc574676..a4211e55f 100644 --- a/pkg/cmd/pr/shared/preserve_test.go +++ b/pkg/cmd/pr/shared/preserve_test.go @@ -101,7 +101,8 @@ func Test_PreserveInput(t *testing.T) { PreserveInput(io, tt.state, &err)() - tf.Seek(0, 0) + _, err = tf.Seek(0, 0) + assert.NoError(t, err) data, err := ioutil.ReadAll(tf) assert.NoError(t, err) From d6e84a75fb3009d50d3a154251a3209097315c04 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Fri, 20 Nov 2020 11:43:41 -0800 Subject: [PATCH 20/43] switch to recover instead of resubmit --- pkg/cmd/issue/create/create.go | 45 +++++++------ pkg/cmd/issue/create/create_test.go | 83 ++++++++++++++++++------ pkg/cmd/pr/create/create.go | 43 ++++++------- pkg/cmd/pr/create/create_test.go | 98 +++++++++++++++++++++-------- pkg/cmd/pr/shared/preserve.go | 6 +- pkg/cmd/pr/shared/preserve_test.go | 30 +++------ pkg/cmd/pr/shared/state.go | 13 ++-- pkg/iostreams/iostreams.go | 1 - 8 files changed, 188 insertions(+), 131 deletions(-) diff --git a/pkg/cmd/issue/create/create.go b/pkg/cmd/issue/create/create.go index 6681af62a..a2912f638 100644 --- a/pkg/cmd/issue/create/create.go +++ b/pkg/cmd/issue/create/create.go @@ -26,8 +26,7 @@ type CreateOptions struct { RepoOverride string WebMode bool - JSONFill bool - JSONInput string + RecoverFile string Title string Body string @@ -64,7 +63,10 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co titleProvided := cmd.Flags().Changed("title") bodyProvided := cmd.Flags().Changed("body") opts.RepoOverride, _ = cmd.Flags().GetString("repo") - opts.JSONFill = cmd.Flags().Changed("json") + + if !opts.IO.CanPrompt() && opts.RecoverFile != "" { + return &cmdutil.FlagError{Err: errors.New("--recover only supported when running interactively")} + } opts.Interactive = !(titleProvided && bodyProvided) @@ -72,14 +74,6 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co return &cmdutil.FlagError{Err: errors.New("must provide --title and --body when not running interactively")} } - if opts.JSONFill { - opts.Interactive = false - - if opts.WebMode { - return errors.New("--web and --json are mutually exclusive") - } - } - if runF != nil { return runF(opts) } @@ -94,7 +88,7 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co cmd.Flags().StringSliceVarP(&opts.Labels, "label", "l", nil, "Add labels by `name`") cmd.Flags().StringSliceVarP(&opts.Projects, "project", "p", nil, "Add the issue to projects by `name`") cmd.Flags().StringVarP(&opts.Milestone, "milestone", "m", "", "Add the issue to a milestone by `name`") - cmd.Flags().StringVarP(&opts.JSONInput, "json", "j", "", "Use JSON to populate and submit issue") + cmd.Flags().StringVarP(&opts.RecoverFile, "recover", "e", "", "Recover input from a failed run of create") return cmd } @@ -130,6 +124,14 @@ func createRun(opts *CreateOptions) (err error) { Body: opts.Body, } + if opts.RecoverFile != "" { + err = prShared.FillFromJSON(opts.IO, opts.RecoverFile, &tb) + if err != nil { + err = fmt.Errorf("failed to recover input: %w", err) + return + } + } + if opts.WebMode { openURL := ghrepo.GenerateRepoURL(baseRepo, "issues/new") if opts.Title != "" || opts.Body != "" { @@ -170,19 +172,21 @@ func createRun(opts *CreateOptions) (err error) { defer prShared.PreserveInput(opts.IO, &tb, &err)() - if tb.Title == "" { + if opts.Title == "" { err = prShared.TitleSurvey(&tb) if err != nil { return } } - if tb.Body == "" { + if opts.Body == "" { templateContent := "" - templateContent, err = prShared.TemplateSurvey(templateFiles, legacyTemplate, tb) - if err != nil { - return + if opts.RecoverFile == "" { + templateContent, err = prShared.TemplateSurvey(templateFiles, legacyTemplate, tb) + if err != nil { + return + } } err = prShared.BodySurvey(&tb, templateContent, editorCommand) @@ -218,13 +222,6 @@ func createRun(opts *CreateOptions) (err error) { fmt.Fprintln(opts.IO.ErrOut, "Discarding.") return } - } else if opts.JSONFill { - err = prShared.FillFromJSON(opts.IO, opts.JSONInput, &tb) - if err != nil { - return - } - - action = prShared.SubmitAction } else { if tb.Title == "" { err = fmt.Errorf("title can't be blank") diff --git a/pkg/cmd/issue/create/create_test.go b/pkg/cmd/issue/create/create_test.go index cd05cf6c4..733094964 100644 --- a/pkg/cmd/issue/create/create_test.go +++ b/pkg/cmd/issue/create/create_test.go @@ -3,8 +3,10 @@ package create import ( "bytes" "encoding/json" + "fmt" "io/ioutil" "net/http" + "os" "os/exec" "reflect" "strings" @@ -13,6 +15,7 @@ import ( "github.com/cli/cli/internal/config" "github.com/cli/cli/internal/ghrepo" "github.com/cli/cli/internal/run" + prShared "github.com/cli/cli/pkg/cmd/pr/shared" "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/httpmock" "github.com/cli/cli/pkg/iostreams" @@ -126,7 +129,7 @@ func TestIssueCreate(t *testing.T) { eq(t, output.String(), "https://github.com/OWNER/REPO/issues/12\n") } -func TestIssueCreate_JSON(t *testing.T) { +func TestIssueCreate_recover(t *testing.T) { http := &httpmock.Registry{} defer http.Verify(t) @@ -136,33 +139,73 @@ func TestIssueCreate_JSON(t *testing.T) { "hasIssuesEnabled": true } } } `)) - http.StubResponse(200, bytes.NewBufferString(` + http.Register( + httpmock.GraphQL(`query RepositoryResolveMetadataIDs\b`), + httpmock.StringResponse(` + { "data": { + "u000": { "login": "MonaLisa", "id": "MONAID" }, + "repository": { + "l000": { "name": "bug", "id": "BUGID" }, + "l001": { "name": "TODO", "id": "TODOID" } + } + } } + `)) + http.Register( + httpmock.GraphQL(`mutation IssueCreate\b`), + httpmock.GraphQLMutation(` { "data": { "createIssue": { "issue": { "URL": "https://github.com/OWNER/REPO/issues/12" } } } } - `)) + `, func(inputs map[string]interface{}) { + eq(t, inputs["title"], "recovered title") + eq(t, inputs["body"], "recovered body") + eq(t, inputs["labelIds"], []interface{}{"BUGID", "TODOID"}) + })) - output, err := runCommand(http, true, `-j'{"title":"cool", "body":"issue"}'`) + as, teardown := prompt.InitAskStubber() + defer teardown() + + as.Stub([]*prompt.QuestionStub{ + { + Name: "Title", + Default: true, + }, + }) + as.Stub([]*prompt.QuestionStub{ + { + Name: "Body", + Default: true, + }, + }) + as.Stub([]*prompt.QuestionStub{ + { + Name: "confirmation", + Value: 0, + }, + }) + + tmpfile, err := ioutil.TempFile(os.TempDir(), "testrecover*") + assert.NoError(t, err) + + state := prShared.IssueMetadataState{ + Title: "recovered title", + Body: "recovered body", + Labels: []string{"bug", "TODO"}, + } + + data, err := json.Marshal(state) + assert.NoError(t, err) + + _, err = tmpfile.Write(data) + assert.NoError(t, err) + + args := fmt.Sprintf("-e '%s'", tmpfile.Name()) + + output, err := runCommandWithRootDirOverridden(http, true, args, "") if err != nil { t.Errorf("error running command `issue create`: %v", err) } - bodyBytes, _ := ioutil.ReadAll(http.Requests[1].Body) - reqBody := struct { - Variables struct { - Input struct { - RepositoryID string - Title string - Body string - } - } - }{} - _ = json.Unmarshal(bodyBytes, &reqBody) - - eq(t, reqBody.Variables.Input.RepositoryID, "REPOID") - eq(t, reqBody.Variables.Input.Title, "cool") - eq(t, reqBody.Variables.Input.Body, "issue") - eq(t, output.String(), "https://github.com/OWNER/REPO/issues/12\n") } diff --git a/pkg/cmd/pr/create/create.go b/pkg/cmd/pr/create/create.go index 39d97dcd6..502de1126 100644 --- a/pkg/cmd/pr/create/create.go +++ b/pkg/cmd/pr/create/create.go @@ -38,10 +38,9 @@ type CreateOptions struct { RootDirOverride string RepoOverride string - Autofill bool - WebMode bool - JSONFill bool - JSONInput string + Autofill bool + WebMode bool + RecoverFile string IsDraft bool Title string @@ -101,12 +100,15 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co `), Args: cmdutil.NoArgsQuoteReminder, RunE: func(cmd *cobra.Command, args []string) error { - opts.JSONFill = cmd.Flags().Changed("json") opts.TitleProvided = cmd.Flags().Changed("title") opts.BodyProvided = cmd.Flags().Changed("body") opts.RepoOverride, _ = cmd.Flags().GetString("repo") - if !opts.IO.CanPrompt() && !opts.JSONFill && !opts.WebMode && !opts.TitleProvided && !opts.Autofill { + if !opts.IO.CanPrompt() && opts.RecoverFile != "" { + return &cmdutil.FlagError{Err: errors.New("--recover only supported when running interactively")} + } + + if !opts.IO.CanPrompt() && !opts.WebMode && !opts.TitleProvided && !opts.Autofill { return &cmdutil.FlagError{Err: errors.New("--title or --fill required when not running interactively")} } @@ -117,10 +119,6 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co return errors.New("the --reviewer flag is not supported with --web") } - if opts.JSONFill && opts.WebMode { - return errors.New("--web and --json are mutually exclusive") - } - if runF != nil { return runF(opts) } @@ -141,7 +139,7 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co fl.StringSliceVarP(&opts.Labels, "label", "l", nil, "Add labels by `name`") fl.StringSliceVarP(&opts.Projects, "project", "p", nil, "Add the pull request to projects by `name`") fl.StringVarP(&opts.Milestone, "milestone", "m", "", "Add the pull request to a milestone by `name`") - fl.StringVarP(&opts.JSONInput, "json", "j", "", "Use JSON to populate and submit PR") + fl.StringVarP(&opts.RecoverFile, "recover", "e", "", "Recover input from a failed run of create") return cmd } @@ -212,18 +210,11 @@ func createRun(opts *CreateOptions) (err error) { return submitPR(*opts, *ctx, *state) } - if opts.JSONFill { - err = shared.FillFromJSON(opts.IO, opts.JSONInput, state) + if opts.RecoverFile != "" { + err = shared.FillFromJSON(opts.IO, opts.RecoverFile, state) if err != nil { - return fmt.Errorf("could not use JSON input: %w", err) + return fmt.Errorf("failed to recover input: %w", err) } - - err = handlePush(*opts, *ctx) - if err != nil { - return - } - - return submitPR(*opts, *ctx, *state) } if !opts.TitleProvided { @@ -242,11 +233,13 @@ func createRun(opts *CreateOptions) (err error) { templateContent := "" if !opts.BodyProvided { - templateFiles, legacyTemplate := shared.FindTemplates(opts.RootDirOverride, "PULL_REQUEST_TEMPLATE") + if opts.RecoverFile == "" { + templateFiles, legacyTemplate := shared.FindTemplates(opts.RootDirOverride, "PULL_REQUEST_TEMPLATE") - templateContent, err = shared.TemplateSurvey(templateFiles, legacyTemplate, *state) - if err != nil { - return + templateContent, err = shared.TemplateSurvey(templateFiles, legacyTemplate, *state) + if err != nil { + return + } } err = shared.BodySurvey(state, templateContent, editorCommand) diff --git a/pkg/cmd/pr/create/create_test.go b/pkg/cmd/pr/create/create_test.go index 3228d51f8..2c9fd6a41 100644 --- a/pkg/cmd/pr/create/create_test.go +++ b/pkg/cmd/pr/create/create_test.go @@ -3,8 +3,10 @@ package create import ( "bytes" "encoding/json" + "fmt" "io/ioutil" "net/http" + "os" "reflect" "strings" "testing" @@ -136,20 +138,45 @@ func TestPRCreate_nontty_insufficient_flags(t *testing.T) { assert.Equal(t, "", output.String()) } -func TestPRCreate_json(t *testing.T) { +func TestPRCreate_recover(t *testing.T) { http := initFakeHTTP() defer http.Verify(t) http.StubRepoInfoResponse("OWNER", "REPO", "master") - http.StubResponse(200, bytes.NewBufferString(` + http.Register( + httpmock.GraphQL(`query PullRequestForBranch\b`), + httpmock.StringResponse(` { "data": { "repository": { "pullRequests": { "nodes" : [ ] } } } } - `)) - http.StubResponse(200, bytes.NewBufferString(` + `)) + http.Register( + httpmock.GraphQL(`query RepositoryResolveMetadataIDs\b`), + httpmock.StringResponse(` + { "data": { + "u000": { "login": "jillValentine", "id": "JILLID" }, + "repository": {}, + "organization": {} + } } + `)) + http.Register( + httpmock.GraphQL(`mutation PullRequestCreateRequestReviews\b`), + httpmock.GraphQLMutation(` + { "data": { "requestReviews": { + "clientMutationId": "" + } } } + `, func(inputs map[string]interface{}) { + eq(t, inputs["userIds"], []interface{}{"JILLID"}) + })) + 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, "recovered title", input["title"].(string)) + assert.Equal(t, "recovered body", input["body"].(string)) + })) cs, cmdTeardown := test.InitCmdStubber() defer cmdTeardown() @@ -157,31 +184,48 @@ func TestPRCreate_json(t *testing.T) { cs.Stub("") // git status cs.Stub("1234567890,commit 0\n2345678901,commit 1") // git log - output, err := runCommand(http, nil, "feature", false, `-j'{"title":"cool", "body":"pr"}' -H feature`) - require.NoError(t, err) + as, teardown := prompt.InitAskStubber() + defer teardown() + as.Stub([]*prompt.QuestionStub{ + { + Name: "Title", + Default: true, + }, + }) + as.Stub([]*prompt.QuestionStub{ + { + Name: "Body", + Default: true, + }, + }) + as.Stub([]*prompt.QuestionStub{ + { + Name: "confirmation", + Value: 0, + }, + }) - 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) + tmpfile, err := ioutil.TempFile(os.TempDir(), "testrecover*") + assert.NoError(t, err) - assert.Equal(t, "REPOID", reqBody.Variables.Input.RepositoryID) - assert.Equal(t, "cool", reqBody.Variables.Input.Title) - assert.Equal(t, "pr", reqBody.Variables.Input.Body) - assert.Equal(t, "master", reqBody.Variables.Input.BaseRefName) - assert.Equal(t, "feature", reqBody.Variables.Input.HeadRefName) + state := prShared.IssueMetadataState{ + Title: "recovered title", + Body: "recovered body", + Reviewers: []string{"jillValentine"}, + } - assert.Equal(t, "", output.Stderr()) - assert.Equal(t, "https://github.com/OWNER/REPO/pull/12\n", output.String()) + data, err := json.Marshal(state) + assert.NoError(t, err) + + _, err = tmpfile.Write(data) + assert.NoError(t, err) + + args := fmt.Sprintf("-e '%s' -Hfeature", tmpfile.Name()) + + output, err := runCommandWithRootDirOverridden(http, nil, "feature", true, args, "") + assert.NoError(t, err) + + eq(t, output.String(), "https://github.com/OWNER/REPO/pull/12\n") } func TestPRCreate_nontty(t *testing.T) { diff --git a/pkg/cmd/pr/shared/preserve.go b/pkg/cmd/pr/shared/preserve.go index 8002e4a59..2bd5729ff 100644 --- a/pkg/cmd/pr/shared/preserve.go +++ b/pkg/cmd/pr/shared/preserve.go @@ -4,7 +4,6 @@ import ( "encoding/json" "fmt" "os" - "path/filepath" "github.com/cli/cli/pkg/iostreams" ) @@ -40,8 +39,6 @@ func PreserveInput(io *iostreams.IOStreams, state *IssueMetadataState, createErr return } - tmpfilePath := filepath.Join(os.TempDir(), tmpfile.Name()) - _, err = tmpfile.Write(data) if err != nil { fmt.Fprintf(out, "failed to save input to file: %s\n", err) @@ -57,8 +54,7 @@ func PreserveInput(io *iostreams.IOStreams, state *IssueMetadataState, createErr issueType = "issue" } - fmt.Fprintf(out, "%s operation failed. input saved to: %s\n", cs.FailureIcon(), tmpfilePath) - fmt.Fprintf(out, "resubmit with: gh %s create -j@%s\n", issueType, tmpfilePath) + fmt.Fprintf(out, "%s operation failed. recover with: gh %s create -e%s\n", cs.FailureIcon(), issueType, tmpfile.Name()) // some whitespace before the actual error fmt.Fprintln(out) diff --git a/pkg/cmd/pr/shared/preserve_test.go b/pkg/cmd/pr/shared/preserve_test.go index a4211e55f..fc6450164 100644 --- a/pkg/cmd/pr/shared/preserve_test.go +++ b/pkg/cmd/pr/shared/preserve_test.go @@ -5,7 +5,6 @@ import ( "errors" "io/ioutil" "os" - "path/filepath" "testing" "github.com/cli/cli/pkg/iostreams" @@ -18,7 +17,7 @@ func Test_PreserveInput(t *testing.T) { name string state *IssueMetadataState err bool - wantErrLines []string + wantErrLine string wantPreservation bool }{ { @@ -44,10 +43,7 @@ func Test_PreserveInput(t *testing.T) { Reviewers: []string{"barry", "chris"}, Labels: []string{"sandwich"}, }, - wantErrLines: []string{ - `X operation failed. input saved to:.*testfile.*`, - `resubmit with: gh issue create -j@.*testfile.*`, - }, + wantErrLine: `X operation failed. recover with: gh issue create -e.*testfile.*`, err: true, wantPreservation: true, }, @@ -57,10 +53,7 @@ func Test_PreserveInput(t *testing.T) { Reviewers: []string{"barry", "chris"}, Labels: []string{"sandwich"}, }, - wantErrLines: []string{ - `X operation failed. input saved to:.*testfile.*`, - `resubmit with: gh issue create -j@.*testfile.*`, - }, + wantErrLine: `X operation failed. recover with: gh issue create -e.*testfile.*`, err: true, wantPreservation: true, }, @@ -71,10 +64,7 @@ func Test_PreserveInput(t *testing.T) { Title: "a pull request", Type: PRMetadata, }, - wantErrLines: []string{ - `X operation failed. input saved to:.*testfile.*`, - `resubmit with: gh pr create -j@.*testfile.*`, - }, + wantErrLine: `X operation failed. recover with: gh pr create -e.*testfile.*`, err: true, wantPreservation: true, }, @@ -88,9 +78,9 @@ func Test_PreserveInput(t *testing.T) { io, _, _, errOut := iostreams.Test() - tfPath, tf, tferr := tmpfile() + tf, tferr := tmpfile() assert.NoError(t, tferr) - defer os.Remove(tfPath) + defer os.Remove(tf.Name()) io.TempFileOverride = tf @@ -108,7 +98,7 @@ func Test_PreserveInput(t *testing.T) { assert.NoError(t, err) if tt.wantPreservation { - test.ExpectLines(t, errOut.String(), tt.wantErrLines...) + test.ExpectLines(t, errOut.String(), tt.wantErrLine) preserved := &IssueMetadataState{} assert.NoError(t, json.Unmarshal(data, preserved)) preserved.dirty = tt.state.dirty @@ -121,12 +111,12 @@ func Test_PreserveInput(t *testing.T) { } } -func tmpfile() (string, *os.File, error) { +func tmpfile() (*os.File, error) { dir := os.TempDir() tmpfile, err := ioutil.TempFile(dir, "testfile*") if err != nil { - return "", nil, err + return nil, err } - return filepath.Join(dir, tmpfile.Name()), tmpfile, nil + return tmpfile, nil } diff --git a/pkg/cmd/pr/shared/state.go b/pkg/cmd/pr/shared/state.go index 6c8e3531e..930a69fcc 100644 --- a/pkg/cmd/pr/shared/state.go +++ b/pkg/cmd/pr/shared/state.go @@ -3,7 +3,6 @@ package shared import ( "encoding/json" "fmt" - "strings" "github.com/cli/cli/api" "github.com/cli/cli/pkg/iostreams" @@ -52,16 +51,12 @@ func (tb *IssueMetadataState) HasMetadata() bool { len(tb.Milestones) > 0 } -func FillFromJSON(io *iostreams.IOStreams, JSONInput string, state *IssueMetadataState) error { +func FillFromJSON(io *iostreams.IOStreams, recoverFile string, state *IssueMetadataState) error { var data []byte var err error - if strings.HasPrefix(JSONInput, "@") { - data, err = io.ReadUserFile(JSONInput[1:]) - if err != nil { - return fmt.Errorf("failed to read file %s: %w", JSONInput[1:], err) - } - } else { - data = []byte(JSONInput) + data, err = io.ReadUserFile(recoverFile) + if err != nil { + return fmt.Errorf("failed to read file %s: %w", recoverFile, err) } err = json.Unmarshal(data, state) diff --git a/pkg/iostreams/iostreams.go b/pkg/iostreams/iostreams.go index ae200e3d1..5df44098d 100644 --- a/pkg/iostreams/iostreams.go +++ b/pkg/iostreams/iostreams.go @@ -272,7 +272,6 @@ func (s *IOStreams) ReadUserFile(fn string) ([]byte, error) { func (s *IOStreams) TempFile(dir, pattern string) (*os.File, error) { if s.TempFileOverride != nil { - fmt.Printf("DBG %#v\n", s.TempFileOverride) return s.TempFileOverride, nil } return ioutil.TempFile(dir, pattern) From cf37ce74634be6e6f5fc6b7f506141712d95b592 Mon Sep 17 00:00:00 2001 From: vilmibm Date: Mon, 23 Nov 2020 11:20:27 -0800 Subject: [PATCH 21/43] no shorthand for --recover --- pkg/cmd/issue/create/create.go | 2 +- pkg/cmd/issue/create/create_test.go | 2 +- pkg/cmd/pr/create/create.go | 2 +- pkg/cmd/pr/create/create_test.go | 2 +- pkg/cmd/pr/shared/preserve.go | 2 +- pkg/cmd/pr/shared/preserve_test.go | 6 +++--- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/pkg/cmd/issue/create/create.go b/pkg/cmd/issue/create/create.go index a2912f638..44cac6ae1 100644 --- a/pkg/cmd/issue/create/create.go +++ b/pkg/cmd/issue/create/create.go @@ -88,7 +88,7 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co cmd.Flags().StringSliceVarP(&opts.Labels, "label", "l", nil, "Add labels by `name`") cmd.Flags().StringSliceVarP(&opts.Projects, "project", "p", nil, "Add the issue to projects by `name`") cmd.Flags().StringVarP(&opts.Milestone, "milestone", "m", "", "Add the issue to a milestone by `name`") - cmd.Flags().StringVarP(&opts.RecoverFile, "recover", "e", "", "Recover input from a failed run of create") + cmd.Flags().StringVar(&opts.RecoverFile, "recover", "", "Recover input from a failed run of create") return cmd } diff --git a/pkg/cmd/issue/create/create_test.go b/pkg/cmd/issue/create/create_test.go index 733094964..d87411f18 100644 --- a/pkg/cmd/issue/create/create_test.go +++ b/pkg/cmd/issue/create/create_test.go @@ -199,7 +199,7 @@ func TestIssueCreate_recover(t *testing.T) { _, err = tmpfile.Write(data) assert.NoError(t, err) - args := fmt.Sprintf("-e '%s'", tmpfile.Name()) + args := fmt.Sprintf("--recover '%s'", tmpfile.Name()) output, err := runCommandWithRootDirOverridden(http, true, args, "") if err != nil { diff --git a/pkg/cmd/pr/create/create.go b/pkg/cmd/pr/create/create.go index 502de1126..8748582eb 100644 --- a/pkg/cmd/pr/create/create.go +++ b/pkg/cmd/pr/create/create.go @@ -139,7 +139,7 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co fl.StringSliceVarP(&opts.Labels, "label", "l", nil, "Add labels by `name`") fl.StringSliceVarP(&opts.Projects, "project", "p", nil, "Add the pull request to projects by `name`") fl.StringVarP(&opts.Milestone, "milestone", "m", "", "Add the pull request to a milestone by `name`") - fl.StringVarP(&opts.RecoverFile, "recover", "e", "", "Recover input from a failed run of create") + fl.StringVar(&opts.RecoverFile, "recover", "", "Recover input from a failed run of create") return cmd } diff --git a/pkg/cmd/pr/create/create_test.go b/pkg/cmd/pr/create/create_test.go index 2c9fd6a41..7f703cef3 100644 --- a/pkg/cmd/pr/create/create_test.go +++ b/pkg/cmd/pr/create/create_test.go @@ -220,7 +220,7 @@ func TestPRCreate_recover(t *testing.T) { _, err = tmpfile.Write(data) assert.NoError(t, err) - args := fmt.Sprintf("-e '%s' -Hfeature", tmpfile.Name()) + args := fmt.Sprintf("--recover '%s' -Hfeature", tmpfile.Name()) output, err := runCommandWithRootDirOverridden(http, nil, "feature", true, args, "") assert.NoError(t, err) diff --git a/pkg/cmd/pr/shared/preserve.go b/pkg/cmd/pr/shared/preserve.go index 2bd5729ff..4105823cd 100644 --- a/pkg/cmd/pr/shared/preserve.go +++ b/pkg/cmd/pr/shared/preserve.go @@ -54,7 +54,7 @@ func PreserveInput(io *iostreams.IOStreams, state *IssueMetadataState, createErr issueType = "issue" } - fmt.Fprintf(out, "%s operation failed. recover with: gh %s create -e%s\n", cs.FailureIcon(), issueType, tmpfile.Name()) + fmt.Fprintf(out, "%s operation failed. To restore: gh %s create --recover %s\n", cs.FailureIcon(), issueType, tmpfile.Name()) // some whitespace before the actual error fmt.Fprintln(out) diff --git a/pkg/cmd/pr/shared/preserve_test.go b/pkg/cmd/pr/shared/preserve_test.go index fc6450164..28949d957 100644 --- a/pkg/cmd/pr/shared/preserve_test.go +++ b/pkg/cmd/pr/shared/preserve_test.go @@ -43,7 +43,7 @@ func Test_PreserveInput(t *testing.T) { Reviewers: []string{"barry", "chris"}, Labels: []string{"sandwich"}, }, - wantErrLine: `X operation failed. recover with: gh issue create -e.*testfile.*`, + wantErrLine: `X operation failed. To restore: gh issue create --recover .*testfile.*`, err: true, wantPreservation: true, }, @@ -53,7 +53,7 @@ func Test_PreserveInput(t *testing.T) { Reviewers: []string{"barry", "chris"}, Labels: []string{"sandwich"}, }, - wantErrLine: `X operation failed. recover with: gh issue create -e.*testfile.*`, + wantErrLine: `X operation failed. To restore: gh issue create --recover .*testfile.*`, err: true, wantPreservation: true, }, @@ -64,7 +64,7 @@ func Test_PreserveInput(t *testing.T) { Title: "a pull request", Type: PRMetadata, }, - wantErrLine: `X operation failed. recover with: gh pr create -e.*testfile.*`, + wantErrLine: `X operation failed. To restore: gh pr create --recover .*testfile.*`, err: true, wantPreservation: true, }, From 9f84f0ffa1d5b3141399b7ed499eb81b69ef74d2 Mon Sep 17 00:00:00 2001 From: Shubhankar Kanchan Gupta Date: Tue, 24 Nov 2020 17:26:26 +0530 Subject: [PATCH 22/43] Warn termux users with older Android versions (#2467) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Mislav Marohnić --- docs/install_linux.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/install_linux.md b/docs/install_linux.md index d5392ba47..697bfb8f8 100644 --- a/docs/install_linux.md +++ b/docs/install_linux.md @@ -95,7 +95,7 @@ sudo pacman -S github-cli ### Android -Android users can install via Termux: +Android 7+ users can install via [Termux](https://wiki.termux.com/wiki/Main_Page): ```bash pkg install gh From ea50666c304f33774ff39a58cb8ffdf37b7b7a54 Mon Sep 17 00:00:00 2001 From: Cristian Dominguez Date: Tue, 24 Nov 2020 13:49:04 -0300 Subject: [PATCH 23/43] Prompt: avoid resetting PR/issue metadata if no option is selected --- pkg/cmd/pr/shared/survey.go | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/pkg/cmd/pr/shared/survey.go b/pkg/cmd/pr/shared/survey.go index 4e4a72855..30d9073c7 100644 --- a/pkg/cmd/pr/shared/survey.go +++ b/pkg/cmd/pr/shared/survey.go @@ -345,17 +345,22 @@ func MetadataSurvey(io *iostreams.IOStreams, client *api.Client, baseRepo ghrepo fmt.Fprintln(io.ErrOut, "warning: no milestones in the repository") } } - values := metadataValues{} - err = prompt.SurveyAsk(mqs, &values, survey.WithKeepFilter(true)) - if err != nil { - return fmt.Errorf("could not prompt: %w", err) - } - state.Reviewers = values.Reviewers - state.Assignees = values.Assignees - state.Labels = values.Labels - state.Projects = values.Projects - if values.Milestone != "" && values.Milestone != noMilestone { - state.Milestones = []string{values.Milestone} + + if len(mqs) > 0 { + values := metadataValues{} + err = prompt.SurveyAsk(mqs, &values, survey.WithKeepFilter(true)) + if err != nil { + return fmt.Errorf("could not prompt: %w", err) + } + state.Reviewers = values.Reviewers + state.Assignees = values.Assignees + state.Labels = values.Labels + state.Projects = values.Projects + if values.Milestone != "" && values.Milestone != noMilestone { + state.Milestones = []string{values.Milestone} + } + } else { + state.MetadataResult = nil } return nil From 37891a54d93a7c54e0a1d589399f3dda9589f535 Mon Sep 17 00:00:00 2001 From: Vixb Date: Wed, 25 Nov 2020 18:40:30 +0800 Subject: [PATCH 24/43] Update scoop install option (#2478) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Mislav Marohnić Co-authored-by: Jan Pokorný --- README.md | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 146b44d1c..0c2ea4f48 100644 --- a/README.md +++ b/README.md @@ -55,18 +55,9 @@ For more information and distro-specific instructions, see the [Linux installati #### scoop -Install: - -```powershell -scoop bucket add github-gh https://github.com/cli/scoop-gh.git -scoop install gh -``` - -Upgrade: - -```powershell -scoop update gh -``` +| Install: | Upgrade: | +| ------------------ | ------------------ | +| `scoop install gh` | `scoop update gh` | #### Chocolatey From 21e2544d73eb38435c1774ec231a118559062848 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 25 Nov 2020 12:06:35 +0100 Subject: [PATCH 25/43] Sort latest PRs first when looking up PRs for a branch Fixes #2452 --- api/queries_pr.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/api/queries_pr.go b/api/queries_pr.go index a95feab28..4433ef373 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -639,7 +639,7 @@ func PullRequestForBranch(client *Client, repo ghrepo.Interface, baseBranch, hea query := ` query PullRequestForBranch($owner: String!, $repo: String!, $headRefName: String!, $states: [PullRequestState!]) { repository(owner: $owner, name: $repo) { - pullRequests(headRefName: $headRefName, states: $states, first: 30) { + pullRequests(headRefName: $headRefName, states: $states, first: 30, orderBy: { field: CREATED_AT, direction: DESC }) { nodes { id number From e9e8f207cc5c051b6a505dc0c3bb373a703556c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 25 Nov 2020 14:52:13 +0100 Subject: [PATCH 26/43] Bump AlecAivazis/survey --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index ec44ad44e..bd4e69068 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,7 @@ module github.com/cli/cli go 1.13 require ( - github.com/AlecAivazis/survey/v2 v2.1.1 + github.com/AlecAivazis/survey/v2 v2.2.3 github.com/MakeNowJust/heredoc v1.0.0 github.com/briandowns/spinner v1.11.1 github.com/charmbracelet/glamour v0.2.1-0.20200724174618-1246d13c1684 diff --git a/go.sum b/go.sum index fa1ef7c15..ba242e9dc 100644 --- a/go.sum +++ b/go.sum @@ -11,8 +11,8 @@ cloud.google.com/go/firestore v1.1.0/go.mod h1:ulACoGHTpvq5r8rxGJ4ddJZBZqakUQqCl cloud.google.com/go/pubsub v1.0.1/go.mod h1:R0Gpsv3s54REJCy4fxDixWD93lHJMoZTyQ2kNxGRt3I= cloud.google.com/go/storage v1.0.0/go.mod h1:IhtSnM/ZTZV8YYJWCY8RULGVqBDmpoyjwiyrjsg+URw= dmitri.shuralyov.com/gpu/mtl v0.0.0-20190408044501-666a987793e9/go.mod h1:H6x//7gZCb22OMCxBHrMx7a5I7Hp++hsVxbQ4BYO7hU= -github.com/AlecAivazis/survey/v2 v2.1.1 h1:LEMbHE0pLj75faaVEKClEX1TM4AJmmnOh9eimREzLWI= -github.com/AlecAivazis/survey/v2 v2.1.1/go.mod h1:9FJRdMdDm8rnT+zHVbvQT2RTSTLq0Ttd6q3Vl2fahjk= +github.com/AlecAivazis/survey/v2 v2.2.3 h1:utJR2X4Ibp2fBxdjalQUiMFf3zfQNjA15YE8+ftlKEs= +github.com/AlecAivazis/survey/v2 v2.2.3/go.mod h1:9FJRdMdDm8rnT+zHVbvQT2RTSTLq0Ttd6q3Vl2fahjk= github.com/BurntSushi/toml v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ= github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802/go.mod h1:IVnqGOEym/WlBOVXweHU+Q+/VP0lqqI8lqeDx9IjBqo= From ab05736b9806fce952df96e8ac953afa35ae3462 Mon Sep 17 00:00:00 2001 From: Cristian Dominguez Date: Wed, 25 Nov 2020 13:30:54 -0300 Subject: [PATCH 27/43] don't reset previously added metadata --- pkg/cmd/pr/shared/survey.go | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/pkg/cmd/pr/shared/survey.go b/pkg/cmd/pr/shared/survey.go index 30d9073c7..d3838925e 100644 --- a/pkg/cmd/pr/shared/survey.go +++ b/pkg/cmd/pr/shared/survey.go @@ -347,11 +347,21 @@ func MetadataSurvey(io *iostreams.IOStreams, client *api.Client, baseRepo ghrepo } if len(mqs) > 0 { - values := metadataValues{} + values := metadataValues{ + Reviewers: state.Reviewers, + Assignees: state.Assignees, + Labels: state.Labels, + Projects: state.Projects, + } + if len(state.Milestones) > 0 { + values.Milestone = state.Milestones[0] + } + err = prompt.SurveyAsk(mqs, &values, survey.WithKeepFilter(true)) if err != nil { return fmt.Errorf("could not prompt: %w", err) } + state.Reviewers = values.Reviewers state.Assignees = values.Assignees state.Labels = values.Labels @@ -359,10 +369,10 @@ func MetadataSurvey(io *iostreams.IOStreams, client *api.Client, baseRepo ghrepo if values.Milestone != "" && values.Milestone != noMilestone { state.Milestones = []string{values.Milestone} } - } else { - state.MetadataResult = nil } + state.MetadataResult = nil + return nil } From 436846a7154f5261349138604e5cb27ce0f9d6ac Mon Sep 17 00:00:00 2001 From: Amanda Pinsker Date: Wed, 25 Nov 2020 11:58:26 -0800 Subject: [PATCH 28/43] Add design system docs to contributing --- .github/CONTRIBUTING.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index c15d1e8b0..1ed4766ad 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -44,6 +44,10 @@ Please note that this project adheres to a [Contributor Code of Conduct][code-of We generate manual pages from source on every release. You do not need to submit pull requests for documentation specifically; manual pages for commands will automatically get updated after your pull requests gets accepted. +## Design guidelines + +You may reference the [CLI Design System][] when suggesting features, and are welcome to use our [Google Docs Template][] to suggest designs. + ## Resources - [How to Contribute to Open Source][] @@ -61,3 +65,5 @@ We generate manual pages from source on every release. You do not need to submit [How to Contribute to Open Source]: https://opensource.guide/how-to-contribute/ [Using Pull Requests]: https://docs.github.com/en/free-pro-team@latest/github/collaborating-with-issues-and-pull-requests/about-pull-requests [GitHub Help]: https://docs.github.com/ +[CLI Design System]: https://primer.style/cli/ +[Google Docs Template]: https://docs.google.com/document/d/1JIRErIUuJ6fTgabiFYfCH3x91pyHuytbfa0QLnTfXKM/edit#heading=h.or54sa47ylpg From 1135e5e3ededb4261293b4e2b2fdeab6f79c0738 Mon Sep 17 00:00:00 2001 From: Zach Boyle <33520963+zaboyle@users.noreply.github.com> Date: Thu, 26 Nov 2020 05:54:28 -0500 Subject: [PATCH 29/43] set delete-branch merge flag default to false (#2466) Co-authored-by: Divya Ramanathan --- pkg/cmd/pr/merge/merge.go | 5 +---- pkg/cmd/pr/merge/merge_test.go | 24 +++--------------------- 2 files changed, 4 insertions(+), 25 deletions(-) diff --git a/pkg/cmd/pr/merge/merge.go b/pkg/cmd/pr/merge/merge.go index 2eafe9712..d72ed2ca0 100644 --- a/pkg/cmd/pr/merge/merge.go +++ b/pkg/cmd/pr/merge/merge.go @@ -54,9 +54,6 @@ func NewCmdMerge(f *cmdutil.Factory, runF func(*MergeOptions) error) *cobra.Comm Short: "Merge a pull request", Long: heredoc.Doc(` Merge a pull request on GitHub. - - By default, the head branch of the pull request will get deleted on both remote and local repositories. - To retain the branch, use '--delete-branch=false'. `), Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { @@ -102,7 +99,7 @@ func NewCmdMerge(f *cmdutil.Factory, runF func(*MergeOptions) error) *cobra.Comm }, } - cmd.Flags().BoolVarP(&opts.DeleteBranch, "delete-branch", "d", true, "Delete the local and remote branch after merge") + cmd.Flags().BoolVarP(&opts.DeleteBranch, "delete-branch", "d", false, "Delete the local and remote branch after merge") cmd.Flags().BoolVarP(&flagMerge, "merge", "m", false, "Merge the commits with the base branch") cmd.Flags().BoolVarP(&flagRebase, "rebase", "r", false, "Rebase the commits onto the base branch") cmd.Flags().BoolVarP(&flagSquash, "squash", "s", false, "Squash the commits into one commit and merge it into the base branch") diff --git a/pkg/cmd/pr/merge/merge_test.go b/pkg/cmd/pr/merge/merge_test.go index 2815617d2..6cd0e19dc 100644 --- a/pkg/cmd/pr/merge/merge_test.go +++ b/pkg/cmd/pr/merge/merge_test.go @@ -38,7 +38,7 @@ func Test_NewCmdMerge(t *testing.T) { isTTY: true, want: MergeOptions{ SelectorArg: "123", - DeleteBranch: true, + DeleteBranch: false, DeleteLocalBranch: true, MergeMethod: api.PullRequestMergeMethodMerge, InteractiveMode: true, @@ -192,9 +192,6 @@ func TestPrMerge(t *testing.T) { assert.Equal(t, "MERGE", input["mergeMethod"].(string)) assert.NotContains(t, input, "commitHeadline") })) - http.Register( - httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"), - httpmock.StringResponse(`{}`)) cs, cmdTeardown := test.InitCmdStubber() defer cmdTeardown() @@ -238,9 +235,6 @@ func TestPrMerge_nontty(t *testing.T) { assert.Equal(t, "MERGE", input["mergeMethod"].(string)) assert.NotContains(t, input, "commitHeadline") })) - http.Register( - httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"), - httpmock.StringResponse(`{}`)) cs, cmdTeardown := test.InitCmdStubber() defer cmdTeardown() @@ -281,9 +275,6 @@ func TestPrMerge_withRepoFlag(t *testing.T) { assert.Equal(t, "MERGE", input["mergeMethod"].(string)) assert.NotContains(t, input, "commitHeadline") })) - http.Register( - httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"), - httpmock.StringResponse(`{}`)) cs, cmdTeardown := test.InitCmdStubber() defer cmdTeardown() @@ -385,9 +376,6 @@ func TestPrMerge_noPrNumberGiven(t *testing.T) { assert.Equal(t, "MERGE", input["mergeMethod"].(string)) assert.NotContains(t, input, "commitHeadline") })) - http.Register( - httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"), - httpmock.StringResponse(`{}`)) cs, cmdTeardown := test.InitCmdStubber() defer cmdTeardown() @@ -431,9 +419,6 @@ func TestPrMerge_rebase(t *testing.T) { assert.Equal(t, "REBASE", input["mergeMethod"].(string)) assert.NotContains(t, input, "commitHeadline") })) - http.Register( - httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"), - httpmock.StringResponse(`{}`)) cs, cmdTeardown := test.InitCmdStubber() defer cmdTeardown() @@ -476,9 +461,6 @@ func TestPrMerge_squash(t *testing.T) { assert.Equal(t, "SQUASH", input["mergeMethod"].(string)) assert.Equal(t, "The title of the PR (#3)", input["commitHeadline"].(string)) })) - http.Register( - httpmock.REST("DELETE", "repos/OWNER/REPO/git/refs/heads/blueberries"), - httpmock.StringResponse(`{}`)) cs, cmdTeardown := test.InitCmdStubber() defer cmdTeardown() @@ -493,7 +475,7 @@ func TestPrMerge_squash(t *testing.T) { t.Fatalf("error running command `pr merge`: %v", err) } - test.ExpectLines(t, output.Stderr(), "Squashed and merged pull request #3", `Deleted branch.*blueberries`) + test.ExpectLines(t, output.Stderr(), "Squashed and merged pull request #3") } func TestPrMerge_alreadyMerged(t *testing.T) { @@ -581,7 +563,7 @@ func TestPRMerge_interactive(t *testing.T) { t.Fatalf("Got unexpected error running `pr merge` %s", err) } - test.ExpectLines(t, output.Stderr(), "Merged pull request #3", `Deleted branch.*blueberries`) + test.ExpectLines(t, output.Stderr(), "Merged pull request #3") } func TestPRMerge_interactiveCancelled(t *testing.T) { From 34d549e7b61660c7c993181c0be046d6277cad03 Mon Sep 17 00:00:00 2001 From: Max Horstmann Date: Thu, 26 Nov 2020 11:31:15 -0500 Subject: [PATCH 30/43] Document that reviewers can be teams (#2465) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Mislav Marohnić --- pkg/cmd/pr/create/create.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/pr/create/create.go b/pkg/cmd/pr/create/create.go index 8748582eb..0b5b2b306 100644 --- a/pkg/cmd/pr/create/create.go +++ b/pkg/cmd/pr/create/create.go @@ -94,7 +94,7 @@ 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 pr create --reviewer monalisa,hubot + $ gh pr create --reviewer monalisa,hubot --reviewer myorg/team-name $ gh pr create --project "Roadmap" $ gh pr create --base develop --head monalisa:feature `), @@ -134,7 +134,7 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co 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`") + fl.StringSliceVarP(&opts.Reviewers, "reviewer", "r", nil, "Request reviews from people or teams by their `handle`") fl.StringSliceVarP(&opts.Assignees, "assignee", "a", nil, "Assign people by their `login`") fl.StringSliceVarP(&opts.Labels, "label", "l", nil, "Add labels by `name`") fl.StringSliceVarP(&opts.Projects, "project", "p", nil, "Add the pull request to projects by `name`") From da3287c26cc7e39cddee46363df8821a305c7a8c Mon Sep 17 00:00:00 2001 From: Ismael Luceno Date: Sat, 21 Nov 2020 21:46:32 +0100 Subject: [PATCH 31/43] Add make (un)install targets for POSIX systems The implementation imitates the behavior of build-systems generated by GNU Automake. Implemented targets: - install - install-strip - uninstall Implemented variables: - DESTDIR - prefix - bindir - INSTALL_STRIP_FLAG Internal implementation details: - install-bins variable collects user binaries to be installed - install-dirs variable collects directories to be created --- Makefile | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/Makefile b/Makefile index 859461984..a5b5f815a 100644 --- a/Makefile +++ b/Makefile @@ -26,6 +26,7 @@ ifdef GH_OAUTH_CLIENT_SECRET GO_LDFLAGS := -X github.com/cli/cli/internal/authflow.oauthClientSecret=$(GH_OAUTH_CLIENT_SECRET) $(GO_LDFLAGS) endif +install-bins += bin/gh bin/gh: $(BUILD_FILES) @go build -trimpath -ldflags "$(GO_LDFLAGS)" -o "$@" ./cmd/gh @@ -62,3 +63,20 @@ endif .PHONY: manpages manpages: go run ./cmd/gen-docs --man-page --doc-path ./share/man/man1/ + +DESTDIR := +prefix := /usr/local +bindir := ${prefix}/bin + +.PHONY: install install-strip uninstall +INSTALL_STRIP_FLAG = +install-strip: + @${MAKE} INSTALL_STRIP_FLAG=-s install + +install: ${install-bins} + install -d ${DESTDIR}${bindir} + install -m555 ${INSTALL_STRIP_FLAG} ${install-bins} ${DESTDIR}${bindir}/ + +remove-bins := ${install-bins:bin/%=${DESTDIR}${bindir}/%} +uninstall: + rm -f ${remove-bins} From 8d2881d5ea4412439179ca748e7ba47a70e6e4af Mon Sep 17 00:00:00 2001 From: Ismael Luceno Date: Sun, 29 Nov 2020 20:36:32 +0100 Subject: [PATCH 32/43] Install manual pages --- .gitignore | 1 + Makefile | 12 +++++++++--- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/.gitignore b/.gitignore index 9895939a6..8f460efe7 100644 --- a/.gitignore +++ b/.gitignore @@ -6,6 +6,7 @@ /site .github/**/node_modules /CHANGELOG.md +/manpages.list # VS Code .vscode diff --git a/Makefile b/Makefile index a5b5f815a..b6db4eab9 100644 --- a/Makefile +++ b/Makefile @@ -61,22 +61,28 @@ endif .PHONY: manpages -manpages: +manpages: manpages.list +manpages.list: go run ./cmd/gen-docs --man-page --doc-path ./share/man/man1/ + find share/man -type f > $@ DESTDIR := prefix := /usr/local bindir := ${prefix}/bin +mandir := ${prefix}/share/man .PHONY: install install-strip uninstall INSTALL_STRIP_FLAG = install-strip: @${MAKE} INSTALL_STRIP_FLAG=-s install -install: ${install-bins} +install: ${install-bins} manpages.list install -d ${DESTDIR}${bindir} install -m555 ${INSTALL_STRIP_FLAG} ${install-bins} ${DESTDIR}${bindir}/ + install -d ${DESTDIR}${mandir}/man1 + install -m444 $(shell cat manpages.list) ${DESTDIR}${mandir}/man1/ remove-bins := ${install-bins:bin/%=${DESTDIR}${bindir}/%} -uninstall: +uninstall: manpages.list rm -f ${remove-bins} + rm -f $(patsubst share/man/%,${DESTDIR}${mandir}/%,$(shell cat manpages.list)) From 413ccb71cce962dfeddbda86a2a4dab563efa630 Mon Sep 17 00:00:00 2001 From: Nils Leif Fischer Date: Sun, 29 Nov 2020 16:07:43 +0100 Subject: [PATCH 33/43] Delete an error message that is not useful (and had a typo) --- cmd/gh/main.go | 1 - 1 file changed, 1 deletion(-) diff --git a/cmd/gh/main.go b/cmd/gh/main.go index ec505d4b8..61cbf7081 100644 --- a/cmd/gh/main.go +++ b/cmd/gh/main.go @@ -140,7 +140,6 @@ func main() { fmt.Fprintln(stderr, cs.Bold("Welcome to GitHub CLI!")) fmt.Fprintln(stderr) fmt.Fprintln(stderr, "To authenticate, please run `gh auth login`.") - fmt.Fprintln(stderr, "You can also set the one of the auth token environment variables, if preferred.") os.Exit(4) } From dc1fad9cb093959bf3b9a4a388964b242bef7d24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 1 Dec 2020 15:28:39 +0100 Subject: [PATCH 34/43] Fix respecting chosen action in interactive `issue create` The `action` variable started being shadowed in the `if` block in 6671106448ed7b342797b426e15af032dfe3b1be --- pkg/cmd/issue/create/create.go | 1 - pkg/cmd/issue/create/create_test.go | 57 +++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/issue/create/create.go b/pkg/cmd/issue/create/create.go index 44cac6ae1..8df05d723 100644 --- a/pkg/cmd/issue/create/create.go +++ b/pkg/cmd/issue/create/create.go @@ -199,7 +199,6 @@ func createRun(opts *CreateOptions) (err error) { } } - var action prShared.Action action, err = prShared.ConfirmSubmission(!tb.HasMetadata(), repo.ViewerCanTriage()) if err != nil { err = fmt.Errorf("unable to confirm: %w", err) diff --git a/pkg/cmd/issue/create/create_test.go b/pkg/cmd/issue/create/create_test.go index d87411f18..3a62e12e5 100644 --- a/pkg/cmd/issue/create/create_test.go +++ b/pkg/cmd/issue/create/create_test.go @@ -12,6 +12,7 @@ import ( "strings" "testing" + "github.com/MakeNowJust/heredoc" "github.com/cli/cli/internal/config" "github.com/cli/cli/internal/ghrepo" "github.com/cli/cli/internal/run" @@ -274,6 +275,62 @@ func TestIssueCreate_nonLegacyTemplate(t *testing.T) { eq(t, output.String(), "https://github.com/OWNER/REPO/issues/12\n") } +func TestIssueCreate_continueInBrowser(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { + "id": "REPOID", + "hasIssuesEnabled": true + } } } + `)) + + as, teardown := prompt.InitAskStubber() + defer teardown() + + // title + as.Stub([]*prompt.QuestionStub{ + { + Name: "Title", + Value: "hello", + }, + }) + // confirm + as.Stub([]*prompt.QuestionStub{ + { + Name: "confirmation", + Value: 1, + }, + }) + + var seenCmd *exec.Cmd + restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { + seenCmd = cmd + return &test.OutputStub{} + }) + defer restoreCmd() + + output, err := runCommand(http, true, `-b body`) + if err != nil { + t.Errorf("error running command `issue create`: %v", err) + } + + assert.Equal(t, "", output.String()) + assert.Equal(t, heredoc.Doc(` + + Creating issue in OWNER/REPO + + Opening github.com/OWNER/REPO/issues/new in your browser. + `), output.Stderr()) + + if seenCmd == nil { + t.Fatal("expected a command to run") + } + url := seenCmd.Args[len(seenCmd.Args)-1] + assert.Equal(t, "https://github.com/OWNER/REPO/issues/new?body=body&title=hello", url) +} + func TestIssueCreate_metadata(t *testing.T) { http := &httpmock.Registry{} defer http.Verify(t) From c92f416cc0a590cdef6e6b8bef88b91306e66a5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 1 Dec 2020 15:46:18 +0100 Subject: [PATCH 35/43] Simplify `make install/uninstall` --- .gitignore | 1 - Makefile | 25 ++++++++----------------- 2 files changed, 8 insertions(+), 18 deletions(-) diff --git a/.gitignore b/.gitignore index 8f460efe7..9895939a6 100644 --- a/.gitignore +++ b/.gitignore @@ -6,7 +6,6 @@ /site .github/**/node_modules /CHANGELOG.md -/manpages.list # VS Code .vscode diff --git a/Makefile b/Makefile index b6db4eab9..8ee9e7dc6 100644 --- a/Makefile +++ b/Makefile @@ -26,7 +26,6 @@ ifdef GH_OAUTH_CLIENT_SECRET GO_LDFLAGS := -X github.com/cli/cli/internal/authflow.oauthClientSecret=$(GH_OAUTH_CLIENT_SECRET) $(GO_LDFLAGS) endif -install-bins += bin/gh bin/gh: $(BUILD_FILES) @go build -trimpath -ldflags "$(GO_LDFLAGS)" -o "$@" ./cmd/gh @@ -59,30 +58,22 @@ endif git -C site commit -m '$(GITHUB_REF:refs/tags/v%=%)' index.html .PHONY: site-bump - .PHONY: manpages -manpages: manpages.list -manpages.list: +manpages: go run ./cmd/gen-docs --man-page --doc-path ./share/man/man1/ - find share/man -type f > $@ DESTDIR := prefix := /usr/local bindir := ${prefix}/bin mandir := ${prefix}/share/man -.PHONY: install install-strip uninstall -INSTALL_STRIP_FLAG = -install-strip: - @${MAKE} INSTALL_STRIP_FLAG=-s install - -install: ${install-bins} manpages.list +.PHONY: install +install: bin/gh manpages install -d ${DESTDIR}${bindir} - install -m555 ${INSTALL_STRIP_FLAG} ${install-bins} ${DESTDIR}${bindir}/ + install -m755 bin/gh ${DESTDIR}${bindir}/ install -d ${DESTDIR}${mandir}/man1 - install -m444 $(shell cat manpages.list) ${DESTDIR}${mandir}/man1/ + install -m644 ./share/man/man1/* ${DESTDIR}${mandir}/man1/ -remove-bins := ${install-bins:bin/%=${DESTDIR}${bindir}/%} -uninstall: manpages.list - rm -f ${remove-bins} - rm -f $(patsubst share/man/%,${DESTDIR}${mandir}/%,$(shell cat manpages.list)) +.PHONY: uninstall +uninstall: + rm -f ${DESTDIR}${bindir}/gh ${DESTDIR}${mandir}/man1/gh.1 ${DESTDIR}${mandir}/man1/gh-*.1 From df2ca9c9f9e98a17e4926854bd2ab7c9d916475e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 1 Dec 2020 15:55:40 +0100 Subject: [PATCH 36/43] Fix browser URL test on Windows --- pkg/cmd/issue/create/create_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/issue/create/create_test.go b/pkg/cmd/issue/create/create_test.go index 3a62e12e5..d7e6b4ec0 100644 --- a/pkg/cmd/issue/create/create_test.go +++ b/pkg/cmd/issue/create/create_test.go @@ -327,7 +327,7 @@ func TestIssueCreate_continueInBrowser(t *testing.T) { if seenCmd == nil { t.Fatal("expected a command to run") } - url := seenCmd.Args[len(seenCmd.Args)-1] + url := strings.ReplaceAll(seenCmd.Args[len(seenCmd.Args)-1], "^", "") assert.Equal(t, "https://github.com/OWNER/REPO/issues/new?body=body&title=hello", url) } From e21c5100fa02696073100a9a94684bd889ad4c6f Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Tue, 1 Dec 2020 11:44:14 -0500 Subject: [PATCH 37/43] Properly check env auth tokens in CheckAuth --- internal/config/from_env.go | 7 ++++ internal/config/from_env_test.go | 57 ++++++++++++++++++++++++++++++++ pkg/cmdutil/auth_check.go | 4 +++ pkg/cmdutil/auth_check_test.go | 21 ++++++++++++ 4 files changed, 89 insertions(+) diff --git a/internal/config/from_env.go b/internal/config/from_env.go index da4ac1536..333dc879b 100644 --- a/internal/config/from_env.go +++ b/internal/config/from_env.go @@ -78,3 +78,10 @@ func AuthTokenFromEnv(hostname string) (string, string) { return os.Getenv(GITHUB_TOKEN), GITHUB_TOKEN } + +func AuthTokenProvidedFromEnv() bool { + return os.Getenv(GH_ENTERPRISE_TOKEN) != "" || + os.Getenv(GITHUB_ENTERPRISE_TOKEN) != "" || + os.Getenv(GH_TOKEN) != "" || + os.Getenv(GITHUB_TOKEN) != "" +} diff --git a/internal/config/from_env_test.go b/internal/config/from_env_test.go index baeb63194..c280b8a90 100644 --- a/internal/config/from_env_test.go +++ b/internal/config/from_env_test.go @@ -283,3 +283,60 @@ func TestInheritEnv(t *testing.T) { }) } } + +func TestAuthTokenProvidedFromEnv(t *testing.T) { + orig_GITHUB_TOKEN := os.Getenv("GITHUB_TOKEN") + orig_GITHUB_ENTERPRISE_TOKEN := os.Getenv("GITHUB_ENTERPRISE_TOKEN") + orig_GH_TOKEN := os.Getenv("GH_TOKEN") + orig_GH_ENTERPRISE_TOKEN := os.Getenv("GH_ENTERPRISE_TOKEN") + t.Cleanup(func() { + os.Setenv("GITHUB_TOKEN", orig_GITHUB_TOKEN) + os.Setenv("GITHUB_ENTERPRISE_TOKEN", orig_GITHUB_ENTERPRISE_TOKEN) + os.Setenv("GH_TOKEN", orig_GH_TOKEN) + os.Setenv("GH_ENTERPRISE_TOKEN", orig_GH_ENTERPRISE_TOKEN) + }) + + tests := []struct { + name string + GITHUB_TOKEN string + GITHUB_ENTERPRISE_TOKEN string + GH_TOKEN string + GH_ENTERPRISE_TOKEN string + provided bool + }{ + { + name: "no env tokens", + provided: false, + }, + { + name: "GH_TOKEN", + GH_TOKEN: "TOKEN", + provided: true, + }, + { + name: "GITHUB_TOKEN", + GITHUB_TOKEN: "TOKEN", + provided: true, + }, + { + name: "GH_ENTERPRISE_TOKEN", + GH_ENTERPRISE_TOKEN: "TOKEN", + provided: true, + }, + { + name: "GITHUB_ENTERPRISE_TOKEN", + GITHUB_ENTERPRISE_TOKEN: "TOKEN", + provided: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + os.Setenv("GITHUB_TOKEN", tt.GITHUB_TOKEN) + os.Setenv("GITHUB_ENTERPRISE_TOKEN", tt.GITHUB_ENTERPRISE_TOKEN) + os.Setenv("GH_TOKEN", tt.GH_TOKEN) + os.Setenv("GH_ENTERPRISE_TOKEN", tt.GH_ENTERPRISE_TOKEN) + assert.Equal(t, tt.provided, AuthTokenProvidedFromEnv()) + }) + } +} diff --git a/pkg/cmdutil/auth_check.go b/pkg/cmdutil/auth_check.go index 5d40d0143..10df9fade 100644 --- a/pkg/cmdutil/auth_check.go +++ b/pkg/cmdutil/auth_check.go @@ -17,6 +17,10 @@ func DisableAuthCheck(cmd *cobra.Command) { } func CheckAuth(cfg config.Config) bool { + if config.AuthTokenProvidedFromEnv() { + return true + } + hosts, err := cfg.Hosts() if err != nil { return false diff --git a/pkg/cmdutil/auth_check_test.go b/pkg/cmdutil/auth_check_test.go index 22b8ff5d4..2798750f0 100644 --- a/pkg/cmdutil/auth_check_test.go +++ b/pkg/cmdutil/auth_check_test.go @@ -1,6 +1,7 @@ package cmdutil import ( + "os" "testing" "github.com/cli/cli/internal/config" @@ -8,21 +9,34 @@ import ( ) func Test_CheckAuth(t *testing.T) { + orig_GITHUB_TOKEN := os.Getenv("GITHUB_TOKEN") + t.Cleanup(func() { + os.Setenv("GITHUB_TOKEN", orig_GITHUB_TOKEN) + }) + tests := []struct { name string cfg func(config.Config) + envToken bool expected bool }{ { name: "no hosts", cfg: func(c config.Config) {}, + envToken: false, expected: false, }, + {name: "no hosts, env auth token", + cfg: func(c config.Config) {}, + envToken: true, + expected: true, + }, { name: "host, no token", cfg: func(c config.Config) { _ = c.Set("github.com", "oauth_token", "") }, + envToken: false, expected: false, }, { @@ -30,12 +44,19 @@ func Test_CheckAuth(t *testing.T) { cfg: func(c config.Config) { _ = c.Set("github.com", "oauth_token", "a token") }, + envToken: false, expected: true, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + if tt.envToken { + os.Setenv("GITHUB_TOKEN", "TOKEN") + } else { + os.Setenv("GITHUB_TOKEN", "") + } + cfg := config.NewBlankConfig() tt.cfg(cfg) result := CheckAuth(cfg) From 6f689ff051d0be160fb3014d57f3162722ac658d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 1 Dec 2020 20:31:20 +0100 Subject: [PATCH 38/43] Document `make install` --- docs/source.md | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/docs/source.md b/docs/source.md index fc90ef94f..1c6a8470a 100644 --- a/docs/source.md +++ b/docs/source.md @@ -15,16 +15,18 @@ $ cd gh-cli ``` -2. Build the project - - ``` - $ make - ``` - -3. Move the resulting `bin/gh` executable to somewhere in your PATH +2. Build and install ```sh - $ sudo mv ./bin/gh /usr/local/bin/ + # installs to '/usr/local' by default; sudo may be required + $ make install ``` -4. Run `gh version` to check if it worked. + To install to a different location: + ```sh + $ make install prefix=/path/to/gh + ``` + + Make sure that the `${prefix}/bin` directory is in your PATH. + +3. Run `gh version` to check if it worked. From be759785f0b47bcc4288df03f69e186a127c5202 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 1 Dec 2020 21:23:39 +0100 Subject: [PATCH 39/43] Fix "Continue in browser" for `pr create` coming from forks Ensures that the `owner:` prefix is present when referencing the head branch --- pkg/cmd/pr/create/create.go | 2 +- pkg/cmd/pr/create/create_test.go | 18 +++++++++--------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/pkg/cmd/pr/create/create.go b/pkg/cmd/pr/create/create.go index 8748582eb..e58cd5b1d 100644 --- a/pkg/cmd/pr/create/create.go +++ b/pkg/cmd/pr/create/create.go @@ -684,7 +684,7 @@ func generateCompareURL(ctx CreateContext, state shared.IssueMetadataState) (str u := ghrepo.GenerateRepoURL( ctx.BaseRepo, "compare/%s...%s?expand=1", - url.QueryEscape(ctx.BaseBranch), url.QueryEscape(ctx.HeadBranch)) + url.QueryEscape(ctx.BaseBranch), url.QueryEscape(ctx.HeadBranchLabel)) url, err := shared.WithPrAndIssueQueryParams(u, state) if err != nil { return "", err diff --git a/pkg/cmd/pr/create/create_test.go b/pkg/cmd/pr/create/create_test.go index 7f703cef3..171207ea8 100644 --- a/pkg/cmd/pr/create/create_test.go +++ b/pkg/cmd/pr/create/create_test.go @@ -827,9 +827,9 @@ func Test_generateCompareURL(t *testing.T) { { name: "basic", ctx: CreateContext{ - BaseRepo: ghrepo.New("OWNER", "REPO"), - BaseBranch: "main", - HeadBranch: "feature", + BaseRepo: ghrepo.New("OWNER", "REPO"), + BaseBranch: "main", + HeadBranchLabel: "feature", }, want: "https://github.com/OWNER/REPO/compare/main...feature?expand=1", wantErr: false, @@ -837,9 +837,9 @@ func Test_generateCompareURL(t *testing.T) { { name: "with labels", ctx: CreateContext{ - BaseRepo: ghrepo.New("OWNER", "REPO"), - BaseBranch: "a", - HeadBranch: "b", + BaseRepo: ghrepo.New("OWNER", "REPO"), + BaseBranch: "a", + HeadBranchLabel: "b", }, state: prShared.IssueMetadataState{ Labels: []string{"one", "two three"}, @@ -850,9 +850,9 @@ func Test_generateCompareURL(t *testing.T) { { name: "complex branch names", ctx: CreateContext{ - BaseRepo: ghrepo.New("OWNER", "REPO"), - BaseBranch: "main/trunk", - HeadBranch: "owner:feature", + BaseRepo: ghrepo.New("OWNER", "REPO"), + BaseBranch: "main/trunk", + HeadBranchLabel: "owner:feature", }, want: "https://github.com/OWNER/REPO/compare/main%2Ftrunk...owner%3Afeature?expand=1", wantErr: false, From d6add864b8b08497c5cb127e8d97d3e56348618b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Wed, 2 Dec 2020 17:20:07 +0100 Subject: [PATCH 40/43] Ensure efficient resolving of `issue/pr create` metadata to GraphQL IDs For metadata types chosen in interactive flow, we fetch all records from the API in order to be able to display a multi-select interface. For metadata defined via command-line flags, we resolve records that can be looked up directly, avoiding fetching the entirety of expensive datasets (e.g. all members of an organization) if we can. The new approach ensures efficient fetching when interactive flow is combined with values from flags. --- api/queries_repo.go | 22 ++++++++++++++ pkg/cmd/pr/shared/params.go | 56 ++++++++++++++++++++++++++--------- pkg/cmd/pr/shared/survey.go | 59 ++++++++++++++++++------------------- 3 files changed, 93 insertions(+), 44 deletions(-) diff --git a/api/queries_repo.go b/api/queries_repo.go index 54f88a7e1..b60dfaa92 100644 --- a/api/queries_repo.go +++ b/api/queries_repo.go @@ -464,6 +464,28 @@ func (m *RepoMetadataResult) MilestoneToID(title string) (string, error) { return "", errors.New("not found") } +func (m *RepoMetadataResult) Merge(m2 *RepoMetadataResult) { + if len(m2.AssignableUsers) > 0 || len(m.AssignableUsers) == 0 { + m.AssignableUsers = m2.AssignableUsers + } + + if len(m2.Teams) > 0 || len(m.Teams) == 0 { + m.Teams = m2.Teams + } + + if len(m2.Labels) > 0 || len(m.Labels) == 0 { + m.Labels = m2.Labels + } + + if len(m2.Projects) > 0 || len(m.Projects) == 0 { + m.Projects = m2.Projects + } + + if len(m2.Milestones) > 0 || len(m.Milestones) == 0 { + m.Milestones = m2.Milestones + } +} + type RepoMetadataInput struct { Assignees bool Reviewers bool diff --git a/pkg/cmd/pr/shared/params.go b/pkg/cmd/pr/shared/params.go index 9edb9e9e7..6efdf9494 100644 --- a/pkg/cmd/pr/shared/params.go +++ b/pkg/cmd/pr/shared/params.go @@ -37,25 +37,53 @@ func WithPrAndIssueQueryParams(baseURL string, state IssueMetadataState) (string return u.String(), nil } +// Ensure that tb.MetadataResult object exists and contains enough pre-fetched API data to be able +// to resolve all object listed in tb to GraphQL IDs. +func fillMetadata(client *api.Client, baseRepo ghrepo.Interface, tb *IssueMetadataState) error { + resolveInput := api.RepoResolveInput{} + + if len(tb.Assignees) > 0 && (tb.MetadataResult == nil || len(tb.MetadataResult.AssignableUsers) == 0) { + resolveInput.Assignees = tb.Assignees + } + + if len(tb.Reviewers) > 0 && (tb.MetadataResult == nil || len(tb.MetadataResult.AssignableUsers) == 0) { + resolveInput.Reviewers = tb.Reviewers + } + + if len(tb.Labels) > 0 && (tb.MetadataResult == nil || len(tb.MetadataResult.Labels) == 0) { + resolveInput.Labels = tb.Labels + } + + if len(tb.Projects) > 0 && (tb.MetadataResult == nil || len(tb.MetadataResult.Projects) == 0) { + resolveInput.Projects = tb.Projects + } + + if len(tb.Milestones) > 0 && (tb.MetadataResult == nil || len(tb.MetadataResult.Milestones) == 0) { + resolveInput.Milestones = tb.Milestones + } + + metadataResult, err := api.RepoResolveMetadataIDs(client, baseRepo, resolveInput) + if err != nil { + return err + } + + if tb.MetadataResult == nil { + tb.MetadataResult = metadataResult + } else { + tb.MetadataResult.Merge(metadataResult) + } + + return nil +} + func AddMetadataToIssueParams(client *api.Client, baseRepo ghrepo.Interface, params map[string]interface{}, tb *IssueMetadataState) error { if !tb.HasMetadata() { return nil } - if tb.MetadataResult == nil { - resolveInput := api.RepoResolveInput{ - Reviewers: tb.Reviewers, - Assignees: tb.Assignees, - Labels: tb.Labels, - Projects: tb.Projects, - Milestones: tb.Milestones, - } - - var err error - tb.MetadataResult, err = api.RepoResolveMetadataIDs(client, baseRepo, resolveInput) - if err != nil { - return err - } + err := fillMetadata(client, baseRepo, tb) + if err != nil { + return err } assigneeIDs, err := tb.MetadataResult.MembersToIDs(tb.Assignees) diff --git a/pkg/cmd/pr/shared/survey.go b/pkg/cmd/pr/shared/survey.go index d3838925e..161d662b4 100644 --- a/pkg/cmd/pr/shared/survey.go +++ b/pkg/cmd/pr/shared/survey.go @@ -263,13 +263,6 @@ func MetadataSurvey(io *iostreams.IOStreams, client *api.Client, baseRepo ghrepo milestones = append(milestones, m.Title) } - type metadataValues struct { - Reviewers []string - Assignees []string - Labels []string - Projects []string - Milestone string - } var mqs []*survey.Question if isChosen("Reviewers") { if len(users) > 0 || len(teams) > 0 { @@ -346,32 +339,38 @@ func MetadataSurvey(io *iostreams.IOStreams, client *api.Client, baseRepo ghrepo } } - if len(mqs) > 0 { - values := metadataValues{ - Reviewers: state.Reviewers, - Assignees: state.Assignees, - Labels: state.Labels, - Projects: state.Projects, - } - if len(state.Milestones) > 0 { - values.Milestone = state.Milestones[0] - } + values := struct { + Reviewers []string + Assignees []string + Labels []string + Projects []string + Milestone string + }{} - err = prompt.SurveyAsk(mqs, &values, survey.WithKeepFilter(true)) - if err != nil { - return fmt.Errorf("could not prompt: %w", err) - } - - state.Reviewers = values.Reviewers - state.Assignees = values.Assignees - state.Labels = values.Labels - state.Projects = values.Projects - if values.Milestone != "" && values.Milestone != noMilestone { - state.Milestones = []string{values.Milestone} - } + err = prompt.SurveyAsk(mqs, &values, survey.WithKeepFilter(true)) + if err != nil { + return fmt.Errorf("could not prompt: %w", err) } - state.MetadataResult = nil + if isChosen("Reviewers") { + state.Reviewers = values.Reviewers + } + if isChosen("Assignees") { + state.Assignees = values.Assignees + } + if isChosen("Labels") { + state.Labels = values.Labels + } + if isChosen("Projects") { + state.Projects = values.Projects + } + if isChosen("Milestone") { + if values.Milestone != "" && values.Milestone != noMilestone { + state.Milestones = []string{values.Milestone} + } else { + state.Milestones = []string{} + } + } return nil } From be39f4363bfc14078800ebda3d8ddae7e0424ad1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 3 Dec 2020 17:47:40 +0100 Subject: [PATCH 41/43] Make MetadataSurvey testable by accepting an interface --- pkg/cmd/issue/create/create.go | 8 +- pkg/cmd/pr/create/create.go | 8 +- pkg/cmd/pr/shared/survey.go | 37 +++++--- pkg/cmd/pr/shared/survey_test.go | 144 +++++++++++++++++++++++++++++++ 4 files changed, 184 insertions(+), 13 deletions(-) create mode 100644 pkg/cmd/pr/shared/survey_test.go diff --git a/pkg/cmd/issue/create/create.go b/pkg/cmd/issue/create/create.go index 44cac6ae1..a098b31fa 100644 --- a/pkg/cmd/issue/create/create.go +++ b/pkg/cmd/issue/create/create.go @@ -207,7 +207,13 @@ func createRun(opts *CreateOptions) (err error) { } if action == prShared.MetadataAction { - err = prShared.MetadataSurvey(opts.IO, apiClient, baseRepo, &tb) + fetcher := &prShared.MetadataFetcher{ + IO: opts.IO, + APIClient: apiClient, + Repo: baseRepo, + State: &tb, + } + err = prShared.MetadataSurvey(opts.IO, baseRepo, fetcher, &tb) if err != nil { return } diff --git a/pkg/cmd/pr/create/create.go b/pkg/cmd/pr/create/create.go index 8748582eb..faea5f13d 100644 --- a/pkg/cmd/pr/create/create.go +++ b/pkg/cmd/pr/create/create.go @@ -259,7 +259,13 @@ func createRun(opts *CreateOptions) (err error) { } if action == shared.MetadataAction { - err = shared.MetadataSurvey(opts.IO, client, ctx.BaseRepo, state) + fetcher := &shared.MetadataFetcher{ + IO: opts.IO, + APIClient: client, + Repo: ctx.BaseRepo, + State: state, + } + err = shared.MetadataSurvey(opts.IO, ctx.BaseRepo, fetcher, state) if err != nil { return } diff --git a/pkg/cmd/pr/shared/survey.go b/pkg/cmd/pr/shared/survey.go index 161d662b4..8ef40d047 100644 --- a/pkg/cmd/pr/shared/survey.go +++ b/pkg/cmd/pr/shared/survey.go @@ -12,7 +12,6 @@ import ( "github.com/cli/cli/pkg/iostreams" "github.com/cli/cli/pkg/prompt" "github.com/cli/cli/pkg/surveyext" - "github.com/cli/cli/utils" ) type Action int @@ -196,7 +195,26 @@ func TitleSurvey(state *IssueMetadataState) error { return nil } -func MetadataSurvey(io *iostreams.IOStreams, client *api.Client, baseRepo ghrepo.Interface, state *IssueMetadataState) error { +type MetadataFetcher struct { + IO *iostreams.IOStreams + APIClient *api.Client + Repo ghrepo.Interface + State *IssueMetadataState +} + +func (mf *MetadataFetcher) RepoMetadataFetch(input api.RepoMetadataInput) (*api.RepoMetadataResult, error) { + mf.IO.StartProgressIndicator() + metadataResult, err := api.RepoMetadata(mf.APIClient, mf.Repo, input) + mf.IO.StopProgressIndicator() + mf.State.MetadataResult = metadataResult + return metadataResult, err +} + +type RepoMetadataFetcher interface { + RepoMetadataFetch(api.RepoMetadataInput) (*api.RepoMetadataResult, error) +} + +func MetadataSurvey(io *iostreams.IOStreams, baseRepo ghrepo.Interface, fetcher RepoMetadataFetcher, state *IssueMetadataState) error { isChosen := func(m string) bool { for _, c := range state.Metadata { if m == c { @@ -234,32 +252,29 @@ func MetadataSurvey(io *iostreams.IOStreams, client *api.Client, baseRepo ghrepo Projects: isChosen("Projects"), Milestones: isChosen("Milestone"), } - s := utils.Spinner(io.ErrOut) - utils.StartSpinner(s) - state.MetadataResult, err = api.RepoMetadata(client, baseRepo, metadataInput) - utils.StopSpinner(s) + metadataResult, err := fetcher.RepoMetadataFetch(metadataInput) if err != nil { return fmt.Errorf("error fetching metadata options: %w", err) } var users []string - for _, u := range state.MetadataResult.AssignableUsers { + for _, u := range metadataResult.AssignableUsers { users = append(users, u.Login) } var teams []string - for _, t := range state.MetadataResult.Teams { + for _, t := range metadataResult.Teams { teams = append(teams, fmt.Sprintf("%s/%s", baseRepo.RepoOwner(), t.Slug)) } var labels []string - for _, l := range state.MetadataResult.Labels { + for _, l := range metadataResult.Labels { labels = append(labels, l.Name) } var projects []string - for _, l := range state.MetadataResult.Projects { + for _, l := range metadataResult.Projects { projects = append(projects, l.Name) } milestones := []string{noMilestone} - for _, m := range state.MetadataResult.Milestones { + for _, m := range metadataResult.Milestones { milestones = append(milestones, m.Title) } diff --git a/pkg/cmd/pr/shared/survey_test.go b/pkg/cmd/pr/shared/survey_test.go new file mode 100644 index 000000000..a500040d3 --- /dev/null +++ b/pkg/cmd/pr/shared/survey_test.go @@ -0,0 +1,144 @@ +package shared + +import ( + "testing" + + "github.com/cli/cli/api" + "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/pkg/prompt" + "github.com/stretchr/testify/assert" +) + +type metadataFetcher struct { + metadataResult *api.RepoMetadataResult +} + +func (mf *metadataFetcher) RepoMetadataFetch(input api.RepoMetadataInput) (*api.RepoMetadataResult, error) { + return mf.metadataResult, nil +} + +func TestMetadataSurvey_selectAll(t *testing.T) { + io, _, stdout, stderr := iostreams.Test() + + repo := ghrepo.New("OWNER", "REPO") + + fetcher := &metadataFetcher{ + metadataResult: &api.RepoMetadataResult{ + AssignableUsers: []api.RepoAssignee{ + {Login: "hubot"}, + {Login: "monalisa"}, + }, + Labels: []api.RepoLabel{ + {Name: "help wanted"}, + {Name: "good first issue"}, + }, + Projects: []api.RepoProject{ + {Name: "Huge Refactoring"}, + {Name: "The road to 1.0"}, + }, + Milestones: []api.RepoMilestone{ + {Title: "1.2 patch release"}, + }, + }, + } + + as, restoreAsk := prompt.InitAskStubber() + defer restoreAsk() + + as.Stub([]*prompt.QuestionStub{ + { + Name: "metadata", + Value: []string{"Labels", "Projects", "Assignees", "Reviewers", "Milestone"}, + }, + }) + as.Stub([]*prompt.QuestionStub{ + { + Name: "reviewers", + Value: []string{"monalisa"}, + }, + { + Name: "assignees", + Value: []string{"hubot"}, + }, + { + Name: "labels", + Value: []string{"good first issue"}, + }, + { + Name: "projects", + Value: []string{"The road to 1.0"}, + }, + { + Name: "milestone", + Value: []string{"(none)"}, + }, + }) + + state := &IssueMetadataState{ + Assignees: []string{"hubot"}, + } + err := MetadataSurvey(io, repo, fetcher, state) + assert.NoError(t, err) + + assert.Equal(t, "", stdout.String()) + assert.Equal(t, "", stderr.String()) + + assert.Equal(t, []string{"hubot"}, state.Assignees) + assert.Equal(t, []string{"monalisa"}, state.Reviewers) + assert.Equal(t, []string{"good first issue"}, state.Labels) + assert.Equal(t, []string{"The road to 1.0"}, state.Projects) + assert.Equal(t, []string{}, state.Milestones) +} + +func TestMetadataSurvey_keepExisting(t *testing.T) { + io, _, stdout, stderr := iostreams.Test() + + repo := ghrepo.New("OWNER", "REPO") + + fetcher := &metadataFetcher{ + metadataResult: &api.RepoMetadataResult{ + Labels: []api.RepoLabel{ + {Name: "help wanted"}, + {Name: "good first issue"}, + }, + Projects: []api.RepoProject{ + {Name: "Huge Refactoring"}, + {Name: "The road to 1.0"}, + }, + }, + } + + as, restoreAsk := prompt.InitAskStubber() + defer restoreAsk() + + as.Stub([]*prompt.QuestionStub{ + { + Name: "metadata", + Value: []string{"Labels", "Projects"}, + }, + }) + as.Stub([]*prompt.QuestionStub{ + { + Name: "labels", + Value: []string{"good first issue"}, + }, + { + Name: "projects", + Value: []string{"The road to 1.0"}, + }, + }) + + state := &IssueMetadataState{ + Assignees: []string{"hubot"}, + } + err := MetadataSurvey(io, repo, fetcher, state) + assert.NoError(t, err) + + assert.Equal(t, "", stdout.String()) + assert.Equal(t, "", stderr.String()) + + assert.Equal(t, []string{"hubot"}, state.Assignees) + assert.Equal(t, []string{"good first issue"}, state.Labels) + assert.Equal(t, []string{"The road to 1.0"}, state.Projects) +} From 2b4372bc3a6f4dd6d69293369d54d1c0c0294822 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 3 Dec 2020 17:51:58 +0100 Subject: [PATCH 42/43] AskStubber now throws a more descriptive error when stubs do not match --- pkg/prompt/stubber.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/prompt/stubber.go b/pkg/prompt/stubber.go index a3302c3f9..be920cd25 100644 --- a/pkg/prompt/stubber.go +++ b/pkg/prompt/stubber.go @@ -51,6 +51,9 @@ func InitAskStubber() (*AskStubber, func()) { // actually set response stubbedQuestions := as.Stubs[count] + if len(stubbedQuestions) != len(qs) { + panic(fmt.Sprintf("asked questions: %d; stubbed questions: %d", len(qs), len(stubbedQuestions))) + } for i, sq := range stubbedQuestions { q := qs[i] if q.Name != sq.Name { From 8db2027c99c669e9ed5980e6bf33700815240286 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Thu, 3 Dec 2020 18:02:24 +0100 Subject: [PATCH 43/43] Allow interactive `pr create` even if we failed to look up commits --- pkg/cmd/pr/create/create.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/pr/create/create.go b/pkg/cmd/pr/create/create.go index faea5f13d..68ab074c6 100644 --- a/pkg/cmd/pr/create/create.go +++ b/pkg/cmd/pr/create/create.go @@ -386,7 +386,7 @@ func NewIssueState(ctx CreateContext, opts CreateOptions) (*shared.IssueMetadata if opts.Autofill || !opts.TitleProvided || !opts.BodyProvided { err := initDefaultTitleBody(ctx, state) - if err != nil { + if err != nil && opts.Autofill { return nil, fmt.Errorf("could not compute title or body defaults: %w", err) } }