diff --git a/.github/workflows/releases.yml b/.github/workflows/releases.yml index 2f9908167..9995a483e 100644 --- a/.github/workflows/releases.yml +++ b/.github/workflows/releases.yml @@ -17,6 +17,7 @@ jobs: go-version: 1.13 - name: Generate changelog run: | + echo ::set-env name=GORELEASER_CURRENT_TAG::${GITHUB_REF#refs/tags/} git fetch --unshallow script/changelog | tee CHANGELOG.md - name: Run GoReleaser @@ -26,18 +27,6 @@ jobs: args: release --release-notes=CHANGELOG.md env: GITHUB_TOKEN: ${{secrets.UPLOAD_GITHUB_TOKEN}} - - name: move cards - if: "!contains(github.ref, '-')" - env: - GITHUB_TOKEN: ${{secrets.GITHUB_TOKEN}} - PENDING_COLUMN: 8189733 - DONE_COLUMN: 7110130 - shell: bash - run: | - curl -fsSL https://github.com/github/hub/raw/master/script/get | bash -s 2.14.1 - api() { bin/hub api -H 'accept: application/vnd.github.inertia-preview+json' "$@"; } - cards=$(api projects/columns/$PENDING_COLUMN/cards | jq ".[].id") - for card in $cards; do api projects/columns/cards/$card/moves --field position=top --field column_id=$DONE_COLUMN; done - name: Bump homebrew-core formula uses: mislav/bump-homebrew-formula-action@v1 if: "!contains(github.ref, '-')" # skip prereleases @@ -45,6 +34,28 @@ jobs: formula-name: gh env: COMMITTER_TOKEN: ${{ secrets.UPLOAD_GITHUB_TOKEN }} + - name: Checkout documentation site + if: "!contains(github.ref, '-')" # skip prereleases + uses: actions/checkout@v2 + with: + repository: github/cli.github.com + path: site + fetch-depth: 0 + token: ${{secrets.SITE_GITHUB_TOKEN}} + - name: Publish documentation site + if: "!contains(github.ref, '-')" # skip prereleases + run: make site-publish + - name: Move project cards + if: "!contains(github.ref, '-')" # skip prereleases + env: + GITHUB_TOKEN: ${{secrets.GITHUB_TOKEN}} + PENDING_COLUMN: 8189733 + DONE_COLUMN: 7110130 + run: | + curl -fsSL https://github.com/github/hub/raw/master/script/get | bash -s 2.14.1 + api() { bin/hub api -H 'accept: application/vnd.github.inertia-preview+json' "$@"; } + cards=$(api projects/columns/$PENDING_COLUMN/cards | jq ".[].id") + for card in $cards; do api projects/columns/cards/$card/moves --field position=top --field column_id=$DONE_COLUMN; done msi: needs: goreleaser runs-on: windows-latest diff --git a/Makefile b/Makefile index 5a93dac9b..f2b4805c8 100644 --- a/Makefile +++ b/Makefile @@ -32,5 +32,15 @@ site-docs: site 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' - git -C site commit -m 'update help docs' + git -C site commit -m 'update help docs' || true .PHONY: site-docs + +site-publish: site-docs +ifndef GITHUB_REF + $(error GITHUB_REF is not set) +endif + sed -i.bak -E 's/(assign version = )".+"/\1"$(GITHUB_REF:refs/tags/v%=%)"/' site/index.html + rm -f site/index.html.bak + git -C site commit -m '$(GITHUB_REF:refs/tags/v%=%)' index.html + git -C site push +.PHONY: site-publish diff --git a/README.md b/README.md index 5c96a685f..c4c0e9a5d 100644 --- a/README.md +++ b/README.md @@ -7,7 +7,7 @@ the terminal next to where you are already working with `git` and your code. ## Availability -While in beta, GitHub CLI is available for repos hosted on GitHub.com only. It does not currently support repositories hosted on GitHub Enterprise Server or other hosting providers. We are planning support for GitHub Enterprise Server after GitHub CLI is out of beta (likely toward the end of 2020), and we want to ensure that the API endpoints we use are more widely available for GHES versions that most GitHub customers are on. +While in beta, GitHub CLI is available for repos hosted on GitHub.com only. It does not currently support repositories hosted on GitHub Enterprise Server or other hosting providers. We are planning support for GitHub Enterprise Server after GitHub CLI is out of beta (likely toward the end of 2020), and we want to ensure that the API endpoints we use are more widely available for GHES versions that most GitHub customers are on. ## We need your feedback @@ -22,6 +22,7 @@ And if you spot bugs or have features that you'd really like to see in `gh`, ple - `gh pr [status, list, view, checkout, create]` - `gh issue [status, list, view, create]` - `gh repo [view, create, clone, fork]` +- `gh config [get, set]` - `gh help` ## Documentation diff --git a/api/client.go b/api/client.go index 2fc72aaa6..d7b981bee 100644 --- a/api/client.go +++ b/api/client.go @@ -37,6 +37,16 @@ func AddHeader(name, value string) ClientOption { } } +// AddHeaderFunc is an AddHeader that gets the string value from a function +func AddHeaderFunc(name string, value func() string) ClientOption { + return func(tr http.RoundTripper) http.RoundTripper { + return &funcTripper{roundTrip: func(req *http.Request) (*http.Response, error) { + req.Header.Add(name, value()) + return tr.RoundTrip(req) + }} + } +} + // VerboseLog enables request/response logging within a RoundTripper func VerboseLog(out io.Writer, logTraffic bool, colorize bool) ClientOption { logger := &httpretty.Logger{ @@ -63,6 +73,40 @@ func ReplaceTripper(tr http.RoundTripper) ClientOption { } } +var issuedScopesWarning bool + +// CheckScopes checks whether an OAuth scope is present in a response +func CheckScopes(wantedScope string, cb func(string) error) ClientOption { + return func(tr http.RoundTripper) http.RoundTripper { + return &funcTripper{roundTrip: func(req *http.Request) (*http.Response, error) { + res, err := tr.RoundTrip(req) + if err != nil || res.StatusCode > 299 || issuedScopesWarning { + return res, err + } + + appID := res.Header.Get("X-Oauth-Client-Id") + hasScopes := strings.Split(res.Header.Get("X-Oauth-Scopes"), ",") + + hasWanted := false + for _, s := range hasScopes { + if wantedScope == strings.TrimSpace(s) { + hasWanted = true + break + } + } + + if !hasWanted { + if err := cb(appID); err != nil { + return res, err + } + issuedScopesWarning = true + } + + return res, nil + }} + } +} + type funcTripper struct { roundTrip func(*http.Request) (*http.Response, error) } diff --git a/api/queries_issue.go b/api/queries_issue.go index cc01c039f..56a04edd9 100644 --- a/api/queries_issue.go +++ b/api/queries_issue.go @@ -1,10 +1,12 @@ package api import ( + "context" "fmt" "time" "github.com/cli/cli/internal/ghrepo" + "github.com/shurcooL/githubv4" ) type IssuesPayload struct { @@ -20,10 +22,12 @@ type IssuesAndTotalCount struct { // Ref. https://developer.github.com/v4/object/issue/ type Issue struct { + ID string Number int Title string URL string State string + Closed bool Body string CreatedAt time.Time UpdatedAt time.Time @@ -61,6 +65,10 @@ type Issue struct { } } +type IssuesDisabledError struct { + error +} + const fragments = ` fragment issue on Issue { number @@ -296,8 +304,10 @@ func IssueByNumber(client *Client, repo ghrepo.Interface, number int) (*Issue, e repository(owner: $owner, name: $repo) { hasIssuesEnabled issue(number: $issue_number) { + id title state + closed body author { login @@ -351,8 +361,51 @@ func IssueByNumber(client *Client, repo ghrepo.Interface, number int) (*Issue, e } if !resp.Repository.HasIssuesEnabled { - return nil, fmt.Errorf("the '%s' repository has disabled issues", ghrepo.FullName(repo)) + + return nil, &IssuesDisabledError{fmt.Errorf("the '%s' repository has disabled issues", ghrepo.FullName(repo))} } return &resp.Repository.Issue, nil } + +func IssueClose(client *Client, repo ghrepo.Interface, issue Issue) error { + var mutation struct { + CloseIssue struct { + Issue struct { + ID githubv4.ID + } + } `graphql:"closeIssue(input: $input)"` + } + + input := githubv4.CloseIssueInput{ + IssueID: issue.ID, + } + + v4 := githubv4.NewClient(client.http) + err := v4.Mutate(context.Background(), &mutation, input, nil) + + if err != nil { + return err + } + + return nil +} + +func IssueReopen(client *Client, repo ghrepo.Interface, issue Issue) error { + var mutation struct { + ReopenIssue struct { + Issue struct { + ID githubv4.ID + } + } `graphql:"reopenIssue(input: $input)"` + } + + input := githubv4.ReopenIssueInput{ + IssueID: issue.ID, + } + + v4 := githubv4.NewClient(client.http) + err := v4.Mutate(context.Background(), &mutation, input, nil) + + return err +} diff --git a/api/queries_org.go b/api/queries_org.go index 07c83b514..bc492a88a 100644 --- a/api/queries_org.go +++ b/api/queries_org.go @@ -60,7 +60,7 @@ func OrganizationProjects(client *Client, owner string) ([]RepoProject, error) { if !query.Organization.Projects.PageInfo.HasNextPage { break } - variables["endCursor"] = query.Organization.Projects.PageInfo.EndCursor + variables["endCursor"] = githubv4.String(query.Organization.Projects.PageInfo.EndCursor) } return projects, nil @@ -103,7 +103,7 @@ func OrganizationTeams(client *Client, owner string) ([]OrgTeam, error) { if !query.Organization.Teams.PageInfo.HasNextPage { break } - variables["endCursor"] = query.Organization.Teams.PageInfo.EndCursor + variables["endCursor"] = githubv4.String(query.Organization.Teams.PageInfo.EndCursor) } return teams, nil diff --git a/api/queries_repo.go b/api/queries_repo.go index b10f8cd2e..e92357cf0 100644 --- a/api/queries_repo.go +++ b/api/queries_repo.go @@ -607,7 +607,7 @@ func RepoProjects(client *Client, repo ghrepo.Interface) ([]RepoProject, error) if !query.Repository.Projects.PageInfo.HasNextPage { break } - variables["endCursor"] = query.Repository.Projects.PageInfo.EndCursor + variables["endCursor"] = githubv4.String(query.Repository.Projects.PageInfo.EndCursor) } return projects, nil @@ -651,7 +651,7 @@ func RepoAssignableUsers(client *Client, repo ghrepo.Interface) ([]RepoAssignee, if !query.Repository.AssignableUsers.PageInfo.HasNextPage { break } - variables["endCursor"] = query.Repository.AssignableUsers.PageInfo.EndCursor + variables["endCursor"] = githubv4.String(query.Repository.AssignableUsers.PageInfo.EndCursor) } return users, nil @@ -695,7 +695,7 @@ func RepoLabels(client *Client, repo ghrepo.Interface) ([]RepoLabel, error) { if !query.Repository.Labels.PageInfo.HasNextPage { break } - variables["endCursor"] = query.Repository.Labels.PageInfo.EndCursor + variables["endCursor"] = githubv4.String(query.Repository.Labels.PageInfo.EndCursor) } return labels, nil @@ -739,7 +739,7 @@ func RepoMilestones(client *Client, repo ghrepo.Interface) ([]RepoMilestone, err if !query.Repository.Milestones.PageInfo.HasNextPage { break } - variables["endCursor"] = query.Repository.Milestones.PageInfo.EndCursor + variables["endCursor"] = githubv4.String(query.Repository.Milestones.PageInfo.EndCursor) } return milestones, nil diff --git a/auth/oauth.go b/auth/oauth.go index dc7a9d085..3f9d9ddc4 100644 --- a/auth/oauth.go +++ b/auth/oauth.go @@ -11,6 +11,7 @@ import ( "net/http" "net/url" "os" + "strings" "github.com/cli/cli/pkg/browser" ) @@ -29,6 +30,7 @@ type OAuthFlow struct { Hostname string ClientID string ClientSecret string + Scopes []string WriteSuccessHTML func(io.Writer) VerboseStream io.Writer } @@ -45,11 +47,15 @@ func (oa *OAuthFlow) ObtainAccessToken() (accessToken string, err error) { } port := listener.Addr().(*net.TCPAddr).Port + scopes := "repo" + if oa.Scopes != nil { + scopes = strings.Join(oa.Scopes, " ") + } + q := url.Values{} q.Set("client_id", oa.ClientID) q.Set("redirect_uri", fmt.Sprintf("http://127.0.0.1:%d/callback", port)) - // TODO: make scopes configurable - q.Set("scope", "repo") + q.Set("scope", scopes) q.Set("state", state) startURL := fmt.Sprintf("https://%s/login/oauth/authorize?%s", oa.Hostname, q.Encode()) diff --git a/command/issue.go b/command/issue.go index 29a3bb34d..267a478dc 100644 --- a/command/issue.go +++ b/command/issue.go @@ -43,6 +43,9 @@ func init() { issueCmd.AddCommand(issueViewCmd) issueViewCmd.Flags().BoolP("web", "w", false, "Open an issue in the browser") + + issueCmd.AddCommand(issueCloseCmd) + issueCmd.AddCommand(issueReopenCmd) } var issueCmd = &cobra.Command{ @@ -83,6 +86,18 @@ var issueViewCmd = &cobra.Command{ With '--web', open the issue in a web browser instead.`, RunE: issueView, } +var issueCloseCmd = &cobra.Command{ + Use: "close ", + Short: "close issue", + Args: cobra.ExactArgs(1), + RunE: issueClose, +} +var issueReopenCmd = &cobra.Command{ + Use: "reopen ", + Short: "reopen issue", + Args: cobra.ExactArgs(1), + RunE: issueReopen, +} func issueList(cmd *cobra.Command, args []string) error { ctx := contextForCommand(cmd) @@ -582,7 +597,11 @@ func issueProjectList(issue api.Issue) string { projectNames := make([]string, 0, len(issue.ProjectCards.Nodes)) for _, project := range issue.ProjectCards.Nodes { - projectNames = append(projectNames, fmt.Sprintf("%s (%s)", project.Project.Name, project.Column.Name)) + colName := project.Column.Name + if colName == "" { + colName = "Awaiting triage" + } + projectNames = append(projectNames, fmt.Sprintf("%s (%s)", project.Project.Name, colName)) } list := strings.Join(projectNames, ", ") @@ -592,6 +611,76 @@ func issueProjectList(issue api.Issue) string { return list } +func issueClose(cmd *cobra.Command, args []string) error { + ctx := contextForCommand(cmd) + apiClient, err := apiClientForContext(ctx) + if err != nil { + return err + } + + baseRepo, err := determineBaseRepo(cmd, ctx) + if err != nil { + return err + } + + issue, err := issueFromArg(apiClient, baseRepo, args[0]) + var idErr *api.IssuesDisabledError + if errors.As(err, &idErr) { + return fmt.Errorf("issues disabled for %s", ghrepo.FullName(baseRepo)) + } else if err != nil { + return fmt.Errorf("failed to find issue #%d: %w", issue.Number, err) + } + + if issue.Closed { + fmt.Fprintf(colorableErr(cmd), "%s Issue #%d is already closed\n", utils.Yellow("!"), issue.Number) + return nil + } + + err = api.IssueClose(apiClient, baseRepo, *issue) + if err != nil { + return fmt.Errorf("API call failed:%w", err) + } + + fmt.Fprintf(colorableErr(cmd), "%s Closed issue #%d\n", utils.Red("✔"), issue.Number) + + return nil +} + +func issueReopen(cmd *cobra.Command, args []string) error { + ctx := contextForCommand(cmd) + apiClient, err := apiClientForContext(ctx) + if err != nil { + return err + } + + baseRepo, err := determineBaseRepo(cmd, ctx) + if err != nil { + return err + } + + issue, err := issueFromArg(apiClient, baseRepo, args[0]) + var idErr *api.IssuesDisabledError + if errors.As(err, &idErr) { + return fmt.Errorf("issues disabled for %s", ghrepo.FullName(baseRepo)) + } else if err != nil { + return fmt.Errorf("failed to find issue #%d: %w", issue.Number, err) + } + + if !issue.Closed { + fmt.Fprintf(colorableErr(cmd), "%s Issue #%d is already open\n", utils.Yellow("!"), issue.Number) + return nil + } + + err = api.IssueReopen(apiClient, baseRepo, *issue) + if err != nil { + return fmt.Errorf("API call failed:%w", err) + } + + fmt.Fprintf(colorableErr(cmd), "%s Reopened issue #%d\n", utils.Green("✔"), issue.Number) + + return nil +} + func displayURL(urlStr string) string { u, err := url.Parse(urlStr) if err != nil { diff --git a/command/issue_test.go b/command/issue_test.go index b7f91031d..e408ce3cc 100644 --- a/command/issue_test.go +++ b/command/issue_test.go @@ -312,7 +312,7 @@ func TestIssueView_Preview(t *testing.T) { `Open • marseilles opened about 292 years ago • 9 comments`, `Assignees: marseilles, monaco\n`, `Labels: one, two, three, four, five\n`, - `Projects: Project 1 \(column A\), Project 2 \(column B\), Project 3 \(column C\)\n`, + `Projects: Project 1 \(column A\), Project 2 \(column B\), Project 3 \(column C\), Project 4 \(Awaiting triage\)\n`, `Milestone: uluru\n`, `bold story`, `View this issue on GitHub: https://github.com/OWNER/REPO/issues/123`, @@ -680,3 +680,149 @@ func TestIssueStateTitleWithColor(t *testing.T) { }) } } + +func TestIssueClose(t *testing.T) { + initBlankContext("", "OWNER/REPO", "master") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { + "hasIssuesEnabled": true, + "issue": { "number": 13} + } } } + `)) + + http.StubResponse(200, bytes.NewBufferString(`{"id": "THE-ID"}`)) + + output, err := RunCommand(issueCloseCmd, "issue close 13") + if err != nil { + t.Fatalf("error running command `issue close`: %v", err) + } + + r := regexp.MustCompile(`Closed issue #13`) + + if !r.MatchString(output.Stderr()) { + t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) + } +} + +func TestIssueClose_alreadyClosed(t *testing.T) { + initBlankContext("", "OWNER/REPO", "master") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { + "hasIssuesEnabled": true, + "issue": { "number": 13, "closed": true} + } } } + `)) + + http.StubResponse(200, bytes.NewBufferString(`{"id": "THE-ID"}`)) + + output, err := RunCommand(issueCloseCmd, "issue close 13") + if err != nil { + t.Fatalf("error running command `issue close`: %v", err) + } + + r := regexp.MustCompile(`#13 is already closed`) + + if !r.MatchString(output.Stderr()) { + t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) + } +} + +func TestIssueClose_issuesDisabled(t *testing.T) { + initBlankContext("", "OWNER/REPO", "master") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { + "hasIssuesEnabled": false + } } } + `)) + + _, err := RunCommand(issueCloseCmd, "issue close 13") + if err == nil { + t.Fatalf("expected error when issues are disabled") + } + + if !strings.Contains(err.Error(), "issues disabled") { + t.Fatalf("got unexpected error: %s", err) + } +} + +func TestIssueReopen(t *testing.T) { + initBlankContext("", "OWNER/REPO", "master") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { + "hasIssuesEnabled": true, + "issue": { "number": 2, "closed": true} + } } } + `)) + + http.StubResponse(200, bytes.NewBufferString(`{"id": "THE-ID"}`)) + + output, err := RunCommand(issueReopenCmd, "issue reopen 2") + if err != nil { + t.Fatalf("error running command `issue reopen`: %v", err) + } + + r := regexp.MustCompile(`Reopened issue #2`) + + if !r.MatchString(output.Stderr()) { + t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) + } +} + +func TestIssueReopen_alreadyOpen(t *testing.T) { + initBlankContext("", "OWNER/REPO", "master") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { + "hasIssuesEnabled": true, + "issue": { "number": 2, "closed": false} + } } } + `)) + + http.StubResponse(200, bytes.NewBufferString(`{"id": "THE-ID"}`)) + + output, err := RunCommand(issueReopenCmd, "issue reopen 2") + if err != nil { + t.Fatalf("error running command `issue reopen`: %v", err) + } + + r := regexp.MustCompile(`#2 is already open`) + + if !r.MatchString(output.Stderr()) { + t.Fatalf("output did not match regexp /%s/\n> output\n%q\n", r, output.Stderr()) + } +} + +func TestIssueReopen_issuesDisabled(t *testing.T) { + initBlankContext("", "OWNER/REPO", "master") + http := initFakeHTTP() + http.StubRepoResponse("OWNER", "REPO") + + http.StubResponse(200, bytes.NewBufferString(` + { "data": { "repository": { + "hasIssuesEnabled": false + } } } + `)) + + _, err := RunCommand(issueReopenCmd, "issue reopen 2") + if err == nil { + t.Fatalf("expected error when issues are disabled") + } + + if !strings.Contains(err.Error(), "issues disabled") { + t.Fatalf("got unexpected error: %s", err) + } +} diff --git a/command/pr.go b/command/pr.go index a5b1f0079..009548061 100644 --- a/command/pr.go +++ b/command/pr.go @@ -417,7 +417,11 @@ func prProjectList(pr api.PullRequest) string { projectNames := make([]string, 0, len(pr.ProjectCards.Nodes)) for _, project := range pr.ProjectCards.Nodes { - projectNames = append(projectNames, fmt.Sprintf("%s (%s)", project.Project.Name, project.Column.Name)) + colName := project.Column.Name + if colName == "" { + colName = "Awaiting triage" + } + projectNames = append(projectNames, fmt.Sprintf("%s (%s)", project.Project.Name, colName)) } list := strings.Join(projectNames, ", ") diff --git a/command/pr_test.go b/command/pr_test.go index 074420deb..ba0989645 100644 --- a/command/pr_test.go +++ b/command/pr_test.go @@ -429,7 +429,7 @@ func TestPRView_Preview(t *testing.T) { `Open • nobody wants to merge 12 commits into master from blueberries`, `Assignees: marseilles, monaco\n`, `Labels: one, two, three, four, five\n`, - `Projects: Project 1 \(column A\), Project 2 \(column B\), Project 3 \(column C\)\n`, + `Projects: Project 1 \(column A\), Project 2 \(column B\), Project 3 \(column C\), Project 4 \(Awaiting triage\)\n`, `Milestone: uluru\n`, `blueberries taste good`, `View this pull request on GitHub: https://github.com/OWNER/REPO/pull/12\n`, diff --git a/command/repo.go b/command/repo.go index b96a48164..a611643be 100644 --- a/command/repo.go +++ b/command/repo.go @@ -379,11 +379,11 @@ func repoFork(cmd *cobra.Command, args []string) error { loading := utils.Gray("Forking ") + utils.Bold(utils.Gray(ghrepo.FullName(repoToFork))) + utils.Gray("...") s.Suffix = " " + loading s.FinalMSG = utils.Gray(fmt.Sprintf("- %s\n", loading)) - s.Start() + utils.StartSpinner(s) forkedRepo, err := api.ForkRepo(apiClient, repoToFork) if err != nil { - s.Stop() + utils.StopSpinner(s) return fmt.Errorf("failed to fork: %w", err) } diff --git a/command/repo_test.go b/command/repo_test.go index 118e365d5..3e9fca954 100644 --- a/command/repo_test.go +++ b/command/repo_test.go @@ -11,12 +11,24 @@ import ( "testing" "time" + "github.com/briandowns/spinner" + "github.com/cli/cli/context" "github.com/cli/cli/internal/run" "github.com/cli/cli/test" + "github.com/cli/cli/utils" ) +func stubSpinner() { + // not bothering with teardown since we never want spinners when doing tests + utils.StartSpinner = func(_ *spinner.Spinner) { + } + utils.StopSpinner = func(_ *spinner.Spinner) { + } +} + func TestRepoFork_already_forked(t *testing.T) { + stubSpinner() initContext = func() context.Context { ctx := context.NewBlank() ctx.SetBaseRepo("OWNER/REPO") @@ -42,6 +54,7 @@ func TestRepoFork_already_forked(t *testing.T) { } func TestRepoFork_reuseRemote(t *testing.T) { + stubSpinner() initContext = func() context.Context { ctx := context.NewBlank() ctx.SetBaseRepo("OWNER/REPO") @@ -77,6 +90,7 @@ func stubSince(d time.Duration) func() { } func TestRepoFork_in_parent(t *testing.T) { + stubSpinner() initBlankContext("", "OWNER/REPO", "master") defer stubSince(2 * time.Second)() http := initFakeHTTP() @@ -98,6 +112,7 @@ func TestRepoFork_in_parent(t *testing.T) { } func TestRepoFork_outside(t *testing.T) { + stubSpinner() tests := []struct { name string args string @@ -134,6 +149,7 @@ func TestRepoFork_outside(t *testing.T) { } func TestRepoFork_in_parent_yes(t *testing.T) { + stubSpinner() initBlankContext("", "OWNER/REPO", "master") defer stubSince(2 * time.Second)() http := initFakeHTTP() @@ -168,6 +184,7 @@ func TestRepoFork_in_parent_yes(t *testing.T) { } func TestRepoFork_outside_yes(t *testing.T) { + stubSpinner() defer stubSince(2 * time.Second)() http := initFakeHTTP() defer http.StubWithFixture(200, "forkResult.json")() @@ -194,6 +211,7 @@ func TestRepoFork_outside_yes(t *testing.T) { } func TestRepoFork_outside_survey_yes(t *testing.T) { + stubSpinner() defer stubSince(2 * time.Second)() http := initFakeHTTP() defer http.StubWithFixture(200, "forkResult.json")() @@ -227,6 +245,7 @@ func TestRepoFork_outside_survey_yes(t *testing.T) { } func TestRepoFork_outside_survey_no(t *testing.T) { + stubSpinner() defer stubSince(2 * time.Second)() http := initFakeHTTP() defer http.StubWithFixture(200, "forkResult.json")() @@ -261,6 +280,7 @@ func TestRepoFork_outside_survey_no(t *testing.T) { } func TestRepoFork_in_parent_survey_yes(t *testing.T) { + stubSpinner() initBlankContext("", "OWNER/REPO", "master") defer stubSince(2 * time.Second)() http := initFakeHTTP() @@ -303,6 +323,7 @@ func TestRepoFork_in_parent_survey_yes(t *testing.T) { } func TestRepoFork_in_parent_survey_no(t *testing.T) { + stubSpinner() initBlankContext("", "OWNER/REPO", "master") defer stubSince(2 * time.Second)() http := initFakeHTTP() diff --git a/command/root.go b/command/root.go index 40be6a7e0..a2d8fed95 100644 --- a/command/root.go +++ b/command/root.go @@ -51,6 +51,8 @@ func init() { // TODO: // RootCmd.PersistentFlags().BoolP("verbose", "V", false, "enable verbose output") + RootCmd.SetHelpFunc(rootHelpFunc) + RootCmd.SetFlagErrorFunc(func(cmd *cobra.Command, err error) error { if err == pflag.ErrHelp { return err @@ -74,12 +76,9 @@ func (fe FlagError) Unwrap() error { // RootCmd is the entry point of command-line execution var RootCmd = &cobra.Command{ - Use: "gh", + Use: "gh [flags]", Short: "GitHub CLI", - Long: `Work seamlessly with GitHub from the command line. - -GitHub CLI is in early stages of development, and we'd love to hear your -feedback at `, + Long: `Work seamlessly with GitHub from the command line.`, SilenceErrors: true, SilenceUsage: true, @@ -111,20 +110,11 @@ func BasicClient() (*api.Client, error) { } opts = append(opts, api.AddHeader("User-Agent", fmt.Sprintf("GitHub CLI %s", Version))) - c, err := config.ParseDefaultConfig() - if err != nil { - return nil, err + if c, err := config.ParseDefaultConfig(); err == nil { + if token, _ := c.Get(defaultHostname, "oauth_token"); token != "" { + opts = append(opts, api.AddHeader("Authorization", fmt.Sprintf("token %s", token))) + } } - - token, err := c.Get(defaultHostname, "oauth_token") - if err != nil { - return nil, err - } - if token == "" { - return nil, fmt.Errorf("no oauth_token set in config") - } - - opts = append(opts, api.AddHeader("Authorization", fmt.Sprintf("token %s", token))) return api.NewClient(opts...), nil } @@ -142,12 +132,47 @@ var apiClientForContext = func(ctx context.Context) (*api.Client, error) { if err != nil { return nil, err } + var opts []api.ClientOption if verbose := os.Getenv("DEBUG"); verbose != "" { opts = append(opts, apiVerboseLog()) } + + getAuthValue := func() string { + return fmt.Sprintf("token %s", token) + } + + checkScopesFunc := func(appID string) error { + if config.IsGitHubApp(appID) && utils.IsTerminal(os.Stdin) && utils.IsTerminal(os.Stderr) { + newToken, loginHandle, err := config.AuthFlow("Notice: additional authorization required") + if err != nil { + return err + } + cfg, err := ctx.Config() + if err != nil { + return err + } + _ = cfg.Set(defaultHostname, "oauth_token", newToken) + _ = cfg.Set(defaultHostname, "user", loginHandle) + // update config file on disk + err = cfg.Write() + if err != nil { + return err + } + // update configuration in memory + token = newToken + config.AuthFlowComplete() + } else { + fmt.Fprintln(os.Stderr, "Warning: gh now requires the `read:org` OAuth scope.") + fmt.Fprintln(os.Stderr, "Visit https://github.com/settings/tokens and edit your token to enable `read:org`") + fmt.Fprintln(os.Stderr, "or generate a new token and paste it via `gh config set -h github.com oauth_token MYTOKEN`") + } + return nil + } + opts = append(opts, - api.AddHeader("Authorization", fmt.Sprintf("token %s", token)), + api.CheckScopes("read:org", checkScopesFunc), + api.AddHeaderFunc("Authorization", getAuthValue), api.AddHeader("User-Agent", fmt.Sprintf("GitHub CLI %s", Version)), // antiope-preview: Checks api.AddHeader("Accept", "application/vnd.github.antiope-preview+json"), @@ -223,6 +248,71 @@ func determineBaseRepo(cmd *cobra.Command, ctx context.Context) (ghrepo.Interfac return baseRepo, nil } +func rootHelpFunc(command *cobra.Command, s []string) { + type helpEntry struct { + Title string + Body string + } + + coreCommandNames := []string{"issue", "pr", "repo"} + var coreCommands []string + var additionalCommands []string + for _, c := range command.Commands() { + if c.Short == "" { + continue + } + s := " " + rpad(c.Name()+":", c.NamePadding()) + c.Short + if includes(coreCommandNames, c.Name()) { + coreCommands = append(coreCommands, s) + } else { + additionalCommands = append(additionalCommands, s) + } + } + + helpEntries := []helpEntry{ + { + "", + command.Long}, + {"USAGE", command.Use}, + {"CORE COMMANDS", strings.Join(coreCommands, "\n")}, + {"ADDITIONAL COMMANDS", strings.Join(additionalCommands, "\n")}, + {"FLAGS", strings.TrimRight(command.LocalFlags().FlagUsages(), "\n")}, + {"EXAMPLES", ` + $ gh issue create + $ gh repo clone + $ gh pr checkout 321`}, + {"LEARN MORE", ` + Use "gh --help" for more information about a command. + Read the manual at `}, + {"FEEDBACK", ` + Fill out our feedback form + Open an issue using “gh issue create -R cli/cli”`}, + } + + out := colorableOut(command) + for _, e := range helpEntries { + if e.Title != "" { + fmt.Fprintln(out, utils.Bold(e.Title)) + } + fmt.Fprintln(out, strings.TrimLeft(e.Body, "\n")+"\n") + } +} + +// rpad adds padding to the right of a string. +func rpad(s string, padding int) string { + template := fmt.Sprintf("%%-%ds ", padding) + return fmt.Sprintf(template, s) +} + +func includes(a []string, s string) bool { + for _, x := range a { + if x == s { + return true + } + } + return false +} + func formatRemoteURL(cmd *cobra.Command, fullRepoName string) string { ctx := contextForCommand(cmd) diff --git a/docs/releasing.md b/docs/releasing.md index 00646a18a..dfaebd76a 100644 --- a/docs/releasing.md +++ b/docs/releasing.md @@ -1,27 +1,17 @@ # Releasing -## Test Locally - -`go test ./...` - -## Push new docs - -build docs locally: `make site` - -build and push docs to production: `make site-docs` - -## Release locally for debugging - -A local release can be created for testing without creating anything official on the release page. - -1. `env GH_OAUTH_CLIENT_SECRET= GH_OAUTH_CLIENT_ID= goreleaser --skip-validate --skip-publish --rm-dist` -2. Check and test files in `dist/` - ## Release to production This can all be done from your local terminal. -1. `git tag 'vVERSION_NUMBER' # example git tag 'v0.0.1'` -2. `git push origin vVERSION_NUMBER` -3. Wait a few minutes for the build to run and CI to pass. Look at the [actions tab](https://github.com/cli/cli/actions) to check the progress. -4. Go to and look at the release +1. `git tag v1.2.3` +2. `git push origin v1.2.3` +3. Wait a few minutes for the build to run +4. Check + +## Release locally for debugging + +A local release can be created for testing without creating anything official on the release page. + +1. `goreleaser --skip-validate --skip-publish --rm-dist` +2. Check and test files in `dist/` diff --git a/internal/config/config_setup.go b/internal/config/config_setup.go index 9bc0fb775..4d2a929cb 100644 --- a/internal/config/config_setup.go +++ b/internal/config/config_setup.go @@ -24,9 +24,14 @@ var ( oauthClientSecret = "34ddeff2b558a23d38fba8a6de74f086ede1cc0b" ) -// TODO: have a conversation about whether this belongs in the "context" package -// FIXME: make testable -func setupConfigFile(filename string) (Config, error) { +// IsGitHubApp reports whether an OAuth app is "GitHub CLI" or "GitHub CLI (dev)" +func IsGitHubApp(id string) bool { + // this intentionally doesn't use `oauthClientID` because that is a variable + // that can potentially be changed at build time via GH_OAUTH_CLIENT_ID + return id == "178c6fc778ccc68e1d6a" || id == "4d747ba5675d5d66553f" +} + +func AuthFlow(notice string) (string, string, error) { var verboseStream io.Writer if strings.Contains(os.Getenv("DEBUG"), "oauth") { verboseStream = os.Stderr @@ -36,21 +41,37 @@ func setupConfigFile(filename string) (Config, error) { Hostname: oauthHost, ClientID: oauthClientID, ClientSecret: oauthClientSecret, + Scopes: []string{"repo", "read:org"}, WriteSuccessHTML: func(w io.Writer) { fmt.Fprintln(w, oauthSuccessPage) }, VerboseStream: verboseStream, } - fmt.Fprintln(os.Stderr, "Notice: authentication required") + fmt.Fprintln(os.Stderr, notice) fmt.Fprintf(os.Stderr, "Press Enter to open %s in your browser... ", flow.Hostname) _ = waitForEnter(os.Stdin) token, err := flow.ObtainAccessToken() if err != nil { - return nil, err + return "", "", err } userLogin, err := getViewer(token) + if err != nil { + return "", "", err + } + + return token, userLogin, nil +} + +func AuthFlowComplete() { + fmt.Fprintln(os.Stderr, "Authentication complete. Press Enter to continue... ") + _ = waitForEnter(os.Stdin) +} + +// FIXME: make testable +func setupConfigFile(filename string) (Config, error) { + token, userLogin, err := AuthFlow("Notice: authentication required") if err != nil { return nil, err } @@ -61,9 +82,9 @@ func setupConfigFile(filename string) (Config, error) { } yamlHosts := map[string]map[string]string{} - yamlHosts[flow.Hostname] = map[string]string{} - yamlHosts[flow.Hostname]["user"] = userLogin - yamlHosts[flow.Hostname]["oauth_token"] = token + yamlHosts[oauthHost] = map[string]string{} + yamlHosts[oauthHost]["user"] = userLogin + yamlHosts[oauthHost]["oauth_token"] = token defaultConfig := yamlConfig{ Hosts: yamlHosts, @@ -84,14 +105,9 @@ func setupConfigFile(filename string) (Config, error) { if err != nil { return nil, err } - n, err := cfgFile.Write(yamlData) - if err == nil && n < len(yamlData) { - err = io.ErrShortWrite - } - - if err == nil { - fmt.Fprintln(os.Stderr, "Authentication complete. Press Enter to continue... ") - _ = waitForEnter(os.Stdin) + _, err = cfgFile.Write(yamlData) + if err != nil { + return nil, err } // TODO cleaner error handling? this "should" always work given that we /just/ wrote the file... diff --git a/test/fixtures/issueView_previewWithMetadata.json b/test/fixtures/issueView_previewWithMetadata.json index a420eec66..246bbd77b 100644 --- a/test/fixtures/issueView_previewWithMetadata.json +++ b/test/fixtures/issueView_previewWithMetadata.json @@ -67,6 +67,14 @@ "column": { "name": "column C" } + }, + { + "project": { + "name": "Project 4" + }, + "column": { + "name": "" + } } ], "totalcount": 3 diff --git a/test/fixtures/prViewPreviewWithMetadataByNumber.json b/test/fixtures/prViewPreviewWithMetadataByNumber.json index de0da816a..e49bcdc85 100644 --- a/test/fixtures/prViewPreviewWithMetadataByNumber.json +++ b/test/fixtures/prViewPreviewWithMetadataByNumber.json @@ -66,6 +66,14 @@ "column": { "name": "column C" } + }, + { + "project": { + "name": "Project 4" + }, + "column": { + "name": "" + } } ], "totalcount": 3 diff --git a/utils/utils.go b/utils/utils.go index 2efc5a95d..c84860d9e 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -74,6 +74,16 @@ func Humanize(s string) string { return strings.Map(h, s) } +// We do this so we can stub out the spinner in tests -- it made things really flakey. this is not +// an elegant solution. +var StartSpinner = func(s *spinner.Spinner) { + s.Start() +} + +var StopSpinner = func(s *spinner.Spinner) { + s.Stop() +} + func Spinner(w io.Writer) *spinner.Spinner { return spinner.New(spinner.CharSets[11], 400*time.Millisecond, spinner.WithWriter(w)) }