diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 56f1248d6..d339a0685 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -36,6 +36,8 @@ Run the new binary as: Run tests with: `go test ./...` +See [project layout documentation](../project-layout.md) for information on where to find specific source files. + ## Submitting a pull request 1. Create a new branch: `git checkout -b my-branch-name` diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md new file mode 100644 index 000000000..aa6662d49 --- /dev/null +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -0,0 +1,4 @@ + diff --git a/.github/PULL_REQUEST_TEMPLATE/bug_fix.md b/.github/PULL_REQUEST_TEMPLATE/bug_fix.md deleted file mode 100644 index ca33dd34c..000000000 --- a/.github/PULL_REQUEST_TEMPLATE/bug_fix.md +++ /dev/null @@ -1,19 +0,0 @@ ---- -name: "\U0001F41B Bug fix" -about: Fix a bug in GitHub CLI - ---- - - - -## Summary - -closes #[issue number] - -## Details - -- diff --git a/.github/workflows/releases.yml b/.github/workflows/releases.yml index ccb42c8df..4f593f36a 100644 --- a/.github/workflows/releases.yml +++ b/.github/workflows/releases.yml @@ -133,9 +133,11 @@ jobs: - name: Build MSI id: buildmsi shell: bash + env: + ZIP_FILE: ${{ steps.download_exe.outputs.zip }} run: | mkdir -p build - msi="$(basename "${{ steps.download_exe.outputs.zip }}" ".zip").msi" + msi="$(basename "$ZIP_FILE" ".zip").msi" printf "::set-output name=msi::%s\n" "$msi" go-msi make --msi "$PWD/$msi" --out "$PWD/build" --version "${GITHUB_REF#refs/tags/}" - name: Obtain signing cert @@ -145,14 +147,24 @@ jobs: run: .\script\setup-windows-certificate.ps1 - name: Sign MSI env: + CERT_FILE: ${{ steps.obtain_cert.outputs.cert-file }} + EXE_FILE: ${{ steps.buildmsi.outputs.msi }} GITHUB_CERT_PASSWORD: ${{ secrets.GITHUB_CERT_PASSWORD }} - run: | - .\script\sign.ps1 -Certificate "${{ steps.obtain_cert.outputs.cert-file }}" ` - -Executable "${{ steps.buildmsi.outputs.msi }}" + run: .\script\sign.ps1 -Certificate $env:CERT_FILE -Executable $env:EXE_FILE - name: Upload MSI shell: bash - run: hub release edit "${GITHUB_REF#refs/tags/}" -m "" --draft=false -a "${{ steps.buildmsi.outputs.msi }}" + run: | + tag_name="${GITHUB_REF#refs/tags/}" + hub release edit "$tag_name" -m "" -a "$MSI_FILE" + release_url="$(gh api repos/:owner/:repo/releases -q ".[]|select(.tag_name==\"${tag_name}\")|.url")" + publish_args=( -F draft=false ) + if [[ $GITHUB_REF != *-* ]]; then + publish_args+=( -f discussion_category_name="$DISCUSSION_CATEGORY" ) + fi + gh api -X PATCH "$release_url" "${publish_args[@]}" env: + MSI_FILE: ${{ steps.buildmsi.outputs.msi }} + DISCUSSION_CATEGORY: General GITHUB_TOKEN: ${{secrets.GITHUB_TOKEN}} - name: Bump homebrew-core formula uses: mislav/bump-homebrew-formula-action@v1 diff --git a/api/export_pr.go b/api/export_pr.go index dc70ee4fa..9a0e10702 100644 --- a/api/export_pr.go +++ b/api/export_pr.go @@ -6,13 +6,14 @@ import ( ) func (issue *Issue) ExportData(fields []string) *map[string]interface{} { + v := reflect.ValueOf(issue).Elem() data := map[string]interface{}{} for _, f := range fields { switch f { case "milestone": if issue.Milestone.Title != "" { - data[f] = &issue.Milestone + data[f] = map[string]string{"title": issue.Milestone.Title} } else { data[f] = nil } @@ -25,7 +26,6 @@ func (issue *Issue) ExportData(fields []string) *map[string]interface{} { case "projectCards": data[f] = issue.ProjectCards.Nodes default: - v := reflect.ValueOf(issue).Elem() sf := fieldByName(v, f) data[f] = sf.Interface() } @@ -35,6 +35,7 @@ func (issue *Issue) ExportData(fields []string) *map[string]interface{} { } func (pr *PullRequest) ExportData(fields []string) *map[string]interface{} { + v := reflect.ValueOf(pr).Elem() data := map[string]interface{}{} for _, f := range fields { @@ -43,7 +44,7 @@ func (pr *PullRequest) ExportData(fields []string) *map[string]interface{} { data[f] = map[string]string{"name": pr.HeadRepository.Name} case "milestone": if pr.Milestone.Title != "" { - data[f] = &pr.Milestone + data[f] = map[string]string{"title": pr.Milestone.Title} } else { data[f] = nil } @@ -75,7 +76,6 @@ func (pr *PullRequest) ExportData(fields []string) *map[string]interface{} { } data[f] = &requests default: - v := reflect.ValueOf(pr).Elem() sf := fieldByName(v, f) data[f] = sf.Interface() } @@ -84,22 +84,6 @@ func (pr *PullRequest) ExportData(fields []string) *map[string]interface{} { return &data } -func ExportIssues(issues []Issue, fields []string) *[]interface{} { - data := make([]interface{}, len(issues)) - for i := range issues { - data[i] = issues[i].ExportData(fields) - } - return &data -} - -func ExportPRs(prs []PullRequest, fields []string) *[]interface{} { - data := make([]interface{}, len(prs)) - for i := range prs { - data[i] = prs[i].ExportData(fields) - } - return &data -} - func fieldByName(v reflect.Value, field string) reflect.Value { return v.FieldByNameFunc(func(s string) bool { return strings.EqualFold(field, s) diff --git a/api/export_pr_test.go b/api/export_pr_test.go index eff3c157d..4e02f9f09 100644 --- a/api/export_pr_test.go +++ b/api/export_pr_test.go @@ -90,31 +90,6 @@ func TestIssue_ExportData(t *testing.T) { } } -func TestExportIssues(t *testing.T) { - issues := []Issue{ - {Milestone: Milestone{Title: "hi"}}, - {}, - } - exported := ExportIssues(issues, []string{"milestone"}) - - buf := bytes.Buffer{} - enc := json.NewEncoder(&buf) - enc.SetIndent("", "\t") - require.NoError(t, enc.Encode(exported)) - assert.Equal(t, heredoc.Doc(` - [ - { - "milestone": { - "title": "hi" - } - }, - { - "milestone": null - } - ] - `), buf.String()) -} - func TestPullRequest_ExportData(t *testing.T) { tests := []struct { name string diff --git a/api/export_repo.go b/api/export_repo.go new file mode 100644 index 000000000..8d4e669ad --- /dev/null +++ b/api/export_repo.go @@ -0,0 +1,53 @@ +package api + +import ( + "reflect" +) + +func (repo *Repository) ExportData(fields []string) *map[string]interface{} { + v := reflect.ValueOf(repo).Elem() + data := map[string]interface{}{} + + for _, f := range fields { + switch f { + case "parent": + data[f] = miniRepoExport(repo.Parent) + case "templateRepository": + data[f] = miniRepoExport(repo.TemplateRepository) + case "languages": + data[f] = repo.Languages.Edges + case "labels": + data[f] = repo.Labels.Nodes + case "assignableUsers": + data[f] = repo.AssignableUsers.Nodes + case "mentionableUsers": + data[f] = repo.MentionableUsers.Nodes + case "milestones": + data[f] = repo.Milestones.Nodes + case "projects": + data[f] = repo.Projects.Nodes + case "repositoryTopics": + var topics []RepositoryTopic + for _, n := range repo.RepositoryTopics.Nodes { + topics = append(topics, n.Topic) + } + data[f] = topics + default: + sf := fieldByName(v, f) + data[f] = sf.Interface() + } + } + + return &data +} + +func miniRepoExport(r *Repository) map[string]interface{} { + if r == nil { + return nil + } + return map[string]interface{}{ + "id": r.ID, + "name": r.Name, + "owner": r.Owner, + } +} diff --git a/api/queries_issue.go b/api/queries_issue.go index 35e4e418f..18f2c83c7 100644 --- a/api/queries_issue.go +++ b/api/queries_issue.go @@ -91,7 +91,10 @@ func (p ProjectCards) ProjectNames() []string { } type Milestone struct { - Title string `json:"title"` + Number int `json:"number"` + Title string `json:"title"` + Description string `json:"description"` + DueOn *time.Time `json:"dueOn"` } type IssuesDisabledError struct { @@ -241,7 +244,6 @@ func IssueByNumber(client *Client, repo ghrepo.Interface, number int) (*Issue, e id title state - closed body author { login diff --git a/api/queries_repo.go b/api/queries_repo.go index 90d3949a5..5f1056c26 100644 --- a/api/queries_repo.go +++ b/api/queries_repo.go @@ -16,25 +16,100 @@ import ( // Repository contains information about a GitHub repo type Repository struct { - ID string - Name string - Description string - URL string - CloneURL string - CreatedAt time.Time - Owner RepositoryOwner + ID string + Name string + NameWithOwner string + Owner RepositoryOwner + Parent *Repository + TemplateRepository *Repository + Description string + HomepageURL string + OpenGraphImageURL string + UsesCustomOpenGraphImage bool + URL string + SSHURL string + MirrorURL string + SecurityPolicyURL string - IsPrivate bool - HasIssuesEnabled bool - HasWikiEnabled bool - ViewerPermission string - DefaultBranchRef BranchRef + CreatedAt time.Time + PushedAt *time.Time + UpdatedAt time.Time - Parent *Repository + IsBlankIssuesEnabled bool + IsSecurityPolicyEnabled bool + HasIssuesEnabled bool + HasProjectsEnabled bool + HasWikiEnabled bool + MergeCommitAllowed bool + SquashMergeAllowed bool + RebaseMergeAllowed bool - MergeCommitAllowed bool - RebaseMergeAllowed bool - SquashMergeAllowed bool + ForkCount int + StargazerCount int + Watchers struct { + TotalCount int `json:"totalCount"` + } + Issues struct { + TotalCount int `json:"totalCount"` + } + PullRequests struct { + TotalCount int `json:"totalCount"` + } + + CodeOfConduct *CodeOfConduct + ContactLinks []ContactLink + DefaultBranchRef BranchRef + DeleteBranchOnMerge bool + DiskUsage int + FundingLinks []FundingLink + IsArchived bool + IsEmpty bool + IsFork bool + IsInOrganization bool + IsMirror bool + IsPrivate bool + IsTemplate bool + IsUserConfigurationRepository bool + LicenseInfo *RepositoryLicense + ViewerCanAdminister bool + ViewerDefaultCommitEmail string + ViewerDefaultMergeMethod string + ViewerHasStarred bool + ViewerPermission string + ViewerPossibleCommitEmails []string + ViewerSubscription string + + RepositoryTopics struct { + Nodes []struct { + Topic RepositoryTopic + } + } + PrimaryLanguage *CodingLanguage + Languages struct { + Edges []struct { + Size int `json:"size"` + Node CodingLanguage `json:"node"` + } + } + IssueTemplates []IssueTemplate + PullRequestTemplates []PullRequestTemplate + Labels struct { + Nodes []IssueLabel + } + Milestones struct { + Nodes []Milestone + } + LatestRelease *RepositoryRelease + + AssignableUsers struct { + Nodes []GitHubUser + } + MentionableUsers struct { + Nodes []GitHubUser + } + Projects struct { + Nodes []RepoProject + } // pseudo-field that keeps track of host name of this repo hostname string @@ -42,12 +117,76 @@ type Repository struct { // RepositoryOwner is the owner of a GitHub repository type RepositoryOwner struct { - Login string + ID string `json:"id"` + Login string `json:"login"` +} + +type GitHubUser struct { + ID string `json:"id"` + Login string `json:"login"` + Name string `json:"name"` } // BranchRef is the branch name in a GitHub repository type BranchRef struct { - Name string + Name string `json:"name"` +} + +type CodeOfConduct struct { + Key string `json:"key"` + Name string `json:"name"` + URL string `json:"url"` +} + +type RepositoryLicense struct { + Key string `json:"key"` + Name string `json:"name"` + Nickname string `json:"nickname"` +} + +type ContactLink struct { + About string `json:"about"` + Name string `json:"name"` + URL string `json:"url"` +} + +type FundingLink struct { + Platform string `json:"platform"` + URL string `json:"url"` +} + +type CodingLanguage struct { + Name string `json:"name"` +} + +type IssueTemplate struct { + Name string `json:"name"` + Title string `json:"title"` + Body string `json:"body"` + About string `json:"about"` +} + +type PullRequestTemplate struct { + Filename string `json:"filename"` + Body string `json:"body"` +} + +type RepositoryTopic struct { + Name string `json:"name"` +} + +type RepositoryRelease struct { + Name string `json:"name"` + TagName string `json:"tagName"` + URL string `json:"url"` + PublishedAt time.Time `json:"publishedAt"` +} + +type IssueLabel struct { + ID string `json:"id"` + Name string `json:"name"` + Description string `json:"description"` + Color string `json:"color"` } // RepoOwner is the login name of the owner @@ -65,11 +204,6 @@ func (r Repository) RepoHost() string { return r.hostname } -// IsFork is true when this repository has a parent repository -func (r Repository) IsFork() bool { - return r.Parent != nil -} - // ViewerCanPush is true when the requesting user has push access func (r Repository) ViewerCanPush() bool { switch r.ViewerPermission { @@ -305,16 +439,26 @@ type repositoryV3 struct { NodeID string Name string CreatedAt time.Time `json:"created_at"` - CloneURL string `json:"clone_url"` Owner struct { Login string } } // ForkRepo forks the repository on GitHub and returns the new repository -func ForkRepo(client *Client, repo ghrepo.Interface) (*Repository, error) { +func ForkRepo(client *Client, repo ghrepo.Interface, org string) (*Repository, error) { path := fmt.Sprintf("repos/%s/forks", ghrepo.FullName(repo)) - body := bytes.NewBufferString(`{}`) + + params := map[string]interface{}{} + if org != "" { + params["organization"] = org + } + + body := &bytes.Buffer{} + enc := json.NewEncoder(body) + if err := enc.Encode(params); err != nil { + return nil, err + } + result := repositoryV3{} err := client.REST(repo.RepoHost(), "POST", path, body, &result) if err != nil { @@ -324,7 +468,6 @@ func ForkRepo(client *Client, repo ghrepo.Interface) (*Repository, error) { return &Repository{ ID: result.NodeID, Name: result.Name, - CloneURL: result.CloneURL, CreatedAt: result.CreatedAt, Owner: RepositoryOwner{ Login: result.Owner.Login, @@ -707,9 +850,10 @@ func RepoResolveMetadataIDs(client *Client, repo ghrepo.Interface, input RepoRes } type RepoProject struct { - ID string - Name string - ResourcePath string + ID string `json:"id"` + Name string `json:"name"` + Number int `json:"number"` + ResourcePath string `json:"resourcePath"` } // RepoProjects fetches all open projects for a repository diff --git a/api/queries_repo_test.go b/api/queries_repo_test.go index a9d535e6e..50a2067f1 100644 --- a/api/queries_repo_test.go +++ b/api/queries_repo_test.go @@ -144,9 +144,9 @@ func Test_RepoMetadata(t *testing.T) { func Test_ProjectsToPaths(t *testing.T) { expectedProjectPaths := []string{"OWNER/REPO/PROJECT_NUMBER", "ORG/PROJECT_NUMBER"} projects := []RepoProject{ - {"id1", "My Project", "/OWNER/REPO/projects/PROJECT_NUMBER"}, - {"id2", "Org Project", "/orgs/ORG/projects/PROJECT_NUMBER"}, - {"id3", "Project", "/orgs/ORG/projects/PROJECT_NUMBER_2"}, + {ID: "id1", Name: "My Project", ResourcePath: "/OWNER/REPO/projects/PROJECT_NUMBER"}, + {ID: "id2", Name: "Org Project", ResourcePath: "/orgs/ORG/projects/PROJECT_NUMBER"}, + {ID: "id3", Name: "Project", ResourcePath: "/orgs/ORG/projects/PROJECT_NUMBER_2"}, } projectNames := []string{"My Project", "Org Project"} diff --git a/api/query_builder.go b/api/query_builder.go index 84d9d8f91..25862e179 100644 --- a/api/query_builder.go +++ b/api/query_builder.go @@ -186,3 +186,133 @@ func PullRequestGraphQL(fields []string) string { } return strings.Join(q, ",") } + +var RepositoryFields = []string{ + "id", + "name", + "nameWithOwner", + "owner", + "parent", + "templateRepository", + "description", + "homepageUrl", + "openGraphImageUrl", + "usesCustomOpenGraphImage", + "url", + "sshUrl", + "mirrorUrl", + "securityPolicyUrl", + + "createdAt", + "pushedAt", + "updatedAt", + + "isBlankIssuesEnabled", + "isSecurityPolicyEnabled", + "hasIssuesEnabled", + "hasProjectsEnabled", + "hasWikiEnabled", + "mergeCommitAllowed", + "squashMergeAllowed", + "rebaseMergeAllowed", + + "forkCount", + "stargazerCount", + "watchers", + "issues", + "pullRequests", + + "codeOfConduct", + "contactLinks", + "defaultBranchRef", + "deleteBranchOnMerge", + "diskUsage", + "fundingLinks", + "isArchived", + "isEmpty", + "isFork", + "isInOrganization", + "isMirror", + "isPrivate", + "isTemplate", + "isUserConfigurationRepository", + "licenseInfo", + "viewerCanAdminister", + "viewerDefaultCommitEmail", + "viewerDefaultMergeMethod", + "viewerHasStarred", + "viewerPermission", + "viewerPossibleCommitEmails", + "viewerSubscription", + + "repositoryTopics", + "primaryLanguage", + "languages", + "issueTemplates", + "pullRequestTemplates", + "labels", + "milestones", + "latestRelease", + + "assignableUsers", + "mentionableUsers", + "projects", + + // "branchProtectionRules", // too complex to expose + // "collaborators", // does it make sense to expose without affiliation filter? +} + +func RepositoryGraphQL(fields []string) string { + var q []string + for _, field := range fields { + switch field { + case "codeOfConduct": + q = append(q, "codeOfConduct{key,name,url}") + case "contactLinks": + q = append(q, "contactLinks{about,name,url}") + case "fundingLinks": + q = append(q, "fundingLinks{platform,url}") + case "licenseInfo": + q = append(q, "licenseInfo{key,name,nickname}") + case "owner": + q = append(q, "owner{id,login}") + case "parent": + q = append(q, "parent{id,name,owner{id,login}}") + case "templateRepository": + q = append(q, "templateRepository{id,name,owner{id,login}}") + case "repositoryTopics": + q = append(q, "repositoryTopics(first:100){nodes{topic{name}}}") + case "issueTemplates": + q = append(q, "issueTemplates{name,title,body,about}") + case "pullRequestTemplates": + q = append(q, "pullRequestTemplates{body,filename}") + case "labels": + q = append(q, "labels(first:100){nodes{id,color,name,description}}") + case "languages": + q = append(q, "languages(first:100){edges{size,node{name}}}") + case "primaryLanguage": + q = append(q, "primaryLanguage{name}") + case "latestRelease": + q = append(q, "latestRelease{publishedAt,tagName,name,url}") + case "milestones": + q = append(q, "milestones(first:100,states:OPEN){nodes{number,title,description,dueOn}}") + case "assignableUsers": + q = append(q, "assignableUsers(first:100){nodes{id,login,name}}") + case "mentionableUsers": + q = append(q, "mentionableUsers(first:100){nodes{id,login,name}}") + case "projects": + q = append(q, "projects(first:100,states:OPEN){nodes{id,name,number,body,resourcePath}}") + case "watchers": + q = append(q, "watchers{totalCount}") + case "issues": + q = append(q, "issues(states:OPEN){totalCount}") + case "pullRequests": + q = append(q, "pullRequests(states:OPEN){totalCount}") + case "defaultBranchRef": + q = append(q, "defaultBranchRef{name}") + default: + q = append(q, field) + } + } + return strings.Join(q, ",") +} diff --git a/context/context.go b/context/context.go index babd51f55..4c1a64c73 100644 --- a/context/context.go +++ b/context/context.go @@ -104,7 +104,7 @@ func (r *ResolvedRemotes) BaseRepo(io *iostreams.IOStreams) (ghrepo.Interface, e if repo == nil { continue } - if repo.IsFork() { + if repo.Parent != nil { add(repo.Parent) } add(repo) diff --git a/docs/project-layout.md b/docs/project-layout.md new file mode 100644 index 000000000..cf594a2e7 --- /dev/null +++ b/docs/project-layout.md @@ -0,0 +1,84 @@ +# GitHub CLI project layout + +At a high level, these areas make up the `github.com/cli/cli` project: +- [`cmd/`](../cmd) - `main` packages for building binaries such as the `gh` executable +- [`pkg/`](../pkg) - most other packages, including the implementation for individual gh commands +- [`docs/`](../docs) - documentation for maintainers and contributors +- [`script/`](../script) - build and release scripts +- [`internal/`](../internal) - Go packages highly specific to our needs and thus internal +- [`go.mod`](../go.mod) - external Go dependencies for this project, automatically fetched by Go at build time + +Some auxiliary Go packages are at the top level of the project for historical reasons: +- [`api/`](../api) - main utilities for making requests to the GitHub API +- [`context/`](../context) - DEPRECATED: use only for referencing git remotes +- [`git/`](../git) - utilities to gather information from a local git repository +- [`test/`](../test) - DEPRECATED: do not use +- [`utils/`](../utils) - DEPRECATED: use only for printing table output + +## Command-line help text + +Running `gh help issue list` displays help text for a topic. In this case, the topic is a specific command, +and help text for every command is embedded in that command's source code. The naming convention for gh +commands is: +``` +pkg/cmd///.go +``` +Following the above example, the main implementation for the `gh issue list` command, including its help +text, is in [pkg/cmd/issue/view/view.go](../pkg/cmd/issue/view/view.go) + +Other help topics not specific to any command, for example `gh help environment`, are found in +[pkg/cmd/root/help_topic.go](../pkg/cmd/root/help_topic.go). + +During our release process, these help topics are [automatically converted](../cmd/gen-docs/main.go) to +manual pages and published under https://cli.github.com/manual/. + +## How GitHub CLI works + +To illustrate how GitHub CLI works in its typical mode of operation, let's build the project, run a command, +and talk through which code gets run in order. + +1. `go run script/build.go` - Makes sure all external Go depedencies are fetched, then compiles the + `cmd/gh/main.go` file into a `bin/gh` binary. +2. `bin/gh issue list --limit 5` - Runs the newly built `bin/gh` binary (note: on Windows you must use + backslashes like `bin\gh`) and passes the following arguments to the process: `["issue", "list", "--limit", "5"]`. +3. `func main()` inside `cmd/gh/main.go` is the first Go function that runs. The arguments passed to the + process are available through `os.Args`. +4. The `main` package initializes the "root" command with `root.NewCmdRoot()` and dispatches execution to it + with `rootCmd.ExecuteC()`. +5. The [root command](../pkg/cmd/root/root.go) represents the top-level `gh` command and knows how to + dispatch execution to any other gh command nested under it. +6. Based on `["issue", "list"]` arguments, the execution reaches the `RunE` block of the `cobra.Command` + within [pkg/cmd/issue/list/list.go](../pkg/cmd/issue/list/list.go). +7. The `--limit 5` flag originally passed as arguments be automatically parsed and its value stored as + `opts.LimitResults`. +8. `func listRun()` is called, which is responsible for implementing the logic of the `gh issue list` command. +9. The command collects information from sources like the GitHub API then writes the final output to + standard output and standard error [streams](../pkg/iostreams/iostreams.go) available at `opts.IO`. +10. The program execution is now back at `func main()` of `cmd/gh/main.go`. If there were any Go errors as a + result of processing the command, the function will abort the process with a non-zero exit status. + Otherwise, the process ends with status 0 indicating success. + +## How to add a new command + +0. First, check on our issue tracker to verify that our team had approved the plans for a new command. +1. Create a package for the new command, e.g. for a new command `gh boom` create the following directory + structure: `pkg/cmd/boom/` +2. The new package should expose a method, e.g. `NewCmdBoom()`, that accepts a `*cmdutil.Factory` type and + returns a `*cobra.Command`. + * Any logic specific to this command should be kept within the command's package and not added to any + "global" packages like `api` or `utils`. +3. Use the method from the previous step to generate the command and add it to the command tree, typically + somewhere in the `NewCmdRoot()` method. + +## How to write tests + +This task might be tricky. Typically, gh commands do things like look up information from the git repository +in the current directory, query the GitHub API, scan the user's `~/.ssh/config` file, clone or fetch git +repositories, etc. Naturally, none of these things should ever happen for real when running tests, unless +you are sure that any filesystem operations are stricly scoped to a location made for and maintained by the +test itself. To avoid actually running things like making real API requests or shelling out to `git` +commands, we stub them. You should look at how that's done within some existing tests. + +To make your code testable, write small, isolated pieces of functionality that are designed to be composed +together. Prefer table-driven tests for maintaining variations of different test inputs and expectations +when exercising a single piece of functionality. diff --git a/go.mod b/go.mod index b44012c93..96b74584c 100644 --- a/go.mod +++ b/go.mod @@ -27,7 +27,7 @@ require ( github.com/rivo/uniseg v0.1.0 github.com/shurcooL/githubv4 v0.0.0-20200928013246-d292edc3691b github.com/shurcooL/graphql v0.0.0-20181231061246-d48a9a75455f - github.com/spf13/cobra v1.1.1 + github.com/spf13/cobra v1.1.3 github.com/spf13/pflag v1.0.5 github.com/stretchr/testify v1.6.1 golang.org/x/crypto v0.0.0-20201016220609-9e8e0b390897 diff --git a/go.sum b/go.sum index 523a32a85..d1f1f19db 100644 --- a/go.sum +++ b/go.sum @@ -248,8 +248,8 @@ github.com/soheilhy/cmux v0.1.4/go.mod h1:IM3LyeVVIOuxMH7sFAkER9+bJ4dT7Ms6E4xg4k github.com/spaolacci/murmur3 v0.0.0-20180118202830-f09979ecbc72/go.mod h1:JwIasOWyU6f++ZhiEuf87xNszmSA2myDM2Kzu9HwQUA= github.com/spf13/afero v1.1.2/go.mod h1:j4pytiNVoe2o6bmDsKpLACNPDBIoEAkihy7loJ1B0CQ= github.com/spf13/cast v1.3.0/go.mod h1:Qx5cxh0v+4UWYiBimWS+eyWzqEqokIECu5etghLkUJE= -github.com/spf13/cobra v1.1.1 h1:KfztREH0tPxJJ+geloSLaAkaPkr4ki2Er5quFV1TDo4= -github.com/spf13/cobra v1.1.1/go.mod h1:WnodtKOvamDL/PwE2M4iKs8aMDBZ5Q5klgD3qfVJQMI= +github.com/spf13/cobra v1.1.3 h1:xghbfqPkxzxP3C/f3n5DdpAbdKLj4ZE4BWQI362l53M= +github.com/spf13/cobra v1.1.3/go.mod h1:pGADOWyqRD/YMrPZigI/zbliZ2wVD/23d+is3pSWzOo= github.com/spf13/jwalterweatherman v1.0.0/go.mod h1:cQK4TGJAtQXfYWX+Ddv3mKDzgVb68N+wFjFa4jdeBTo= github.com/spf13/pflag v1.0.3/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnInEg4= github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= @@ -419,7 +419,7 @@ gopkg.in/resty.v1 v1.12.0/go.mod h1:mDo4pnntr5jdWRML875a/NmxYqAlA73dVijT2AXvQQo= gopkg.in/yaml.v2 v2.0.0-20170812160011-eb3733d160e7/go.mod h1:JAlM8MvJe8wmxCU4Bli9HhUf9+ttbYbLASfIpnQbh74= gopkg.in/yaml.v2 v2.2.1/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.4/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= -gopkg.in/yaml.v2 v2.2.8/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= +gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b h1:h8qDotaEPuJATrMmW04NCwg7v22aHH28wwpauUhK9Oo= gopkg.in/yaml.v3 v3.0.0-20210107192922-496545a6307b/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/internal/docs/man_test.go b/internal/docs/man_test.go index 58591e19e..d72ea7214 100644 --- a/internal/docs/man_test.go +++ b/internal/docs/man_test.go @@ -175,11 +175,10 @@ func assertNextLineEquals(scanner *bufio.Scanner, expectedLine string) error { } func BenchmarkGenManToFile(b *testing.B) { - file, err := ioutil.TempFile("", "") + file, err := ioutil.TempFile(b.TempDir(), "") if err != nil { b.Fatal(err) } - defer os.Remove(file.Name()) defer file.Close() b.ResetTimer() diff --git a/internal/docs/markdown_test.go b/internal/docs/markdown_test.go index 27be95efa..497a0384a 100644 --- a/internal/docs/markdown_test.go +++ b/internal/docs/markdown_test.go @@ -83,11 +83,10 @@ func TestGenMdTree(t *testing.T) { } func BenchmarkGenMarkdownToFile(b *testing.B) { - file, err := ioutil.TempFile("", "") + file, err := ioutil.TempFile(b.TempDir(), "") if err != nil { b.Fatal(err) } - defer os.Remove(file.Name()) defer file.Close() b.ResetTimer() diff --git a/pkg/cmd/actions/actions.go b/pkg/cmd/actions/actions.go index 33ba4b1a7..356ca1581 100644 --- a/pkg/cmd/actions/actions.go +++ b/pkg/cmd/actions/actions.go @@ -48,26 +48,26 @@ func actionsExplainer(cs *iostreams.ColorScheme) string { GitHub CLI integrates with Actions to help you manage runs and workflows. - %s - gh run list: List recent workflow runs - gh run view: View details for a workflow run or one of its jobs - gh run watch: Watch a workflow run while it executes - gh run rerun: Rerun a failed workflow run + %s + gh run list: List recent workflow runs + gh run view: View details for a workflow run or one of its jobs + gh run watch: Watch a workflow run while it executes + gh run rerun: Rerun a failed workflow run gh run download: Download artifacts generated by runs To see more help, run 'gh help run ' - %s - gh workflow list: List all the workflow files in your repository - gh workflow view: View details for a workflow file - gh workflow enable: Enable a workflow file - gh workflow disable: Disable a workflow file + %s + gh workflow list: List all the workflow files in your repository + gh workflow view: View details for a workflow file + gh workflow enable: Enable a workflow file + gh workflow disable: Disable a workflow file gh workflow run: Trigger a workflow_dispatch run for a workflow file To see more help, run 'gh help workflow ' For more in depth help including examples, see online documentation at: - https://docs.github.com/en/actions/guides/managing-github-actions-with-github-cli + `, header, runHeader, workflowHeader) } diff --git a/pkg/cmd/api/api.go b/pkg/cmd/api/api.go index da208b08a..27e26f004 100644 --- a/pkg/cmd/api/api.go +++ b/pkg/cmd/api/api.go @@ -71,8 +71,10 @@ func NewCmdApi(f *cmdutil.Factory, runF func(*ApiOptions) error) *cobra.Command The endpoint argument should either be a path of a GitHub API v3 endpoint, or "graphql" to access the GitHub API v4. - Placeholder values ":owner", ":repo", and ":branch" in the endpoint argument will - get replaced with values from the repository of the current directory. + Placeholder values "{owner}", "{repo}", and "{branch}" in the endpoint argument will + get replaced with values from the repository of the current directory. Note that in + some shells, for example PowerShell, you may need to enclose any value that contains + "{...}" in quotes to prevent the shell from applying special meaning to curly braces. The default HTTP request method is "GET" normally and "POST" if any parameters were added. Override the method with %[1]s--method%[1]s. @@ -87,7 +89,7 @@ func NewCmdApi(f *cmdutil.Factory, runF func(*ApiOptions) error) *cobra.Command - literal values "true", "false", "null", and integer numbers get converted to appropriate JSON types; - - placeholder values ":owner", ":repo", and ":branch" get populated with values + - placeholder values "{owner}", "{repo}", and "{branch}" get populated with values from the repository of the current directory; - if the value starts with "@", the rest of the value is interpreted as a filename to read the value from. Pass "-" to read from standard input. @@ -106,10 +108,10 @@ func NewCmdApi(f *cmdutil.Factory, runF func(*ApiOptions) error) *cobra.Command `, "`"), Example: heredoc.Doc(` # list releases in the current repository - $ gh api repos/:owner/:repo/releases + $ gh api repos/{owner}/{repo}/releases # post an issue comment - $ gh api repos/:owner/:repo/issues/123/comments -f body='Hi from CLI' + $ gh api repos/{owner}/{repo}/issues/123/comments -f body='Hi from CLI' # add parameters to a GET request $ gh api -X GET search/issues -f q='repo:cli/cli is:open remote' @@ -121,14 +123,14 @@ func NewCmdApi(f *cmdutil.Factory, runF func(*ApiOptions) error) *cobra.Command $ gh api --preview baptiste,nebula ... # print only specific fields from the response - $ gh api repos/:owner/:repo/issues --jq '.[].title' + $ gh api repos/{owner}/{repo}/issues --jq '.[].title' # use a template for the output - $ gh api repos/:owner/:repo/issues --template \ + $ gh api repos/{owner}/{repo}/issues --template \ '{{range .}}{{.title}} ({{.labels | pluck "name" | join ", " | color "yellow"}}){{"\n"}}{{end}}' # list releases with GraphQL - $ gh api graphql -F owner=':owner' -F name=':repo' -f query=' + $ gh api graphql -F owner='{owner}' -F name='{repo}' -f query=' query($name: String!, $owner: String!) { repository(owner: $owner, name: $name) { releases(last: 3) { @@ -397,41 +399,41 @@ func processResponse(resp *http.Response, opts *ApiOptions, headersOutputStream return } -var placeholderRE = regexp.MustCompile(`\:(owner|repo|branch)\b`) +var placeholderRE = regexp.MustCompile(`(\:(owner|repo|branch)\b|\{[a-z]+\})`) -// fillPlaceholders populates `:owner` and `:repo` placeholders with values from the current repository +// fillPlaceholders replaces placeholders with values from the current repository func fillPlaceholders(value string, opts *ApiOptions) (string, error) { - if !placeholderRE.MatchString(value) { - return value, nil - } + var err error + return placeholderRE.ReplaceAllStringFunc(value, func(m string) string { + var name string + if m[0] == ':' { + name = m[1:] + } else { + name = m[1 : len(m)-1] + } - baseRepo, err := opts.BaseRepo() - if err != nil { - return value, err - } - - filled := placeholderRE.ReplaceAllStringFunc(value, func(m string) string { - switch m { - case ":owner": - return baseRepo.RepoOwner() - case ":repo": - return baseRepo.RepoName() - case ":branch": - branch, e := opts.Branch() - if e != nil { + switch name { + case "owner": + if baseRepo, e := opts.BaseRepo(); e == nil { + return baseRepo.RepoOwner() + } else { + err = e + } + case "repo": + if baseRepo, e := opts.BaseRepo(); e == nil { + return baseRepo.RepoName() + } else { + err = e + } + case "branch": + if branch, e := opts.Branch(); e == nil { + return branch + } else { err = e } - return branch - default: - panic(fmt.Sprintf("invalid placeholder: %q", m)) } - }) - - if err != nil { - return value, err - } - - return filled, nil + return m + }), err } func printHeaders(w io.Writer, headers http.Header, colorize bool) { diff --git a/pkg/cmd/api/api_test.go b/pkg/cmd/api/api_test.go index 6bab83f09..0d7290835 100644 --- a/pkg/cmd/api/api_test.go +++ b/pkg/cmd/api/api_test.go @@ -693,6 +693,9 @@ func Test_apiRun_inputFile(t *testing.T) { contentLength: 10, }, } + + tempDir := t.TempDir() + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { io, stdin, _, _ := iostreams.Test() @@ -702,13 +705,12 @@ func Test_apiRun_inputFile(t *testing.T) { if tt.inputFile == "-" { _, _ = stdin.Write(tt.inputContents) } else { - f, err := ioutil.TempFile("", tt.inputFile) + f, err := ioutil.TempFile(tempDir, tt.inputFile) if err != nil { t.Fatal(err) } _, _ = f.Write(tt.inputContents) - f.Close() - t.Cleanup(func() { os.Remove(f.Name()) }) + defer f.Close() inputFile = f.Name() } @@ -825,13 +827,13 @@ func Test_parseFields(t *testing.T) { } func Test_magicFieldValue(t *testing.T) { - f, err := ioutil.TempFile("", "gh-test") + f, err := ioutil.TempFile(t.TempDir(), "gh-test") if err != nil { t.Fatal(err) } + defer f.Close() + fmt.Fprint(f, "file contents") - f.Close() - t.Cleanup(func() { os.Remove(f.Name()) }) io, _, _, _ := iostreams.Test() @@ -870,7 +872,7 @@ func Test_magicFieldValue(t *testing.T) { wantErr: false, }, { - name: "placeholder", + name: "placeholder colon", args: args{ v: ":owner", opts: &ApiOptions{ @@ -883,6 +885,20 @@ func Test_magicFieldValue(t *testing.T) { want: "hubot", wantErr: false, }, + { + name: "placeholder braces", + args: args{ + v: "{owner}", + opts: &ApiOptions{ + IO: io, + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("hubot", "robot-uprising"), nil + }, + }, + }, + want: "hubot", + wantErr: false, + }, { name: "file", args: args{ @@ -918,13 +934,13 @@ func Test_magicFieldValue(t *testing.T) { } func Test_openUserFile(t *testing.T) { - f, err := ioutil.TempFile("", "gh-test") + f, err := ioutil.TempFile(t.TempDir(), "gh-test") if err != nil { t.Fatal(err) } + defer f.Close() + fmt.Fprint(f, "file contents") - f.Close() - t.Cleanup(func() { os.Remove(f.Name()) }) file, length, err := openUserFile(f.Name(), nil) if err != nil { @@ -964,7 +980,7 @@ func Test_fillPlaceholders(t *testing.T) { wantErr: false, }, { - name: "has substitutes", + name: "has substitutes (colon)", args: args{ value: "repos/:owner/:repo/releases", opts: &ApiOptions{ @@ -977,39 +993,96 @@ func Test_fillPlaceholders(t *testing.T) { wantErr: false, }, { - name: "has branch placeholder", + name: "has branch placeholder (colon)", args: args{ - value: "repos/cli/cli/branches/:branch/protection/required_status_checks", + value: "repos/owner/repo/branches/:branch/protection/required_status_checks", opts: &ApiOptions{ - BaseRepo: func() (ghrepo.Interface, error) { - return ghrepo.New("cli", "cli"), nil - }, + BaseRepo: nil, Branch: func() (string, error) { return "trunk", nil }, }, }, - want: "repos/cli/cli/branches/trunk/protection/required_status_checks", + want: "repos/owner/repo/branches/trunk/protection/required_status_checks", wantErr: false, }, { - name: "has branch placeholder and git is in detached head", + name: "has branch placeholder and git is in detached head (colon)", args: args{ value: "repos/:owner/:repo/branches/:branch", opts: &ApiOptions{ BaseRepo: func() (ghrepo.Interface, error) { - return ghrepo.New("cli", "cli"), nil + return ghrepo.New("hubot", "robot-uprising"), nil }, Branch: func() (string, error) { return "", git.ErrNotOnAnyBranch }, }, }, - want: "repos/:owner/:repo/branches/:branch", + want: "repos/hubot/robot-uprising/branches/:branch", wantErr: true, }, { - name: "no greedy substitutes", + name: "has substitutes", + args: args{ + value: "repos/{owner}/{repo}/releases", + opts: &ApiOptions{ + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("hubot", "robot-uprising"), nil + }, + }, + }, + want: "repos/hubot/robot-uprising/releases", + wantErr: false, + }, + { + name: "has branch placeholder", + args: args{ + value: "repos/owner/repo/branches/{branch}/protection/required_status_checks", + opts: &ApiOptions{ + BaseRepo: nil, + Branch: func() (string, error) { + return "trunk", nil + }, + }, + }, + want: "repos/owner/repo/branches/trunk/protection/required_status_checks", + wantErr: false, + }, + { + name: "has branch placeholder and git is in detached head", + args: args{ + value: "repos/{owner}/{repo}/branches/{branch}", + opts: &ApiOptions{ + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("hubot", "robot-uprising"), nil + }, + Branch: func() (string, error) { + return "", git.ErrNotOnAnyBranch + }, + }, + }, + want: "repos/hubot/robot-uprising/branches/{branch}", + wantErr: true, + }, + { + name: "surfaces errors in earlier placeholders", + args: args{ + value: "{branch}-{owner}", + opts: &ApiOptions{ + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("hubot", "robot-uprising"), nil + }, + Branch: func() (string, error) { + return "", git.ErrNotOnAnyBranch + }, + }, + }, + want: "{branch}-hubot", + wantErr: true, + }, + { + name: "no greedy substitutes (colon)", args: args{ value: ":ownership/:repository", opts: &ApiOptions{ @@ -1019,6 +1092,17 @@ func Test_fillPlaceholders(t *testing.T) { want: ":ownership/:repository", wantErr: false, }, + { + name: "non-placeholders are left intact", + args: args{ + value: "{}{ownership}/{repository}", + opts: &ApiOptions{ + BaseRepo: nil, + }, + }, + want: "{}{ownership}/{repository}", + wantErr: false, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/cmd/completion/completion.go b/pkg/cmd/completion/completion.go index 24414a5fc..7af5271ca 100644 --- a/pkg/cmd/completion/completion.go +++ b/pkg/cmd/completion/completion.go @@ -21,7 +21,7 @@ func NewCmdCompletion(io *iostreams.IOStreams) *cobra.Command { When installing GitHub CLI through a package manager, it's possible that no additional shell configuration is necessary to gain completion support. For - Homebrew, see https://docs.brew.sh/Shell-Completion + Homebrew, see If you need to set up completions manually, follow the instructions below. The exact config file locations might vary based on your system. Make sure to restart your diff --git a/pkg/cmd/gist/create/create.go b/pkg/cmd/gist/create/create.go index 645638bca..c10b5d27b 100644 --- a/pkg/cmd/gist/create/create.go +++ b/pkg/cmd/gist/create/create.go @@ -151,7 +151,13 @@ func createRun(opts *CreateOptions) error { var httpError api.HTTPError if errors.As(err, &httpError) { if httpError.OAuthScopes != "" && !strings.Contains(httpError.OAuthScopes, "gist") { - return fmt.Errorf("This command requires the 'gist' OAuth scope.\nPlease re-authenticate by doing `gh config set -h github.com oauth_token ''` and running the command again.") + return fmt.Errorf("This command requires the 'gist' OAuth scope.\nPlease re-authenticate with: gh auth refresh -h %s -s gist", host) + } + if httpError.StatusCode == http.StatusUnprocessableEntity { + if detectEmptyFiles(files) { + fmt.Fprintf(errOut, "%s Failed to create gist: %s\n", cs.FailureIcon(), "a gist file cannot be blank") + return cmdutil.SilentError + } } } return fmt.Errorf("%s Failed to create gist: %w", cs.Red("X"), err) @@ -266,3 +272,12 @@ func createGist(client *http.Client, hostname, description string, public bool, return &result, nil } + +func detectEmptyFiles(files map[string]*shared.GistFile) bool { + for _, file := range files { + if strings.TrimSpace(file.Content) == "" { + return true + } + } + return false +} diff --git a/pkg/cmd/gist/create/create_test.go b/pkg/cmd/gist/create/create_test.go index 0c2d0f39f..b454b1598 100644 --- a/pkg/cmd/gist/create/create_test.go +++ b/pkg/cmd/gist/create/create_test.go @@ -5,9 +5,11 @@ import ( "encoding/json" "io/ioutil" "net/http" + "path" "strings" "testing" + "github.com/MakeNowJust/heredoc" "github.com/cli/cli/internal/config" "github.com/cli/cli/internal/run" "github.com/cli/cli/pkg/cmd/gist/shared" @@ -18,10 +20,6 @@ import ( "github.com/stretchr/testify/assert" ) -const ( - fixtureFile = "../fixture.txt" -) - func Test_processFiles(t *testing.T) { fakeStdin := strings.NewReader("hey cool how is it going") files, err := processFiles(ioutil.NopCloser(fakeStdin), "", []string{"-"}) @@ -164,15 +162,22 @@ func TestNewCmdCreate(t *testing.T) { } func Test_createRun(t *testing.T) { + tempDir := t.TempDir() + fixtureFile := path.Join(tempDir, "fixture.txt") + assert.NoError(t, ioutil.WriteFile(fixtureFile, []byte("{}"), 0644)) + emptyFile := path.Join(tempDir, "empty.txt") + assert.NoError(t, ioutil.WriteFile(emptyFile, []byte(" \t\n"), 0644)) + tests := []struct { - name string - opts *CreateOptions - stdin string - wantOut string - wantStderr string - wantParams map[string]interface{} - wantErr bool - wantBrowse string + name string + opts *CreateOptions + stdin string + wantOut string + wantStderr string + wantParams map[string]interface{} + wantErr bool + wantBrowse string + responseStatus int }{ { name: "public", @@ -193,6 +198,7 @@ func Test_createRun(t *testing.T) { }, }, }, + responseStatus: http.StatusOK, }, { name: "with description", @@ -213,6 +219,7 @@ func Test_createRun(t *testing.T) { }, }, }, + responseStatus: http.StatusOK, }, { name: "multiple files", @@ -236,6 +243,28 @@ func Test_createRun(t *testing.T) { }, }, }, + responseStatus: http.StatusOK, + }, + { + name: "file with empty content", + opts: &CreateOptions{ + Filenames: []string{emptyFile}, + }, + wantOut: "", + wantStderr: heredoc.Doc(` + - Creating gist empty.txt + X Failed to create gist: a gist file cannot be blank + `), + wantErr: true, + wantParams: map[string]interface{}{ + "description": "", + "updated_at": "0001-01-01T00:00:00Z", + "public": false, + "files": map[string]interface{}{ + "empty.txt": map[string]interface{}{"content": " \t\n"}, + }, + }, + responseStatus: http.StatusUnprocessableEntity, }, { name: "stdin arg", @@ -256,6 +285,7 @@ func Test_createRun(t *testing.T) { }, }, }, + responseStatus: http.StatusOK, }, { name: "web arg", @@ -277,14 +307,22 @@ func Test_createRun(t *testing.T) { }, }, }, + responseStatus: http.StatusOK, }, } for _, tt := range tests { reg := &httpmock.Registry{} - reg.Register(httpmock.REST("POST", "gists"), - httpmock.JSONResponse(struct { - Html_url string - }{"https://gist.github.com/aa5a315d61ae9438b18d"})) + if tt.responseStatus == http.StatusOK { + reg.Register( + httpmock.REST("POST", "gists"), + httpmock.StringResponse(`{ + "html_url": "https://gist.github.com/aa5a315d61ae9438b18d" + }`)) + } else { + reg.Register( + httpmock.REST("POST", "gists"), + httpmock.StatusStringResponse(tt.responseStatus, "{}")) + } mockClient := func() (*http.Client, error) { return &http.Client{Transport: reg}, nil @@ -325,6 +363,32 @@ func Test_createRun(t *testing.T) { } } +func Test_detectEmptyFiles(t *testing.T) { + tests := []struct { + content string + isEmptyFile bool + }{ + { + content: "{}", + isEmptyFile: false, + }, + { + content: "\n\t", + isEmptyFile: true, + }, + } + + for _, tt := range tests { + files := map[string]*shared.GistFile{} + files["file"] = &shared.GistFile{ + Content: tt.content, + } + + isEmptyFile := detectEmptyFiles(files) + assert.Equal(t, tt.isEmptyFile, isEmptyFile) + } +} + func Test_CreateRun_reauth(t *testing.T) { reg := &httpmock.Registry{} reg.Register(httpmock.REST("POST", "gists"), func(req *http.Request) (*http.Response, error) { @@ -332,33 +396,24 @@ func Test_CreateRun_reauth(t *testing.T) { StatusCode: 404, Request: req, Header: map[string][]string{ - "X-Oauth-Scopes": {"coolScope"}, + "X-Oauth-Scopes": {"repo, read:org"}, }, Body: ioutil.NopCloser(bytes.NewBufferString("oh no")), }, nil }) - mockClient := func() (*http.Client, error) { - return &http.Client{Transport: reg}, nil - } - io, _, _, _ := iostreams.Test() opts := &CreateOptions{ - IO: io, - HttpClient: mockClient, + IO: io, + HttpClient: func() (*http.Client, error) { + return &http.Client{Transport: reg}, nil + }, Config: func() (config.Config, error) { return config.NewBlankConfig(), nil }, - Filenames: []string{fixtureFile}, } err := createRun(opts) - if err == nil { - t.Fatalf("expected oauth error") - } - - if !strings.Contains(err.Error(), "Please re-authenticate") { - t.Errorf("got unexpected error: %s", err) - } + assert.EqualError(t, err, "This command requires the 'gist' OAuth scope.\nPlease re-authenticate with: gh auth refresh -h github.com -s gist") } diff --git a/pkg/cmd/gist/fixture.txt b/pkg/cmd/gist/fixture.txt deleted file mode 100644 index 9e26dfeeb..000000000 --- a/pkg/cmd/gist/fixture.txt +++ /dev/null @@ -1 +0,0 @@ -{} \ No newline at end of file diff --git a/pkg/cmd/gist/shared/shared.go b/pkg/cmd/gist/shared/shared.go index 98d66a9f8..1eac50d32 100644 --- a/pkg/cmd/gist/shared/shared.go +++ b/pkg/cmd/gist/shared/shared.go @@ -20,7 +20,7 @@ type GistFile struct { Filename string `json:"filename,omitempty"` Type string `json:"type,omitempty"` Language string `json:"language,omitempty"` - Content string `json:"content,omitempty"` + Content string `json:"content"` } type GistOwner struct { diff --git a/pkg/cmd/issue/close/close.go b/pkg/cmd/issue/close/close.go index 0a5355c21..a90f99e13 100644 --- a/pkg/cmd/issue/close/close.go +++ b/pkg/cmd/issue/close/close.go @@ -65,7 +65,7 @@ func closeRun(opts *CloseOptions) error { return err } - if issue.Closed { + if issue.State == "CLOSED" { fmt.Fprintf(opts.IO.ErrOut, "%s Issue #%d (%s) is already closed\n", cs.Yellow("!"), issue.Number, issue.Title) return nil } diff --git a/pkg/cmd/issue/close/close_test.go b/pkg/cmd/issue/close/close_test.go index 5a2054309..35c2fe10f 100644 --- a/pkg/cmd/issue/close/close_test.go +++ b/pkg/cmd/issue/close/close_test.go @@ -96,7 +96,7 @@ func TestIssueClose_alreadyClosed(t *testing.T) { httpmock.StringResponse(` { "data": { "repository": { "hasIssuesEnabled": true, - "issue": { "number": 13, "title": "The title of the issue", "closed": true} + "issue": { "number": 13, "title": "The title of the issue", "state": "CLOSED"} } } }`), ) diff --git a/pkg/cmd/issue/create/create_test.go b/pkg/cmd/issue/create/create_test.go index eece13daf..3e69f386c 100644 --- a/pkg/cmd/issue/create/create_test.go +++ b/pkg/cmd/issue/create/create_test.go @@ -6,7 +6,6 @@ import ( "fmt" "io/ioutil" "net/http" - "os" "path/filepath" "strings" "testing" @@ -422,8 +421,9 @@ func TestIssueCreate_recover(t *testing.T) { }, }) - tmpfile, err := ioutil.TempFile(os.TempDir(), "testrecover*") + tmpfile, err := ioutil.TempFile(t.TempDir(), "testrecover*") assert.NoError(t, err) + defer tmpfile.Close() state := prShared.IssueMetadataState{ Title: "recovered title", diff --git a/pkg/cmd/issue/list/list.go b/pkg/cmd/issue/list/list.go index ff3aba976..c543a84dc 100644 --- a/pkg/cmd/issue/list/list.go +++ b/pkg/cmd/issue/list/list.go @@ -155,8 +155,7 @@ func listRun(opts *ListOptions) error { defer opts.IO.StopPager() if opts.Exporter != nil { - data := api.ExportIssues(listResult.Issues, opts.Exporter.Fields()) - return opts.Exporter.Write(opts.IO.Out, data, opts.IO.ColorEnabled()) + return opts.Exporter.Write(opts.IO.Out, listResult.Issues, opts.IO.ColorEnabled()) } if isTerminal { diff --git a/pkg/cmd/issue/reopen/reopen.go b/pkg/cmd/issue/reopen/reopen.go index b0bb784f3..8c57a157c 100644 --- a/pkg/cmd/issue/reopen/reopen.go +++ b/pkg/cmd/issue/reopen/reopen.go @@ -65,7 +65,7 @@ func reopenRun(opts *ReopenOptions) error { return err } - if !issue.Closed { + if issue.State == "OPEN" { fmt.Fprintf(opts.IO.ErrOut, "%s Issue #%d (%s) is already open\n", cs.Yellow("!"), issue.Number, issue.Title) return nil } diff --git a/pkg/cmd/issue/reopen/reopen_test.go b/pkg/cmd/issue/reopen/reopen_test.go index bce4bc882..e2604baaa 100644 --- a/pkg/cmd/issue/reopen/reopen_test.go +++ b/pkg/cmd/issue/reopen/reopen_test.go @@ -64,7 +64,7 @@ func TestIssueReopen(t *testing.T) { httpmock.StringResponse(` { "data": { "repository": { "hasIssuesEnabled": true, - "issue": { "id": "THE-ID", "number": 2, "closed": true, "title": "The title of the issue"} + "issue": { "id": "THE-ID", "number": 2, "state": "CLOSED", "title": "The title of the issue"} } } }`), ) http.Register( @@ -96,7 +96,7 @@ func TestIssueReopen_alreadyOpen(t *testing.T) { httpmock.StringResponse(` { "data": { "repository": { "hasIssuesEnabled": true, - "issue": { "number": 2, "closed": false, "title": "The title of the issue"} + "issue": { "number": 2, "state": "OPEN", "title": "The title of the issue"} } } }`), ) diff --git a/pkg/cmd/issue/status/status.go b/pkg/cmd/issue/status/status.go index 764ec52fa..2a8c97ce8 100644 --- a/pkg/cmd/issue/status/status.go +++ b/pkg/cmd/issue/status/status.go @@ -96,11 +96,11 @@ func statusRun(opts *StatusOptions) error { if opts.Exporter != nil { data := map[string]interface{}{ - "createdBy": api.ExportIssues(issuePayload.Authored.Issues, opts.Exporter.Fields()), - "assigned": api.ExportIssues(issuePayload.Assigned.Issues, opts.Exporter.Fields()), - "mentioned": api.ExportIssues(issuePayload.Mentioned.Issues, opts.Exporter.Fields()), + "createdBy": issuePayload.Authored.Issues, + "assigned": issuePayload.Assigned.Issues, + "mentioned": issuePayload.Mentioned.Issues, } - return opts.Exporter.Write(opts.IO.Out, &data, opts.IO.ColorEnabled()) + return opts.Exporter.Write(opts.IO.Out, data, opts.IO.ColorEnabled()) } out := opts.IO.Out diff --git a/pkg/cmd/issue/transfer/transfer_test.go b/pkg/cmd/issue/transfer/transfer_test.go index 51b2e1066..44b9ff112 100644 --- a/pkg/cmd/issue/transfer/transfer_test.go +++ b/pkg/cmd/issue/transfer/transfer_test.go @@ -118,7 +118,7 @@ func Test_transferRunSuccessfulIssueTransfer(t *testing.T) { httpmock.StringResponse(` { "data": { "repository": { "hasIssuesEnabled": true, - "issue": { "id": "THE-ID", "number": 1234, "closed": true, "title": "The title of the issue"} + "issue": { "id": "THE-ID", "number": 1234, "title": "The title of the issue"} } } }`)) http.Register( diff --git a/pkg/cmd/issue/view/view.go b/pkg/cmd/issue/view/view.go index e90c6a528..6888cc791 100644 --- a/pkg/cmd/issue/view/view.go +++ b/pkg/cmd/issue/view/view.go @@ -116,8 +116,7 @@ func viewRun(opts *ViewOptions) error { defer opts.IO.StopPager() if opts.Exporter != nil { - exportIssue := issue.ExportData(opts.Exporter.Fields()) - return opts.Exporter.Write(opts.IO.Out, exportIssue, opts.IO.ColorEnabled()) + return opts.Exporter.Write(opts.IO.Out, issue, opts.IO.ColorEnabled()) } if opts.IO.IsStdoutTTY() { diff --git a/pkg/cmd/pr/create/create.go b/pkg/cmd/pr/create/create.go index c368e4e43..7ab899e85 100644 --- a/pkg/cmd/pr/create/create.go +++ b/pkg/cmd/pr/create/create.go @@ -681,7 +681,7 @@ func handlePush(opts CreateOptions, ctx CreateContext) error { // one by forking the base repository if headRepo == nil && ctx.IsPushEnabled { opts.IO.StartProgressIndicator() - headRepo, err = api.ForkRepo(client, ctx.BaseRepo) + headRepo, err = api.ForkRepo(client, ctx.BaseRepo, "") opts.IO.StopProgressIndicator() if err != nil { return fmt.Errorf("error forking repo: %w", err) diff --git a/pkg/cmd/pr/create/create_test.go b/pkg/cmd/pr/create/create_test.go index 8cc2549eb..a3d24c4d6 100644 --- a/pkg/cmd/pr/create/create_test.go +++ b/pkg/cmd/pr/create/create_test.go @@ -6,7 +6,6 @@ import ( "fmt" "io/ioutil" "net/http" - "os" "path/filepath" "testing" @@ -305,8 +304,9 @@ func TestPRCreate_recover(t *testing.T) { }, }) - tmpfile, err := ioutil.TempFile(os.TempDir(), "testrecover*") + tmpfile, err := ioutil.TempFile(t.TempDir(), "testrecover*") assert.NoError(t, err) + defer tmpfile.Close() state := prShared.IssueMetadataState{ Title: "recovered title", diff --git a/pkg/cmd/pr/list/list.go b/pkg/cmd/pr/list/list.go index f0ccfe11a..faf7a3e24 100644 --- a/pkg/cmd/pr/list/list.go +++ b/pkg/cmd/pr/list/list.go @@ -155,8 +155,7 @@ func listRun(opts *ListOptions) error { defer opts.IO.StopPager() if opts.Exporter != nil { - data := api.ExportPRs(listResult.PullRequests, opts.Exporter.Fields()) - return opts.Exporter.Write(opts.IO.Out, data, opts.IO.ColorEnabled()) + return opts.Exporter.Write(opts.IO.Out, listResult.PullRequests, opts.IO.ColorEnabled()) } if opts.IO.IsStdoutTTY() { diff --git a/pkg/cmd/pr/shared/preserve_test.go b/pkg/cmd/pr/shared/preserve_test.go index 892973f26..6b6e348d0 100644 --- a/pkg/cmd/pr/shared/preserve_test.go +++ b/pkg/cmd/pr/shared/preserve_test.go @@ -4,7 +4,6 @@ import ( "encoding/json" "errors" "io/ioutil" - "os" "testing" "github.com/cli/cli/pkg/iostreams" @@ -70,6 +69,8 @@ func Test_PreserveInput(t *testing.T) { }, } + tempDir := t.TempDir() + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { if tt.state == nil { @@ -78,9 +79,9 @@ func Test_PreserveInput(t *testing.T) { io, _, _, errOut := iostreams.Test() - tf, tferr := tmpfile() + tf, tferr := ioutil.TempFile(tempDir, "testfile*") assert.NoError(t, tferr) - defer os.Remove(tf.Name()) + defer tf.Close() io.TempFileOverride = tf @@ -111,13 +112,3 @@ func Test_PreserveInput(t *testing.T) { }) } } - -func tmpfile() (*os.File, error) { - dir := os.TempDir() - tmpfile, err := ioutil.TempFile(dir, "testfile*") - if err != nil { - return nil, err - } - - return tmpfile, nil -} diff --git a/pkg/cmd/pr/status/status.go b/pkg/cmd/pr/status/status.go index 115ee035b..d14ae5ec2 100644 --- a/pkg/cmd/pr/status/status.go +++ b/pkg/cmd/pr/status/status.go @@ -113,13 +113,13 @@ func statusRun(opts *StatusOptions) error { if opts.Exporter != nil { data := map[string]interface{}{ "currentBranch": nil, - "createdBy": api.ExportPRs(prPayload.ViewerCreated.PullRequests, opts.Exporter.Fields()), - "needsReview": api.ExportPRs(prPayload.ReviewRequested.PullRequests, opts.Exporter.Fields()), + "createdBy": prPayload.ViewerCreated.PullRequests, + "needsReview": prPayload.ReviewRequested.PullRequests, } if prPayload.CurrentPR != nil { - data["currentBranch"] = prPayload.CurrentPR.ExportData(opts.Exporter.Fields()) + data["currentBranch"] = prPayload.CurrentPR } - return opts.Exporter.Write(opts.IO.Out, &data, opts.IO.ColorEnabled()) + return opts.Exporter.Write(opts.IO.Out, data, opts.IO.ColorEnabled()) } out := opts.IO.Out diff --git a/pkg/cmd/pr/view/view.go b/pkg/cmd/pr/view/view.go index b8e32189d..fd924387c 100644 --- a/pkg/cmd/pr/view/view.go +++ b/pkg/cmd/pr/view/view.go @@ -119,8 +119,7 @@ func viewRun(opts *ViewOptions) error { defer opts.IO.StopPager() if opts.Exporter != nil { - exportPR := pr.ExportData(opts.Exporter.Fields()) - return opts.Exporter.Write(opts.IO.Out, exportPR, opts.IO.ColorEnabled()) + return opts.Exporter.Write(opts.IO.Out, pr, opts.IO.ColorEnabled()) } if connectedToTerminal { diff --git a/pkg/cmd/repo/fork/fork.go b/pkg/cmd/repo/fork/fork.go index 30df5ae67..1bebcd7df 100644 --- a/pkg/cmd/repo/fork/fork.go +++ b/pkg/cmd/repo/fork/fork.go @@ -36,6 +36,7 @@ type ForkOptions struct { PromptClone bool PromptRemote bool RemoteName string + Organization string Rename bool } @@ -110,6 +111,7 @@ Additional 'git clone' flags can be passed in by listing them after '--'.`, cmd.Flags().BoolVar(&opts.Clone, "clone", false, "Clone the fork {true|false}") cmd.Flags().BoolVar(&opts.Remote, "remote", false, "Add remote for fork {true|false}") cmd.Flags().StringVar(&opts.RemoteName, "remote-name", "origin", "Specify a name for a fork's new remote.") + cmd.Flags().StringVar(&opts.Organization, "org", "", "Create the fork in an organization") return cmd } @@ -169,7 +171,7 @@ func forkRun(opts *ForkOptions) error { apiClient := api.NewClientFromHTTP(httpClient) opts.IO.StartProgressIndicator() - forkedRepo, err := api.ForkRepo(apiClient, repoToFork) + forkedRepo, err := api.ForkRepo(apiClient, repoToFork, opts.Organization) opts.IO.StopProgressIndicator() if err != nil { return fmt.Errorf("failed to fork: %w", err) diff --git a/pkg/cmd/repo/fork/fork_test.go b/pkg/cmd/repo/fork/fork_test.go index d1210048b..26f69aa97 100644 --- a/pkg/cmd/repo/fork/fork_test.go +++ b/pkg/cmd/repo/fork/fork_test.go @@ -2,12 +2,14 @@ package fork import ( "bytes" + "io/ioutil" "net/http" "net/url" "regexp" "testing" "time" + "github.com/MakeNowJust/heredoc" "github.com/cli/cli/context" "github.com/cli/cli/git" "github.com/cli/cli/internal/config" @@ -72,8 +74,9 @@ func TestNewCmdFork(t *testing.T) { name: "blank nontty", cli: "", wants: ForkOptions{ - RemoteName: "origin", - Rename: true, + RemoteName: "origin", + Rename: true, + Organization: "", }, }, { @@ -85,6 +88,7 @@ func TestNewCmdFork(t *testing.T) { PromptClone: true, PromptRemote: true, Rename: true, + Organization: "", }, }, { @@ -104,6 +108,16 @@ func TestNewCmdFork(t *testing.T) { Rename: true, }, }, + { + name: "to org", + cli: "--org batmanshome", + wants: ForkOptions{ + RemoteName: "origin", + Remote: false, + Rename: false, + Organization: "batmanshome", + }, + }, } for _, tt := range tests { @@ -141,6 +155,7 @@ func TestNewCmdFork(t *testing.T) { assert.Equal(t, tt.wants.Remote, gotOpts.Remote) assert.Equal(t, tt.wants.PromptRemote, gotOpts.PromptRemote) assert.Equal(t, tt.wants.PromptClone, gotOpts.PromptClone) + assert.Equal(t, tt.wants.Organization, gotOpts.Organization) }) } } @@ -289,6 +304,7 @@ func TestRepoFork_in_parent_tty(t *testing.T) { assert.Equal(t, "✓ Created fork someone/REPO\n✓ Added remote origin\n", output.Stderr()) reg.Verify(t) } + func TestRepoFork_in_parent_nontty(t *testing.T) { defer stubSince(2 * time.Second)() reg := &httpmock.Registry{} @@ -409,37 +425,65 @@ func TestRepoFork_in_parent(t *testing.T) { func TestRepoFork_outside(t *testing.T) { tests := []struct { - name string - args string + name string + args string + postBody string + responseBody string + wantStderr string }{ { - name: "url arg", - args: "--clone=false http://github.com/OWNER/REPO.git", + name: "url arg", + args: "--clone=false http://github.com/OWNER/REPO.git", + postBody: "{}\n", + responseBody: `{"name":"REPO", "owner":{"login":"monalisa"}}`, + wantStderr: heredoc.Doc(` + ✓ Created fork monalisa/REPO + `), }, { - name: "full name arg", - args: "--clone=false OWNER/REPO", + name: "full name arg", + args: "--clone=false OWNER/REPO", + postBody: "{}\n", + responseBody: `{"name":"REPO", "owner":{"login":"monalisa"}}`, + wantStderr: heredoc.Doc(` + ✓ Created fork monalisa/REPO + `), + }, + { + name: "fork to org without clone", + args: "--clone=false OWNER/REPO --org batmanshome", + postBody: "{\"organization\":\"batmanshome\"}\n", + responseBody: `{"name":"REPO", "owner":{"login":"BatmansHome"}}`, + wantStderr: heredoc.Doc(` + ✓ Created fork BatmansHome/REPO + `), }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { defer stubSince(2 * time.Second)() + reg := &httpmock.Registry{} - defer reg.StubWithFixturePath(200, "./forkResult.json")() + reg.Register( + httpmock.REST("POST", "repos/OWNER/REPO/forks"), + func(req *http.Request) (*http.Response, error) { + bb, err := ioutil.ReadAll(req.Body) + if err != nil { + return nil, err + } + assert.Equal(t, tt.postBody, string(bb)) + return &http.Response{ + Request: req, + StatusCode: 200, + Body: ioutil.NopCloser(bytes.NewBufferString(tt.responseBody)), + }, nil + }) + httpClient := &http.Client{Transport: reg} - output, err := runCommand(httpClient, nil, true, tt.args) - if err != nil { - t.Errorf("error running command `repo fork`: %v", err) - } - + assert.NoError(t, err) assert.Equal(t, "", output.String()) - - r := regexp.MustCompile(`Created fork.*someone/REPO`) - if !r.MatchString(output.Stderr()) { - t.Errorf("output did not match regexp /%s/\n> output\n%s\n", r, output) - return - } + assert.Equal(t, tt.wantStderr, output.Stderr()) reg.Verify(t) }) } diff --git a/pkg/cmd/repo/list/http.go b/pkg/cmd/repo/list/http.go index 90e31fa4a..ef574b43c 100644 --- a/pkg/cmd/repo/list/http.go +++ b/pkg/cmd/repo/list/http.go @@ -1,48 +1,18 @@ package list import ( - "context" + "fmt" "net/http" - "reflect" "strings" - "time" - "github.com/cli/cli/internal/ghinstance" + "github.com/cli/cli/api" "github.com/cli/cli/pkg/githubsearch" "github.com/shurcooL/githubv4" - "github.com/shurcooL/graphql" ) -type Repository struct { - NameWithOwner string - Description string - IsFork bool - IsPrivate bool - IsArchived bool - PushedAt time.Time -} - -func (r Repository) Info() string { - var tags []string - - if r.IsPrivate { - tags = append(tags, "private") - } else { - tags = append(tags, "public") - } - if r.IsFork { - tags = append(tags, "fork") - } - if r.IsArchived { - tags = append(tags, "archived") - } - - return strings.Join(tags, ", ") -} - type RepositoryList struct { Owner string - Repositories []Repository + Repositories []api.Repository TotalCount int FromSearch bool } @@ -54,6 +24,7 @@ type FilterOptions struct { Language string Archived bool NonArchived bool + Fields []string } func listRepos(client *http.Client, hostname string, limit int, owner string, filter FilterOptions) (*RepositoryList, error) { @@ -67,62 +38,65 @@ func listRepos(client *http.Client, hostname string, limit int, owner string, fi } variables := map[string]interface{}{ - "perPage": githubv4.Int(perPage), - "endCursor": (*githubv4.String)(nil), + "perPage": githubv4.Int(perPage), } if filter.Visibility != "" { variables["privacy"] = githubv4.RepositoryPrivacy(strings.ToUpper(filter.Visibility)) - } else { - variables["privacy"] = (*githubv4.RepositoryPrivacy)(nil) } if filter.Fork { variables["fork"] = githubv4.Boolean(true) } else if filter.Source { variables["fork"] = githubv4.Boolean(false) - } else { - variables["fork"] = (*githubv4.Boolean)(nil) } + inputs := []string{"$perPage:Int!", "$endCursor:String", "$privacy:RepositoryPrivacy", "$fork:Boolean"} var ownerConnection string if owner == "" { - ownerConnection = `graphql:"repositoryOwner: viewer"` + ownerConnection = "repositoryOwner: viewer" } else { - ownerConnection = `graphql:"repositoryOwner(login: $owner)"` + ownerConnection = "repositoryOwner(login: $owner)" variables["owner"] = githubv4.String(owner) + inputs = append(inputs, "$owner:String!") } - type repositoryOwner struct { - Login string - Repositories struct { - Nodes []Repository - TotalCount int - PageInfo struct { - HasNextPage bool - EndCursor string + type result struct { + RepositoryOwner struct { + Login string + Repositories struct { + Nodes []api.Repository + TotalCount int + PageInfo struct { + HasNextPage bool + EndCursor string + } } - } `graphql:"repositories(first: $perPage, after: $endCursor, privacy: $privacy, isFork: $fork, ownerAffiliations: OWNER, orderBy: { field: PUSHED_AT, direction: DESC })"` + } } - query := reflect.StructOf([]reflect.StructField{ - { - Name: "RepositoryOwner", - Type: reflect.TypeOf(repositoryOwner{}), - Tag: reflect.StructTag(ownerConnection), - }, - }) - gql := graphql.NewClient(ghinstance.GraphQLEndpoint(hostname), client) + query := fmt.Sprintf(`query RepositoryList(%s) { + %s { + login + repositories(first: $perPage, after: $endCursor, privacy: $privacy, isFork: $fork, ownerAffiliations: OWNER, orderBy: { field: PUSHED_AT, direction: DESC }) { + nodes{%s} + totalCount + pageInfo{hasNextPage,endCursor} + } + } + }`, strings.Join(inputs, ","), ownerConnection, api.RepositoryGraphQL(filter.Fields)) + + apiClient := api.NewClientFromHTTP(client) listResult := RepositoryList{} pagination: for { - result := reflect.New(query) - err := gql.QueryNamed(context.Background(), "RepositoryList", result.Interface(), variables) + var res result + err := apiClient.GraphQL(hostname, query, variables, &res) if err != nil { return nil, err } - owner := result.Elem().FieldByName("RepositoryOwner").Interface().(repositoryOwner) + owner := res.RepositoryOwner listResult.TotalCount = owner.Repositories.TotalCount listResult.Owner = owner.Login @@ -143,47 +117,52 @@ pagination: } func searchRepos(client *http.Client, hostname string, limit int, owner string, filter FilterOptions) (*RepositoryList, error) { - type query struct { + type result struct { Search struct { RepositoryCount int - Nodes []struct { - Repository Repository `graphql:"...on Repository"` - } - PageInfo struct { + Nodes []api.Repository + PageInfo struct { HasNextPage bool EndCursor string } - } `graphql:"search(type: REPOSITORY, query: $query, first: $perPage, after: $endCursor)"` + } } + query := fmt.Sprintf(`query RepositoryListSearch($query:String!,$perPage:Int!,$endCursor:String) { + search(type: REPOSITORY, query: $query, first: $perPage, after: $endCursor) { + repositoryCount + nodes{...on Repository{%s}} + pageInfo{hasNextPage,endCursor} + } + }`, api.RepositoryGraphQL(filter.Fields)) + perPage := limit if perPage > 100 { perPage = 100 } variables := map[string]interface{}{ - "query": githubv4.String(searchQuery(owner, filter)), - "perPage": githubv4.Int(perPage), - "endCursor": (*githubv4.String)(nil), + "query": githubv4.String(searchQuery(owner, filter)), + "perPage": githubv4.Int(perPage), } - gql := graphql.NewClient(ghinstance.GraphQLEndpoint(hostname), client) + apiClient := api.NewClientFromHTTP(client) listResult := RepositoryList{FromSearch: true} pagination: for { - var result query - err := gql.QueryNamed(context.Background(), "RepositoryListSearch", &result, variables) + var result result + err := apiClient.GraphQL(hostname, query, variables, &result) if err != nil { return nil, err } listResult.TotalCount = result.Search.RepositoryCount - for _, node := range result.Search.Nodes { - if listResult.Owner == "" { - idx := strings.IndexRune(node.Repository.NameWithOwner, '/') - listResult.Owner = node.Repository.NameWithOwner[:idx] + for _, repo := range result.Search.Nodes { + if listResult.Owner == "" && repo.NameWithOwner != "" { + idx := strings.IndexRune(repo.NameWithOwner, '/') + listResult.Owner = repo.NameWithOwner[:idx] } - listResult.Repositories = append(listResult.Repositories, node.Repository) + listResult.Repositories = append(listResult.Repositories, repo) if len(listResult.Repositories) >= limit { break pagination } diff --git a/pkg/cmd/repo/list/list.go b/pkg/cmd/repo/list/list.go index 5b0af46f0..3f1bf64f3 100644 --- a/pkg/cmd/repo/list/list.go +++ b/pkg/cmd/repo/list/list.go @@ -3,8 +3,10 @@ package list import ( "fmt" "net/http" + "strings" "time" + "github.com/cli/cli/api" "github.com/cli/cli/internal/config" "github.com/cli/cli/pkg/cmdutil" "github.com/cli/cli/pkg/iostreams" @@ -17,6 +19,7 @@ type ListOptions struct { HttpClient func() (*http.Client, error) Config func() (config.Config, error) IO *iostreams.IOStreams + Exporter cmdutil.Exporter Limit int Owner string @@ -88,10 +91,13 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman cmd.Flags().StringVarP(&opts.Language, "language", "l", "", "Filter by primary coding language") cmd.Flags().BoolVar(&opts.Archived, "archived", false, "Show only archived repositories") cmd.Flags().BoolVar(&opts.NonArchived, "no-archived", false, "Omit archived repositories") + cmdutil.AddJSONFlags(cmd, &opts.Exporter, api.RepositoryFields) return cmd } +var defaultFields = []string{"nameWithOwner", "description", "isPrivate", "isFork", "isArchived", "createdAt", "pushedAt"} + func listRun(opts *ListOptions) error { httpClient, err := opts.HttpClient() if err != nil { @@ -105,6 +111,10 @@ func listRun(opts *ListOptions) error { Language: opts.Language, Archived: opts.Archived, NonArchived: opts.NonArchived, + Fields: defaultFields, + } + if opts.Exporter != nil { + filter.Fields = opts.Exporter.Fields() } cfg, err := opts.Config() @@ -127,27 +137,31 @@ func listRun(opts *ListOptions) error { } defer opts.IO.StopPager() + if opts.Exporter != nil { + return opts.Exporter.Write(opts.IO.Out, listResult.Repositories, opts.IO.ColorEnabled()) + } + cs := opts.IO.ColorScheme() tp := utils.NewTablePrinter(opts.IO) now := opts.Now() for _, repo := range listResult.Repositories { - info := repo.Info() + info := repoInfo(repo) infoColor := cs.Gray if repo.IsPrivate { infoColor = cs.Yellow } t := repo.PushedAt - // if listResult.FromSearch { - // t = repo.UpdatedAt - // } + if repo.PushedAt == nil { + t = &repo.CreatedAt + } tp.AddField(repo.NameWithOwner, nil, cs.Bold) tp.AddField(text.ReplaceExcessiveWhitespace(repo.Description), nil, nil) tp.AddField(info, nil, infoColor) if tp.IsTTY() { - tp.AddField(utils.FuzzyAgoAbbr(now, t), nil, cs.Gray) + tp.AddField(utils.FuzzyAgoAbbr(now, *t), nil, cs.Gray) } else { tp.AddField(t.Format(time.RFC3339), nil, nil) } @@ -179,3 +193,21 @@ func listHeader(owner string, matchCount, totalMatchCount int, hasFilters bool) } return fmt.Sprintf("Showing %d of %d repositories in @%s%s", matchCount, totalMatchCount, owner, matchStr) } + +func repoInfo(r api.Repository) string { + var tags []string + + if r.IsPrivate { + tags = append(tags, "private") + } else { + tags = append(tags, "public") + } + if r.IsFork { + tags = append(tags, "fork") + } + if r.IsArchived { + tags = append(tags, "archived") + } + + return strings.Join(tags, ", ") +} diff --git a/pkg/cmd/repo/view/http.go b/pkg/cmd/repo/view/http.go index 24eaaa676..3ef1dc784 100644 --- a/pkg/cmd/repo/view/http.go +++ b/pkg/cmd/repo/view/http.go @@ -12,6 +12,25 @@ import ( var NotFoundError = errors.New("not found") +func fetchRepository(apiClient *api.Client, repo ghrepo.Interface, fields []string) (*api.Repository, error) { + query := fmt.Sprintf(`query RepositoryInfo($owner: String!, $name: String!) { + repository(owner: $owner, name: $name) {%s} + }`, api.RepositoryGraphQL(fields)) + + variables := map[string]interface{}{ + "owner": repo.RepoOwner(), + "name": repo.RepoName(), + } + + var result struct { + Repository api.Repository + } + if err := apiClient.GraphQL(repo.RepoHost(), query, variables, &result); err != nil { + return nil, err + } + return api.InitRepoHostname(&result.Repository, repo.RepoHost()), nil +} + type RepoReadme struct { Filename string Content string diff --git a/pkg/cmd/repo/view/view.go b/pkg/cmd/repo/view/view.go index fcc785e62..7e507c182 100644 --- a/pkg/cmd/repo/view/view.go +++ b/pkg/cmd/repo/view/view.go @@ -29,6 +29,7 @@ type ViewOptions struct { IO *iostreams.IOStreams BaseRepo func() (ghrepo.Interface, error) Browser browser + Exporter cmdutil.Exporter RepoArg string Web bool @@ -67,10 +68,13 @@ With '--branch', view a specific branch of the repository.`, cmd.Flags().BoolVarP(&opts.Web, "web", "w", false, "Open a repository in the browser") cmd.Flags().StringVarP(&opts.Branch, "branch", "b", "", "View a specific branch of the repository") + cmdutil.AddJSONFlags(cmd, &opts.Exporter, api.RepositoryFields) return cmd } +var defaultFields = []string{"name", "owner", "description"} + func viewRun(opts *ViewOptions) error { httpClient, err := opts.HttpClient() if err != nil { @@ -101,11 +105,24 @@ func viewRun(opts *ViewOptions) error { } } - repo, err := api.GitHubRepo(apiClient, toView) + var readme *RepoReadme + fields := defaultFields + if opts.Exporter != nil { + fields = opts.Exporter.Fields() + } + + repo, err := fetchRepository(apiClient, toView, fields) if err != nil { return err } + if !opts.Web && opts.Exporter == nil { + readme, err = RepositoryReadme(httpClient, toView, opts.Branch) + if err != nil && !errors.Is(err, NotFoundError) { + return err + } + } + openURL := generateBranchURL(toView, opts.Branch) if opts.Web { if opts.IO.IsStdoutTTY() { @@ -114,21 +131,17 @@ func viewRun(opts *ViewOptions) error { return opts.Browser.Browse(openURL) } - fullName := ghrepo.FullName(toView) - - readme, err := RepositoryReadme(httpClient, toView, opts.Branch) - if err != nil && err != NotFoundError { - return err - } - opts.IO.DetectTerminalTheme() - - err = opts.IO.StartPager() - if err != nil { + if err := opts.IO.StartPager(); err != nil { return err } defer opts.IO.StopPager() + if opts.Exporter != nil { + return opts.Exporter.Write(opts.IO.Out, repo, opts.IO.ColorEnabled()) + } + + fullName := ghrepo.FullName(toView) stdout := opts.IO.Out if !opts.IO.IsStdoutTTY() { diff --git a/pkg/cmd/repo/view/view_test.go b/pkg/cmd/repo/view/view_test.go index fd91dcd70..b208b1f5a 100644 --- a/pkg/cmd/repo/view/view_test.go +++ b/pkg/cmd/repo/view/view_test.go @@ -3,10 +3,12 @@ package view import ( "bytes" "fmt" + "io" "net/http" "testing" "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/api" "github.com/cli/cli/internal/ghrepo" "github.com/cli/cli/internal/run" "github.com/cli/cli/pkg/cmdutil" @@ -625,3 +627,51 @@ func Test_ViewRun_HandlesSpecialCharacters(t *testing.T) { }) } } + +func Test_viewRun_json(t *testing.T) { + io, _, stdout, stderr := iostreams.Test() + io.SetStdoutTTY(false) + + reg := &httpmock.Registry{} + defer reg.Verify(t) + reg.StubRepoInfoResponse("OWNER", "REPO", "main") + + opts := &ViewOptions{ + IO: io, + HttpClient: func() (*http.Client, error) { + return &http.Client{Transport: reg}, nil + }, + BaseRepo: func() (ghrepo.Interface, error) { + return ghrepo.New("OWNER", "REPO"), nil + }, + Exporter: &testExporter{ + fields: []string{"name", "defaultBranchRef"}, + }, + } + + _, teardown := run.Stub() + defer teardown(t) + + err := viewRun(opts) + assert.NoError(t, err) + assert.Equal(t, heredoc.Doc(` + name: REPO + defaultBranchRef: main + `), stdout.String()) + assert.Equal(t, "", stderr.String()) +} + +type testExporter struct { + fields []string +} + +func (e *testExporter) Fields() []string { + return e.fields +} + +func (e *testExporter) Write(w io.Writer, data interface{}, colorize bool) error { + r := data.(*api.Repository) + fmt.Fprintf(w, "name: %s\n", r.Name) + fmt.Fprintf(w, "defaultBranchRef: %s\n", r.DefaultBranchRef.Name) + return nil +} diff --git a/pkg/cmd/workflow/run/run_test.go b/pkg/cmd/workflow/run/run_test.go index ee5195071..49693faef 100644 --- a/pkg/cmd/workflow/run/run_test.go +++ b/pkg/cmd/workflow/run/run_test.go @@ -7,7 +7,6 @@ import ( "fmt" "io/ioutil" "net/http" - "os" "testing" "github.com/cli/cli/api" @@ -149,13 +148,13 @@ func TestNewCmdRun(t *testing.T) { } func Test_magicFieldValue(t *testing.T) { - f, err := ioutil.TempFile("", "gh-test") + f, err := ioutil.TempFile(t.TempDir(), "gh-test") if err != nil { t.Fatal(err) } + defer f.Close() + fmt.Fprint(f, "file contents") - f.Close() - t.Cleanup(func() { os.Remove(f.Name()) }) io, _, _, _ := iostreams.Test() diff --git a/pkg/cmdutil/json_flags.go b/pkg/cmdutil/json_flags.go index 7b783c3ee..0ce5a8f50 100644 --- a/pkg/cmdutil/json_flags.go +++ b/pkg/cmdutil/json_flags.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "io" + "reflect" "sort" "strings" @@ -26,6 +27,21 @@ func AddJSONFlags(cmd *cobra.Command, exportTarget *Exporter, fields []string) { f.StringP("jq", "q", "", "Filter JSON output using a jq `expression`") f.StringP("template", "t", "", "Format JSON output using a Go template") + _ = cmd.RegisterFlagCompletionFunc("json", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { + var results []string + if idx := strings.IndexRune(toComplete, ','); idx >= 0 { + toComplete = toComplete[idx+1:] + } + toComplete = strings.ToLower(toComplete) + for _, f := range fields { + if strings.HasPrefix(strings.ToLower(f), toComplete) { + results = append(results, f) + } + } + sort.Strings(results) + return results, cobra.ShellCompDirectiveNoSpace + }) + oldPreRun := cmd.PreRunE cmd.PreRunE = func(c *cobra.Command, args []string) error { if oldPreRun != nil { @@ -102,11 +118,14 @@ func (e *exportFormat) Fields() []string { return e.fields } +// Write serializes data into JSON output written to w. If the object passed as data implements exportable, +// or if data is a map or slice of exportable object, ExportData() will be called on each object to obtain +// raw data for serialization. func (e *exportFormat) Write(w io.Writer, data interface{}, colorEnabled bool) error { buf := bytes.Buffer{} encoder := json.NewEncoder(&buf) encoder.SetEscapeHTML(false) - if err := encoder.Encode(data); err != nil { + if err := encoder.Encode(e.exportData(reflect.ValueOf(data))); err != nil { return err } @@ -121,3 +140,44 @@ func (e *exportFormat) Write(w io.Writer, data interface{}, colorEnabled bool) e _, err := io.Copy(w, &buf) return err } + +func (e *exportFormat) exportData(v reflect.Value) interface{} { + switch v.Kind() { + case reflect.Ptr, reflect.Interface: + if !v.IsNil() { + return e.exportData(v.Elem()) + } + case reflect.Slice: + a := make([]interface{}, v.Len()) + for i := 0; i < v.Len(); i++ { + a[i] = e.exportData(v.Index(i)) + } + return a + case reflect.Map: + t := reflect.MapOf(v.Type().Key(), emptyInterfaceType) + m := reflect.MakeMapWithSize(t, v.Len()) + iter := v.MapRange() + for iter.Next() { + ve := reflect.ValueOf(e.exportData(iter.Value())) + m.SetMapIndex(iter.Key(), ve) + } + return m.Interface() + case reflect.Struct: + if v.CanAddr() && reflect.PtrTo(v.Type()).Implements(exportableType) { + ve := v.Addr().Interface().(exportable) + return ve.ExportData(e.fields) + } else if v.Type().Implements(exportableType) { + ve := v.Interface().(exportable) + return ve.ExportData(e.fields) + } + } + return v.Interface() +} + +type exportable interface { + ExportData([]string) *map[string]interface{} +} + +var exportableType = reflect.TypeOf((*exportable)(nil)).Elem() +var sliceOfEmptyInterface []interface{} +var emptyInterfaceType = reflect.TypeOf(sliceOfEmptyInterface).Elem() diff --git a/pkg/cmdutil/json_flags_test.go b/pkg/cmdutil/json_flags_test.go index 61780db96..84825e30a 100644 --- a/pkg/cmdutil/json_flags_test.go +++ b/pkg/cmdutil/json_flags_test.go @@ -2,6 +2,7 @@ package cmdutil import ( "bytes" + "fmt" "io/ioutil" "testing" @@ -137,6 +138,29 @@ func Test_exportFormat_Write(t *testing.T) { wantW: "{\"name\":\"hubot\"}\n", wantErr: false, }, + { + name: "call ExportData", + exporter: exportFormat{fields: []string{"field1", "field2"}}, + args: args{ + data: &exportableItem{"item1"}, + colorEnabled: false, + }, + wantW: "{\"field1\":\"item1:field1\",\"field2\":\"item1:field2\"}\n", + wantErr: false, + }, + { + name: "recursively call ExportData", + exporter: exportFormat{fields: []string{"f1", "f2"}}, + args: args{ + data: map[string]interface{}{ + "s1": []exportableItem{{"i1"}, {"i2"}}, + "s2": []exportableItem{{"i3"}}, + }, + colorEnabled: false, + }, + wantW: "{\"s1\":[{\"f1\":\"i1:f1\",\"f2\":\"i1:f2\"},{\"f1\":\"i2:f1\",\"f2\":\"i2:f2\"}],\"s2\":[{\"f1\":\"i3:f1\",\"f2\":\"i3:f2\"}]}\n", + wantErr: false, + }, { name: "with jq filter", exporter: exportFormat{filter: ".name"}, @@ -166,8 +190,20 @@ func Test_exportFormat_Write(t *testing.T) { return } if gotW := w.String(); gotW != tt.wantW { - t.Errorf("exportFormat.Write() = %v, want %v", gotW, tt.wantW) + t.Errorf("exportFormat.Write() = %q, want %q", gotW, tt.wantW) } }) } } + +type exportableItem struct { + Name string +} + +func (e *exportableItem) ExportData(fields []string) *map[string]interface{} { + m := map[string]interface{}{} + for _, f := range fields { + m[f] = fmt.Sprintf("%s:%s", e.Name, f) + } + return &m +} diff --git a/pkg/githubtemplate/github_template_test.go b/pkg/githubtemplate/github_template_test.go index c9f42f552..b92d4523c 100644 --- a/pkg/githubtemplate/github_template_test.go +++ b/pkg/githubtemplate/github_template_test.go @@ -261,12 +261,11 @@ func TestFindLegacy(t *testing.T) { } func TestExtractName(t *testing.T) { - tmpfile, err := ioutil.TempFile("", "gh-cli") + tmpfile, err := ioutil.TempFile(t.TempDir(), "gh-cli") if err != nil { t.Fatal(err) } - tmpfile.Close() - defer os.Remove(tmpfile.Name()) + defer tmpfile.Close() type args struct { filePath string @@ -322,12 +321,11 @@ about: This is how you report bugs } func TestExtractContents(t *testing.T) { - tmpfile, err := ioutil.TempFile("", "gh-cli") + tmpfile, err := ioutil.TempFile(t.TempDir(), "gh-cli") if err != nil { t.Fatal(err) } - tmpfile.Close() - defer os.Remove(tmpfile.Name()) + defer tmpfile.Close() type args struct { filePath string diff --git a/pkg/jsoncolor/jsoncolor.go b/pkg/jsoncolor/jsoncolor.go index cf3bd064c..d7c808a14 100644 --- a/pkg/jsoncolor/jsoncolor.go +++ b/pkg/jsoncolor/jsoncolor.go @@ -10,7 +10,7 @@ import ( const ( colorDelim = "1;38" // bright white colorKey = "1;34" // bright blue - colorNull = "1;30" // gray + colorNull = "36" // cyan colorString = "32" // green colorBool = "33" // yellow )