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 diff --git a/Makefile b/Makefile index 859461984..22f3672b7 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) @@ -27,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 @@ -58,7 +55,22 @@ endif git -C site commit -m '$(GITHUB_REF:refs/tags/v%=%)' index.html .PHONY: site-bump - .PHONY: manpages manpages: go run ./cmd/gen-docs --man-page --doc-path ./share/man/man1/ + +DESTDIR := +prefix := /usr/local +bindir := ${prefix}/bin +mandir := ${prefix}/share/man + +.PHONY: install +install: bin/gh manpages + install -d ${DESTDIR}${bindir} + install -m755 bin/gh ${DESTDIR}${bindir}/ + install -d ${DESTDIR}${mandir}/man1 + install -m644 ./share/man/man1/* ${DESTDIR}${mandir}/man1/ + +.PHONY: uninstall +uninstall: + rm -f ${DESTDIR}${bindir}/gh ${DESTDIR}${mandir}/man1/gh.1 ${DESTDIR}${mandir}/man1/gh-*.1 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 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) 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 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/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) } 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/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 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. 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/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= 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") } 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/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") 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, ", ")) } } 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/cmd/issue/create/create.go b/pkg/cmd/issue/create/create.go index 28c8f0148..f8ee73abb 100644 --- a/pkg/cmd/issue/create/create.go +++ b/pkg/cmd/issue/create/create.go @@ -26,6 +26,7 @@ type CreateOptions struct { RepoOverride string WebMode bool + RecoverFile string Title string Body string @@ -63,6 +64,10 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co bodyProvided := cmd.Flags().Changed("body") opts.RepoOverride, _ = cmd.Flags().GetString("repo") + if !opts.IO.CanPrompt() && opts.RecoverFile != "" { + return &cmdutil.FlagError{Err: errors.New("--recover only supported when running interactively")} + } + opts.Interactive = !(titleProvided && bodyProvided) if opts.Interactive && !opts.IO.CanPrompt() { @@ -83,20 +88,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().StringVar(&opts.RecoverFile, "recover", "", "Recover input from a failed run of create") 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") @@ -118,12 +124,20 @@ func createRun(opts *CreateOptions) 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 != "" { openURL, err = prShared.WithPrAndIssueQueryParams(openURL, tb) if err != nil { - return err + return } } else if len(templateFiles) > 1 { openURL += "/choose" @@ -140,38 +154,44 @@ 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 } - if tb.Title == "" { + defer prShared.PreserveInput(opts.IO, &tb, &err)() + + if opts.Title == "" { err = prShared.TitleSurvey(&tb) if err != nil { - return err + return } } - if tb.Body == "" { + if opts.Body == "" { templateContent := "" - templateContent, err = prShared.TemplateSurvey(templateFiles, legacyTemplate, tb) - if err != nil { - return err + if opts.RecoverFile == "" { + templateContent, err = prShared.TemplateSurvey(templateFiles, legacyTemplate, tb) + if err != nil { + return + } } err = prShared.BodySurvey(&tb, templateContent, editorCommand) if err != nil { - return err + return } if tb.Body == "" { @@ -179,31 +199,38 @@ func createRun(opts *CreateOptions) error { } } - action, err := prShared.ConfirmSubmission(!tb.HasMetadata(), repo.ViewerCanTriage()) + 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) + fetcher := &prShared.MetadataFetcher{ + IO: opts.IO, + APIClient: apiClient, + Repo: baseRepo, + State: &tb, + } + err = prShared.MetadataSurvey(opts.IO, baseRepo, fetcher, &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 tb.Title == "" { - return fmt.Errorf("title can't be blank") + err = fmt.Errorf("title can't be blank") + return } } @@ -211,7 +238,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 +252,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 +266,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 c89a349a0..d7e6b4ec0 100644 --- a/pkg/cmd/issue/create/create_test.go +++ b/pkg/cmd/issue/create/create_test.go @@ -3,16 +3,20 @@ package create import ( "bytes" "encoding/json" + "fmt" "io/ioutil" "net/http" + "os" "os/exec" "reflect" "strings" "testing" + "github.com/MakeNowJust/heredoc" "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,6 +130,86 @@ func TestIssueCreate(t *testing.T) { eq(t, output.String(), "https://github.com/OWNER/REPO/issues/12\n") } +func TestIssueCreate_recover(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { + "id": "REPOID", + "hasIssuesEnabled": true + } } } + `)) + 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"}) + })) + + 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("--recover '%s'", tmpfile.Name()) + + output, err := runCommandWithRootDirOverridden(http, true, args, "") + if err != nil { + t.Errorf("error running command `issue create`: %v", err) + } + + 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) @@ -145,7 +229,7 @@ func TestIssueCreate_nonLegacyTemplate(t *testing.T) { as, teardown := prompt.InitAskStubber() defer teardown() - // tmeplate + // template as.Stub([]*prompt.QuestionStub{ { Name: "index", @@ -191,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 := strings.ReplaceAll(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) diff --git a/pkg/cmd/pr/create/create.go b/pkg/cmd/pr/create/create.go index 9e0180965..db623acff 100644 --- a/pkg/cmd/pr/create/create.go +++ b/pkg/cmd/pr/create/create.go @@ -38,8 +38,9 @@ type CreateOptions struct { RootDirOverride string RepoOverride string - Autofill bool - WebMode bool + Autofill bool + WebMode bool + RecoverFile string IsDraft bool Title string @@ -93,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 `), @@ -103,6 +104,10 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co opts.BodyProvided = cmd.Flags().Changed("body") opts.RepoOverride, _ = cmd.Flags().GetString("repo") + 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")} } @@ -129,11 +134,12 @@ 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`") fl.StringVarP(&opts.Milestone, "milestone", "m", "", "Add the pull request to a milestone by `name`") + fl.StringVar(&opts.RecoverFile, "recover", "", "Recover input from a failed run of create") return cmd } @@ -141,14 +147,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 +162,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 +205,46 @@ 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.RecoverFile != "" { + err = shared.FillFromJSON(opts.IO, opts.RecoverFile, state) + if err != nil { + return fmt.Errorf("failed to recover input: %w", err) + } + } + 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") + if opts.RecoverFile == "" { + templateFiles, legacyTemplate := shared.FindTemplates(opts.RootDirOverride, "PULL_REQUEST_TEMPLATE") - templateContent, err = shared.TemplateSurvey(templateFiles, legacyTemplate, *state) - if err != nil { - return err + templateContent, err = shared.TemplateSurvey(templateFiles, legacyTemplate, *state) + if err != nil { + return + } } err = shared.BodySurvey(state, templateContent, editorCommand) if err != nil { - return err + return } if state.Body == "" { @@ -242,14 +259,20 @@ 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 err + return } action, err = shared.ConfirmSubmission(!state.HasMetadata(), false) if err != nil { - return err + return } } @@ -260,7 +283,7 @@ func createRun(opts *CreateOptions) (err error) { err = handlePush(*opts, *ctx) if err != nil { - return err + return } if action == shared.PreviewAction { @@ -271,7 +294,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 { @@ -362,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) } } @@ -666,7 +690,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 0c9a91c1a..171207ea8 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,6 +138,96 @@ func TestPRCreate_nontty_insufficient_flags(t *testing.T) { assert.Equal(t, "", output.String()) } +func TestPRCreate_recover(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + + http.StubRepoInfoResponse("OWNER", "REPO", "master") + http.Register( + httpmock.GraphQL(`query PullRequestForBranch\b`), + httpmock.StringResponse(` + { "data": { "repository": { "pullRequests": { "nodes" : [ + ] } } } } + `)) + http.Register( + httpmock.GraphQL(`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() + + cs.Stub("") // git status + cs.Stub("1234567890,commit 0\n2345678901,commit 1") // git log + + 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", + Reviewers: []string{"jillValentine"}, + } + + data, err := json.Marshal(state) + assert.NoError(t, err) + + _, err = tmpfile.Write(data) + assert.NoError(t, err) + + args := fmt.Sprintf("--recover '%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) { http := initFakeHTTP() defer http.Verify(t) @@ -735,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, @@ -745,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"}, @@ -758,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, 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, }, }, { 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) { 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 { 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/preserve.go b/pkg/cmd/pr/shared/preserve.go new file mode 100644 index 000000000..4105823cd --- /dev/null +++ b/pkg/cmd/pr/shared/preserve.go @@ -0,0 +1,62 @@ +package shared + +import ( + "encoding/json" + "fmt" + "os" + + "github.com/cli/cli/pkg/iostreams" +) + +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 + } + + 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 = tmpfile.Write(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. 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 new file mode 100644 index 000000000..28949d957 --- /dev/null +++ b/pkg/cmd/pr/shared/preserve_test.go @@ -0,0 +1,122 @@ +package shared + +import ( + "encoding/json" + "errors" + "io/ioutil" + "os" + "testing" + + "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/test" + "github.com/stretchr/testify/assert" +) + +func Test_PreserveInput(t *testing.T) { + tests := []struct { + name string + state *IssueMetadataState + err bool + wantErrLine 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"}, + }, + wantErrLine: `X operation failed. To restore: gh issue create --recover .*testfile.*`, + err: true, + wantPreservation: true, + }, + { + name: "err, metadata received", + state: &IssueMetadataState{ + Reviewers: []string{"barry", "chris"}, + Labels: []string{"sandwich"}, + }, + wantErrLine: `X operation failed. To restore: gh issue create --recover .*testfile.*`, + err: true, + wantPreservation: true, + }, + { + name: "err, dirty, pull request", + state: &IssueMetadataState{ + dirty: true, + Title: "a pull request", + Type: PRMetadata, + }, + wantErrLine: `X operation failed. To restore: gh pr create --recover .*testfile.*`, + 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() + + tf, tferr := tmpfile() + assert.NoError(t, tferr) + defer os.Remove(tf.Name()) + + io.TempFileOverride = tf + + var err error + if tt.err { + err = errors.New("error during creation") + } + + PreserveInput(io, tt.state, &err)() + + _, err = tf.Seek(0, 0) + assert.NoError(t, err) + + data, err := ioutil.ReadAll(tf) + assert.NoError(t, err) + + if tt.wantPreservation { + test.ExpectLines(t, errOut.String(), tt.wantErrLine) + preserved := &IssueMetadataState{} + 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(data), "") + } + }) + } +} + +func tmpfile() (*os.File, error) { + dir := os.TempDir() + tmpfile, err := ioutil.TempFile(dir, "testfile*") + if err != nil { + return nil, err + } + + return tmpfile, nil +} diff --git a/pkg/cmd/pr/shared/state.go b/pkg/cmd/pr/shared/state.go new file mode 100644 index 000000000..930a69fcc --- /dev/null +++ b/pkg/cmd/pr/shared/state.go @@ -0,0 +1,68 @@ +package shared + +import ( + "encoding/json" + "fmt" + + "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, recoverFile string, state *IssueMetadataState) error { + var data []byte + var err error + data, err = io.ReadUserFile(recoverFile) + if err != nil { + return fmt.Errorf("failed to read file %s: %w", recoverFile, err) + } + + 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..8ef40d047 100644 --- a/pkg/cmd/pr/shared/survey.go +++ b/pkg/cmd/pr/shared/survey.go @@ -12,42 +12,9 @@ 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 -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 +137,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 +162,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,10 +188,33 @@ func TitleSurvey(state *IssueMetadataState) error { return err } + if preTitle != state.Title { + state.MarkDirty() + } + 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 { @@ -254,42 +252,32 @@ 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) } - 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 { @@ -365,17 +353,38 @@ func MetadataSurvey(io *iostreams.IOStreams, client *api.Client, baseRepo ghrepo fmt.Fprintln(io.ErrOut, "warning: no milestones in the repository") } } - values := metadataValues{} + + 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} + + 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 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) +} 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 } 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) 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) { 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 } 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) 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) 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 99ae0cfdc..5df44098d 100644 --- a/pkg/iostreams/iostreams.go +++ b/pkg/iostreams/iostreams.go @@ -45,6 +45,8 @@ type IOStreams struct { pagerProcess *os.Process neverPrompt bool + + TempFileOverride *os.File } func (s *IOStreams) ColorEnabled() bool { @@ -253,6 +255,28 @@ 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 (s *IOStreams) TempFile(dir, pattern string) (*os.File, error) { + if s.TempFileOverride != nil { + return s.TempFileOverride, nil + } + return ioutil.TempFile(dir, pattern) +} + func System() *IOStreams { stdoutIsTTY := isTerminal(os.Stdout) stderrIsTTY := isTerminal(os.Stderr) 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 {