diff --git a/command/issue.go b/command/issue.go index 93c77eb80..663d50690 100644 --- a/command/issue.go +++ b/command/issue.go @@ -15,6 +15,7 @@ import ( "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/text" "github.com/cli/cli/utils" "github.com/spf13/cobra" "github.com/spf13/pflag" @@ -298,28 +299,28 @@ func issueStatus(cmd *cobra.Command, args []string) error { fmt.Fprintf(out, "Relevant issues in %s\n", ghrepo.FullName(baseRepo)) fmt.Fprintln(out, "") - printHeader(out, "Issues assigned to you") + 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" - printMessage(out, message) + shared.PrintMessage(out, message) } fmt.Fprintln(out) - printHeader(out, "Issues mentioning you") + shared.PrintHeader(out, "Issues mentioning you") if issuePayload.Mentioned.TotalCount > 0 { printIssues(out, " ", issuePayload.Mentioned.TotalCount, issuePayload.Mentioned.Issues) } else { - printMessage(out, " There are no issues mentioning you") + shared.PrintMessage(out, " There are no issues mentioning you") } fmt.Fprintln(out) - printHeader(out, "Issues opened by you") + shared.PrintHeader(out, "Issues opened by you") if issuePayload.Authored.TotalCount > 0 { printIssues(out, " ", issuePayload.Authored.TotalCount, issuePayload.Authored.Issues) } else { - printMessage(out, " There are no issues opened by you") + shared.PrintMessage(out, " There are no issues opened by you") } fmt.Fprintln(out) @@ -713,7 +714,7 @@ func printIssues(w io.Writer, prefix string, totalCount int, issues []api.Issue) if !table.IsTTY() { table.AddField(issue.State, nil, nil) } - table.AddField(replaceExcessiveWhitespace(issue.Title), 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) diff --git a/command/pr.go b/command/pr.go index e5ff19ebd..c5ba3144e 100644 --- a/command/pr.go +++ b/command/pr.go @@ -1,17 +1,11 @@ package command import ( - "errors" "fmt" - "io" - "regexp" "strconv" - "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/ghrepo" "github.com/cli/cli/pkg/cmd/pr/shared" "github.com/cli/cli/pkg/cmdutil" @@ -26,7 +20,6 @@ func init() { RootCmd.AddCommand(prCmd) prCmd.AddCommand(prCreateCmd) - prCmd.AddCommand(prStatusCmd) prCmd.AddCommand(prCloseCmd) prCmd.AddCommand(prReopenCmd) prCmd.AddCommand(prReadyCmd) @@ -68,12 +61,6 @@ var prListCmd = &cobra.Command{ `), RunE: prList, } -var prStatusCmd = &cobra.Command{ - Use: "status", - Short: "Show status of relevant pull requests", - Args: cmdutil.NoArgsQuoteReminder, - RunE: prStatus, -} var prCloseCmd = &cobra.Command{ Use: "close { | | }", Short: "Close a pull request", @@ -93,72 +80,6 @@ var prReadyCmd = &cobra.Command{ RunE: prReady, } -func prStatus(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 - } - - repoOverride, _ := cmd.Flags().GetString("repo") - currentPRNumber, currentPRHeadRef, err := prSelectorForCurrentBranch(ctx, baseRepo) - - if err != nil && repoOverride == "" && !errors.Is(err, git.ErrNotOnAnyBranch) { - return fmt.Errorf("could not query for pull request for current branch: %w", err) - } - - // the `@me` macro is available because the API lookup is ElasticSearch-based - currentUser := "@me" - prPayload, err := api.PullRequests(apiClient, baseRepo, currentPRNumber, currentPRHeadRef, currentUser) - if err != nil { - return err - } - - out := colorableOut(cmd) - - fmt.Fprintln(out, "") - fmt.Fprintf(out, "Relevant pull requests in %s\n", ghrepo.FullName(baseRepo)) - fmt.Fprintln(out, "") - - printHeader(out, "Current branch") - currentPR := prPayload.CurrentPR - currentBranch, _ := ctx.Branch() - if currentPR != nil && currentPR.State != "OPEN" && prPayload.DefaultBranch == currentBranch { - currentPR = nil - } - if currentPR != nil { - printPrs(out, 1, *currentPR) - } else if currentPRHeadRef == "" { - printMessage(out, " There is no current branch") - } else { - printMessage(out, fmt.Sprintf(" There is no pull request associated with %s", utils.Cyan("["+currentPRHeadRef+"]"))) - } - fmt.Fprintln(out) - - printHeader(out, "Created by you") - if prPayload.ViewerCreated.TotalCount > 0 { - printPrs(out, prPayload.ViewerCreated.TotalCount, prPayload.ViewerCreated.PullRequests...) - } else { - printMessage(out, " You have no open pull requests") - } - fmt.Fprintln(out) - - printHeader(out, "Requesting a code review from you") - if prPayload.ReviewRequested.TotalCount > 0 { - printPrs(out, prPayload.ReviewRequested.TotalCount, prPayload.ReviewRequested.PullRequests...) - } else { - printMessage(out, " You have no pull requests to review") - } - fmt.Fprintln(out) - - return nil -} - func prList(cmd *cobra.Command, args []string) error { ctx := contextForCommand(cmd) apiClient, err := apiClientForContext(ctx) @@ -271,7 +192,7 @@ func prList(cmd *cobra.Command, args []string) error { prNum = "#" + prNum } table.AddField(prNum, nil, shared.ColorFuncForPR(pr)) - table.AddField(replaceExcessiveWhitespace(pr.Title), nil, nil) + table.AddField(text.ReplaceExcessiveWhitespace(pr.Title), nil, nil) table.AddField(pr.HeadLabel(), nil, utils.Cyan) if !table.IsTTY() { table.AddField(prStateWithDraft(&pr), nil, nil) @@ -385,124 +306,3 @@ func prReady(cmd *cobra.Command, args []string) error { return nil } - -func prSelectorForCurrentBranch(ctx context.Context, baseRepo ghrepo.Interface) (prNumber int, prHeadRef string, err error) { - prHeadRef, err = ctx.Branch() - if err != nil { - return - } - branchConfig := git.ReadBranchConfig(prHeadRef) - - // the branch is configured to merge a special PR head ref - prHeadRE := regexp.MustCompile(`^refs/pull/(\d+)/head$`) - if m := prHeadRE.FindStringSubmatch(branchConfig.MergeRef); m != nil { - prNumber, _ = strconv.Atoi(m[1]) - return - } - - var branchOwner string - if branchConfig.RemoteURL != nil { - // the branch merges from a remote specified by URL - if r, err := ghrepo.FromURL(branchConfig.RemoteURL); err == nil { - branchOwner = r.RepoOwner() - } - } else if branchConfig.RemoteName != "" { - // the branch merges from a remote specified by name - rem, _ := ctx.Remotes() - if r, err := rem.FindByName(branchConfig.RemoteName); err == nil { - branchOwner = r.RepoOwner() - } - } - - if branchOwner != "" { - if strings.HasPrefix(branchConfig.MergeRef, "refs/heads/") { - prHeadRef = strings.TrimPrefix(branchConfig.MergeRef, "refs/heads/") - } - // prepend `OWNER:` if this branch is pushed to a fork - if !strings.EqualFold(branchOwner, baseRepo.RepoOwner()) { - prHeadRef = fmt.Sprintf("%s:%s", branchOwner, prHeadRef) - } - } - - return -} - -func printPrs(w io.Writer, totalCount int, prs ...api.PullRequest) { - for _, pr := range prs { - prNumber := fmt.Sprintf("#%d", pr.Number) - - prStateColorFunc := utils.Green - if pr.IsDraft { - prStateColorFunc = utils.Gray - } else if pr.State == "MERGED" { - prStateColorFunc = utils.Magenta - } else if pr.State == "CLOSED" { - prStateColorFunc = utils.Red - } - - fmt.Fprintf(w, " %s %s %s", prStateColorFunc(prNumber), text.Truncate(50, replaceExcessiveWhitespace(pr.Title)), utils.Cyan("["+pr.HeadLabel()+"]")) - - checks := pr.ChecksStatus() - reviews := pr.ReviewStatus() - - if pr.State == "OPEN" { - reviewStatus := reviews.ChangesRequested || reviews.Approved || reviews.ReviewRequired - if checks.Total > 0 || reviewStatus { - // show checks & reviews on their own line - fmt.Fprintf(w, "\n ") - } - - if checks.Total > 0 { - var summary string - if checks.Failing > 0 { - if checks.Failing == checks.Total { - summary = utils.Red("× All checks failing") - } else { - summary = utils.Red(fmt.Sprintf("× %d/%d checks failing", checks.Failing, checks.Total)) - } - } else if checks.Pending > 0 { - summary = utils.Yellow("- Checks pending") - } else if checks.Passing == checks.Total { - summary = utils.Green("✓ Checks passing") - } - fmt.Fprint(w, summary) - } - - if checks.Total > 0 && reviewStatus { - // add padding between checks & reviews - fmt.Fprint(w, " ") - } - - if reviews.ChangesRequested { - fmt.Fprint(w, utils.Red("+ Changes requested")) - } else if reviews.ReviewRequired { - fmt.Fprint(w, utils.Yellow("- Review required")) - } else if reviews.Approved { - fmt.Fprint(w, utils.Green("✓ Approved")) - } - } else { - fmt.Fprintf(w, " - %s", shared.StateTitleWithColor(pr)) - } - - fmt.Fprint(w, "\n") - } - remaining := totalCount - len(prs) - if remaining > 0 { - fmt.Fprintf(w, utils.Gray(" And %d more\n"), remaining) - } -} - -func printHeader(w io.Writer, s string) { - fmt.Fprintln(w, utils.Bold(s)) -} - -func printMessage(w io.Writer, s string) { - fmt.Fprintln(w, utils.Gray(s)) -} - -func replaceExcessiveWhitespace(s string) string { - s = strings.TrimSpace(s) - s = regexp.MustCompile(`\r?\n`).ReplaceAllString(s, " ") - s = regexp.MustCompile(`\s{2,}`).ReplaceAllString(s, " ") - return s -} diff --git a/command/pr_test.go b/command/pr_test.go index 0b49911fa..e22df501c 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -21,276 +21,6 @@ func eq(t *testing.T, got interface{}, expected interface{}) { } } -func TestPRStatus(t *testing.T) { - initBlankContext("", "OWNER/REPO", "blueberries") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.FileResponse("../test/fixtures/prStatus.json")) - - output, err := RunCommand("pr status") - if err != nil { - t.Errorf("error running command `pr status`: %v", err) - } - - expectedPrs := []*regexp.Regexp{ - regexp.MustCompile(`#8.*\[strawberries\]`), - regexp.MustCompile(`#9.*\[apples\]`), - regexp.MustCompile(`#10.*\[blueberries\]`), - regexp.MustCompile(`#11.*\[figs\]`), - } - - for _, r := range expectedPrs { - if !r.MatchString(output.String()) { - t.Errorf("output did not match regexp /%s/", r) - } - } -} - -func TestPRStatus_fork(t *testing.T) { - initBlankContext("", "OWNER/REPO", "blueberries") - http := initFakeHTTP() - http.StubForkedRepoResponse("OWNER/REPO", "PARENT/REPO") - http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.FileResponse("../test/fixtures/prStatusFork.json")) - - defer run.SetPrepareCmd(func(cmd *exec.Cmd) run.Runnable { - switch strings.Join(cmd.Args, " ") { - case `git config --get-regexp ^branch\.blueberries\.(remote|merge)$`: - return &test.OutputStub{Out: []byte(`branch.blueberries.remote origin -branch.blueberries.merge refs/heads/blueberries`)} - default: - panic("not implemented") - } - })() - - output, err := RunCommand("pr status") - if err != nil { - t.Fatalf("error running command `pr status`: %v", err) - } - - branchRE := regexp.MustCompile(`#10.*\[OWNER:blueberries\]`) - if !branchRE.MatchString(output.String()) { - t.Errorf("did not match current branch:\n%v", output.String()) - } -} - -func TestPRStatus_reviewsAndChecks(t *testing.T) { - initBlankContext("", "OWNER/REPO", "blueberries") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.FileResponse("../test/fixtures/prStatusChecks.json")) - - output, err := RunCommand("pr status") - if err != nil { - t.Errorf("error running command `pr status`: %v", err) - } - - expected := []string{ - "✓ Checks passing + Changes requested", - "- Checks pending ✓ Approved", - "× 1/3 checks failing - Review required", - } - - for _, line := range expected { - if !strings.Contains(output.String(), line) { - t.Errorf("output did not contain %q: %q", line, output.String()) - } - } -} - -func TestPRStatus_currentBranch_showTheMostRecentPR(t *testing.T) { - initBlankContext("", "OWNER/REPO", "blueberries") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.FileResponse("../test/fixtures/prStatusCurrentBranch.json")) - - output, err := RunCommand("pr status") - if err != nil { - t.Errorf("error running command `pr status`: %v", err) - } - - expectedLine := regexp.MustCompile(`#10 Blueberries are certainly a good fruit \[blueberries\]`) - if !expectedLine.MatchString(output.String()) { - t.Errorf("output did not match regexp /%s/\n> output\n%s\n", expectedLine, output) - return - } - - unexpectedLines := []*regexp.Regexp{ - regexp.MustCompile(`#9 Blueberries are a good fruit \[blueberries\] - Merged`), - regexp.MustCompile(`#8 Blueberries are probably a good fruit \[blueberries\] - Closed`), - } - for _, r := range unexpectedLines { - if r.MatchString(output.String()) { - t.Errorf("output unexpectedly match regexp /%s/\n> output\n%s\n", r, output) - return - } - } -} - -func TestPRStatus_currentBranch_defaultBranch(t *testing.T) { - initBlankContext("", "OWNER/REPO", "blueberries") - http := initFakeHTTP() - http.StubRepoResponseWithDefaultBranch("OWNER", "REPO", "blueberries") - http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.FileResponse("../test/fixtures/prStatusCurrentBranch.json")) - - output, err := RunCommand("pr status") - if err != nil { - t.Errorf("error running command `pr status`: %v", err) - } - - expectedLine := regexp.MustCompile(`#10 Blueberries are certainly a good fruit \[blueberries\]`) - if !expectedLine.MatchString(output.String()) { - t.Errorf("output did not match regexp /%s/\n> output\n%s\n", expectedLine, output) - return - } -} - -func TestPRStatus_currentBranch_defaultBranch_repoFlag(t *testing.T) { - initBlankContext("", "OWNER/REPO", "blueberries") - http := initFakeHTTP() - http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.FileResponse("../test/fixtures/prStatusCurrentBranchClosedOnDefaultBranch.json")) - - output, err := RunCommand("pr status -R OWNER/REPO") - if err != nil { - t.Errorf("error running command `pr status`: %v", err) - } - - expectedLine := regexp.MustCompile(`#8 Blueberries are a good fruit \[blueberries\]`) - if expectedLine.MatchString(output.String()) { - t.Errorf("output not expected to match regexp /%s/\n> output\n%s\n", expectedLine, output) - return - } -} - -func TestPRStatus_currentBranch_Closed(t *testing.T) { - initBlankContext("", "OWNER/REPO", "blueberries") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.FileResponse("../test/fixtures/prStatusCurrentBranchClosed.json")) - - output, err := RunCommand("pr status") - if err != nil { - t.Errorf("error running command `pr status`: %v", err) - } - - expectedLine := regexp.MustCompile(`#8 Blueberries are a good fruit \[blueberries\] - Closed`) - if !expectedLine.MatchString(output.String()) { - t.Errorf("output did not match regexp /%s/\n> output\n%s\n", expectedLine, output) - return - } -} - -func TestPRStatus_currentBranch_Closed_defaultBranch(t *testing.T) { - initBlankContext("", "OWNER/REPO", "blueberries") - http := initFakeHTTP() - http.StubRepoResponseWithDefaultBranch("OWNER", "REPO", "blueberries") - http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.FileResponse("../test/fixtures/prStatusCurrentBranchClosedOnDefaultBranch.json")) - - output, err := RunCommand("pr status") - if err != nil { - t.Errorf("error running command `pr status`: %v", err) - } - - expectedLine := regexp.MustCompile(`There is no pull request associated with \[blueberries\]`) - if !expectedLine.MatchString(output.String()) { - t.Errorf("output did not match regexp /%s/\n> output\n%s\n", expectedLine, output) - return - } -} - -func TestPRStatus_currentBranch_Merged(t *testing.T) { - initBlankContext("", "OWNER/REPO", "blueberries") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.FileResponse("../test/fixtures/prStatusCurrentBranchMerged.json")) - - output, err := RunCommand("pr status") - if err != nil { - t.Errorf("error running command `pr status`: %v", err) - } - - expectedLine := regexp.MustCompile(`#8 Blueberries are a good fruit \[blueberries\] - Merged`) - if !expectedLine.MatchString(output.String()) { - t.Errorf("output did not match regexp /%s/\n> output\n%s\n", expectedLine, output) - return - } -} - -func TestPRStatus_currentBranch_Merged_defaultBranch(t *testing.T) { - initBlankContext("", "OWNER/REPO", "blueberries") - http := initFakeHTTP() - http.StubRepoResponseWithDefaultBranch("OWNER", "REPO", "blueberries") - http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.FileResponse("../test/fixtures/prStatusCurrentBranchMergedOnDefaultBranch.json")) - - output, err := RunCommand("pr status") - if err != nil { - t.Errorf("error running command `pr status`: %v", err) - } - - expectedLine := regexp.MustCompile(`There is no pull request associated with \[blueberries\]`) - if !expectedLine.MatchString(output.String()) { - t.Errorf("output did not match regexp /%s/\n> output\n%s\n", expectedLine, output) - return - } -} - -func TestPRStatus_blankSlate(t *testing.T) { - initBlankContext("", "OWNER/REPO", "blueberries") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.StringResponse(`{"data": {}}`)) - - output, err := RunCommand("pr status") - if err != nil { - t.Errorf("error running command `pr status`: %v", err) - } - - expected := ` -Relevant pull requests in OWNER/REPO - -Current branch - There is no pull request associated with [blueberries] - -Created by you - You have no open pull requests - -Requesting a code review from you - You have no pull requests to review - -` - if output.String() != expected { - t.Errorf("expected %q, got %q", expected, output.String()) - } -} - -func TestPRStatus_detachedHead(t *testing.T) { - initBlankContext("", "OWNER/REPO", "") - http := initFakeHTTP() - http.StubRepoResponse("OWNER", "REPO") - http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.StringResponse(`{"data": {}}`)) - - output, err := RunCommand("pr status") - if err != nil { - t.Errorf("error running command `pr status`: %v", err) - } - - expected := ` -Relevant pull requests in OWNER/REPO - -Current branch - There is no current branch - -Created by you - You have no open pull requests - -Requesting a code review from you - You have no pull requests to review - -` - if output.String() != expected { - t.Errorf("expected %q, got %q", expected, output.String()) - } -} - func TestPRList(t *testing.T) { initBlankContext("", "OWNER/REPO", "master") defer stubTerminal(true)() @@ -481,13 +211,6 @@ func TestPRList_web(t *testing.T) { eq(t, url, expectedURL) } -func TestReplaceExcessiveWhitespace(t *testing.T) { - eq(t, replaceExcessiveWhitespace("hello\ngoodbye"), "hello goodbye") - eq(t, replaceExcessiveWhitespace(" hello goodbye "), "hello goodbye") - eq(t, replaceExcessiveWhitespace("hello goodbye"), "hello goodbye") - eq(t, replaceExcessiveWhitespace(" hello \n goodbye "), "hello goodbye") -} - func TestPrClose(t *testing.T) { initBlankContext("", "OWNER/REPO", "master") http := initFakeHTTP() diff --git a/command/root.go b/command/root.go index 8752a3d65..78667d8d4 100644 --- a/command/root.go +++ b/command/root.go @@ -26,6 +26,7 @@ import ( prDiffCmd "github.com/cli/cli/pkg/cmd/pr/diff" prMergeCmd "github.com/cli/cli/pkg/cmd/pr/merge" prReviewCmd "github.com/cli/cli/pkg/cmd/pr/review" + prStatusCmd "github.com/cli/cli/pkg/cmd/pr/status" prViewCmd "github.com/cli/cli/pkg/cmd/pr/view" repoCmd "github.com/cli/cli/pkg/cmd/repo" repoCloneCmd "github.com/cli/cli/pkg/cmd/repo/clone" @@ -178,6 +179,7 @@ func init() { prCmd.AddCommand(prCheckoutCmd.NewCmdCheckout(&repoResolvingCmdFactory, nil)) prCmd.AddCommand(prViewCmd.NewCmdView(&repoResolvingCmdFactory, nil)) prCmd.AddCommand(prMergeCmd.NewCmdMerge(&repoResolvingCmdFactory, nil)) + prCmd.AddCommand(prStatusCmd.NewCmdStatus(&repoResolvingCmdFactory, nil)) RootCmd.AddCommand(creditsCmd.NewCmdCredits(cmdFactory, nil)) } diff --git a/pkg/cmd/pr/shared/display.go b/pkg/cmd/pr/shared/display.go index f0f02072b..f43b7fa67 100644 --- a/pkg/cmd/pr/shared/display.go +++ b/pkg/cmd/pr/shared/display.go @@ -1,6 +1,8 @@ package shared import ( + "fmt" + "io" "strings" "github.com/cli/cli/api" @@ -35,3 +37,11 @@ func ColorFuncForState(state string) func(string) string { return nil } } + +func PrintHeader(w io.Writer, s string) { + fmt.Fprintln(w, utils.Bold(s)) +} + +func PrintMessage(w io.Writer, s string) { + fmt.Fprintln(w, utils.Gray(s)) +} diff --git a/test/fixtures/prStatus.json b/pkg/cmd/pr/status/fixtures/prStatus.json similarity index 100% rename from test/fixtures/prStatus.json rename to pkg/cmd/pr/status/fixtures/prStatus.json diff --git a/test/fixtures/prStatusChecks.json b/pkg/cmd/pr/status/fixtures/prStatusChecks.json similarity index 100% rename from test/fixtures/prStatusChecks.json rename to pkg/cmd/pr/status/fixtures/prStatusChecks.json diff --git a/test/fixtures/prStatusCurrentBranch.json b/pkg/cmd/pr/status/fixtures/prStatusCurrentBranch.json similarity index 100% rename from test/fixtures/prStatusCurrentBranch.json rename to pkg/cmd/pr/status/fixtures/prStatusCurrentBranch.json diff --git a/test/fixtures/prStatusCurrentBranchClosed.json b/pkg/cmd/pr/status/fixtures/prStatusCurrentBranchClosed.json similarity index 100% rename from test/fixtures/prStatusCurrentBranchClosed.json rename to pkg/cmd/pr/status/fixtures/prStatusCurrentBranchClosed.json diff --git a/test/fixtures/prStatusCurrentBranchClosedOnDefaultBranch.json b/pkg/cmd/pr/status/fixtures/prStatusCurrentBranchClosedOnDefaultBranch.json similarity index 100% rename from test/fixtures/prStatusCurrentBranchClosedOnDefaultBranch.json rename to pkg/cmd/pr/status/fixtures/prStatusCurrentBranchClosedOnDefaultBranch.json diff --git a/test/fixtures/prStatusCurrentBranchMerged.json b/pkg/cmd/pr/status/fixtures/prStatusCurrentBranchMerged.json similarity index 100% rename from test/fixtures/prStatusCurrentBranchMerged.json rename to pkg/cmd/pr/status/fixtures/prStatusCurrentBranchMerged.json diff --git a/test/fixtures/prStatusCurrentBranchMergedOnDefaultBranch.json b/pkg/cmd/pr/status/fixtures/prStatusCurrentBranchMergedOnDefaultBranch.json similarity index 100% rename from test/fixtures/prStatusCurrentBranchMergedOnDefaultBranch.json rename to pkg/cmd/pr/status/fixtures/prStatusCurrentBranchMergedOnDefaultBranch.json diff --git a/pkg/cmd/pr/status/status.go b/pkg/cmd/pr/status/status.go new file mode 100644 index 000000000..8708dae65 --- /dev/null +++ b/pkg/cmd/pr/status/status.go @@ -0,0 +1,238 @@ +package status + +import ( + "errors" + "fmt" + "io" + "net/http" + "regexp" + "strconv" + "strings" + + "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/ghrepo" + "github.com/cli/cli/pkg/cmd/pr/shared" + "github.com/cli/cli/pkg/cmdutil" + "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/pkg/text" + "github.com/cli/cli/utils" + "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) + Remotes func() (context.Remotes, error) + Branch func() (string, error) + + HasRepoOverride bool +} + +func NewCmdStatus(f *cmdutil.Factory, runF func(*StatusOptions) error) *cobra.Command { + opts := &StatusOptions{ + IO: f.IOStreams, + HttpClient: f.HttpClient, + Config: f.Config, + BaseRepo: f.BaseRepo, + Remotes: f.Remotes, + Branch: f.Branch, + } + + cmd := &cobra.Command{ + Use: "status", + Short: "Show status of relevant pull requests", + Args: cmdutil.NoArgsQuoteReminder, + RunE: func(cmd *cobra.Command, args []string) error { + opts.HasRepoOverride = cmd.Flags().Changed("repo") + + 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 + } + + var currentBranch string + var currentPRNumber int + var currentPRHeadRef string + + if !opts.HasRepoOverride { + currentBranch, err = opts.Branch() + if err != nil && !errors.Is(err, git.ErrNotOnAnyBranch) { + return fmt.Errorf("could not query for pull request for current branch: %w", err) + } + + remotes, _ := opts.Remotes() + currentPRNumber, currentPRHeadRef, err = prSelectorForCurrentBranch(baseRepo, currentBranch, remotes) + if err != nil { + return fmt.Errorf("could not query for pull request for current branch: %w", err) + } + } + + // the `@me` macro is available because the API lookup is ElasticSearch-based + currentUser := "@me" + prPayload, err := api.PullRequests(apiClient, baseRepo, currentPRNumber, currentPRHeadRef, currentUser) + if err != nil { + return err + } + + out := opts.IO.Out + + fmt.Fprintln(out, "") + fmt.Fprintf(out, "Relevant pull requests in %s\n", ghrepo.FullName(baseRepo)) + fmt.Fprintln(out, "") + + shared.PrintHeader(out, "Current branch") + currentPR := prPayload.CurrentPR + if currentPR != nil && currentPR.State != "OPEN" && prPayload.DefaultBranch == currentBranch { + currentPR = nil + } + if currentPR != nil { + printPrs(out, 1, *currentPR) + } else if currentPRHeadRef == "" { + shared.PrintMessage(out, " There is no current branch") + } else { + shared.PrintMessage(out, fmt.Sprintf(" There is no pull request associated with %s", utils.Cyan("["+currentPRHeadRef+"]"))) + } + fmt.Fprintln(out) + + shared.PrintHeader(out, "Created by you") + if prPayload.ViewerCreated.TotalCount > 0 { + printPrs(out, prPayload.ViewerCreated.TotalCount, prPayload.ViewerCreated.PullRequests...) + } else { + shared.PrintMessage(out, " You have no open pull requests") + } + fmt.Fprintln(out) + + shared.PrintHeader(out, "Requesting a code review from you") + if prPayload.ReviewRequested.TotalCount > 0 { + printPrs(out, prPayload.ReviewRequested.TotalCount, prPayload.ReviewRequested.PullRequests...) + } else { + shared.PrintMessage(out, " You have no pull requests to review") + } + fmt.Fprintln(out) + + return nil +} + +func prSelectorForCurrentBranch(baseRepo ghrepo.Interface, prHeadRef string, rem context.Remotes) (prNumber int, selector string, err error) { + selector = prHeadRef + branchConfig := git.ReadBranchConfig(prHeadRef) + + // the branch is configured to merge a special PR head ref + prHeadRE := regexp.MustCompile(`^refs/pull/(\d+)/head$`) + if m := prHeadRE.FindStringSubmatch(branchConfig.MergeRef); m != nil { + prNumber, _ = strconv.Atoi(m[1]) + return + } + + var branchOwner string + if branchConfig.RemoteURL != nil { + // the branch merges from a remote specified by URL + if r, err := ghrepo.FromURL(branchConfig.RemoteURL); err == nil { + branchOwner = r.RepoOwner() + } + } else if branchConfig.RemoteName != "" { + // the branch merges from a remote specified by name + if r, err := rem.FindByName(branchConfig.RemoteName); err == nil { + branchOwner = r.RepoOwner() + } + } + + if branchOwner != "" { + if strings.HasPrefix(branchConfig.MergeRef, "refs/heads/") { + selector = strings.TrimPrefix(branchConfig.MergeRef, "refs/heads/") + } + // prepend `OWNER:` if this branch is pushed to a fork + if !strings.EqualFold(branchOwner, baseRepo.RepoOwner()) { + selector = fmt.Sprintf("%s:%s", branchOwner, prHeadRef) + } + } + + return +} + +func printPrs(w io.Writer, totalCount int, prs ...api.PullRequest) { + for _, pr := range prs { + prNumber := fmt.Sprintf("#%d", pr.Number) + + prStateColorFunc := utils.Green + if pr.IsDraft { + prStateColorFunc = utils.Gray + } else if pr.State == "MERGED" { + prStateColorFunc = utils.Magenta + } else if pr.State == "CLOSED" { + prStateColorFunc = utils.Red + } + + fmt.Fprintf(w, " %s %s %s", prStateColorFunc(prNumber), text.Truncate(50, text.ReplaceExcessiveWhitespace(pr.Title)), utils.Cyan("["+pr.HeadLabel()+"]")) + + checks := pr.ChecksStatus() + reviews := pr.ReviewStatus() + + if pr.State == "OPEN" { + reviewStatus := reviews.ChangesRequested || reviews.Approved || reviews.ReviewRequired + if checks.Total > 0 || reviewStatus { + // show checks & reviews on their own line + fmt.Fprintf(w, "\n ") + } + + if checks.Total > 0 { + var summary string + if checks.Failing > 0 { + if checks.Failing == checks.Total { + summary = utils.Red("× All checks failing") + } else { + summary = utils.Red(fmt.Sprintf("× %d/%d checks failing", checks.Failing, checks.Total)) + } + } else if checks.Pending > 0 { + summary = utils.Yellow("- Checks pending") + } else if checks.Passing == checks.Total { + summary = utils.Green("✓ Checks passing") + } + fmt.Fprint(w, summary) + } + + if checks.Total > 0 && reviewStatus { + // add padding between checks & reviews + fmt.Fprint(w, " ") + } + + if reviews.ChangesRequested { + fmt.Fprint(w, utils.Red("+ Changes requested")) + } else if reviews.ReviewRequired { + fmt.Fprint(w, utils.Yellow("- Review required")) + } else if reviews.Approved { + fmt.Fprint(w, utils.Green("✓ Approved")) + } + } else { + fmt.Fprintf(w, " - %s", shared.StateTitleWithColor(pr)) + } + + fmt.Fprint(w, "\n") + } + remaining := totalCount - len(prs) + if remaining > 0 { + fmt.Fprintf(w, utils.Gray(" And %d more\n"), remaining) + } +} diff --git a/pkg/cmd/pr/status/status_test.go b/pkg/cmd/pr/status/status_test.go new file mode 100644 index 000000000..bc4f4e4f9 --- /dev/null +++ b/pkg/cmd/pr/status/status_test.go @@ -0,0 +1,310 @@ +package status + +import ( + "bytes" + "io/ioutil" + "net/http" + "regexp" + "strings" + "testing" + + "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/httpmock" + "github.com/cli/cli/pkg/iostreams" + "github.com/cli/cli/test" + "github.com/google/shlex" +) + +func runCommand(rt http.RoundTripper, branch string, 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 + }, + Remotes: func() (context.Remotes, error) { + return context.Remotes{ + { + Remote: &git.Remote{Name: "origin"}, + Repo: ghrepo.New("OWNER", "REPO"), + }, + }, nil + }, + Branch: func() (string, error) { + if branch == "" { + return "", git.ErrNotOnAnyBranch + } + return branch, nil + }, + } + + cmd := NewCmdStatus(factory, nil) + cmd.PersistentFlags().StringP("repo", "R", "", "") + + 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 initFakeHTTP() *httpmock.Registry { + return &httpmock.Registry{} +} + +func TestPRStatus(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.FileResponse("./fixtures/prStatus.json")) + + output, err := runCommand(http, "blueberries", true, "") + if err != nil { + t.Errorf("error running command `pr status`: %v", err) + } + + expectedPrs := []*regexp.Regexp{ + regexp.MustCompile(`#8.*\[strawberries\]`), + regexp.MustCompile(`#9.*\[apples\]`), + regexp.MustCompile(`#10.*\[blueberries\]`), + regexp.MustCompile(`#11.*\[figs\]`), + } + + for _, r := range expectedPrs { + if !r.MatchString(output.String()) { + t.Errorf("output did not match regexp /%s/", r) + } + } +} + +func TestPRStatus_reviewsAndChecks(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.FileResponse("./fixtures/prStatusChecks.json")) + + output, err := runCommand(http, "blueberries", true, "") + if err != nil { + t.Errorf("error running command `pr status`: %v", err) + } + + expected := []string{ + "✓ Checks passing + Changes requested", + "- Checks pending ✓ Approved", + "× 1/3 checks failing - Review required", + } + + for _, line := range expected { + if !strings.Contains(output.String(), line) { + t.Errorf("output did not contain %q: %q", line, output.String()) + } + } +} + +func TestPRStatus_currentBranch_showTheMostRecentPR(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.FileResponse("./fixtures/prStatusCurrentBranch.json")) + + output, err := runCommand(http, "blueberries", true, "") + if err != nil { + t.Errorf("error running command `pr status`: %v", err) + } + + expectedLine := regexp.MustCompile(`#10 Blueberries are certainly a good fruit \[blueberries\]`) + if !expectedLine.MatchString(output.String()) { + t.Errorf("output did not match regexp /%s/\n> output\n%s\n", expectedLine, output) + return + } + + unexpectedLines := []*regexp.Regexp{ + regexp.MustCompile(`#9 Blueberries are a good fruit \[blueberries\] - Merged`), + regexp.MustCompile(`#8 Blueberries are probably a good fruit \[blueberries\] - Closed`), + } + for _, r := range unexpectedLines { + if r.MatchString(output.String()) { + t.Errorf("output unexpectedly match regexp /%s/\n> output\n%s\n", r, output) + return + } + } +} + +func TestPRStatus_currentBranch_defaultBranch(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.FileResponse("./fixtures/prStatusCurrentBranch.json")) + + output, err := runCommand(http, "blueberries", true, "") + if err != nil { + t.Errorf("error running command `pr status`: %v", err) + } + + expectedLine := regexp.MustCompile(`#10 Blueberries are certainly a good fruit \[blueberries\]`) + if !expectedLine.MatchString(output.String()) { + t.Errorf("output did not match regexp /%s/\n> output\n%s\n", expectedLine, output) + return + } +} + +func TestPRStatus_currentBranch_defaultBranch_repoFlag(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.FileResponse("./fixtures/prStatusCurrentBranchClosedOnDefaultBranch.json")) + + output, err := runCommand(http, "blueberries", true, "-R OWNER/REPO") + if err != nil { + t.Errorf("error running command `pr status`: %v", err) + } + + expectedLine := regexp.MustCompile(`#8 Blueberries are a good fruit \[blueberries\]`) + if expectedLine.MatchString(output.String()) { + t.Errorf("output not expected to match regexp /%s/\n> output\n%s\n", expectedLine, output) + return + } +} + +func TestPRStatus_currentBranch_Closed(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.FileResponse("./fixtures/prStatusCurrentBranchClosed.json")) + + output, err := runCommand(http, "blueberries", true, "") + if err != nil { + t.Errorf("error running command `pr status`: %v", err) + } + + expectedLine := regexp.MustCompile(`#8 Blueberries are a good fruit \[blueberries\] - Closed`) + if !expectedLine.MatchString(output.String()) { + t.Errorf("output did not match regexp /%s/\n> output\n%s\n", expectedLine, output) + return + } +} + +func TestPRStatus_currentBranch_Closed_defaultBranch(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.FileResponse("./fixtures/prStatusCurrentBranchClosedOnDefaultBranch.json")) + + output, err := runCommand(http, "blueberries", true, "") + if err != nil { + t.Errorf("error running command `pr status`: %v", err) + } + + expectedLine := regexp.MustCompile(`There is no pull request associated with \[blueberries\]`) + if !expectedLine.MatchString(output.String()) { + t.Errorf("output did not match regexp /%s/\n> output\n%s\n", expectedLine, output) + return + } +} + +func TestPRStatus_currentBranch_Merged(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.FileResponse("./fixtures/prStatusCurrentBranchMerged.json")) + + output, err := runCommand(http, "blueberries", true, "") + if err != nil { + t.Errorf("error running command `pr status`: %v", err) + } + + expectedLine := regexp.MustCompile(`#8 Blueberries are a good fruit \[blueberries\] - Merged`) + if !expectedLine.MatchString(output.String()) { + t.Errorf("output did not match regexp /%s/\n> output\n%s\n", expectedLine, output) + return + } +} + +func TestPRStatus_currentBranch_Merged_defaultBranch(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.FileResponse("./fixtures/prStatusCurrentBranchMergedOnDefaultBranch.json")) + + output, err := runCommand(http, "blueberries", true, "") + if err != nil { + t.Errorf("error running command `pr status`: %v", err) + } + + expectedLine := regexp.MustCompile(`There is no pull request associated with \[blueberries\]`) + if !expectedLine.MatchString(output.String()) { + t.Errorf("output did not match regexp /%s/\n> output\n%s\n", expectedLine, output) + return + } +} + +func TestPRStatus_blankSlate(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.StringResponse(`{"data": {}}`)) + + output, err := runCommand(http, "blueberries", true, "") + if err != nil { + t.Errorf("error running command `pr status`: %v", err) + } + + expected := ` +Relevant pull requests in OWNER/REPO + +Current branch + There is no pull request associated with [blueberries] + +Created by you + You have no open pull requests + +Requesting a code review from you + You have no pull requests to review + +` + if output.String() != expected { + t.Errorf("expected %q, got %q", expected, output.String()) + } +} + +func TestPRStatus_detachedHead(t *testing.T) { + http := initFakeHTTP() + defer http.Verify(t) + http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.StringResponse(`{"data": {}}`)) + + output, err := runCommand(http, "", true, "") + if err != nil { + t.Errorf("error running command `pr status`: %v", err) + } + + expected := ` +Relevant pull requests in OWNER/REPO + +Current branch + There is no current branch + +Created by you + You have no open pull requests + +Requesting a code review from you + You have no pull requests to review + +` + if output.String() != expected { + t.Errorf("expected %q, got %q", expected, output.String()) + } +} diff --git a/pkg/text/sanitize.go b/pkg/text/sanitize.go new file mode 100644 index 000000000..16bb902dc --- /dev/null +++ b/pkg/text/sanitize.go @@ -0,0 +1,12 @@ +package text + +import ( + "regexp" + "strings" +) + +var ws = regexp.MustCompile(`\s+`) + +func ReplaceExcessiveWhitespace(s string) string { + return ws.ReplaceAllString(strings.TrimSpace(s), " ") +} diff --git a/pkg/text/sanitize_test.go b/pkg/text/sanitize_test.go new file mode 100644 index 000000000..1c03362d9 --- /dev/null +++ b/pkg/text/sanitize_test.go @@ -0,0 +1,29 @@ +package text + +import "testing" + +func TestReplaceExcessiveWhitespace(t *testing.T) { + tests := []struct { + name string + input string + want string + }{ + { + name: "no replacements", + input: "one two three", + want: "one two three", + }, + { + name: "whitespace b-gone", + input: "\n one\n\t two three\r\n ", + want: "one two three", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := ReplaceExcessiveWhitespace(tt.input); got != tt.want { + t.Errorf("ReplaceExcessiveWhitespace() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/test/fixtures/prStatusFork.json b/test/fixtures/prStatusFork.json deleted file mode 100644 index c9a7a5b3a..000000000 --- a/test/fixtures/prStatusFork.json +++ /dev/null @@ -1,33 +0,0 @@ -{ - "data": { - "repository": { - "pullRequests": { - "totalCount": 1, - "edges": [ - { - "node": { - "number": 10, - "title": "Blueberries are a good fruit", - "state": "OPEN", - "url": "https://github.com/PARENT/REPO/pull/10", - "headRefName": "blueberries", - "isDraft": false, - "headRepositoryOwner": { - "login": "OWNER" - }, - "isCrossRepository": true - } - } - ] - } - }, - "viewerCreated": { - "totalCount": 0, - "edges": [] - }, - "reviewRequested": { - "totalCount": 0, - "edges": [] - } - } -}