From c00fe73d5a2838aed417eb107a32ebbb4c795104 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 7 Aug 2020 00:27:55 +0200 Subject: [PATCH 01/11] Isolate `issue` commands --- command/issue.go | 694 ------------- command/issue_test.go | 962 ------------------ command/root.go | 2 + pkg/cmd/issue/close/close.go | 80 ++ pkg/cmd/issue/close/close_test.go | 119 +++ pkg/cmd/issue/create/create.go | 228 +++++ pkg/cmd/issue/create/create_test.go | 280 +++++ pkg/cmd/issue/issue.go | 54 + .../cmd/issue/list}/fixtures/issueList.json | 0 pkg/cmd/issue/list/list.go | 127 +++ pkg/cmd/issue/list/list_test.go | 223 ++++ pkg/cmd/issue/reopen/reopen.go | 80 ++ pkg/cmd/issue/reopen/reopen_test.go | 119 +++ pkg/cmd/issue/shared/display.go | 65 ++ .../cmd/issue/shared/lookup.go | 8 +- .../issue/status}/fixtures/issueStatus.json | 0 pkg/cmd/issue/status/status.go | 103 ++ pkg/cmd/issue/status/status_test.go | 146 +++ .../view}/fixtures/issueView_preview.json | 0 .../issueView_previewClosedState.json | 0 .../issueView_previewWithEmptyBody.json | 0 .../issueView_previewWithMetadata.json | 0 pkg/cmd/issue/view/view.go | 207 ++++ pkg/cmd/issue/view/view_test.go | 340 +++++++ pkg/cmd/pr/close/close.go | 2 +- pkg/cmd/pr/pr.go | 3 +- 26 files changed, 2178 insertions(+), 1664 deletions(-) delete mode 100644 command/issue.go delete mode 100644 command/issue_test.go create mode 100644 pkg/cmd/issue/close/close.go create mode 100644 pkg/cmd/issue/close/close_test.go create mode 100644 pkg/cmd/issue/create/create.go create mode 100644 pkg/cmd/issue/create/create_test.go create mode 100644 pkg/cmd/issue/issue.go rename {test => pkg/cmd/issue/list}/fixtures/issueList.json (100%) create mode 100644 pkg/cmd/issue/list/list.go create mode 100644 pkg/cmd/issue/list/list_test.go create mode 100644 pkg/cmd/issue/reopen/reopen.go create mode 100644 pkg/cmd/issue/reopen/reopen_test.go create mode 100644 pkg/cmd/issue/shared/display.go rename command/issue_lookup.go => pkg/cmd/issue/shared/lookup.go (83%) rename {test => pkg/cmd/issue/status}/fixtures/issueStatus.json (100%) create mode 100644 pkg/cmd/issue/status/status.go create mode 100644 pkg/cmd/issue/status/status_test.go rename {test => pkg/cmd/issue/view}/fixtures/issueView_preview.json (100%) rename {test => pkg/cmd/issue/view}/fixtures/issueView_previewClosedState.json (100%) rename {test => pkg/cmd/issue/view}/fixtures/issueView_previewWithEmptyBody.json (100%) rename {test => pkg/cmd/issue/view}/fixtures/issueView_previewWithMetadata.json (100%) create mode 100644 pkg/cmd/issue/view/view.go create mode 100644 pkg/cmd/issue/view/view_test.go diff --git a/command/issue.go b/command/issue.go deleted file mode 100644 index 41dd30b7f..000000000 --- a/command/issue.go +++ /dev/null @@ -1,694 +0,0 @@ -package command - -import ( - "fmt" - "io" - "strconv" - "strings" - "time" - - "github.com/MakeNowJust/heredoc" - "github.com/cli/cli/api" - "github.com/cli/cli/git" - "github.com/cli/cli/internal/ghrepo" - "github.com/cli/cli/pkg/cmd/pr/shared" - "github.com/cli/cli/pkg/cmdutil" - "github.com/cli/cli/pkg/githubtemplate" - "github.com/cli/cli/pkg/iostreams" - "github.com/cli/cli/pkg/text" - "github.com/cli/cli/utils" - "github.com/spf13/cobra" - "github.com/spf13/pflag" -) - -func init() { - issueCmd.PersistentFlags().StringP("repo", "R", "", "Select another repository using the `OWNER/REPO` format") - - RootCmd.AddCommand(issueCmd) - issueCmd.AddCommand(issueStatusCmd) - - issueCmd.AddCommand(issueCreateCmd) - issueCreateCmd.Flags().StringP("title", "t", "", - "Supply a title. Will prompt for one otherwise.") - issueCreateCmd.Flags().StringP("body", "b", "", - "Supply a body. Will prompt for one otherwise.") - issueCreateCmd.Flags().BoolP("web", "w", false, "Open the browser to create an issue") - issueCreateCmd.Flags().StringSliceP("assignee", "a", nil, "Assign people by their `login`") - issueCreateCmd.Flags().StringSliceP("label", "l", nil, "Add labels by `name`") - issueCreateCmd.Flags().StringSliceP("project", "p", nil, "Add the issue to projects by `name`") - issueCreateCmd.Flags().StringP("milestone", "m", "", "Add the issue to a milestone by `name`") - - issueCmd.AddCommand(issueListCmd) - issueListCmd.Flags().BoolP("web", "w", false, "Open the browser to list the issue(s)") - issueListCmd.Flags().StringP("assignee", "a", "", "Filter by assignee") - issueListCmd.Flags().StringSliceP("label", "l", nil, "Filter by labels") - issueListCmd.Flags().StringP("state", "s", "open", "Filter by state: {open|closed|all}") - issueListCmd.Flags().IntP("limit", "L", 30, "Maximum number of issues to fetch") - issueListCmd.Flags().StringP("author", "A", "", "Filter by author") - issueListCmd.Flags().String("mention", "", "Filter by mention") - issueListCmd.Flags().StringP("milestone", "m", "", "Filter by milestone `name`") - - issueCmd.AddCommand(issueViewCmd) - issueViewCmd.Flags().BoolP("web", "w", false, "Open an issue in the browser") - - issueCmd.AddCommand(issueCloseCmd) - issueCmd.AddCommand(issueReopenCmd) -} - -var issueCmd = &cobra.Command{ - Use: "issue ", - Short: "Create and view issues", - Long: `Work with GitHub issues`, - Example: heredoc.Doc(` - $ gh issue list - $ gh issue create --label bug - $ gh issue view --web - `), - Annotations: map[string]string{ - "IsCore": "true", - "help:arguments": `An issue can be supplied as argument in any of the following formats: -- by number, e.g. "123"; or -- by URL, e.g. "https://github.com/OWNER/REPO/issues/123".`}, -} -var issueCreateCmd = &cobra.Command{ - Use: "create", - Short: "Create a new issue", - Args: cmdutil.NoArgsQuoteReminder, - RunE: issueCreate, - Example: heredoc.Doc(` - $ gh issue create --title "I found a bug" --body "Nothing works" - $ gh issue create --label "bug,help wanted" - $ gh issue create --label bug --label "help wanted" - $ gh issue create --assignee monalisa,hubot - $ gh issue create --project "Roadmap" - `), -} -var issueListCmd = &cobra.Command{ - Use: "list", - Short: "List and filter issues in this repository", - Example: heredoc.Doc(` - $ gh issue list -l "help wanted" - $ gh issue list -A monalisa - $ gh issue list --web - `), - Args: cmdutil.NoArgsQuoteReminder, - RunE: issueList, -} -var issueStatusCmd = &cobra.Command{ - Use: "status", - Short: "Show status of relevant issues", - Args: cmdutil.NoArgsQuoteReminder, - RunE: issueStatus, -} -var issueViewCmd = &cobra.Command{ - Use: "view { | }", - Short: "View an issue", - Args: cobra.ExactArgs(1), - Long: `Display the title, body, and other information about an issue. - -With '--web', open the issue in a web browser instead.`, - RunE: issueView, -} -var issueCloseCmd = &cobra.Command{ - Use: "close { | }", - Short: "Close issue", - Args: cobra.ExactArgs(1), - RunE: issueClose, -} -var issueReopenCmd = &cobra.Command{ - Use: "reopen { | }", - Short: "Reopen issue", - Args: cobra.ExactArgs(1), - RunE: issueReopen, -} - -func issueList(cmd *cobra.Command, args []string) error { - ctx := contextForCommand(cmd) - apiClient, err := apiClientForContext(ctx) - if err != nil { - return err - } - - baseRepo, err := determineBaseRepo(apiClient, cmd, ctx) - if err != nil { - return err - } - - web, err := cmd.Flags().GetBool("web") - if err != nil { - return err - } - - state, err := cmd.Flags().GetString("state") - if err != nil { - return err - } - - labels, err := cmd.Flags().GetStringSlice("label") - if err != nil { - return err - } - - assignee, err := cmd.Flags().GetString("assignee") - if err != nil { - return err - } - - limit, err := cmd.Flags().GetInt("limit") - if err != nil { - return err - } - if limit <= 0 { - return fmt.Errorf("invalid limit: %v", limit) - } - - author, err := cmd.Flags().GetString("author") - if err != nil { - return err - } - - mention, err := cmd.Flags().GetString("mention") - if err != nil { - return err - } - - milestone, err := cmd.Flags().GetString("milestone") - if err != nil { - return err - } - - if web { - issueListURL := ghrepo.GenerateRepoURL(baseRepo, "issues") - openURL, err := shared.ListURLWithQuery(issueListURL, shared.FilterOptions{ - Entity: "issue", - State: state, - Assignee: assignee, - Labels: labels, - Author: author, - Mention: mention, - Milestone: milestone, - }) - if err != nil { - return err - } - fmt.Fprintf(cmd.ErrOrStderr(), "Opening %s in your browser.\n", utils.DisplayURL(openURL)) - return utils.OpenInBrowser(openURL) - } - - listResult, err := api.IssueList(apiClient, baseRepo, state, labels, assignee, limit, author, mention, milestone) - if err != nil { - return err - } - - hasFilters := false - cmd.Flags().Visit(func(f *pflag.Flag) { - switch f.Name { - case "state", "label", "assignee", "author", "mention", "milestone": - hasFilters = true - } - }) - - title := shared.ListHeader(ghrepo.FullName(baseRepo), "issue", len(listResult.Issues), listResult.TotalCount, hasFilters) - if connectedToTerminal(cmd) { - fmt.Fprintf(colorableErr(cmd), "\n%s\n\n", title) - } - - out := cmd.OutOrStdout() - - printIssues(out, "", len(listResult.Issues), listResult.Issues) - - return nil -} - -func issueStatus(cmd *cobra.Command, args []string) error { - ctx := contextForCommand(cmd) - apiClient, err := apiClientForContext(ctx) - if err != nil { - return err - } - - baseRepo, err := determineBaseRepo(apiClient, cmd, ctx) - if err != nil { - return err - } - - currentUser, err := api.CurrentLoginName(apiClient, baseRepo.RepoHost()) - if err != nil { - return err - } - - issuePayload, err := api.IssueStatus(apiClient, baseRepo, currentUser) - if err != nil { - return err - } - - out := colorableOut(cmd) - - fmt.Fprintln(out, "") - fmt.Fprintf(out, "Relevant issues in %s\n", ghrepo.FullName(baseRepo)) - fmt.Fprintln(out, "") - - shared.PrintHeader(out, "Issues assigned to you") - if issuePayload.Assigned.TotalCount > 0 { - printIssues(out, " ", issuePayload.Assigned.TotalCount, issuePayload.Assigned.Issues) - } else { - message := " There are no issues assigned to you" - shared.PrintMessage(out, message) - } - fmt.Fprintln(out) - - shared.PrintHeader(out, "Issues mentioning you") - if issuePayload.Mentioned.TotalCount > 0 { - printIssues(out, " ", issuePayload.Mentioned.TotalCount, issuePayload.Mentioned.Issues) - } else { - shared.PrintMessage(out, " There are no issues mentioning you") - } - fmt.Fprintln(out) - - shared.PrintHeader(out, "Issues opened by you") - if issuePayload.Authored.TotalCount > 0 { - printIssues(out, " ", issuePayload.Authored.TotalCount, issuePayload.Authored.Issues) - } else { - shared.PrintMessage(out, " There are no issues opened by you") - } - fmt.Fprintln(out) - - return nil -} - -func issueView(cmd *cobra.Command, args []string) error { - ctx := contextForCommand(cmd) - - apiClient, err := apiClientForContext(ctx) - if err != nil { - return err - } - - issue, _, err := issueFromArg(ctx, apiClient, cmd, args[0]) - if err != nil { - return err - } - openURL := issue.URL - - web, err := cmd.Flags().GetBool("web") - if err != nil { - return err - } - - if web { - fmt.Fprintf(cmd.ErrOrStderr(), "Opening %s in your browser.\n", openURL) - return utils.OpenInBrowser(openURL) - } - if connectedToTerminal(cmd) { - return printHumanIssuePreview(colorableOut(cmd), issue) - } - - return printRawIssuePreview(cmd.OutOrStdout(), issue) -} - -func issueStateTitleWithColor(state string) string { - colorFunc := shared.ColorFuncForState(state) - return colorFunc(strings.Title(strings.ToLower(state))) -} - -func printRawIssuePreview(out io.Writer, issue *api.Issue) error { - assignees := issueAssigneeList(*issue) - labels := issueLabelList(*issue) - projects := issueProjectList(*issue) - - // Print empty strings for empty values so the number of metadata lines is consistent when - // processing many issues with head and grep. - fmt.Fprintf(out, "title:\t%s\n", issue.Title) - fmt.Fprintf(out, "state:\t%s\n", issue.State) - fmt.Fprintf(out, "author:\t%s\n", issue.Author.Login) - fmt.Fprintf(out, "labels:\t%s\n", labels) - fmt.Fprintf(out, "comments:\t%d\n", issue.Comments.TotalCount) - fmt.Fprintf(out, "assignees:\t%s\n", assignees) - fmt.Fprintf(out, "projects:\t%s\n", projects) - fmt.Fprintf(out, "milestone:\t%s\n", issue.Milestone.Title) - - fmt.Fprintln(out, "--") - fmt.Fprintln(out, issue.Body) - return nil -} - -func printHumanIssuePreview(out io.Writer, issue *api.Issue) error { - now := time.Now() - ago := now.Sub(issue.CreatedAt) - - // Header (Title and State) - fmt.Fprintln(out, utils.Bold(issue.Title)) - fmt.Fprint(out, issueStateTitleWithColor(issue.State)) - fmt.Fprintln(out, utils.Gray(fmt.Sprintf( - " • %s opened %s • %s", - issue.Author.Login, - utils.FuzzyAgo(ago), - utils.Pluralize(issue.Comments.TotalCount, "comment"), - ))) - - // Metadata - fmt.Fprintln(out) - if assignees := issueAssigneeList(*issue); assignees != "" { - fmt.Fprint(out, utils.Bold("Assignees: ")) - fmt.Fprintln(out, assignees) - } - if labels := issueLabelList(*issue); labels != "" { - fmt.Fprint(out, utils.Bold("Labels: ")) - fmt.Fprintln(out, labels) - } - if projects := issueProjectList(*issue); projects != "" { - fmt.Fprint(out, utils.Bold("Projects: ")) - fmt.Fprintln(out, projects) - } - if issue.Milestone.Title != "" { - fmt.Fprint(out, utils.Bold("Milestone: ")) - fmt.Fprintln(out, issue.Milestone.Title) - } - - // Body - if issue.Body != "" { - fmt.Fprintln(out) - md, err := utils.RenderMarkdown(issue.Body) - if err != nil { - return err - } - fmt.Fprintln(out, md) - } - fmt.Fprintln(out) - - // Footer - fmt.Fprintf(out, utils.Gray("View this issue on GitHub: %s\n"), issue.URL) - return nil -} - -func issueCreate(cmd *cobra.Command, args []string) error { - ctx := contextForCommand(cmd) - apiClient, err := apiClientForContext(ctx) - if err != nil { - return err - } - - // NB no auto forking like over in pr create - baseRepo, err := determineBaseRepo(apiClient, cmd, ctx) - if err != nil { - return err - } - - baseOverride, err := cmd.Flags().GetString("repo") - if err != nil { - return err - } - - var nonLegacyTemplateFiles []string - if baseOverride == "" { - if rootDir, err := git.ToplevelDir(); err == nil { - // TODO: figure out how to stub this in tests - nonLegacyTemplateFiles = githubtemplate.FindNonLegacy(rootDir, "ISSUE_TEMPLATE") - } - } - - title, err := cmd.Flags().GetString("title") - if err != nil { - return fmt.Errorf("could not parse title: %w", err) - } - body, err := cmd.Flags().GetString("body") - if err != nil { - return fmt.Errorf("could not parse body: %w", err) - } - - assignees, err := cmd.Flags().GetStringSlice("assignee") - if err != nil { - return fmt.Errorf("could not parse assignees: %w", err) - } - labelNames, err := cmd.Flags().GetStringSlice("label") - if err != nil { - return fmt.Errorf("could not parse labels: %w", err) - } - projectNames, err := cmd.Flags().GetStringSlice("project") - if err != nil { - return fmt.Errorf("could not parse projects: %w", err) - } - var milestoneTitles []string - if milestoneTitle, err := cmd.Flags().GetString("milestone"); err != nil { - return fmt.Errorf("could not parse milestone: %w", err) - } else if milestoneTitle != "" { - milestoneTitles = append(milestoneTitles, milestoneTitle) - } - - if isWeb, err := cmd.Flags().GetBool("web"); err == nil && isWeb { - openURL := ghrepo.GenerateRepoURL(baseRepo, "issues/new") - if title != "" || body != "" { - openURL, err = shared.WithPrAndIssueQueryParams(openURL, title, body, assignees, labelNames, projectNames, milestoneTitles) - if err != nil { - return err - } - } else if len(nonLegacyTemplateFiles) > 1 { - openURL += "/choose" - } - if connectedToTerminal(cmd) { - cmd.Printf("Opening %s in your browser.\n", utils.DisplayURL(openURL)) - } - return utils.OpenInBrowser(openURL) - } - - fmt.Fprintf(colorableErr(cmd), "\nCreating issue in %s\n\n", ghrepo.FullName(baseRepo)) - - repo, err := api.GitHubRepo(apiClient, baseRepo) - if err != nil { - return err - } - if !repo.HasIssuesEnabled { - return fmt.Errorf("the '%s' repository has disabled issues", ghrepo.FullName(baseRepo)) - } - - action := shared.SubmitAction - tb := shared.IssueMetadataState{ - Type: shared.IssueMetadata, - Assignees: assignees, - Labels: labelNames, - Projects: projectNames, - Milestones: milestoneTitles, - } - - interactive := !(cmd.Flags().Changed("title") && cmd.Flags().Changed("body")) - - if interactive && !connectedToTerminal(cmd) { - return fmt.Errorf("must provide --title and --body when not attached to a terminal") - } - - if interactive { - var legacyTemplateFile *string - if baseOverride == "" { - if rootDir, err := git.ToplevelDir(); err == nil { - // TODO: figure out how to stub this in tests - legacyTemplateFile = githubtemplate.FindLegacy(rootDir, "ISSUE_TEMPLATE") - } - } - - editorCommand, err := cmdutil.DetermineEditor(ctx.Config) - if err != nil { - return err - } - - err = shared.TitleBodySurvey(defaultStreams, editorCommand, &tb, apiClient, baseRepo, title, body, shared.Defaults{}, nonLegacyTemplateFiles, legacyTemplateFile, false, repo.ViewerCanTriage()) - if err != nil { - return fmt.Errorf("could not collect title and/or body: %w", err) - } - - action = tb.Action - - if tb.Action == shared.CancelAction { - fmt.Fprintln(cmd.ErrOrStderr(), "Discarding.") - - return nil - } - - if title == "" { - title = tb.Title - } - if body == "" { - body = tb.Body - } - } else { - if title == "" { - return fmt.Errorf("title can't be blank") - } - } - - if action == shared.PreviewAction { - openURL := ghrepo.GenerateRepoURL(baseRepo, "issues/new") - openURL, err = shared.WithPrAndIssueQueryParams(openURL, title, body, assignees, labelNames, projectNames, milestoneTitles) - if err != nil { - return err - } - // TODO could exceed max url length for explorer - fmt.Fprintf(cmd.ErrOrStderr(), "Opening %s in your browser.\n", utils.DisplayURL(openURL)) - return utils.OpenInBrowser(openURL) - } else if action == shared.SubmitAction { - params := map[string]interface{}{ - "title": title, - "body": body, - } - - err = shared.AddMetadataToIssueParams(apiClient, baseRepo, params, &tb) - if err != nil { - return err - } - - newIssue, err := api.IssueCreate(apiClient, repo, params) - if err != nil { - return err - } - - fmt.Fprintln(cmd.OutOrStdout(), newIssue.URL) - } else { - panic("Unreachable state") - } - - return nil -} - -func printIssues(w io.Writer, prefix string, totalCount int, issues []api.Issue) { - io := &iostreams.IOStreams{Out: w} - io.SetStdoutTTY(utils.IsTerminal(w)) - table := utils.NewTablePrinter(io) - for _, issue := range issues { - issueNum := strconv.Itoa(issue.Number) - if table.IsTTY() { - issueNum = "#" + issueNum - } - issueNum = prefix + issueNum - labels := issueLabelList(issue) - if labels != "" && table.IsTTY() { - labels = fmt.Sprintf("(%s)", labels) - } - now := time.Now() - ago := now.Sub(issue.UpdatedAt) - table.AddField(issueNum, nil, shared.ColorFuncForState(issue.State)) - if !table.IsTTY() { - table.AddField(issue.State, nil, nil) - } - table.AddField(text.ReplaceExcessiveWhitespace(issue.Title), nil, nil) - table.AddField(labels, nil, utils.Gray) - if table.IsTTY() { - table.AddField(utils.FuzzyAgo(ago), nil, utils.Gray) - } else { - table.AddField(issue.UpdatedAt.String(), nil, nil) - } - table.EndRow() - } - _ = table.Render() - remaining := totalCount - len(issues) - if remaining > 0 { - fmt.Fprintf(w, utils.Gray("%sAnd %d more\n"), prefix, remaining) - } -} - -func issueAssigneeList(issue api.Issue) string { - if len(issue.Assignees.Nodes) == 0 { - return "" - } - - AssigneeNames := make([]string, 0, len(issue.Assignees.Nodes)) - for _, assignee := range issue.Assignees.Nodes { - AssigneeNames = append(AssigneeNames, assignee.Login) - } - - list := strings.Join(AssigneeNames, ", ") - if issue.Assignees.TotalCount > len(issue.Assignees.Nodes) { - list += ", …" - } - return list -} - -func issueLabelList(issue api.Issue) string { - if len(issue.Labels.Nodes) == 0 { - return "" - } - - labelNames := make([]string, 0, len(issue.Labels.Nodes)) - for _, label := range issue.Labels.Nodes { - labelNames = append(labelNames, label.Name) - } - - list := strings.Join(labelNames, ", ") - if issue.Labels.TotalCount > len(issue.Labels.Nodes) { - list += ", …" - } - return list -} - -func issueProjectList(issue api.Issue) string { - if len(issue.ProjectCards.Nodes) == 0 { - return "" - } - - projectNames := make([]string, 0, len(issue.ProjectCards.Nodes)) - for _, project := range issue.ProjectCards.Nodes { - colName := project.Column.Name - if colName == "" { - colName = "Awaiting triage" - } - projectNames = append(projectNames, fmt.Sprintf("%s (%s)", project.Project.Name, colName)) - } - - list := strings.Join(projectNames, ", ") - if issue.ProjectCards.TotalCount > len(issue.ProjectCards.Nodes) { - list += ", …" - } - return list -} - -func issueClose(cmd *cobra.Command, args []string) error { - ctx := contextForCommand(cmd) - apiClient, err := apiClientForContext(ctx) - if err != nil { - return err - } - - issue, baseRepo, err := issueFromArg(ctx, apiClient, cmd, args[0]) - if err != nil { - return err - } - - if issue.Closed { - fmt.Fprintf(colorableErr(cmd), "%s Issue #%d (%s) is already closed\n", utils.Yellow("!"), issue.Number, issue.Title) - return nil - } - - err = api.IssueClose(apiClient, baseRepo, *issue) - if err != nil { - return err - } - - fmt.Fprintf(colorableErr(cmd), "%s Closed issue #%d (%s)\n", utils.Red("✔"), issue.Number, issue.Title) - - return nil -} - -func issueReopen(cmd *cobra.Command, args []string) error { - ctx := contextForCommand(cmd) - apiClient, err := apiClientForContext(ctx) - if err != nil { - return err - } - - issue, baseRepo, err := issueFromArg(ctx, apiClient, cmd, args[0]) - if err != nil { - return err - } - - if !issue.Closed { - fmt.Fprintf(colorableErr(cmd), "%s Issue #%d (%s) is already open\n", utils.Yellow("!"), issue.Number, issue.Title) - return nil - } - - err = api.IssueReopen(apiClient, baseRepo, *issue) - if err != nil { - return err - } - - fmt.Fprintf(colorableErr(cmd), "%s Reopened issue #%d (%s)\n", utils.Green("✔"), issue.Number, issue.Title) - - return nil -} diff --git a/command/issue_test.go b/command/issue_test.go deleted file mode 100644 index 16d613d72..000000000 --- a/command/issue_test.go +++ /dev/null @@ -1,962 +0,0 @@ -package command - -import ( - "bytes" - "encoding/json" - "io/ioutil" - "os/exec" - "regexp" - "strings" - "testing" - - "github.com/cli/cli/internal/run" - "github.com/cli/cli/pkg/httpmock" - "github.com/cli/cli/test" - "github.com/google/go-cmp/cmp" - "github.com/stretchr/testify/assert" -) - -func TestIssueStatus(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - defer stubTerminal(true)() - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - http.Register( - httpmock.GraphQL(`query UserCurrent\b`), - httpmock.StringResponse(`{"data":{"viewer":{"login":"octocat"}}}`)) - http.Register( - httpmock.GraphQL(`query IssueStatus\b`), - httpmock.FileResponse("../test/fixtures/issueStatus.json")) - - output, err := RunCommand("issue status") - if err != nil { - t.Errorf("error running command `issue status`: %v", err) - } - - expectedIssues := []*regexp.Regexp{ - regexp.MustCompile(`(?m)8.*carrots.*about.*ago`), - regexp.MustCompile(`(?m)9.*squash.*about.*ago`), - regexp.MustCompile(`(?m)10.*broccoli.*about.*ago`), - regexp.MustCompile(`(?m)11.*swiss chard.*about.*ago`), - } - - for _, r := range expectedIssues { - if !r.MatchString(output.String()) { - t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output) - return - } - } -} - -func TestIssueStatus_blankSlate(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - http.Register( - httpmock.GraphQL(`query UserCurrent\b`), - httpmock.StringResponse(`{"data":{"viewer":{"login":"octocat"}}}`)) - http.Register( - httpmock.GraphQL(`query IssueStatus\b`), - httpmock.StringResponse(` - { "data": { "repository": { - "hasIssuesEnabled": true, - "assigned": { "nodes": [] }, - "mentioned": { "nodes": [] }, - "authored": { "nodes": [] } - } } }`)) - - output, err := RunCommand("issue status") - if err != nil { - t.Errorf("error running command `issue status`: %v", err) - } - - expectedOutput := ` -Relevant issues in OWNER/REPO - -Issues assigned to you - There are no issues assigned to you - -Issues mentioning you - There are no issues mentioning you - -Issues opened by you - There are no issues opened by you - -` - if output.String() != expectedOutput { - t.Errorf("expected %q, got %q", expectedOutput, output) - } -} - -func TestIssueStatus_disabledIssues(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - http.Register( - httpmock.GraphQL(`query UserCurrent\b`), - httpmock.StringResponse(`{"data":{"viewer":{"login":"octocat"}}}`)) - http.Register( - httpmock.GraphQL(`query IssueStatus\b`), - httpmock.StringResponse(` - { "data": { "repository": { - "hasIssuesEnabled": false - } } }`)) - - _, err := RunCommand("issue status") - if err == nil || err.Error() != "the 'OWNER/REPO' repository has disabled issues" { - t.Errorf("error running command `issue status`: %v", err) - } -} - -func TestIssueList_nontty(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - defer stubTerminal(false)() - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - - http.Register( - httpmock.GraphQL(`query IssueList\b`), - httpmock.FileResponse("../test/fixtures/issueList.json")) - - output, err := RunCommand("issue list") - if err != nil { - t.Errorf("error running command `issue list`: %v", err) - } - - eq(t, output.Stderr(), "") - test.ExpectLines(t, output.String(), - `1[\t]+number won[\t]+label[\t]+\d+`, - `2[\t]+number too[\t]+label[\t]+\d+`, - `4[\t]+number fore[\t]+label[\t]+\d+`) -} - -func TestIssueList_tty(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - defer stubTerminal(true)() - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - http.Register( - httpmock.GraphQL(`query IssueList\b`), - httpmock.FileResponse("../test/fixtures/issueList.json")) - - output, err := RunCommand("issue list") - if err != nil { - t.Errorf("error running command `issue list`: %v", err) - } - - eq(t, output.Stderr(), ` -Showing 3 of 3 open issues in OWNER/REPO - -`) - - test.ExpectLines(t, output.String(), - "number won", - "number too", - "number fore") -} - -func TestIssueList_tty_withFlags(t *testing.T) { - defer stubTerminal(true)() - initBlankContext("", "OWNER/REPO", "master") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - http.Register( - httpmock.GraphQL(`query IssueList\b`), - httpmock.GraphQLQuery(` - { "data": { "repository": { - "hasIssuesEnabled": true, - "issues": { "nodes": [] } - } } }`, func(_ string, params map[string]interface{}) { - assert.Equal(t, "probablyCher", params["assignee"].(string)) - assert.Equal(t, "foo", params["author"].(string)) - assert.Equal(t, "me", params["mention"].(string)) - assert.Equal(t, "1.x", params["milestone"].(string)) - assert.Equal(t, []interface{}{"web", "bug"}, params["labels"].([]interface{})) - assert.Equal(t, []interface{}{"OPEN"}, params["states"].([]interface{})) - })) - - output, err := RunCommand("issue list -a probablyCher -l web,bug -s open -A foo --mention me --milestone 1.x") - if err != nil { - t.Errorf("error running command `issue list`: %v", err) - } - - eq(t, output.String(), "") - eq(t, output.Stderr(), ` -No issues match your search in OWNER/REPO - -`) -} - -func TestIssueList_withInvalidLimitFlag(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - - _, err := RunCommand("issue list --limit=0") - - if err == nil || err.Error() != "invalid limit: 0" { - t.Errorf("error running command `issue list`: %v", err) - } -} - -func TestIssueList_nullAssigneeLabels(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { - "hasIssuesEnabled": true, - "issues": { "nodes": [] } - } } } - `)) - - _, err := RunCommand("issue list") - if err != nil { - t.Errorf("error running command `issue list`: %v", err) - } - - bodyBytes, _ := ioutil.ReadAll(http.Requests[1].Body) - reqBody := struct { - Variables map[string]interface{} - }{} - _ = json.Unmarshal(bodyBytes, &reqBody) - - _, assigneeDeclared := reqBody.Variables["assignee"] - _, labelsDeclared := reqBody.Variables["labels"] - eq(t, assigneeDeclared, false) - eq(t, labelsDeclared, false) -} - -func TestIssueList_disabledIssues(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { - "hasIssuesEnabled": false - } } } - `)) - - _, err := RunCommand("issue list") - if err == nil || err.Error() != "the 'OWNER/REPO' repository has disabled issues" { - t.Errorf("error running command `issue list`: %v", err) - } -} - -func TestIssueList_web(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - - var seenCmd *exec.Cmd - restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - seenCmd = cmd - return &test.OutputStub{} - }) - defer restoreCmd() - - output, err := RunCommand("issue list --web -a peter -A john -l bug -l docs -L 10 -s all --mention frank --milestone v1.1") - if err != nil { - t.Errorf("error running command `issue list` with `--web` flag: %v", err) - } - - expectedURL := "https://github.com/OWNER/REPO/issues?q=is%3Aissue+assignee%3Apeter+label%3Abug+label%3Adocs+author%3Ajohn+mentions%3Afrank+milestone%3Av1.1" - - eq(t, output.String(), "") - eq(t, output.Stderr(), "Opening github.com/OWNER/REPO/issues in your browser.\n") - - if seenCmd == nil { - t.Fatal("expected a command to run") - } - url := seenCmd.Args[len(seenCmd.Args)-1] - eq(t, url, expectedURL) -} - -func TestIssueView_web(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "hasIssuesEnabled": true, "issue": { - "number": 123, - "url": "https://github.com/OWNER/REPO/issues/123" - } } } } - `)) - - var seenCmd *exec.Cmd - restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - seenCmd = cmd - return &test.OutputStub{} - }) - defer restoreCmd() - - output, err := RunCommand("issue view -w 123") - if err != nil { - t.Errorf("error running command `issue view`: %v", err) - } - - eq(t, output.String(), "") - eq(t, output.Stderr(), "Opening https://github.com/OWNER/REPO/issues/123 in your browser.\n") - - if seenCmd == nil { - t.Fatal("expected a command to run") - } - url := seenCmd.Args[len(seenCmd.Args)-1] - eq(t, url, "https://github.com/OWNER/REPO/issues/123") -} - -func TestIssueView_web_numberArgWithHash(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "hasIssuesEnabled": true, "issue": { - "number": 123, - "url": "https://github.com/OWNER/REPO/issues/123" - } } } } - `)) - - var seenCmd *exec.Cmd - restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - seenCmd = cmd - return &test.OutputStub{} - }) - defer restoreCmd() - - output, err := RunCommand("issue view -w \"#123\"") - if err != nil { - t.Errorf("error running command `issue view`: %v", err) - } - - eq(t, output.String(), "") - eq(t, output.Stderr(), "Opening https://github.com/OWNER/REPO/issues/123 in your browser.\n") - - if seenCmd == nil { - t.Fatal("expected a command to run") - } - url := seenCmd.Args[len(seenCmd.Args)-1] - eq(t, url, "https://github.com/OWNER/REPO/issues/123") -} - -func TestIssueView_nontty_Preview(t *testing.T) { - defer stubTerminal(false)() - tests := map[string]struct { - ownerRepo string - command string - fixture string - expectedOutputs []string - }{ - "Open issue without metadata": { - ownerRepo: "master", - command: "issue view 123", - fixture: "../test/fixtures/issueView_preview.json", - expectedOutputs: []string{ - `title:\tix of coins`, - `state:\tOPEN`, - `comments:\t9`, - `author:\tmarseilles`, - `assignees:`, - `\*\*bold story\*\*`, - }, - }, - "Open issue with metadata": { - ownerRepo: "master", - command: "issue view 123", - fixture: "../test/fixtures/issueView_previewWithMetadata.json", - expectedOutputs: []string{ - `title:\tix of coins`, - `assignees:\tmarseilles, monaco`, - `author:\tmarseilles`, - `state:\tOPEN`, - `comments:\t9`, - `labels:\tone, two, three, four, five`, - `projects:\tProject 1 \(column A\), Project 2 \(column B\), Project 3 \(column C\), Project 4 \(Awaiting triage\)\n`, - `milestone:\tuluru\n`, - `\*\*bold story\*\*`, - }, - }, - "Open issue with empty body": { - ownerRepo: "master", - command: "issue view 123", - fixture: "../test/fixtures/issueView_previewWithEmptyBody.json", - expectedOutputs: []string{ - `title:\tix of coins`, - `state:\tOPEN`, - `author:\tmarseilles`, - `labels:\ttarot`, - }, - }, - "Closed issue": { - ownerRepo: "master", - command: "issue view 123", - fixture: "../test/fixtures/issueView_previewClosedState.json", - expectedOutputs: []string{ - `title:\tix of coins`, - `state:\tCLOSED`, - `\*\*bold story\*\*`, - `author:\tmarseilles`, - `labels:\ttarot`, - `\*\*bold story\*\*`, - }, - }, - } - for name, tc := range tests { - t.Run(name, func(t *testing.T) { - initBlankContext("", "OWNER/REPO", tc.ownerRepo) - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - - http.Register(httpmock.GraphQL(`query IssueByNumber\b`), httpmock.FileResponse(tc.fixture)) - - output, err := RunCommand(tc.command) - if err != nil { - t.Errorf("error running command `%v`: %v", tc.command, err) - } - - eq(t, output.Stderr(), "") - - test.ExpectLines(t, output.String(), tc.expectedOutputs...) - }) - } -} - -func TestIssueView_tty_Preview(t *testing.T) { - defer stubTerminal(true)() - tests := map[string]struct { - ownerRepo string - command string - fixture string - expectedOutputs []string - }{ - "Open issue without metadata": { - ownerRepo: "master", - command: "issue view 123", - fixture: "../test/fixtures/issueView_preview.json", - expectedOutputs: []string{ - `ix of coins`, - `Open.*marseilles opened about 292 years ago.*9 comments`, - `bold story`, - `View this issue on GitHub: https://github.com/OWNER/REPO/issues/123`, - }, - }, - "Open issue with metadata": { - ownerRepo: "master", - command: "issue view 123", - fixture: "../test/fixtures/issueView_previewWithMetadata.json", - expectedOutputs: []string{ - `ix of coins`, - `Open.*marseilles opened about 292 years ago.*9 comments`, - `Assignees:.*marseilles, monaco\n`, - `Labels:.*one, two, three, four, five\n`, - `Projects:.*Project 1 \(column A\), Project 2 \(column B\), Project 3 \(column C\), Project 4 \(Awaiting triage\)\n`, - `Milestone:.*uluru\n`, - `bold story`, - `View this issue on GitHub: https://github.com/OWNER/REPO/issues/123`, - }, - }, - "Open issue with empty body": { - ownerRepo: "master", - command: "issue view 123", - fixture: "../test/fixtures/issueView_previewWithEmptyBody.json", - expectedOutputs: []string{ - `ix of coins`, - `Open.*marseilles opened about 292 years ago.*9 comments`, - `View this issue on GitHub: https://github.com/OWNER/REPO/issues/123`, - }, - }, - "Closed issue": { - ownerRepo: "master", - command: "issue view 123", - fixture: "../test/fixtures/issueView_previewClosedState.json", - expectedOutputs: []string{ - `ix of coins`, - `Closed.*marseilles opened about 292 years ago.*9 comments`, - `bold story`, - `View this issue on GitHub: https://github.com/OWNER/REPO/issues/123`, - }, - }, - } - for name, tc := range tests { - t.Run(name, func(t *testing.T) { - initBlankContext("", "OWNER/REPO", tc.ownerRepo) - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - http.Register(httpmock.GraphQL(`query IssueByNumber\b`), httpmock.FileResponse(tc.fixture)) - - output, err := RunCommand(tc.command) - if err != nil { - t.Errorf("error running command `%v`: %v", tc.command, err) - } - - eq(t, output.Stderr(), "") - - test.ExpectLines(t, output.String(), tc.expectedOutputs...) - }) - } -} - -func TestIssueView_web_notFound(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - - http.StubResponse(200, bytes.NewBufferString(` - { "errors": [ - { "message": "Could not resolve to an Issue with the number of 9999." } - ] } - `)) - - var seenCmd *exec.Cmd - restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - seenCmd = cmd - return &test.OutputStub{} - }) - defer restoreCmd() - - _, err := RunCommand("issue view -w 9999") - if err == nil || err.Error() != "GraphQL error: Could not resolve to an Issue with the number of 9999." { - t.Errorf("error running command `issue view`: %v", err) - } - - if seenCmd != nil { - t.Fatal("did not expect any command to run") - } -} - -func TestIssueView_disabledIssues(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { - "id": "REPOID", - "hasIssuesEnabled": false - } } } - `)) - - _, err := RunCommand(`issue view 6666`) - if err == nil || err.Error() != "the 'OWNER/REPO' repository has disabled issues" { - t.Errorf("error running command `issue view`: %v", err) - } -} - -func TestIssueView_web_urlArg(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - http := initFakeHTTP() - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { "hasIssuesEnabled": true, "issue": { - "number": 123, - "url": "https://github.com/OWNER/REPO/issues/123" - } } } } - `)) - - var seenCmd *exec.Cmd - restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - seenCmd = cmd - return &test.OutputStub{} - }) - defer restoreCmd() - - output, err := RunCommand("issue view -w https://github.com/OWNER/REPO/issues/123") - if err != nil { - t.Errorf("error running command `issue view`: %v", err) - } - - eq(t, output.String(), "") - - if seenCmd == nil { - t.Fatal("expected a command to run") - } - url := seenCmd.Args[len(seenCmd.Args)-1] - eq(t, url, "https://github.com/OWNER/REPO/issues/123") -} - -func TestIssueCreate_nontty_error(t *testing.T) { - defer stubTerminal(false)() - initBlankContext("", "OWNER/REPO", "master") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { - "id": "REPOID", - "hasIssuesEnabled": true - } } } - `)) - - _, err := RunCommand(`issue create -t hello`) - if err == nil { - t.Fatal("expected error running command `issue create`") - } - - assert.Equal(t, "must provide --title and --body when not attached to a terminal", err.Error()) - -} - -func TestIssueCreate(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - - 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(`issue create -t hello -b "cash rules everything around me"`) - if err != nil { - t.Errorf("error running command `issue create`: %v", err) - } - - bodyBytes, _ := ioutil.ReadAll(http.Requests[2].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, "hello") - eq(t, reqBody.Variables.Input.Body, "cash rules everything around me") - - eq(t, output.String(), "https://github.com/OWNER/REPO/issues/12\n") -} - -func TestIssueCreate_metadata(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - http := initFakeHTTP() - defer http.Verify(t) - - http.Register( - httpmock.GraphQL(`query RepositoryNetwork\b`), - httpmock.StringResponse(httpmock.RepoNetworkStubResponse("OWNER", "REPO", "master", "WRITE"))) - http.Register( - httpmock.GraphQL(`query RepositoryInfo\b`), - httpmock.StringResponse(` - { "data": { "repository": { - "id": "REPOID", - "hasIssuesEnabled": true, - "viewerPermission": "WRITE" - } } } - `)) - 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(`query RepositoryMilestoneList\b`), - httpmock.StringResponse(` - { "data": { "repository": { "milestones": { - "nodes": [ - { "title": "GA", "id": "GAID" }, - { "title": "Big One.oh", "id": "BIGONEID" } - ], - "pageInfo": { "hasNextPage": false } - } } } } - `)) - http.Register( - httpmock.GraphQL(`query RepositoryProjectList\b`), - httpmock.StringResponse(` - { "data": { "repository": { "projects": { - "nodes": [ - { "name": "Cleanup", "id": "CLEANUPID" }, - { "name": "Roadmap", "id": "ROADMAPID" } - ], - "pageInfo": { "hasNextPage": false } - } } } } - `)) - http.Register( - httpmock.GraphQL(`query OrganizationProjectList\b`), - httpmock.StringResponse(` - { "data": { "organization": null }, - "errors": [{ - "type": "NOT_FOUND", - "path": [ "organization" ], - "message": "Could not resolve to an Organization with the login of 'OWNER'." - }] - } - `)) - 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"], "TITLE") - eq(t, inputs["body"], "BODY") - eq(t, inputs["assigneeIds"], []interface{}{"MONAID"}) - eq(t, inputs["labelIds"], []interface{}{"BUGID", "TODOID"}) - eq(t, inputs["projectIds"], []interface{}{"ROADMAPID"}) - eq(t, inputs["milestoneId"], "BIGONEID") - if v, ok := inputs["userIds"]; ok { - t.Errorf("did not expect userIds: %v", v) - } - if v, ok := inputs["teamIds"]; ok { - t.Errorf("did not expect teamIds: %v", v) - } - })) - - output, err := RunCommand(`issue create -t TITLE -b BODY -a monalisa -l bug -l todo -p roadmap -m 'big one.oh'`) - 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_disabledIssues(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { - "id": "REPOID", - "hasIssuesEnabled": false - } } } - `)) - - _, err := RunCommand(`issue create -t heres -b johnny`) - if err == nil || err.Error() != "the 'OWNER/REPO' repository has disabled issues" { - t.Errorf("error running command `issue create`: %v", err) - } -} - -func TestIssueCreate_web(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - - defer stubTerminal(true)() - - var seenCmd *exec.Cmd - restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - seenCmd = cmd - return &test.OutputStub{} - }) - defer restoreCmd() - - output, err := RunCommand(`issue create --web`) - if err != nil { - t.Errorf("error running command `issue create`: %v", err) - } - - if seenCmd == nil { - t.Fatal("expected a command to run") - } - url := seenCmd.Args[len(seenCmd.Args)-1] - eq(t, url, "https://github.com/OWNER/REPO/issues/new") - eq(t, output.String(), "Opening github.com/OWNER/REPO/issues/new in your browser.\n") - eq(t, output.Stderr(), "") -} - -func TestIssueCreate_webTitleBody(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - - defer stubTerminal(true)() - - var seenCmd *exec.Cmd - restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - seenCmd = cmd - return &test.OutputStub{} - }) - defer restoreCmd() - - output, err := RunCommand(`issue create -w -t mytitle -b mybody`) - if err != nil { - t.Errorf("error running command `issue create`: %v", err) - } - - if seenCmd == nil { - t.Fatal("expected a command to run") - } - url := strings.ReplaceAll(seenCmd.Args[len(seenCmd.Args)-1], "^", "") - eq(t, url, "https://github.com/OWNER/REPO/issues/new?body=mybody&title=mytitle") - eq(t, output.String(), "Opening github.com/OWNER/REPO/issues/new in your browser.\n") -} - -func TestIssueStateTitleWithColor(t *testing.T) { - tests := map[string]struct { - state string - want string - }{ - "Open state": {state: "OPEN", want: "Open"}, - "Closed state": {state: "CLOSED", want: "Closed"}, - } - - for name, tc := range tests { - t.Run(name, func(t *testing.T) { - got := issueStateTitleWithColor(tc.state) - diff := cmp.Diff(tc.want, got) - if diff != "" { - t.Fatalf(diff) - } - }) - } -} - -func TestIssueClose(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { - "hasIssuesEnabled": true, - "issue": { "number": 13, "title": "The title of the issue"} - } } } - `)) - - http.StubResponse(200, bytes.NewBufferString(`{"id": "THE-ID"}`)) - - output, err := RunCommand("issue close 13") - if err != nil { - t.Fatalf("error running command `issue close`: %v", err) - } - - r := regexp.MustCompile(`Closed issue #13 \(The title of the issue\)`) - - if !r.MatchString(output.Stderr()) { - t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) - } -} - -func TestIssueClose_alreadyClosed(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { - "hasIssuesEnabled": true, - "issue": { "number": 13, "title": "The title of the issue", "closed": true} - } } } - `)) - - http.StubResponse(200, bytes.NewBufferString(`{"id": "THE-ID"}`)) - - output, err := RunCommand("issue close 13") - if err != nil { - t.Fatalf("error running command `issue close`: %v", err) - } - - r := regexp.MustCompile(`Issue #13 \(The title of the issue\) is already closed`) - - if !r.MatchString(output.Stderr()) { - t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) - } -} - -func TestIssueClose_issuesDisabled(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { - "hasIssuesEnabled": false - } } } - `)) - - _, err := RunCommand("issue close 13") - if err == nil || err.Error() != "the 'OWNER/REPO' repository has disabled issues" { - t.Fatalf("got error: %v", err) - } -} - -func TestIssueReopen(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { - "hasIssuesEnabled": true, - "issue": { "number": 2, "closed": true, "title": "The title of the issue"} - } } } - `)) - - http.StubResponse(200, bytes.NewBufferString(`{"id": "THE-ID"}`)) - - output, err := RunCommand("issue reopen 2") - if err != nil { - t.Fatalf("error running command `issue reopen`: %v", err) - } - - r := regexp.MustCompile(`Reopened issue #2 \(The title of the issue\)`) - - if !r.MatchString(output.Stderr()) { - t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) - } -} - -func TestIssueReopen_alreadyOpen(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { - "hasIssuesEnabled": true, - "issue": { "number": 2, "closed": false, "title": "The title of the issue"} - } } } - `)) - - http.StubResponse(200, bytes.NewBufferString(`{"id": "THE-ID"}`)) - - output, err := RunCommand("issue reopen 2") - if err != nil { - t.Fatalf("error running command `issue reopen`: %v", err) - } - - r := regexp.MustCompile(`Issue #2 \(The title of the issue\) is already open`) - - if !r.MatchString(output.Stderr()) { - t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) - } -} - -func TestIssueReopen_issuesDisabled(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - - http.StubResponse(200, bytes.NewBufferString(` - { "data": { "repository": { - "hasIssuesEnabled": false - } } } - `)) - - _, err := RunCommand("issue reopen 2") - if err == nil || err.Error() != "the 'OWNER/REPO' repository has disabled issues" { - t.Fatalf("got error: %v", err) - } -} diff --git a/command/root.go b/command/root.go index ad64676f3..19206b648 100644 --- a/command/root.go +++ b/command/root.go @@ -23,6 +23,7 @@ import ( "github.com/cli/cli/internal/run" apiCmd "github.com/cli/cli/pkg/cmd/api" gistCreateCmd "github.com/cli/cli/pkg/cmd/gist/create" + issueCmd "github.com/cli/cli/pkg/cmd/issue" prCmd "github.com/cli/cli/pkg/cmd/pr" repoCmd "github.com/cli/cli/pkg/cmd/repo" repoCloneCmd "github.com/cli/cli/pkg/cmd/repo/clone" @@ -169,6 +170,7 @@ func init() { repoCmd.Cmd.AddCommand(creditsCmd.NewCmdRepoCredits(&repoResolvingCmdFactory, nil)) RootCmd.AddCommand(prCmd.NewCmdPR(&repoResolvingCmdFactory)) + RootCmd.AddCommand(issueCmd.NewCmdIssue(&repoResolvingCmdFactory)) RootCmd.AddCommand(creditsCmd.NewCmdCredits(cmdFactory, nil)) } diff --git a/pkg/cmd/issue/close/close.go b/pkg/cmd/issue/close/close.go new file mode 100644 index 000000000..66314a86d --- /dev/null +++ b/pkg/cmd/issue/close/close.go @@ -0,0 +1,80 @@ +package close + +import ( + "fmt" + "net/http" + + "github.com/cli/cli/api" + "github.com/cli/cli/internal/config" + "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/pkg/cmd/issue/shared" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/utils" + "github.com/spf13/cobra" +) + +type CloseOptions struct { + HttpClient func() (*http.Client, error) + Config func() (config.Config, error) + IO *iostreams.IOStreams + BaseRepo func() (ghrepo.Interface, error) + + SelectorArg string +} + +func NewCmdClose(f *cmdutil.Factory, runF func(*CloseOptions) error) *cobra.Command { + opts := &CloseOptions{ + IO: f.IOStreams, + HttpClient: f.HttpClient, + Config: f.Config, + } + + cmd := &cobra.Command{ + Use: "close { | }", + Short: "Close issue", + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + // support `-R, --repo` override + opts.BaseRepo = f.BaseRepo + + if len(args) > 0 { + opts.SelectorArg = args[0] + } + + if runF != nil { + return runF(opts) + } + return closeRun(opts) + }, + } + + return cmd +} + +func closeRun(opts *CloseOptions) error { + httpClient, err := opts.HttpClient() + if err != nil { + return err + } + apiClient := api.NewClientFromHTTP(httpClient) + + issue, baseRepo, err := shared.IssueFromArg(apiClient, opts.BaseRepo, opts.SelectorArg) + if err != nil { + return err + } + + if issue.Closed { + fmt.Fprintf(opts.IO.ErrOut, "%s Issue #%d (%s) is already closed\n", utils.Yellow("!"), issue.Number, issue.Title) + return nil + } + + err = api.IssueClose(apiClient, baseRepo, *issue) + if err != nil { + return err + } + + fmt.Fprintf(opts.IO.ErrOut, "%s Closed issue #%d (%s)\n", utils.Red("✔"), issue.Number, issue.Title) + + return nil +} diff --git a/pkg/cmd/issue/close/close_test.go b/pkg/cmd/issue/close/close_test.go new file mode 100644 index 000000000..eadf7f440 --- /dev/null +++ b/pkg/cmd/issue/close/close_test.go @@ -0,0 +1,119 @@ +package close + +import ( + "bytes" + "io/ioutil" + "net/http" + "regexp" + "testing" + + "github.com/cli/cli/internal/config" + "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/httpmock" + "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/test" + "github.com/google/shlex" +) + +func runCommand(rt http.RoundTripper, isTTY bool, cli string) (*test.CmdOut, error) { + io, _, stdout, stderr := iostreams.Test() + io.SetStdoutTTY(isTTY) + io.SetStdinTTY(isTTY) + io.SetStderrTTY(isTTY) + + factory := &cmdutil.Factory{ + IOStreams: io, + HttpClient: func() (*http.Client, error) { + return &http.Client{Transport: rt}, nil + }, + Config: func() (config.Config, error) { + return config.NewBlankConfig(), nil + }, + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("OWNER", "REPO"), nil + }, + } + + cmd := NewCmdClose(factory, nil) + + argv, err := shlex.Split(cli) + if err != nil { + return nil, err + } + cmd.SetArgs(argv) + + cmd.SetIn(&bytes.Buffer{}) + cmd.SetOut(ioutil.Discard) + cmd.SetErr(ioutil.Discard) + + _, err = cmd.ExecuteC() + return &test.CmdOut{ + OutBuf: stdout, + ErrBuf: stderr, + }, err +} + +func TestIssueClose(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { + "hasIssuesEnabled": true, + "issue": { "number": 13, "title": "The title of the issue"} + } } } + `)) + + http.StubResponse(200, bytes.NewBufferString(`{"id": "THE-ID"}`)) + + output, err := runCommand(http, true, "13") + if err != nil { + t.Fatalf("error running command `issue close`: %v", err) + } + + r := regexp.MustCompile(`Closed issue #13 \(The title of the issue\)`) + + if !r.MatchString(output.Stderr()) { + t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) + } +} + +func TestIssueClose_alreadyClosed(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { + "hasIssuesEnabled": true, + "issue": { "number": 13, "title": "The title of the issue", "closed": true} + } } } + `)) + + output, err := runCommand(http, true, "13") + if err != nil { + t.Fatalf("error running command `issue close`: %v", err) + } + + r := regexp.MustCompile(`Issue #13 \(The title of the issue\) is already closed`) + + if !r.MatchString(output.Stderr()) { + t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) + } +} + +func TestIssueClose_issuesDisabled(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { + "hasIssuesEnabled": false + } } } + `)) + + _, err := runCommand(http, true, "13") + if err == nil || err.Error() != "the 'OWNER/REPO' repository has disabled issues" { + t.Fatalf("got error: %v", err) + } +} diff --git a/pkg/cmd/issue/create/create.go b/pkg/cmd/issue/create/create.go new file mode 100644 index 000000000..8d47e4e27 --- /dev/null +++ b/pkg/cmd/issue/create/create.go @@ -0,0 +1,228 @@ +package create + +import ( + "fmt" + "net/http" + + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/api" + "github.com/cli/cli/git" + "github.com/cli/cli/internal/config" + "github.com/cli/cli/internal/ghrepo" + prShared "github.com/cli/cli/pkg/cmd/pr/shared" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/githubtemplate" + "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/utils" + "github.com/spf13/cobra" +) + +type CreateOptions struct { + HttpClient func() (*http.Client, error) + Config func() (config.Config, error) + IO *iostreams.IOStreams + BaseRepo func() (ghrepo.Interface, error) + + RepoOverride string + WebMode bool + + Title string + TitleProvided bool + Body string + BodyProvided bool + + Assignees []string + Labels []string + Projects []string + Milestone string +} + +func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Command { + opts := &CreateOptions{ + IO: f.IOStreams, + HttpClient: f.HttpClient, + Config: f.Config, + } + + cmd := &cobra.Command{ + Use: "create", + Short: "Create a new issue", + Example: heredoc.Doc(` + $ gh issue create --title "I found a bug" --body "Nothing works" + $ gh issue create --label "bug,help wanted" + $ gh issue create --label bug --label "help wanted" + $ gh issue create --assignee monalisa,hubot + $ gh issue create --project "Roadmap" + `), + Args: cmdutil.NoArgsQuoteReminder, + RunE: func(cmd *cobra.Command, args []string) error { + // support `-R, --repo` override + opts.BaseRepo = f.BaseRepo + + opts.TitleProvided = cmd.Flags().Changed("title") + opts.BodyProvided = cmd.Flags().Changed("body") + opts.RepoOverride, _ = cmd.Flags().GetString("repo") + + if runF != nil { + return runF(opts) + } + return createRun(opts) + }, + } + + cmd.Flags().StringVarP(&opts.Title, "title", "t", "", "Supply a title. Will prompt for one otherwise.") + cmd.Flags().StringVarP(&opts.Body, "body", "b", "", "Supply a body. Will prompt for one otherwise.") + cmd.Flags().BoolVarP(&opts.WebMode, "web", "w", false, "Open the browser to create an issue") + cmd.Flags().StringSliceVarP(&opts.Assignees, "assignee", "a", nil, "Assign people by their `login`") + 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`") + + return cmd +} + +func createRun(opts *CreateOptions) error { + httpClient, err := opts.HttpClient() + if err != nil { + return err + } + apiClient := api.NewClientFromHTTP(httpClient) + + baseRepo, err := opts.BaseRepo() + if err != nil { + return err + } + + var nonLegacyTemplateFiles []string + if opts.RepoOverride == "" { + if rootDir, err := git.ToplevelDir(); err == nil { + // TODO: figure out how to stub this in tests + nonLegacyTemplateFiles = githubtemplate.FindNonLegacy(rootDir, "ISSUE_TEMPLATE") + } + } + + isTerminal := opts.IO.IsStdoutTTY() + + var milestones []string + if opts.Milestone != "" { + milestones = []string{opts.Milestone} + } + + if opts.WebMode { + openURL := ghrepo.GenerateRepoURL(baseRepo, "issues/new") + if opts.Title != "" || opts.Body != "" { + openURL, err = prShared.WithPrAndIssueQueryParams(openURL, opts.Title, opts.Body, opts.Assignees, opts.Labels, opts.Projects, milestones) + if err != nil { + return err + } + } else if len(nonLegacyTemplateFiles) > 1 { + openURL += "/choose" + } + if isTerminal { + fmt.Fprintf(opts.IO.ErrOut, "Opening %s in your browser.\n", utils.DisplayURL(openURL)) + } + return utils.OpenInBrowser(openURL) + } + + if isTerminal { + fmt.Fprintf(opts.IO.ErrOut, "\nCreating issue in %s\n\n", ghrepo.FullName(baseRepo)) + } + + repo, err := api.GitHubRepo(apiClient, baseRepo) + if err != nil { + return err + } + if !repo.HasIssuesEnabled { + return fmt.Errorf("the '%s' repository has disabled issues", ghrepo.FullName(baseRepo)) + } + + action := prShared.SubmitAction + tb := prShared.IssueMetadataState{ + Type: prShared.IssueMetadata, + Assignees: opts.Assignees, + Labels: opts.Labels, + Projects: opts.Projects, + Milestones: milestones, + } + + title := opts.Title + body := opts.Body + + interactive := !(opts.TitleProvided && opts.BodyProvided) + + if interactive && !isTerminal { + return fmt.Errorf("must provide --title and --body when not attached to a terminal") + } + + if interactive { + var legacyTemplateFile *string + if opts.RepoOverride == "" { + if rootDir, err := git.ToplevelDir(); err == nil { + // TODO: figure out how to stub this in tests + legacyTemplateFile = githubtemplate.FindLegacy(rootDir, "ISSUE_TEMPLATE") + } + } + + editorCommand, err := cmdutil.DetermineEditor(opts.Config) + if err != nil { + return err + } + + err = prShared.TitleBodySurvey(opts.IO, editorCommand, &tb, apiClient, baseRepo, title, body, prShared.Defaults{}, nonLegacyTemplateFiles, legacyTemplateFile, false, repo.ViewerCanTriage()) + if err != nil { + return fmt.Errorf("could not collect title and/or body: %w", err) + } + + action = tb.Action + + if tb.Action == prShared.CancelAction { + fmt.Fprintln(opts.IO.ErrOut, "Discarding.") + + return nil + } + + if title == "" { + title = tb.Title + } + if body == "" { + body = tb.Body + } + } else { + if title == "" { + return fmt.Errorf("title can't be blank") + } + } + + if action == prShared.PreviewAction { + openURL := ghrepo.GenerateRepoURL(baseRepo, "issues/new") + openURL, err = prShared.WithPrAndIssueQueryParams(openURL, title, body, tb.Assignees, tb.Labels, tb.Projects, tb.Milestones) + if err != nil { + return err + } + if isTerminal { + fmt.Fprintf(opts.IO.ErrOut, "Opening %s in your browser.\n", utils.DisplayURL(openURL)) + } + return utils.OpenInBrowser(openURL) + } else if action == prShared.SubmitAction { + params := map[string]interface{}{ + "title": title, + "body": body, + } + + err = prShared.AddMetadataToIssueParams(apiClient, baseRepo, params, &tb) + if err != nil { + return err + } + + newIssue, err := api.IssueCreate(apiClient, repo, params) + if err != nil { + return err + } + + fmt.Fprintln(opts.IO.Out, newIssue.URL) + } else { + panic("Unreachable state") + } + + return nil +} diff --git a/pkg/cmd/issue/create/create_test.go b/pkg/cmd/issue/create/create_test.go new file mode 100644 index 000000000..b483d090b --- /dev/null +++ b/pkg/cmd/issue/create/create_test.go @@ -0,0 +1,280 @@ +package create + +import ( + "bytes" + "encoding/json" + "io/ioutil" + "net/http" + "os/exec" + "reflect" + "strings" + "testing" + + "github.com/cli/cli/internal/config" + "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/internal/run" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/httpmock" + "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/test" + "github.com/google/shlex" + "github.com/stretchr/testify/assert" +) + +func eq(t *testing.T, got interface{}, expected interface{}) { + t.Helper() + if !reflect.DeepEqual(got, expected) { + t.Errorf("expected: %v, got: %v", expected, got) + } +} + +func runCommand(rt http.RoundTripper, isTTY bool, cli string) (*test.CmdOut, error) { + io, _, stdout, stderr := iostreams.Test() + io.SetStdoutTTY(isTTY) + io.SetStdinTTY(isTTY) + io.SetStderrTTY(isTTY) + + factory := &cmdutil.Factory{ + IOStreams: io, + HttpClient: func() (*http.Client, error) { + return &http.Client{Transport: rt}, nil + }, + Config: func() (config.Config, error) { + return config.NewBlankConfig(), nil + }, + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("OWNER", "REPO"), nil + }, + } + + cmd := NewCmdCreate(factory, nil) + + argv, err := shlex.Split(cli) + if err != nil { + return nil, err + } + cmd.SetArgs(argv) + + cmd.SetIn(&bytes.Buffer{}) + cmd.SetOut(ioutil.Discard) + cmd.SetErr(ioutil.Discard) + + _, err = cmd.ExecuteC() + return &test.CmdOut{ + OutBuf: stdout, + ErrBuf: stderr, + }, err +} + +func TestIssueCreate_nontty_error(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { + "id": "REPOID", + "hasIssuesEnabled": true + } } } + `)) + + _, err := runCommand(http, false, `-t hello`) + if err == nil { + t.Fatal("expected error running command `issue create`") + } + + assert.Equal(t, "must provide --title and --body when not attached to a terminal", err.Error()) + +} + +func TestIssueCreate(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, `-t hello -b "cash rules everything around me"`) + 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, "hello") + eq(t, reqBody.Variables.Input.Body, "cash rules everything around me") + + eq(t, output.String(), "https://github.com/OWNER/REPO/issues/12\n") +} + +func TestIssueCreate_metadata(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + + http.Register( + httpmock.GraphQL(`query RepositoryInfo\b`), + httpmock.StringResponse(` + { "data": { "repository": { + "id": "REPOID", + "hasIssuesEnabled": true, + "viewerPermission": "WRITE" + } } } + `)) + http.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(`query RepositoryMilestoneList\b`), + httpmock.StringResponse(` + { "data": { "repository": { "milestones": { + "nodes": [ + { "title": "GA", "id": "GAID" }, + { "title": "Big One.oh", "id": "BIGONEID" } + ], + "pageInfo": { "hasNextPage": false } + } } } } + `)) + http.Register( + httpmock.GraphQL(`query RepositoryProjectList\b`), + httpmock.StringResponse(` + { "data": { "repository": { "projects": { + "nodes": [ + { "name": "Cleanup", "id": "CLEANUPID" }, + { "name": "Roadmap", "id": "ROADMAPID" } + ], + "pageInfo": { "hasNextPage": false } + } } } } + `)) + http.Register( + httpmock.GraphQL(`query OrganizationProjectList\b`), + httpmock.StringResponse(` + { "data": { "organization": null }, + "errors": [{ + "type": "NOT_FOUND", + "path": [ "organization" ], + "message": "Could not resolve to an Organization with the login of 'OWNER'." + }] + } + `)) + 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"], "TITLE") + eq(t, inputs["body"], "BODY") + eq(t, inputs["assigneeIds"], []interface{}{"MONAID"}) + eq(t, inputs["labelIds"], []interface{}{"BUGID", "TODOID"}) + eq(t, inputs["projectIds"], []interface{}{"ROADMAPID"}) + eq(t, inputs["milestoneId"], "BIGONEID") + if v, ok := inputs["userIds"]; ok { + t.Errorf("did not expect userIds: %v", v) + } + if v, ok := inputs["teamIds"]; ok { + t.Errorf("did not expect teamIds: %v", v) + } + })) + + output, err := runCommand(http, true, `-t TITLE -b BODY -a monalisa -l bug -l todo -p roadmap -m 'big one.oh'`) + 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_disabledIssues(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { + "id": "REPOID", + "hasIssuesEnabled": false + } } } + `)) + + _, err := runCommand(http, true, `-t heres -b johnny`) + if err == nil || err.Error() != "the 'OWNER/REPO' repository has disabled issues" { + t.Errorf("error running command `issue create`: %v", err) + } +} + +func TestIssueCreate_web(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + + 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, `--web`) + if err != nil { + t.Errorf("error running command `issue create`: %v", err) + } + + if seenCmd == nil { + t.Fatal("expected a command to run") + } + url := seenCmd.Args[len(seenCmd.Args)-1] + eq(t, url, "https://github.com/OWNER/REPO/issues/new") + eq(t, output.String(), "") + eq(t, output.Stderr(), "Opening github.com/OWNER/REPO/issues/new in your browser.\n") +} + +func TestIssueCreate_webTitleBody(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + + 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, `-w -t mytitle -b mybody`) + if err != nil { + t.Errorf("error running command `issue create`: %v", err) + } + + if seenCmd == nil { + t.Fatal("expected a command to run") + } + url := strings.ReplaceAll(seenCmd.Args[len(seenCmd.Args)-1], "^", "") + eq(t, url, "https://github.com/OWNER/REPO/issues/new?body=mybody&title=mytitle") + eq(t, output.String(), "") + eq(t, output.Stderr(), "Opening github.com/OWNER/REPO/issues/new in your browser.\n") +} diff --git a/pkg/cmd/issue/issue.go b/pkg/cmd/issue/issue.go new file mode 100644 index 000000000..91ed04b22 --- /dev/null +++ b/pkg/cmd/issue/issue.go @@ -0,0 +1,54 @@ +package issue + +import ( + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/internal/ghrepo" + cmdClose "github.com/cli/cli/pkg/cmd/issue/close" + cmdCreate "github.com/cli/cli/pkg/cmd/issue/create" + cmdList "github.com/cli/cli/pkg/cmd/issue/list" + cmdReopen "github.com/cli/cli/pkg/cmd/issue/reopen" + cmdStatus "github.com/cli/cli/pkg/cmd/issue/status" + cmdView "github.com/cli/cli/pkg/cmd/issue/view" + "github.com/cli/cli/pkg/cmdutil" + "github.com/spf13/cobra" +) + +func NewCmdIssue(f *cmdutil.Factory) *cobra.Command { + cmd := &cobra.Command{ + Use: "issue ", + Short: "Manage issues", + Long: `Work with GitHub issues`, + Example: heredoc.Doc(` + $ gh issue list + $ gh issue create --label bug + $ gh issue view --web + `), + Annotations: map[string]string{ + "IsCore": "true", + "help:arguments": heredoc.Doc(` + An issue can be supplied as argument in any of the following formats: + - by number, e.g. "123"; or + - by URL, e.g. "https://github.com/OWNER/REPO/issues/123". + `), + }, + PersistentPreRun: func(cmd *cobra.Command, args []string) { + if repo, _ := cmd.Flags().GetString("repo"); repo != "" { + // NOTE: this mutates the factory + f.BaseRepo = func() (ghrepo.Interface, error) { + return ghrepo.FromFullName(repo) + } + } + }, + } + + cmd.PersistentFlags().StringP("repo", "R", "", "Select another repository using the `OWNER/REPO` format") + + cmd.AddCommand(cmdClose.NewCmdClose(f, nil)) + cmd.AddCommand(cmdCreate.NewCmdCreate(f, nil)) + cmd.AddCommand(cmdList.NewCmdList(f, nil)) + cmd.AddCommand(cmdReopen.NewCmdReopen(f, nil)) + cmd.AddCommand(cmdStatus.NewCmdStatus(f, nil)) + cmd.AddCommand(cmdView.NewCmdView(f, nil)) + + return cmd +} diff --git a/test/fixtures/issueList.json b/pkg/cmd/issue/list/fixtures/issueList.json similarity index 100% rename from test/fixtures/issueList.json rename to pkg/cmd/issue/list/fixtures/issueList.json diff --git a/pkg/cmd/issue/list/list.go b/pkg/cmd/issue/list/list.go new file mode 100644 index 000000000..d599da975 --- /dev/null +++ b/pkg/cmd/issue/list/list.go @@ -0,0 +1,127 @@ +package list + +import ( + "fmt" + "net/http" + + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/api" + "github.com/cli/cli/internal/config" + "github.com/cli/cli/internal/ghrepo" + issueShared "github.com/cli/cli/pkg/cmd/issue/shared" + prShared "github.com/cli/cli/pkg/cmd/pr/shared" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/utils" + "github.com/spf13/cobra" +) + +type ListOptions struct { + HttpClient func() (*http.Client, error) + Config func() (config.Config, error) + IO *iostreams.IOStreams + BaseRepo func() (ghrepo.Interface, error) + + WebMode bool + + Assignee string + Labels []string + State string + LimitResults int + Author string + Mention string + Milestone string +} + +func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Command { + opts := &ListOptions{ + IO: f.IOStreams, + HttpClient: f.HttpClient, + Config: f.Config, + } + + cmd := &cobra.Command{ + Use: "list", + Short: "List and filter issues in this repository", + Example: heredoc.Doc(` + $ gh issue list -l "help wanted" + $ gh issue list -A monalisa + $ gh issue list --web + `), + Args: cmdutil.NoArgsQuoteReminder, + RunE: func(cmd *cobra.Command, args []string) error { + // support `-R, --repo` override + opts.BaseRepo = f.BaseRepo + + if opts.LimitResults < 1 { + return &cmdutil.FlagError{Err: fmt.Errorf("invalid limit: %v", opts.LimitResults)} + } + + if runF != nil { + return runF(opts) + } + return listRun(opts) + }, + } + + cmd.Flags().BoolVarP(&opts.WebMode, "web", "w", false, "Open the browser to list the issue(s)") + cmd.Flags().StringVarP(&opts.Assignee, "assignee", "a", "", "Filter by assignee") + cmd.Flags().StringSliceVarP(&opts.Labels, "label", "l", nil, "Filter by labels") + cmd.Flags().StringVarP(&opts.State, "state", "s", "open", "Filter by state: {open|closed|all}") + cmd.Flags().IntVarP(&opts.LimitResults, "limit", "L", 30, "Maximum number of issues to fetch") + cmd.Flags().StringVarP(&opts.Author, "author", "A", "", "Filter by author") + cmd.Flags().StringVar(&opts.Mention, "mention", "", "Filter by mention") + cmd.Flags().StringVarP(&opts.Milestone, "milestone", "m", "", "Filter by milestone `name`") + + return cmd +} + +func listRun(opts *ListOptions) error { + httpClient, err := opts.HttpClient() + if err != nil { + return err + } + apiClient := api.NewClientFromHTTP(httpClient) + + baseRepo, err := opts.BaseRepo() + if err != nil { + return err + } + + isTerminal := opts.IO.IsStdoutTTY() + + if opts.WebMode { + issueListURL := ghrepo.GenerateRepoURL(baseRepo, "issues") + openURL, err := prShared.ListURLWithQuery(issueListURL, prShared.FilterOptions{ + Entity: "issue", + State: opts.State, + Assignee: opts.Assignee, + Labels: opts.Labels, + Author: opts.Author, + Mention: opts.Mention, + Milestone: opts.Milestone, + }) + if err != nil { + return err + } + if isTerminal { + fmt.Fprintf(opts.IO.ErrOut, "Opening %s in your browser.\n", utils.DisplayURL(openURL)) + } + return utils.OpenInBrowser(openURL) + } + + listResult, err := api.IssueList(apiClient, baseRepo, opts.State, opts.Labels, opts.Assignee, opts.LimitResults, opts.Author, opts.Mention, opts.Milestone) + if err != nil { + return err + } + + if isTerminal { + hasFilters := opts.State != "open" || len(opts.Labels) > 0 || opts.Assignee != "" || opts.Author != "" || opts.Mention != "" || opts.Milestone != "" + title := prShared.ListHeader(ghrepo.FullName(baseRepo), "issue", len(listResult.Issues), listResult.TotalCount, hasFilters) + fmt.Fprintf(opts.IO.ErrOut, "\n%s\n\n", title) + } + + issueShared.PrintIssues(opts.IO, "", len(listResult.Issues), listResult.Issues) + + return nil +} diff --git a/pkg/cmd/issue/list/list_test.go b/pkg/cmd/issue/list/list_test.go new file mode 100644 index 000000000..7a42eb286 --- /dev/null +++ b/pkg/cmd/issue/list/list_test.go @@ -0,0 +1,223 @@ +package list + +import ( + "bytes" + "encoding/json" + "io/ioutil" + "net/http" + "os/exec" + "reflect" + "testing" + + "github.com/cli/cli/internal/config" + "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/internal/run" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/httpmock" + "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/test" + "github.com/google/shlex" + "github.com/stretchr/testify/assert" +) + +func eq(t *testing.T, got interface{}, expected interface{}) { + t.Helper() + if !reflect.DeepEqual(got, expected) { + t.Errorf("expected: %v, got: %v", expected, got) + } +} + +func runCommand(rt http.RoundTripper, isTTY bool, cli string) (*test.CmdOut, error) { + io, _, stdout, stderr := iostreams.Test() + io.SetStdoutTTY(isTTY) + io.SetStdinTTY(isTTY) + io.SetStderrTTY(isTTY) + + factory := &cmdutil.Factory{ + IOStreams: io, + HttpClient: func() (*http.Client, error) { + return &http.Client{Transport: rt}, nil + }, + Config: func() (config.Config, error) { + return config.NewBlankConfig(), nil + }, + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("OWNER", "REPO"), nil + }, + } + + cmd := NewCmdList(factory, nil) + + argv, err := shlex.Split(cli) + if err != nil { + return nil, err + } + cmd.SetArgs(argv) + + cmd.SetIn(&bytes.Buffer{}) + cmd.SetOut(ioutil.Discard) + cmd.SetErr(ioutil.Discard) + + _, err = cmd.ExecuteC() + return &test.CmdOut{ + OutBuf: stdout, + ErrBuf: stderr, + }, err +} +func TestIssueList_nontty(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + + http.Register( + httpmock.GraphQL(`query IssueList\b`), + httpmock.FileResponse("./fixtures/issueList.json")) + + output, err := runCommand(http, false, "") + if err != nil { + t.Errorf("error running command `issue list`: %v", err) + } + + eq(t, output.Stderr(), "") + test.ExpectLines(t, output.String(), + `1[\t]+number won[\t]+label[\t]+\d+`, + `2[\t]+number too[\t]+label[\t]+\d+`, + `4[\t]+number fore[\t]+label[\t]+\d+`) +} + +func TestIssueList_tty(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + + http.Register( + httpmock.GraphQL(`query IssueList\b`), + httpmock.FileResponse("./fixtures/issueList.json")) + + output, err := runCommand(http, true, "") + if err != nil { + t.Errorf("error running command `issue list`: %v", err) + } + + eq(t, output.Stderr(), ` +Showing 3 of 3 open issues in OWNER/REPO + +`) + + test.ExpectLines(t, output.String(), + "number won", + "number too", + "number fore") +} + +func TestIssueList_tty_withFlags(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + + http.Register( + httpmock.GraphQL(`query IssueList\b`), + httpmock.GraphQLQuery(` + { "data": { "repository": { + "hasIssuesEnabled": true, + "issues": { "nodes": [] } + } } }`, func(_ string, params map[string]interface{}) { + assert.Equal(t, "probablyCher", params["assignee"].(string)) + assert.Equal(t, "foo", params["author"].(string)) + assert.Equal(t, "me", params["mention"].(string)) + assert.Equal(t, "1.x", params["milestone"].(string)) + assert.Equal(t, []interface{}{"web", "bug"}, params["labels"].([]interface{})) + assert.Equal(t, []interface{}{"OPEN"}, params["states"].([]interface{})) + })) + + output, err := runCommand(http, true, "-a probablyCher -l web,bug -s open -A foo --mention me --milestone 1.x") + if err != nil { + t.Errorf("error running command `issue list`: %v", err) + } + + eq(t, output.String(), "") + eq(t, output.Stderr(), ` +No issues match your search in OWNER/REPO + +`) +} + +func TestIssueList_withInvalidLimitFlag(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + + _, err := runCommand(http, true, "--limit=0") + + if err == nil || err.Error() != "invalid limit: 0" { + t.Errorf("error running command `issue list`: %v", err) + } +} + +func TestIssueList_nullAssigneeLabels(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { + "hasIssuesEnabled": true, + "issues": { "nodes": [] } + } } } + `)) + + _, err := runCommand(http, true, "") + if err != nil { + t.Errorf("error running command `issue list`: %v", err) + } + + bodyBytes, _ := ioutil.ReadAll(http.Requests[0].Body) + reqBody := struct { + Variables map[string]interface{} + }{} + _ = json.Unmarshal(bodyBytes, &reqBody) + + _, assigneeDeclared := reqBody.Variables["assignee"] + _, labelsDeclared := reqBody.Variables["labels"] + eq(t, assigneeDeclared, false) + eq(t, labelsDeclared, false) +} + +func TestIssueList_disabledIssues(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { + "hasIssuesEnabled": false + } } } + `)) + + _, err := runCommand(http, true, "") + if err == nil || err.Error() != "the 'OWNER/REPO' repository has disabled issues" { + t.Errorf("error running command `issue list`: %v", err) + } +} + +func TestIssueList_web(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + + 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, "--web -a peter -A john -l bug -l docs -L 10 -s all --mention frank --milestone v1.1") + if err != nil { + t.Errorf("error running command `issue list` with `--web` flag: %v", err) + } + + expectedURL := "https://github.com/OWNER/REPO/issues?q=is%3Aissue+assignee%3Apeter+label%3Abug+label%3Adocs+author%3Ajohn+mentions%3Afrank+milestone%3Av1.1" + + eq(t, output.String(), "") + eq(t, output.Stderr(), "Opening github.com/OWNER/REPO/issues in your browser.\n") + + if seenCmd == nil { + t.Fatal("expected a command to run") + } + url := seenCmd.Args[len(seenCmd.Args)-1] + eq(t, url, expectedURL) +} diff --git a/pkg/cmd/issue/reopen/reopen.go b/pkg/cmd/issue/reopen/reopen.go new file mode 100644 index 000000000..72a052503 --- /dev/null +++ b/pkg/cmd/issue/reopen/reopen.go @@ -0,0 +1,80 @@ +package reopen + +import ( + "fmt" + "net/http" + + "github.com/cli/cli/api" + "github.com/cli/cli/internal/config" + "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/pkg/cmd/issue/shared" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/utils" + "github.com/spf13/cobra" +) + +type ReopenOptions struct { + HttpClient func() (*http.Client, error) + Config func() (config.Config, error) + IO *iostreams.IOStreams + BaseRepo func() (ghrepo.Interface, error) + + SelectorArg string +} + +func NewCmdReopen(f *cmdutil.Factory, runF func(*ReopenOptions) error) *cobra.Command { + opts := &ReopenOptions{ + IO: f.IOStreams, + HttpClient: f.HttpClient, + Config: f.Config, + } + + cmd := &cobra.Command{ + Use: "reopen { | }", + Short: "Reopen issue", + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + // support `-R, --repo` override + opts.BaseRepo = f.BaseRepo + + if len(args) > 0 { + opts.SelectorArg = args[0] + } + + if runF != nil { + return runF(opts) + } + return reopenRun(opts) + }, + } + + return cmd +} + +func reopenRun(opts *ReopenOptions) error { + httpClient, err := opts.HttpClient() + if err != nil { + return err + } + apiClient := api.NewClientFromHTTP(httpClient) + + issue, baseRepo, err := shared.IssueFromArg(apiClient, opts.BaseRepo, opts.SelectorArg) + if err != nil { + return err + } + + if !issue.Closed { + fmt.Fprintf(opts.IO.ErrOut, "%s Issue #%d (%s) is already open\n", utils.Yellow("!"), issue.Number, issue.Title) + return nil + } + + err = api.IssueReopen(apiClient, baseRepo, *issue) + if err != nil { + return err + } + + fmt.Fprintf(opts.IO.ErrOut, "%s Reopened issue #%d (%s)\n", utils.Green("✔"), issue.Number, issue.Title) + + return nil +} diff --git a/pkg/cmd/issue/reopen/reopen_test.go b/pkg/cmd/issue/reopen/reopen_test.go new file mode 100644 index 000000000..df8e8ecd7 --- /dev/null +++ b/pkg/cmd/issue/reopen/reopen_test.go @@ -0,0 +1,119 @@ +package reopen + +import ( + "bytes" + "io/ioutil" + "net/http" + "regexp" + "testing" + + "github.com/cli/cli/internal/config" + "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/httpmock" + "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/test" + "github.com/google/shlex" +) + +func runCommand(rt http.RoundTripper, isTTY bool, cli string) (*test.CmdOut, error) { + io, _, stdout, stderr := iostreams.Test() + io.SetStdoutTTY(isTTY) + io.SetStdinTTY(isTTY) + io.SetStderrTTY(isTTY) + + factory := &cmdutil.Factory{ + IOStreams: io, + HttpClient: func() (*http.Client, error) { + return &http.Client{Transport: rt}, nil + }, + Config: func() (config.Config, error) { + return config.NewBlankConfig(), nil + }, + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("OWNER", "REPO"), nil + }, + } + + cmd := NewCmdReopen(factory, nil) + + argv, err := shlex.Split(cli) + if err != nil { + return nil, err + } + cmd.SetArgs(argv) + + cmd.SetIn(&bytes.Buffer{}) + cmd.SetOut(ioutil.Discard) + cmd.SetErr(ioutil.Discard) + + _, err = cmd.ExecuteC() + return &test.CmdOut{ + OutBuf: stdout, + ErrBuf: stderr, + }, err +} + +func TestIssueReopen(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { + "hasIssuesEnabled": true, + "issue": { "number": 2, "closed": true, "title": "The title of the issue"} + } } } + `)) + + http.StubResponse(200, bytes.NewBufferString(`{"id": "THE-ID"}`)) + + output, err := runCommand(http, true, "2") + if err != nil { + t.Fatalf("error running command `issue reopen`: %v", err) + } + + r := regexp.MustCompile(`Reopened issue #2 \(The title of the issue\)`) + + if !r.MatchString(output.Stderr()) { + t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) + } +} + +func TestIssueReopen_alreadyOpen(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { + "hasIssuesEnabled": true, + "issue": { "number": 2, "closed": false, "title": "The title of the issue"} + } } } + `)) + + output, err := runCommand(http, true, "2") + if err != nil { + t.Fatalf("error running command `issue reopen`: %v", err) + } + + r := regexp.MustCompile(`Issue #2 \(The title of the issue\) is already open`) + + if !r.MatchString(output.Stderr()) { + t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) + } +} + +func TestIssueReopen_issuesDisabled(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { + "hasIssuesEnabled": false + } } } + `)) + + _, err := runCommand(http, true, "2") + if err == nil || err.Error() != "the 'OWNER/REPO' repository has disabled issues" { + t.Fatalf("got error: %v", err) + } +} diff --git a/pkg/cmd/issue/shared/display.go b/pkg/cmd/issue/shared/display.go new file mode 100644 index 000000000..983052f19 --- /dev/null +++ b/pkg/cmd/issue/shared/display.go @@ -0,0 +1,65 @@ +package shared + +import ( + "fmt" + "strconv" + "strings" + "time" + + "github.com/cli/cli/api" + prShared "github.com/cli/cli/pkg/cmd/pr/shared" + "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/pkg/text" + "github.com/cli/cli/utils" +) + +func PrintIssues(io *iostreams.IOStreams, prefix string, totalCount int, issues []api.Issue) { + table := utils.NewTablePrinter(io) + for _, issue := range issues { + issueNum := strconv.Itoa(issue.Number) + if table.IsTTY() { + issueNum = "#" + issueNum + } + issueNum = prefix + issueNum + labels := IssueLabelList(issue) + if labels != "" && table.IsTTY() { + labels = fmt.Sprintf("(%s)", labels) + } + now := time.Now() + ago := now.Sub(issue.UpdatedAt) + table.AddField(issueNum, nil, prShared.ColorFuncForState(issue.State)) + if !table.IsTTY() { + table.AddField(issue.State, nil, nil) + } + table.AddField(text.ReplaceExcessiveWhitespace(issue.Title), nil, nil) + table.AddField(labels, nil, utils.Gray) + if table.IsTTY() { + table.AddField(utils.FuzzyAgo(ago), nil, utils.Gray) + } else { + table.AddField(issue.UpdatedAt.String(), nil, nil) + } + table.EndRow() + } + _ = table.Render() + remaining := totalCount - len(issues) + if remaining > 0 { + fmt.Fprintf(io.Out, utils.Gray("%sAnd %d more\n"), prefix, remaining) + } +} + +func IssueLabelList(issue api.Issue) string { + if len(issue.Labels.Nodes) == 0 { + return "" + } + + labelNames := make([]string, 0, len(issue.Labels.Nodes)) + for _, label := range issue.Labels.Nodes { + labelNames = append(labelNames, label.Name) + } + + list := strings.Join(labelNames, ", ") + if issue.Labels.TotalCount > len(issue.Labels.Nodes) { + list += ", …" + } + return list +} diff --git a/command/issue_lookup.go b/pkg/cmd/issue/shared/lookup.go similarity index 83% rename from command/issue_lookup.go rename to pkg/cmd/issue/shared/lookup.go index aa8e0b12c..90a729599 100644 --- a/command/issue_lookup.go +++ b/pkg/cmd/issue/shared/lookup.go @@ -1,4 +1,4 @@ -package command +package shared import ( "fmt" @@ -8,12 +8,10 @@ import ( "strings" "github.com/cli/cli/api" - "github.com/cli/cli/context" "github.com/cli/cli/internal/ghrepo" - "github.com/spf13/cobra" ) -func issueFromArg(ctx context.Context, apiClient *api.Client, cmd *cobra.Command, arg string) (*api.Issue, ghrepo.Interface, error) { +func IssueFromArg(apiClient *api.Client, baseRepoFn func() (ghrepo.Interface, error), arg string) (*api.Issue, ghrepo.Interface, error) { issue, baseRepo, err := issueFromURL(apiClient, arg) if err != nil { return nil, nil, err @@ -22,7 +20,7 @@ func issueFromArg(ctx context.Context, apiClient *api.Client, cmd *cobra.Command return issue, baseRepo, nil } - baseRepo, err = determineBaseRepo(apiClient, cmd, ctx) + baseRepo, err = baseRepoFn() if err != nil { return nil, nil, fmt.Errorf("could not determine base repo: %w", err) } diff --git a/test/fixtures/issueStatus.json b/pkg/cmd/issue/status/fixtures/issueStatus.json similarity index 100% rename from test/fixtures/issueStatus.json rename to pkg/cmd/issue/status/fixtures/issueStatus.json diff --git a/pkg/cmd/issue/status/status.go b/pkg/cmd/issue/status/status.go new file mode 100644 index 000000000..d1b68bc0d --- /dev/null +++ b/pkg/cmd/issue/status/status.go @@ -0,0 +1,103 @@ +package status + +import ( + "fmt" + "net/http" + + "github.com/cli/cli/api" + "github.com/cli/cli/internal/config" + "github.com/cli/cli/internal/ghrepo" + issueShared "github.com/cli/cli/pkg/cmd/issue/shared" + prShared "github.com/cli/cli/pkg/cmd/pr/shared" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/iostreams" + "github.com/spf13/cobra" +) + +type StatusOptions struct { + HttpClient func() (*http.Client, error) + Config func() (config.Config, error) + IO *iostreams.IOStreams + BaseRepo func() (ghrepo.Interface, error) +} + +func NewCmdStatus(f *cmdutil.Factory, runF func(*StatusOptions) error) *cobra.Command { + opts := &StatusOptions{ + IO: f.IOStreams, + HttpClient: f.HttpClient, + Config: f.Config, + } + + cmd := &cobra.Command{ + Use: "status", + Short: "Show status of relevant issues", + Args: cmdutil.NoArgsQuoteReminder, + RunE: func(cmd *cobra.Command, args []string) error { + // support `-R, --repo` override + opts.BaseRepo = f.BaseRepo + + if runF != nil { + return runF(opts) + } + return statusRun(opts) + }, + } + + return cmd +} + +func statusRun(opts *StatusOptions) error { + httpClient, err := opts.HttpClient() + if err != nil { + return err + } + apiClient := api.NewClientFromHTTP(httpClient) + + baseRepo, err := opts.BaseRepo() + if err != nil { + return err + } + + currentUser, err := api.CurrentLoginName(apiClient, baseRepo.RepoHost()) + if err != nil { + return err + } + + issuePayload, err := api.IssueStatus(apiClient, baseRepo, currentUser) + if err != nil { + return err + } + + out := opts.IO.Out + + fmt.Fprintln(out, "") + fmt.Fprintf(out, "Relevant issues in %s\n", ghrepo.FullName(baseRepo)) + fmt.Fprintln(out, "") + + prShared.PrintHeader(out, "Issues assigned to you") + if issuePayload.Assigned.TotalCount > 0 { + issueShared.PrintIssues(opts.IO, " ", issuePayload.Assigned.TotalCount, issuePayload.Assigned.Issues) + } else { + message := " There are no issues assigned to you" + prShared.PrintMessage(out, message) + } + fmt.Fprintln(out) + + prShared.PrintHeader(out, "Issues mentioning you") + if issuePayload.Mentioned.TotalCount > 0 { + issueShared.PrintIssues(opts.IO, " ", issuePayload.Mentioned.TotalCount, issuePayload.Mentioned.Issues) + } else { + prShared.PrintMessage(out, " There are no issues mentioning you") + } + fmt.Fprintln(out) + + prShared.PrintHeader(out, "Issues opened by you") + if issuePayload.Authored.TotalCount > 0 { + issueShared.PrintIssues(opts.IO, " ", issuePayload.Authored.TotalCount, issuePayload.Authored.Issues) + } else { + prShared.PrintMessage(out, " There are no issues opened by you") + } + fmt.Fprintln(out) + + return nil +} diff --git a/pkg/cmd/issue/status/status_test.go b/pkg/cmd/issue/status/status_test.go new file mode 100644 index 000000000..6087abe1a --- /dev/null +++ b/pkg/cmd/issue/status/status_test.go @@ -0,0 +1,146 @@ +package status + +import ( + "bytes" + "io/ioutil" + "net/http" + "regexp" + "testing" + + "github.com/cli/cli/internal/config" + "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/httpmock" + "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/test" + "github.com/google/shlex" +) + +func runCommand(rt http.RoundTripper, isTTY bool, cli string) (*test.CmdOut, error) { + io, _, stdout, stderr := iostreams.Test() + io.SetStdoutTTY(isTTY) + io.SetStdinTTY(isTTY) + io.SetStderrTTY(isTTY) + + factory := &cmdutil.Factory{ + IOStreams: io, + HttpClient: func() (*http.Client, error) { + return &http.Client{Transport: rt}, nil + }, + Config: func() (config.Config, error) { + return config.NewBlankConfig(), nil + }, + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("OWNER", "REPO"), nil + }, + } + + cmd := NewCmdStatus(factory, nil) + + argv, err := shlex.Split(cli) + if err != nil { + return nil, err + } + cmd.SetArgs(argv) + + cmd.SetIn(&bytes.Buffer{}) + cmd.SetOut(ioutil.Discard) + cmd.SetErr(ioutil.Discard) + + _, err = cmd.ExecuteC() + return &test.CmdOut{ + OutBuf: stdout, + ErrBuf: stderr, + }, err +} + +func TestIssueStatus(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + + http.Register( + httpmock.GraphQL(`query UserCurrent\b`), + httpmock.StringResponse(`{"data":{"viewer":{"login":"octocat"}}}`)) + http.Register( + httpmock.GraphQL(`query IssueStatus\b`), + httpmock.FileResponse("./fixtures/issueStatus.json")) + + output, err := runCommand(http, true, "") + if err != nil { + t.Errorf("error running command `issue status`: %v", err) + } + + expectedIssues := []*regexp.Regexp{ + regexp.MustCompile(`(?m)8.*carrots.*about.*ago`), + regexp.MustCompile(`(?m)9.*squash.*about.*ago`), + regexp.MustCompile(`(?m)10.*broccoli.*about.*ago`), + regexp.MustCompile(`(?m)11.*swiss chard.*about.*ago`), + } + + for _, r := range expectedIssues { + if !r.MatchString(output.String()) { + t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output) + return + } + } +} + +func TestIssueStatus_blankSlate(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + + http.Register( + httpmock.GraphQL(`query UserCurrent\b`), + httpmock.StringResponse(`{"data":{"viewer":{"login":"octocat"}}}`)) + http.Register( + httpmock.GraphQL(`query IssueStatus\b`), + httpmock.StringResponse(` + { "data": { "repository": { + "hasIssuesEnabled": true, + "assigned": { "nodes": [] }, + "mentioned": { "nodes": [] }, + "authored": { "nodes": [] } + } } }`)) + + output, err := runCommand(http, true, "") + if err != nil { + t.Errorf("error running command `issue status`: %v", err) + } + + expectedOutput := ` +Relevant issues in OWNER/REPO + +Issues assigned to you + There are no issues assigned to you + +Issues mentioning you + There are no issues mentioning you + +Issues opened by you + There are no issues opened by you + +` + if output.String() != expectedOutput { + t.Errorf("expected %q, got %q", expectedOutput, output) + } +} + +func TestIssueStatus_disabledIssues(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + + http.Register( + httpmock.GraphQL(`query UserCurrent\b`), + httpmock.StringResponse(`{"data":{"viewer":{"login":"octocat"}}}`)) + http.Register( + httpmock.GraphQL(`query IssueStatus\b`), + httpmock.StringResponse(` + { "data": { "repository": { + "hasIssuesEnabled": false + } } }`)) + + _, err := runCommand(http, true, "") + if err == nil || err.Error() != "the 'OWNER/REPO' repository has disabled issues" { + t.Errorf("error running command `issue status`: %v", err) + } +} diff --git a/test/fixtures/issueView_preview.json b/pkg/cmd/issue/view/fixtures/issueView_preview.json similarity index 100% rename from test/fixtures/issueView_preview.json rename to pkg/cmd/issue/view/fixtures/issueView_preview.json diff --git a/test/fixtures/issueView_previewClosedState.json b/pkg/cmd/issue/view/fixtures/issueView_previewClosedState.json similarity index 100% rename from test/fixtures/issueView_previewClosedState.json rename to pkg/cmd/issue/view/fixtures/issueView_previewClosedState.json diff --git a/test/fixtures/issueView_previewWithEmptyBody.json b/pkg/cmd/issue/view/fixtures/issueView_previewWithEmptyBody.json similarity index 100% rename from test/fixtures/issueView_previewWithEmptyBody.json rename to pkg/cmd/issue/view/fixtures/issueView_previewWithEmptyBody.json diff --git a/test/fixtures/issueView_previewWithMetadata.json b/pkg/cmd/issue/view/fixtures/issueView_previewWithMetadata.json similarity index 100% rename from test/fixtures/issueView_previewWithMetadata.json rename to pkg/cmd/issue/view/fixtures/issueView_previewWithMetadata.json diff --git a/pkg/cmd/issue/view/view.go b/pkg/cmd/issue/view/view.go new file mode 100644 index 000000000..6d2b0eeaa --- /dev/null +++ b/pkg/cmd/issue/view/view.go @@ -0,0 +1,207 @@ +package view + +import ( + "fmt" + "io" + "net/http" + "strings" + "time" + + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/api" + "github.com/cli/cli/internal/config" + "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/pkg/cmd/issue/shared" + issueShared "github.com/cli/cli/pkg/cmd/issue/shared" + prShared "github.com/cli/cli/pkg/cmd/pr/shared" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/utils" + "github.com/spf13/cobra" +) + +type ViewOptions struct { + HttpClient func() (*http.Client, error) + Config func() (config.Config, error) + IO *iostreams.IOStreams + BaseRepo func() (ghrepo.Interface, error) + + SelectorArg string + WebMode bool +} + +func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Command { + opts := &ViewOptions{ + IO: f.IOStreams, + HttpClient: f.HttpClient, + Config: f.Config, + } + + cmd := &cobra.Command{ + Use: "view { | }", + Short: "View an issue", + Long: heredoc.Doc(` + Display the title, body, and other information about an issue. + + With '--web', open the issue in a web browser instead. + `), + Example: heredoc.Doc(` + `), + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + // support `-R, --repo` override + opts.BaseRepo = f.BaseRepo + + if len(args) > 0 { + opts.SelectorArg = args[0] + } + + if runF != nil { + return runF(opts) + } + return viewRun(opts) + }, + } + + cmd.Flags().BoolVarP(&opts.WebMode, "web", "w", false, "Open an issue in the browser") + + return cmd +} + +func viewRun(opts *ViewOptions) error { + httpClient, err := opts.HttpClient() + if err != nil { + return err + } + apiClient := api.NewClientFromHTTP(httpClient) + + issue, _, err := issueShared.IssueFromArg(apiClient, opts.BaseRepo, opts.SelectorArg) + if err != nil { + return err + } + + openURL := issue.URL + + if opts.WebMode { + fmt.Fprintf(opts.IO.ErrOut, "Opening %s in your browser.\n", openURL) + return utils.OpenInBrowser(openURL) + } + if opts.IO.IsStdoutTTY() { + return printHumanIssuePreview(opts.IO.Out, issue) + } + + return printRawIssuePreview(opts.IO.Out, issue) +} + +func printRawIssuePreview(out io.Writer, issue *api.Issue) error { + assignees := issueAssigneeList(*issue) + labels := shared.IssueLabelList(*issue) + projects := issueProjectList(*issue) + + // Print empty strings for empty values so the number of metadata lines is consistent when + // processing many issues with head and grep. + fmt.Fprintf(out, "title:\t%s\n", issue.Title) + fmt.Fprintf(out, "state:\t%s\n", issue.State) + fmt.Fprintf(out, "author:\t%s\n", issue.Author.Login) + fmt.Fprintf(out, "labels:\t%s\n", labels) + fmt.Fprintf(out, "comments:\t%d\n", issue.Comments.TotalCount) + fmt.Fprintf(out, "assignees:\t%s\n", assignees) + fmt.Fprintf(out, "projects:\t%s\n", projects) + fmt.Fprintf(out, "milestone:\t%s\n", issue.Milestone.Title) + + fmt.Fprintln(out, "--") + fmt.Fprintln(out, issue.Body) + return nil +} + +func printHumanIssuePreview(out io.Writer, issue *api.Issue) error { + now := time.Now() + ago := now.Sub(issue.CreatedAt) + + // Header (Title and State) + fmt.Fprintln(out, utils.Bold(issue.Title)) + fmt.Fprint(out, issueStateTitleWithColor(issue.State)) + fmt.Fprintln(out, utils.Gray(fmt.Sprintf( + " • %s opened %s • %s", + issue.Author.Login, + utils.FuzzyAgo(ago), + utils.Pluralize(issue.Comments.TotalCount, "comment"), + ))) + + // Metadata + fmt.Fprintln(out) + if assignees := issueAssigneeList(*issue); assignees != "" { + fmt.Fprint(out, utils.Bold("Assignees: ")) + fmt.Fprintln(out, assignees) + } + if labels := shared.IssueLabelList(*issue); labels != "" { + fmt.Fprint(out, utils.Bold("Labels: ")) + fmt.Fprintln(out, labels) + } + if projects := issueProjectList(*issue); projects != "" { + fmt.Fprint(out, utils.Bold("Projects: ")) + fmt.Fprintln(out, projects) + } + if issue.Milestone.Title != "" { + fmt.Fprint(out, utils.Bold("Milestone: ")) + fmt.Fprintln(out, issue.Milestone.Title) + } + + // Body + if issue.Body != "" { + fmt.Fprintln(out) + md, err := utils.RenderMarkdown(issue.Body) + if err != nil { + return err + } + fmt.Fprintln(out, md) + } + fmt.Fprintln(out) + + // Footer + fmt.Fprintf(out, utils.Gray("View this issue on GitHub: %s\n"), issue.URL) + return nil +} + +func issueStateTitleWithColor(state string) string { + colorFunc := prShared.ColorFuncForState(state) + return colorFunc(strings.Title(strings.ToLower(state))) +} + +func issueAssigneeList(issue api.Issue) string { + if len(issue.Assignees.Nodes) == 0 { + return "" + } + + AssigneeNames := make([]string, 0, len(issue.Assignees.Nodes)) + for _, assignee := range issue.Assignees.Nodes { + AssigneeNames = append(AssigneeNames, assignee.Login) + } + + list := strings.Join(AssigneeNames, ", ") + if issue.Assignees.TotalCount > len(issue.Assignees.Nodes) { + list += ", …" + } + return list +} + +func issueProjectList(issue api.Issue) string { + if len(issue.ProjectCards.Nodes) == 0 { + return "" + } + + projectNames := make([]string, 0, len(issue.ProjectCards.Nodes)) + for _, project := range issue.ProjectCards.Nodes { + colName := project.Column.Name + if colName == "" { + colName = "Awaiting triage" + } + projectNames = append(projectNames, fmt.Sprintf("%s (%s)", project.Project.Name, colName)) + } + + list := strings.Join(projectNames, ", ") + if issue.ProjectCards.TotalCount > len(issue.ProjectCards.Nodes) { + list += ", …" + } + return list +} diff --git a/pkg/cmd/issue/view/view_test.go b/pkg/cmd/issue/view/view_test.go new file mode 100644 index 000000000..e27505f92 --- /dev/null +++ b/pkg/cmd/issue/view/view_test.go @@ -0,0 +1,340 @@ +package view + +import ( + "bytes" + "io/ioutil" + "net/http" + "os/exec" + "reflect" + "testing" + + "github.com/cli/cli/internal/config" + "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/internal/run" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/httpmock" + "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/test" + "github.com/google/shlex" +) + +func eq(t *testing.T, got interface{}, expected interface{}) { + t.Helper() + if !reflect.DeepEqual(got, expected) { + t.Errorf("expected: %v, got: %v", expected, got) + } +} + +func runCommand(rt http.RoundTripper, isTTY bool, cli string) (*test.CmdOut, error) { + io, _, stdout, stderr := iostreams.Test() + io.SetStdoutTTY(isTTY) + io.SetStdinTTY(isTTY) + io.SetStderrTTY(isTTY) + + factory := &cmdutil.Factory{ + IOStreams: io, + HttpClient: func() (*http.Client, error) { + return &http.Client{Transport: rt}, nil + }, + Config: func() (config.Config, error) { + return config.NewBlankConfig(), nil + }, + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("OWNER", "REPO"), nil + }, + } + + cmd := NewCmdView(factory, nil) + + argv, err := shlex.Split(cli) + if err != nil { + return nil, err + } + cmd.SetArgs(argv) + + cmd.SetIn(&bytes.Buffer{}) + cmd.SetOut(ioutil.Discard) + cmd.SetErr(ioutil.Discard) + + _, err = cmd.ExecuteC() + return &test.CmdOut{ + OutBuf: stdout, + ErrBuf: stderr, + }, err +} + +func TestIssueView_web(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { "hasIssuesEnabled": true, "issue": { + "number": 123, + "url": "https://github.com/OWNER/REPO/issues/123" + } } } } + `)) + + 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, "-w 123") + if err != nil { + t.Errorf("error running command `issue view`: %v", err) + } + + eq(t, output.String(), "") + eq(t, output.Stderr(), "Opening https://github.com/OWNER/REPO/issues/123 in your browser.\n") + + if seenCmd == nil { + t.Fatal("expected a command to run") + } + url := seenCmd.Args[len(seenCmd.Args)-1] + eq(t, url, "https://github.com/OWNER/REPO/issues/123") +} + +func TestIssueView_web_numberArgWithHash(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { "hasIssuesEnabled": true, "issue": { + "number": 123, + "url": "https://github.com/OWNER/REPO/issues/123" + } } } } + `)) + + 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, "-w \"#123\"") + if err != nil { + t.Errorf("error running command `issue view`: %v", err) + } + + eq(t, output.String(), "") + eq(t, output.Stderr(), "Opening https://github.com/OWNER/REPO/issues/123 in your browser.\n") + + if seenCmd == nil { + t.Fatal("expected a command to run") + } + url := seenCmd.Args[len(seenCmd.Args)-1] + eq(t, url, "https://github.com/OWNER/REPO/issues/123") +} + +func TestIssueView_nontty_Preview(t *testing.T) { + tests := map[string]struct { + fixture string + expectedOutputs []string + }{ + "Open issue without metadata": { + fixture: "./fixtures/issueView_preview.json", + expectedOutputs: []string{ + `title:\tix of coins`, + `state:\tOPEN`, + `comments:\t9`, + `author:\tmarseilles`, + `assignees:`, + `\*\*bold story\*\*`, + }, + }, + "Open issue with metadata": { + fixture: "./fixtures/issueView_previewWithMetadata.json", + expectedOutputs: []string{ + `title:\tix of coins`, + `assignees:\tmarseilles, monaco`, + `author:\tmarseilles`, + `state:\tOPEN`, + `comments:\t9`, + `labels:\tone, two, three, four, five`, + `projects:\tProject 1 \(column A\), Project 2 \(column B\), Project 3 \(column C\), Project 4 \(Awaiting triage\)\n`, + `milestone:\tuluru\n`, + `\*\*bold story\*\*`, + }, + }, + "Open issue with empty body": { + fixture: "./fixtures/issueView_previewWithEmptyBody.json", + expectedOutputs: []string{ + `title:\tix of coins`, + `state:\tOPEN`, + `author:\tmarseilles`, + `labels:\ttarot`, + }, + }, + "Closed issue": { + fixture: "./fixtures/issueView_previewClosedState.json", + expectedOutputs: []string{ + `title:\tix of coins`, + `state:\tCLOSED`, + `\*\*bold story\*\*`, + `author:\tmarseilles`, + `labels:\ttarot`, + `\*\*bold story\*\*`, + }, + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + + http.Register(httpmock.GraphQL(`query IssueByNumber\b`), httpmock.FileResponse(tc.fixture)) + + output, err := runCommand(http, false, "123") + if err != nil { + t.Errorf("error running `issue view`: %v", err) + } + + eq(t, output.Stderr(), "") + + test.ExpectLines(t, output.String(), tc.expectedOutputs...) + }) + } +} + +func TestIssueView_tty_Preview(t *testing.T) { + tests := map[string]struct { + fixture string + expectedOutputs []string + }{ + "Open issue without metadata": { + fixture: "./fixtures/issueView_preview.json", + expectedOutputs: []string{ + `ix of coins`, + `Open.*marseilles opened about 292 years ago.*9 comments`, + `bold story`, + `View this issue on GitHub: https://github.com/OWNER/REPO/issues/123`, + }, + }, + "Open issue with metadata": { + fixture: "./fixtures/issueView_previewWithMetadata.json", + expectedOutputs: []string{ + `ix of coins`, + `Open.*marseilles opened about 292 years ago.*9 comments`, + `Assignees:.*marseilles, monaco\n`, + `Labels:.*one, two, three, four, five\n`, + `Projects:.*Project 1 \(column A\), Project 2 \(column B\), Project 3 \(column C\), Project 4 \(Awaiting triage\)\n`, + `Milestone:.*uluru\n`, + `bold story`, + `View this issue on GitHub: https://github.com/OWNER/REPO/issues/123`, + }, + }, + "Open issue with empty body": { + fixture: "./fixtures/issueView_previewWithEmptyBody.json", + expectedOutputs: []string{ + `ix of coins`, + `Open.*marseilles opened about 292 years ago.*9 comments`, + `View this issue on GitHub: https://github.com/OWNER/REPO/issues/123`, + }, + }, + "Closed issue": { + fixture: "./fixtures/issueView_previewClosedState.json", + expectedOutputs: []string{ + `ix of coins`, + `Closed.*marseilles opened about 292 years ago.*9 comments`, + `bold story`, + `View this issue on GitHub: https://github.com/OWNER/REPO/issues/123`, + }, + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + + http.Register(httpmock.GraphQL(`query IssueByNumber\b`), httpmock.FileResponse(tc.fixture)) + + output, err := runCommand(http, true, "123") + if err != nil { + t.Errorf("error running `issue view`: %v", err) + } + + eq(t, output.Stderr(), "") + + test.ExpectLines(t, output.String(), tc.expectedOutputs...) + }) + } +} + +func TestIssueView_web_notFound(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + + http.StubResponse(200, bytes.NewBufferString(` + { "errors": [ + { "message": "Could not resolve to an Issue with the number of 9999." } + ] } + `)) + + var seenCmd *exec.Cmd + restoreCmd := run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { + seenCmd = cmd + return &test.OutputStub{} + }) + defer restoreCmd() + + _, err := runCommand(http, true, "-w 9999") + if err == nil || err.Error() != "GraphQL error: Could not resolve to an Issue with the number of 9999." { + t.Errorf("error running command `issue view`: %v", err) + } + + if seenCmd != nil { + t.Fatal("did not expect any command to run") + } +} + +func TestIssueView_disabledIssues(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { + "id": "REPOID", + "hasIssuesEnabled": false + } } } + `)) + + _, err := runCommand(http, true, `6666`) + if err == nil || err.Error() != "the 'OWNER/REPO' repository has disabled issues" { + t.Errorf("error running command `issue view`: %v", err) + } +} + +func TestIssueView_web_urlArg(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { "hasIssuesEnabled": true, "issue": { + "number": 123, + "url": "https://github.com/OWNER/REPO/issues/123" + } } } } + `)) + + 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, "-w https://github.com/OWNER/REPO/issues/123") + if err != nil { + t.Errorf("error running command `issue view`: %v", err) + } + + eq(t, output.String(), "") + + if seenCmd == nil { + t.Fatal("expected a command to run") + } + url := seenCmd.Args[len(seenCmd.Args)-1] + eq(t, url, "https://github.com/OWNER/REPO/issues/123") +} diff --git a/pkg/cmd/pr/close/close.go b/pkg/cmd/pr/close/close.go index 2fba67e06..a7d175624 100644 --- a/pkg/cmd/pr/close/close.go +++ b/pkg/cmd/pr/close/close.go @@ -77,7 +77,7 @@ func closeRun(opts *CloseOptions) error { return fmt.Errorf("API call failed: %w", err) } - fmt.Fprintf(opts.IO.ErrOut, "%s Closed pull request #%d (%s)\n", utils.Green("✔"), pr.Number, pr.Title) + fmt.Fprintf(opts.IO.ErrOut, "%s Closed pull request #%d (%s)\n", utils.Red("✔"), pr.Number, pr.Title) return nil } diff --git a/pkg/cmd/pr/pr.go b/pkg/cmd/pr/pr.go index 599b01758..4a50f8356 100644 --- a/pkg/cmd/pr/pr.go +++ b/pkg/cmd/pr/pr.go @@ -1,6 +1,7 @@ package pr import ( + "github.com/MakeNowJust/heredoc" "github.com/cli/cli/internal/ghrepo" cmdCheckout "github.com/cli/cli/pkg/cmd/pr/checkout" cmdClose "github.com/cli/cli/pkg/cmd/pr/close" @@ -13,8 +14,6 @@ import ( cmdReview "github.com/cli/cli/pkg/cmd/pr/review" cmdStatus "github.com/cli/cli/pkg/cmd/pr/status" cmdView "github.com/cli/cli/pkg/cmd/pr/view" - - "github.com/MakeNowJust/heredoc" "github.com/cli/cli/pkg/cmdutil" "github.com/spf13/cobra" ) From 34cc84c1e09d89f870565adba007e1ed987c4b2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 7 Aug 2020 00:45:45 +0200 Subject: [PATCH 02/11] Provide consistent factory functions for top-level commands --- command/root.go | 24 ++++------------------ pkg/cmd/gist/gist.go | 19 ++++++++++++++++++ pkg/cmd/repo/repo.go | 48 ++++++++++++++++++++++++++++++-------------- 3 files changed, 56 insertions(+), 35 deletions(-) create mode 100644 pkg/cmd/gist/gist.go diff --git a/command/root.go b/command/root.go index 19206b648..5eb6e930f 100644 --- a/command/root.go +++ b/command/root.go @@ -22,15 +22,11 @@ import ( "github.com/cli/cli/internal/ghrepo" "github.com/cli/cli/internal/run" apiCmd "github.com/cli/cli/pkg/cmd/api" - gistCreateCmd "github.com/cli/cli/pkg/cmd/gist/create" + gistCmd "github.com/cli/cli/pkg/cmd/gist" issueCmd "github.com/cli/cli/pkg/cmd/issue" prCmd "github.com/cli/cli/pkg/cmd/pr" repoCmd "github.com/cli/cli/pkg/cmd/repo" - repoCloneCmd "github.com/cli/cli/pkg/cmd/repo/clone" - repoCreateCmd "github.com/cli/cli/pkg/cmd/repo/create" creditsCmd "github.com/cli/cli/pkg/cmd/repo/credits" - repoForkCmd "github.com/cli/cli/pkg/cmd/repo/fork" - repoViewCmd "github.com/cli/cli/pkg/cmd/repo/view" "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/iostreams" "github.com/cli/cli/utils" @@ -123,15 +119,9 @@ func init() { return currentBranch, nil }, } - RootCmd.AddCommand(apiCmd.NewCmdApi(cmdFactory, nil)) - gistCmd := &cobra.Command{ - Use: "gist", - Short: "Create gists", - Long: `Work with GitHub gists.`, - } - RootCmd.AddCommand(gistCmd) - gistCmd.AddCommand(gistCreateCmd.NewCmdCreate(cmdFactory, nil)) + RootCmd.AddCommand(apiCmd.NewCmdApi(cmdFactory, nil)) + RootCmd.AddCommand(gistCmd.NewCmdGist(cmdFactory)) resolvedBaseRepo := func() (ghrepo.Interface, error) { httpClient, err := cmdFactory.HttpClient() @@ -162,15 +152,9 @@ func init() { repoResolvingCmdFactory.BaseRepo = resolvedBaseRepo - RootCmd.AddCommand(repoCmd.Cmd) - repoCmd.Cmd.AddCommand(repoViewCmd.NewCmdView(&repoResolvingCmdFactory, nil)) - repoCmd.Cmd.AddCommand(repoForkCmd.NewCmdFork(&repoResolvingCmdFactory, nil)) - repoCmd.Cmd.AddCommand(repoCloneCmd.NewCmdClone(cmdFactory, nil)) - repoCmd.Cmd.AddCommand(repoCreateCmd.NewCmdCreate(cmdFactory, nil)) - repoCmd.Cmd.AddCommand(creditsCmd.NewCmdRepoCredits(&repoResolvingCmdFactory, nil)) - RootCmd.AddCommand(prCmd.NewCmdPR(&repoResolvingCmdFactory)) RootCmd.AddCommand(issueCmd.NewCmdIssue(&repoResolvingCmdFactory)) + RootCmd.AddCommand(repoCmd.NewCmdRepo(&repoResolvingCmdFactory)) RootCmd.AddCommand(creditsCmd.NewCmdCredits(cmdFactory, nil)) } diff --git a/pkg/cmd/gist/gist.go b/pkg/cmd/gist/gist.go new file mode 100644 index 000000000..ea10a8c40 --- /dev/null +++ b/pkg/cmd/gist/gist.go @@ -0,0 +1,19 @@ +package gist + +import ( + gistCreateCmd "github.com/cli/cli/pkg/cmd/gist/create" + "github.com/cli/cli/pkg/cmdutil" + "github.com/spf13/cobra" +) + +func NewCmdGist(f *cmdutil.Factory) *cobra.Command { + cmd := &cobra.Command{ + Use: "gist", + Short: "Create gists", + Long: `Work with GitHub gists.`, + } + + cmd.AddCommand(gistCreateCmd.NewCmdCreate(f, nil)) + + return cmd +} diff --git a/pkg/cmd/repo/repo.go b/pkg/cmd/repo/repo.go index 7de2e3ee7..551c96641 100644 --- a/pkg/cmd/repo/repo.go +++ b/pkg/cmd/repo/repo.go @@ -2,22 +2,40 @@ package repo import ( "github.com/MakeNowJust/heredoc" + repoCloneCmd "github.com/cli/cli/pkg/cmd/repo/clone" + repoCreateCmd "github.com/cli/cli/pkg/cmd/repo/create" + creditsCmd "github.com/cli/cli/pkg/cmd/repo/credits" + repoForkCmd "github.com/cli/cli/pkg/cmd/repo/fork" + repoViewCmd "github.com/cli/cli/pkg/cmd/repo/view" + "github.com/cli/cli/pkg/cmdutil" "github.com/spf13/cobra" ) -var Cmd = &cobra.Command{ - Use: "repo ", - Short: "Create, clone, fork, and view repositories", - Long: `Work with GitHub repositories`, - Example: heredoc.Doc(` - $ gh repo create - $ gh repo clone cli/cli - $ gh repo view --web - `), - Annotations: map[string]string{ - "IsCore": "true", - "help:arguments": ` -A repository can be supplied as an argument in any of the following formats: -- "OWNER/REPO" -- by URL, e.g. "https://github.com/OWNER/REPO"`}, +func NewCmdRepo(f *cmdutil.Factory) *cobra.Command { + cmd := &cobra.Command{ + Use: "repo ", + Short: "Create, clone, fork, and view repositories", + Long: `Work with GitHub repositories`, + Example: heredoc.Doc(` + $ gh repo create + $ gh repo clone cli/cli + $ gh repo view --web + `), + Annotations: map[string]string{ + "IsCore": "true", + "help:arguments": heredoc.Doc(` + A repository can be supplied as an argument in any of the following formats: + - "OWNER/REPO" + - by URL, e.g. "https://github.com/OWNER/REPO" + `), + }, + } + + cmd.AddCommand(repoViewCmd.NewCmdView(f, nil)) + cmd.AddCommand(repoForkCmd.NewCmdFork(f, nil)) + cmd.AddCommand(repoCloneCmd.NewCmdClone(f, nil)) + cmd.AddCommand(repoCreateCmd.NewCmdCreate(f, nil)) + cmd.AddCommand(creditsCmd.NewCmdRepoCredits(f, nil)) + + return cmd } From 5e24e0d9b9d7e8b76003b1d8b6f7ab4aea6ddf14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 7 Aug 2020 00:48:50 +0200 Subject: [PATCH 03/11] :fire: unused legacy helpers --- command/root.go | 39 --------------------------------------- command/testing.go | 10 ---------- 2 files changed, 49 deletions(-) diff --git a/command/root.go b/command/root.go index 5eb6e930f..ee57d3a2d 100644 --- a/command/root.go +++ b/command/root.go @@ -301,17 +301,6 @@ func httpClient(io *iostreams.IOStreams, cfg config.Config, setAccept bool) *htt return api.NewHTTPClient(opts...) } -// LEGACY; overridden in tests -var apiClientForContext = func(ctx context.Context) (*api.Client, error) { - cfg, err := ctx.Config() - if err != nil { - return nil, err - } - - http := httpClient(defaultStreams, cfg, true) - return api.NewClientFromHTTP(http), nil -} - func apiVerboseLog() api.ClientOption { logTraffic := strings.Contains(os.Getenv("DEBUG"), "api") colorize := utils.IsTerminal(os.Stderr) @@ -345,34 +334,6 @@ func changelogURL(version string) string { return url } -func determineBaseRepo(apiClient *api.Client, cmd *cobra.Command, ctx context.Context) (ghrepo.Interface, error) { - repo, _ := cmd.Flags().GetString("repo") - if repo != "" { - baseRepo, err := ghrepo.FromFullName(repo) - if err != nil { - return nil, fmt.Errorf("argument error: %w", err) - } - return baseRepo, nil - } - - remotes, err := ctx.Remotes() - if err != nil { - return nil, err - } - - repoContext, err := context.ResolveRemotesToRepos(remotes, apiClient, "") - if err != nil { - return nil, err - } - - baseRepo, err := repoContext.BaseRepo() - if err != nil { - return nil, err - } - - return baseRepo, nil -} - func ExecuteShellAlias(args []string) error { externalCmd := exec.Command(args[0], args[1:]...) externalCmd.Stderr = os.Stderr diff --git a/command/testing.go b/command/testing.go index fe9b76ee7..7fe86c255 100644 --- a/command/testing.go +++ b/command/testing.go @@ -6,10 +6,8 @@ import ( "reflect" "testing" - "github.com/cli/cli/api" "github.com/cli/cli/context" "github.com/cli/cli/internal/config" - "github.com/cli/cli/pkg/httpmock" "github.com/cli/cli/utils" "github.com/google/shlex" "github.com/spf13/pflag" @@ -49,14 +47,6 @@ func initBlankContext(cfg, repo, branch string) { } } -func initFakeHTTP() *httpmock.Registry { - http := &httpmock.Registry{} - apiClientForContext = func(context.Context) (*api.Client, error) { - return api.NewClient(api.ReplaceTripper(http)), nil - } - return http -} - type cmdOut struct { outBuf, errBuf *bytes.Buffer } From aef1a4ba4dc5c8d9436ee4998144c95f302f3f07 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 7 Aug 2020 14:40:33 +0200 Subject: [PATCH 04/11] Extract root command and factory logic into separate packages --- cmd/gh/main.go | 3 +- command/alias.go | 1 - command/completion.go | 1 - command/config.go | 1 - command/root.go | 243 +------------------------ context/context.go | 57 ------ context/remote.go | 2 +- context/remote_test.go | 2 +- pkg/cmd/factory/default.go | 105 +++++++++++ pkg/cmd/factory/http.go | 68 +++++++ {command => pkg/cmd/root}/help.go | 10 +- {command => pkg/cmd/root}/help_test.go | 2 +- pkg/cmd/root/root.go | 147 +++++++++++++++ {command => pkg/cmd/root}/root_test.go | 2 +- 14 files changed, 341 insertions(+), 303 deletions(-) create mode 100644 pkg/cmd/factory/default.go create mode 100644 pkg/cmd/factory/http.go rename {command => pkg/cmd/root}/help.go (96%) rename {command => pkg/cmd/root}/help_test.go (99%) create mode 100644 pkg/cmd/root/root.go rename {command => pkg/cmd/root}/root_test.go (98%) diff --git a/cmd/gh/main.go b/cmd/gh/main.go index 5336033fd..4d801f008 100644 --- a/cmd/gh/main.go +++ b/cmd/gh/main.go @@ -12,6 +12,7 @@ import ( "github.com/cli/cli/command" "github.com/cli/cli/internal/config" + "github.com/cli/cli/pkg/cmd/root" "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/update" "github.com/cli/cli/utils" @@ -73,7 +74,7 @@ func main() { printError(os.Stderr, err, cmd, hasDebug) os.Exit(1) } - if command.HasFailed() { + if root.HasFailed() { os.Exit(1) } diff --git a/command/alias.go b/command/alias.go index fc27c1cb0..64b6ab19e 100644 --- a/command/alias.go +++ b/command/alias.go @@ -13,7 +13,6 @@ import ( ) func init() { - RootCmd.AddCommand(aliasCmd) aliasCmd.AddCommand(aliasSetCmd) aliasCmd.AddCommand(aliasListCmd) aliasCmd.AddCommand(aliasDeleteCmd) diff --git a/command/completion.go b/command/completion.go index 706ef311a..099b10a47 100644 --- a/command/completion.go +++ b/command/completion.go @@ -10,7 +10,6 @@ import ( ) func init() { - RootCmd.AddCommand(completionCmd) completionCmd.Flags().StringP("shell", "s", "", "Shell type: {bash|zsh|fish|powershell}") } diff --git a/command/config.go b/command/config.go index 0a77e99e5..35fa09508 100644 --- a/command/config.go +++ b/command/config.go @@ -8,7 +8,6 @@ import ( ) func init() { - RootCmd.AddCommand(configCmd) configCmd.AddCommand(configGetCmd) configCmd.AddCommand(configSetCmd) diff --git a/command/root.go b/command/root.go index ee57d3a2d..a72b3eae4 100644 --- a/command/root.go +++ b/command/root.go @@ -4,7 +4,6 @@ import ( "errors" "fmt" "io" - "net/http" "os" "os/exec" "path/filepath" @@ -13,27 +12,17 @@ import ( "runtime/debug" "strings" - "github.com/MakeNowJust/heredoc" "github.com/cli/cli/api" "github.com/cli/cli/context" - "github.com/cli/cli/git" "github.com/cli/cli/internal/config" "github.com/cli/cli/internal/ghinstance" - "github.com/cli/cli/internal/ghrepo" "github.com/cli/cli/internal/run" - apiCmd "github.com/cli/cli/pkg/cmd/api" - gistCmd "github.com/cli/cli/pkg/cmd/gist" - issueCmd "github.com/cli/cli/pkg/cmd/issue" - prCmd "github.com/cli/cli/pkg/cmd/pr" - repoCmd "github.com/cli/cli/pkg/cmd/repo" - creditsCmd "github.com/cli/cli/pkg/cmd/repo/credits" - "github.com/cli/cli/pkg/cmdutil" - "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/pkg/cmd/factory" + "github.com/cli/cli/pkg/cmd/root" "github.com/cli/cli/utils" "github.com/google/shlex" "github.com/spf13/cobra" - "github.com/spf13/pflag" ) // Version is dynamically set by the toolchain or overridden by the Makefile. @@ -42,9 +31,7 @@ var Version = "DEV" // BuildDate is dynamically set at build time in the Makefile. var BuildDate = "" // YYYY-MM-DD -var versionOutput = "" - -var defaultStreams *iostreams.IOStreams +var RootCmd *cobra.Command func init() { if Version == "DEV" { @@ -52,159 +39,12 @@ func init() { Version = info.Main.Version } } - Version = strings.TrimPrefix(Version, "v") - if BuildDate == "" { - RootCmd.Version = Version - } else { - RootCmd.Version = fmt.Sprintf("%s (%s)", Version, BuildDate) - } - versionOutput = fmt.Sprintf("gh version %s\n%s\n", RootCmd.Version, changelogURL(Version)) - RootCmd.AddCommand(versionCmd) - RootCmd.SetVersionTemplate(versionOutput) - RootCmd.PersistentFlags().Bool("help", false, "Show help for command") - RootCmd.Flags().Bool("version", false, "Show gh version") - // TODO: - // RootCmd.PersistentFlags().BoolP("verbose", "V", false, "enable verbose output") - - RootCmd.SetHelpFunc(rootHelpFunc) - RootCmd.SetUsageFunc(rootUsageFunc) - - RootCmd.SetFlagErrorFunc(func(cmd *cobra.Command, err error) error { - if err == pflag.ErrHelp { - return err - } - return &cmdutil.FlagError{Err: err} - }) - - defaultStreams = iostreams.System() - - // TODO: iron out how a factory incorporates context - cmdFactory := &cmdutil.Factory{ - IOStreams: defaultStreams, - HttpClient: func() (*http.Client, error) { - // TODO: decouple from `context` - ctx := context.New() - cfg, err := ctx.Config() - if err != nil { - return nil, err - } - - // TODO: avoid setting Accept header for `api` command - return httpClient(defaultStreams, cfg, true), nil - }, - BaseRepo: func() (ghrepo.Interface, error) { - // TODO: decouple from `context` - ctx := context.New() - return ctx.BaseRepo() - }, - Remotes: func() (context.Remotes, error) { - ctx := context.New() - return ctx.Remotes() - }, - Config: func() (config.Config, error) { - cfg, err := config.ParseDefaultConfig() - if errors.Is(err, os.ErrNotExist) { - cfg = config.NewBlankConfig() - } else if err != nil { - return nil, err - } - return cfg, nil - }, - Branch: func() (string, error) { - currentBranch, err := git.CurrentBranch() - if err != nil { - return "", fmt.Errorf("could not determine current branch: %w", err) - } - return currentBranch, nil - }, - } - - RootCmd.AddCommand(apiCmd.NewCmdApi(cmdFactory, nil)) - RootCmd.AddCommand(gistCmd.NewCmdGist(cmdFactory)) - - resolvedBaseRepo := func() (ghrepo.Interface, error) { - httpClient, err := cmdFactory.HttpClient() - if err != nil { - return nil, err - } - - apiClient := api.NewClientFromHTTP(httpClient) - - ctx := context.New() - remotes, err := ctx.Remotes() - if err != nil { - return nil, err - } - repoContext, err := context.ResolveRemotesToRepos(remotes, apiClient, "") - if err != nil { - return nil, err - } - baseRepo, err := repoContext.BaseRepo() - if err != nil { - return nil, err - } - - return baseRepo, nil - } - - repoResolvingCmdFactory := *cmdFactory - - repoResolvingCmdFactory.BaseRepo = resolvedBaseRepo - - RootCmd.AddCommand(prCmd.NewCmdPR(&repoResolvingCmdFactory)) - RootCmd.AddCommand(issueCmd.NewCmdIssue(&repoResolvingCmdFactory)) - RootCmd.AddCommand(repoCmd.NewCmdRepo(&repoResolvingCmdFactory)) - RootCmd.AddCommand(creditsCmd.NewCmdCredits(cmdFactory, nil)) -} - -// RootCmd is the entry point of command-line execution -var RootCmd = &cobra.Command{ - Use: "gh [flags]", - Short: "GitHub CLI", - Long: `Work seamlessly with GitHub from the command line.`, - - SilenceErrors: true, - SilenceUsage: true, - Example: heredoc.Doc(` - $ gh issue create - $ gh repo clone cli/cli - $ gh pr checkout 321 - `), - Annotations: map[string]string{ - "help:feedback": heredoc.Doc(` - Fill out our feedback form https://forms.gle/umxd3h31c7aMQFKG7 - Open an issue using “gh issue create -R cli/cli” - `), - "help:environment": heredoc.Doc(` - GITHUB_TOKEN: an authentication token for API requests. Setting this avoids being - prompted to authenticate and overrides any previously stored credentials. - - GH_REPO: specify the GitHub repository in "OWNER/REPO" format for commands that - otherwise operate on a local repository. - - GH_EDITOR, GIT_EDITOR, VISUAL, EDITOR (in order of precedence): the editor tool to use - for authoring text. - - BROWSER: the web browser to use for opening links. - - DEBUG: set to any value to enable verbose output to standard error. Include values "api" - or "oauth" to print detailed information about HTTP requests or authentication flow. - - GLAMOUR_STYLE: the style to use for rendering Markdown. See - https://github.com/charmbracelet/glamour#styles - - NO_COLOR: avoid printing ANSI escape sequences for color output. - `), - }, -} - -var versionCmd = &cobra.Command{ - Use: "version", - Hidden: true, - Run: func(cmd *cobra.Command, args []string) { - fmt.Print(versionOutput) - }, + cmdFactory := factory.New(Version) + RootCmd = root.NewCmdRoot(cmdFactory, Version, BuildDate) + RootCmd.AddCommand(aliasCmd) + RootCmd.AddCommand(completionCmd) + RootCmd.AddCommand(configCmd) } // overridden in tests @@ -245,62 +85,6 @@ func contextForCommand(cmd *cobra.Command) context.Context { return ctx } -// generic authenticated HTTP client for commands -func httpClient(io *iostreams.IOStreams, cfg config.Config, setAccept bool) *http.Client { - var opts []api.ClientOption - if verbose := os.Getenv("DEBUG"); verbose != "" { - opts = append(opts, apiVerboseLog()) - } - - opts = append(opts, - api.AddHeader("User-Agent", fmt.Sprintf("GitHub CLI %s", Version)), - // antiope-preview: Checks - // FIXME: avoid setting this header for `api` command - api.AddHeader("Accept", "application/vnd.github.antiope-preview+json"), - api.AddHeaderFunc("Authorization", func(req *http.Request) (string, error) { - if token := os.Getenv("GITHUB_TOKEN"); token != "" { - return fmt.Sprintf("token %s", token), nil - } - - hostname := ghinstance.NormalizeHostname(req.URL.Hostname()) - token, err := cfg.Get(hostname, "oauth_token") - if token == "" { - var notFound *config.NotFoundError - // TODO: check if stdout is TTY too - if errors.As(err, ¬Found) && io.IsStdinTTY() { - // interactive OAuth flow - token, err = config.AuthFlowWithConfig(cfg, hostname, "Notice: authentication required") - } - if err != nil { - return "", err - } - if token == "" { - // TODO: instruct user how to manually authenticate - return "", fmt.Errorf("authentication required for %s", hostname) - } - } - - return fmt.Sprintf("token %s", token), nil - }), - ) - - if setAccept { - opts = append(opts, - api.AddHeaderFunc("Accept", func(req *http.Request) (string, error) { - // antiope-preview: Checks - accept := "application/vnd.github.antiope-preview+json" - if ghinstance.IsEnterprise(req.URL.Hostname()) { - // shadow-cat-preview: Draft pull requests - accept += ", application/vnd.github.shadow-cat-preview" - } - return accept, nil - }), - ) - } - - return api.NewHTTPClient(opts...) -} - func apiVerboseLog() api.ClientOption { logTraffic := strings.Contains(os.Getenv("DEBUG"), "api") colorize := utils.IsTerminal(os.Stderr) @@ -323,17 +107,6 @@ func colorableErr(cmd *cobra.Command) io.Writer { return err } -func changelogURL(version string) string { - path := "https://github.com/cli/cli" - r := regexp.MustCompile(`^v?\d+\.\d+\.\d+(-[\w.]+)?$`) - if !r.MatchString(version) { - return fmt.Sprintf("%s/releases/latest", path) - } - - url := fmt.Sprintf("%s/releases/tag/v%s", path, strings.TrimPrefix(version, "v")) - return url -} - func ExecuteShellAlias(args []string) error { externalCmd := exec.Command(args[0], args[1:]...) externalCmd.Stderr = os.Stderr diff --git a/context/context.go b/context/context.go index 6cff06b4c..21ebc9b1f 100644 --- a/context/context.go +++ b/context/context.go @@ -17,8 +17,6 @@ import ( type Context interface { Branch() (string, error) SetBranch(string) - Remotes() (Remotes, error) - BaseRepo() (ghrepo.Interface, error) SetBaseRepo(string) Config() (config.Config, error) } @@ -160,7 +158,6 @@ func New() Context { // A Context implementation that queries the filesystem type fsContext struct { config config.Config - remotes Remotes branch string baseRepo ghrepo.Interface } @@ -196,60 +193,6 @@ func (c *fsContext) SetBranch(b string) { c.branch = b } -func (c *fsContext) Remotes() (Remotes, error) { - if c.remotes == nil { - gitRemotes, err := git.Remotes() - if err != nil { - return nil, err - } - if len(gitRemotes) == 0 { - return nil, errors.New("no git remotes found") - } - - sshTranslate := git.ParseSSHConfig().Translator() - resolvedRemotes := translateRemotes(gitRemotes, sshTranslate) - - // determine hostname by looking at the "main" remote - var hostname string - if mainRemote, err := resolvedRemotes.FindByName("upstream", "github", "origin", "*"); err == nil { - hostname = mainRemote.RepoHost() - } - - // filter the rest of the remotes to just that hostname - filteredRemotes := Remotes{} - for _, r := range resolvedRemotes { - if r.RepoHost() != hostname { - continue - } - filteredRemotes = append(filteredRemotes, r) - } - c.remotes = filteredRemotes - } - - if len(c.remotes) == 0 { - return nil, errors.New("no git remote found for a github.com repository") - } - return c.remotes, nil -} - -func (c *fsContext) BaseRepo() (ghrepo.Interface, error) { - if c.baseRepo != nil { - return c.baseRepo, nil - } - - remotes, err := c.Remotes() - if err != nil { - return nil, err - } - rem, err := remotes.FindByName("upstream", "github", "origin", "*") - if err != nil { - return nil, err - } - - c.baseRepo = rem - return c.baseRepo, nil -} - func (c *fsContext) SetBaseRepo(nwo string) { c.baseRepo, _ = ghrepo.FromFullName(nwo) } diff --git a/context/remote.go b/context/remote.go index 79ba0e8c8..b88878483 100644 --- a/context/remote.go +++ b/context/remote.go @@ -76,7 +76,7 @@ func (r Remote) RepoHost() string { } // TODO: accept an interface instead of git.RemoteSet -func translateRemotes(gitRemotes git.RemoteSet, urlTranslate func(*url.URL) *url.URL) (remotes Remotes) { +func TranslateRemotes(gitRemotes git.RemoteSet, urlTranslate func(*url.URL) *url.URL) (remotes Remotes) { for _, r := range gitRemotes { var repo ghrepo.Interface if r.FetchURL != nil { diff --git a/context/remote_test.go b/context/remote_test.go index 6d5801e26..98326b3aa 100644 --- a/context/remote_test.go +++ b/context/remote_test.go @@ -57,7 +57,7 @@ func Test_translateRemotes(t *testing.T) { identityURL := func(u *url.URL) *url.URL { return u } - result := translateRemotes(gitRemotes, identityURL) + result := TranslateRemotes(gitRemotes, identityURL) if len(result) != 1 { t.Errorf("got %d results", len(result)) diff --git a/pkg/cmd/factory/default.go b/pkg/cmd/factory/default.go new file mode 100644 index 000000000..bea610d88 --- /dev/null +++ b/pkg/cmd/factory/default.go @@ -0,0 +1,105 @@ +package factory + +import ( + "errors" + "fmt" + "net/http" + "os" + + "github.com/cli/cli/context" + "github.com/cli/cli/git" + "github.com/cli/cli/internal/config" + "github.com/cli/cli/internal/ghrepo" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/iostreams" +) + +func New(appVersion string) *cmdutil.Factory { + io := iostreams.System() + + var cachedConfig config.Config + var configError error + configFunc := func() (config.Config, error) { + if cachedConfig != nil || configError != nil { + return cachedConfig, configError + } + cachedConfig, configError = config.ParseDefaultConfig() + if errors.Is(configError, os.ErrNotExist) { + cachedConfig = config.NewBlankConfig() + configError = nil + } + return cachedConfig, configError + } + + remotesFunc := remotesResolver() + + return &cmdutil.Factory{ + IOStreams: io, + Config: configFunc, + Remotes: remotesFunc, + HttpClient: func() (*http.Client, error) { + cfg, err := configFunc() + if err != nil { + return nil, err + } + + // TODO: avoid setting Accept header for `api` command + return httpClient(io, cfg, appVersion, true), nil + }, + BaseRepo: func() (ghrepo.Interface, error) { + remotes, err := remotesFunc() + if err != nil { + return nil, err + } + return remotes.FindByName("upstream", "github", "origin", "*") + }, + Branch: func() (string, error) { + currentBranch, err := git.CurrentBranch() + if err != nil { + return "", fmt.Errorf("could not determine current branch: %w", err) + } + return currentBranch, nil + }, + } +} + +// TODO: pass in a Config instance to parse remotes based on pre-authenticated hostnames +func remotesResolver() func() (context.Remotes, error) { + var cachedRemotes context.Remotes + var remotesError error + + return func() (context.Remotes, error) { + if cachedRemotes != nil || remotesError != nil { + return cachedRemotes, remotesError + } + + gitRemotes, err := git.Remotes() + if err != nil { + remotesError = err + return nil, err + } + if len(gitRemotes) == 0 { + remotesError = errors.New("no git remotes found") + return nil, remotesError + } + + sshTranslate := git.ParseSSHConfig().Translator() + resolvedRemotes := context.TranslateRemotes(gitRemotes, sshTranslate) + + // determine hostname by looking at the primary remotes + var hostname string + if mainRemote, err := resolvedRemotes.FindByName("upstream", "github", "origin", "*"); err == nil { + hostname = mainRemote.RepoHost() + } + + // filter the rest of the remotes to just that hostname + cachedRemotes = context.Remotes{} + for _, r := range resolvedRemotes { + if r.RepoHost() != hostname { + continue + } + cachedRemotes = append(cachedRemotes, r) + } + return cachedRemotes, nil + } +} diff --git a/pkg/cmd/factory/http.go b/pkg/cmd/factory/http.go new file mode 100644 index 000000000..8a6dab198 --- /dev/null +++ b/pkg/cmd/factory/http.go @@ -0,0 +1,68 @@ +package factory + +import ( + "errors" + "fmt" + "net/http" + "os" + "strings" + + "github.com/cli/cli/api" + "github.com/cli/cli/internal/config" + "github.com/cli/cli/internal/ghinstance" + "github.com/cli/cli/pkg/iostreams" +) + +// generic authenticated HTTP client for commands +func httpClient(io *iostreams.IOStreams, cfg config.Config, appVersion string, setAccept bool) *http.Client { + var opts []api.ClientOption + if verbose := os.Getenv("DEBUG"); verbose != "" { + logTraffic := strings.Contains(verbose, "api") + opts = append(opts, api.VerboseLog(io.ErrOut, logTraffic, io.IsStderrTTY())) + } + + opts = append(opts, + api.AddHeader("User-Agent", fmt.Sprintf("GitHub CLI %s", appVersion)), + api.AddHeaderFunc("Authorization", func(req *http.Request) (string, error) { + if token := os.Getenv("GITHUB_TOKEN"); token != "" { + return fmt.Sprintf("token %s", token), nil + } + + hostname := ghinstance.NormalizeHostname(req.URL.Hostname()) + token, err := cfg.Get(hostname, "oauth_token") + if token == "" { + var notFound *config.NotFoundError + // TODO: check if stdout is TTY too + if errors.As(err, ¬Found) && io.IsStdinTTY() { + // interactive OAuth flow + token, err = config.AuthFlowWithConfig(cfg, hostname, "Notice: authentication required") + } + if err != nil { + return "", err + } + if token == "" { + // TODO: instruct user how to manually authenticate + return "", fmt.Errorf("authentication required for %s", hostname) + } + } + + return fmt.Sprintf("token %s", token), nil + }), + ) + + if setAccept { + opts = append(opts, + api.AddHeaderFunc("Accept", func(req *http.Request) (string, error) { + // antiope-preview: Checks + accept := "application/vnd.github.antiope-preview+json" + if ghinstance.IsEnterprise(req.URL.Hostname()) { + // shadow-cat-preview: Draft pull requests + accept += ", application/vnd.github.shadow-cat-preview" + } + return accept, nil + }), + ) + } + + return api.NewHTTPClient(opts...) +} diff --git a/command/help.go b/pkg/cmd/root/help.go similarity index 96% rename from command/help.go rename to pkg/cmd/root/help.go index d35c2602b..5570fbaaa 100644 --- a/command/help.go +++ b/pkg/cmd/root/help.go @@ -1,4 +1,4 @@ -package command +package root import ( "bytes" @@ -67,8 +67,12 @@ func nestedSuggestFunc(command *cobra.Command, arg string) { _ = rootUsageFunc(command) } +func isRootCmd(command *cobra.Command) bool { + return command != nil && !command.HasParent() +} + func rootHelpFunc(command *cobra.Command, args []string) { - if command.Parent() == RootCmd && len(args) >= 2 && args[1] != "--help" && args[1] != "-h" { + if isRootCmd(command.Parent()) && len(args) >= 2 && args[1] != "--help" && args[1] != "-h" { nestedSuggestFunc(command, args[1]) hasFailed = true return @@ -141,7 +145,7 @@ Read the manual at https://cli.github.com/manual`}) helpEntries = append(helpEntries, helpEntry{"FEEDBACK", command.Annotations["help:feedback"]}) } - out := colorableOut(command) + out := command.OutOrStdout() for _, e := range helpEntries { if e.Title != "" { // If there is a title, add indentation to each line in the body diff --git a/command/help_test.go b/pkg/cmd/root/help_test.go similarity index 99% rename from command/help_test.go rename to pkg/cmd/root/help_test.go index e07542928..cbda48fe8 100644 --- a/command/help_test.go +++ b/pkg/cmd/root/help_test.go @@ -1,4 +1,4 @@ -package command +package root import ( "testing" diff --git a/pkg/cmd/root/root.go b/pkg/cmd/root/root.go new file mode 100644 index 000000000..15140800b --- /dev/null +++ b/pkg/cmd/root/root.go @@ -0,0 +1,147 @@ +package root + +import ( + "fmt" + "regexp" + "strings" + + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/api" + "github.com/cli/cli/context" + "github.com/cli/cli/internal/ghrepo" + apiCmd "github.com/cli/cli/pkg/cmd/api" + gistCmd "github.com/cli/cli/pkg/cmd/gist" + issueCmd "github.com/cli/cli/pkg/cmd/issue" + prCmd "github.com/cli/cli/pkg/cmd/pr" + repoCmd "github.com/cli/cli/pkg/cmd/repo" + creditsCmd "github.com/cli/cli/pkg/cmd/repo/credits" + "github.com/cli/cli/pkg/cmdutil" + "github.com/spf13/cobra" + "github.com/spf13/pflag" +) + +func NewCmdRoot(f *cmdutil.Factory, version, buildDate string) *cobra.Command { + cmd := &cobra.Command{ + Use: "gh [flags]", + Short: "GitHub CLI", + Long: `Work seamlessly with GitHub from the command line.`, + + SilenceErrors: true, + SilenceUsage: true, + Example: heredoc.Doc(` + $ gh issue create + $ gh repo clone cli/cli + $ gh pr checkout 321 + `), + Annotations: map[string]string{ + "help:feedback": heredoc.Doc(` + Fill out our feedback form https://forms.gle/umxd3h31c7aMQFKG7 + Open an issue using “gh issue create -R cli/cli” + `), + "help:environment": heredoc.Doc(` + GITHUB_TOKEN: an authentication token for API requests. Setting this avoids being + prompted to authenticate and overrides any previously stored credentials. + + GH_REPO: specify the GitHub repository in "OWNER/REPO" format for commands that + otherwise operate on a local repository. + + GH_EDITOR, GIT_EDITOR, VISUAL, EDITOR (in order of precedence): the editor tool to use + for authoring text. + + BROWSER: the web browser to use for opening links. + + DEBUG: set to any value to enable verbose output to standard error. Include values "api" + or "oauth" to print detailed information about HTTP requests or authentication flow. + + GLAMOUR_STYLE: the style to use for rendering Markdown. See + https://github.com/charmbracelet/glamour#styles + + NO_COLOR: avoid printing ANSI escape sequences for color output. + `), + }, + } + + version = strings.TrimPrefix(version, "v") + if buildDate == "" { + cmd.Version = version + } else { + cmd.Version = fmt.Sprintf("%s (%s)", version, buildDate) + } + versionOutput := fmt.Sprintf("gh version %s\n%s\n", cmd.Version, changelogURL(version)) + cmd.AddCommand(&cobra.Command{ + Use: "version", + Hidden: true, + Run: func(cmd *cobra.Command, args []string) { + fmt.Print(versionOutput) + }, + }) + cmd.SetVersionTemplate(versionOutput) + cmd.Flags().Bool("version", false, "Show gh version") + + cmd.SetOut(f.IOStreams.Out) + cmd.SetErr(f.IOStreams.ErrOut) + + cmd.PersistentFlags().Bool("help", false, "Show help for command") + cmd.SetHelpFunc(rootHelpFunc) + cmd.SetUsageFunc(rootUsageFunc) + + cmd.SetFlagErrorFunc(func(cmd *cobra.Command, err error) error { + if err == pflag.ErrHelp { + return err + } + return &cmdutil.FlagError{Err: err} + }) + + // CHILD COMMANDS + + cmd.AddCommand(apiCmd.NewCmdApi(f, nil)) + cmd.AddCommand(gistCmd.NewCmdGist(f)) + cmd.AddCommand(creditsCmd.NewCmdCredits(f, nil)) + + // below here at the commands that require the "intelligent" BaseRepo resolver + repoResolvingCmdFactory := *f + repoResolvingCmdFactory.BaseRepo = resolvedBaseRepo(f) + + cmd.AddCommand(prCmd.NewCmdPR(&repoResolvingCmdFactory)) + cmd.AddCommand(issueCmd.NewCmdIssue(&repoResolvingCmdFactory)) + cmd.AddCommand(repoCmd.NewCmdRepo(&repoResolvingCmdFactory)) + + return cmd +} + +func resolvedBaseRepo(f *cmdutil.Factory) func() (ghrepo.Interface, error) { + return func() (ghrepo.Interface, error) { + httpClient, err := f.HttpClient() + if err != nil { + return nil, err + } + + apiClient := api.NewClientFromHTTP(httpClient) + + remotes, err := f.Remotes() + if err != nil { + return nil, err + } + repoContext, err := context.ResolveRemotesToRepos(remotes, apiClient, "") + if err != nil { + return nil, err + } + baseRepo, err := repoContext.BaseRepo() + if err != nil { + return nil, err + } + + return baseRepo, nil + } +} + +func changelogURL(version string) string { + path := "https://github.com/cli/cli" + r := regexp.MustCompile(`^v?\d+\.\d+\.\d+(-[\w.]+)?$`) + if !r.MatchString(version) { + return fmt.Sprintf("%s/releases/latest", path) + } + + url := fmt.Sprintf("%s/releases/tag/v%s", path, strings.TrimPrefix(version, "v")) + return url +} diff --git a/command/root_test.go b/pkg/cmd/root/root_test.go similarity index 98% rename from command/root_test.go rename to pkg/cmd/root/root_test.go index c0b1fd7d3..714032ec3 100644 --- a/command/root_test.go +++ b/pkg/cmd/root/root_test.go @@ -1,4 +1,4 @@ -package command +package root import ( "testing" From 47cef736f4acaf46c4af65529e8120297e642e94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 7 Aug 2020 14:47:58 +0200 Subject: [PATCH 05/11] Fix GH_REPO override --- command/root.go | 12 ++---------- context/context.go | 3 --- pkg/cmd/issue/issue.go | 11 +---------- pkg/cmd/pr/pr.go | 11 +---------- pkg/cmdutil/repo_override.go | 25 +++++++++++++++++++++++++ 5 files changed, 29 insertions(+), 33 deletions(-) create mode 100644 pkg/cmdutil/repo_override.go diff --git a/command/root.go b/command/root.go index a72b3eae4..38ed2e99e 100644 --- a/command/root.go +++ b/command/root.go @@ -49,11 +49,7 @@ func init() { // overridden in tests var initContext = func() context.Context { - ctx := context.New() - if repo := os.Getenv("GH_REPO"); repo != "" { - ctx.SetBaseRepo(repo) - } - return ctx + return context.New() } // BasicClient returns an API client for github.com only that borrows from but @@ -78,11 +74,7 @@ func BasicClient() (*api.Client, error) { } func contextForCommand(cmd *cobra.Command) context.Context { - ctx := initContext() - if repo, err := cmd.Flags().GetString("repo"); err == nil && repo != "" { - ctx.SetBaseRepo(repo) - } - return ctx + return initContext() } func apiVerboseLog() api.ClientOption { diff --git a/context/context.go b/context/context.go index 21ebc9b1f..06a89067d 100644 --- a/context/context.go +++ b/context/context.go @@ -15,9 +15,6 @@ import ( // Context represents the interface for querying information about the current environment type Context interface { - Branch() (string, error) - SetBranch(string) - SetBaseRepo(string) Config() (config.Config, error) } diff --git a/pkg/cmd/issue/issue.go b/pkg/cmd/issue/issue.go index 91ed04b22..6f2cea6d9 100644 --- a/pkg/cmd/issue/issue.go +++ b/pkg/cmd/issue/issue.go @@ -2,7 +2,6 @@ package issue import ( "github.com/MakeNowJust/heredoc" - "github.com/cli/cli/internal/ghrepo" cmdClose "github.com/cli/cli/pkg/cmd/issue/close" cmdCreate "github.com/cli/cli/pkg/cmd/issue/create" cmdList "github.com/cli/cli/pkg/cmd/issue/list" @@ -31,17 +30,9 @@ func NewCmdIssue(f *cmdutil.Factory) *cobra.Command { - by URL, e.g. "https://github.com/OWNER/REPO/issues/123". `), }, - PersistentPreRun: func(cmd *cobra.Command, args []string) { - if repo, _ := cmd.Flags().GetString("repo"); repo != "" { - // NOTE: this mutates the factory - f.BaseRepo = func() (ghrepo.Interface, error) { - return ghrepo.FromFullName(repo) - } - } - }, } - cmd.PersistentFlags().StringP("repo", "R", "", "Select another repository using the `OWNER/REPO` format") + cmdutil.EnableRepoOverride(cmd, f) cmd.AddCommand(cmdClose.NewCmdClose(f, nil)) cmd.AddCommand(cmdCreate.NewCmdCreate(f, nil)) diff --git a/pkg/cmd/pr/pr.go b/pkg/cmd/pr/pr.go index 4a50f8356..a4b746656 100644 --- a/pkg/cmd/pr/pr.go +++ b/pkg/cmd/pr/pr.go @@ -2,7 +2,6 @@ package pr import ( "github.com/MakeNowJust/heredoc" - "github.com/cli/cli/internal/ghrepo" cmdCheckout "github.com/cli/cli/pkg/cmd/pr/checkout" cmdClose "github.com/cli/cli/pkg/cmd/pr/close" cmdCreate "github.com/cli/cli/pkg/cmd/pr/create" @@ -37,17 +36,9 @@ func NewCmdPR(f *cmdutil.Factory) *cobra.Command { - by the name of its head branch, e.g. "patch-1" or "OWNER:patch-1". `), }, - PersistentPreRun: func(cmd *cobra.Command, args []string) { - if repo, _ := cmd.Flags().GetString("repo"); repo != "" { - // NOTE: this mutates the factory - f.BaseRepo = func() (ghrepo.Interface, error) { - return ghrepo.FromFullName(repo) - } - } - }, } - cmd.PersistentFlags().StringP("repo", "R", "", "Select another repository using the `OWNER/REPO` format") + cmdutil.EnableRepoOverride(cmd, f) cmd.AddCommand(cmdCheckout.NewCmdCheckout(f, nil)) cmd.AddCommand(cmdClose.NewCmdClose(f, nil)) diff --git a/pkg/cmdutil/repo_override.go b/pkg/cmdutil/repo_override.go new file mode 100644 index 000000000..aede57ea6 --- /dev/null +++ b/pkg/cmdutil/repo_override.go @@ -0,0 +1,25 @@ +package cmdutil + +import ( + "os" + + "github.com/cli/cli/internal/ghrepo" + "github.com/spf13/cobra" +) + +func EnableRepoOverride(cmd *cobra.Command, f *Factory) { + cmd.PersistentFlags().StringP("repo", "R", "", "Select another repository using the `OWNER/REPO` format") + + cmd.PersistentPreRun = func(cmd *cobra.Command, args []string) { + repoOverride, _ := cmd.Flags().GetString("repo") + if repoFromEnv := os.Getenv("GH_REPO"); repoOverride == "" && repoFromEnv != "" { + repoOverride = repoFromEnv + } + if repoOverride != "" { + // NOTE: this mutates the factory + f.BaseRepo = func() (ghrepo.Interface, error) { + return ghrepo.FromFullName(repoOverride) + } + } + } +} From dbbf76df109aa8df2cc5486d3e39d6e815690c2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 7 Aug 2020 14:51:48 +0200 Subject: [PATCH 06/11] Cleanup in `context` --- command/testing.go | 5 ---- context/blank_context.go | 54 ---------------------------------------- context/context.go | 27 +------------------- 3 files changed, 1 insertion(+), 85 deletions(-) diff --git a/command/testing.go b/command/testing.go index 7fe86c255..0668776c8 100644 --- a/command/testing.go +++ b/command/testing.go @@ -29,11 +29,6 @@ const defaultTestConfig = `hosts: func initBlankContext(cfg, repo, branch string) { initContext = func() context.Context { ctx := context.NewBlank() - ctx.SetBaseRepo(repo) - ctx.SetBranch(branch) - ctx.SetRemotes(map[string]string{ - "origin": "OWNER/REPO", - }) if cfg == "" { cfg = defaultTestConfig diff --git a/context/blank_context.go b/context/blank_context.go index f14310937..96f599981 100644 --- a/context/blank_context.go +++ b/context/blank_context.go @@ -2,11 +2,8 @@ package context import ( "fmt" - "strings" - "github.com/cli/cli/git" "github.com/cli/cli/internal/config" - "github.com/cli/cli/internal/ghrepo" ) // NewBlank initializes a blank Context suitable for testing @@ -16,9 +13,6 @@ func NewBlank() *blankContext { // A Context implementation that queries the filesystem type blankContext struct { - branch string - baseRepo ghrepo.Interface - remotes Remotes } func (c *blankContext) Config() (config.Config, error) { @@ -28,51 +22,3 @@ func (c *blankContext) Config() (config.Config, error) { } return cfg, nil } - -func (c *blankContext) Branch() (string, error) { - if c.branch == "" { - return "", fmt.Errorf("branch was not initialized: %w", git.ErrNotOnAnyBranch) - } - return c.branch, nil -} - -func (c *blankContext) SetBranch(b string) { - c.branch = b -} - -func (c *blankContext) Remotes() (Remotes, error) { - if c.remotes == nil { - return nil, fmt.Errorf("remotes were not initialized") - } - return c.remotes, nil -} - -func (c *blankContext) SetRemotes(stubs map[string]string) { - c.remotes = make([]*Remote, 0, len(stubs)) - for remoteName, repo := range stubs { - ownerWithName := strings.SplitN(repo, "/", 2) - c.remotes = append(c.remotes, &Remote{ - Remote: &git.Remote{Name: remoteName}, - Repo: ghrepo.New(ownerWithName[0], ownerWithName[1]), - }) - } -} - -func (c *blankContext) BaseRepo() (ghrepo.Interface, error) { - if c.baseRepo != nil { - return c.baseRepo, nil - } - remotes, err := c.Remotes() - if err != nil { - return nil, err - } - if len(remotes) < 1 { - return nil, fmt.Errorf("remotes are empty") - } - return remotes[0], nil -} - -func (c *blankContext) SetBaseRepo(nwo string) { - repo, _ := ghrepo.FromFullName(nwo) - c.baseRepo = repo -} diff --git a/context/context.go b/context/context.go index 06a89067d..e2600b02e 100644 --- a/context/context.go +++ b/context/context.go @@ -8,7 +8,6 @@ import ( "strings" "github.com/cli/cli/api" - "github.com/cli/cli/git" "github.com/cli/cli/internal/config" "github.com/cli/cli/internal/ghrepo" ) @@ -154,9 +153,7 @@ func New() Context { // A Context implementation that queries the filesystem type fsContext struct { - config config.Config - branch string - baseRepo ghrepo.Interface + config config.Config } func (c *fsContext) Config() (config.Config, error) { @@ -171,25 +168,3 @@ func (c *fsContext) Config() (config.Config, error) { } return c.config, nil } - -func (c *fsContext) Branch() (string, error) { - if c.branch != "" { - return c.branch, nil - } - - currentBranch, err := git.CurrentBranch() - if err != nil { - return "", fmt.Errorf("could not determine current branch: %w", err) - } - - c.branch = currentBranch - return c.branch, nil -} - -func (c *fsContext) SetBranch(b string) { - c.branch = b -} - -func (c *fsContext) SetBaseRepo(nwo string) { - c.baseRepo, _ = ghrepo.FromFullName(nwo) -} From d933cd91d446b3a266e4f8404ad238529256921b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 7 Aug 2020 15:20:01 +0200 Subject: [PATCH 07/11] Migrate `completion` command --- command/completion.go | 64 -------------------------- command/completion_test.go | 57 ------------------------ command/root.go | 2 +- pkg/cmd/root/completion.go | 62 ++++++++++++++++++++++++++ pkg/cmd/root/completion_test.go | 79 +++++++++++++++++++++++++++++++++ 5 files changed, 142 insertions(+), 122 deletions(-) delete mode 100644 command/completion.go delete mode 100644 command/completion_test.go create mode 100644 pkg/cmd/root/completion.go create mode 100644 pkg/cmd/root/completion_test.go diff --git a/command/completion.go b/command/completion.go deleted file mode 100644 index 099b10a47..000000000 --- a/command/completion.go +++ /dev/null @@ -1,64 +0,0 @@ -package command - -import ( - "errors" - "fmt" - "os" - - "github.com/cli/cli/utils" - "github.com/spf13/cobra" -) - -func init() { - completionCmd.Flags().StringP("shell", "s", "", "Shell type: {bash|zsh|fish|powershell}") -} - -var completionCmd = &cobra.Command{ - Use: "completion", - Short: "Generate shell completion scripts", - Long: `Generate shell completion scripts for GitHub CLI commands. - -The output of this command will be computer code and is meant to be saved to a -file or immediately evaluated by an interactive shell. - -For example, for bash you could add this to your '~/.bash_profile': - - eval "$(gh completion -s bash)" - -When installing GitHub CLI through a package manager, however, it's possible that -no additional shell configuration is necessary to gain completion support. For -Homebrew, see https://docs.brew.sh/Shell-Completion -`, - RunE: func(cmd *cobra.Command, args []string) error { - shellType, err := cmd.Flags().GetString("shell") - if err != nil { - return err - } - - if shellType == "" { - out := cmd.OutOrStdout() - isTTY := false - if outFile, isFile := out.(*os.File); isFile { - isTTY = utils.IsTerminal(outFile) - } - - if isTTY { - return errors.New("error: the value for `--shell` is required\nsee `gh help completion` for more information") - } - shellType = "bash" - } - - switch shellType { - case "bash": - return RootCmd.GenBashCompletion(cmd.OutOrStdout()) - case "zsh": - return RootCmd.GenZshCompletion(cmd.OutOrStdout()) - case "powershell": - return RootCmd.GenPowerShellCompletion(cmd.OutOrStdout()) - case "fish": - return RootCmd.GenFishCompletion(cmd.OutOrStdout(), true) - default: - return fmt.Errorf("unsupported shell type %q", shellType) - } - }, -} diff --git a/command/completion_test.go b/command/completion_test.go deleted file mode 100644 index 029330a16..000000000 --- a/command/completion_test.go +++ /dev/null @@ -1,57 +0,0 @@ -package command - -import ( - "strings" - "testing" -) - -func TestCompletion_bash(t *testing.T) { - output, err := RunCommand(`completion`) - if err != nil { - t.Fatal(err) - } - - if !strings.Contains(output.String(), "complete -o default -F __start_gh gh") { - t.Errorf("problem in bash completion:\n%s", output) - } -} - -func TestCompletion_zsh(t *testing.T) { - output, err := RunCommand(`completion -s zsh`) - if err != nil { - t.Fatal(err) - } - - if !strings.Contains(output.String(), "#compdef _gh gh") { - t.Errorf("problem in zsh completion:\n%s", output) - } -} - -func TestCompletion_fish(t *testing.T) { - output, err := RunCommand(`completion -s fish`) - if err != nil { - t.Fatal(err) - } - - if !strings.Contains(output.String(), "complete -c gh ") { - t.Errorf("problem in fish completion:\n%s", output) - } -} - -func TestCompletion_powerShell(t *testing.T) { - output, err := RunCommand(`completion -s powershell`) - if err != nil { - t.Fatal(err) - } - - if !strings.Contains(output.String(), "Register-ArgumentCompleter") { - t.Errorf("problem in powershell completion:\n%s", output) - } -} - -func TestCompletion_unsupported(t *testing.T) { - _, err := RunCommand(`completion -s csh`) - if err == nil || err.Error() != `unsupported shell type "csh"` { - t.Fatal(err) - } -} diff --git a/command/root.go b/command/root.go index 38ed2e99e..fb31e886c 100644 --- a/command/root.go +++ b/command/root.go @@ -43,7 +43,7 @@ func init() { cmdFactory := factory.New(Version) RootCmd = root.NewCmdRoot(cmdFactory, Version, BuildDate) RootCmd.AddCommand(aliasCmd) - RootCmd.AddCommand(completionCmd) + RootCmd.AddCommand(root.NewCmdCompletion(cmdFactory.IOStreams)) RootCmd.AddCommand(configCmd) } diff --git a/pkg/cmd/root/completion.go b/pkg/cmd/root/completion.go new file mode 100644 index 000000000..ed907f6db --- /dev/null +++ b/pkg/cmd/root/completion.go @@ -0,0 +1,62 @@ +package root + +import ( + "errors" + "fmt" + + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/iostreams" + "github.com/spf13/cobra" +) + +func NewCmdCompletion(io *iostreams.IOStreams) *cobra.Command { + var shellType string + + cmd := &cobra.Command{ + Use: "completion", + Short: "Generate shell completion scripts", + Long: heredoc.Doc(` + Generate shell completion scripts for GitHub CLI commands. + + The output of this command will be computer code and is meant to be saved to a + file or immediately evaluated by an interactive shell. + + For example, for bash you could add this to your '~/.bash_profile': + + eval "$(gh completion -s bash)" + + When installing GitHub CLI through a package manager, however, it's possible that + no additional shell configuration is necessary to gain completion support. For + Homebrew, see https://docs.brew.sh/Shell-Completion + `), + RunE: func(cmd *cobra.Command, args []string) error { + if shellType == "" { + if io.IsStdoutTTY() { + return &cmdutil.FlagError{Err: errors.New("error: the value for `--shell` is required")} + } + shellType = "bash" + } + + w := io.Out + rootCmd := cmd.Parent() + + switch shellType { + case "bash": + return rootCmd.GenBashCompletion(w) + case "zsh": + return rootCmd.GenZshCompletion(w) + case "powershell": + return rootCmd.GenPowerShellCompletion(w) + case "fish": + return rootCmd.GenFishCompletion(w, true) + default: + return fmt.Errorf("unsupported shell type %q", shellType) + } + }, + } + + cmd.Flags().StringVarP(&shellType, "shell", "s", "", "Shell type: {bash|zsh|fish|powershell}") + + return cmd +} diff --git a/pkg/cmd/root/completion_test.go b/pkg/cmd/root/completion_test.go new file mode 100644 index 000000000..505a65319 --- /dev/null +++ b/pkg/cmd/root/completion_test.go @@ -0,0 +1,79 @@ +package root + +import ( + "strings" + "testing" + + "github.com/cli/cli/pkg/iostreams" + "github.com/google/shlex" + "github.com/spf13/cobra" +) + +func TestNewCmdCompletion(t *testing.T) { + tests := []struct { + name string + args string + wantOut string + wantErr string + }{ + { + name: "no arguments", + args: "completion", + wantOut: "complete -o default -F __start_gh gh", + }, + { + name: "zsh completion", + args: "completion -s zsh", + wantOut: "#compdef _gh gh", + }, + { + name: "fish completion", + args: "completion -s fish", + wantOut: "complete -c gh ", + }, + { + name: "PowerShell completion", + args: "completion -s powershell", + wantOut: "Register-ArgumentCompleter", + }, + { + name: "unsupported shell", + args: "completion -s csh", + wantErr: "unsupported shell type \"csh\"", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + io, _, stdout, stderr := iostreams.Test() + completeCmd := NewCmdCompletion(io) + rootCmd := &cobra.Command{Use: "gh"} + rootCmd.AddCommand(completeCmd) + + argv, err := shlex.Split(tt.args) + if err != nil { + t.Fatalf("argument splitting error: %v", err) + } + rootCmd.SetArgs(argv) + rootCmd.SetOut(stdout) + rootCmd.SetErr(stderr) + + _, err = rootCmd.ExecuteC() + if tt.wantErr != "" { + if err == nil || err.Error() != tt.wantErr { + t.Fatalf("expected error %q, got %q", tt.wantErr, err) + } + return + } + if err != nil { + t.Fatalf("error executing command: %v", err) + } + + if !strings.Contains(stdout.String(), tt.wantOut) { + t.Errorf("completion output did not match:\n%s", stdout.String()) + } + if len(stderr.String()) > 0 { + t.Errorf("expected nothing on stderr, got %q", stderr.String()) + } + }) + } +} From 7f6d6876292c12a1b389d7e20ca852058faa7e8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 7 Aug 2020 16:56:13 +0200 Subject: [PATCH 08/11] Isolate `config` command --- command/config.go | 110 ----------------- command/config_test.go | 225 ---------------------------------- command/root.go | 3 +- pkg/cmd/config/config.go | 98 +++++++++++++++ pkg/cmd/config/config_test.go | 149 ++++++++++++++++++++++ 5 files changed, 249 insertions(+), 336 deletions(-) delete mode 100644 command/config.go delete mode 100644 command/config_test.go create mode 100644 pkg/cmd/config/config.go create mode 100644 pkg/cmd/config/config_test.go diff --git a/command/config.go b/command/config.go deleted file mode 100644 index 35fa09508..000000000 --- a/command/config.go +++ /dev/null @@ -1,110 +0,0 @@ -package command - -import ( - "fmt" - - "github.com/MakeNowJust/heredoc" - "github.com/spf13/cobra" -) - -func init() { - configCmd.AddCommand(configGetCmd) - configCmd.AddCommand(configSetCmd) - - configGetCmd.Flags().StringP("host", "h", "", "Get per-host setting") - configSetCmd.Flags().StringP("host", "h", "", "Set per-host setting") - - // TODO reveal and add usage once we properly support multiple hosts - _ = configGetCmd.Flags().MarkHidden("host") - // TODO reveal and add usage once we properly support multiple hosts - _ = configSetCmd.Flags().MarkHidden("host") -} - -var configCmd = &cobra.Command{ - Use: "config", - Short: "Manage configuration for gh", - Long: `Display or change configuration settings for gh. - -Current respected settings: -- git_protocol: "https" or "ssh". Default is "https". -- editor: if unset, defaults to environment variables. -`, -} - -var configGetCmd = &cobra.Command{ - Use: "get ", - Short: "Print the value of a given configuration key", - Example: heredoc.Doc(` - $ gh config get git_protocol - https - `), - Args: cobra.ExactArgs(1), - RunE: configGet, -} - -var configSetCmd = &cobra.Command{ - Use: "set ", - Short: "Update configuration with a value for the given key", - Example: heredoc.Doc(` - $ gh config set editor vim - $ gh config set editor "code --wait" - `), - Args: cobra.ExactArgs(2), - RunE: configSet, -} - -func configGet(cmd *cobra.Command, args []string) error { - key := args[0] - hostname, err := cmd.Flags().GetString("host") - if err != nil { - return err - } - - ctx := contextForCommand(cmd) - - cfg, err := ctx.Config() - if err != nil { - return err - } - - val, err := cfg.Get(hostname, key) - if err != nil { - return err - } - - if val != "" { - out := colorableOut(cmd) - fmt.Fprintf(out, "%s\n", val) - } - - return nil -} - -func configSet(cmd *cobra.Command, args []string) error { - key := args[0] - value := args[1] - - hostname, err := cmd.Flags().GetString("host") - if err != nil { - return err - } - - ctx := contextForCommand(cmd) - - cfg, err := ctx.Config() - if err != nil { - return err - } - - err = cfg.Set(hostname, key, value) - if err != nil { - return fmt.Errorf("failed to set %q to %q: %w", key, value, err) - } - - err = cfg.Write() - if err != nil { - return fmt.Errorf("failed to write config to disk: %w", err) - } - - return nil -} diff --git a/command/config_test.go b/command/config_test.go deleted file mode 100644 index 53654f54f..000000000 --- a/command/config_test.go +++ /dev/null @@ -1,225 +0,0 @@ -package command - -import ( - "bytes" - "testing" - - "github.com/cli/cli/internal/config" -) - -func TestConfigGet(t *testing.T) { - cfg := `--- -hosts: - github.com: - user: OWNER - oauth_token: MUSTBEHIGHCUZIMATOKEN -editor: ed -` - initBlankContext(cfg, "OWNER/REPO", "master") - - output, err := RunCommand("config get editor") - if err != nil { - t.Fatalf("error running command `config get editor`: %v", err) - } - - eq(t, output.String(), "ed\n") -} - -func TestConfigGet_default(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - output, err := RunCommand("config get git_protocol") - if err != nil { - t.Fatalf("error running command `config get git_protocol`: %v", err) - } - - eq(t, output.String(), "https\n") -} - -func TestConfigGet_not_found(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - - output, err := RunCommand("config get missing") - if err != nil { - t.Fatalf("error running command `config get missing`: %v", err) - } - - eq(t, output.String(), "") -} - -func TestConfigSet(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - - mainBuf := bytes.Buffer{} - hostsBuf := bytes.Buffer{} - defer config.StubWriteConfig(&mainBuf, &hostsBuf)() - - output, err := RunCommand("config set editor ed") - if err != nil { - t.Fatalf("error running command `config set editor ed`: %v", err) - } - - if len(output.String()) > 0 { - t.Errorf("expected output to be blank: %q", output.String()) - } - - expectedMain := "editor: ed\n" - expectedHosts := `github.com: - user: OWNER - oauth_token: "1234567890" -` - - if mainBuf.String() != expectedMain { - t.Errorf("expected config.yml to be %q, got %q", expectedMain, mainBuf.String()) - } - if hostsBuf.String() != expectedHosts { - t.Errorf("expected hosts.yml to be %q, got %q", expectedHosts, hostsBuf.String()) - } -} - -func TestConfigSet_update(t *testing.T) { - cfg := `--- -hosts: - github.com: - user: OWNER - oauth_token: MUSTBEHIGHCUZIMATOKEN -editor: ed -` - - initBlankContext(cfg, "OWNER/REPO", "master") - - mainBuf := bytes.Buffer{} - hostsBuf := bytes.Buffer{} - defer config.StubWriteConfig(&mainBuf, &hostsBuf)() - - output, err := RunCommand("config set editor vim") - if err != nil { - t.Fatalf("error running command `config get editor`: %v", err) - } - - if len(output.String()) > 0 { - t.Errorf("expected output to be blank: %q", output.String()) - } - - expectedMain := "editor: vim\n" - expectedHosts := `github.com: - user: OWNER - oauth_token: MUSTBEHIGHCUZIMATOKEN -` - - if mainBuf.String() != expectedMain { - t.Errorf("expected config.yml to be %q, got %q", expectedMain, mainBuf.String()) - } - if hostsBuf.String() != expectedHosts { - t.Errorf("expected hosts.yml to be %q, got %q", expectedHosts, hostsBuf.String()) - } -} - -func TestConfigGetHost(t *testing.T) { - cfg := `--- -hosts: - github.com: - git_protocol: ssh - user: OWNER - oauth_token: MUSTBEHIGHCUZIMATOKEN -editor: ed -git_protocol: https -` - initBlankContext(cfg, "OWNER/REPO", "master") - - output, err := RunCommand("config get -hgithub.com git_protocol") - if err != nil { - t.Fatalf("error running command `config get editor`: %v", err) - } - - eq(t, output.String(), "ssh\n") -} - -func TestConfigGetHost_unset(t *testing.T) { - cfg := `--- -hosts: - github.com: - user: OWNER - oauth_token: MUSTBEHIGHCUZIMATOKEN - -editor: ed -git_protocol: ssh -` - initBlankContext(cfg, "OWNER/REPO", "master") - - output, err := RunCommand("config get -hgithub.com git_protocol") - if err != nil { - t.Fatalf("error running command `config get -hgithub.com git_protocol`: %v", err) - } - - eq(t, output.String(), "ssh\n") -} - -func TestConfigSetHost(t *testing.T) { - initBlankContext("", "OWNER/REPO", "master") - - mainBuf := bytes.Buffer{} - hostsBuf := bytes.Buffer{} - defer config.StubWriteConfig(&mainBuf, &hostsBuf)() - - output, err := RunCommand("config set -hgithub.com git_protocol ssh") - if err != nil { - t.Fatalf("error running command `config set editor ed`: %v", err) - } - - if len(output.String()) > 0 { - t.Errorf("expected output to be blank: %q", output.String()) - } - - expectedMain := "" - expectedHosts := `github.com: - user: OWNER - oauth_token: "1234567890" - git_protocol: ssh -` - - if mainBuf.String() != expectedMain { - t.Errorf("expected config.yml to be %q, got %q", expectedMain, mainBuf.String()) - } - if hostsBuf.String() != expectedHosts { - t.Errorf("expected hosts.yml to be %q, got %q", expectedHosts, hostsBuf.String()) - } -} - -func TestConfigSetHost_update(t *testing.T) { - cfg := `--- -hosts: - github.com: - git_protocol: ssh - user: OWNER - oauth_token: MUSTBEHIGHCUZIMATOKEN -` - - initBlankContext(cfg, "OWNER/REPO", "master") - - mainBuf := bytes.Buffer{} - hostsBuf := bytes.Buffer{} - defer config.StubWriteConfig(&mainBuf, &hostsBuf)() - - output, err := RunCommand("config set -hgithub.com git_protocol https") - if err != nil { - t.Fatalf("error running command `config get editor`: %v", err) - } - - if len(output.String()) > 0 { - t.Errorf("expected output to be blank: %q", output.String()) - } - - expectedMain := "" - expectedHosts := `github.com: - git_protocol: https - user: OWNER - oauth_token: MUSTBEHIGHCUZIMATOKEN -` - - if mainBuf.String() != expectedMain { - t.Errorf("expected config.yml to be %q, got %q", expectedMain, mainBuf.String()) - } - if hostsBuf.String() != expectedHosts { - t.Errorf("expected hosts.yml to be %q, got %q", expectedHosts, hostsBuf.String()) - } -} diff --git a/command/root.go b/command/root.go index fb31e886c..37722e8e4 100644 --- a/command/root.go +++ b/command/root.go @@ -17,6 +17,7 @@ import ( "github.com/cli/cli/internal/config" "github.com/cli/cli/internal/ghinstance" "github.com/cli/cli/internal/run" + configCmd "github.com/cli/cli/pkg/cmd/config" "github.com/cli/cli/pkg/cmd/factory" "github.com/cli/cli/pkg/cmd/root" "github.com/cli/cli/utils" @@ -44,7 +45,7 @@ func init() { RootCmd = root.NewCmdRoot(cmdFactory, Version, BuildDate) RootCmd.AddCommand(aliasCmd) RootCmd.AddCommand(root.NewCmdCompletion(cmdFactory.IOStreams)) - RootCmd.AddCommand(configCmd) + RootCmd.AddCommand(configCmd.NewCmdConfig(cmdFactory)) } // overridden in tests diff --git a/pkg/cmd/config/config.go b/pkg/cmd/config/config.go new file mode 100644 index 000000000..b92af7a88 --- /dev/null +++ b/pkg/cmd/config/config.go @@ -0,0 +1,98 @@ +package config + +import ( + "fmt" + + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/pkg/cmdutil" + "github.com/spf13/cobra" +) + +func NewCmdConfig(f *cmdutil.Factory) *cobra.Command { + cmd := &cobra.Command{ + Use: "config", + Short: "Manage configuration for gh", + Long: heredoc.Doc(` + Display or change configuration settings for gh. + + Current respected settings: + - git_protocol: "https" or "ssh". Default is "https". + - editor: if unset, defaults to environment variables. + `), + } + + cmd.AddCommand(NewCmdConfigGet(f)) + cmd.AddCommand(NewCmdConfigSet(f)) + + return cmd +} + +func NewCmdConfigGet(f *cmdutil.Factory) *cobra.Command { + var hostname string + + cmd := &cobra.Command{ + Use: "get ", + Short: "Print the value of a given configuration key", + Example: heredoc.Doc(` + $ gh config get git_protocol + https + `), + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + cfg, err := f.Config() + if err != nil { + return err + } + + val, err := cfg.Get(hostname, args[0]) + if err != nil { + return err + } + + if val != "" { + fmt.Fprintf(f.IOStreams.Out, "%s\n", val) + } + return nil + }, + } + + cmd.Flags().StringVarP(&hostname, "host", "h", "", "Get per-host setting") + + return cmd +} + +func NewCmdConfigSet(f *cmdutil.Factory) *cobra.Command { + var hostname string + + cmd := &cobra.Command{ + Use: "set ", + Short: "Update configuration with a value for the given key", + Example: heredoc.Doc(` + $ gh config set editor vim + $ gh config set editor "code --wait" + `), + Args: cobra.ExactArgs(2), + RunE: func(cmd *cobra.Command, args []string) error { + cfg, err := f.Config() + if err != nil { + return err + } + + key, value := args[0], args[1] + err = cfg.Set(hostname, key, value) + if err != nil { + return fmt.Errorf("failed to set %q to %q: %w", key, value, err) + } + + err = cfg.Write() + if err != nil { + return fmt.Errorf("failed to write config to disk: %w", err) + } + return nil + }, + } + + cmd.Flags().StringVarP(&hostname, "host", "h", "", "Set per-host setting") + + return cmd +} diff --git a/pkg/cmd/config/config_test.go b/pkg/cmd/config/config_test.go new file mode 100644 index 000000000..8a7068820 --- /dev/null +++ b/pkg/cmd/config/config_test.go @@ -0,0 +1,149 @@ +package config + +import ( + "errors" + "testing" + + "github.com/cli/cli/internal/config" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/iostreams" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type configStub map[string]string + +func genKey(host, key string) string { + if host != "" { + return host + ":" + key + } + return key +} + +func (c configStub) Get(host, key string) (string, error) { + if v, found := c[genKey(host, key)]; found { + return v, nil + } + return "", errors.New("not found") +} + +func (c configStub) Set(host, key, value string) error { + c[genKey(host, key)] = value + return nil +} + +func (c configStub) Aliases() (*config.AliasConfig, error) { + return nil, nil +} + +func (c configStub) Write() error { + c["_written"] = "true" + return nil +} + +func TestConfigGet(t *testing.T) { + tests := []struct { + name string + config configStub + args []string + stdout string + stderr string + }{ + { + name: "get key", + config: configStub{ + "editor": "ed", + }, + args: []string{"editor"}, + stdout: "ed\n", + stderr: "", + }, + { + name: "get key scoped by host", + config: configStub{ + "editor": "ed", + "github.com:editor": "vim", + }, + args: []string{"editor", "-h", "github.com"}, + stdout: "vim\n", + stderr: "", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + io, _, stdout, stderr := iostreams.Test() + f := &cmdutil.Factory{ + IOStreams: io, + Config: func() (config.Config, error) { + return tt.config, nil + }, + } + + cmd := NewCmdConfigGet(f) + cmd.Flags().BoolP("help", "x", false, "") + cmd.SetArgs(tt.args) + cmd.SetOut(stdout) + cmd.SetErr(stderr) + + _, err := cmd.ExecuteC() + require.NoError(t, err) + + assert.Equal(t, tt.stdout, stdout.String()) + assert.Equal(t, tt.stderr, stderr.String()) + assert.Equal(t, "", tt.config["_written"]) + }) + } +} + +func TestConfigSet(t *testing.T) { + tests := []struct { + name string + config configStub + args []string + expectKey string + stdout string + stderr string + }{ + { + name: "set key", + config: configStub{}, + args: []string{"editor", "vim"}, + expectKey: "editor", + stdout: "", + stderr: "", + }, + { + name: "set key scoped by host", + config: configStub{}, + args: []string{"editor", "vim", "-h", "github.com"}, + expectKey: "github.com:editor", + stdout: "", + stderr: "", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + io, _, stdout, stderr := iostreams.Test() + f := &cmdutil.Factory{ + IOStreams: io, + Config: func() (config.Config, error) { + return tt.config, nil + }, + } + + cmd := NewCmdConfigSet(f) + cmd.Flags().BoolP("help", "x", false, "") + cmd.SetArgs(tt.args) + cmd.SetOut(stdout) + cmd.SetErr(stderr) + + _, err := cmd.ExecuteC() + require.NoError(t, err) + + assert.Equal(t, tt.stdout, stdout.String()) + assert.Equal(t, tt.stderr, stderr.String()) + assert.Equal(t, "vim", tt.config[tt.expectKey]) + assert.Equal(t, "true", tt.config["_written"]) + }) + } +} From 3c55b29446015b0d1443bbe60fd4000ea79feea1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Fri, 7 Aug 2020 17:02:24 +0200 Subject: [PATCH 09/11] :fire: colorableOut --- command/root.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/command/root.go b/command/root.go index 37722e8e4..8c3b17e51 100644 --- a/command/root.go +++ b/command/root.go @@ -84,14 +84,6 @@ func apiVerboseLog() api.ClientOption { return api.VerboseLog(utils.NewColorable(os.Stderr), logTraffic, colorize) } -func colorableOut(cmd *cobra.Command) io.Writer { - out := cmd.OutOrStdout() - if outFile, isFile := out.(*os.File); isFile { - return utils.NewColorable(outFile) - } - return out -} - func colorableErr(cmd *cobra.Command) io.Writer { err := cmd.ErrOrStderr() if outFile, isFile := err.(*os.File); isFile { From 172ea2b07845a6da1ea500ecb11fb4e8c6d39384 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 11 Aug 2020 13:57:48 +0200 Subject: [PATCH 10/11] Isolate all `alias` commands --- command/alias.go | 233 --------------------- command/alias_test.go | 314 ---------------------------- command/root.go | 22 -- internal/config/config_file.go | 22 +- internal/config/config_type.go | 10 + pkg/cmd/alias/alias.go | 28 +++ pkg/cmd/alias/delete/delete.go | 71 +++++++ pkg/cmd/alias/delete/delete_test.go | 90 ++++++++ pkg/cmd/alias/list/list.go | 83 ++++++++ pkg/cmd/alias/list/list_test.go | 77 +++++++ pkg/cmd/alias/set/set.go | 147 +++++++++++++ pkg/cmd/alias/set/set_test.go | 252 ++++++++++++++++++++++ pkg/cmd/root/root.go | 7 +- test/helpers.go | 9 +- 14 files changed, 786 insertions(+), 579 deletions(-) delete mode 100644 command/alias.go create mode 100644 pkg/cmd/alias/alias.go create mode 100644 pkg/cmd/alias/delete/delete.go create mode 100644 pkg/cmd/alias/delete/delete_test.go create mode 100644 pkg/cmd/alias/list/list.go create mode 100644 pkg/cmd/alias/list/list_test.go create mode 100644 pkg/cmd/alias/set/set.go create mode 100644 pkg/cmd/alias/set/set_test.go diff --git a/command/alias.go b/command/alias.go deleted file mode 100644 index 64b6ab19e..000000000 --- a/command/alias.go +++ /dev/null @@ -1,233 +0,0 @@ -package command - -import ( - "fmt" - "sort" - "strings" - - "github.com/MakeNowJust/heredoc" - "github.com/cli/cli/pkg/iostreams" - "github.com/cli/cli/utils" - "github.com/google/shlex" - "github.com/spf13/cobra" -) - -func init() { - aliasCmd.AddCommand(aliasSetCmd) - aliasCmd.AddCommand(aliasListCmd) - aliasCmd.AddCommand(aliasDeleteCmd) - - aliasSetCmd.Flags().BoolP("shell", "s", false, "Declare an alias to be passed through a shell interpreter") -} - -var aliasCmd = &cobra.Command{ - Use: "alias", - Short: "Create command shortcuts", - Long: heredoc.Doc(` - Aliases can be used to make shortcuts for gh commands or to compose multiple commands. - - Run "gh help alias set" to learn more. - `), -} - -var aliasSetCmd = &cobra.Command{ - Use: "set ", - Short: "Create a shortcut for a gh command", - Long: heredoc.Doc(` - Declare a word as a command alias that will expand to the specified command(s). - - The expansion may specify additional arguments and flags. If the expansion - includes positional placeholders such as '$1', '$2', etc., any extra arguments - that follow the invocation of an alias will be inserted appropriately. - - If '--shell' is specified, the alias will be run through a shell interpreter (sh). This allows you - to compose commands with "|" or redirect with ">". Note that extra arguments following the alias - will not be automatically passed to the expanded expression. To have a shell alias receive - arguments, you must explicitly accept them using "$1", "$2", etc., or "$@" to accept all of them. - - Platform note: on Windows, shell aliases are executed via "sh" as installed by Git For Windows. If - you have installed git on Windows in some other way, shell aliases may not work for you. - - Quotes must always be used when defining a command as in the examples.`), - Example: heredoc.Doc(` - $ gh alias set pv 'pr view' - $ gh pv -w 123 - #=> gh pr view -w 123 - - $ gh alias set bugs 'issue list --label="bugs"' - - $ gh alias set epicsBy 'issue list --author="$1" --label="epic"' - $ gh epicsBy vilmibm - #=> gh issue list --author="vilmibm" --label="epic" - - $ gh alias set --shell igrep 'gh issue list --label="$1" | grep $2' - $ gh igrep epic foo - #=> gh issue list --label="epic" | grep "foo"`), - Args: cobra.ExactArgs(2), - RunE: aliasSet, -} - -func aliasSet(cmd *cobra.Command, args []string) error { - ctx := contextForCommand(cmd) - cfg, err := ctx.Config() - if err != nil { - return err - } - - aliasCfg, err := cfg.Aliases() - if err != nil { - return err - } - - alias := args[0] - expansion := args[1] - - stderr := colorableErr(cmd) - if connectedToTerminal(cmd) { - fmt.Fprintf(stderr, "- Adding alias for %s: %s\n", utils.Bold(alias), utils.Bold(expansion)) - } - - shell, err := cmd.Flags().GetBool("shell") - if err != nil { - return err - } - if shell && !strings.HasPrefix(expansion, "!") { - expansion = "!" + expansion - } - isExternal := strings.HasPrefix(expansion, "!") - - if validCommand(alias) { - return fmt.Errorf("could not create alias: %q is already a gh command", alias) - } - - if !isExternal && !validCommand(expansion) { - return fmt.Errorf("could not create alias: %s does not correspond to a gh command", expansion) - } - - successMsg := fmt.Sprintf("%s Added alias.", utils.Green("✓")) - - oldExpansion, ok := aliasCfg.Get(alias) - if ok { - successMsg = fmt.Sprintf("%s Changed alias %s from %s to %s", - utils.Green("✓"), - utils.Bold(alias), - utils.Bold(oldExpansion), - utils.Bold(expansion), - ) - } - - err = aliasCfg.Add(alias, expansion) - if err != nil { - return fmt.Errorf("could not create alias: %s", err) - } - - if connectedToTerminal(cmd) { - fmt.Fprintln(stderr, successMsg) - } - - return nil -} - -func validCommand(expansion string) bool { - split, err := shlex.Split(expansion) - if err != nil { - return false - } - cmd, _, err := RootCmd.Traverse(split) - return err == nil && cmd != RootCmd -} - -var aliasListCmd = &cobra.Command{ - Use: "list", - Short: "List your aliases", - Long: `This command prints out all of the aliases gh is configured to use.`, - Args: cobra.ExactArgs(0), - RunE: aliasList, -} - -func aliasList(cmd *cobra.Command, args []string) error { - ctx := contextForCommand(cmd) - cfg, err := ctx.Config() - if err != nil { - return fmt.Errorf("couldn't read config: %w", err) - } - - aliasCfg, err := cfg.Aliases() - if err != nil { - return fmt.Errorf("couldn't read aliases config: %w", err) - } - - stderr := colorableErr(cmd) - - if aliasCfg.Empty() { - if connectedToTerminal(cmd) { - fmt.Fprintf(stderr, "no aliases configured\n") - } - return nil - } - - tp := utils.NewTablePrinter(&iostreams.IOStreams{ - Out: cmd.OutOrStdout(), - }) - - aliasMap := aliasCfg.All() - keys := []string{} - for alias := range aliasMap { - keys = append(keys, alias) - } - sort.Strings(keys) - - for _, alias := range keys { - if tp.IsTTY() { - // ensure that screen readers pause - tp.AddField(alias+":", nil, nil) - } else { - tp.AddField(alias, nil, nil) - } - tp.AddField(aliasMap[alias], nil, nil) - tp.EndRow() - } - - return tp.Render() -} - -var aliasDeleteCmd = &cobra.Command{ - Use: "delete ", - Short: "Delete an alias.", - Args: cobra.ExactArgs(1), - RunE: aliasDelete, -} - -func aliasDelete(cmd *cobra.Command, args []string) error { - alias := args[0] - - ctx := contextForCommand(cmd) - cfg, err := ctx.Config() - if err != nil { - return fmt.Errorf("couldn't read config: %w", err) - } - - aliasCfg, err := cfg.Aliases() - if err != nil { - return fmt.Errorf("couldn't read aliases config: %w", err) - } - - expansion, ok := aliasCfg.Get(alias) - if !ok { - return fmt.Errorf("no such alias %s", alias) - - } - - err = aliasCfg.Delete(alias) - if err != nil { - return fmt.Errorf("failed to delete alias %s: %w", alias, err) - } - - if connectedToTerminal(cmd) { - stderr := colorableErr(cmd) - redCheck := utils.Red("✓") - fmt.Fprintf(stderr, "%s Deleted alias %s; was %s\n", redCheck, alias, expansion) - } - - return nil -} diff --git a/command/alias_test.go b/command/alias_test.go index 63646f7a6..7bb76f553 100644 --- a/command/alias_test.go +++ b/command/alias_test.go @@ -1,12 +1,9 @@ package command import ( - "bytes" "strings" "testing" - "github.com/cli/cli/internal/config" - "github.com/cli/cli/test" "github.com/stretchr/testify/assert" ) @@ -20,180 +17,6 @@ func stubSh(value string) func() { } } -func TestAliasSet_gh_command(t *testing.T) { - initBlankContext("", "OWNER/REPO", "trunk") - - mainBuf := bytes.Buffer{} - hostsBuf := bytes.Buffer{} - defer config.StubWriteConfig(&mainBuf, &hostsBuf)() - - _, err := RunCommand("alias set pr 'pr status'") - if err == nil { - t.Fatal("expected error") - } - - eq(t, err.Error(), `could not create alias: "pr" is already a gh command`) -} - -func TestAliasSet_empty_aliases(t *testing.T) { - cfg := `--- -aliases: -editor: vim -` - initBlankContext(cfg, "OWNER/REPO", "trunk") - - defer stubTerminal(true)() - - mainBuf := bytes.Buffer{} - hostsBuf := bytes.Buffer{} - defer config.StubWriteConfig(&mainBuf, &hostsBuf)() - - output, err := RunCommand("alias set co 'pr checkout'") - - if err != nil { - t.Fatalf("unexpected error: %s", err) - } - - test.ExpectLines(t, output.Stderr(), "Added alias") - test.ExpectLines(t, output.String(), "") - - expected := `aliases: - co: pr checkout -editor: vim -` - eq(t, mainBuf.String(), expected) -} - -func TestAliasSet_existing_alias(t *testing.T) { - cfg := `--- -hosts: - github.com: - user: OWNER - oauth_token: token123 -aliases: - co: pr checkout -` - initBlankContext(cfg, "OWNER/REPO", "trunk") - defer stubTerminal(true)() - - mainBuf := bytes.Buffer{} - hostsBuf := bytes.Buffer{} - defer config.StubWriteConfig(&mainBuf, &hostsBuf)() - - output, err := RunCommand("alias set co 'pr checkout -Rcool/repo'") - - if err != nil { - t.Fatalf("unexpected error: %s", err) - } - - test.ExpectLines(t, output.Stderr(), "Changed alias.*co.*from.*pr checkout.*to.*pr checkout -Rcool/repo") -} - -func TestAliasSet_space_args(t *testing.T) { - initBlankContext("", "OWNER/REPO", "trunk") - defer stubTerminal(true)() - - mainBuf := bytes.Buffer{} - hostsBuf := bytes.Buffer{} - defer config.StubWriteConfig(&mainBuf, &hostsBuf)() - - output, err := RunCommand(`alias set il 'issue list -l "cool story"'`) - - if err != nil { - t.Fatalf("unexpected error: %s", err) - } - - test.ExpectLines(t, output.Stderr(), `Adding alias for.*il.*issue list -l "cool story"`) - - test.ExpectLines(t, mainBuf.String(), `il: issue list -l "cool story"`) -} - -func TestAliasSet_arg_processing(t *testing.T) { - initBlankContext("", "OWNER/REPO", "trunk") - defer stubTerminal(true)() - cases := []struct { - Cmd string - ExpectedOutputLine string - ExpectedConfigLine string - }{ - {`alias set il "issue list"`, "- Adding alias for.*il.*issue list", "il: issue list"}, - - {`alias set iz 'issue list'`, "- Adding alias for.*iz.*issue list", "iz: issue list"}, - - {`alias set ii 'issue list --author="$1" --label="$2"'`, - `- Adding alias for.*ii.*issue list --author="\$1" --label="\$2"`, - `ii: issue list --author="\$1" --label="\$2"`}, - - {`alias set ix "issue list --author='\$1' --label='\$2'"`, - `- Adding alias for.*ix.*issue list --author='\$1' --label='\$2'`, - `ix: issue list --author='\$1' --label='\$2'`}, - } - - for _, c := range cases { - mainBuf := bytes.Buffer{} - hostsBuf := bytes.Buffer{} - defer config.StubWriteConfig(&mainBuf, &hostsBuf)() - - output, err := RunCommand(c.Cmd) - if err != nil { - t.Fatalf("got unexpected error running %s: %s", c.Cmd, err) - } - - test.ExpectLines(t, output.Stderr(), c.ExpectedOutputLine) - test.ExpectLines(t, mainBuf.String(), c.ExpectedConfigLine) - } -} - -func TestAliasSet_init_alias_cfg(t *testing.T) { - cfg := `--- -editor: vim -` - initBlankContext(cfg, "OWNER/REPO", "trunk") - defer stubTerminal(true)() - - mainBuf := bytes.Buffer{} - hostsBuf := bytes.Buffer{} - defer config.StubWriteConfig(&mainBuf, &hostsBuf)() - - output, err := RunCommand("alias set diff 'pr diff'") - if err != nil { - t.Fatalf("unexpected error: %s", err) - } - expected := `editor: vim -aliases: - diff: pr diff -` - - test.ExpectLines(t, output.Stderr(), "Adding alias for.*diff.*pr diff", "Added alias.") - eq(t, mainBuf.String(), expected) -} - -func TestAliasSet_existing_aliases(t *testing.T) { - cfg := `--- -aliases: - foo: bar -` - initBlankContext(cfg, "OWNER/REPO", "trunk") - defer stubTerminal(true)() - - mainBuf := bytes.Buffer{} - hostsBuf := bytes.Buffer{} - defer config.StubWriteConfig(&mainBuf, &hostsBuf)() - - output, err := RunCommand("alias set view 'pr view'") - if err != nil { - t.Fatalf("unexpected error: %s", err) - } - expected := `aliases: - foo: bar - view: pr view -` - - test.ExpectLines(t, output.Stderr(), "Adding alias for.*view.*pr view", "Added alias.") - eq(t, mainBuf.String(), expected) - -} - func TestExpandAlias_shell(t *testing.T) { defer stubSh("sh")() cfg := `--- @@ -284,140 +107,3 @@ aliases: assert.Equal(t, c.ExpectedArgs, expanded) } } - -func TestAliasSet_invalid_command(t *testing.T) { - initBlankContext("", "OWNER/REPO", "trunk") - _, err := RunCommand("alias set co 'pe checkout'") - if err == nil { - t.Fatal("expected error") - } - - eq(t, err.Error(), "could not create alias: pe checkout does not correspond to a gh command") -} - -func TestAliasList_empty(t *testing.T) { - initBlankContext("", "OWNER/REPO", "trunk") - - output, err := RunCommand("alias list") - - if err != nil { - t.Fatalf("unexpected error: %s", err) - } - - eq(t, output.String(), "") -} - -func TestAliasList(t *testing.T) { - cfg := `--- -aliases: - co: pr checkout - il: issue list --author=$1 --label=$2 - clone: repo clone - prs: pr status - cs: config set editor 'quoted path' -` - initBlankContext(cfg, "OWNER/REPO", "trunk") - - output, err := RunCommand("alias list") - if err != nil { - t.Fatalf("unexpected error: %s", err) - } - expected := `clone repo clone -co pr checkout -cs config set editor 'quoted path' -il issue list --author=$1 --label=$2 -prs pr status -` - - eq(t, output.String(), expected) -} - -func TestAliasDelete_nonexistent_command(t *testing.T) { - cfg := `--- -aliases: - co: pr checkout - il: issue list --author="$1" --label="$2" - ia: issue list --author="$1" --assignee="$1" -` - initBlankContext(cfg, "OWNER/REPO", "trunk") - - _, err := RunCommand("alias delete cool") - if err == nil { - t.Fatalf("expected error") - } - - eq(t, err.Error(), "no such alias cool") -} - -func TestAliasDelete(t *testing.T) { - cfg := `--- -aliases: - co: pr checkout - il: issue list --author="$1" --label="$2" - ia: issue list --author="$1" --assignee="$1" -` - initBlankContext(cfg, "OWNER/REPO", "trunk") - defer stubTerminal(true)() - - mainBuf := bytes.Buffer{} - hostsBuf := bytes.Buffer{} - defer config.StubWriteConfig(&mainBuf, &hostsBuf)() - - output, err := RunCommand("alias delete co") - if err != nil { - t.Fatalf("unexpected error: %s", err) - } - - test.ExpectLines(t, output.Stderr(), "Deleted alias co; was pr checkout") - - expected := `aliases: - il: issue list --author="$1" --label="$2" - ia: issue list --author="$1" --assignee="$1" -` - - eq(t, mainBuf.String(), expected) -} - -func TestShellAlias_flag(t *testing.T) { - initBlankContext("", "OWNER/REPO", "trunk") - - defer stubTerminal(true)() - - mainBuf := bytes.Buffer{} - hostsBuf := bytes.Buffer{} - defer config.StubWriteConfig(&mainBuf, &hostsBuf)() - - output, err := RunCommand("alias set --shell igrep 'gh issue list | grep'") - if err != nil { - t.Fatalf("unexpected error: %s", err) - } - - test.ExpectLines(t, output.Stderr(), "Adding alias for.*igrep") - expected := `aliases: - igrep: '!gh issue list | grep' -` - - eq(t, mainBuf.String(), expected) -} - -func TestShellAlias_bang(t *testing.T) { - initBlankContext("", "OWNER/REPO", "trunk") - - defer stubTerminal(true)() - - mainBuf := bytes.Buffer{} - hostsBuf := bytes.Buffer{} - defer config.StubWriteConfig(&mainBuf, &hostsBuf)() - - output, err := RunCommand("alias set igrep '!gh issue list | grep'") - if err != nil { - t.Fatalf("unexpected error: %s", err) - } - - test.ExpectLines(t, output.Stderr(), "Adding alias for.*igrep") - expected := `aliases: - igrep: '!gh issue list | grep' -` - - eq(t, mainBuf.String(), expected) -} diff --git a/command/root.go b/command/root.go index 8c3b17e51..1ff53e783 100644 --- a/command/root.go +++ b/command/root.go @@ -3,7 +3,6 @@ package command import ( "errors" "fmt" - "io" "os" "os/exec" "path/filepath" @@ -17,12 +16,10 @@ import ( "github.com/cli/cli/internal/config" "github.com/cli/cli/internal/ghinstance" "github.com/cli/cli/internal/run" - configCmd "github.com/cli/cli/pkg/cmd/config" "github.com/cli/cli/pkg/cmd/factory" "github.com/cli/cli/pkg/cmd/root" "github.com/cli/cli/utils" "github.com/google/shlex" - "github.com/spf13/cobra" ) @@ -43,9 +40,6 @@ func init() { cmdFactory := factory.New(Version) RootCmd = root.NewCmdRoot(cmdFactory, Version, BuildDate) - RootCmd.AddCommand(aliasCmd) - RootCmd.AddCommand(root.NewCmdCompletion(cmdFactory.IOStreams)) - RootCmd.AddCommand(configCmd.NewCmdConfig(cmdFactory)) } // overridden in tests @@ -74,24 +68,12 @@ func BasicClient() (*api.Client, error) { return api.NewClient(opts...), nil } -func contextForCommand(cmd *cobra.Command) context.Context { - return initContext() -} - func apiVerboseLog() api.ClientOption { logTraffic := strings.Contains(os.Getenv("DEBUG"), "api") colorize := utils.IsTerminal(os.Stderr) return api.VerboseLog(utils.NewColorable(os.Stderr), logTraffic, colorize) } -func colorableErr(cmd *cobra.Command) io.Writer { - err := cmd.ErrOrStderr() - if outFile, isFile := err.(*os.File); isFile { - return utils.NewColorable(outFile) - } - return err -} - func ExecuteShellAlias(args []string) error { externalCmd := exec.Command(args[0], args[1:]...) externalCmd.Stderr = os.Stderr @@ -198,7 +180,3 @@ func ExpandAlias(args []string) (expanded []string, isShell bool, err error) { expanded = args[1:] return } - -func connectedToTerminal(cmd *cobra.Command) bool { - return utils.IsTerminal(cmd.InOrStdin()) && utils.IsTerminal(cmd.OutOrStdout()) -} diff --git a/internal/config/config_file.go b/internal/config/config_file.go index 2f97d2956..d4c132360 100644 --- a/internal/config/config_file.go +++ b/internal/config/config_file.go @@ -75,22 +75,30 @@ func parseConfigFile(filename string) ([]byte, *yaml.Node, error) { return nil, nil, err } - var root yaml.Node - err = yaml.Unmarshal(data, &root) + root, err := parseConfigData(data) if err != nil { - return data, nil, err + return nil, nil, err } + return data, root, err +} + +func parseConfigData(data []byte) (*yaml.Node, error) { + var root yaml.Node + err := yaml.Unmarshal(data, &root) + if err != nil { + return nil, err + } + if len(root.Content) == 0 { - return data, &yaml.Node{ + return &yaml.Node{ Kind: yaml.DocumentNode, Content: []*yaml.Node{{Kind: yaml.MappingNode}}, }, nil } if root.Content[0].Kind != yaml.MappingNode { - return data, &root, fmt.Errorf("expected a top level map") + return &root, fmt.Errorf("expected a top level map") } - - return data, &root, nil + return &root, nil } func isLegacy(root *yaml.Node) bool { diff --git a/internal/config/config_type.go b/internal/config/config_type.go index 85e5c0d4a..721c4340e 100644 --- a/internal/config/config_type.go +++ b/internal/config/config_type.go @@ -122,6 +122,16 @@ func NewConfig(root *yaml.Node) Config { } } +// NewFromString initializes a Config from a yaml string +func NewFromString(str string) Config { + root, err := parseConfigData([]byte(str)) + if err != nil { + panic(err) + } + return NewConfig(root) +} + +// NewBlankConfig initializes a config file pre-populated with comments and default values func NewBlankConfig() Config { return NewConfig(NewBlankRoot()) } diff --git a/pkg/cmd/alias/alias.go b/pkg/cmd/alias/alias.go new file mode 100644 index 000000000..20bc26e83 --- /dev/null +++ b/pkg/cmd/alias/alias.go @@ -0,0 +1,28 @@ +package alias + +import ( + "github.com/MakeNowJust/heredoc" + deleteCmd "github.com/cli/cli/pkg/cmd/alias/delete" + listCmd "github.com/cli/cli/pkg/cmd/alias/list" + setCmd "github.com/cli/cli/pkg/cmd/alias/set" + "github.com/cli/cli/pkg/cmdutil" + "github.com/spf13/cobra" +) + +func NewCmdAlias(f *cmdutil.Factory) *cobra.Command { + cmd := &cobra.Command{ + Use: "alias", + Short: "Create command shortcuts", + Long: heredoc.Doc(` + Aliases can be used to make shortcuts for gh commands or to compose multiple commands. + + Run "gh help alias set" to learn more. + `), + } + + cmd.AddCommand(deleteCmd.NewCmdDelete(f, nil)) + cmd.AddCommand(listCmd.NewCmdList(f, nil)) + cmd.AddCommand(setCmd.NewCmdSet(f, nil)) + + return cmd +} diff --git a/pkg/cmd/alias/delete/delete.go b/pkg/cmd/alias/delete/delete.go new file mode 100644 index 000000000..ccf98ca68 --- /dev/null +++ b/pkg/cmd/alias/delete/delete.go @@ -0,0 +1,71 @@ +package delete + +import ( + "fmt" + + "github.com/cli/cli/internal/config" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/utils" + "github.com/spf13/cobra" +) + +type DeleteOptions struct { + Config func() (config.Config, error) + IO *iostreams.IOStreams + + Name string +} + +func NewCmdDelete(f *cmdutil.Factory, runF func(*DeleteOptions) error) *cobra.Command { + opts := &DeleteOptions{ + IO: f.IOStreams, + Config: f.Config, + } + + cmd := &cobra.Command{ + Use: "delete ", + Short: "Delete an alias", + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + opts.Name = args[0] + + if runF != nil { + return runF(opts) + } + return deleteRun(opts) + }, + } + + return cmd +} + +func deleteRun(opts *DeleteOptions) error { + cfg, err := opts.Config() + if err != nil { + return err + } + + aliasCfg, err := cfg.Aliases() + if err != nil { + return fmt.Errorf("couldn't read aliases config: %w", err) + } + + expansion, ok := aliasCfg.Get(opts.Name) + if !ok { + return fmt.Errorf("no such alias %s", opts.Name) + + } + + err = aliasCfg.Delete(opts.Name) + if err != nil { + return fmt.Errorf("failed to delete alias %s: %w", opts.Name, err) + } + + if opts.IO.IsStdoutTTY() { + redCheck := utils.Red("✓") + fmt.Fprintf(opts.IO.ErrOut, "%s Deleted alias %s; was %s\n", redCheck, opts.Name, expansion) + } + + return nil +} diff --git a/pkg/cmd/alias/delete/delete_test.go b/pkg/cmd/alias/delete/delete_test.go new file mode 100644 index 000000000..3c6acea28 --- /dev/null +++ b/pkg/cmd/alias/delete/delete_test.go @@ -0,0 +1,90 @@ +package delete + +import ( + "bytes" + "io/ioutil" + "testing" + + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/internal/config" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/iostreams" + "github.com/google/shlex" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestAliasDelete(t *testing.T) { + tests := []struct { + name string + config string + cli string + isTTY bool + wantStdout string + wantStderr string + wantErr string + }{ + { + name: "no aliases", + config: "", + cli: "co", + isTTY: true, + wantStdout: "", + wantStderr: "", + wantErr: "no such alias co", + }, + { + name: "delete one", + config: heredoc.Doc(` + aliases: + il: issue list + co: pr checkout + `), + cli: "co", + isTTY: true, + wantStdout: "", + wantStderr: "✓ Deleted alias co; was pr checkout\n", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + defer config.StubWriteConfig(ioutil.Discard, ioutil.Discard)() + + cfg := config.NewFromString(tt.config) + + io, _, stdout, stderr := iostreams.Test() + io.SetStdoutTTY(tt.isTTY) + io.SetStdinTTY(tt.isTTY) + io.SetStderrTTY(tt.isTTY) + + factory := &cmdutil.Factory{ + IOStreams: io, + Config: func() (config.Config, error) { + return cfg, nil + }, + } + + cmd := NewCmdDelete(factory, nil) + + argv, err := shlex.Split(tt.cli) + require.NoError(t, err) + cmd.SetArgs(argv) + + cmd.SetIn(&bytes.Buffer{}) + cmd.SetOut(ioutil.Discard) + cmd.SetErr(ioutil.Discard) + + _, err = cmd.ExecuteC() + if tt.wantErr != "" { + if assert.Error(t, err) { + assert.Equal(t, tt.wantErr, err.Error()) + } + return + } + require.NoError(t, err) + + assert.Equal(t, tt.wantStdout, stdout.String()) + assert.Equal(t, tt.wantStderr, stderr.String()) + }) + } +} diff --git a/pkg/cmd/alias/list/list.go b/pkg/cmd/alias/list/list.go new file mode 100644 index 000000000..b9d67c2ae --- /dev/null +++ b/pkg/cmd/alias/list/list.go @@ -0,0 +1,83 @@ +package list + +import ( + "fmt" + "sort" + + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/internal/config" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/utils" + "github.com/spf13/cobra" +) + +type ListOptions struct { + Config func() (config.Config, error) + IO *iostreams.IOStreams +} + +func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Command { + opts := &ListOptions{ + IO: f.IOStreams, + Config: f.Config, + } + + cmd := &cobra.Command{ + Use: "list", + Short: "List your aliases", + Long: heredoc.Doc(` + This command prints out all of the aliases gh is configured to use. + `), + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, args []string) error { + if runF != nil { + return runF(opts) + } + return listRun(opts) + }, + } + + return cmd +} + +func listRun(opts *ListOptions) error { + cfg, err := opts.Config() + if err != nil { + return err + } + + aliasCfg, err := cfg.Aliases() + if err != nil { + return fmt.Errorf("couldn't read aliases config: %w", err) + } + + if aliasCfg.Empty() { + if opts.IO.IsStdoutTTY() { + fmt.Fprintf(opts.IO.ErrOut, "no aliases configured\n") + } + return nil + } + + tp := utils.NewTablePrinter(opts.IO) + + aliasMap := aliasCfg.All() + keys := []string{} + for alias := range aliasMap { + keys = append(keys, alias) + } + sort.Strings(keys) + + for _, alias := range keys { + if tp.IsTTY() { + // ensure that screen readers pause + tp.AddField(alias+":", nil, nil) + } else { + tp.AddField(alias, nil, nil) + } + tp.AddField(aliasMap[alias], nil, nil) + tp.EndRow() + } + + return tp.Render() +} diff --git a/pkg/cmd/alias/list/list_test.go b/pkg/cmd/alias/list/list_test.go new file mode 100644 index 000000000..b058a04ce --- /dev/null +++ b/pkg/cmd/alias/list/list_test.go @@ -0,0 +1,77 @@ +package list + +import ( + "bytes" + "io/ioutil" + "testing" + + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/internal/config" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/iostreams" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestAliasList(t *testing.T) { + tests := []struct { + name string + config string + isTTY bool + wantStdout string + wantStderr string + }{ + { + name: "empty", + config: "", + isTTY: true, + wantStdout: "", + wantStderr: "no aliases configured\n", + }, + { + name: "some", + config: heredoc.Doc(` + aliases: + co: pr checkout + gc: "!gh gist create \"$@\" | pbcopy" + `), + isTTY: true, + wantStdout: "co: pr checkout\ngc: !gh gist create \"$@\" | pbcopy\n", + wantStderr: "", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // TODO: change underlying config implementation so Write is not + // automatically called when editing aliases in-memory + defer config.StubWriteConfig(ioutil.Discard, ioutil.Discard)() + + cfg := config.NewFromString(tt.config) + + io, _, stdout, stderr := iostreams.Test() + io.SetStdoutTTY(tt.isTTY) + io.SetStdinTTY(tt.isTTY) + io.SetStderrTTY(tt.isTTY) + + factory := &cmdutil.Factory{ + IOStreams: io, + Config: func() (config.Config, error) { + return cfg, nil + }, + } + + cmd := NewCmdList(factory, nil) + cmd.SetArgs([]string{}) + + cmd.SetIn(&bytes.Buffer{}) + cmd.SetOut(ioutil.Discard) + cmd.SetErr(ioutil.Discard) + + _, err := cmd.ExecuteC() + require.NoError(t, err) + + assert.Equal(t, tt.wantStdout, stdout.String()) + assert.Equal(t, tt.wantStderr, stderr.String()) + }) + } +} diff --git a/pkg/cmd/alias/set/set.go b/pkg/cmd/alias/set/set.go new file mode 100644 index 000000000..a9f628471 --- /dev/null +++ b/pkg/cmd/alias/set/set.go @@ -0,0 +1,147 @@ +package set + +import ( + "fmt" + "strings" + + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/internal/config" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/utils" + "github.com/google/shlex" + "github.com/spf13/cobra" +) + +type SetOptions struct { + Config func() (config.Config, error) + IO *iostreams.IOStreams + + Name string + Expansion string + IsShell bool + RootCmd *cobra.Command +} + +func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command { + opts := &SetOptions{ + IO: f.IOStreams, + Config: f.Config, + } + + cmd := &cobra.Command{ + Use: "set ", + Short: "Create a shortcut for a gh command", + Long: heredoc.Doc(` + Declare a word as a command alias that will expand to the specified command(s). + + The expansion may specify additional arguments and flags. If the expansion + includes positional placeholders such as '$1', '$2', etc., any extra arguments + that follow the invocation of an alias will be inserted appropriately. + + If '--shell' is specified, the alias will be run through a shell interpreter (sh). This allows you + to compose commands with "|" or redirect with ">". Note that extra arguments following the alias + will not be automatically passed to the expanded expression. To have a shell alias receive + arguments, you must explicitly accept them using "$1", "$2", etc., or "$@" to accept all of them. + + Platform note: on Windows, shell aliases are executed via "sh" as installed by Git For Windows. If + you have installed git on Windows in some other way, shell aliases may not work for you. + + Quotes must always be used when defining a command as in the examples. + `), + Example: heredoc.Doc(` + $ gh alias set pv 'pr view' + $ gh pv -w 123 + #=> gh pr view -w 123 + + $ gh alias set bugs 'issue list --label="bugs"' + + $ gh alias set epicsBy 'issue list --author="$1" --label="epic"' + $ gh epicsBy vilmibm + #=> gh issue list --author="vilmibm" --label="epic" + + $ gh alias set --shell igrep 'gh issue list --label="$1" | grep $2' + $ gh igrep epic foo + #=> gh issue list --label="epic" | grep "foo" + `), + Args: cobra.ExactArgs(2), + RunE: func(cmd *cobra.Command, args []string) error { + opts.RootCmd = cmd.Root() + + opts.Name = args[0] + opts.Expansion = args[1] + + if runF != nil { + return runF(opts) + } + return setRun(opts) + }, + } + + cmd.Flags().BoolVarP(&opts.IsShell, "shell", "s", false, "Declare an alias to be passed through a shell interpreter") + + return cmd +} + +func setRun(opts *SetOptions) error { + cfg, err := opts.Config() + if err != nil { + return err + } + + aliasCfg, err := cfg.Aliases() + if err != nil { + return err + } + + isTerminal := opts.IO.IsStdoutTTY() + if isTerminal { + fmt.Fprintf(opts.IO.ErrOut, "- Adding alias for %s: %s\n", utils.Bold(opts.Name), utils.Bold(opts.Expansion)) + } + + expansion := opts.Expansion + isShell := opts.IsShell + if isShell && !strings.HasPrefix(expansion, "!") { + expansion = "!" + expansion + } + isShell = strings.HasPrefix(expansion, "!") + + if validCommand(opts.RootCmd, opts.Name) { + return fmt.Errorf("could not create alias: %q is already a gh command", opts.Name) + } + + if !isShell && !validCommand(opts.RootCmd, expansion) { + return fmt.Errorf("could not create alias: %s does not correspond to a gh command", expansion) + } + + successMsg := fmt.Sprintf("%s Added alias.", utils.Green("✓")) + if oldExpansion, ok := aliasCfg.Get(opts.Name); ok { + successMsg = fmt.Sprintf("%s Changed alias %s from %s to %s", + utils.Green("✓"), + utils.Bold(opts.Name), + utils.Bold(oldExpansion), + utils.Bold(expansion), + ) + } + + err = aliasCfg.Add(opts.Name, expansion) + if err != nil { + return fmt.Errorf("could not create alias: %s", err) + } + + if isTerminal { + fmt.Fprintln(opts.IO.ErrOut, successMsg) + } + + return nil +} + +func validCommand(rootCmd *cobra.Command, expansion string) bool { + split, err := shlex.Split(expansion) + if err != nil { + return false + } + + cmd, _, err := rootCmd.Traverse(split) + return err == nil && cmd != rootCmd +} diff --git a/pkg/cmd/alias/set/set_test.go b/pkg/cmd/alias/set/set_test.go new file mode 100644 index 000000000..95f3a5031 --- /dev/null +++ b/pkg/cmd/alias/set/set_test.go @@ -0,0 +1,252 @@ +package set + +import ( + "bytes" + "io/ioutil" + "testing" + + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/internal/config" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/test" + "github.com/google/shlex" + "github.com/spf13/cobra" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func runCommand(cfg config.Config, isTTY bool, cli string) (*test.CmdOut, error) { + io, _, stdout, stderr := iostreams.Test() + io.SetStdoutTTY(isTTY) + io.SetStdinTTY(isTTY) + io.SetStderrTTY(isTTY) + + factory := &cmdutil.Factory{ + IOStreams: io, + Config: func() (config.Config, error) { + return cfg, nil + }, + } + + cmd := NewCmdSet(factory, nil) + + // fake command nesting structure needed for validCommand + rootCmd := &cobra.Command{} + rootCmd.AddCommand(cmd) + prCmd := &cobra.Command{Use: "pr"} + prCmd.AddCommand(&cobra.Command{Use: "checkout"}) + prCmd.AddCommand(&cobra.Command{Use: "status"}) + rootCmd.AddCommand(prCmd) + issueCmd := &cobra.Command{Use: "issue"} + issueCmd.AddCommand(&cobra.Command{Use: "list"}) + rootCmd.AddCommand(issueCmd) + + argv, err := shlex.Split("set " + cli) + if err != nil { + return nil, err + } + rootCmd.SetArgs(argv) + + rootCmd.SetIn(&bytes.Buffer{}) + rootCmd.SetOut(ioutil.Discard) + rootCmd.SetErr(ioutil.Discard) + + _, err = rootCmd.ExecuteC() + return &test.CmdOut{ + OutBuf: stdout, + ErrBuf: stderr, + }, err +} + +func TestAliasSet_gh_command(t *testing.T) { + defer config.StubWriteConfig(ioutil.Discard, ioutil.Discard)() + + cfg := config.NewFromString(``) + + _, err := runCommand(cfg, true, "pr 'pr status'") + + if assert.Error(t, err) { + assert.Equal(t, `could not create alias: "pr" is already a gh command`, err.Error()) + } +} + +func TestAliasSet_empty_aliases(t *testing.T) { + mainBuf := bytes.Buffer{} + defer config.StubWriteConfig(&mainBuf, ioutil.Discard)() + + cfg := config.NewFromString(heredoc.Doc(` + aliases: + editor: vim + `)) + + output, err := runCommand(cfg, true, "co 'pr checkout'") + + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + + test.ExpectLines(t, output.Stderr(), "Added alias") + test.ExpectLines(t, output.String(), "") + + expected := `aliases: + co: pr checkout +editor: vim +` + assert.Equal(t, expected, mainBuf.String()) +} + +func TestAliasSet_existing_alias(t *testing.T) { + mainBuf := bytes.Buffer{} + defer config.StubWriteConfig(&mainBuf, ioutil.Discard)() + + cfg := config.NewFromString(heredoc.Doc(` + aliases: + co: pr checkout + `)) + + output, err := runCommand(cfg, true, "co 'pr checkout -Rcool/repo'") + require.NoError(t, err) + + test.ExpectLines(t, output.Stderr(), "Changed alias.*co.*from.*pr checkout.*to.*pr checkout -Rcool/repo") +} + +func TestAliasSet_space_args(t *testing.T) { + mainBuf := bytes.Buffer{} + defer config.StubWriteConfig(&mainBuf, ioutil.Discard)() + + cfg := config.NewFromString(``) + + output, err := runCommand(cfg, true, `il 'issue list -l "cool story"'`) + require.NoError(t, err) + + test.ExpectLines(t, output.Stderr(), `Adding alias for.*il.*issue list -l "cool story"`) + + test.ExpectLines(t, mainBuf.String(), `il: issue list -l "cool story"`) +} + +func TestAliasSet_arg_processing(t *testing.T) { + cases := []struct { + Cmd string + ExpectedOutputLine string + ExpectedConfigLine string + }{ + {`il "issue list"`, "- Adding alias for.*il.*issue list", "il: issue list"}, + + {`iz 'issue list'`, "- Adding alias for.*iz.*issue list", "iz: issue list"}, + + {`ii 'issue list --author="$1" --label="$2"'`, + `- Adding alias for.*ii.*issue list --author="\$1" --label="\$2"`, + `ii: issue list --author="\$1" --label="\$2"`}, + + {`ix "issue list --author='\$1' --label='\$2'"`, + `- Adding alias for.*ix.*issue list --author='\$1' --label='\$2'`, + `ix: issue list --author='\$1' --label='\$2'`}, + } + + for _, c := range cases { + t.Run(c.Cmd, func(t *testing.T) { + mainBuf := bytes.Buffer{} + defer config.StubWriteConfig(&mainBuf, ioutil.Discard)() + + cfg := config.NewFromString(``) + + output, err := runCommand(cfg, true, c.Cmd) + if err != nil { + t.Fatalf("got unexpected error running %s: %s", c.Cmd, err) + } + + test.ExpectLines(t, output.Stderr(), c.ExpectedOutputLine) + test.ExpectLines(t, mainBuf.String(), c.ExpectedConfigLine) + }) + } +} + +func TestAliasSet_init_alias_cfg(t *testing.T) { + mainBuf := bytes.Buffer{} + defer config.StubWriteConfig(&mainBuf, ioutil.Discard)() + + cfg := config.NewFromString(heredoc.Doc(` + editor: vim + `)) + + output, err := runCommand(cfg, true, "diff 'pr diff'") + require.NoError(t, err) + + expected := `editor: vim +aliases: + diff: pr diff +` + + test.ExpectLines(t, output.Stderr(), "Adding alias for.*diff.*pr diff", "Added alias.") + assert.Equal(t, expected, mainBuf.String()) +} + +func TestAliasSet_existing_aliases(t *testing.T) { + mainBuf := bytes.Buffer{} + defer config.StubWriteConfig(&mainBuf, ioutil.Discard)() + + cfg := config.NewFromString(heredoc.Doc(` + aliases: + foo: bar + `)) + + output, err := runCommand(cfg, true, "view 'pr view'") + require.NoError(t, err) + + expected := `aliases: + foo: bar + view: pr view +` + + test.ExpectLines(t, output.Stderr(), "Adding alias for.*view.*pr view", "Added alias.") + assert.Equal(t, expected, mainBuf.String()) + +} + +func TestAliasSet_invalid_command(t *testing.T) { + defer config.StubWriteConfig(ioutil.Discard, ioutil.Discard)() + + cfg := config.NewFromString(``) + + _, err := runCommand(cfg, true, "co 'pe checkout'") + if assert.Error(t, err) { + assert.Equal(t, "could not create alias: pe checkout does not correspond to a gh command", err.Error()) + } +} + +func TestShellAlias_flag(t *testing.T) { + mainBuf := bytes.Buffer{} + defer config.StubWriteConfig(&mainBuf, ioutil.Discard)() + + cfg := config.NewFromString(``) + + output, err := runCommand(cfg, true, "--shell igrep 'gh issue list | grep'") + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + + test.ExpectLines(t, output.Stderr(), "Adding alias for.*igrep") + + expected := `aliases: + igrep: '!gh issue list | grep' +` + assert.Equal(t, expected, mainBuf.String()) +} + +func TestShellAlias_bang(t *testing.T) { + mainBuf := bytes.Buffer{} + defer config.StubWriteConfig(&mainBuf, ioutil.Discard)() + + cfg := config.NewFromString(``) + + output, err := runCommand(cfg, true, "igrep '!gh issue list | grep'") + require.NoError(t, err) + + test.ExpectLines(t, output.Stderr(), "Adding alias for.*igrep") + + expected := `aliases: + igrep: '!gh issue list | grep' +` + assert.Equal(t, expected, mainBuf.String()) +} diff --git a/pkg/cmd/root/root.go b/pkg/cmd/root/root.go index 15140800b..cd0321a5e 100644 --- a/pkg/cmd/root/root.go +++ b/pkg/cmd/root/root.go @@ -9,7 +9,9 @@ import ( "github.com/cli/cli/api" "github.com/cli/cli/context" "github.com/cli/cli/internal/ghrepo" + aliasCmd "github.com/cli/cli/pkg/cmd/alias" apiCmd "github.com/cli/cli/pkg/cmd/api" + configCmd "github.com/cli/cli/pkg/cmd/config" gistCmd "github.com/cli/cli/pkg/cmd/gist" issueCmd "github.com/cli/cli/pkg/cmd/issue" prCmd "github.com/cli/cli/pkg/cmd/pr" @@ -94,9 +96,12 @@ func NewCmdRoot(f *cmdutil.Factory, version, buildDate string) *cobra.Command { // CHILD COMMANDS + cmd.AddCommand(aliasCmd.NewCmdAlias(f)) cmd.AddCommand(apiCmd.NewCmdApi(f, nil)) - cmd.AddCommand(gistCmd.NewCmdGist(f)) + cmd.AddCommand(configCmd.NewCmdConfig(f)) cmd.AddCommand(creditsCmd.NewCmdCredits(f, nil)) + cmd.AddCommand(gistCmd.NewCmdGist(f)) + cmd.AddCommand(NewCmdCompletion(f.IOStreams)) // below here at the commands that require the "intelligent" BaseRepo resolver repoResolvingCmdFactory := *f diff --git a/test/helpers.go b/test/helpers.go index b10e87a85..edfa25bda 100644 --- a/test/helpers.go +++ b/test/helpers.go @@ -6,7 +6,6 @@ import ( "fmt" "os/exec" "regexp" - "testing" "github.com/cli/cli/internal/run" ) @@ -86,7 +85,13 @@ func createStubbedPrepareCmd(cs *CmdStubber) func(*exec.Cmd) run.Runnable { } } -func ExpectLines(t *testing.T, output string, lines ...string) { +type T interface { + Helper() + Errorf(string, ...interface{}) +} + +func ExpectLines(t T, output string, lines ...string) { + t.Helper() var r *regexp.Regexp for _, l := range lines { r = regexp.MustCompile(l) From 2b9de23637b300c41c7228566a7caa8d1d78865f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mislav=20Marohni=C4=87?= Date: Tue, 11 Aug 2020 15:18:48 +0200 Subject: [PATCH 11/11] Extract alias expansion into a separate package --- cmd/gen-docs/main.go | 11 +- cmd/gh/main.go | 43 +++++-- command/alias_test.go | 109 ---------------- command/root.go | 128 ------------------- command/testing.go | 119 ------------------ pkg/cmd/alias/expand/expand.go | 106 ++++++++++++++++ pkg/cmd/alias/expand/expand_test.go | 185 ++++++++++++++++++++++++++++ 7 files changed, 329 insertions(+), 372 deletions(-) delete mode 100644 command/alias_test.go delete mode 100644 command/testing.go create mode 100644 pkg/cmd/alias/expand/expand.go create mode 100644 pkg/cmd/alias/expand/expand_test.go diff --git a/cmd/gen-docs/main.go b/cmd/gen-docs/main.go index 92581d07a..631185393 100644 --- a/cmd/gen-docs/main.go +++ b/cmd/gen-docs/main.go @@ -5,7 +5,9 @@ import ( "os" "strings" - "github.com/cli/cli/command" + "github.com/cli/cli/pkg/cmd/root" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/iostreams" "github.com/spf13/cobra/doc" "github.com/spf13/pflag" ) @@ -35,13 +37,16 @@ func main() { fatal("no dir set") } + io, _, _, _ := iostreams.Test() + rootCmd := root.NewCmdRoot(&cmdutil.Factory{IOStreams: io}, "", "") + err := os.MkdirAll(*dir, 0755) if err != nil { fatal(err) } if *website { - err = doc.GenMarkdownTreeCustom(command.RootCmd, *dir, filePrepender, linkHandler) + err = doc.GenMarkdownTreeCustom(rootCmd, *dir, filePrepender, linkHandler) if err != nil { fatal(err) } @@ -54,7 +59,7 @@ func main() { Source: "", //source and manual are just put at the top of the manpage, before name Manual: "", //if source is an empty string, it's set to "Auto generated by spf13/cobra" } - err = doc.GenManTree(command.RootCmd, header, *dir) + err = doc.GenManTree(rootCmd, header, *dir) if err != nil { fatal(err) } diff --git a/cmd/gh/main.go b/cmd/gh/main.go index 4d801f008..3e92b3b0d 100644 --- a/cmd/gh/main.go +++ b/cmd/gh/main.go @@ -12,6 +12,9 @@ import ( "github.com/cli/cli/command" "github.com/cli/cli/internal/config" + "github.com/cli/cli/internal/run" + "github.com/cli/cli/pkg/cmd/alias/expand" + "github.com/cli/cli/pkg/cmd/factory" "github.com/cli/cli/pkg/cmd/root" "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/update" @@ -32,25 +35,44 @@ func main() { hasDebug := os.Getenv("DEBUG") != "" - stderr := utils.NewColorable(os.Stderr) + cmdFactory := factory.New(command.Version) + stderr := cmdFactory.IOStreams.ErrOut + rootCmd := root.NewCmdRoot(cmdFactory, command.Version, command.BuildDate) expandedArgs := []string{} if len(os.Args) > 0 { expandedArgs = os.Args[1:] } - cmd, _, err := command.RootCmd.Traverse(expandedArgs) - if err != nil || cmd == command.RootCmd { + cmd, _, err := rootCmd.Traverse(expandedArgs) + if err != nil || cmd == rootCmd { originalArgs := expandedArgs isShell := false - expandedArgs, isShell, err = command.ExpandAlias(os.Args) + + cfg, err := cmdFactory.Config() + if err != nil { + fmt.Fprintf(stderr, "failed to read configuration: %s\n", err) + os.Exit(2) + } + + expandedArgs, isShell, err = expand.ExpandAlias(cfg, os.Args, nil) if err != nil { fmt.Fprintf(stderr, "failed to process aliases: %s\n", err) os.Exit(2) } + if hasDebug { + fmt.Fprintf(stderr, "%v -> %v\n", originalArgs, expandedArgs) + } + if isShell { - err = command.ExecuteShellAlias(expandedArgs) + externalCmd := exec.Command(expandedArgs[0], expandedArgs[1:]...) + externalCmd.Stderr = os.Stderr + externalCmd.Stdout = os.Stdout + externalCmd.Stdin = os.Stdin + preparedCmd := run.PrepareCmd(externalCmd) + + err = preparedCmd.Run() if err != nil { if ee, ok := err.(*exec.ExitError); ok { os.Exit(ee.ExitCode()) @@ -62,16 +84,12 @@ func main() { os.Exit(0) } - - if hasDebug { - fmt.Fprintf(stderr, "%v -> %v\n", originalArgs, expandedArgs) - } } - command.RootCmd.SetArgs(expandedArgs) + rootCmd.SetArgs(expandedArgs) - if cmd, err := command.RootCmd.ExecuteC(); err != nil { - printError(os.Stderr, err, cmd, hasDebug) + if cmd, err := rootCmd.ExecuteC(); err != nil { + printError(stderr, err, cmd, hasDebug) os.Exit(1) } if root.HasFailed() { @@ -86,7 +104,6 @@ func main() { ansi.Color(newRelease.Version, "cyan"), ansi.Color(newRelease.URL, "yellow")) - stderr := utils.NewColorable(os.Stderr) fmt.Fprintf(stderr, "\n\n%s\n\n", msg) } } diff --git a/command/alias_test.go b/command/alias_test.go deleted file mode 100644 index 7bb76f553..000000000 --- a/command/alias_test.go +++ /dev/null @@ -1,109 +0,0 @@ -package command - -import ( - "strings" - "testing" - - "github.com/stretchr/testify/assert" -) - -func stubSh(value string) func() { - orig := findSh - findSh = func() (string, error) { - return value, nil - } - return func() { - findSh = orig - } -} - -func TestExpandAlias_shell(t *testing.T) { - defer stubSh("sh")() - cfg := `--- -aliases: - ig: '!gh issue list | grep cool' -` - initBlankContext(cfg, "OWNER/REPO", "trunk") - - expanded, isShell, err := ExpandAlias([]string{"gh", "ig"}) - - assert.True(t, isShell) - - if err != nil { - t.Fatalf("unexpected error: %s", err) - } - - expected := []string{"sh", "-c", "gh issue list | grep cool"} - - assert.Equal(t, expected, expanded) -} - -func TestExpandAlias_shell_extra_args(t *testing.T) { - defer stubSh("sh")() - cfg := `--- -aliases: - ig: '!gh issue list --label=$1 | grep' -` - initBlankContext(cfg, "OWNER/REPO", "trunk") - - expanded, isShell, err := ExpandAlias([]string{"gh", "ig", "bug", "foo"}) - - assert.True(t, isShell) - - if err != nil { - t.Fatalf("unexpected error: %s", err) - } - - expected := []string{"sh", "-c", "gh issue list --label=$1 | grep", "--", "bug", "foo"} - - assert.Equal(t, expected, expanded) -} - -func TestExpandAlias(t *testing.T) { - cfg := `--- -aliases: - co: pr checkout - il: issue list --author="$1" --label="$2" - ia: issue list --author="$1" --assignee="$1" -` - initBlankContext(cfg, "OWNER/REPO", "trunk") - for _, c := range []struct { - Args string - ExpectedArgs []string - Err string - }{ - {"gh co", []string{"pr", "checkout"}, ""}, - {"gh il", nil, `not enough arguments for alias: issue list --author="$1" --label="$2"`}, - {"gh il vilmibm", nil, `not enough arguments for alias: issue list --author="vilmibm" --label="$2"`}, - {"gh co 123", []string{"pr", "checkout", "123"}, ""}, - {"gh il vilmibm epic", []string{"issue", "list", `--author=vilmibm`, `--label=epic`}, ""}, - {"gh ia vilmibm", []string{"issue", "list", `--author=vilmibm`, `--assignee=vilmibm`}, ""}, - {"gh ia $coolmoney$", []string{"issue", "list", `--author=$coolmoney$`, `--assignee=$coolmoney$`}, ""}, - {"gh pr status", []string{"pr", "status"}, ""}, - {"gh il vilmibm epic -R vilmibm/testing", []string{"issue", "list", "--author=vilmibm", "--label=epic", "-R", "vilmibm/testing"}, ""}, - {"gh dne", []string{"dne"}, ""}, - {"gh", []string{}, ""}, - {"", []string{}, ""}, - } { - args := []string{} - if c.Args != "" { - args = strings.Split(c.Args, " ") - } - - expanded, isShell, err := ExpandAlias(args) - - assert.False(t, isShell) - - if err == nil && c.Err != "" { - t.Errorf("expected error %s for %s", c.Err, c.Args) - continue - } - - if err != nil { - eq(t, err.Error(), c.Err) - continue - } - - assert.Equal(t, c.ExpectedArgs, expanded) - } -} diff --git a/command/root.go b/command/root.go index 1ff53e783..5e5260793 100644 --- a/command/root.go +++ b/command/root.go @@ -1,26 +1,15 @@ package command import ( - "errors" "fmt" "os" - "os/exec" - "path/filepath" - "regexp" - "runtime" "runtime/debug" "strings" "github.com/cli/cli/api" - "github.com/cli/cli/context" "github.com/cli/cli/internal/config" "github.com/cli/cli/internal/ghinstance" - "github.com/cli/cli/internal/run" - "github.com/cli/cli/pkg/cmd/factory" - "github.com/cli/cli/pkg/cmd/root" "github.com/cli/cli/utils" - "github.com/google/shlex" - "github.com/spf13/cobra" ) // Version is dynamically set by the toolchain or overridden by the Makefile. @@ -29,22 +18,12 @@ var Version = "DEV" // BuildDate is dynamically set at build time in the Makefile. var BuildDate = "" // YYYY-MM-DD -var RootCmd *cobra.Command - func init() { if Version == "DEV" { if info, ok := debug.ReadBuildInfo(); ok && info.Main.Version != "(devel)" { Version = info.Main.Version } } - - cmdFactory := factory.New(Version) - RootCmd = root.NewCmdRoot(cmdFactory, Version, BuildDate) -} - -// overridden in tests -var initContext = func() context.Context { - return context.New() } // BasicClient returns an API client for github.com only that borrows from but @@ -73,110 +52,3 @@ func apiVerboseLog() api.ClientOption { colorize := utils.IsTerminal(os.Stderr) return api.VerboseLog(utils.NewColorable(os.Stderr), logTraffic, colorize) } - -func ExecuteShellAlias(args []string) error { - externalCmd := exec.Command(args[0], args[1:]...) - externalCmd.Stderr = os.Stderr - externalCmd.Stdout = os.Stdout - externalCmd.Stdin = os.Stdin - preparedCmd := run.PrepareCmd(externalCmd) - - return preparedCmd.Run() -} - -var findSh = func() (string, error) { - shPath, err := exec.LookPath("sh") - if err == nil { - return shPath, nil - } - - if runtime.GOOS == "windows" { - winNotFoundErr := errors.New("unable to locate sh to execute the shell alias with. The sh.exe interpreter is typically distributed with Git for Windows.") - // We can try and find a sh executable in a Git for Windows install - gitPath, err := exec.LookPath("git") - if err != nil { - return "", winNotFoundErr - } - - shPath = filepath.Join(filepath.Dir(gitPath), "..", "bin", "sh.exe") - _, err = os.Stat(shPath) - if err != nil { - return "", winNotFoundErr - } - - return shPath, nil - } - - return "", errors.New("unable to locate sh to execute shell alias with") -} - -// ExpandAlias processes argv to see if it should be rewritten according to a user's aliases. The -// second return value indicates whether the alias should be executed in a new shell process instead -// of running gh itself. -func ExpandAlias(args []string) (expanded []string, isShell bool, err error) { - err = nil - isShell = false - expanded = []string{} - - if len(args) < 2 { - // the command is lacking a subcommand - return - } - - ctx := initContext() - cfg, err := ctx.Config() - if err != nil { - return - } - aliases, err := cfg.Aliases() - if err != nil { - return - } - - expansion, ok := aliases.Get(args[1]) - if ok { - if strings.HasPrefix(expansion, "!") { - isShell = true - shPath, shErr := findSh() - if shErr != nil { - err = shErr - return - } - - expanded = []string{shPath, "-c", expansion[1:]} - - if len(args[2:]) > 0 { - expanded = append(expanded, "--") - expanded = append(expanded, args[2:]...) - } - - return - } - - extraArgs := []string{} - for i, a := range args[2:] { - if !strings.Contains(expansion, "$") { - extraArgs = append(extraArgs, a) - } else { - expansion = strings.ReplaceAll(expansion, fmt.Sprintf("$%d", i+1), a) - } - } - lingeringRE := regexp.MustCompile(`\$\d`) - if lingeringRE.MatchString(expansion) { - err = fmt.Errorf("not enough arguments for alias: %s", expansion) - return - } - - var newArgs []string - newArgs, err = shlex.Split(expansion) - if err != nil { - return - } - - expanded = append(newArgs, extraArgs...) - return - } - - expanded = args[1:] - return -} diff --git a/command/testing.go b/command/testing.go deleted file mode 100644 index 0668776c8..000000000 --- a/command/testing.go +++ /dev/null @@ -1,119 +0,0 @@ -package command - -import ( - "bytes" - "fmt" - "reflect" - "testing" - - "github.com/cli/cli/context" - "github.com/cli/cli/internal/config" - "github.com/cli/cli/utils" - "github.com/google/shlex" - "github.com/spf13/pflag" -) - -func eq(t *testing.T, got interface{}, expected interface{}) { - t.Helper() - if !reflect.DeepEqual(got, expected) { - t.Errorf("expected: %v, got: %v", expected, got) - } -} - -const defaultTestConfig = `hosts: - github.com: - user: OWNER - oauth_token: "1234567890" -` - -func initBlankContext(cfg, repo, branch string) { - initContext = func() context.Context { - ctx := context.NewBlank() - - if cfg == "" { - cfg = defaultTestConfig - } - - // NOTE we are not restoring the original readConfig; we never want to touch the config file on - // disk during tests. - config.StubConfig(cfg, "") - - return ctx - } -} - -type cmdOut struct { - outBuf, errBuf *bytes.Buffer -} - -func (c cmdOut) String() string { - return c.outBuf.String() -} - -func (c cmdOut) Stderr() string { - return c.errBuf.String() -} - -func RunCommand(args string) (*cmdOut, error) { - rootCmd := RootCmd - rootArgv, err := shlex.Split(args) - if err != nil { - return nil, err - } - - cmd, _, err := rootCmd.Traverse(rootArgv) - if err != nil { - return nil, err - } - - rootCmd.SetArgs(rootArgv) - - outBuf := bytes.Buffer{} - cmd.SetOut(&outBuf) - errBuf := bytes.Buffer{} - cmd.SetErr(&errBuf) - - // Reset flag values so they don't leak between tests - // FIXME: change how we initialize Cobra commands to render this hack unnecessary - cmd.Flags().VisitAll(func(f *pflag.Flag) { - f.Changed = false - switch v := f.Value.(type) { - case pflag.SliceValue: - _ = v.Replace([]string{}) - default: - switch v.Type() { - case "bool", "string", "int": - _ = v.Set(f.DefValue) - } - } - }) - - _, err = rootCmd.ExecuteC() - cmd.SetOut(nil) - cmd.SetErr(nil) - - return &cmdOut{&outBuf, &errBuf}, err -} - -func stubTerminal(connected bool) func() { - isTerminal := utils.IsTerminal - utils.IsTerminal = func(_ interface{}) bool { - return connected - } - - terminalSize := utils.TerminalSize - if connected { - utils.TerminalSize = func(_ interface{}) (int, int, error) { - return 80, 20, nil - } - } else { - utils.TerminalSize = func(_ interface{}) (int, int, error) { - return 0, 0, fmt.Errorf("terminal connection stubbed to false") - } - } - - return func() { - utils.IsTerminal = isTerminal - utils.TerminalSize = terminalSize - } -} diff --git a/pkg/cmd/alias/expand/expand.go b/pkg/cmd/alias/expand/expand.go new file mode 100644 index 000000000..b2117f198 --- /dev/null +++ b/pkg/cmd/alias/expand/expand.go @@ -0,0 +1,106 @@ +package expand + +import ( + "errors" + "fmt" + "os" + "os/exec" + "path/filepath" + "regexp" + "runtime" + "strings" + + "github.com/cli/cli/internal/config" + "github.com/google/shlex" +) + +// ExpandAlias processes argv to see if it should be rewritten according to a user's aliases. The +// second return value indicates whether the alias should be executed in a new shell process instead +// of running gh itself. +func ExpandAlias(cfg config.Config, args []string, findShFunc func() (string, error)) (expanded []string, isShell bool, err error) { + if len(args) < 2 { + // the command is lacking a subcommand + return + } + expanded = args[1:] + + aliases, err := cfg.Aliases() + if err != nil { + return + } + + expansion, ok := aliases.Get(args[1]) + if !ok { + return + } + + if strings.HasPrefix(expansion, "!") { + isShell = true + if findShFunc == nil { + findShFunc = findSh + } + shPath, shErr := findShFunc() + if shErr != nil { + err = shErr + return + } + + expanded = []string{shPath, "-c", expansion[1:]} + + if len(args[2:]) > 0 { + expanded = append(expanded, "--") + expanded = append(expanded, args[2:]...) + } + + return + } + + extraArgs := []string{} + for i, a := range args[2:] { + if !strings.Contains(expansion, "$") { + extraArgs = append(extraArgs, a) + } else { + expansion = strings.ReplaceAll(expansion, fmt.Sprintf("$%d", i+1), a) + } + } + lingeringRE := regexp.MustCompile(`\$\d`) + if lingeringRE.MatchString(expansion) { + err = fmt.Errorf("not enough arguments for alias: %s", expansion) + return + } + + var newArgs []string + newArgs, err = shlex.Split(expansion) + if err != nil { + return + } + + expanded = append(newArgs, extraArgs...) + return +} + +func findSh() (string, error) { + shPath, err := exec.LookPath("sh") + if err == nil { + return shPath, nil + } + + if runtime.GOOS == "windows" { + winNotFoundErr := errors.New("unable to locate sh to execute the shell alias with. The sh.exe interpreter is typically distributed with Git for Windows.") + // We can try and find a sh executable in a Git for Windows install + gitPath, err := exec.LookPath("git") + if err != nil { + return "", winNotFoundErr + } + + shPath = filepath.Join(filepath.Dir(gitPath), "..", "bin", "sh.exe") + _, err = os.Stat(shPath) + if err != nil { + return "", winNotFoundErr + } + + return shPath, nil + } + + return "", errors.New("unable to locate sh to execute shell alias with") +} diff --git a/pkg/cmd/alias/expand/expand_test.go b/pkg/cmd/alias/expand/expand_test.go new file mode 100644 index 000000000..b9535f7c7 --- /dev/null +++ b/pkg/cmd/alias/expand/expand_test.go @@ -0,0 +1,185 @@ +package expand + +import ( + "errors" + "reflect" + "testing" + + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/internal/config" +) + +func TestExpandAlias(t *testing.T) { + findShFunc := func() (string, error) { + return "/usr/bin/sh", nil + } + + cfg := config.NewFromString(heredoc.Doc(` + aliases: + co: pr checkout + il: issue list --author="$1" --label="$2" + ia: issue list --author="$1" --assignee="$1" + `)) + + type args struct { + config config.Config + argv []string + } + tests := []struct { + name string + args args + wantExpanded []string + wantIsShell bool + wantErr error + }{ + { + name: "no arguments", + args: args{ + config: cfg, + argv: []string{}, + }, + wantExpanded: []string(nil), + wantIsShell: false, + wantErr: nil, + }, + { + name: "too few arguments", + args: args{ + config: cfg, + argv: []string{"gh"}, + }, + wantExpanded: []string(nil), + wantIsShell: false, + wantErr: nil, + }, + { + name: "no expansion", + args: args{ + config: cfg, + argv: []string{"gh", "pr", "status"}, + }, + wantExpanded: []string{"pr", "status"}, + wantIsShell: false, + wantErr: nil, + }, + { + name: "simple expansion", + args: args{ + config: cfg, + argv: []string{"gh", "co"}, + }, + wantExpanded: []string{"pr", "checkout"}, + wantIsShell: false, + wantErr: nil, + }, + { + name: "adding arguments after expansion", + args: args{ + config: cfg, + argv: []string{"gh", "co", "123"}, + }, + wantExpanded: []string{"pr", "checkout", "123"}, + wantIsShell: false, + wantErr: nil, + }, + { + name: "not enough arguments for expansion", + args: args{ + config: cfg, + argv: []string{"gh", "il"}, + }, + wantExpanded: []string{}, + wantIsShell: false, + wantErr: errors.New(`not enough arguments for alias: issue list --author="$1" --label="$2"`), + }, + { + name: "not enough arguments for expansion 2", + args: args{ + config: cfg, + argv: []string{"gh", "il", "vilmibm"}, + }, + wantExpanded: []string{}, + wantIsShell: false, + wantErr: errors.New(`not enough arguments for alias: issue list --author="vilmibm" --label="$2"`), + }, + { + name: "satisfy expansion arguments", + args: args{ + config: cfg, + argv: []string{"gh", "il", "vilmibm", "help wanted"}, + }, + wantExpanded: []string{"issue", "list", "--author=vilmibm", "--label=help wanted"}, + wantIsShell: false, + wantErr: nil, + }, + { + name: "mixed positional and non-positional arguments", + args: args{ + config: cfg, + argv: []string{"gh", "il", "vilmibm", "epic", "-R", "monalisa/testing"}, + }, + wantExpanded: []string{"issue", "list", "--author=vilmibm", "--label=epic", "-R", "monalisa/testing"}, + wantIsShell: false, + wantErr: nil, + }, + { + name: "dollar in expansion", + args: args{ + config: cfg, + argv: []string{"gh", "ia", "$coolmoney$"}, + }, + wantExpanded: []string{"issue", "list", "--author=$coolmoney$", "--assignee=$coolmoney$"}, + wantIsShell: false, + wantErr: nil, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotExpanded, gotIsShell, err := ExpandAlias(tt.args.config, tt.args.argv, findShFunc) + if tt.wantErr != nil { + if err == nil { + t.Fatal("expected error") + } + if tt.wantErr.Error() != err.Error() { + t.Fatalf("expected error %q, got %q", tt.wantErr, err) + } + return + } + if err != nil { + t.Fatalf("got error: %v", err) + } + if !reflect.DeepEqual(gotExpanded, tt.wantExpanded) { + t.Errorf("ExpandAlias() gotExpanded = %v, want %v", gotExpanded, tt.wantExpanded) + } + if gotIsShell != tt.wantIsShell { + t.Errorf("ExpandAlias() gotIsShell = %v, want %v", gotIsShell, tt.wantIsShell) + } + }) + } +} + +// cfg := `--- +// aliases: +// co: pr checkout +// il: issue list --author="$1" --label="$2" +// ia: issue list --author="$1" --assignee="$1" +// ` +// initBlankContext(cfg, "OWNER/REPO", "trunk") +// for _, c := range []struct { +// Args string +// ExpectedArgs []string +// Err string +// }{ +// {"gh co", []string{"pr", "checkout"}, ""}, +// {"gh il", nil, `not enough arguments for alias: issue list --author="$1" --label="$2"`}, +// {"gh il vilmibm", nil, `not enough arguments for alias: issue list --author="vilmibm" --label="$2"`}, +// {"gh co 123", []string{"pr", "checkout", "123"}, ""}, +// {"gh il vilmibm epic", []string{"issue", "list", `--author=vilmibm`, `--label=epic`}, ""}, +// {"gh ia vilmibm", []string{"issue", "list", `--author=vilmibm`, `--assignee=vilmibm`}, ""}, +// {"gh ia $coolmoney$", []string{"issue", "list", `--author=$coolmoney$`, `--assignee=$coolmoney$`}, ""}, +// {"gh pr status", []string{"pr", "status"}, ""}, +// {"gh il vilmibm epic -R vilmibm/testing", []string{"issue", "list", "--author=vilmibm", "--label=epic", "-R", "vilmibm/testing"}, ""}, +// {"gh dne", []string{"dne"}, ""}, +// {"gh", []string{}, ""}, +// {"", []string{}, ""}, +// } {