diff --git a/.github/workflows/releases.yml b/.github/workflows/releases.yml index 73974448c..b7fc01545 100644 --- a/.github/workflows/releases.yml +++ b/.github/workflows/releases.yml @@ -44,6 +44,11 @@ jobs: token: ${{secrets.SITE_GITHUB_TOKEN}} - name: Publish documentation site if: "!contains(github.ref, '-')" # skip prereleases + env: + GIT_COMMITTER_NAME: cli automation + GIT_AUTHOR_NAME: cli automation + GIT_COMMITTER_EMAIL: noreply@github.com + GIT_AUTHOR_EMAIL: noreply@github.com run: make site-publish - name: Move project cards if: "!contains(github.ref, '-')" # skip prereleases diff --git a/.gitignore b/.gitignore index 5ef399ba7..00a5bb5a6 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,5 @@ /bin +/share/man/man1 /gh-cli .envrc /dist diff --git a/.goreleaser.yml b/.goreleaser.yml index 8fca69fb3..ceb22c12e 100644 --- a/.goreleaser.yml +++ b/.goreleaser.yml @@ -6,6 +6,7 @@ release: before: hooks: - go mod tidy + - make manpages builds: - <<: &build_defaults @@ -17,10 +18,12 @@ builds: id: macos goos: [darwin] goarch: [amd64] + - <<: *build_defaults id: linux goos: [linux] goarch: [386, amd64, arm64] + - <<: *build_defaults id: windows goos: [windows] @@ -35,11 +38,16 @@ archives: replacements: darwin: macOS format: tar.gz + files: + - LICENSE + - ./share/man/man1/gh*.1 - id: windows builds: [windows] <<: *archive_defaults wrap_in_directory: false format: zip + files: + - LICENSE brews: - name: gh @@ -57,8 +65,9 @@ brews: depends_on "go" end install: | - system "make" if build.head? + system "make", "bin/gh", "manpages" if build.head? bin.install "bin/gh" + man1.install Dir["./share/man/man1/gh*.1"] (bash_completion/"gh.sh").write `#{bin}/gh completion -s bash` (zsh_completion/"_gh").write `#{bin}/gh completion -s zsh` (fish_completion/"gh.fish").write `#{bin}/gh completion -s fish` @@ -76,6 +85,8 @@ nfpms: formats: - deb - rpm + files: + "./share/man/man1/gh*.1": "/usr/share/man/man1" scoop: bucket: diff --git a/Makefile b/Makefile index f2b4805c8..b927c0eb1 100644 --- a/Makefile +++ b/Makefile @@ -22,13 +22,13 @@ test: go test ./... .PHONY: test -site: - git clone https://github.com/github/cli.github.com.git "$@" +site: bin/gh + bin/gh repo clone github/cli.github.com "$@" site-docs: site git -C site pull git -C site rm 'manual/gh*.md' 2>/dev/null || true - go run ./cmd/gen-docs site/manual + go run ./cmd/gen-docs --website --doc-path site/manual for f in site/manual/gh*.md; do sed -i.bak -e '/^### SEE ALSO/,$$d' "$$f"; done rm -f site/manual/*.bak git -C site add 'manual/gh*.md' @@ -44,3 +44,8 @@ endif git -C site commit -m '$(GITHUB_REF:refs/tags/v%=%)' index.html git -C site push .PHONY: site-publish + + +.PHONY: manpages +manpages: + go run ./cmd/gen-docs --man-page --doc-path ./share/man/man1/ diff --git a/api/queries_issue_test.go b/api/queries_issue_test.go index c8ee505cf..ffe4aaad1 100644 --- a/api/queries_issue_test.go +++ b/api/queries_issue_test.go @@ -39,7 +39,8 @@ func TestIssueList(t *testing.T) { } } } `)) - _, err := IssueList(client, ghrepo.FromFullName("OWNER/REPO"), "open", []string{}, "", 251, "") + repo, _ := ghrepo.FromFullName("OWNER/REPO") + _, err := IssueList(client, repo, "open", []string{}, "", 251, "") if err != nil { t.Fatalf("unexpected error: %v", err) } diff --git a/api/queries_pr.go b/api/queries_pr.go index c1afc6bc9..6ee26176c 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -2,7 +2,10 @@ package api import ( "context" + "errors" "fmt" + "io/ioutil" + "net/http" "strings" "github.com/shurcooL/githubv4" @@ -201,6 +204,39 @@ func (pr *PullRequest) ChecksStatus() (summary PullRequestChecksStatus) { return } +func (c Client) PullRequestDiff(baseRepo ghrepo.Interface, prNum int) (string, error) { + url := fmt.Sprintf("https://api.github.com/repos/%s/pulls/%d", + ghrepo.FullName(baseRepo), prNum) + req, err := http.NewRequest("GET", url, nil) + if err != nil { + return "", err + } + + req.Header.Set("Accept", "application/vnd.github.v3.diff; charset=utf-8") + + resp, err := c.http.Do(req) + if err != nil { + return "", err + } + defer resp.Body.Close() + + b, err := ioutil.ReadAll(resp.Body) + if err != nil { + return "", err + } + + if resp.StatusCode == 200 { + return string(b), nil + } + + if resp.StatusCode == 404 { + return "", &NotFoundError{errors.New("pull request not found")} + } + + return "", errors.New("pull request diff lookup failed") + +} + func PullRequests(client *Client, repo ghrepo.Interface, currentPRNumber int, currentPRHeadRef, currentUsername string) (*PullRequestsPayload, error) { type edges struct { TotalCount int @@ -667,6 +703,7 @@ func CreatePullRequest(client *Client, repo *Repository, params map[string]inter requestReviews(input: $input) { clientMutationId } }` reviewParams["pullRequestId"] = pr.ID + reviewParams["union"] = true variables := map[string]interface{}{ "input": reviewParams, } @@ -952,6 +989,27 @@ func PullRequestMerge(client *Client, repo ghrepo.Interface, pr *PullRequest, m return err } +func PullRequestReady(client *Client, repo ghrepo.Interface, pr *PullRequest) error { + var mutation struct { + MarkPullRequestReadyForReviewInput struct { + PullRequest struct { + ID githubv4.ID + } + } `graphql:"markPullRequestReadyForReview(input: $input)"` + } + + type MarkPullRequestReadyForReviewInput struct { + PullRequestID githubv4.ID `json:"pullRequestId"` + } + + input := MarkPullRequestReadyForReviewInput{PullRequestID: pr.ID} + + v4 := githubv4.NewClient(client.http) + err := v4.Mutate(context.Background(), &mutation, input, nil) + + return err +} + func min(a, b int) int { if a < b { return a diff --git a/api/queries_repo.go b/api/queries_repo.go index af578413d..c8ba31348 100644 --- a/api/queries_repo.go +++ b/api/queries_repo.go @@ -580,6 +580,108 @@ func RepoMetadata(client *Client, repo ghrepo.Interface, input RepoMetadataInput return &result, err } +type RepoResolveInput struct { + Assignees []string + Reviewers []string + Labels []string + Projects []string + Milestones []string +} + +// RepoResolveMetadataIDs looks up GraphQL node IDs in bulk +func RepoResolveMetadataIDs(client *Client, repo ghrepo.Interface, input RepoResolveInput) (*RepoMetadataResult, error) { + users := input.Assignees + hasUser := func(target string) bool { + for _, u := range users { + if strings.EqualFold(u, target) { + return true + } + } + return false + } + + var teams []string + for _, r := range input.Reviewers { + if i := strings.IndexRune(r, '/'); i > -1 { + teams = append(teams, r[i+1:]) + } else if !hasUser(r) { + users = append(users, r) + } + } + + // there is no way to look up projects nor milestones by name, so preload them all + mi := RepoMetadataInput{ + Projects: len(input.Projects) > 0, + Milestones: len(input.Milestones) > 0, + } + result, err := RepoMetadata(client, repo, mi) + if err != nil { + return result, err + } + if len(users) == 0 && len(teams) == 0 && len(input.Labels) == 0 { + return result, nil + } + + query := &bytes.Buffer{} + fmt.Fprint(query, "{\n") + for i, u := range users { + fmt.Fprintf(query, "u%03d: user(login:%q){id,login}\n", i, u) + } + if len(input.Labels) > 0 { + fmt.Fprintf(query, "repository(owner:%q,name:%q){\n", repo.RepoOwner(), repo.RepoName()) + for i, l := range input.Labels { + fmt.Fprintf(query, "l%03d: label(name:%q){id,name}\n", i, l) + } + fmt.Fprint(query, "}\n") + } + if len(teams) > 0 { + fmt.Fprintf(query, "organization(login:%q){\n", repo.RepoOwner()) + for i, t := range teams { + fmt.Fprintf(query, "t%03d: team(slug:%q){id,slug}\n", i, t) + } + fmt.Fprint(query, "}\n") + } + fmt.Fprint(query, "}\n") + + response := make(map[string]json.RawMessage) + err = client.GraphQL(query.String(), nil, &response) + if err != nil { + return result, err + } + + for key, v := range response { + switch key { + case "repository": + repoResponse := make(map[string]RepoLabel) + err := json.Unmarshal(v, &repoResponse) + if err != nil { + return result, err + } + for _, l := range repoResponse { + result.Labels = append(result.Labels, l) + } + case "organization": + orgResponse := make(map[string]OrgTeam) + err := json.Unmarshal(v, &orgResponse) + if err != nil { + return result, err + } + for _, t := range orgResponse { + result.Teams = append(result.Teams, t) + } + default: + user := RepoAssignee{} + err := json.Unmarshal(v, &user) + if err != nil { + return result, err + } + result.AssignableUsers = append(result.AssignableUsers, user) + } + } + + return result, nil +} + type RepoProject struct { ID string Name string diff --git a/api/queries_repo_test.go b/api/queries_repo_test.go index d8a75f4ff..03123cf43 100644 --- a/api/queries_repo_test.go +++ b/api/queries_repo_test.go @@ -6,6 +6,7 @@ import ( "io/ioutil" "testing" + "github.com/cli/cli/internal/ghrepo" "github.com/cli/cli/pkg/httpmock" ) @@ -45,3 +46,229 @@ func Test_RepoCreate(t *testing.T) { t.Errorf("expected homepageUrl to be %q, got %q", "http://example.com", homepage) } } +func Test_RepoMetadata(t *testing.T) { + http := &httpmock.Registry{} + client := NewClient(ReplaceTripper(http)) + + repo, _ := ghrepo.FromFullName("OWNER/REPO") + input := RepoMetadataInput{ + Assignees: true, + Reviewers: true, + Labels: true, + Projects: true, + Milestones: true, + } + + http.Register( + httpmock.GraphQL(`\bassignableUsers\(`), + httpmock.StringResponse(` + { "data": { "repository": { "assignableUsers": { + "nodes": [ + { "login": "hubot", "id": "HUBOTID" }, + { "login": "MonaLisa", "id": "MONAID" } + ], + "pageInfo": { "hasNextPage": false } + } } } } + `)) + http.Register( + httpmock.GraphQL(`\blabels\(`), + httpmock.StringResponse(` + { "data": { "repository": { "labels": { + "nodes": [ + { "name": "feature", "id": "FEATUREID" }, + { "name": "TODO", "id": "TODOID" }, + { "name": "bug", "id": "BUGID" } + ], + "pageInfo": { "hasNextPage": false } + } } } } + `)) + http.Register( + httpmock.GraphQL(`\bmilestones\(`), + httpmock.StringResponse(` + { "data": { "repository": { "milestones": { + "nodes": [ + { "title": "GA", "id": "GAID" }, + { "title": "Big One.oh", "id": "BIGONEID" } + ], + "pageInfo": { "hasNextPage": false } + } } } } + `)) + http.Register( + httpmock.GraphQL(`\brepository\(.+\bprojects\(`), + httpmock.StringResponse(` + { "data": { "repository": { "projects": { + "nodes": [ + { "name": "Cleanup", "id": "CLEANUPID" }, + { "name": "Roadmap", "id": "ROADMAPID" } + ], + "pageInfo": { "hasNextPage": false } + } } } } + `)) + http.Register( + httpmock.GraphQL(`\borganization\(.+\bprojects\(`), + httpmock.StringResponse(` + { "data": { "organization": { "projects": { + "nodes": [ + { "name": "Triage", "id": "TRIAGEID" } + ], + "pageInfo": { "hasNextPage": false } + } } } } + `)) + http.Register( + httpmock.GraphQL(`\borganization\(.+\bteams\(`), + httpmock.StringResponse(` + { "data": { "organization": { "teams": { + "nodes": [ + { "slug": "owners", "id": "OWNERSID" }, + { "slug": "Core", "id": "COREID" } + ], + "pageInfo": { "hasNextPage": false } + } } } } + `)) + + result, err := RepoMetadata(client, repo, input) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + expectedMemberIDs := []string{"MONAID", "HUBOTID"} + memberIDs, err := result.MembersToIDs([]string{"monalisa", "hubot"}) + if err != nil { + t.Errorf("error resolving members: %v", err) + } + if !sliceEqual(memberIDs, expectedMemberIDs) { + t.Errorf("expected members %v, got %v", expectedMemberIDs, memberIDs) + } + + expectedTeamIDs := []string{"COREID", "OWNERSID"} + teamIDs, err := result.TeamsToIDs([]string{"OWNER/core", "/owners"}) + if err != nil { + t.Errorf("error resolving teams: %v", err) + } + if !sliceEqual(teamIDs, expectedTeamIDs) { + t.Errorf("expected teams %v, got %v", expectedTeamIDs, teamIDs) + } + + expectedLabelIDs := []string{"BUGID", "TODOID"} + labelIDs, err := result.LabelsToIDs([]string{"bug", "todo"}) + if err != nil { + t.Errorf("error resolving labels: %v", err) + } + if !sliceEqual(labelIDs, expectedLabelIDs) { + t.Errorf("expected labels %v, got %v", expectedLabelIDs, labelIDs) + } + + expectedProjectIDs := []string{"TRIAGEID", "ROADMAPID"} + projectIDs, err := result.ProjectsToIDs([]string{"triage", "roadmap"}) + if err != nil { + t.Errorf("error resolving projects: %v", err) + } + if !sliceEqual(projectIDs, expectedProjectIDs) { + t.Errorf("expected projects %v, got %v", expectedProjectIDs, projectIDs) + } + + expectedMilestoneID := "BIGONEID" + milestoneID, err := result.MilestoneToID("big one.oh") + if err != nil { + t.Errorf("error resolving milestone: %v", err) + } + if milestoneID != expectedMilestoneID { + t.Errorf("expected milestone %v, got %v", expectedMilestoneID, milestoneID) + } +} + +func Test_RepoResolveMetadataIDs(t *testing.T) { + http := &httpmock.Registry{} + client := NewClient(ReplaceTripper(http)) + + repo, _ := ghrepo.FromFullName("OWNER/REPO") + input := RepoResolveInput{ + Assignees: []string{"monalisa", "hubot"}, + Reviewers: []string{"monalisa", "octocat", "OWNER/core", "/robots"}, + Labels: []string{"bug", "help wanted"}, + } + + expectedQuery := `{ +u000: user(login:"monalisa"){id,login} +u001: user(login:"hubot"){id,login} +u002: user(login:"octocat"){id,login} +repository(owner:"OWNER",name:"REPO"){ +l000: label(name:"bug"){id,name} +l001: label(name:"help wanted"){id,name} +} +organization(login:"OWNER"){ +t000: team(slug:"core"){id,slug} +t001: team(slug:"robots"){id,slug} +} +} +` + responseJSON := ` + { "data": { + "u000": { "login": "MonaLisa", "id": "MONAID" }, + "u001": { "login": "hubot", "id": "HUBOTID" }, + "u002": { "login": "octocat", "id": "OCTOID" }, + "repository": { + "l000": { "name": "bug", "id": "BUGID" }, + "l001": { "name": "Help Wanted", "id": "HELPID" } + }, + "organization": { + "t000": { "slug": "core", "id": "COREID" }, + "t001": { "slug": "Robots", "id": "ROBOTID" } + } + } } + ` + + http.Register( + httpmock.MatchAny, + httpmock.GraphQLQuery(responseJSON, func(q string, _ map[string]interface{}) { + if q != expectedQuery { + t.Errorf("expected query %q, got %q", expectedQuery, q) + } + })) + + result, err := RepoResolveMetadataIDs(client, repo, input) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + expectedMemberIDs := []string{"MONAID", "HUBOTID", "OCTOID"} + memberIDs, err := result.MembersToIDs([]string{"monalisa", "hubot", "octocat"}) + if err != nil { + t.Errorf("error resolving members: %v", err) + } + if !sliceEqual(memberIDs, expectedMemberIDs) { + t.Errorf("expected members %v, got %v", expectedMemberIDs, memberIDs) + } + + expectedTeamIDs := []string{"COREID", "ROBOTID"} + teamIDs, err := result.TeamsToIDs([]string{"/core", "/robots"}) + if err != nil { + t.Errorf("error resolving teams: %v", err) + } + if !sliceEqual(teamIDs, expectedTeamIDs) { + t.Errorf("expected members %v, got %v", expectedTeamIDs, teamIDs) + } + + expectedLabelIDs := []string{"BUGID", "HELPID"} + labelIDs, err := result.LabelsToIDs([]string{"bug", "help wanted"}) + if err != nil { + t.Errorf("error resolving labels: %v", err) + } + if !sliceEqual(labelIDs, expectedLabelIDs) { + t.Errorf("expected members %v, got %v", expectedLabelIDs, labelIDs) + } +} + +func sliceEqual(a, b []string) bool { + if len(a) != len(b) { + return false + } + + for i := range a { + if a[i] != b[i] { + return false + } + } + + return true +} diff --git a/cmd/gen-docs/main.go b/cmd/gen-docs/main.go index 6607054a9..92581d07a 100644 --- a/cmd/gen-docs/main.go +++ b/cmd/gen-docs/main.go @@ -7,22 +7,57 @@ import ( "github.com/cli/cli/command" "github.com/spf13/cobra/doc" + "github.com/spf13/pflag" ) func main() { - if len(os.Args) < 2 { - fatal("Usage: gen-docs ") - } - dir := os.Args[1] - err := os.MkdirAll(dir, 0755) + var flagError pflag.ErrorHandling + docCmd := pflag.NewFlagSet("", flagError) + manPage := docCmd.BoolP("man-page", "", false, "Generate manual pages") + website := docCmd.BoolP("website", "", false, "Generate website pages") + dir := docCmd.StringP("doc-path", "", "", "Path directory where you want generate doc files") + help := docCmd.BoolP("help", "h", false, "Help about any command") + + if err := docCmd.Parse(os.Args); err != nil { + os.Exit(1) + } + + if *help { + _, err := fmt.Fprintf(os.Stderr, "Usage of %s:\n\n%s", os.Args[0], docCmd.FlagUsages()) + if err != nil { + fatal(err) + } + os.Exit(1) + } + + if *dir == "" { + fatal("no dir set") + } + + err := os.MkdirAll(*dir, 0755) if err != nil { fatal(err) } - err = doc.GenMarkdownTreeCustom(command.RootCmd, dir, filePrepender, linkHandler) - if err != nil { - fatal(err) + if *website { + err = doc.GenMarkdownTreeCustom(command.RootCmd, *dir, filePrepender, linkHandler) + if err != nil { + fatal(err) + } + } + + if *manPage { + header := &doc.GenManHeader{ + Title: "gh", + Section: "1", + 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) + if err != nil { + fatal(err) + } } } diff --git a/command/config.go b/command/config.go index 0350e528b..7e2a541f7 100644 --- a/command/config.go +++ b/command/config.go @@ -32,7 +32,7 @@ Current respected settings: var configGetCmd = &cobra.Command{ Use: "get ", - Short: "Prints the value of a given configuration key.", + Short: "Prints the value of a given configuration key", Long: `Get the value for a given configuration key. Examples: @@ -45,7 +45,7 @@ Examples: var configSetCmd = &cobra.Command{ Use: "set ", - Short: "Updates configuration with the value of a given key.", + Short: "Updates configuration with the value of a given key", Long: `Update the configuration by setting a key to a value. Examples: diff --git a/command/issue.go b/command/issue.go index e5cf24026..3d3f01708 100644 --- a/command/issue.go +++ b/command/issue.go @@ -88,13 +88,13 @@ With '--web', open the issue in a web browser instead.`, } var issueCloseCmd = &cobra.Command{ Use: "close { | }", - Short: "close issue", + Short: "Close issue", Args: cobra.ExactArgs(1), RunE: issueClose, } var issueReopenCmd = &cobra.Command{ Use: "reopen { | }", - Short: "reopen issue", + Short: "Reopen issue", Args: cobra.ExactArgs(1), RunE: issueReopen, } @@ -106,7 +106,7 @@ func issueList(cmd *cobra.Command, args []string) error { return err } - baseRepo, err := determineBaseRepo(cmd, ctx) + baseRepo, err := determineBaseRepo(apiClient, cmd, ctx) if err != nil { return err } @@ -167,7 +167,7 @@ func issueStatus(cmd *cobra.Command, args []string) error { return err } - baseRepo, err := determineBaseRepo(cmd, ctx) + baseRepo, err := determineBaseRepo(apiClient, cmd, ctx) if err != nil { return err } @@ -224,7 +224,7 @@ func issueView(cmd *cobra.Command, args []string) error { return err } - baseRepo, err := determineBaseRepo(cmd, ctx) + baseRepo, err := determineBaseRepo(apiClient, cmd, ctx) if err != nil { return err } @@ -340,9 +340,13 @@ func issueFromArg(apiClient *api.Client, baseRepo ghrepo.Interface, arg string) 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(cmd, ctx) + baseRepo, err := determineBaseRepo(apiClient, cmd, ctx) if err != nil { return err } @@ -381,9 +385,11 @@ func issueCreate(cmd *cobra.Command, args []string) error { if err != nil { return fmt.Errorf("could not parse projects: %w", err) } - milestoneTitle, err := cmd.Flags().GetString("milestone") - if err != nil { + 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 { @@ -404,11 +410,6 @@ func issueCreate(cmd *cobra.Command, args []string) error { fmt.Fprintf(colorableErr(cmd), "\nCreating issue in %s\n\n", ghrepo.FullName(baseRepo)) - apiClient, err := apiClientForContext(ctx) - if err != nil { - return err - } - repo, err := api.GitHubRepo(apiClient, baseRepo) if err != nil { return err @@ -419,10 +420,10 @@ func issueCreate(cmd *cobra.Command, args []string) error { action := SubmitAction tb := issueMetadataState{ - Assignees: assignees, - Labels: labelNames, - Projects: projectNames, - Milestone: milestoneTitle, + Assignees: assignees, + Labels: labelNames, + Projects: projectNames, + Milestones: milestoneTitles, } interactive := !(cmd.Flags().Changed("title") && cmd.Flags().Changed("body")) @@ -469,26 +470,9 @@ func issueCreate(cmd *cobra.Command, args []string) error { "body": body, } - if tb.HasMetadata() { - if tb.MetadataResult == nil { - metadataInput := api.RepoMetadataInput{ - Assignees: len(tb.Assignees) > 0, - Labels: len(tb.Labels) > 0, - Projects: len(tb.Projects) > 0, - Milestones: tb.Milestone != "", - } - - // TODO: for non-interactive mode, only translate given objects to GraphQL IDs - tb.MetadataResult, err = api.RepoMetadata(apiClient, baseRepo, metadataInput) - if err != nil { - return err - } - } - - err = addMetadataToIssueParams(params, tb.MetadataResult, tb.Assignees, tb.Labels, tb.Projects, tb.Milestone) - if err != nil { - return err - } + err = addMetadataToIssueParams(apiClient, baseRepo, params, &tb) + if err != nil { + return err } newIssue, err := api.IssueCreate(apiClient, repo, params) @@ -504,33 +488,79 @@ func issueCreate(cmd *cobra.Command, args []string) error { return nil } -func addMetadataToIssueParams(params map[string]interface{}, metadata *api.RepoMetadataResult, assignees, labelNames, projectNames []string, milestoneTitle string) error { - assigneeIDs, err := metadata.MembersToIDs(assignees) +func addMetadataToIssueParams(client *api.Client, baseRepo ghrepo.Interface, params map[string]interface{}, tb *issueMetadataState) error { + if !tb.HasMetadata() { + return nil + } + + if tb.MetadataResult == nil { + resolveInput := api.RepoResolveInput{ + Reviewers: tb.Reviewers, + Assignees: tb.Assignees, + Labels: tb.Labels, + Projects: tb.Projects, + Milestones: tb.Milestones, + } + + var err error + tb.MetadataResult, err = api.RepoResolveMetadataIDs(client, baseRepo, resolveInput) + if err != nil { + return err + } + } + + assigneeIDs, err := tb.MetadataResult.MembersToIDs(tb.Assignees) if err != nil { return fmt.Errorf("could not assign user: %w", err) } params["assigneeIds"] = assigneeIDs - labelIDs, err := metadata.LabelsToIDs(labelNames) + labelIDs, err := tb.MetadataResult.LabelsToIDs(tb.Labels) if err != nil { return fmt.Errorf("could not add label: %w", err) } params["labelIds"] = labelIDs - projectIDs, err := metadata.ProjectsToIDs(projectNames) + projectIDs, err := tb.MetadataResult.ProjectsToIDs(tb.Projects) if err != nil { return fmt.Errorf("could not add to project: %w", err) } params["projectIds"] = projectIDs - if milestoneTitle != "" { - milestoneID, err := metadata.MilestoneToID(milestoneTitle) + if len(tb.Milestones) > 0 { + milestoneID, err := tb.MetadataResult.MilestoneToID(tb.Milestones[0]) if err != nil { - return fmt.Errorf("could not add to milestone '%s': %w", milestoneTitle, err) + return fmt.Errorf("could not add to milestone '%s': %w", tb.Milestones[0], err) } params["milestoneId"] = milestoneID } + if len(tb.Reviewers) == 0 { + return nil + } + + var userReviewers []string + var teamReviewers []string + for _, r := range tb.Reviewers { + if strings.ContainsRune(r, '/') { + teamReviewers = append(teamReviewers, r) + } else { + userReviewers = append(userReviewers, r) + } + } + + userReviewerIDs, err := tb.MetadataResult.MembersToIDs(userReviewers) + if err != nil { + return fmt.Errorf("could not request reviewer: %w", err) + } + params["userReviewerIds"] = userReviewerIDs + + teamReviewerIDs, err := tb.MetadataResult.TeamsToIDs(teamReviewers) + if err != nil { + return fmt.Errorf("could not request reviewer: %w", err) + } + params["teamReviewerIds"] = teamReviewerIDs + return nil } @@ -623,7 +653,7 @@ func issueClose(cmd *cobra.Command, args []string) error { return err } - baseRepo, err := determineBaseRepo(cmd, ctx) + baseRepo, err := determineBaseRepo(apiClient, cmd, ctx) if err != nil { return err } @@ -658,7 +688,7 @@ func issueReopen(cmd *cobra.Command, args []string) error { return err } - baseRepo, err := determineBaseRepo(cmd, ctx) + baseRepo, err := determineBaseRepo(apiClient, cmd, ctx) if err != nil { return err } diff --git a/command/issue_test.go b/command/issue_test.go index 5a9d4c239..3df6910f5 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -500,27 +500,15 @@ func TestIssueCreate_metadata(t *testing.T) { } } } `)) http.Register( - httpmock.GraphQL(`\bassignableUsers\(`), + httpmock.GraphQL(`\bu000:`), httpmock.StringResponse(` - { "data": { "repository": { "assignableUsers": { - "nodes": [ - { "login": "hubot", "id": "HUBOTID" }, - { "login": "MonaLisa", "id": "MONAID" } - ], - "pageInfo": { "hasNextPage": false } - } } } } - `)) - http.Register( - httpmock.GraphQL(`\blabels\(`), - httpmock.StringResponse(` - { "data": { "repository": { "labels": { - "nodes": [ - { "name": "feature", "id": "FEATUREID" }, - { "name": "TODO", "id": "TODOID" }, - { "name": "bug", "id": "BUGID" } - ], - "pageInfo": { "hasNextPage": false } - } } } } + { "data": { + "u000": { "login": "MonaLisa", "id": "MONAID" }, + "repository": { + "l000": { "name": "bug", "id": "BUGID" }, + "l001": { "name": "TODO", "id": "TODOID" } + } + } } `)) http.Register( httpmock.GraphQL(`\bmilestones\(`), @@ -568,6 +556,12 @@ func TestIssueCreate_metadata(t *testing.T) { 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'`) diff --git a/command/pr.go b/command/pr.go index 2407e0867..3bf36b432 100644 --- a/command/pr.go +++ b/command/pr.go @@ -29,6 +29,7 @@ func init() { prMergeCmd.Flags().BoolP("merge", "m", true, "Merge the commits with the base branch") prMergeCmd.Flags().BoolP("rebase", "r", false, "Rebase the commits onto the base branch") prMergeCmd.Flags().BoolP("squash", "s", false, "Squash the commits into one commit and merge it into the base branch") + prCmd.AddCommand(prReadyCmd) prCmd.AddCommand(prListCmd) prListCmd.Flags().IntP("limit", "L", 30, "Maximum number of items to fetch") @@ -84,13 +85,18 @@ var prReopenCmd = &cobra.Command{ Args: cobra.ExactArgs(1), RunE: prReopen, } - var prMergeCmd = &cobra.Command{ Use: "merge [ | | ]", Short: "Merge a pull request", Args: cobra.MaximumNArgs(1), RunE: prMerge, } +var prReadyCmd = &cobra.Command{ + Use: "ready [ | | ]", + Short: "Make a pull request as ready for review", + Args: cobra.MaximumNArgs(1), + RunE: prReady, +} func prStatus(cmd *cobra.Command, args []string) error { ctx := contextForCommand(cmd) @@ -104,7 +110,7 @@ func prStatus(cmd *cobra.Command, args []string) error { return err } - baseRepo, err := determineBaseRepo(cmd, ctx) + baseRepo, err := determineBaseRepo(apiClient, cmd, ctx) if err != nil { return err } @@ -168,7 +174,7 @@ func prList(cmd *cobra.Command, args []string) error { return err } - baseRepo, err := determineBaseRepo(cmd, ctx) + baseRepo, err := determineBaseRepo(apiClient, cmd, ctx) if err != nil { return err } @@ -307,7 +313,7 @@ func prView(cmd *cobra.Command, args []string) error { } if baseRepo == nil { - baseRepo, err = determineBaseRepo(cmd, ctx) + baseRepo, err = determineBaseRepo(apiClient, cmd, ctx) if err != nil { return err } @@ -366,7 +372,7 @@ func prClose(cmd *cobra.Command, args []string) error { return err } - baseRepo, err := determineBaseRepo(cmd, ctx) + baseRepo, err := determineBaseRepo(apiClient, cmd, ctx) if err != nil { return err } @@ -401,7 +407,7 @@ func prReopen(cmd *cobra.Command, args []string) error { return err } - baseRepo, err := determineBaseRepo(cmd, ctx) + baseRepo, err := determineBaseRepo(apiClient, cmd, ctx) if err != nil { return err } @@ -438,7 +444,7 @@ func prMerge(cmd *cobra.Command, args []string) error { return err } - baseRepo, err := determineBaseRepo(cmd, ctx) + baseRepo, err := determineBaseRepo(apiClient, cmd, ctx) if err != nil { return err } @@ -551,12 +557,74 @@ func printPrPreview(out io.Writer, pr *api.PullRequest) error { return nil } +func prReady(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 + } + + var pr *api.PullRequest + if len(args) > 0 { + var prNumber string + n, _ := prFromURL(args[0]) + if n != "" { + prNumber = n + } else { + prNumber = args[0] + } + + pr, err = prFromArg(apiClient, baseRepo, prNumber) + if err != nil { + return err + } + } else { + prNumber, branchWithOwner, err := prSelectorForCurrentBranch(ctx, baseRepo) + if err != nil { + return err + } + + if prNumber != 0 { + pr, err = api.PullRequestByNumber(apiClient, baseRepo, prNumber) + } else { + pr, err = api.PullRequestForBranch(apiClient, baseRepo, "", branchWithOwner) + } + if err != nil { + return err + } + } + + if pr.Closed { + err := fmt.Errorf("%s Pull request #%d is closed. Only draft pull requests can be marked as \"ready for review\"", utils.Red("!"), pr.Number) + return err + } else if !pr.IsDraft { + fmt.Fprintf(colorableErr(cmd), "%s Pull request #%d is already \"ready for review\"\n", utils.Yellow("!"), pr.Number) + return nil + } + + err = api.PullRequestReady(apiClient, baseRepo, pr) + if err != nil { + return fmt.Errorf("API call failed: %w", err) + } + + fmt.Fprintf(colorableErr(cmd), "%s Pull request #%d is marked as \"ready for review\"\n", utils.Green("✔"), pr.Number) + + return nil +} + // Ref. https://developer.github.com/v4/enum/pullrequestreviewstate/ const ( requestedReviewState = "REQUESTED" // This is our own state for review request approvedReviewState = "APPROVED" changesRequestedReviewState = "CHANGES_REQUESTED" commentedReviewState = "COMMENTED" + dismissedReviewState = "DISMISSED" + pendingReviewState = "PENDING" ) type reviewerState struct { @@ -582,8 +650,14 @@ func colorFuncForReviewerState(state string) func(string) string { // formattedReviewerState formats a reviewerState with state color func formattedReviewerState(reviewer *reviewerState) string { - stateColorFunc := colorFuncForReviewerState(reviewer.State) - return fmt.Sprintf("%s (%s)", reviewer.Name, stateColorFunc(strings.ReplaceAll(strings.Title(strings.ToLower(reviewer.State)), "_", " "))) + state := reviewer.State + if state == dismissedReviewState { + // Show "DISMISSED" review as "COMMENTED", since "dimissed" only makes + // sense when displayed in an events timeline but not in the final tally. + state = commentedReviewState + } + stateColorFunc := colorFuncForReviewerState(state) + return fmt.Sprintf("%s (%s)", reviewer.Name, stateColorFunc(strings.ReplaceAll(strings.Title(strings.ToLower(state)), "_", " "))) } // prReviewerList generates a reviewer list with their last state @@ -639,6 +713,9 @@ func parseReviewers(pr api.PullRequest) []*reviewerState { // Convert map to slice for ease of sort result := make([]*reviewerState, 0, len(reviewerStates)) for _, reviewer := range reviewerStates { + if reviewer.State == pendingReviewState { + continue + } result = append(result, reviewer) } diff --git a/command/pr_checkout.go b/command/pr_checkout.go index 7da76f6ce..aa9e3afcc 100644 --- a/command/pr_checkout.go +++ b/command/pr_checkout.go @@ -34,7 +34,7 @@ func prCheckout(cmd *cobra.Command, args []string) error { } if baseRepo == nil { - baseRepo, err = determineBaseRepo(cmd, ctx) + baseRepo, err = determineBaseRepo(apiClient, cmd, ctx) if err != nil { return err } diff --git a/command/pr_create.go b/command/pr_create.go index 4ce58a681..96dc08791 100644 --- a/command/pr_create.go +++ b/command/pr_create.go @@ -140,9 +140,11 @@ func prCreate(cmd *cobra.Command, _ []string) error { if err != nil { return fmt.Errorf("could not parse projects: %w", err) } - milestoneTitle, err := cmd.Flags().GetString("milestone") - if err != nil { + 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) } baseTrackingBranch := baseBranch @@ -201,11 +203,11 @@ func prCreate(cmd *cobra.Command, _ []string) error { } tb := issueMetadataState{ - Reviewers: reviewers, - Assignees: assignees, - Labels: labelNames, - Projects: projectNames, - Milestone: milestoneTitle, + Reviewers: reviewers, + Assignees: assignees, + Labels: labelNames, + Projects: projectNames, + Milestones: milestoneTitles, } interactive := !(cmd.Flags().Changed("title") && cmd.Flags().Changed("body")) @@ -323,49 +325,9 @@ func prCreate(cmd *cobra.Command, _ []string) error { "headRefName": headBranchLabel, } - if tb.HasMetadata() { - if tb.MetadataResult == nil { - metadataInput := api.RepoMetadataInput{ - Reviewers: len(tb.Reviewers) > 0, - Assignees: len(tb.Assignees) > 0, - Labels: len(tb.Labels) > 0, - Projects: len(tb.Projects) > 0, - Milestones: tb.Milestone != "", - } - - // TODO: for non-interactive mode, only translate given objects to GraphQL IDs - tb.MetadataResult, err = api.RepoMetadata(client, baseRepo, metadataInput) - if err != nil { - return err - } - } - - err = addMetadataToIssueParams(params, tb.MetadataResult, tb.Assignees, tb.Labels, tb.Projects, tb.Milestone) - if err != nil { - return err - } - - var userReviewers []string - var teamReviewers []string - for _, r := range tb.Reviewers { - if strings.ContainsRune(r, '/') { - teamReviewers = append(teamReviewers, r) - } else { - userReviewers = append(teamReviewers, r) - } - } - - userReviewerIDs, err := tb.MetadataResult.MembersToIDs(userReviewers) - if err != nil { - return fmt.Errorf("could not request reviewer: %w", err) - } - params["userReviewerIds"] = userReviewerIDs - - teamReviewerIDs, err := tb.MetadataResult.TeamsToIDs(teamReviewers) - if err != nil { - return fmt.Errorf("could not request reviewer: %w", err) - } - params["teamReviewerIds"] = teamReviewerIDs + err = addMetadataToIssueParams(client, baseRepo, params, &tb) + if err != nil { + return err } pr, err := api.CreatePullRequest(client, baseRepo, params) diff --git a/command/pr_create_test.go b/command/pr_create_test.go index 85966aa72..e13ecc15f 100644 --- a/command/pr_create_test.go +++ b/command/pr_create_test.go @@ -87,27 +87,20 @@ func TestPRCreate_metadata(t *testing.T) { ] } } } } `)) http.Register( - httpmock.GraphQL(`\bassignableUsers\(`), + httpmock.GraphQL(`\bteam\(`), httpmock.StringResponse(` - { "data": { "repository": { "assignableUsers": { - "nodes": [ - { "login": "hubot", "id": "HUBOTID" }, - { "login": "MonaLisa", "id": "MONAID" } - ], - "pageInfo": { "hasNextPage": false } - } } } } - `)) - http.Register( - httpmock.GraphQL(`\blabels\(`), - httpmock.StringResponse(` - { "data": { "repository": { "labels": { - "nodes": [ - { "name": "feature", "id": "FEATUREID" }, - { "name": "TODO", "id": "TODOID" }, - { "name": "bug", "id": "BUGID" } - ], - "pageInfo": { "hasNextPage": false } - } } } } + { "data": { + "u000": { "login": "MonaLisa", "id": "MONAID" }, + "u001": { "login": "hubot", "id": "HUBOTID" }, + "repository": { + "l000": { "name": "bug", "id": "BUGID" }, + "l001": { "name": "TODO", "id": "TODOID" } + }, + "organization": { + "t000": { "slug": "core", "id": "COREID" }, + "t001": { "slug": "robots", "id": "ROBOTID" } + } + } } `)) http.Register( httpmock.GraphQL(`\bmilestones\(`), @@ -139,17 +132,6 @@ func TestPRCreate_metadata(t *testing.T) { "pageInfo": { "hasNextPage": false } } } } } `)) - http.Register( - httpmock.GraphQL(`\borganization\(.+\bteams\(`), - httpmock.StringResponse(` - { "data": { "organization": { "teams": { - "nodes": [ - { "slug": "owners", "id": "OWNERSID" }, - { "slug": "Core", "id": "COREID" } - ], - "pageInfo": { "hasNextPage": false } - } } } } - `)) http.Register( httpmock.GraphQL(`\bcreatePullRequest\(`), httpmock.GraphQLMutation(` @@ -160,6 +142,12 @@ func TestPRCreate_metadata(t *testing.T) { `, func(inputs map[string]interface{}) { eq(t, inputs["title"], "TITLE") eq(t, inputs["body"], "BODY") + if v, ok := inputs["assigneeIds"]; ok { + t.Errorf("did not expect assigneeIds: %v", v) + } + if v, ok := inputs["userIds"]; ok { + t.Errorf("did not expect userIds: %v", v) + } })) http.Register( httpmock.GraphQL(`\bupdatePullRequest\(`), @@ -182,8 +170,9 @@ func TestPRCreate_metadata(t *testing.T) { } } } `, func(inputs map[string]interface{}) { eq(t, inputs["pullRequestId"], "NEWPULLID") - eq(t, inputs["userIds"], []interface{}{"HUBOTID"}) - eq(t, inputs["teamIds"], []interface{}{"COREID"}) + eq(t, inputs["userIds"], []interface{}{"HUBOTID", "MONAID"}) + eq(t, inputs["teamIds"], []interface{}{"COREID", "ROBOTID"}) + eq(t, inputs["union"], true) })) cs, cmdTeardown := test.InitCmdStubber() @@ -195,7 +184,7 @@ func TestPRCreate_metadata(t *testing.T) { cs.Stub("1234567890,commit 0\n2345678901,commit 1") // git log cs.Stub("") // git push - output, err := RunCommand(`pr create -t TITLE -b BODY -a monalisa -l bug -l todo -p roadmap -m 'big one.oh' -r hubot -r /core`) + output, err := RunCommand(`pr create -t TITLE -b BODY -a monalisa -l bug -l todo -p roadmap -m 'big one.oh' -r hubot -r monalisa -r /core -r /robots`) eq(t, err, nil) eq(t, output.String(), "https://github.com/OWNER/REPO/pull/12\n") diff --git a/command/pr_diff.go b/command/pr_diff.go new file mode 100644 index 000000000..6e4bb4bdb --- /dev/null +++ b/command/pr_diff.go @@ -0,0 +1,138 @@ +package command + +import ( + "errors" + "fmt" + "os" + "strconv" + "strings" + + "github.com/cli/cli/api" + "github.com/cli/cli/utils" + "github.com/spf13/cobra" +) + +var prDiffCmd = &cobra.Command{ + Use: "diff { | }", + Short: "View a pull request's changes.", + RunE: prDiff, +} + +func init() { + prDiffCmd.Flags().StringP("color", "c", "auto", "Whether or not to output color: {always|never|auto}") + + prCmd.AddCommand(prDiffCmd) +} + +func prDiff(cmd *cobra.Command, args []string) error { + color, err := cmd.Flags().GetString("color") + if err != nil { + return err + } + if !validColorFlag(color) { + return fmt.Errorf("did not understand color: %q. Expected one of always, never, or auto", color) + } + + ctx := contextForCommand(cmd) + apiClient, err := apiClientForContext(ctx) + if err != nil { + return err + } + + baseRepo, err := determineBaseRepo(apiClient, cmd, ctx) + if err != nil { + return fmt.Errorf("could not determine base repo: %w", err) + } + + // begin pr resolution boilerplate + var prNum int + branchWithOwner := "" + + if len(args) == 0 { + prNum, branchWithOwner, err = prSelectorForCurrentBranch(ctx, baseRepo) + if err != nil { + return fmt.Errorf("could not query for pull request for current branch: %w", err) + } + } else { + prArg, repo := prFromURL(args[0]) + if repo != nil { + baseRepo = repo + } else { + prArg = strings.TrimPrefix(args[0], "#") + } + prNum, err = strconv.Atoi(prArg) + if err != nil { + return errors.New("could not parse pull request argument") + } + } + + if prNum < 1 { + pr, err := api.PullRequestForBranch(apiClient, baseRepo, "", branchWithOwner) + if err != nil { + return fmt.Errorf("could not find pull request: %w", err) + } + prNum = pr.Number + } + // end pr resolution boilerplate + + diff, err := apiClient.PullRequestDiff(baseRepo, prNum) + if err != nil { + return fmt.Errorf("could not find pull request diff: %w", err) + } + + out := cmd.OutOrStdout() + if color == "auto" { + color = "never" + isTTY := false + if outFile, isFile := out.(*os.File); isFile { + isTTY = utils.IsTerminal(outFile) + if isTTY { + color = "always" + } + } + } + + if color == "never" { + fmt.Fprint(out, diff) + return nil + } + + out = colorableOut(cmd) + for _, diffLine := range strings.Split(diff, "\n") { + output := diffLine + switch { + case isHeaderLine(diffLine): + output = utils.Bold(diffLine) + case isAdditionLine(diffLine): + output = utils.Green(diffLine) + case isRemovalLine(diffLine): + output = utils.Red(diffLine) + } + + fmt.Fprintln(out, output) + } + + return nil +} + +func isHeaderLine(dl string) bool { + prefixes := []string{"+++", "---", "diff", "index"} + for _, p := range prefixes { + if strings.HasPrefix(dl, p) { + return true + } + } + return false +} + +func isAdditionLine(dl string) bool { + return strings.HasPrefix(dl, "+") +} + +func isRemovalLine(dl string) bool { + return strings.HasPrefix(dl, "-") +} + +func validColorFlag(c string) bool { + return c == "auto" || c == "always" || c == "never" +} diff --git a/command/pr_diff_test.go b/command/pr_diff_test.go new file mode 100644 index 000000000..ab883dcab --- /dev/null +++ b/command/pr_diff_test.go @@ -0,0 +1,99 @@ +package command + +import ( + "bytes" + "testing" +) + +func TestPRDiff_validation(t *testing.T) { + _, err := RunCommand("pr diff --color=doublerainbow") + if err == nil { + t.Fatal("expected error") + } + eq(t, err.Error(), `did not understand color: "doublerainbow". Expected one of always, never, or auto`) +} + +func TestPRDiff_no_current_pr(t *testing.T) { + initBlankContext("", "OWNER/REPO", "master") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { "pullRequests": { "nodes": [ + { "url": "https://github.com/OWNER/REPO/pull/123", + "number": 123, + "id": "foobar123", + "headRefName": "feature", + "baseRefName": "master" } + ] } } } }`)) + http.StubResponse(200, bytes.NewBufferString(testDiff)) + _, err := RunCommand("pr diff") + if err == nil { + t.Fatal("expected error", err) + } + eq(t, err.Error(), `could not find pull request: no open pull requests found for branch "master"`) +} + +func TestPRDiff_argument_not_found(t *testing.T) { + initBlankContext("", "OWNER/REPO", "master") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + http.StubResponse(404, bytes.NewBufferString("")) + _, err := RunCommand("pr diff 123") + if err == nil { + t.Fatal("expected error", err) + } + eq(t, err.Error(), `could not find pull request diff: pull request not found`) +} + +func TestPRDiff(t *testing.T) { + initBlankContext("", "OWNER/REPO", "feature") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { "pullRequests": { "nodes": [ + { "url": "https://github.com/OWNER/REPO/pull/123", + "number": 123, + "id": "foobar123", + "headRefName": "feature", + "baseRefName": "master" } + ] } } } }`)) + http.StubResponse(200, bytes.NewBufferString(testDiff)) + output, err := RunCommand("pr diff") + if err != nil { + t.Fatalf("unexpected error: %s", err) + } + eq(t, output.String(), testDiff) +} + +const testDiff = `diff --git a/.github/workflows/releases.yml b/.github/workflows/releases.yml +index 73974448..b7fc0154 100644 +--- a/.github/workflows/releases.yml ++++ b/.github/workflows/releases.yml +@@ -44,6 +44,11 @@ jobs: + token: ${{secrets.SITE_GITHUB_TOKEN}} + - name: Publish documentation site + if: "!contains(github.ref, '-')" # skip prereleases ++ env: ++ GIT_COMMITTER_NAME: cli automation ++ GIT_AUTHOR_NAME: cli automation ++ GIT_COMMITTER_EMAIL: noreply@github.com ++ GIT_AUTHOR_EMAIL: noreply@github.com + run: make site-publish + - name: Move project cards + if: "!contains(github.ref, '-')" # skip prereleases +diff --git a/Makefile b/Makefile +index f2b4805c..3d7bd0f9 100644 +--- a/Makefile ++++ b/Makefile +@@ -22,8 +22,8 @@ test: + go test ./... + .PHONY: test + +-site: +- git clone https://github.com/github/cli.github.com.git "$@" ++site: bin/gh ++ bin/gh repo clone github/cli.github.com "$@" + + site-docs: site + git -C site pull +` diff --git a/command/pr_review.go b/command/pr_review.go index d51aaff03..3b1a4ab88 100644 --- a/command/pr_review.go +++ b/command/pr_review.go @@ -86,16 +86,16 @@ func processReviewOpt(cmd *cobra.Command) (*api.PullRequestReviewInput, error) { func prReview(cmd *cobra.Command, args []string) error { ctx := contextForCommand(cmd) - baseRepo, err := determineBaseRepo(cmd, ctx) - if err != nil { - return fmt.Errorf("could not determine base repo: %w", err) - } - apiClient, err := apiClientForContext(ctx) if err != nil { return err } + baseRepo, err := determineBaseRepo(apiClient, cmd, ctx) + if err != nil { + return fmt.Errorf("could not determine base repo: %w", err) + } + var prNum int branchWithOwner := "" diff --git a/command/pr_test.go b/command/pr_test.go index 528e2ee77..22f7102f9 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -477,7 +477,7 @@ func TestPRView_Preview(t *testing.T) { fixture: "../test/fixtures/prViewPreviewWithReviewersByNumber.json", expectedOutputs: []string{ `Blueberries are from a fork`, - `Reviewers: DEF \(Commented\), def \(Changes requested\), ghost \(Approved\), xyz \(Approved\), 123 \(Requested\), Team 1 \(Requested\), abc \(Requested\)\n`, + `Reviewers: DEF \(Commented\), def \(Changes requested\), ghost \(Approved\), hubot \(Commented\), xyz \(Approved\), 123 \(Requested\), Team 1 \(Requested\), abc \(Requested\)\n`, `blueberries taste good`, `View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12\n`, }, @@ -1093,3 +1093,72 @@ func TestPrMerge_alreadyMerged(t *testing.T) { t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) } } + +func TestPRReady(t *testing.T) { + initBlankContext("", "OWNER/REPO", "master") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { + "pullRequest": { "number": 444, "closed": false, "isDraft": true} + } } } + `)) + http.StubResponse(200, bytes.NewBufferString(`{"id": "THE-ID"}`)) + + output, err := RunCommand("pr ready 444") + if err != nil { + t.Fatalf("error running command `pr ready`: %v", err) + } + + r := regexp.MustCompile(`Pull request #444 is marked as "ready for review"`) + + if !r.MatchString(output.Stderr()) { + t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) + } +} + +func TestPRReady_alreadyReady(t *testing.T) { + initBlankContext("", "OWNER/REPO", "master") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { + "pullRequest": { "number": 445, "closed": false, "isDraft": false} + } } } + `)) + http.StubResponse(200, bytes.NewBufferString(`{"id": "THE-ID"}`)) + + output, err := RunCommand("pr ready 445") + if err != nil { + t.Fatalf("error running command `pr ready`: %v", err) + } + + r := regexp.MustCompile(`Pull request #445 is already "ready for review"`) + + if !r.MatchString(output.Stderr()) { + t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) + } +} + +func TestPRReady_closed(t *testing.T) { + initBlankContext("", "OWNER/REPO", "master") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { + "pullRequest": { "number": 446, "closed": true, "isDraft": true} + } } } + `)) + http.StubResponse(200, bytes.NewBufferString(`{"id": "THE-ID"}`)) + + _, err := RunCommand("pr ready 446") + if err == nil { + t.Fatalf("expected an error running command `pr ready` on a review that is closed!: %v", err) + } + + r := regexp.MustCompile(`Pull request #446 is closed. Only draft pull requests can be marked as "ready for review"`) + + if !r.MatchString(err.Error()) { + t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, err.Error()) + } +} diff --git a/command/repo.go b/command/repo.go index a611643be..ad208a277 100644 --- a/command/repo.go +++ b/command/repo.go @@ -28,7 +28,7 @@ func init() { repoCreateCmd.Flags().StringP("team", "t", "", "The name of the organization team to be granted access") repoCreateCmd.Flags().Bool("enable-issues", true, "Enable issues in the new repository") repoCreateCmd.Flags().Bool("enable-wiki", true, "Enable wiki in the new repository") - repoCreateCmd.Flags().Bool("public", false, "Make the new repository public") + repoCreateCmd.Flags().Bool("public", false, "Make the new repository public (default: private)") repoCmd.AddCommand(repoForkCmd) repoForkCmd.Flags().String("clone", "prompt", "Clone fork: {true|false|prompt}") @@ -189,7 +189,10 @@ func repoCreate(cmd *cobra.Command, args []string) error { if len(args) > 0 { name = args[0] if strings.Contains(name, "/") { - newRepo := ghrepo.FromFullName(name) + newRepo, err := ghrepo.FromFullName(name) + if err != nil { + return fmt.Errorf("argument error: %w", err) + } orgName = newRepo.RepoOwner() name = newRepo.RepoName() } @@ -336,7 +339,7 @@ func repoFork(cmd *cobra.Command, args []string) error { var repoToFork ghrepo.Interface inParent := false // whether or not we're forking the repo we're currently "in" if len(args) == 0 { - baseRepo, err := determineBaseRepo(cmd, ctx) + baseRepo, err := determineBaseRepo(apiClient, cmd, ctx) if err != nil { return fmt.Errorf("unable to determine base repository: %w", err) } @@ -366,9 +369,9 @@ func repoFork(cmd *cobra.Command, args []string) error { return fmt.Errorf("did not understand argument: %w", err) } } else { - repoToFork = ghrepo.FromFullName(repoArg) - if repoToFork.RepoName() == "" || repoToFork.RepoOwner() == "" { - return fmt.Errorf("could not parse owner or repo name from %s", repoArg) + repoToFork, err = ghrepo.FromFullName(repoArg) + if err != nil { + return fmt.Errorf("argument error: %w", err) } } } @@ -487,11 +490,15 @@ var Confirm = func(prompt string, result *bool) error { func repoView(cmd *cobra.Command, args []string) error { ctx := contextForCommand(cmd) + apiClient, err := apiClientForContext(ctx) + if err != nil { + return err + } var toView ghrepo.Interface if len(args) == 0 { var err error - toView, err = determineBaseRepo(cmd, ctx) + toView, err = determineBaseRepo(apiClient, cmd, ctx) if err != nil { return err } @@ -508,14 +515,14 @@ func repoView(cmd *cobra.Command, args []string) error { return fmt.Errorf("did not understand argument: %w", err) } } else { - toView = ghrepo.FromFullName(repoArg) + var err error + toView, err = ghrepo.FromFullName(repoArg) + if err != nil { + return fmt.Errorf("argument error: %w", err) + } } } - apiClient, err := apiClientForContext(ctx) - if err != nil { - return err - } repo, err := api.GitHubRepo(apiClient, toView) if err != nil { return err diff --git a/command/root.go b/command/root.go index ef88a3184..6233be957 100644 --- a/command/root.go +++ b/command/root.go @@ -216,15 +216,14 @@ func changelogURL(version string) string { return url } -func determineBaseRepo(cmd *cobra.Command, ctx context.Context) (ghrepo.Interface, error) { +func determineBaseRepo(apiClient *api.Client, cmd *cobra.Command, ctx context.Context) (ghrepo.Interface, error) { repo, err := cmd.Flags().GetString("repo") if err == nil && repo != "" { - return ghrepo.FromFullName(repo), nil - } - - apiClient, err := apiClientForContext(ctx) - if err != nil { - return nil, err + baseRepo, err := ghrepo.FromFullName(repo) + if err != nil { + return nil, fmt.Errorf("argument error: %w", err) + } + return baseRepo, nil } baseOverride, err := cmd.Flags().GetString("repo") diff --git a/command/title_body_survey.go b/command/title_body_survey.go index 488efa34a..0fb39d349 100644 --- a/command/title_body_survey.go +++ b/command/title_body_survey.go @@ -19,12 +19,12 @@ type issueMetadataState struct { Title string Action Action - Metadata []string - Reviewers []string - Assignees []string - Labels []string - Projects []string - Milestone string + Metadata []string + Reviewers []string + Assignees []string + Labels []string + Projects []string + Milestones []string MetadataResult *api.RepoMetadataResult } @@ -34,7 +34,7 @@ func (tb *issueMetadataState) HasMetadata() bool { len(tb.Assignees) > 0 || len(tb.Labels) > 0 || len(tb.Projects) > 0 || - tb.Milestone != "" + len(tb.Milestones) > 0 } const ( @@ -42,6 +42,8 @@ const ( PreviewAction CancelAction MetadataAction + + noMilestone = "(none)" ) var SurveyAsk = func(qs []*survey.Question, response interface{}, opts ...survey.AskOpt) error { @@ -157,6 +159,7 @@ func titleBodySurvey(cmd *cobra.Command, issueState *issueMetadataState, apiClie bodyQuestion := &survey.Question{ Name: "body", Prompt: &surveyext.GhEditor{ + BlankAllowed: true, EditorCommand: editorCommand, Editor: &survey.Editor{ Message: "Body", @@ -251,7 +254,7 @@ func titleBodySurvey(cmd *cobra.Command, issueState *issueMetadataState, apiClie for _, l := range issueState.MetadataResult.Projects { projects = append(projects, l.Name) } - milestones := []string{"(none)"} + milestones := []string{noMilestone} for _, m := range issueState.MetadataResult.Milestones { milestones = append(milestones, m.Title) } @@ -315,12 +318,16 @@ func titleBodySurvey(cmd *cobra.Command, issueState *issueMetadataState, apiClie } if isChosen("Milestone") { if len(milestones) > 1 { + var milestoneDefault interface{} + if len(issueState.Milestones) > 0 { + milestoneDefault = issueState.Milestones[0] + } mqs = append(mqs, &survey.Question{ Name: "milestone", Prompt: &survey.Select{ Message: "Milestone", Options: milestones, - Default: issueState.Milestone, + Default: milestoneDefault, }, }) } else { @@ -333,8 +340,8 @@ func titleBodySurvey(cmd *cobra.Command, issueState *issueMetadataState, apiClie return fmt.Errorf("could not prompt: %w", err) } - if issueState.Milestone == "(none)" { - issueState.Milestone = "" + if len(issueState.Milestones) > 0 && issueState.Milestones[0] == noMilestone { + issueState.Milestones = issueState.Milestones[0:0] } allowPreview = !issueState.HasMetadata() diff --git a/context/blank_context.go b/context/blank_context.go index c5ceddff7..343f32c7e 100644 --- a/context/blank_context.go +++ b/context/blank_context.go @@ -92,5 +92,6 @@ func (c *blankContext) BaseRepo() (ghrepo.Interface, error) { } func (c *blankContext) SetBaseRepo(nwo string) { - c.baseRepo = ghrepo.FromFullName(nwo) + repo, _ := ghrepo.FromFullName(nwo) + c.baseRepo = repo } diff --git a/context/context.go b/context/context.go index a5cd823fc..2a25f5dda 100644 --- a/context/context.go +++ b/context/context.go @@ -39,7 +39,7 @@ func ResolveRemotesToRepos(remotes Remotes, client *api.Client, base string) (Re } hasBaseOverride := base != "" - baseOverride := ghrepo.FromFullName(base) + baseOverride, _ := ghrepo.FromFullName(base) foundBaseOverride := false repos := make([]ghrepo.Interface, 0, lenRemotesForLookup) for _, r := range remotes[:lenRemotesForLookup] { @@ -266,5 +266,5 @@ func (c *fsContext) BaseRepo() (ghrepo.Interface, error) { } func (c *fsContext) SetBaseRepo(nwo string) { - c.baseRepo = ghrepo.FromFullName(nwo) + c.baseRepo, _ = ghrepo.FromFullName(nwo) } diff --git a/docs/source.md b/docs/source.md index 7df8bfe60..059ef2094 100644 --- a/docs/source.md +++ b/docs/source.md @@ -1,37 +1,29 @@ # Installation from source 0. Verify that you have Go 1.14+ installed -``` -$ go version -go version go1.14 -``` -1. Clone cli into `~/.githubcli` -``` -$ git clone https://github.com/cli/cli.git ~/.githubcli -``` + ```sh + $ go version + go version go1.14 + ``` -2. Compile -``` -$ cd ~/.githubcli && make -``` +1. Clone this repository -3. Add `~/.githubcli/bin` to your $PATH for access to the gh command-line utility. + ```sh + $ git clone https://github.com/cli/cli.git gh-cli + $ cd gh-cli + ``` - * For **bash**: - ~~~ bash - $ echo 'export PATH="$HOME/.githubcli/bin:$PATH"' >> ~/.bash_profile - ~~~ - - * For **Zsh**: - ~~~ zsh - $ echo 'export PATH="$HOME/.githubcli/bin:$PATH"' >> ~/.zshrc - ~~~ - - * For **Fish shell**: - ~~~ fish - $ set -Ux fish_user_paths $HOME/.githubcli/bin $fish_user_paths - ~~~ +2. Build the project -4. Restart your shell so that PATH changes take effect. + ``` + $ make + ``` +3. Move the resulting `bin/gh` executable to somewhere in your PATH + + ```sh + $ sudo mv ./bin/gh /usr/local/bin/ + ``` + +4. Run `gh version` to check if it worked. diff --git a/internal/ghrepo/repo.go b/internal/ghrepo/repo.go index dc3115b90..37fecbe8d 100644 --- a/internal/ghrepo/repo.go +++ b/internal/ghrepo/repo.go @@ -28,14 +28,15 @@ func FullName(r Interface) string { return fmt.Sprintf("%s/%s", r.RepoOwner(), r.RepoName()) } -// FromFullName extracts the GitHub repository inforation from an "OWNER/REPO" string -func FromFullName(nwo string) Interface { +// FromFullName extracts the GitHub repository information from an "OWNER/REPO" string +func FromFullName(nwo string) (Interface, error) { var r ghRepo parts := strings.SplitN(nwo, "/", 2) - if len(parts) == 2 { - r.owner, r.name = parts[0], parts[1] + if len(parts) != 2 || parts[0] == "" || parts[1] == "" { + return &r, fmt.Errorf("expected OWNER/REPO format, got %q", nwo) } - return &r + r.owner, r.name = parts[0], parts[1] + return &r, nil } // FromURL extracts the GitHub repository information from a URL diff --git a/pkg/httpmock/stub.go b/pkg/httpmock/stub.go index b27ceea6c..48077ed4e 100644 --- a/pkg/httpmock/stub.go +++ b/pkg/httpmock/stub.go @@ -88,6 +88,22 @@ func GraphQLMutation(body string, cb func(map[string]interface{})) Responder { } } +func GraphQLQuery(body string, cb func(string, map[string]interface{})) Responder { + return func(req *http.Request) (*http.Response, error) { + var bodyData struct { + Query string + Variables map[string]interface{} + } + err := decodeJSONBody(req, &bodyData) + if err != nil { + return nil, err + } + cb(bodyData.Query, bodyData.Variables) + + return httpResponse(200, bytes.NewBufferString(body)), nil + } +} + func httpResponse(status int, body io.Reader) *http.Response { return &http.Response{ StatusCode: status, diff --git a/test/fixtures/prViewPreviewWithReviewersByNumber.json b/test/fixtures/prViewPreviewWithReviewersByNumber.json index 791512d41..4d2bd57af 100644 --- a/test/fixtures/prViewPreviewWithReviewersByNumber.json +++ b/test/fixtures/prViewPreviewWithReviewersByNumber.json @@ -70,6 +70,24 @@ "login": "" }, "state": "APPROVED" + }, + { + "author": { + "login": "hubot" + }, + "state": "CHANGES_REQUESTED" + }, + { + "author": { + "login": "hubot" + }, + "state": "DISMISSED" + }, + { + "author": { + "login": "monalisa" + }, + "state": "PENDING" } ] },