diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json index 20bf83882..bc047d1c5 100644 --- a/.devcontainer/devcontainer.json +++ b/.devcontainer/devcontainer.json @@ -1,5 +1,5 @@ { - "image": "mcr.microsoft.com/devcontainers/go:1.22", + "image": "mcr.microsoft.com/devcontainers/go:1.23", "features": { "ghcr.io/devcontainers/features/sshd:1": {} }, diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 40d0a83e3..4e2032a87 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -24,7 +24,7 @@ We accept pull requests for bug fixes and features where we've discussed the app ## Building the project Prerequisites: -- Go 1.22+ +- Go 1.23+ Build with: * Unix-like systems: `make` diff --git a/README.md b/README.md index c227cffdc..fbd99a5a8 100644 --- a/README.md +++ b/README.md @@ -134,7 +134,7 @@ There are two common ways to verify a downloaded release, depending if `gh` is a - **Option 1: Using `gh` if already installed:** ```shell - $ % gh at verify -R cli/cli gh_2.62.0_macOS_arm64.zip + $ gh at verify -R cli/cli gh_2.62.0_macOS_arm64.zip Loaded digest sha256:fdb77f31b8a6dd23c3fd858758d692a45f7fc76383e37d475bdcae038df92afc for file://gh_2.62.0_macOS_arm64.zip Loaded 1 attestation from GitHub API ✓ Verification succeeded! diff --git a/api/queries_user.go b/api/queries_user.go index cf0121a8b..dd7be0105 100644 --- a/api/queries_user.go +++ b/api/queries_user.go @@ -31,7 +31,7 @@ func CurrentLoginNameAndOrgs(client *Client, hostname string) (string, []string, for _, org := range query.Viewer.Organizations.Nodes { orgNames = append(orgNames, org.Login) } - return query.Viewer.Login, orgNames, err + return query.Viewer.Login, orgNames, nil } func CurrentUserID(client *Client, hostname string) (string, error) { diff --git a/go.mod b/go.mod index 4e88a54d5..9d1afbc38 100644 --- a/go.mod +++ b/go.mod @@ -43,7 +43,7 @@ require ( github.com/sigstore/protobuf-specs v0.3.3 github.com/sigstore/sigstore-go v0.7.0 github.com/spf13/cobra v1.8.1 - github.com/spf13/pflag v1.0.5 + github.com/spf13/pflag v1.0.6 github.com/stretchr/testify v1.10.0 github.com/zalando/go-keyring v0.2.5 golang.org/x/crypto v0.32.0 @@ -79,7 +79,7 @@ require ( github.com/fsnotify/fsnotify v1.7.0 // indirect github.com/gdamore/encoding v1.0.0 // indirect github.com/go-chi/chi v4.1.2+incompatible // indirect - github.com/go-jose/go-jose/v4 v4.0.2 // indirect + github.com/go-jose/go-jose/v4 v4.0.5 // indirect github.com/go-logr/logr v1.4.2 // indirect github.com/go-logr/stdr v1.2.2 // indirect github.com/go-openapi/analysis v0.23.0 // indirect diff --git a/go.sum b/go.sum index d58e8de5b..2c14dc4c3 100644 --- a/go.sum +++ b/go.sum @@ -165,8 +165,8 @@ github.com/go-chi/chi v4.1.2+incompatible h1:fGFk2Gmi/YKXk0OmGfBh0WgmN3XB8lVnEyN github.com/go-chi/chi v4.1.2+incompatible/go.mod h1:eB3wogJHnLi3x/kFX2A+IbTBlXxmMeXJVKy9tTv1XzQ= github.com/go-jose/go-jose/v3 v3.0.3 h1:fFKWeig/irsp7XD2zBxvnmA/XaRWp5V3CBsZXJF7G7k= github.com/go-jose/go-jose/v3 v3.0.3/go.mod h1:5b+7YgP7ZICgJDBdfjZaIt+H/9L9T/YQrVfLAMboGkQ= -github.com/go-jose/go-jose/v4 v4.0.2 h1:R3l3kkBds16bO7ZFAEEcofK0MkrAJt3jlJznWZG0nvk= -github.com/go-jose/go-jose/v4 v4.0.2/go.mod h1:WVf9LFMHh/QVrmqrOfqun0C45tMe3RoiKJMPvgWwLfY= +github.com/go-jose/go-jose/v4 v4.0.5 h1:M6T8+mKZl/+fNNuFHvGIzDz7BTLQPIounk/b9dw3AaE= +github.com/go-jose/go-jose/v4 v4.0.5/go.mod h1:s3P1lRrkT8igV8D9OjyL4WRyHvjB6a4JSllnOrmmBOA= github.com/go-logr/logr v1.2.2/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= github.com/go-logr/logr v1.4.2 h1:6pFjapn8bFcIbiKo3XT4j/BhANplGihG6tvd+8rYgrY= github.com/go-logr/logr v1.4.2/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY= @@ -433,8 +433,9 @@ github.com/spf13/cast v1.7.0 h1:ntdiHjuueXFgm5nzDRdOS4yfT43P5Fnud6DH50rz/7w= github.com/spf13/cast v1.7.0/go.mod h1:ancEpBxwJDODSW/UG4rDrAqiKolqNNh2DX3mk86cAdo= github.com/spf13/cobra v1.8.1 h1:e5/vxKd/rZsfSJMUX1agtjeTDf+qv1/JdBF8gg5k9ZM= github.com/spf13/cobra v1.8.1/go.mod h1:wHxEcudfqmLYa8iTfL+OuZPbBZkmvliBWKIezN3kD9Y= -github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= +github.com/spf13/pflag v1.0.6 h1:jFzHGLGAlb3ruxLB8MhbI6A8+AQX/2eW4qeyNZXNp2o= +github.com/spf13/pflag v1.0.6/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/spf13/viper v1.19.0 h1:RWq5SEjt8o25SROyN3z2OrDB9l7RPd3lwTWU8EcEdcI= github.com/spf13/viper v1.19.0/go.mod h1:GQUN9bilAbhU/jgc1bKs99f/suXKeUMct8Adx5+Ntkg= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= diff --git a/pkg/cmd/actions/actions.go b/pkg/cmd/actions/actions.go index 663866844..76d72b954 100644 --- a/pkg/cmd/actions/actions.go +++ b/pkg/cmd/actions/actions.go @@ -34,19 +34,19 @@ func actionsExplainer(cs *iostreams.ColorScheme) string { GitHub CLI integrates with GitHub Actions to help you manage runs and workflows. %[3]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 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 %[1]sgh help run %[1]s %[4]s gh workflow list: List 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 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 %[1]sgh help workflow %[1]s diff --git a/pkg/cmd/alias/set/set.go b/pkg/cmd/alias/set/set.go index c09f7c0cc..6d4e21422 100644 --- a/pkg/cmd/alias/set/set.go +++ b/pkg/cmd/alias/set/set.go @@ -51,7 +51,7 @@ func NewCmdSet(f *cmdutil.Factory, runF func(*SetOptions) error) *cobra.Command invoked. This allows for chaining multiple commands via piping and redirection. `, "`"), Example: heredoc.Doc(` - # note: Command Prompt on Windows requires using double quotes for arguments + # Note: Command Prompt on Windows requires using double quotes for arguments $ gh alias set pv 'pr view' $ gh pv -w 123 #=> gh pr view -w 123 diff --git a/pkg/cmd/api/api.go b/pkg/cmd/api/api.go index a1308d09f..d25b4ce64 100644 --- a/pkg/cmd/api/api.go +++ b/pkg/cmd/api/api.go @@ -7,6 +7,7 @@ import ( "fmt" "io" "net/http" + "net/url" "os" "path/filepath" "regexp" @@ -124,39 +125,39 @@ func NewCmdApi(f *cmdutil.Factory, runF func(*ApiOptions) error) *cobra.Command into an outer JSON array. `, "`"), Example: heredoc.Doc(` - # list releases in the current repository + # List releases in the current repository $ gh api repos/{owner}/{repo}/releases - # post an issue comment + # Post an issue comment $ gh api repos/{owner}/{repo}/issues/123/comments -f body='Hi from CLI' - # post nested parameter read from a file + # Post nested parameter read from a file $ gh api gists -F 'files[myfile.txt][content]=@myfile.txt' - # add parameters to a GET request + # Add parameters to a GET request $ gh api -X GET search/issues -f q='repo:cli/cli is:open remote' - # set a custom HTTP header + # Set a custom HTTP header $ gh api -H 'Accept: application/vnd.github.v3.raw+json' ... - # opt into GitHub API previews + # Opt into GitHub API previews $ gh api --preview baptiste,nebula ... - # print only specific fields from the response + # Print only specific fields from the response $ gh api repos/{owner}/{repo}/issues --jq '.[].title' - # use a template for the output + # Use a template for the output $ gh api repos/{owner}/{repo}/issues --template \ '{{range .}}{{.title}} ({{.labels | pluck "name" | join ", " | color "yellow"}}){{"\n"}}{{end}}' - # update allowed values of the "environment" custom property in a deeply nested array - gh api -X PATCH /orgs/{org}/properties/schema \ + # Update allowed values of the "environment" custom property in a deeply nested array + $ gh api -X PATCH /orgs/{org}/properties/schema \ -F 'properties[][property_name]=environment' \ -F 'properties[][default_value]=production' \ -F 'properties[][allowed_values][]=staging' \ -F 'properties[][allowed_values][]=production' - # list releases with GraphQL + # List releases with GraphQL $ gh api graphql -F owner='{owner}' -F name='{repo}' -f query=' query($name: String!, $owner: String!) { repository(owner: $owner, name: $name) { @@ -167,7 +168,7 @@ func NewCmdApi(f *cmdutil.Factory, runF func(*ApiOptions) error) *cobra.Command } ' - # list all repositories for a user + # List all repositories for a user $ gh api graphql --paginate -f query=' query($endCursor: String) { viewer { @@ -182,7 +183,7 @@ func NewCmdApi(f *cmdutil.Factory, runF func(*ApiOptions) error) *cobra.Command } ' - # get the percentage of forks for the current user + # Get the percentage of forks for the current user $ gh api graphql --paginate --slurp -f query=' query($endCursor: String) { viewer { @@ -201,12 +202,12 @@ func NewCmdApi(f *cmdutil.Factory, runF func(*ApiOptions) error) *cobra.Command Annotations: map[string]string{ "help:environment": heredoc.Doc(` GH_TOKEN, GITHUB_TOKEN (in order of precedence): an authentication token for - github.com API requests. + API requests. GH_ENTERPRISE_TOKEN, GITHUB_ENTERPRISE_TOKEN (in order of precedence): an authentication token for API requests to GitHub Enterprise. - GH_HOST: make the request to a GitHub host other than github.com. + GH_HOST: make the request to a GitHub host other than . `), }, Args: cobra.ExactArgs(1), @@ -264,6 +265,8 @@ func NewCmdApi(f *cmdutil.Factory, runF func(*ApiOptions) error) *cobra.Command return err } + opts.RequestPath = escapePackageNameInPath(opts.RequestPath) + if runF != nil { return runF(&opts) } @@ -691,3 +694,37 @@ func previewNamesToMIMETypes(names []string) string { } return strings.Join(types, ", ") } + +// The package name part in the `packages` endpoints may contain slashes and +// other characters that need to be URL encoded. +// +// The `escapePackageNameInPath` function extracts and normalizes package names +// in the path. The regex `pathWithPackageNameRE` is being used to extract the +// package name with a capture group named `package`. +// +// See https://docs.github.com/en/rest/packages/packages APIs for more details. +// +// Here's an example: +// +// The package name `orders/cache` needs to be URL encoded because it contains +// a slash `/`. The `escapePackageNameInPath` function will extract the +// `orders/cache` part, perform the URL encoding, and return the normalized API +// endpoint with `%2F` replacing the slash `/` in the package name part only. +// +// - Package name: `orders/cache` +// - API endpoint: `/users/USER/packages/container/orders/cache` +// - Normalized: `/users/USER/packages/container/orders%2Fcache` + +var pathWithPackageNameRE = regexp.MustCompile(`^\/(?:orgs|user|users)(?:\/.*)?\/packages\/(?:npm|maven|rubygems|docker|nuget|container)\/(?.*?)(?:\/(?:restore|versions)|$)`) + +func escapePackageNameInPath(path string) string { + matches := pathWithPackageNameRE.FindStringSubmatch(path) + if len(matches) > 0 { + i := pathWithPackageNameRE.SubexpIndex("package") + packageName := matches[i] + if packageName != "" { + return strings.Replace(path, packageName, url.QueryEscape(packageName), 1) + } + } + return path +} diff --git a/pkg/cmd/api/api_test.go b/pkg/cmd/api/api_test.go index 321f7b7c0..ee58d55d0 100644 --- a/pkg/cmd/api/api_test.go +++ b/pkg/cmd/api/api_test.go @@ -367,6 +367,72 @@ func Test_NewCmdApi(t *testing.T) { }, wantsErr: false, }, + { + name: "request path with container package name containing slashes", + cli: "/user/packages/container/github.com/username/package_name --verbose", + wants: ApiOptions{ + Hostname: "", + RequestMethod: "GET", + RequestMethodPassed: false, + RequestPath: "/user/packages/container/github.com%2Fusername%2Fpackage_name", + RequestInputFile: "", + RawFields: []string(nil), + MagicFields: []string(nil), + RequestHeaders: []string(nil), + ShowResponseHeaders: false, + Paginate: false, + Silent: false, + CacheTTL: 0, + Template: "", + FilterOutput: "", + Verbose: true, + }, + wantsErr: false, + }, + { + name: "request path with container package name containing slashes and restore", + cli: "/user/packages/container/github.com/username/package_name/restore --verbose", + wants: ApiOptions{ + Hostname: "", + RequestMethod: "GET", + RequestMethodPassed: false, + RequestPath: "/user/packages/container/github.com%2Fusername%2Fpackage_name/restore", + RequestInputFile: "", + RawFields: []string(nil), + MagicFields: []string(nil), + RequestHeaders: []string(nil), + ShowResponseHeaders: false, + Paginate: false, + Silent: false, + CacheTTL: 0, + Template: "", + FilterOutput: "", + Verbose: true, + }, + wantsErr: false, + }, + { + name: "request path with container package name containing slashes and versions", + cli: "/user/packages/container/github.com/username/package_name/versions --verbose", + wants: ApiOptions{ + Hostname: "", + RequestMethod: "GET", + RequestMethodPassed: false, + RequestPath: "/user/packages/container/github.com%2Fusername%2Fpackage_name/versions", + RequestInputFile: "", + RawFields: []string(nil), + MagicFields: []string(nil), + RequestHeaders: []string(nil), + ShowResponseHeaders: false, + Paginate: false, + Silent: false, + CacheTTL: 0, + Template: "", + FilterOutput: "", + Verbose: true, + }, + wantsErr: false, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/cmd/attestation/api/mock_httpClient_test.go b/pkg/cmd/attestation/api/mock_httpClient_test.go index 26933ae2e..b082d13d2 100644 --- a/pkg/cmd/attestation/api/mock_httpClient_test.go +++ b/pkg/cmd/attestation/api/mock_httpClient_test.go @@ -5,6 +5,7 @@ import ( "fmt" "io" "net/http" + "sync" "github.com/cli/cli/v2/pkg/cmd/attestation/test/data" "github.com/golang/snappy" @@ -58,12 +59,16 @@ func (m *reqFailHttpClient) Get(url string) (*http.Response, error) { type failAfterNCallsHttpClient struct { mock.Mock + mu sync.Mutex FailOnCallN int FailOnAllSubsequentCalls bool NumCalls int } func (m *failAfterNCallsHttpClient) Get(url string) (*http.Response, error) { + m.mu.Lock() + defer m.mu.Unlock() + m.On("OnGetFailAfterNCalls").Return() m.NumCalls++ diff --git a/pkg/cmd/attestation/artifact/oci/client.go b/pkg/cmd/attestation/artifact/oci/client.go index bda114708..87e0189a8 100644 --- a/pkg/cmd/attestation/artifact/oci/client.go +++ b/pkg/cmd/attestation/artifact/oci/client.go @@ -89,7 +89,7 @@ func (a *noncompliantRegistryTransport) RoundTrip(req *http.Request) (*http.Resp resp.StatusCode = http.StatusNotFound } - return resp, err + return resp, nil } func (c LiveClient) GetAttestations(ref name.Reference, digest string) ([]*api.Attestation, error) { diff --git a/pkg/cmd/attestation/download/download.go b/pkg/cmd/attestation/download/download.go index cdbdc0078..6913c0787 100644 --- a/pkg/cmd/attestation/download/download.go +++ b/pkg/cmd/attestation/download/download.go @@ -107,7 +107,7 @@ func NewDownloadCmd(f *cmdutil.Factory, runF func(*Options) error) *cobra.Comman }, } - downloadCmd.Flags().StringVarP(&opts.Owner, "owner", "o", "", "a GitHub organization to scope attestation lookup by") + downloadCmd.Flags().StringVarP(&opts.Owner, "owner", "o", "", "GitHub organization to scope attestation lookup by") downloadCmd.Flags().StringVarP(&opts.Repo, "repo", "R", "", "Repository name in the format /") downloadCmd.MarkFlagsMutuallyExclusive("owner", "repo") downloadCmd.MarkFlagsOneRequired("owner", "repo") diff --git a/pkg/cmd/attestation/inspect/inspect.go b/pkg/cmd/attestation/inspect/inspect.go index b2d0adfb7..6fbddd6da 100644 --- a/pkg/cmd/attestation/inspect/inspect.go +++ b/pkg/cmd/attestation/inspect/inspect.go @@ -47,7 +47,7 @@ func NewInspectCmd(f *cmdutil.Factory, runF func(*Options) error) *cobra.Command timestamps, and if the included signatures match the provided public key. This command cannot be used to verify a bundle. To verify a bundle, see the - %[1]sgh at verify%[1]s command. + %[1]sgh at verify%[1]s command. By default, this command prints a condensed table. To see full results, provide the %[1]s--format=json%[1]s flag. diff --git a/pkg/cmd/attestation/trustedroot/trustedroot.go b/pkg/cmd/attestation/trustedroot/trustedroot.go index 1a40aae3f..4e55e27ab 100644 --- a/pkg/cmd/attestation/trustedroot/trustedroot.go +++ b/pkg/cmd/attestation/trustedroot/trustedroot.go @@ -11,6 +11,7 @@ import ( "github.com/cli/cli/v2/pkg/cmd/attestation/io" "github.com/cli/cli/v2/pkg/cmd/attestation/verification" "github.com/cli/cli/v2/pkg/cmdutil" + o "github.com/cli/cli/v2/pkg/option" ghauth "github.com/cli/go-gh/v2/pkg/auth" "github.com/MakeNowJust/heredoc" @@ -56,7 +57,7 @@ func NewTrustedRootCmd(f *cmdutil.Factory, runF func(*Options) error) *cobra.Com `, "`"), Example: heredoc.Doc(` # Get a trusted_root.jsonl for both Sigstore Public Good and GitHub's instance - gh attestation trusted-root + $ gh attestation trusted-root `), RunE: func(cmd *cobra.Command, args []string) error { if opts.Hostname == "" { @@ -121,7 +122,7 @@ func getTrustedRoot(makeTUF tufClientInstantiator, opts *Options) error { var tufOptions []tufConfig var defaultTR = "trusted_root.json" - tufOpt := verification.DefaultOptionsWithCacheSetting() + tufOpt := verification.DefaultOptionsWithCacheSetting(o.None[string]()) // Disable local caching, so we get up-to-date response from TUF repository tufOpt.CacheValidity = 0 @@ -150,7 +151,7 @@ func getTrustedRoot(makeTUF tufClientInstantiator, opts *Options) error { targets: []string{defaultTR}, }) - tufOpt = verification.GitHubTUFOptions() + tufOpt = verification.GitHubTUFOptions(o.None[string]()) tufOpt.CacheValidity = 0 tufOptions = append(tufOptions, tufConfig{ tufOptions: tufOpt, diff --git a/pkg/cmd/attestation/verification/extensions.go b/pkg/cmd/attestation/verification/extensions.go index 2958408d0..3ac9ac0a0 100644 --- a/pkg/cmd/attestation/verification/extensions.go +++ b/pkg/cmd/attestation/verification/extensions.go @@ -59,5 +59,15 @@ func verifyCertExtensions(given, expected certificate.Summary) error { return fmt.Errorf("expected Issuer to be %s, got %s", expected.Issuer, given.Issuer) } + if expected.BuildSignerDigest != "" && !strings.EqualFold(expected.BuildSignerDigest, given.BuildSignerDigest) { + return fmt.Errorf("expected BuildSignerDigest to be %s, got %s", expected.BuildSignerDigest, given.BuildSignerDigest) + } + if expected.SourceRepositoryDigest != "" && !strings.EqualFold(expected.SourceRepositoryDigest, given.SourceRepositoryDigest) { + return fmt.Errorf("expected SourceRepositoryDigest to be %s, got %s", expected.SourceRepositoryDigest, given.SourceRepositoryDigest) + } + if expected.SourceRepositoryRef != "" && !strings.EqualFold(expected.SourceRepositoryRef, given.SourceRepositoryRef) { + return fmt.Errorf("expected SourceRepositoryRef to be %s, got %s", expected.SourceRepositoryRef, given.SourceRepositoryRef) + } + return nil } diff --git a/pkg/cmd/attestation/verification/policy.go b/pkg/cmd/attestation/verification/policy.go index f2bd126d0..284560466 100644 --- a/pkg/cmd/attestation/verification/policy.go +++ b/pkg/cmd/attestation/verification/policy.go @@ -52,7 +52,7 @@ func (c EnforcementCriteria) Valid() error { } func (c EnforcementCriteria) BuildPolicyInformation() string { - policyAttr := make([][]string, 0, 6) + policyAttr := [][]string{} policyAttr = appendStr(policyAttr, "- Predicate type must match", c.PredicateType) @@ -62,6 +62,16 @@ func (c EnforcementCriteria) BuildPolicyInformation() string { policyAttr = appendStr(policyAttr, "- Source Repository URI must match", c.Certificate.SourceRepositoryURI) } + if c.Certificate.BuildSignerDigest != "" { + policyAttr = appendStr(policyAttr, "- Build signer digest must match", c.Certificate.BuildSignerDigest) + } + if c.Certificate.SourceRepositoryDigest != "" { + policyAttr = appendStr(policyAttr, "- Source repo digest digest must match", c.Certificate.SourceRepositoryDigest) + } + if c.Certificate.SourceRepositoryRef != "" { + policyAttr = appendStr(policyAttr, "- Source repo ref must match", c.Certificate.SourceRepositoryRef) + } + if c.SAN != "" { policyAttr = appendStr(policyAttr, "- Subject Alternative Name must match", c.SAN) } else if c.SANRegex != "" { diff --git a/pkg/cmd/attestation/verification/sigstore.go b/pkg/cmd/attestation/verification/sigstore.go index c71c600c7..6dd31dac0 100644 --- a/pkg/cmd/attestation/verification/sigstore.go +++ b/pkg/cmd/attestation/verification/sigstore.go @@ -10,6 +10,7 @@ import ( "github.com/cli/cli/v2/pkg/cmd/attestation/api" "github.com/cli/cli/v2/pkg/cmd/attestation/io" + o "github.com/cli/cli/v2/pkg/option" "github.com/sigstore/sigstore-go/pkg/bundle" "github.com/sigstore/sigstore-go/pkg/root" @@ -34,6 +35,8 @@ type SigstoreConfig struct { NoPublicGood bool // If tenancy mode is not used, trust domain is empty TrustDomain string + // TUFMetadataDir + TUFMetadataDir o.Option[string] } type SigstoreVerifier interface { @@ -45,7 +48,8 @@ type LiveSigstoreVerifier struct { Logger *io.Handler NoPublicGood bool // If tenancy mode is not used, trust domain is empty - TrustDomain string + TrustDomain string + TUFMetadataDir o.Option[string] } var ErrNoAttestationsVerified = errors.New("no attestations were verified") @@ -55,10 +59,11 @@ var ErrNoAttestationsVerified = errors.New("no attestations were verified") // Public Good, GitHub, or a custom trusted root. func NewLiveSigstoreVerifier(config SigstoreConfig) *LiveSigstoreVerifier { return &LiveSigstoreVerifier{ - TrustedRoot: config.TrustedRoot, - Logger: config.Logger, - NoPublicGood: config.NoPublicGood, - TrustDomain: config.TrustDomain, + TrustedRoot: config.TrustedRoot, + Logger: config.Logger, + NoPublicGood: config.NoPublicGood, + TrustDomain: config.TrustDomain, + TUFMetadataDir: config.TUFMetadataDir, } } @@ -89,9 +94,9 @@ func (v *LiveSigstoreVerifier) chooseVerifier(issuer string) (*verify.SignedEnti if v.NoPublicGood { return nil, fmt.Errorf("detected public good instance but requested verification without public good instance") } - return newPublicGoodVerifier() + return newPublicGoodVerifier(v.TUFMetadataDir) case GitHubIssuerOrg: - return newGitHubVerifier(v.TrustDomain) + return newGitHubVerifier(v.TrustDomain, v.TUFMetadataDir) default: return nil, fmt.Errorf("leaf certificate issuer is not recognized") } @@ -255,10 +260,10 @@ func newCustomVerifier(trustedRoot *root.TrustedRoot) (*verify.SignedEntityVerif return gv, nil } -func newGitHubVerifier(trustDomain string) (*verify.SignedEntityVerifier, error) { +func newGitHubVerifier(trustDomain string, tufMetadataDir o.Option[string]) (*verify.SignedEntityVerifier, error) { var tr string - opts := GitHubTUFOptions() + opts := GitHubTUFOptions(tufMetadataDir) client, err := tuf.New(opts) if err != nil { return nil, fmt.Errorf("failed to create TUF client: %v", err) @@ -289,8 +294,8 @@ func newGitHubVerifierWithTrustedRoot(trustedRoot *root.TrustedRoot) (*verify.Si return gv, nil } -func newPublicGoodVerifier() (*verify.SignedEntityVerifier, error) { - opts := DefaultOptionsWithCacheSetting() +func newPublicGoodVerifier(tufMetadataDir o.Option[string]) (*verify.SignedEntityVerifier, error) { + opts := DefaultOptionsWithCacheSetting(tufMetadataDir) client, err := tuf.New(opts) if err != nil { return nil, fmt.Errorf("failed to create TUF client: %v", err) diff --git a/pkg/cmd/attestation/verification/sigstore_integration_test.go b/pkg/cmd/attestation/verification/sigstore_integration_test.go index e14b472b0..987fb9caa 100644 --- a/pkg/cmd/attestation/verification/sigstore_integration_test.go +++ b/pkg/cmd/attestation/verification/sigstore_integration_test.go @@ -9,6 +9,7 @@ import ( "github.com/cli/cli/v2/pkg/cmd/attestation/artifact" "github.com/cli/cli/v2/pkg/cmd/attestation/io" "github.com/cli/cli/v2/pkg/cmd/attestation/test" + o "github.com/cli/cli/v2/pkg/option" "github.com/sigstore/sigstore-go/pkg/verify" "github.com/stretchr/testify/require" @@ -48,25 +49,29 @@ func TestLiveSigstoreVerifier(t *testing.T) { } for _, tc := range testcases { - verifier := NewLiveSigstoreVerifier(SigstoreConfig{ - Logger: io.NewTestHandler(), + t.Run(tc.name, func(t *testing.T) { + verifier := NewLiveSigstoreVerifier(SigstoreConfig{ + Logger: io.NewTestHandler(), + TUFMetadataDir: o.Some(t.TempDir()), + }) + + results, err := verifier.Verify(tc.attestations, publicGoodPolicy(t)) + + if tc.expectErr { + require.Error(t, err) + require.ErrorContains(t, err, tc.errContains) + require.Nil(t, results) + } else { + require.NoError(t, err) + require.Equal(t, len(tc.attestations), len(results)) + } }) - - results, err := verifier.Verify(tc.attestations, publicGoodPolicy(t)) - - if tc.expectErr { - require.Error(t, err, "test case: %s", tc.name) - require.ErrorContains(t, err, tc.errContains, "test case: %s", tc.name) - require.Nil(t, results, "test case: %s", tc.name) - } else { - require.Equal(t, len(tc.attestations), len(results), "test case: %s", tc.name) - require.NoError(t, err, "test case: %s", tc.name) - } } t.Run("with 2/3 verified attestations", func(t *testing.T) { verifier := NewLiveSigstoreVerifier(SigstoreConfig{ - Logger: io.NewTestHandler(), + Logger: io.NewTestHandler(), + TUFMetadataDir: o.Some(t.TempDir()), }) invalidBundle := getAttestationsFor(t, "../test/data/sigstore-js-2.1.0-bundle-v0.1.json") @@ -82,7 +87,8 @@ func TestLiveSigstoreVerifier(t *testing.T) { t.Run("fail with 0/2 verified attestations", func(t *testing.T) { verifier := NewLiveSigstoreVerifier(SigstoreConfig{ - Logger: io.NewTestHandler(), + Logger: io.NewTestHandler(), + TUFMetadataDir: o.Some(t.TempDir()), }) invalidBundle := getAttestationsFor(t, "../test/data/sigstore-js-2.1.0-bundle-v0.1.json") @@ -105,7 +111,8 @@ func TestLiveSigstoreVerifier(t *testing.T) { attestations := getAttestationsFor(t, "../test/data/github_provenance_demo-0.0.12-py3-none-any-bundle.jsonl") verifier := NewLiveSigstoreVerifier(SigstoreConfig{ - Logger: io.NewTestHandler(), + Logger: io.NewTestHandler(), + TUFMetadataDir: o.Some(t.TempDir()), }) results, err := verifier.Verify(attestations, githubPolicy) @@ -117,8 +124,9 @@ func TestLiveSigstoreVerifier(t *testing.T) { attestations := getAttestationsFor(t, "../test/data/sigstore-js-2.1.0_with_2_bundles.jsonl") verifier := NewLiveSigstoreVerifier(SigstoreConfig{ - Logger: io.NewTestHandler(), - TrustedRoot: test.NormalizeRelativePath("../test/data/trusted_root.json"), + Logger: io.NewTestHandler(), + TrustedRoot: test.NormalizeRelativePath("../test/data/trusted_root.json"), + TUFMetadataDir: o.Some(t.TempDir()), }) results, err := verifier.Verify(attestations, publicGoodPolicy(t)) diff --git a/pkg/cmd/attestation/verification/tuf.go b/pkg/cmd/attestation/verification/tuf.go index f46a483d8..dcfdd0b32 100644 --- a/pkg/cmd/attestation/verification/tuf.go +++ b/pkg/cmd/attestation/verification/tuf.go @@ -5,6 +5,7 @@ import ( "os" "path/filepath" + o "github.com/cli/cli/v2/pkg/option" "github.com/cli/go-gh/v2/pkg/config" "github.com/sigstore/sigstore-go/pkg/tuf" ) @@ -14,7 +15,7 @@ var githubRoot []byte const GitHubTUFMirror = "https://tuf-repo.github.com" -func DefaultOptionsWithCacheSetting() *tuf.Options { +func DefaultOptionsWithCacheSetting(tufMetadataDir o.Option[string]) *tuf.Options { opts := tuf.DefaultOptions() // The CODESPACES environment variable will be set to true in a Codespaces workspace @@ -25,8 +26,8 @@ func DefaultOptionsWithCacheSetting() *tuf.Options { opts.DisableLocalCache = true } - // Set the cache path to a directory owned by the CLI - opts.CachePath = filepath.Join(config.CacheDir(), ".sigstore", "root") + // Set the cache path to the provided dir, or a directory owned by the CLI + opts.CachePath = tufMetadataDir.UnwrapOr(filepath.Join(config.CacheDir(), ".sigstore", "root")) // Allow TUF cache for 1 day opts.CacheValidity = 1 @@ -34,8 +35,8 @@ func DefaultOptionsWithCacheSetting() *tuf.Options { return opts } -func GitHubTUFOptions() *tuf.Options { - opts := DefaultOptionsWithCacheSetting() +func GitHubTUFOptions(tufMetadataDir o.Option[string]) *tuf.Options { + opts := DefaultOptionsWithCacheSetting(tufMetadataDir) opts.Root = githubRoot opts.RepositoryBaseURL = GitHubTUFMirror diff --git a/pkg/cmd/attestation/verification/tuf_test.go b/pkg/cmd/attestation/verification/tuf_test.go index 7d816bf82..e8b6ecf98 100644 --- a/pkg/cmd/attestation/verification/tuf_test.go +++ b/pkg/cmd/attestation/verification/tuf_test.go @@ -5,16 +5,22 @@ import ( "path/filepath" "testing" + o "github.com/cli/cli/v2/pkg/option" "github.com/cli/go-gh/v2/pkg/config" "github.com/stretchr/testify/require" ) -func TestGitHubTUFOptions(t *testing.T) { +func TestGitHubTUFOptionsNoMetadataDir(t *testing.T) { os.Setenv("CODESPACES", "true") - opts := GitHubTUFOptions() + opts := GitHubTUFOptions(o.None[string]()) require.Equal(t, GitHubTUFMirror, opts.RepositoryBaseURL) require.NotNil(t, opts.Root) require.True(t, opts.DisableLocalCache) require.Equal(t, filepath.Join(config.CacheDir(), ".sigstore", "root"), opts.CachePath) } + +func TestGitHubTUFOptionsWithMetadataDir(t *testing.T) { + opts := GitHubTUFOptions(o.Some("anything")) + require.Equal(t, "anything", opts.CachePath) +} diff --git a/pkg/cmd/attestation/verify/attestation_integration_test.go b/pkg/cmd/attestation/verify/attestation_integration_test.go index 630eb1dc5..9ff174141 100644 --- a/pkg/cmd/attestation/verify/attestation_integration_test.go +++ b/pkg/cmd/attestation/verify/attestation_integration_test.go @@ -10,6 +10,7 @@ import ( "github.com/cli/cli/v2/pkg/cmd/attestation/io" "github.com/cli/cli/v2/pkg/cmd/attestation/test" "github.com/cli/cli/v2/pkg/cmd/attestation/verification" + o "github.com/cli/cli/v2/pkg/option" "github.com/sigstore/sigstore-go/pkg/fulcio/certificate" "github.com/stretchr/testify/require" ) @@ -25,7 +26,8 @@ func getAttestationsFor(t *testing.T, bundlePath string) []*api.Attestation { func TestVerifyAttestations(t *testing.T) { sgVerifier := verification.NewLiveSigstoreVerifier(verification.SigstoreConfig{ - Logger: io.NewTestHandler(), + Logger: io.NewTestHandler(), + TUFMetadataDir: o.Some(t.TempDir()), }) certSummary := certificate.Summary{} diff --git a/pkg/cmd/attestation/verify/options.go b/pkg/cmd/attestation/verify/options.go index 4296cb8ec..0fbbec55a 100644 --- a/pkg/cmd/attestation/verify/options.go +++ b/pkg/cmd/attestation/verify/options.go @@ -31,8 +31,11 @@ type Options struct { Repo string SAN string SANRegex string + SignerDigest string SignerRepo string SignerWorkflow string + SourceDigest string + SourceRef string APIClient api.Client Logger *io.Handler OCIClient oci.Client diff --git a/pkg/cmd/attestation/verify/policy.go b/pkg/cmd/attestation/verify/policy.go index 1e1dcd4d8..1060a781e 100644 --- a/pkg/cmd/attestation/verify/policy.go +++ b/pkg/cmd/attestation/verify/policy.go @@ -66,7 +66,7 @@ func newEnforcementCriteria(opts *Options) (verification.EnforcementCriteria, er // then we default to the repo option c.SANRegex = expandToGitHubURLRegex(opts.Tenant, opts.Repo) } else { - // if opts.Repo was not provided, we fallback to the opts.Owner value + // if opts.Repo was not provided, we fall back to the opts.Owner value c.SANRegex = expandToGitHubURLRegex(opts.Tenant, owner) } @@ -98,6 +98,12 @@ func newEnforcementCriteria(opts *Options) (verification.EnforcementCriteria, er c.Certificate.Issuer = opts.OIDCIssuer } + // set the SourceRepositoryDigest, SourceRepositoryRef, and BuildSignerDigest + // extensions if the options are provided + c.Certificate.BuildSignerDigest = opts.SignerDigest + c.Certificate.SourceRepositoryDigest = opts.SourceDigest + c.Certificate.SourceRepositoryRef = opts.SourceRef + return c, nil } diff --git a/pkg/cmd/attestation/verify/policy_test.go b/pkg/cmd/attestation/verify/policy_test.go index d033ba4fa..d376498b6 100644 --- a/pkg/cmd/attestation/verify/policy_test.go +++ b/pkg/cmd/attestation/verify/policy_test.go @@ -216,6 +216,48 @@ func TestNewEnforcementCriteria(t *testing.T) { require.NoError(t, err) require.Equal(t, "https://foo.com", c.Certificate.Issuer) }) + + t.Run("sets Certificate.BuildSignerDigest using opts.SignerDigest", func(t *testing.T) { + opts := &Options{ + ArtifactPath: artifactPath, + Owner: "wrong", + Repo: "wrong/value", + SignerDigest: "foo", + Hostname: "github.com", + } + + c, err := newEnforcementCriteria(opts) + require.NoError(t, err) + require.Equal(t, "foo", c.Certificate.BuildSignerDigest) + }) + + t.Run("sets Certificate.SourceRepositoryDigest using opts.SourceDigest", func(t *testing.T) { + opts := &Options{ + ArtifactPath: artifactPath, + Owner: "wrong", + Repo: "wrong/value", + SourceDigest: "foo", + Hostname: "github.com", + } + + c, err := newEnforcementCriteria(opts) + require.NoError(t, err) + require.Equal(t, "foo", c.Certificate.SourceRepositoryDigest) + }) + + t.Run("sets Certificate.SourceRepositoryRef using opts.SourceRef", func(t *testing.T) { + opts := &Options{ + ArtifactPath: artifactPath, + Owner: "wrong", + Repo: "wrong/value", + SourceRef: "refs/heads/main", + Hostname: "github.com", + } + + c, err := newEnforcementCriteria(opts) + require.NoError(t, err) + require.Equal(t, "refs/heads/main", c.Certificate.SourceRepositoryRef) + }) } func TestValidateSignerWorkflow(t *testing.T) { diff --git a/pkg/cmd/attestation/verify/verify.go b/pkg/cmd/attestation/verify/verify.go index 0a8de8b45..65ae8ca3e 100644 --- a/pkg/cmd/attestation/verify/verify.go +++ b/pkg/cmd/attestation/verify/verify.go @@ -195,6 +195,9 @@ func NewVerifyCmd(f *cmdutil.Factory, runF func(*Options) error) *cobra.Command verifyCmd.MarkFlagsMutuallyExclusive("cert-identity", "cert-identity-regex", "signer-repo", "signer-workflow") verifyCmd.Flags().StringVarP(&opts.OIDCIssuer, "cert-oidc-issuer", "", verification.GitHubOIDCIssuer, "Issuer of the OIDC token") verifyCmd.Flags().StringVarP(&opts.Hostname, "hostname", "", "", "Configure host to use") + verifyCmd.Flags().StringVarP(&opts.SignerDigest, "signer-digest", "", "", "Digest associated with the signer workflow") + verifyCmd.Flags().StringVarP(&opts.SourceRef, "source-ref", "", "", "Ref associated with the source workflow") + verifyCmd.Flags().StringVarP(&opts.SourceDigest, "source-digest", "", "", "Digest associated with the source workflow") return verifyCmd } diff --git a/pkg/cmd/attestation/verify/verify_integration_test.go b/pkg/cmd/attestation/verify/verify_integration_test.go index f25055d22..09479995c 100644 --- a/pkg/cmd/attestation/verify/verify_integration_test.go +++ b/pkg/cmd/attestation/verify/verify_integration_test.go @@ -11,6 +11,7 @@ import ( "github.com/cli/cli/v2/pkg/cmd/attestation/test" "github.com/cli/cli/v2/pkg/cmd/attestation/verification" "github.com/cli/cli/v2/pkg/cmd/factory" + o "github.com/cli/cli/v2/pkg/option" "github.com/cli/go-gh/v2/pkg/auth" "github.com/stretchr/testify/require" ) @@ -19,7 +20,8 @@ func TestVerifyIntegration(t *testing.T) { logger := io.NewTestHandler() sigstoreConfig := verification.SigstoreConfig{ - Logger: logger, + Logger: logger, + TUFMetadataDir: o.Some(t.TempDir()), } cmdFactory := factory.New("test") @@ -130,7 +132,8 @@ func TestVerifyIntegrationCustomIssuer(t *testing.T) { logger := io.NewTestHandler() sigstoreConfig := verification.SigstoreConfig{ - Logger: logger, + Logger: logger, + TUFMetadataDir: o.Some(t.TempDir()), } cmdFactory := factory.New("test") @@ -200,7 +203,8 @@ func TestVerifyIntegrationReusableWorkflow(t *testing.T) { logger := io.NewTestHandler() sigstoreConfig := verification.SigstoreConfig{ - Logger: logger, + Logger: logger, + TUFMetadataDir: o.Some(t.TempDir()), } cmdFactory := factory.New("test") @@ -289,7 +293,8 @@ func TestVerifyIntegrationReusableWorkflowSignerWorkflow(t *testing.T) { logger := io.NewTestHandler() sigstoreConfig := verification.SigstoreConfig{ - Logger: logger, + Logger: logger, + TUFMetadataDir: o.Some(t.TempDir()), } cmdFactory := factory.New("test") diff --git a/pkg/cmd/auth/login/login.go b/pkg/cmd/auth/login/login.go index 347fcdb6a..40b2fb382 100644 --- a/pkg/cmd/auth/login/login.go +++ b/pkg/cmd/auth/login/login.go @@ -72,7 +72,7 @@ func NewCmdLogin(f *cmdutil.Factory, runF func(*LoginOptions) error) *cobra.Comm The minimum required scopes for the token are: %[1]srepo%[1]s, %[1]sread:org%[1]s, and %[1]sgist%[1]s. Take care when passing a fine-grained personal access token to %[1]s--with-token%[1]s as the inherent scoping to certain resources may cause confusing behaviour when interacting with other - resources. Favour setting %[1]sGH_TOKEN$%[1]s for fine-grained personal access token usage. + resources. Favour setting %[1]sGH_TOKEN%[1]s for fine-grained personal access token usage. Alternatively, gh will use the authentication token found in environment variables. This method is most suitable for "headless" use of gh such as in automation. See @@ -88,13 +88,14 @@ func NewCmdLogin(f *cmdutil.Factory, runF func(*LoginOptions) error) *cobra.Comm prompting to create and upload a new key if one is not found. This can be skipped with %[1]s--skip-ssh-key%[1]s flag. - For more information on OAuth scopes, . + For more information on OAuth scopes, see + . `, "`"), Example: heredoc.Doc(` # Start interactive setup $ gh auth login - # Authenticate against github.com by reading the token from a file + # Authenticate against by reading the token from a file $ gh auth login --with-token < mytoken.txt # Authenticate with specific host diff --git a/pkg/cmd/auth/logout/logout.go b/pkg/cmd/auth/logout/logout.go index ef978ebac..dd908a62d 100644 --- a/pkg/cmd/auth/logout/logout.go +++ b/pkg/cmd/auth/logout/logout.go @@ -39,7 +39,20 @@ func NewCmdLogout(f *cmdutil.Factory, runF func(*LogoutOptions) error) *cobra.Co for an account. The authentication configuration is only removed locally. - This command does not invalidate authentication tokens. + This command does not revoke authentication tokens. + + To revoke all authentication tokens generated by the GitHub CLI: + + 1. Visit + 2. Select the "GitHub CLI" application + 3. Select "Revoke Access" + 4. Select "I understand, revoke access" + + Note: this procedure will revoke all authentication tokens ever + generated by the GitHub CLI across all your devices. + + For more information about revoking OAuth application tokens, see: + `), Example: heredoc.Doc(` # Select what host and account to log out of via a prompt diff --git a/pkg/cmd/auth/refresh/refresh.go b/pkg/cmd/auth/refresh/refresh.go index 7a6a39ff3..c0050b8a3 100644 --- a/pkg/cmd/auth/refresh/refresh.go +++ b/pkg/cmd/auth/refresh/refresh.go @@ -74,20 +74,21 @@ func NewCmdRefresh(f *cmdutil.Factory, runF func(*RefreshOptions) error) *cobra. inactive account, you will have to use %[1]sgh auth switch%[1]s to that account first before using this command, and then switch back when you are done. - For more information on OAuth scopes, . + For more information on OAuth scopes, see + . `, "`"), Example: heredoc.Doc(` + # Open a browser to add write:org and read:public_key scopes $ gh auth refresh --scopes write:org,read:public_key - # => open a browser to add write:org and read:public_key scopes + # Open a browser to ensure your authentication credentials have the correct minimum scopes $ gh auth refresh - # => open a browser to ensure your authentication credentials have the correct minimum scopes + # Open a browser to idempotently remove the delete_repo scope $ gh auth refresh --remove-scopes delete_repo - # => open a browser to idempotently remove the delete_repo scope + # Open a browser to re-authenticate with the default minimum scopes $ gh auth refresh --reset-scopes - # => open a browser to re-authenticate with the default minimum scopes `), RunE: func(cmd *cobra.Command, args []string) error { opts.Interactive = opts.IO.CanPrompt() diff --git a/pkg/cmd/browse/browse.go b/pkg/cmd/browse/browse.go index f4e3d8fec..68c97526a 100644 --- a/pkg/cmd/browse/browse.go +++ b/pkg/cmd/browse/browse.go @@ -69,32 +69,32 @@ func NewCmdBrowse(f *cmdutil.Factory, runF func(*BrowseOptions) error) *cobra.Co - Repository home page - Repository settings `), - Use: "browse [ | | ]", + Use: "browse [ | | ]", Args: cobra.MaximumNArgs(1), Example: heredoc.Doc(` + # Open the home page of the current repository $ gh browse - #=> Open the home page of the current repository + # Open the script directory of the current repository $ gh browse script/ - #=> Open the script directory of the current repository + # Open issue or pull request 217 $ gh browse 217 - #=> Open issue or pull request 217 + # Open commit page $ gh browse 77507cd94ccafcf568f8560cfecde965fcfa63 - #=> Open commit page + # Open repository settings $ gh browse --settings - #=> Open repository settings + # Open main.go at line 312 $ gh browse main.go:312 - #=> Open main.go at line 312 + # Open main.go with the repository at head of bug-fix branch $ gh browse main.go --branch bug-fix - #=> Open main.go with the repository at head of bug-fix branch + # Open main.go with the repository at commit 775007cd $ gh browse main.go --commit=77507cd94ccafcf568f8560cfecde965fcfa63 - #=> Open main.go with the repository at commit 775007cd `), Annotations: map[string]string{ "help:arguments": heredoc.Doc(` diff --git a/pkg/cmd/cache/delete/delete.go b/pkg/cmd/cache/delete/delete.go index 65a9d696a..ab76368ad 100644 --- a/pkg/cmd/cache/delete/delete.go +++ b/pkg/cmd/cache/delete/delete.go @@ -22,8 +22,9 @@ type DeleteOptions struct { HttpClient func() (*http.Client, error) IO *iostreams.IOStreams - DeleteAll bool - Identifier string + DeleteAll bool + SucceedOnNoCaches bool + Identifier string } func NewCmdDelete(f *cmdutil.Factory, runF func(*DeleteOptions) error) *cobra.Command { @@ -33,7 +34,7 @@ func NewCmdDelete(f *cmdutil.Factory, runF func(*DeleteOptions) error) *cobra.Co } cmd := &cobra.Command{ - Use: "delete [| | --all]", + Use: "delete [ | | --all]", Short: "Delete GitHub Actions caches", Long: heredoc.Docf(` Delete GitHub Actions caches. @@ -50,8 +51,11 @@ func NewCmdDelete(f *cmdutil.Factory, runF func(*DeleteOptions) error) *cobra.Co # Delete a cache by id in a specific repo $ gh cache delete 1234 --repo cli/cli - # Delete all caches + # Delete all caches (exit code 1 on no caches) $ gh cache delete --all + + # Delete all caches (exit code 0 on no caches) + $ gh cache delete --all --succeed-on-no-caches `), Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { @@ -65,6 +69,10 @@ func NewCmdDelete(f *cmdutil.Factory, runF func(*DeleteOptions) error) *cobra.Co return err } + if !opts.DeleteAll && opts.SucceedOnNoCaches { + return cmdutil.FlagErrorf("--succeed-on-no-caches must be used in conjunction with --all") + } + if !opts.DeleteAll && len(args) == 0 { return cmdutil.FlagErrorf("must provide either cache id, cache key, or use --all") } @@ -82,6 +90,7 @@ func NewCmdDelete(f *cmdutil.Factory, runF func(*DeleteOptions) error) *cobra.Co } cmd.Flags().BoolVarP(&opts.DeleteAll, "all", "a", false, "Delete all caches") + cmd.Flags().BoolVar(&opts.SucceedOnNoCaches, "succeed-on-no-caches", false, "Return exit code 0 if no caches found. Must be used in conjunction with `--all`") return cmd } @@ -100,12 +109,21 @@ func deleteRun(opts *DeleteOptions) error { var toDelete []string if opts.DeleteAll { + opts.IO.StartProgressIndicator() caches, err := shared.GetCaches(client, repo, shared.GetCachesOptions{Limit: -1}) + opts.IO.StopProgressIndicator() if err != nil { return err } if len(caches.ActionsCaches) == 0 { - return fmt.Errorf("%s No caches to delete", opts.IO.ColorScheme().FailureIcon()) + if opts.SucceedOnNoCaches { + if opts.IO.IsStdoutTTY() { + fmt.Fprintf(opts.IO.Out, "%s No caches to delete\n", opts.IO.ColorScheme().SuccessIcon()) + } + return nil + } else { + return fmt.Errorf("%s No caches to delete", opts.IO.ColorScheme().FailureIcon()) + } } for _, cache := range caches.ActionsCaches { toDelete = append(toDelete, strconv.Itoa(cache.Id)) diff --git a/pkg/cmd/cache/delete/delete_test.go b/pkg/cmd/cache/delete/delete_test.go index 43fc7d213..8660d693b 100644 --- a/pkg/cmd/cache/delete/delete_test.go +++ b/pkg/cmd/cache/delete/delete_test.go @@ -43,6 +43,21 @@ func TestNewCmdDelete(t *testing.T) { cli: "--all", wants: DeleteOptions{DeleteAll: true}, }, + { + name: "delete all and succeed-on-no-caches flags", + cli: "--all --succeed-on-no-caches", + wants: DeleteOptions{DeleteAll: true, SucceedOnNoCaches: true}, + }, + { + name: "succeed-on-no-caches flag", + cli: "--succeed-on-no-caches", + wantsErr: "--succeed-on-no-caches must be used in conjunction with --all", + }, + { + name: "succeed-on-no-caches flag and id argument", + cli: "--succeed-on-no-caches 123", + wantsErr: "--succeed-on-no-caches must be used in conjunction with --all", + }, { name: "id argument and delete all flag", cli: "1 --all", @@ -72,6 +87,7 @@ func TestNewCmdDelete(t *testing.T) { } assert.NoError(t, err) assert.Equal(t, tt.wants.DeleteAll, gotOpts.DeleteAll) + assert.Equal(t, tt.wants.SucceedOnNoCaches, gotOpts.SucceedOnNoCaches) assert.Equal(t, tt.wants.Identifier, gotOpts.Identifier) }) } @@ -160,6 +176,19 @@ func TestDeleteRun(t *testing.T) { tty: true, wantStdout: "✓ Deleted 2 caches from OWNER/REPO\n", }, + { + name: "attempts to delete all caches but api errors", + opts: DeleteOptions{DeleteAll: true}, + stubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/caches"), + httpmock.StatusStringResponse(500, ""), + ) + }, + tty: true, + wantErr: true, + wantErrMsg: "HTTP 500 (https://api.github.com/repos/OWNER/REPO/actions/caches?per_page=100)", + }, { name: "displays delete error", opts: DeleteOptions{Identifier: "123"}, @@ -186,6 +215,54 @@ func TestDeleteRun(t *testing.T) { tty: true, wantStdout: "✓ Deleted 1 cache from OWNER/REPO\n", }, + { + name: "no caches to delete when deleting all", + opts: DeleteOptions{DeleteAll: true}, + stubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/caches"), + httpmock.JSONResponse(shared.CachePayload{ + ActionsCaches: []shared.Cache{}, + TotalCount: 0, + }), + ) + }, + tty: false, + wantErr: true, + wantErrMsg: "X No caches to delete", + }, + { + name: "no caches to delete when deleting all but succeed on no cache tty", + opts: DeleteOptions{DeleteAll: true, SucceedOnNoCaches: true}, + stubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/caches"), + httpmock.JSONResponse(shared.CachePayload{ + ActionsCaches: []shared.Cache{}, + TotalCount: 0, + }), + ) + }, + tty: true, + wantErr: false, + wantStdout: "✓ No caches to delete\n", + }, + { + name: "no caches to delete when deleting all but succeed on no cache non-tty", + opts: DeleteOptions{DeleteAll: true, SucceedOnNoCaches: true}, + stubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.REST("GET", "repos/OWNER/REPO/actions/caches"), + httpmock.JSONResponse(shared.CachePayload{ + ActionsCaches: []shared.Cache{}, + TotalCount: 0, + }), + ) + }, + tty: false, + wantErr: false, + wantStdout: "", + }, } for _, tt := range tests { diff --git a/pkg/cmd/cache/list/list.go b/pkg/cmd/cache/list/list.go index 902285df6..9f39876cb 100644 --- a/pkg/cmd/cache/list/list.go +++ b/pkg/cmd/cache/list/list.go @@ -41,23 +41,23 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman Use: "list", Short: "List GitHub Actions caches", Example: heredoc.Doc(` - # List caches for current repository - $ gh cache list + # List caches for current repository + $ gh cache list - # List caches for specific repository - $ gh cache list --repo cli/cli + # List caches for specific repository + $ gh cache list --repo cli/cli - # List caches sorted by least recently accessed - $ gh cache list --sort last_accessed_at --order asc + # List caches sorted by least recently accessed + $ gh cache list --sort last_accessed_at --order asc - # List caches that have keys matching a prefix (or that match exactly) - $ gh cache list --key key-prefix + # List caches that have keys matching a prefix (or that match exactly) + $ gh cache list --key key-prefix - # To list caches for a specific branch, replace with the actual branch name - $ gh cache list --ref refs/heads/ + # List caches for a specific branch, replace with the actual branch name + $ gh cache list --ref refs/heads/ - # To list caches for a specific pull request, replace with the actual pull request number - $ gh cache list --ref refs/pull//merge + # List caches for a specific pull request, replace with the actual pull request number + $ gh cache list --ref refs/pull//merge `), Aliases: []string{"ls"}, Args: cobra.NoArgs, diff --git a/pkg/cmd/codespace/create.go b/pkg/cmd/codespace/create.go index 67eb5461e..bd2eb4623 100644 --- a/pkg/cmd/codespace/create.go +++ b/pkg/cmd/codespace/create.go @@ -99,22 +99,22 @@ func newCreateCmd(app *App) *cobra.Command { }, } - createCmd.Flags().BoolVarP(&opts.useWeb, "web", "w", false, "create codespace from browser, cannot be used with --display-name, --idle-timeout, or --retention-period") + createCmd.Flags().BoolVarP(&opts.useWeb, "web", "w", false, "Create codespace from browser, cannot be used with --display-name, --idle-timeout, or --retention-period") - createCmd.Flags().StringVarP(&opts.repo, "repo", "R", "", "repository name with owner: user/repo") + createCmd.Flags().StringVarP(&opts.repo, "repo", "R", "", "Repository name with owner: user/repo") if err := addDeprecatedRepoShorthand(createCmd, &opts.repo); err != nil { fmt.Fprintf(app.io.ErrOut, "%v\n", err) } - createCmd.Flags().StringVarP(&opts.branch, "branch", "b", "", "repository branch") - createCmd.Flags().StringVarP(&opts.location, "location", "l", "", "location: {EastUs|SouthEastAsia|WestEurope|WestUs2} (determined automatically if not provided)") - createCmd.Flags().StringVarP(&opts.machine, "machine", "m", "", "hardware specifications for the VM") - createCmd.Flags().BoolVarP(&opts.permissionsOptOut, "default-permissions", "", false, "do not prompt to accept additional permissions requested by the codespace") - createCmd.Flags().BoolVarP(&opts.showStatus, "status", "s", false, "show status of post-create command and dotfiles") - createCmd.Flags().DurationVar(&opts.idleTimeout, "idle-timeout", 0, "allowed inactivity before codespace is stopped, e.g. \"10m\", \"1h\"") - createCmd.Flags().Var(&opts.retentionPeriod, "retention-period", "allowed time after shutting down before the codespace is automatically deleted (maximum 30 days), e.g. \"1h\", \"72h\"") - createCmd.Flags().StringVar(&opts.devContainerPath, "devcontainer-path", "", "path to the devcontainer.json file to use when creating codespace") - createCmd.Flags().StringVarP(&opts.displayName, "display-name", "d", "", fmt.Sprintf("display name for the codespace (%d characters or less)", displayNameMaxLength)) + createCmd.Flags().StringVarP(&opts.branch, "branch", "b", "", "Repository branch") + createCmd.Flags().StringVarP(&opts.location, "location", "l", "", "Location: {EastUs|SouthEastAsia|WestEurope|WestUs2} (determined automatically if not provided)") + createCmd.Flags().StringVarP(&opts.machine, "machine", "m", "", "Hardware specifications for the VM") + createCmd.Flags().BoolVarP(&opts.permissionsOptOut, "default-permissions", "", false, "Do not prompt to accept additional permissions requested by the codespace") + createCmd.Flags().BoolVarP(&opts.showStatus, "status", "s", false, "Show status of post-create command and dotfiles") + createCmd.Flags().DurationVar(&opts.idleTimeout, "idle-timeout", 0, "Allowed inactivity before codespace is stopped, e.g. \"10m\", \"1h\"") + createCmd.Flags().Var(&opts.retentionPeriod, "retention-period", "Allowed time after shutting down before the codespace is automatically deleted (maximum 30 days), e.g. \"1h\", \"72h\"") + createCmd.Flags().StringVar(&opts.devContainerPath, "devcontainer-path", "", "Path to the devcontainer.json file to use when creating codespace") + createCmd.Flags().StringVarP(&opts.displayName, "display-name", "d", "", fmt.Sprintf("Display name for the codespace (%d characters or less)", displayNameMaxLength)) return createCmd } diff --git a/pkg/cmd/codespace/edit.go b/pkg/cmd/codespace/edit.go index c84b271c4..3fff28855 100644 --- a/pkg/cmd/codespace/edit.go +++ b/pkg/cmd/codespace/edit.go @@ -34,7 +34,7 @@ func newEditCmd(app *App) *cobra.Command { opts.selector = AddCodespaceSelector(editCmd, app.apiClient) editCmd.Flags().StringVarP(&opts.displayName, "display-name", "d", "", "Set the display name") - editCmd.Flags().StringVar(&opts.displayName, "displayName", "", "display name") + editCmd.Flags().StringVar(&opts.displayName, "displayName", "", "Display name") if err := editCmd.Flags().MarkDeprecated("displayName", "use `--display-name` instead"); err != nil { fmt.Fprintf(app.io.ErrOut, "error marking flag as deprecated: %v\n", err) } diff --git a/pkg/cmd/codespace/ports.go b/pkg/cmd/codespace/ports.go index 0608f0732..cd6656cf4 100644 --- a/pkg/cmd/codespace/ports.go +++ b/pkg/cmd/codespace/ports.go @@ -10,6 +10,7 @@ import ( "strings" "time" + "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/internal/codespaces" "github.com/cli/cli/v2/internal/codespaces/api" "github.com/cli/cli/v2/internal/codespaces/portforwarder" @@ -217,10 +218,12 @@ func getDevContainer(ctx context.Context, apiClient apiClient, codespace *api.Co func newPortsVisibilityCmd(app *App, selector *CodespaceSelector) *cobra.Command { return &cobra.Command{ - Use: "visibility :{public|private|org}...", - Short: "Change the visibility of the forwarded port", - Example: "gh codespace ports visibility 80:org 3000:private 8000:public", - Args: cobra.MinimumNArgs(1), + Use: "visibility :{public|private|org}...", + Short: "Change the visibility of the forwarded port", + Example: heredoc.Doc(` + $ gh codespace ports visibility 80:org 3000:private 8000:public + `), + Args: cobra.MinimumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { return app.UpdatePortVisibility(cmd.Context(), selector, args) }, diff --git a/pkg/cmd/codespace/rebuild.go b/pkg/cmd/codespace/rebuild.go index 565406edc..a7a9b9ba0 100644 --- a/pkg/cmd/codespace/rebuild.go +++ b/pkg/cmd/codespace/rebuild.go @@ -23,7 +23,7 @@ func newRebuildCmd(app *App) *cobra.Command { Short: "Rebuild a codespace", Long: heredoc.Doc(` Rebuilding recreates your codespace. - + Your code and any current changes will be preserved. Your codespace will be rebuilt using your working directory's dev container. A full rebuild also removes cached Docker images. `), @@ -35,7 +35,7 @@ func newRebuildCmd(app *App) *cobra.Command { selector = AddCodespaceSelector(rebuildCmd, app.apiClient) - rebuildCmd.Flags().BoolVar(&fullRebuild, "full", false, "perform a full rebuild") + rebuildCmd.Flags().BoolVar(&fullRebuild, "full", false, "Perform a full rebuild") return rebuildCmd } diff --git a/pkg/cmd/codespace/ssh.go b/pkg/cmd/codespace/ssh.go index b79ec68c9..70e7ceb42 100644 --- a/pkg/cmd/codespace/ssh.go +++ b/pkg/cmd/codespace/ssh.go @@ -55,7 +55,7 @@ func newSSHCmd(app *App) *cobra.Command { Long: heredoc.Docf(` The %[1]sssh%[1]s command is used to SSH into a codespace. In its simplest form, you can run %[1]sgh cs ssh%[1]s, select a codespace interactively, and connect. - + The %[1]sssh%[1]s command will automatically create a public/private ssh key pair in the %[1]s~/.ssh%[1]s directory if you do not have an existing valid key pair. When selecting the key pair to use, the preferred order is: @@ -732,8 +732,8 @@ func newCpCmd(app *App) *cobra.Command { be evaluated on the remote machine, subject to expansion of tildes, braces, globs, environment variables, and backticks. For security, do not use this flag with arguments provided by untrusted users; see for discussion. - - By default, the %[1]scp%[1]s command will create a public/private ssh key pair to authenticate with + + By default, the %[1]scp%[1]s command will create a public/private ssh key pair to authenticate with the codespace inside the %[1]s~/.ssh directory%[1]s. `, "`"), Example: heredoc.Doc(` diff --git a/pkg/cmd/codespace/view.go b/pkg/cmd/codespace/view.go index e212f0820..0bcbc8b5b 100644 --- a/pkg/cmd/codespace/view.go +++ b/pkg/cmd/codespace/view.go @@ -29,16 +29,16 @@ func newViewCmd(app *App) *cobra.Command { Use: "view", Short: "View details about a codespace", Example: heredoc.Doc(` - # select a codespace from a list of all codespaces you own - $ gh cs view + # Select a codespace from a list of all codespaces you own + $ gh cs view - # view the details of a specific codespace + # View the details of a specific codespace $ gh cs view -c codespace-name-12345 - # view the list of all available fields for a codespace + # View the list of all available fields for a codespace $ gh cs view --json - # view specific fields for a codespace + # View specific fields for a codespace $ gh cs view --json displayName,machineDisplayName,state `), Args: noArgsConstraint, diff --git a/pkg/cmd/completion/completion.go b/pkg/cmd/completion/completion.go index d83dd31ef..b34bf7abf 100644 --- a/pkg/cmd/completion/completion.go +++ b/pkg/cmd/completion/completion.go @@ -33,7 +33,7 @@ func NewCmdCompletion(io *iostreams.IOStreams) *cobra.Command { After, add this to your %[1]s~/.bash_profile%[1]s: eval "$(gh completion -s bash)" - + ### zsh Generate a %[1]s_gh%[1]s completion script and put it somewhere in your %[1]s$fpath%[1]s: @@ -44,7 +44,7 @@ func NewCmdCompletion(io *iostreams.IOStreams) *cobra.Command { autoload -U compinit compinit -i - + Zsh version 5.7 or later is recommended. ### fish @@ -59,7 +59,7 @@ func NewCmdCompletion(io *iostreams.IOStreams) *cobra.Command { mkdir -Path (Split-Path -Parent $profile) -ErrorAction SilentlyContinue notepad $profile - + Add the line and save the file: Invoke-Expression -Command $(gh completion -s powershell | Out-String) diff --git a/pkg/cmd/config/get/get.go b/pkg/cmd/config/get/get.go index bec22a979..17892485c 100644 --- a/pkg/cmd/config/get/get.go +++ b/pkg/cmd/config/get/get.go @@ -29,7 +29,6 @@ func NewCmdConfigGet(f *cmdutil.Factory, runF func(*GetOptions) error) *cobra.Co Short: "Print the value of a given configuration key", Example: heredoc.Doc(` $ gh config get git_protocol - https `), Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { diff --git a/pkg/cmd/extension/command.go b/pkg/cmd/extension/command.go index 93c385374..9431c0a92 100644 --- a/pkg/cmd/extension/command.go +++ b/pkg/cmd/extension/command.go @@ -300,7 +300,7 @@ func NewCmdExtension(f *cmdutil.Factory) *cobra.Command { For GitHub repositories, the repository argument can be specified in %[1]sOWNER/REPO%[1]s format or as a full repository URL. - The URL format is useful when the repository is not hosted on github.com. + The URL format is useful when the repository is not hosted on . For remote repositories, the GitHub CLI first looks for the release artifacts assuming that it's a binary extension i.e. prebuilt binaries provided as part of the release. @@ -411,8 +411,8 @@ func NewCmdExtension(f *cmdutil.Factory) *cobra.Command { return nil }, } - cmd.Flags().BoolVar(&forceFlag, "force", false, "force upgrade extension, or ignore if latest already installed") - cmd.Flags().StringVar(&pinFlag, "pin", "", "pin extension to a release tag or commit ref") + cmd.Flags().BoolVar(&forceFlag, "force", false, "Force upgrade extension, or ignore if latest already installed") + cmd.Flags().StringVar(&pinFlag, "pin", "", "Pin extension to a release tag or commit ref") return cmd }(), func() *cobra.Command { @@ -526,7 +526,7 @@ func NewCmdExtension(f *cmdutil.Factory) *cobra.Command { return browse.ExtBrowse(opts) }, } - cmd.Flags().BoolVar(&debug, "debug", false, "log to /tmp/extBrowse-*") + cmd.Flags().BoolVar(&debug, "debug", false, "Log to /tmp/extBrowse-*") cmd.Flags().BoolVarP(&singleColumn, "single-column", "s", false, "Render TUI with only one column of text") return cmd }(), @@ -542,7 +542,7 @@ func NewCmdExtension(f *cmdutil.Factory) *cobra.Command { of the extension. `, "`"), Example: heredoc.Doc(` - # execute a label extension instead of the core gh label command + # Execute a label extension instead of the core gh label command $ gh extension exec label `), Args: cobra.MinimumNArgs(1), @@ -573,16 +573,16 @@ func NewCmdExtension(f *cmdutil.Factory) *cobra.Command { Short: "Create a new extension", Example: heredoc.Doc(` # Use interactively - gh extension create + $ gh extension create # Create a script-based extension - gh extension create foobar + $ gh extension create foobar # Create a Go extension - gh extension create --precompiled=go foobar + $ gh extension create --precompiled=go foobar # Create a non-Go precompiled extension - gh extension create --precompiled=other foobar + $ gh extension create --precompiled=other foobar `), Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { diff --git a/pkg/cmd/factory/remote_resolver.go b/pkg/cmd/factory/remote_resolver.go index b008ade7e..b5cad393e 100644 --- a/pkg/cmd/factory/remote_resolver.go +++ b/pkg/cmd/factory/remote_resolver.go @@ -21,25 +21,24 @@ type remoteResolver struct { readRemotes func() (git.RemoteSet, error) getConfig func() (gh.Config, error) urlTranslator context.Translator + cachedRemotes context.Remotes + remotesError error } func (rr *remoteResolver) Resolver() func() (context.Remotes, error) { - var cachedRemotes context.Remotes - var remotesError error - return func() (context.Remotes, error) { - if cachedRemotes != nil || remotesError != nil { - return cachedRemotes, remotesError + if rr.cachedRemotes != nil || rr.remotesError != nil { + return rr.cachedRemotes, rr.remotesError } gitRemotes, err := rr.readRemotes() if err != nil { - remotesError = err + rr.remotesError = err return nil, err } if len(gitRemotes) == 0 { - remotesError = errors.New("no git remotes found") - return nil, remotesError + rr.remotesError = errors.New("no git remotes found") + return nil, rr.remotesError } sshTranslate := rr.urlTranslator @@ -68,30 +67,31 @@ func (rr *remoteResolver) Resolver() func() (context.Remotes, error) { // Sort remotes sort.Sort(resolvedRemotes) - // Filter remotes by hosts - // Note that this is not caching correctly: https://github.com/cli/cli/issues/10103 - cachedRemotes := resolvedRemotes.FilterByHosts(hosts) + rr.cachedRemotes = resolvedRemotes.FilterByHosts(hosts) // Filter again by default host if one is set // For config file default host fallback to cachedRemotes if none match // For environment default host (GH_HOST) do not fallback to cachedRemotes if none match if src != "default" { - filteredRemotes := cachedRemotes.FilterByHosts([]string{defaultHost}) + filteredRemotes := rr.cachedRemotes.FilterByHosts([]string{defaultHost}) if isHostEnv(src) || len(filteredRemotes) > 0 { - cachedRemotes = filteredRemotes + rr.cachedRemotes = filteredRemotes } } - if len(cachedRemotes) == 0 { + if len(rr.cachedRemotes) == 0 { if isHostEnv(src) { - return nil, fmt.Errorf("none of the git remotes configured for this repository correspond to the %s environment variable. Try adding a matching remote or unsetting the variable.", src) + rr.remotesError = fmt.Errorf("none of the git remotes configured for this repository correspond to the %s environment variable. Try adding a matching remote or unsetting the variable", src) + return nil, rr.remotesError } else if cfg.Authentication().HasEnvToken() { - return nil, errors.New("set the GH_HOST environment variable to specify which GitHub host to use") + rr.remotesError = errors.New("set the GH_HOST environment variable to specify which GitHub host to use") + return nil, rr.remotesError } - return nil, errors.New("none of the git remotes configured for this repository point to a known GitHub host. To tell gh about a new GitHub host, please use `gh auth login`") + rr.remotesError = errors.New("none of the git remotes configured for this repository point to a known GitHub host. To tell gh about a new GitHub host, please use `gh auth login`") + return nil, rr.remotesError } - return cachedRemotes, nil + return rr.cachedRemotes, nil } } diff --git a/pkg/cmd/factory/remote_resolver_test.go b/pkg/cmd/factory/remote_resolver_test.go index 8d537826e..77d4a9d65 100644 --- a/pkg/cmd/factory/remote_resolver_test.go +++ b/pkg/cmd/factory/remote_resolver_test.go @@ -1,14 +1,17 @@ package factory import ( + "errors" "net/url" "testing" + "github.com/cli/cli/v2/context" "github.com/cli/cli/v2/git" "github.com/cli/cli/v2/internal/config" "github.com/cli/cli/v2/internal/gh" ghmock "github.com/cli/cli/v2/internal/gh/mock" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) type identityTranslator struct{} @@ -288,3 +291,95 @@ func Test_remoteResolver(t *testing.T) { }) } } + +func Test_remoteResolver_Caching(t *testing.T) { + t.Run("cache remotes", func(t *testing.T) { + var readRemotesCalled bool + + rr := &remoteResolver{ + readRemotes: func() (git.RemoteSet, error) { + if readRemotesCalled { + return git.RemoteSet{}, errors.New("readRemotes should only be called once") + } + + readRemotesCalled = true + return git.RemoteSet{ + git.NewRemote("origin", "https://github.com/owner/repo.git"), + }, nil + }, + getConfig: func() (gh.Config, error) { + cfg := &ghmock.ConfigMock{} + cfg.AuthenticationFunc = func() gh.AuthConfig { + authCfg := &config.AuthConfig{} + authCfg.SetHosts([]string{"github.com"}) + authCfg.SetDefaultHost("github.com", "default") + return authCfg + } + return cfg, nil + }, + urlTranslator: identityTranslator{}, + } + + resolver := rr.Resolver() + + expectedRemoteNames := []string{"origin"} + remotes, err := resolver() + require.NoError(t, err) + require.Equal(t, expectedRemoteNames, mapRemotesToNames(remotes)) + + require.Equal(t, readRemotesCalled, true) + + cachedRemotes, err := resolver() + require.NoError(t, err, "expected no error to be cached") + require.Equal(t, expectedRemoteNames, mapRemotesToNames(cachedRemotes), "expected the remotes to be cached") + }) + + t.Run("cache error", func(t *testing.T) { + var readRemotesCalled bool + + rr := &remoteResolver{ + readRemotes: func() (git.RemoteSet, error) { + if readRemotesCalled { + return git.RemoteSet{ + git.NewRemote("origin", "https://github.com/owner/repo.git"), + }, nil + } + + readRemotesCalled = true + return git.RemoteSet{}, errors.New("error to be cached") + }, + getConfig: func() (gh.Config, error) { + cfg := &ghmock.ConfigMock{} + cfg.AuthenticationFunc = func() gh.AuthConfig { + authCfg := &config.AuthConfig{} + authCfg.SetHosts([]string{"github.com"}) + authCfg.SetDefaultHost("github.com", "default") + return authCfg + } + return cfg, nil + }, + urlTranslator: identityTranslator{}, + } + + resolver := rr.Resolver() + + expectedErr := errors.New("error to be cached") + remotes, err := resolver() + require.Equal(t, expectedErr, err) + require.Empty(t, remotes, "should return no remotes") + + require.Equal(t, readRemotesCalled, true) + + cachedRemotes, err := resolver() + require.Equal(t, expectedErr, err, "expected the error to be cached") + require.Empty(t, cachedRemotes, "should return no remotes") + }) +} + +func mapRemotesToNames(remotes context.Remotes) []string { + names := make([]string, len(remotes)) + for i, r := range remotes { + names[i] = r.Name + } + return names +} diff --git a/pkg/cmd/gist/create/create.go b/pkg/cmd/gist/create/create.go index e212a08f3..4e950b77b 100644 --- a/pkg/cmd/gist/create/create.go +++ b/pkg/cmd/gist/create/create.go @@ -59,22 +59,22 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co By default, gists are secret; use %[1]s--public%[1]s to make publicly listed ones. `, "`"), Example: heredoc.Doc(` - # publish file 'hello.py' as a public gist + # Publish file 'hello.py' as a public gist $ gh gist create --public hello.py - # create a gist with a description + # Create a gist with a description $ gh gist create hello.py -d "my Hello-World program in Python" - # create a gist containing several files + # Create a gist containing several files $ gh gist create hello.py world.py cool.txt - # create a gist containing several files using patterns + # Create a gist containing several files using patterns $ gh gist create *.md *.txt artifact.* - # read from standard input to create a gist + # Read from standard input to create a gist $ gh gist create - - # create a gist from output piped from another command + # Create a gist from output piped from another command $ cat cool.txt | gh gist create `), Args: func(cmd *cobra.Command, args []string) error { diff --git a/pkg/cmd/gist/delete/delete.go b/pkg/cmd/gist/delete/delete.go index 8b8a47540..319f9265e 100644 --- a/pkg/cmd/gist/delete/delete.go +++ b/pkg/cmd/gist/delete/delete.go @@ -38,19 +38,19 @@ func NewCmdDelete(f *cmdutil.Factory, runF func(*DeleteOptions) error) *cobra.Co Use: "delete { | }", Short: "Delete a gist", Long: heredoc.Docf(` - Delete a GitHub gist. + Delete a GitHub gist. - To delete a gist interactively, use %[1]sgh gist delete%[1]s with no arguments. + To delete a gist interactively, use %[1]sgh gist delete%[1]s with no arguments. - To delete a gist non-interactively, supply the gist id or url. - `, "`"), + To delete a gist non-interactively, supply the gist id or url. + `, "`"), Example: heredoc.Doc(` - # delete a gist interactively - gh gist delete + # Delete a gist interactively + $ gh gist delete - # delete a gist non-interactively - gh gist delete 1234 - `), + # Delete a gist non-interactively + $ gh gist delete 1234 + `), Args: cobra.MaximumNArgs(1), RunE: func(c *cobra.Command, args []string) error { if !opts.IO.CanPrompt() && !opts.Confirmed { @@ -71,7 +71,7 @@ func NewCmdDelete(f *cmdutil.Factory, runF func(*DeleteOptions) error) *cobra.Co return deleteRun(&opts) }, } - cmd.Flags().BoolVar(&opts.Confirmed, "yes", false, "confirm deletion without prompting") + cmd.Flags().BoolVar(&opts.Confirmed, "yes", false, "Confirm deletion without prompting") return cmd } diff --git a/pkg/cmd/gist/list/list.go b/pkg/cmd/gist/list/list.go index 00a401be4..d46d4da02 100644 --- a/pkg/cmd/gist/list/list.go +++ b/pkg/cmd/gist/list/list.go @@ -61,10 +61,10 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman No highlights or other color is printed when output is redirected. `, "`"), Example: heredoc.Doc(` - # list all secret gists from your user account + # List all secret gists from your user account $ gh gist list --secret - # find all gists from your user account mentioning "octo" anywhere + # Find all gists from your user account mentioning "octo" anywhere $ gh gist list --filter octo --include-content `), Aliases: []string{"ls"}, diff --git a/pkg/cmd/gist/list/list_test.go b/pkg/cmd/gist/list/list_test.go index e6cb35d53..0acfae109 100644 --- a/pkg/cmd/gist/list/list_test.go +++ b/pkg/cmd/gist/list/list_test.go @@ -540,14 +540,14 @@ func Test_listRun(t *testing.T) { wantOut: heredoc.Doc(` 1234 main.txt octo match in the description - + 2345 octo.txt match in the file name 3456 main.txt match in the file text octo in the text - + `), }, { @@ -599,14 +599,14 @@ func Test_listRun(t *testing.T) { wantOut: heredoc.Docf(` %[1]s[0;34m1234%[1]s[0m %[1]s[0;32mmain.txt%[1]s[0m %[1]s[0;30;43mocto%[1]s[0m%[1]s[0;1;39m match in the description%[1]s[0m - + %[1]s[0;34m2345%[1]s[0m %[1]s[0;30;43mocto%[1]s[0m%[1]s[0;32m.txt%[1]s[0m %[1]s[0;1;39mmatch in the file name%[1]s[0m - + %[1]s[0;34m3456%[1]s[0m %[1]s[0;32mmain.txt%[1]s[0m %[1]s[0;1;39mmatch in the file text%[1]s[0m %[1]s[0;30;43mocto%[1]s[0m in the text - + `, "\x1b"), }, } diff --git a/pkg/cmd/gist/rename/rename.go b/pkg/cmd/gist/rename/rename.go index 630e67019..96f630c02 100644 --- a/pkg/cmd/gist/rename/rename.go +++ b/pkg/cmd/gist/rename/rename.go @@ -35,7 +35,7 @@ func NewCmdRename(f *cmdutil.Factory, runf func(*RenameOptions) error) *cobra.Co } cmd := &cobra.Command{ - Use: "rename { | } ", + Use: "rename { | } ", Short: "Rename a file in a gist", Long: heredoc.Doc(`Rename a file in the given gist ID / URL.`), Args: cobra.ExactArgs(3), diff --git a/pkg/cmd/issue/comment/comment.go b/pkg/cmd/issue/comment/comment.go index c91cf79c9..090b0748c 100644 --- a/pkg/cmd/issue/comment/comment.go +++ b/pkg/cmd/issue/comment/comment.go @@ -11,12 +11,13 @@ import ( func NewCmdComment(f *cmdutil.Factory, runF func(*prShared.CommentableOptions) error) *cobra.Command { opts := &prShared.CommentableOptions{ - IO: f.IOStreams, - HttpClient: f.HttpClient, - EditSurvey: prShared.CommentableEditSurvey(f.Config, f.IOStreams), - InteractiveEditSurvey: prShared.CommentableInteractiveEditSurvey(f.Config, f.IOStreams), - ConfirmSubmitSurvey: prShared.CommentableConfirmSubmitSurvey(f.Prompter), - OpenInBrowser: f.Browser.Browse, + IO: f.IOStreams, + HttpClient: f.HttpClient, + EditSurvey: prShared.CommentableEditSurvey(f.Config, f.IOStreams), + InteractiveEditSurvey: prShared.CommentableInteractiveEditSurvey(f.Config, f.IOStreams), + ConfirmSubmitSurvey: prShared.CommentableConfirmSubmitSurvey(f.Prompter), + ConfirmCreateIfNoneSurvey: prShared.CommentableInteractiveCreateIfNoneSurvey(f.Prompter), + OpenInBrowser: f.Browser.Browse, } var bodyFile string @@ -69,6 +70,7 @@ func NewCmdComment(f *cmdutil.Factory, runF func(*prShared.CommentableOptions) e cmd.Flags().BoolP("editor", "e", false, "Skip prompts and open the text editor to write the body in") cmd.Flags().BoolP("web", "w", false, "Open the web browser to write the comment") cmd.Flags().BoolVar(&opts.EditLast, "edit-last", false, "Edit the last comment of the same author") + cmd.Flags().BoolVar(&opts.CreateIfNone, "create-if-none", false, "Create a new comment if no comments are found. Can be used only with --edit-last") return cmd } diff --git a/pkg/cmd/issue/comment/comment_test.go b/pkg/cmd/issue/comment/comment_test.go index 668d758e5..461ae1065 100644 --- a/pkg/cmd/issue/comment/comment_test.go +++ b/pkg/cmd/issue/comment/comment_test.go @@ -109,6 +109,29 @@ func TestNewCmdComment(t *testing.T) { }, wantsErr: false, }, + { + name: "edit last flag", + input: "1 --edit-last", + output: shared.CommentableOptions{ + Interactive: true, + InputType: shared.InputTypeEditor, + Body: "", + EditLast: true, + }, + wantsErr: false, + }, + { + name: "edit last flag with create if none", + input: "1 --edit-last --create-if-none", + output: shared.CommentableOptions{ + Interactive: true, + InputType: shared.InputTypeEditor, + Body: "", + EditLast: true, + CreateIfNone: true, + }, + wantsErr: false, + }, { name: "body and body-file flags", input: "1 --body 'test' --body-file 'test-file.txt'", @@ -139,6 +162,12 @@ func TestNewCmdComment(t *testing.T) { output: shared.CommentableOptions{}, wantsErr: true, }, + { + name: "create-if-none flag without edit-last", + input: "1 --create-if-none", + output: shared.CommentableOptions{}, + wantsErr: true, + }, } for _, tt := range tests { @@ -188,11 +217,12 @@ func TestNewCmdComment(t *testing.T) { func Test_commentRun(t *testing.T) { tests := []struct { - name string - input *shared.CommentableOptions - httpStubs func(*testing.T, *httpmock.Registry) - stdout string - stderr string + name string + input *shared.CommentableOptions + emptyComments bool + httpStubs func(*testing.T, *httpmock.Registry) + stdout string + stderr string }{ { name: "interactive editor", @@ -225,6 +255,24 @@ func Test_commentRun(t *testing.T) { }, stdout: "https://github.com/OWNER/REPO/issues/123#issuecomment-111\n", }, + { + name: "interactive editor with edit last and create if none", + input: &shared.CommentableOptions{ + Interactive: true, + InputType: 0, + Body: "", + EditLast: true, + CreateIfNone: true, + + InteractiveEditSurvey: func(string) (string, error) { return "comment body", nil }, + ConfirmCreateIfNoneSurvey: func() (bool, error) { return true, nil }, + ConfirmSubmitSurvey: func() (bool, error) { return true, nil }, + }, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + mockCommentUpdate(t, reg) + }, + stdout: "https://github.com/OWNER/REPO/issues/123#issuecomment-111\n", + }, { name: "non-interactive web", input: &shared.CommentableOptions{ @@ -248,6 +296,39 @@ func Test_commentRun(t *testing.T) { }, stderr: "Opening https://github.com/OWNER/REPO/issues/123 in your browser.\n", }, + { + name: "non-interactive web with edit last and create if none for empty comments", + input: &shared.CommentableOptions{ + Interactive: false, + InputType: shared.InputTypeWeb, + Body: "", + EditLast: true, + CreateIfNone: true, + + OpenInBrowser: func(u string) error { + assert.Contains(t, u, "#issuecomment-new") + return nil + }, + }, + emptyComments: true, + stderr: "Opening https://github.com/OWNER/REPO/issues/123 in your browser.\n", + }, + { + name: "non-interactive web with edit last and create if none", + input: &shared.CommentableOptions{ + Interactive: false, + InputType: shared.InputTypeWeb, + Body: "", + EditLast: true, + CreateIfNone: true, + + OpenInBrowser: func(u string) error { + assert.Contains(t, u, "#issuecomment-111") + return nil + }, + }, + stderr: "Opening https://github.com/OWNER/REPO/issues/123 in your browser.\n", + }, { name: "non-interactive editor", input: &shared.CommentableOptions{ @@ -277,6 +358,23 @@ func Test_commentRun(t *testing.T) { }, stdout: "https://github.com/OWNER/REPO/issues/123#issuecomment-111\n", }, + { + name: "non-interactive editor with edit last and create if none", + input: &shared.CommentableOptions{ + Interactive: false, + InputType: shared.InputTypeEditor, + Body: "", + EditLast: true, + CreateIfNone: true, + + EditSurvey: func(string) (string, error) { return "comment body", nil }, + }, + emptyComments: true, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + mockCommentCreate(t, reg) + }, + stdout: "https://github.com/OWNER/REPO/issues/123#issuecomment-456\n", + }, { name: "non-interactive inline", input: &shared.CommentableOptions{ @@ -319,14 +417,21 @@ func Test_commentRun(t *testing.T) { tt.input.HttpClient = func() (*http.Client, error) { return &http.Client{Transport: reg}, nil } + + comments := api.Comments{Nodes: []api.Comment{ + {ID: "id1", Author: api.CommentAuthor{Login: "octocat"}, URL: "https://github.com/OWNER/REPO/issues/123#issuecomment-111", ViewerDidAuthor: true}, + {ID: "id2", Author: api.CommentAuthor{Login: "monalisa"}, URL: "https://github.com/OWNER/REPO/issues/123#issuecomment-222"}, + }} + + if tt.emptyComments { + comments.Nodes = []api.Comment{} + } + tt.input.RetrieveCommentable = func() (shared.Commentable, ghrepo.Interface, error) { return &api.Issue{ - ID: "ISSUE-ID", - URL: "https://github.com/OWNER/REPO/issues/123", - Comments: api.Comments{Nodes: []api.Comment{ - {ID: "id1", Author: api.CommentAuthor{Login: "octocat"}, URL: "https://github.com/OWNER/REPO/issues/123#issuecomment-111", ViewerDidAuthor: true}, - {ID: "id2", Author: api.CommentAuthor{Login: "monalisa"}, URL: "https://github.com/OWNER/REPO/issues/123#issuecomment-222"}, - }}, + ID: "ISSUE-ID", + URL: "https://github.com/OWNER/REPO/issues/123", + Comments: comments, }, ghrepo.New("OWNER", "REPO"), nil } diff --git a/pkg/cmd/issue/delete/delete.go b/pkg/cmd/issue/delete/delete.go index a70c6abb2..fb41f288e 100644 --- a/pkg/cmd/issue/delete/delete.go +++ b/pkg/cmd/issue/delete/delete.go @@ -57,9 +57,9 @@ func NewCmdDelete(f *cmdutil.Factory, runF func(*DeleteOptions) error) *cobra.Co }, } - cmd.Flags().BoolVar(&opts.Confirmed, "confirm", false, "confirm deletion without prompting") + cmd.Flags().BoolVar(&opts.Confirmed, "confirm", false, "Confirm deletion without prompting") _ = cmd.Flags().MarkDeprecated("confirm", "use `--yes` instead") - cmd.Flags().BoolVar(&opts.Confirmed, "yes", false, "confirm deletion without prompting") + cmd.Flags().BoolVar(&opts.Confirmed, "yes", false, "Confirm deletion without prompting") return cmd } diff --git a/pkg/cmd/label/clone.go b/pkg/cmd/label/clone.go index 81ab76958..a02c4764a 100644 --- a/pkg/cmd/label/clone.go +++ b/pkg/cmd/label/clone.go @@ -47,10 +47,10 @@ func newCmdClone(f *cmdutil.Factory, runF func(*cloneOptions) error) *cobra.Comm destination repository using the %[1]s--force%[1]s flag. `, "`"), Example: heredoc.Doc(` - # clone and overwrite labels from cli/cli repository into the current repository + # Clone and overwrite labels from cli/cli repository into the current repository $ gh label clone cli/cli --force - # clone labels from cli/cli repository into a octocat/cli repository + # Clone labels from cli/cli repository into a octocat/cli repository $ gh label clone cli/cli --repo octocat/cli `), Args: cmdutil.ExactArgs(1, "cannot clone labels: source-repository argument required"), diff --git a/pkg/cmd/label/create.go b/pkg/cmd/label/create.go index 8db57a415..9d6b2e9ee 100644 --- a/pkg/cmd/label/create.go +++ b/pkg/cmd/label/create.go @@ -66,7 +66,7 @@ func newCmdCreate(f *cmdutil.Factory, runF func(*createOptions) error) *cobra.Co The label color needs to be 6 character hex value. `, "`"), Example: heredoc.Doc(` - # create new bug label + # Create new bug label $ gh label create bug --description "Something isn't working" --color E99695 `), Args: cmdutil.ExactArgs(1, "cannot create label: name argument required"), diff --git a/pkg/cmd/label/edit.go b/pkg/cmd/label/edit.go index beae14045..79eb633f5 100644 --- a/pkg/cmd/label/edit.go +++ b/pkg/cmd/label/edit.go @@ -42,10 +42,10 @@ func newCmdEdit(f *cmdutil.Factory, runF func(*editOptions) error) *cobra.Comman The label color needs to be 6 character hex value. `, "`"), Example: heredoc.Doc(` - # update the color of the bug label + # Update the color of the bug label $ gh label edit bug --color FF0000 - # rename and edit the description of the bug label + # Rename and edit the description of the bug label $ gh label edit bug --name big-bug --description "Bigger than normal bug" `), Args: cmdutil.ExactArgs(1, "cannot update label: name argument required"), diff --git a/pkg/cmd/label/list.go b/pkg/cmd/label/list.go index 7b4c95ed1..fe1e9cdb7 100644 --- a/pkg/cmd/label/list.go +++ b/pkg/cmd/label/list.go @@ -42,10 +42,10 @@ func newCmdList(f *cmdutil.Factory, runF func(*listOptions) error) *cobra.Comman This behavior cannot be configured with the %[1]s--order%[1]s or %[1]s--sort%[1]s flags. `, "`"), Example: heredoc.Doc(` - # sort labels by name + # Sort labels by name $ gh label list --sort name - # find labels with "bug" in the name or description + # Find labels with "bug" in the name or description $ gh label list --search bug `), Args: cobra.NoArgs, diff --git a/pkg/cmd/label/list_test.go b/pkg/cmd/label/list_test.go index 6f066da32..8a1bc7c7a 100644 --- a/pkg/cmd/label/list_test.go +++ b/pkg/cmd/label/list_test.go @@ -218,7 +218,7 @@ func TestListRun(t *testing.T) { wantStdout: heredoc.Doc(` Showing 2 of 2 labels in OWNER/REPO - + NAME DESCRIPTION COLOR bug This is a bug label #d73a4a docs This is a docs label #ffa8da diff --git a/pkg/cmd/pr/checkout/checkout.go b/pkg/cmd/pr/checkout/checkout.go index c73d90339..f289a3919 100644 --- a/pkg/cmd/pr/checkout/checkout.go +++ b/pkg/cmd/pr/checkout/checkout.go @@ -172,7 +172,7 @@ func cmdsForExistingRemote(remote *cliContext.Remote, pr *api.PullRequest, opts refSpec += fmt.Sprintf(":refs/remotes/%s", remoteBranch) } - cmds = append(cmds, []string{"fetch", remote.Name, refSpec}) + cmds = append(cmds, []string{"fetch", remote.Name, refSpec, "--no-tags"}) localBranch := pr.HeadRefName if opts.BranchName != "" { @@ -202,7 +202,7 @@ func cmdsForMissingRemote(pr *api.PullRequest, baseURLOrName, repoHost, defaultB ref := fmt.Sprintf("refs/pull/%d/head", pr.Number) if opts.Detach { - cmds = append(cmds, []string{"fetch", baseURLOrName, ref}) + cmds = append(cmds, []string{"fetch", baseURLOrName, ref, "--no-tags"}) cmds = append(cmds, []string{"checkout", "--detach", "FETCH_HEAD"}) return cmds } @@ -218,7 +218,7 @@ func cmdsForMissingRemote(pr *api.PullRequest, baseURLOrName, repoHost, defaultB currentBranch, _ := opts.Branch() if localBranch == currentBranch { // PR head matches currently checked out branch - cmds = append(cmds, []string{"fetch", baseURLOrName, ref}) + cmds = append(cmds, []string{"fetch", baseURLOrName, ref, "--no-tags"}) if opts.Force { cmds = append(cmds, []string{"reset", "--hard", "FETCH_HEAD"}) } else { @@ -226,13 +226,12 @@ func cmdsForMissingRemote(pr *api.PullRequest, baseURLOrName, repoHost, defaultB cmds = append(cmds, []string{"merge", "--ff-only", "FETCH_HEAD"}) } } else { + // TODO: check if non-fast-forward and suggest to use `--force` + fetchCmd := []string{"fetch", baseURLOrName, fmt.Sprintf("%s:%s", ref, localBranch), "--no-tags"} if opts.Force { - cmds = append(cmds, []string{"fetch", baseURLOrName, fmt.Sprintf("%s:%s", ref, localBranch), "--force"}) - } else { - // TODO: check if non-fast-forward and suggest to use `--force` - cmds = append(cmds, []string{"fetch", baseURLOrName, fmt.Sprintf("%s:%s", ref, localBranch)}) + fetchCmd = append(fetchCmd, "--force") } - + cmds = append(cmds, fetchCmd) cmds = append(cmds, []string{"checkout", localBranch}) } diff --git a/pkg/cmd/pr/checkout/checkout_test.go b/pkg/cmd/pr/checkout/checkout_test.go index bd8bc3984..d4a8bbd5e 100644 --- a/pkg/cmd/pr/checkout/checkout_test.go +++ b/pkg/cmd/pr/checkout/checkout_test.go @@ -110,7 +110,7 @@ func Test_checkoutRun(t *testing.T) { }, runStubs: func(cs *run.CommandStubber) { cs.Register(`git show-ref --verify -- refs/heads/feature`, 1, "") - cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature`, 0, "") + cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "") cs.Register(`git checkout -b feature --track origin/feature`, 0, "") }, }, @@ -140,7 +140,7 @@ func Test_checkoutRun(t *testing.T) { "origin": "OWNER/REPO", }, runStubs: func(cs *run.CommandStubber) { - cs.Register(`git fetch origin refs/pull/123/head:feature`, 0, "") + cs.Register(`git fetch origin refs/pull/123/head:feature --no-tags`, 0, "") cs.Register(`git config branch\.feature\.merge`, 1, "") cs.Register(`git checkout feature`, 0, "") cs.Register(`git config branch\.feature\.remote origin`, 0, "") @@ -174,7 +174,7 @@ func Test_checkoutRun(t *testing.T) { }, runStubs: func(cs *run.CommandStubber) { cs.Register(`git show-ref --verify -- refs/heads/foobar`, 1, "") - cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature`, 0, "") + cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "") cs.Register(`git checkout -b foobar --track origin/feature`, 0, "") }, }, @@ -205,7 +205,7 @@ func Test_checkoutRun(t *testing.T) { }, runStubs: func(cs *run.CommandStubber) { cs.Register(`git config branch\.foobar\.merge`, 1, "") - cs.Register(`git fetch origin refs/pull/123/head:foobar`, 0, "") + cs.Register(`git fetch origin refs/pull/123/head:foobar --no-tags`, 0, "") cs.Register(`git checkout foobar`, 0, "") cs.Register(`git config branch\.foobar\.remote https://github.com/hubot/REPO.git`, 0, "") cs.Register(`git config branch\.foobar\.pushRemote https://github.com/hubot/REPO.git`, 0, "") @@ -260,7 +260,7 @@ func Test_checkoutRun(t *testing.T) { }, runStubs: func(cs *run.CommandStubber) { cs.Register(`git show-ref --verify -- refs/heads/feature`, 1, "") - cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature`, 0, "") + cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "") cs.Register(`git checkout -b feature --track origin/feature`, 0, "") }, remotes: map[string]string{ @@ -417,7 +417,7 @@ func TestPRCheckout_sameRepo(t *testing.T) { cs, cmdTeardown := run.Stub() defer cmdTeardown(t) - cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature`, 0, "") + cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "") cs.Register(`git show-ref --verify -- refs/heads/feature`, 1, "") cs.Register(`git checkout -b feature --track origin/feature`, 0, "") @@ -436,7 +436,7 @@ func TestPRCheckout_existingBranch(t *testing.T) { cs, cmdTeardown := run.Stub() defer cmdTeardown(t) - cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature`, 0, "") + cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "") cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "") cs.Register(`git checkout feature`, 0, "") cs.Register(`git merge --ff-only refs/remotes/origin/feature`, 0, "") @@ -468,7 +468,7 @@ func TestPRCheckout_differentRepo_remoteExists(t *testing.T) { cs, cmdTeardown := run.Stub() defer cmdTeardown(t) - cs.Register(`git fetch robot-fork \+refs/heads/feature:refs/remotes/robot-fork/feature`, 0, "") + cs.Register(`git fetch robot-fork \+refs/heads/feature:refs/remotes/robot-fork/feature --no-tags`, 0, "") cs.Register(`git show-ref --verify -- refs/heads/feature`, 1, "") cs.Register(`git checkout -b feature --track robot-fork/feature`, 0, "") @@ -488,7 +488,7 @@ func TestPRCheckout_differentRepo(t *testing.T) { cs, cmdTeardown := run.Stub() defer cmdTeardown(t) - cs.Register(`git fetch origin refs/pull/123/head:feature`, 0, "") + cs.Register(`git fetch origin refs/pull/123/head:feature --no-tags`, 0, "") cs.Register(`git config branch\.feature\.merge`, 1, "") cs.Register(`git checkout feature`, 0, "") cs.Register(`git config branch\.feature\.remote origin`, 0, "") @@ -501,6 +501,29 @@ func TestPRCheckout_differentRepo(t *testing.T) { assert.Equal(t, "", output.Stderr()) } +func TestPRCheckout_differentRepoForce(t *testing.T) { + http := &httpmock.Registry{} + defer http.Verify(t) + + baseRepo, pr := stubPR("OWNER/REPO:master", "hubot/REPO:feature") + finder := shared.RunCommandFinder("123", pr, baseRepo) + finder.ExpectFields([]string{"number", "headRefName", "headRepository", "headRepositoryOwner", "isCrossRepository", "maintainerCanModify"}) + + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + cs.Register(`git fetch origin refs/pull/123/head:feature --no-tags --force`, 0, "") + cs.Register(`git config branch\.feature\.merge`, 1, "") + cs.Register(`git checkout feature`, 0, "") + cs.Register(`git config branch\.feature\.remote origin`, 0, "") + cs.Register(`git config branch\.feature\.pushRemote origin`, 0, "") + cs.Register(`git config branch\.feature\.merge refs/pull/123/head`, 0, "") + + output, err := runCommand(http, nil, "master", `123 --force`, baseRepo) + assert.NoError(t, err) + assert.Equal(t, "", output.String()) + assert.Equal(t, "", output.Stderr()) +} + func TestPRCheckout_differentRepo_existingBranch(t *testing.T) { http := &httpmock.Registry{} defer http.Verify(t) @@ -510,7 +533,7 @@ func TestPRCheckout_differentRepo_existingBranch(t *testing.T) { cs, cmdTeardown := run.Stub() defer cmdTeardown(t) - cs.Register(`git fetch origin refs/pull/123/head:feature`, 0, "") + cs.Register(`git fetch origin refs/pull/123/head:feature --no-tags`, 0, "") cs.Register(`git config branch\.feature\.merge`, 0, "refs/heads/feature\n") cs.Register(`git checkout feature`, 0, "") @@ -529,7 +552,7 @@ func TestPRCheckout_detachedHead(t *testing.T) { cs, cmdTeardown := run.Stub() defer cmdTeardown(t) - cs.Register(`git fetch origin refs/pull/123/head:feature`, 0, "") + cs.Register(`git fetch origin refs/pull/123/head:feature --no-tags`, 0, "") cs.Register(`git config branch\.feature\.merge`, 0, "refs/heads/feature\n") cs.Register(`git checkout feature`, 0, "") @@ -548,7 +571,7 @@ func TestPRCheckout_differentRepo_currentBranch(t *testing.T) { cs, cmdTeardown := run.Stub() defer cmdTeardown(t) - cs.Register(`git fetch origin refs/pull/123/head`, 0, "") + cs.Register(`git fetch origin refs/pull/123/head --no-tags`, 0, "") cs.Register(`git config branch\.feature\.merge`, 0, "refs/heads/feature\n") cs.Register(`git merge --ff-only FETCH_HEAD`, 0, "") @@ -585,7 +608,7 @@ func TestPRCheckout_maintainerCanModify(t *testing.T) { cs, cmdTeardown := run.Stub() defer cmdTeardown(t) - cs.Register(`git fetch origin refs/pull/123/head:feature`, 0, "") + cs.Register(`git fetch origin refs/pull/123/head:feature --no-tags`, 0, "") cs.Register(`git config branch\.feature\.merge`, 1, "") cs.Register(`git checkout feature`, 0, "") cs.Register(`git config branch\.feature\.remote https://github\.com/hubot/REPO\.git`, 0, "") @@ -606,7 +629,7 @@ func TestPRCheckout_recurseSubmodules(t *testing.T) { cs, cmdTeardown := run.Stub() defer cmdTeardown(t) - cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature`, 0, "") + cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "") cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "") cs.Register(`git checkout feature`, 0, "") cs.Register(`git merge --ff-only refs/remotes/origin/feature`, 0, "") @@ -627,7 +650,7 @@ func TestPRCheckout_force(t *testing.T) { cs, cmdTeardown := run.Stub() defer cmdTeardown(t) - cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature`, 0, "") + cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "") cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "") cs.Register(`git checkout feature`, 0, "") cs.Register(`git reset --hard refs/remotes/origin/feature`, 0, "") @@ -649,7 +672,7 @@ func TestPRCheckout_detach(t *testing.T) { cs, cmdTeardown := run.Stub() defer cmdTeardown(t) cs.Register(`git checkout --detach FETCH_HEAD`, 0, "") - cs.Register(`git fetch origin refs/pull/123/head`, 0, "") + cs.Register(`git fetch origin refs/pull/123/head --no-tags`, 0, "") output, err := runCommand(http, nil, "", `123 --detach`, baseRepo) assert.NoError(t, err) diff --git a/pkg/cmd/pr/close/close.go b/pkg/cmd/pr/close/close.go index ec5132377..063501ef7 100644 --- a/pkg/cmd/pr/close/close.go +++ b/pkg/cmd/pr/close/close.go @@ -143,7 +143,7 @@ func closeRun(opts *CloseOptions) error { branchSwitchString = fmt.Sprintf(" and switched to branch %s", cs.Cyan(branchToSwitchTo)) } } else { - fmt.Fprintf(opts.IO.ErrOut, "%s Skipped deleting the local branch since current directory is not a git repository \n", cs.WarningIcon()) + fmt.Fprintf(opts.IO.ErrOut, "%s Skipped deleting the local branch since current directory is not a git repository\n", cs.WarningIcon()) } } diff --git a/pkg/cmd/pr/close/close_test.go b/pkg/cmd/pr/close/close_test.go index f957581f8..959af0e04 100644 --- a/pkg/cmd/pr/close/close_test.go +++ b/pkg/cmd/pr/close/close_test.go @@ -271,7 +271,7 @@ func TestPrClose_deleteBranch_notInGitRepo(t *testing.T) { assert.Equal(t, "", output.String()) assert.Equal(t, heredoc.Doc(` ✓ Closed pull request OWNER/REPO#96 (The title of the PR) - ! Skipped deleting the local branch since current directory is not a git repository + ! Skipped deleting the local branch since current directory is not a git repository ✓ Deleted branch trunk `), output.Stderr()) } diff --git a/pkg/cmd/pr/comment/comment.go b/pkg/cmd/pr/comment/comment.go index 31a2bc25c..a2ab4bf9e 100644 --- a/pkg/cmd/pr/comment/comment.go +++ b/pkg/cmd/pr/comment/comment.go @@ -10,12 +10,13 @@ import ( func NewCmdComment(f *cmdutil.Factory, runF func(*shared.CommentableOptions) error) *cobra.Command { opts := &shared.CommentableOptions{ - IO: f.IOStreams, - HttpClient: f.HttpClient, - EditSurvey: shared.CommentableEditSurvey(f.Config, f.IOStreams), - InteractiveEditSurvey: shared.CommentableInteractiveEditSurvey(f.Config, f.IOStreams), - ConfirmSubmitSurvey: shared.CommentableConfirmSubmitSurvey(f.Prompter), - OpenInBrowser: f.Browser.Browse, + IO: f.IOStreams, + HttpClient: f.HttpClient, + EditSurvey: shared.CommentableEditSurvey(f.Config, f.IOStreams), + InteractiveEditSurvey: shared.CommentableInteractiveEditSurvey(f.Config, f.IOStreams), + ConfirmSubmitSurvey: shared.CommentableConfirmSubmitSurvey(f.Prompter), + ConfirmCreateIfNoneSurvey: shared.CommentableInteractiveCreateIfNoneSurvey(f.Prompter), + OpenInBrowser: f.Browser.Browse, } var bodyFile string @@ -75,6 +76,7 @@ func NewCmdComment(f *cmdutil.Factory, runF func(*shared.CommentableOptions) err cmd.Flags().BoolP("editor", "e", false, "Skip prompts and open the text editor to write the body in") cmd.Flags().BoolP("web", "w", false, "Open the web browser to write the comment") cmd.Flags().BoolVar(&opts.EditLast, "edit-last", false, "Edit the last comment of the same author") + cmd.Flags().BoolVar(&opts.CreateIfNone, "create-if-none", false, "Create a new comment if no comments are found. Can be used only with --edit-last") return cmd } diff --git a/pkg/cmd/pr/comment/comment_test.go b/pkg/cmd/pr/comment/comment_test.go index 56cf58d21..a6cc36abf 100644 --- a/pkg/cmd/pr/comment/comment_test.go +++ b/pkg/cmd/pr/comment/comment_test.go @@ -129,6 +129,29 @@ func TestNewCmdComment(t *testing.T) { }, wantsErr: false, }, + { + name: "edit last flag", + input: "1 --edit-last", + output: shared.CommentableOptions{ + Interactive: true, + InputType: shared.InputTypeEditor, + Body: "", + EditLast: true, + }, + wantsErr: false, + }, + { + name: "edit last flag with create if none", + input: "1 --edit-last --create-if-none", + output: shared.CommentableOptions{ + Interactive: true, + InputType: shared.InputTypeEditor, + Body: "", + EditLast: true, + CreateIfNone: true, + }, + wantsErr: false, + }, { name: "body and body-file flags", input: "1 --body 'test' --body-file 'test-file.txt'", @@ -159,6 +182,12 @@ func TestNewCmdComment(t *testing.T) { output: shared.CommentableOptions{}, wantsErr: true, }, + { + name: "create-if-none flag without edit-last", + input: "1 --create-if-none", + output: shared.CommentableOptions{}, + wantsErr: true, + }, } for _, tt := range tests { @@ -208,11 +237,12 @@ func TestNewCmdComment(t *testing.T) { func Test_commentRun(t *testing.T) { tests := []struct { - name string - input *shared.CommentableOptions - httpStubs func(*testing.T, *httpmock.Registry) - stdout string - stderr string + name string + input *shared.CommentableOptions + emptyComments bool + httpStubs func(*testing.T, *httpmock.Registry) + stdout string + stderr string }{ { name: "interactive editor", @@ -245,6 +275,24 @@ func Test_commentRun(t *testing.T) { }, stdout: "https://github.com/OWNER/REPO/pull/123#issuecomment-111\n", }, + { + name: "interactive editor with edit last and create if none", + input: &shared.CommentableOptions{ + Interactive: true, + InputType: 0, + Body: "", + EditLast: true, + CreateIfNone: true, + + InteractiveEditSurvey: func(string) (string, error) { return "comment body", nil }, + ConfirmCreateIfNoneSurvey: func() (bool, error) { return true, nil }, + ConfirmSubmitSurvey: func() (bool, error) { return true, nil }, + }, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + mockCommentUpdate(t, reg) + }, + stdout: "https://github.com/OWNER/REPO/pull/123#issuecomment-111\n", + }, { name: "non-interactive web", input: &shared.CommentableOptions{ @@ -264,7 +312,43 @@ func Test_commentRun(t *testing.T) { Body: "", EditLast: true, - OpenInBrowser: func(string) error { return nil }, + OpenInBrowser: func(u string) error { + assert.Contains(t, u, "#issuecomment-111") + return nil + }, + }, + stderr: "Opening https://github.com/OWNER/REPO/pull/123 in your browser.\n", + }, + { + name: "non-interactive web with edit last and create if none for empty comments", + input: &shared.CommentableOptions{ + Interactive: false, + InputType: shared.InputTypeWeb, + Body: "", + EditLast: true, + CreateIfNone: true, + + OpenInBrowser: func(u string) error { + assert.Contains(t, u, "#issuecomment-new") + return nil + }, + }, + emptyComments: true, + stderr: "Opening https://github.com/OWNER/REPO/pull/123 in your browser.\n", + }, + { + name: "non-interactive web with edit last and create if none", + input: &shared.CommentableOptions{ + Interactive: false, + InputType: shared.InputTypeWeb, + Body: "", + EditLast: true, + CreateIfNone: true, + + OpenInBrowser: func(u string) error { + assert.Contains(t, u, "#issuecomment-111") + return nil + }, }, stderr: "Opening https://github.com/OWNER/REPO/pull/123 in your browser.\n", }, @@ -297,6 +381,23 @@ func Test_commentRun(t *testing.T) { }, stdout: "https://github.com/OWNER/REPO/pull/123#issuecomment-111\n", }, + { + name: "non-interactive editor with edit last and create if none", + input: &shared.CommentableOptions{ + Interactive: false, + InputType: shared.InputTypeEditor, + Body: "", + EditLast: true, + CreateIfNone: true, + + EditSurvey: func(string) (string, error) { return "comment body", nil }, + }, + emptyComments: true, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + mockCommentCreate(t, reg) + }, + stdout: "https://github.com/OWNER/REPO/pull/123#issuecomment-456\n", + }, { name: "non-interactive inline", input: &shared.CommentableOptions{ @@ -339,14 +440,20 @@ func Test_commentRun(t *testing.T) { tt.input.IO = ios tt.input.HttpClient = httpClient + + comments := api.Comments{Nodes: []api.Comment{ + {ID: "id1", Author: api.CommentAuthor{Login: "octocat"}, URL: "https://github.com/OWNER/REPO/pull/123#issuecomment-111", ViewerDidAuthor: true}, + {ID: "id2", Author: api.CommentAuthor{Login: "monalisa"}, URL: "https://github.com/OWNER/REPO/pull/123#issuecomment-222"}, + }} + if tt.emptyComments { + comments.Nodes = []api.Comment{} + } + tt.input.RetrieveCommentable = func() (shared.Commentable, ghrepo.Interface, error) { return &api.PullRequest{ - Number: 123, - URL: "https://github.com/OWNER/REPO/pull/123", - Comments: api.Comments{Nodes: []api.Comment{ - {ID: "id1", Author: api.CommentAuthor{Login: "octocat"}, URL: "https://github.com/OWNER/REPO/pull/123#issuecomment-111", ViewerDidAuthor: true}, - {ID: "id2", Author: api.CommentAuthor{Login: "monalisa"}, URL: "https://github.com/OWNER/REPO/pull/123#issuecomment-222"}, - }}, + Number: 123, + URL: "https://github.com/OWNER/REPO/pull/123", + Comments: comments, }, ghrepo.New("OWNER", "REPO"), nil } diff --git a/pkg/cmd/pr/create/create.go b/pkg/cmd/pr/create/create.go index b2abe0938..0066bbc6e 100644 --- a/pkg/cmd/pr/create/create.go +++ b/pkg/cmd/pr/create/create.go @@ -935,6 +935,7 @@ func handlePush(opts CreateOptions, ctx CreateContext) error { headRepo := ctx.HeadRepo headRemote := ctx.HeadRemote client := ctx.Client + gitClient := ctx.GitClient var err error // if a head repository could not be determined so far, automatically create @@ -976,7 +977,8 @@ func handlePush(opts CreateOptions, ctx CreateContext) error { headRepoURL := ghrepo.FormatRemoteURL(headRepo, cloneProtocol) gitClient := ctx.GitClient origin, _ := remotes.FindByName("origin") - upstream, _ := remotes.FindByName("upstream") + upstreamName := "upstream" + upstream, _ := remotes.FindByName(upstreamName) remoteName := "origin" if origin != nil { @@ -984,7 +986,7 @@ func handlePush(opts CreateOptions, ctx CreateContext) error { } if origin != nil && upstream == nil && ghrepo.IsSame(origin, ctx.BaseRepo) { - renameCmd, err := gitClient.Command(context.Background(), "remote", "rename", "origin", "upstream") + renameCmd, err := gitClient.Command(context.Background(), "remote", "rename", "origin", upstreamName) if err != nil { return err } @@ -992,7 +994,7 @@ func handlePush(opts CreateOptions, ctx CreateContext) error { return fmt.Errorf("error renaming origin remote: %w", err) } remoteName = "origin" - fmt.Fprintf(opts.IO.ErrOut, "Changed %s remote to %q\n", ghrepo.FullName(ctx.BaseRepo), "upstream") + fmt.Fprintf(opts.IO.ErrOut, "Changed %s remote to %q\n", ghrepo.FullName(ctx.BaseRepo), upstreamName) } gitRemote, err := gitClient.AddRemote(context.Background(), remoteName, headRepoURL, []string{}) @@ -1002,6 +1004,19 @@ func handlePush(opts CreateOptions, ctx CreateContext) error { fmt.Fprintf(opts.IO.ErrOut, "Added %s as remote %q\n", ghrepo.FullName(headRepo), remoteName) + // Only mark `upstream` remote as default if `gh pr create` created the remote. + if didForkRepo { + err := gitClient.SetRemoteResolution(context.Background(), upstreamName, "base") + if err != nil { + return fmt.Errorf("error setting upstream as default: %w", err) + } + + if opts.IO.IsStdoutTTY() { + cs := opts.IO.ColorScheme() + fmt.Fprintf(opts.IO.ErrOut, "%s Repository %s set as the default repository. To learn more about the default repository, run: gh repo set-default --help\n", cs.WarningIcon(), cs.Bold(ghrepo.FullName(headRepo))) + } + } + headRemote = &ghContext.Remote{ Remote: gitRemote, Repo: headRepo, @@ -1013,7 +1028,6 @@ func handlePush(opts CreateOptions, ctx CreateContext) error { pushBranch := func() error { w := NewRegexpWriter(opts.IO.ErrOut, gitPushRegexp, "") defer w.Flush() - gitClient := ctx.GitClient ref := fmt.Sprintf("HEAD:refs/heads/%s", ctx.HeadBranch) bo := backoff.NewConstantBackOff(2 * time.Second) ctx := context.Background() diff --git a/pkg/cmd/pr/create/create_test.go b/pkg/cmd/pr/create/create_test.go index 6df3f9880..4d7c294db 100644 --- a/pkg/cmd/pr/create/create_test.go +++ b/pkg/cmd/pr/create/create_test.go @@ -798,6 +798,7 @@ func Test_createRun(t *testing.T) { cs.Register("git remote rename origin upstream", 0, "") cs.Register(`git remote add origin https://github.com/monalisa/REPO.git`, 0, "") cs.Register(`git push --set-upstream origin HEAD:refs/heads/feature`, 0, "") + cs.Register(`git config --add remote.upstream.gh-resolved base`, 0, "") }, promptStubs: func(pm *prompter.PrompterMock) { pm.SelectFunc = func(p, _ string, opts []string) (int, error) { @@ -809,7 +810,7 @@ func Test_createRun(t *testing.T) { } }, expectedOut: "https://github.com/OWNER/REPO/pull/12\n", - expectedErrOut: "\nCreating pull request for monalisa:feature into master in OWNER/REPO\n\nChanged OWNER/REPO remote to \"upstream\"\nAdded monalisa/REPO as remote \"origin\"\n", + expectedErrOut: "\nCreating pull request for monalisa:feature into master in OWNER/REPO\n\nChanged OWNER/REPO remote to \"upstream\"\nAdded monalisa/REPO as remote \"origin\"\n! Repository monalisa/REPO set as the default repository. To learn more about the default repository, run: gh repo set-default --help\n", }, { name: "pushed to non base repo", diff --git a/pkg/cmd/pr/list/list.go b/pkg/cmd/pr/list/list.go index 61be95998..ee708f161 100644 --- a/pkg/cmd/pr/list/list.go +++ b/pkg/cmd/pr/list/list.go @@ -61,16 +61,16 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman `), Example: heredoc.Doc(` - List PRs authored by you + # List PRs authored by you $ gh pr list --author "@me" - List only PRs with all of the given labels + # List only PRs with all of the given labels $ gh pr list --label bug --label "priority 1" - Filter PRs using search syntax + # Filter PRs using search syntax $ gh pr list --search "status:success review:required" - Find a PR that introduced a given commit + # Find a PR that introduced a given commit $ gh pr list --search "" --state merged `), Aliases: []string{"ls"}, diff --git a/pkg/cmd/pr/review/review.go b/pkg/cmd/pr/review/review.go index 4fa973a87..25f81d973 100644 --- a/pkg/cmd/pr/review/review.go +++ b/pkg/cmd/pr/review/review.go @@ -56,16 +56,16 @@ func NewCmdReview(f *cmdutil.Factory, runF func(*ReviewOptions) error) *cobra.Co Without an argument, the pull request that belongs to the current branch is reviewed. `), Example: heredoc.Doc(` - # approve the pull request of the current branch + # Approve the pull request of the current branch $ gh pr review --approve - # leave a review comment for the current branch + # Leave a review comment for the current branch $ gh pr review --comment -b "interesting" - # add a review for a specific pull request + # Add a review for a specific pull request $ gh pr review 123 - # request changes on a specific pull request + # Request changes on a specific pull request $ gh pr review 123 -r -b "needs more ASCII art" `), Args: cobra.MaximumNArgs(1), diff --git a/pkg/cmd/pr/shared/commentable.go b/pkg/cmd/pr/shared/commentable.go index 7a38286d2..46e3072e2 100644 --- a/pkg/cmd/pr/shared/commentable.go +++ b/pkg/cmd/pr/shared/commentable.go @@ -17,6 +17,8 @@ import ( "github.com/spf13/cobra" ) +var errNoUserComments = errors.New("no comments found for current user") + type InputType int const ( @@ -32,19 +34,21 @@ type Commentable interface { } type CommentableOptions struct { - IO *iostreams.IOStreams - HttpClient func() (*http.Client, error) - RetrieveCommentable func() (Commentable, ghrepo.Interface, error) - EditSurvey func(string) (string, error) - InteractiveEditSurvey func(string) (string, error) - ConfirmSubmitSurvey func() (bool, error) - OpenInBrowser func(string) error - Interactive bool - InputType InputType - Body string - EditLast bool - Quiet bool - Host string + IO *iostreams.IOStreams + HttpClient func() (*http.Client, error) + RetrieveCommentable func() (Commentable, ghrepo.Interface, error) + EditSurvey func(string) (string, error) + InteractiveEditSurvey func(string) (string, error) + ConfirmSubmitSurvey func() (bool, error) + ConfirmCreateIfNoneSurvey func() (bool, error) + OpenInBrowser func(string) error + Interactive bool + InputType InputType + Body string + EditLast bool + CreateIfNone bool + Quiet bool + Host string } func CommentablePreRun(cmd *cobra.Command, opts *CommentableOptions) error { @@ -66,6 +70,10 @@ func CommentablePreRun(cmd *cobra.Command, opts *CommentableOptions) error { inputFlags++ } + if opts.CreateIfNone && !opts.EditLast { + return cmdutil.FlagErrorf("`--create-if-none` can only be used with `--edit-last`") + } + if inputFlags == 0 { if !opts.IO.CanPrompt() { return cmdutil.FlagErrorf("flags required when not running interactively") @@ -85,7 +93,24 @@ func CommentableRun(opts *CommentableOptions) error { } opts.Host = repo.RepoHost() if opts.EditLast { - return updateComment(commentable, opts) + err := updateComment(commentable, opts) + if !errors.Is(err, errNoUserComments) { + return err + } + + if opts.Interactive { + if opts.CreateIfNone { + fmt.Fprintln(opts.IO.ErrOut, "No comments found. Creating a new comment.") + } else { + ok, err := opts.ConfirmCreateIfNoneSurvey() + if err != nil { + return err + } + if !ok { + return errNoUserComments + } + } + } } return createComment(commentable, opts) } @@ -144,7 +169,7 @@ func createComment(commentable Commentable, opts *CommentableOptions) error { func updateComment(commentable Commentable, opts *CommentableOptions) error { comments := commentable.CurrentUserComments() if len(comments) == 0 { - return fmt.Errorf("no comments found for current user") + return errNoUserComments } lastComment := &comments[len(comments)-1] @@ -219,6 +244,12 @@ func CommentableInteractiveEditSurvey(cf func() (gh.Config, error), io *iostream } } +func CommentableInteractiveCreateIfNoneSurvey(p Prompt) func() (bool, error) { + return func() (bool, error) { + return p.Confirm("No comments found. Create one?", true) + } +} + func CommentableEditSurvey(cf func() (gh.Config, error), io *iostreams.IOStreams) func(string) (string, error) { return func(initialValue string) (string, error) { editorCommand, err := cmdutil.DetermineEditor(cf) diff --git a/pkg/cmd/project/close/close.go b/pkg/cmd/project/close/close.go index d24278a23..3e85596c0 100644 --- a/pkg/cmd/project/close/close.go +++ b/pkg/cmd/project/close/close.go @@ -40,11 +40,11 @@ func NewCmdClose(f *cmdutil.Factory, runF func(config closeConfig) error) *cobra Short: "Close a project", Use: "close []", Example: heredoc.Doc(` - # close project "1" owned by monalisa - gh project close 1 --owner monalisa + # Close project "1" owned by monalisa + $ gh project close 1 --owner monalisa - # reopen closed project "1" owned by github - gh project close 1 --owner github --undo + # Reopen closed project "1" owned by github + $ gh project close 1 --owner github --undo `), Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { diff --git a/pkg/cmd/project/copy/copy.go b/pkg/cmd/project/copy/copy.go index 63047f0a3..c0f255f96 100644 --- a/pkg/cmd/project/copy/copy.go +++ b/pkg/cmd/project/copy/copy.go @@ -42,8 +42,8 @@ func NewCmdCopy(f *cmdutil.Factory, runF func(config copyConfig) error) *cobra.C Short: "Copy a project", Use: "copy []", Example: heredoc.Doc(` - # copy project "1" owned by monalisa to github - gh project copy 1 --source-owner monalisa --target-owner github --title "a new project" + # Copy project "1" owned by monalisa to github + $ gh project copy 1 --source-owner monalisa --target-owner github --title "a new project" `), Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { diff --git a/pkg/cmd/project/create/create.go b/pkg/cmd/project/create/create.go index a261cb600..268a25550 100644 --- a/pkg/cmd/project/create/create.go +++ b/pkg/cmd/project/create/create.go @@ -37,8 +37,8 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(config createConfig) error) *cob Short: "Create a project", Use: "create", Example: heredoc.Doc(` - # create a new project owned by login monalisa - gh project create --owner monalisa --title "a new project" + # Create a new project owned by login monalisa + $ gh project create --owner monalisa --title "a new project" `), RunE: func(cmd *cobra.Command, args []string) error { client, err := client.New(f) diff --git a/pkg/cmd/project/delete/delete.go b/pkg/cmd/project/delete/delete.go index 4848a52ae..993b94d30 100644 --- a/pkg/cmd/project/delete/delete.go +++ b/pkg/cmd/project/delete/delete.go @@ -38,8 +38,8 @@ func NewCmdDelete(f *cmdutil.Factory, runF func(config deleteConfig) error) *cob Short: "Delete a project", Use: "delete []", Example: heredoc.Doc(` - # delete the current user's project "1" - gh project delete 1 --owner "@me" + # Delete the current user's project "1" + $ gh project delete 1 --owner "@me" `), Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { diff --git a/pkg/cmd/project/edit/edit.go b/pkg/cmd/project/edit/edit.go index f303ce357..4eb41f98f 100644 --- a/pkg/cmd/project/edit/edit.go +++ b/pkg/cmd/project/edit/edit.go @@ -45,8 +45,8 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(config editConfig) error) *cobra.C Short: "Edit a project", Use: "edit []", Example: heredoc.Doc(` - # edit the title of monalisa's project "1" - gh project edit 1 --owner monalisa --title "New title" + # Edit the title of monalisa's project "1" + $ gh project edit 1 --owner monalisa --title "New title" `), Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { diff --git a/pkg/cmd/project/field-create/field_create.go b/pkg/cmd/project/field-create/field_create.go index 56a305480..143719f47 100644 --- a/pkg/cmd/project/field-create/field_create.go +++ b/pkg/cmd/project/field-create/field_create.go @@ -41,11 +41,11 @@ func NewCmdCreateField(f *cmdutil.Factory, runF func(config createFieldConfig) e Short: "Create a field in a project", Use: "field-create []", Example: heredoc.Doc(` - # create a field in the current user's project "1" - gh project field-create 1 --owner "@me" --name "new field" --data-type "text" + # Create a field in the current user's project "1" + $ gh project field-create 1 --owner "@me" --name "new field" --data-type "text" - # create a field with three options to select from for owner monalisa - gh project field-create 1 --owner monalisa --name "new field" --data-type "SINGLE_SELECT" --single-select-options "one,two,three" + # Create a field with three options to select from for owner monalisa + $ gh project field-create 1 --owner monalisa --name "new field" --data-type "SINGLE_SELECT" --single-select-options "one,two,three" `), Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { diff --git a/pkg/cmd/project/field-list/field_list.go b/pkg/cmd/project/field-list/field_list.go index 11964cb52..51340c687 100644 --- a/pkg/cmd/project/field-list/field_list.go +++ b/pkg/cmd/project/field-list/field_list.go @@ -30,10 +30,10 @@ func NewCmdList(f *cmdutil.Factory, runF func(config listConfig) error) *cobra.C opts := listOpts{} listCmd := &cobra.Command{ Short: "List the fields in a project", - Use: "field-list number", + Use: "field-list []", Example: heredoc.Doc(` - # list fields in the current user's project "1" - gh project field-list 1 --owner "@me" + # List fields in the current user's project "1" + $ gh project field-list 1 --owner "@me" `), Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { diff --git a/pkg/cmd/project/item-add/item_add.go b/pkg/cmd/project/item-add/item_add.go index 5e4eeff10..00c610f0d 100644 --- a/pkg/cmd/project/item-add/item_add.go +++ b/pkg/cmd/project/item-add/item_add.go @@ -40,8 +40,8 @@ func NewCmdAddItem(f *cmdutil.Factory, runF func(config addItemConfig) error) *c Short: "Add a pull request or an issue to a project", Use: "item-add []", Example: heredoc.Doc(` - # add an item to monalisa's project "1" - gh project item-add 1 --owner monalisa --url https://github.com/monalisa/myproject/issues/23 + # Add an item to monalisa's project "1" + $ gh project item-add 1 --owner monalisa --url https://github.com/monalisa/myproject/issues/23 `), Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { diff --git a/pkg/cmd/project/item-archive/item_archive.go b/pkg/cmd/project/item-archive/item_archive.go index 18a820e86..ec19b4fcc 100644 --- a/pkg/cmd/project/item-archive/item_archive.go +++ b/pkg/cmd/project/item-archive/item_archive.go @@ -46,8 +46,8 @@ func NewCmdArchiveItem(f *cmdutil.Factory, runF func(config archiveItemConfig) e Short: "Archive an item in a project", Use: "item-archive []", Example: heredoc.Doc(` - # archive an item in the current user's project "1" - gh project item-archive 1 --owner "@me" --id + # Archive an item in the current user's project "1" + $ gh project item-archive 1 --owner "@me" --id `), Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { diff --git a/pkg/cmd/project/item-create/item_create.go b/pkg/cmd/project/item-create/item_create.go index d1775172d..dc70fe8a4 100644 --- a/pkg/cmd/project/item-create/item_create.go +++ b/pkg/cmd/project/item-create/item_create.go @@ -40,8 +40,8 @@ func NewCmdCreateItem(f *cmdutil.Factory, runF func(config createItemConfig) err Short: "Create a draft issue item in a project", Use: "item-create []", Example: heredoc.Doc(` - # create a draft issue in the current user's project "1" - gh project item-create 1 --owner "@me" --title "new item" --body "new item body" + # Create a draft issue in the current user's project "1" + $ gh project item-create 1 --owner "@me" --title "new item" --body "new item body" `), Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { diff --git a/pkg/cmd/project/item-delete/item_delete.go b/pkg/cmd/project/item-delete/item_delete.go index fc0d7c5ea..df5df20e9 100644 --- a/pkg/cmd/project/item-delete/item_delete.go +++ b/pkg/cmd/project/item-delete/item_delete.go @@ -39,8 +39,8 @@ func NewCmdDeleteItem(f *cmdutil.Factory, runF func(config deleteItemConfig) err Short: "Delete an item from a project by ID", Use: "item-delete []", Example: heredoc.Doc(` - # delete an item in the current user's project "1" - gh project item-delete 1 --owner "@me" --id + # Delete an item in the current user's project "1" + $ gh project item-delete 1 --owner "@me" --id `), Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { diff --git a/pkg/cmd/project/item-edit/item_edit.go b/pkg/cmd/project/item-edit/item_edit.go index 657af1f54..43aff835a 100644 --- a/pkg/cmd/project/item-edit/item_edit.go +++ b/pkg/cmd/project/item-edit/item_edit.go @@ -24,6 +24,7 @@ type editItemOpts struct { projectID string text string number float64 + numberChanged bool date string singleSelectOptionID string iterationID string @@ -63,23 +64,24 @@ func NewCmdEditItem(f *cmdutil.Factory, runF func(config editItemConfig) error) Short: "Edit an item in a project", Long: heredoc.Docf(` Edit either a draft issue or a project item. Both usages require the ID of the item to edit. - + For non-draft issues, the ID of the project is also required, and only a single field value can be updated per invocation. Remove project item field value using %[1]s--clear%[1]s flag. `, "`"), Example: heredoc.Doc(` - # edit an item's text field value - gh project item-edit --id --field-id --project-id --text "new text" + # Edit an item's text field value + $ gh project item-edit --id --field-id --project-id --text "new text" - # clear an item's field value - gh project item-edit --id --field-id --project-id --clear + # Clear an item's field value + $ gh project item-edit --id --field-id --project-id --clear `), RunE: func(cmd *cobra.Command, args []string) error { + opts.numberChanged = cmd.Flags().Changed("number") if err := cmdutil.MutuallyExclusive( "only one of `--text`, `--number`, `--date`, `--single-select-option-id` or `--iteration-id` may be used", opts.text != "", - opts.number != 0, + opts.numberChanged, opts.date != "", opts.singleSelectOptionID != "", opts.iterationID != "", @@ -89,7 +91,7 @@ func NewCmdEditItem(f *cmdutil.Factory, runF func(config editItemConfig) error) if err := cmdutil.MutuallyExclusive( "cannot use `--text`, `--number`, `--date`, `--single-select-option-id` or `--iteration-id` in conjunction with `--clear`", - opts.text != "" || opts.number != 0 || opts.date != "" || opts.singleSelectOptionID != "" || opts.iterationID != "", + opts.text != "" || opts.numberChanged || opts.date != "" || opts.singleSelectOptionID != "" || opts.iterationID != "", opts.clear, ); err != nil { return err @@ -146,7 +148,7 @@ func runEditItem(config editItemConfig) error { } // update item values - if config.opts.text != "" || config.opts.number != 0 || config.opts.date != "" || config.opts.singleSelectOptionID != "" || config.opts.iterationID != "" { + if config.opts.text != "" || config.opts.numberChanged || config.opts.date != "" || config.opts.singleSelectOptionID != "" || config.opts.iterationID != "" { return updateItemValues(config) } @@ -172,7 +174,7 @@ func buildUpdateItem(config editItemConfig, date time.Time) (*UpdateProjectV2Fie value = githubv4.ProjectV2FieldValue{ Text: githubv4.NewString(githubv4.String(config.opts.text)), } - } else if config.opts.number != 0 { + } else if config.opts.numberChanged { value = githubv4.ProjectV2FieldValue{ Number: githubv4.NewFloat(githubv4.Float(config.opts.number)), } diff --git a/pkg/cmd/project/item-edit/item_edit_test.go b/pkg/cmd/project/item-edit/item_edit_test.go index 2f9a16df6..916bb5899 100644 --- a/pkg/cmd/project/item-edit/item_edit_test.go +++ b/pkg/cmd/project/item-edit/item_edit_test.go @@ -55,6 +55,14 @@ func TestNewCmdeditItem(t *testing.T) { itemID: "123", }, }, + { + name: "number zero", + cli: "--number 0 --id 123", + wants: editItemOpts{ + number: 0, + itemID: "123", + }, + }, { name: "field-id", cli: "--field-id FIELD_ID --id 123", @@ -292,10 +300,64 @@ func TestRunItemEdit_Number(t *testing.T) { config := editItemConfig{ io: ios, opts: editItemOpts{ - number: 123.45, - itemID: "item_id", - projectID: "project_id", - fieldID: "field_id", + number: 123.45, + numberChanged: true, + itemID: "item_id", + projectID: "project_id", + fieldID: "field_id", + }, + client: client, + } + + err := runEditItem(config) + assert.NoError(t, err) + assert.Equal( + t, + "Edited item \"title\"\n", + stdout.String()) +} + +func TestRunItemEdit_NumberZero(t *testing.T) { + defer gock.Off() + // gock.Observe(gock.DumpRequest) + + // edit item + gock.New("https://api.github.com"). + Post("/graphql"). + BodyString(`{"query":"mutation UpdateItemValues.*","variables":{"input":{"projectId":"project_id","itemId":"item_id","fieldId":"field_id","value":{"number":0}}}}`). + Reply(200). + JSON(map[string]interface{}{ + "data": map[string]interface{}{ + "updateProjectV2ItemFieldValue": map[string]interface{}{ + "projectV2Item": map[string]interface{}{ + "ID": "item_id", + "content": map[string]interface{}{ + "__typename": "Issue", + "body": "body", + "title": "title", + "number": 1, + "repository": map[string]interface{}{ + "nameWithOwner": "my-repo", + }, + }, + }, + }, + }, + }) + + client := queries.NewTestClient() + + ios, _, stdout, _ := iostreams.Test() + ios.SetStdoutTTY(true) + + config := editItemConfig{ + io: ios, + opts: editItemOpts{ + number: 0, + numberChanged: true, + itemID: "item_id", + projectID: "project_id", + fieldID: "field_id", }, client: client, } diff --git a/pkg/cmd/project/item-list/item_list.go b/pkg/cmd/project/item-list/item_list.go index d62eb2716..3f792f3bd 100644 --- a/pkg/cmd/project/item-list/item_list.go +++ b/pkg/cmd/project/item-list/item_list.go @@ -32,8 +32,8 @@ func NewCmdList(f *cmdutil.Factory, runF func(config listConfig) error) *cobra.C Short: "List the items in a project", Use: "item-list []", Example: heredoc.Doc(` - # list the items in the current users's project "1" - gh project item-list 1 --owner "@me" + # List the items in the current users's project "1" + $ gh project item-list 1 --owner "@me" `), Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { diff --git a/pkg/cmd/project/link/link.go b/pkg/cmd/project/link/link.go index 1e334af8c..f2477dde4 100644 --- a/pkg/cmd/project/link/link.go +++ b/pkg/cmd/project/link/link.go @@ -41,16 +41,16 @@ func NewCmdLink(f *cmdutil.Factory, runF func(config linkConfig) error) *cobra.C opts := linkOpts{} linkCmd := &cobra.Command{ Short: "Link a project to a repository or a team", - Use: "link [] [flag]", + Use: "link []", Example: heredoc.Doc(` - # link monalisa's project 1 to her repository "my_repo" - gh project link 1 --owner monalisa --repo my_repo + # Link monalisa's project 1 to her repository "my_repo" + $ gh project link 1 --owner monalisa --repo my_repo - # link monalisa's organization's project 1 to her team "my_team" - gh project link 1 --owner my_organization --team my_team + # Link monalisa's organization's project 1 to her team "my_team" + $ gh project link 1 --owner my_organization --team my_team - # link monalisa's project 1 to the repository of current directory if neither --repo nor --team is specified - gh project link 1 + # Link monalisa's project 1 to the repository of current directory if neither --repo nor --team is specified + $ gh project link 1 `), RunE: func(cmd *cobra.Command, args []string) error { client, err := client.New(f) diff --git a/pkg/cmd/project/list/list.go b/pkg/cmd/project/list/list.go index ba6e1d747..01868b4c3 100644 --- a/pkg/cmd/project/list/list.go +++ b/pkg/cmd/project/list/list.go @@ -35,11 +35,11 @@ func NewCmdList(f *cmdutil.Factory, runF func(config listConfig) error) *cobra.C Use: "list", Short: "List the projects for an owner", Example: heredoc.Doc(` - # list the current user's projects - gh project list + # List the current user's projects + $ gh project list - # list the projects for org github including closed projects - gh project list --owner github --closed + # List the projects for org github including closed projects + $ gh project list --owner github --closed `), Aliases: []string{"ls"}, RunE: func(cmd *cobra.Command, args []string) error { diff --git a/pkg/cmd/project/mark-template/mark_template.go b/pkg/cmd/project/mark-template/mark_template.go index 025fe2838..616f8428d 100644 --- a/pkg/cmd/project/mark-template/mark_template.go +++ b/pkg/cmd/project/mark-template/mark_template.go @@ -44,11 +44,11 @@ func NewCmdMarkTemplate(f *cmdutil.Factory, runF func(config markTemplateConfig) Short: "Mark a project as a template", Use: "mark-template []", Example: heredoc.Doc(` - # mark the github org's project "1" as a template - gh project mark-template 1 --owner "github" + # Mark the github org's project "1" as a template + $ gh project mark-template 1 --owner "github" - # unmark the github org's project "1" as a template - gh project mark-template 1 --owner "github" --undo + # Unmark the github org's project "1" as a template + $ gh project mark-template 1 --owner "github" --undo `), Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { diff --git a/pkg/cmd/project/project.go b/pkg/cmd/project/project.go index 6dae4a897..cddbad940 100644 --- a/pkg/cmd/project/project.go +++ b/pkg/cmd/project/project.go @@ -29,7 +29,13 @@ func NewCmdProject(f *cmdutil.Factory) *cobra.Command { var cmd = &cobra.Command{ Use: "project ", Short: "Work with GitHub Projects.", - Long: "Work with GitHub Projects. Note that the token you are using must have 'project' scope, which is not set by default. You can verify your token scope by running 'gh auth status' and add the project scope by running 'gh auth refresh -s project'.", + Long: heredoc.Docf(` + Work with GitHub Projects. + + The minimum required scope for the token is: %[1]sproject%[1]s. + You can verify your token scope by running %[1]sgh auth status%[1]s and + add the %[1]sproject%[1]s scope by running %[1]sgh auth refresh -s project%[1]s. + `, "`"), Example: heredoc.Doc(` $ gh project create --owner monalisa --title "Roadmap" $ gh project view 1 --owner cli --web diff --git a/pkg/cmd/project/unlink/unlink.go b/pkg/cmd/project/unlink/unlink.go index 7a75db6d1..8cc2c0747 100644 --- a/pkg/cmd/project/unlink/unlink.go +++ b/pkg/cmd/project/unlink/unlink.go @@ -41,16 +41,16 @@ func NewCmdUnlink(f *cmdutil.Factory, runF func(config unlinkConfig) error) *cob opts := unlinkOpts{} linkCmd := &cobra.Command{ Short: "Unlink a project from a repository or a team", - Use: "unlink [] [flag]", + Use: "unlink []", Example: heredoc.Doc(` - # unlink monalisa's project 1 from her repository "my_repo" - gh project unlink 1 --owner monalisa --repo my_repo + # Unlink monalisa's project 1 from her repository "my_repo" + $ gh project unlink 1 --owner monalisa --repo my_repo - # unlink monalisa's organization's project 1 from her team "my_team" - gh project unlink 1 --owner my_organization --team my_team + # Unlink monalisa's organization's project 1 from her team "my_team" + $ gh project unlink 1 --owner my_organization --team my_team - # unlink monalisa's project 1 from the repository of current directory if neither --repo nor --team is specified - gh project unlink 1 + # Unlink monalisa's project 1 from the repository of current directory if neither --repo nor --team is specified + $ gh project unlink 1 `), RunE: func(cmd *cobra.Command, args []string) error { client, err := client.New(f) diff --git a/pkg/cmd/project/view/view.go b/pkg/cmd/project/view/view.go index 3d94b5af3..49056b8d7 100644 --- a/pkg/cmd/project/view/view.go +++ b/pkg/cmd/project/view/view.go @@ -34,11 +34,11 @@ func NewCmdView(f *cmdutil.Factory, runF func(config viewConfig) error) *cobra.C Short: "View a project", Use: "view []", Example: heredoc.Doc(` - # view the current user's project "1" - gh project view 1 + # View the current user's project "1" + $ gh project view 1 - # open user monalisa's project "1" in the browser - gh project view 1 --owner monalisa --web + # Open user monalisa's project "1" in the browser + $ gh project view 1 --owner monalisa --web `), Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { diff --git a/pkg/cmd/release/create/create.go b/pkg/cmd/release/create/create.go index dea41dd57..7e3a1d43a 100644 --- a/pkg/cmd/release/create/create.go +++ b/pkg/cmd/release/create/create.go @@ -60,6 +60,7 @@ type CreateOptions struct { NotesStartTag string VerifyTag bool NotesFromTag bool + FailOnNoCommits bool } func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Command { @@ -99,37 +100,45 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co When using automatically generated release notes, a release title will also be automatically generated unless a title was explicitly passed. Additional release notes can be prepended to automatically generated notes by using the %[1]s--notes%[1]s flag. + + By default, the release is created even if there are no new commits since the last release. + This may result in the same or duplicate release which may not be desirable in some cases. + Use %[1]s--fail-on-no-commits%[1]s to fail if no new commits are available. This flag has no + effect if there are no existing releases or this is the very first release. `, "`"), Example: heredoc.Doc(` - Interactively create a release + # Interactively create a release $ gh release create - Interactively create a release from specific tag + # Interactively create a release from specific tag $ gh release create v1.2.3 - Non-interactively create a release + # Non-interactively create a release $ gh release create v1.2.3 --notes "bugfix release" - Use automatically generated release notes + # Use automatically generated release notes $ gh release create v1.2.3 --generate-notes - Use release notes from a file + # Use release notes from a file $ gh release create v1.2.3 -F release-notes.md - Use annotated tag notes + # Use annotated tag notes $ gh release create v1.2.3 --notes-from-tag - Don't mark the release as latest - $ gh release create v1.2.3 --latest=false + # Don't mark the release as latest + $ gh release create v1.2.3 --latest=false - Upload all tarballs in a directory as release assets + # Upload all tarballs in a directory as release assets $ gh release create v1.2.3 ./dist/*.tgz - Upload a release asset with a display label + # Upload a release asset with a display label $ gh release create v1.2.3 '/path/to/asset.zip#My display label' - Create a release and start a discussion + # Create a release and start a discussion $ gh release create v1.2.3 --discussion-category "General" + + # Create a release only if there are new commits available since the last release + $ gh release create v1.2.3 --fail-on-no-commits `), Aliases: []string{"new"}, RunE: func(cmd *cobra.Command, args []string) error { @@ -194,6 +203,7 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co cmdutil.NilBoolFlag(cmd, &opts.IsLatest, "latest", "", "Mark this release as \"Latest\" (default [automatic based on date and version]). --latest=false to explicitly NOT set as latest") cmd.Flags().BoolVarP(&opts.VerifyTag, "verify-tag", "", false, "Abort in case the git tag doesn't already exist in the remote repository") cmd.Flags().BoolVarP(&opts.NotesFromTag, "notes-from-tag", "", false, "Automatically generate notes from annotated tag") + cmd.Flags().BoolVar(&opts.FailOnNoCommits, "fail-on-no-commits", false, "Fail if there are no commits since the last release (no impact on the first release)") _ = cmdutil.RegisterBranchCompletionFlags(f.GitClient, cmd, "target") @@ -211,6 +221,16 @@ func createRun(opts *CreateOptions) error { return err } + if opts.FailOnNoCommits { + isNew, err := isNewRelease(httpClient, baseRepo) + if err != nil { + return fmt.Errorf("failed to check whether there were new commits since last release: %v", err) + } + if !isNew { + return fmt.Errorf("no new commits since the last release") + } + } + var existingTag bool if opts.TagName == "" { tags, err := getTags(httpClient, baseRepo, 5) diff --git a/pkg/cmd/release/create/create_test.go b/pkg/cmd/release/create/create_test.go index c9e9a8c8a..a6e6cd7c6 100644 --- a/pkg/cmd/release/create/create_test.go +++ b/pkg/cmd/release/create/create_test.go @@ -52,17 +52,18 @@ func Test_NewCmdCreate(t *testing.T) { args: "", isTTY: true, want: CreateOptions{ - TagName: "", - Target: "", - Name: "", - Body: "", - BodyProvided: false, - Draft: false, - Prerelease: false, - RepoOverride: "", - Concurrency: 5, - Assets: []*shared.AssetForUpload(nil), - VerifyTag: false, + TagName: "", + Target: "", + Name: "", + Body: "", + BodyProvided: false, + Draft: false, + Prerelease: false, + RepoOverride: "", + Concurrency: 5, + Assets: []*shared.AssetForUpload(nil), + VerifyTag: false, + FailOnNoCommits: false, }, }, { @@ -76,17 +77,18 @@ func Test_NewCmdCreate(t *testing.T) { args: "v1.2.3", isTTY: true, want: CreateOptions{ - TagName: "v1.2.3", - Target: "", - Name: "", - Body: "", - BodyProvided: false, - Draft: false, - Prerelease: false, - RepoOverride: "", - Concurrency: 5, - Assets: []*shared.AssetForUpload(nil), - VerifyTag: false, + TagName: "v1.2.3", + Target: "", + Name: "", + Body: "", + BodyProvided: false, + Draft: false, + Prerelease: false, + RepoOverride: "", + Concurrency: 5, + Assets: []*shared.AssetForUpload(nil), + VerifyTag: false, + FailOnNoCommits: false, }, }, { @@ -347,6 +349,19 @@ func Test_NewCmdCreate(t *testing.T) { isTTY: false, wantErr: "using `--notes-from-tag` with `--generate-notes` or `--notes-start-tag` is not supported", }, + { + name: "with --fail-on-no-commits", + args: "v1.2.3 --fail-on-no-commits", + isTTY: false, + want: CreateOptions{ + TagName: "v1.2.3", + BodyProvided: false, + Concurrency: 5, + Assets: []*shared.AssetForUpload(nil), + NotesFromTag: false, + FailOnNoCommits: true, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -402,6 +417,7 @@ func Test_NewCmdCreate(t *testing.T) { assert.Equal(t, tt.want.IsLatest, opts.IsLatest) assert.Equal(t, tt.want.VerifyTag, opts.VerifyTag) assert.Equal(t, tt.want.NotesFromTag, opts.NotesFromTag) + assert.Equal(t, tt.want.FailOnNoCommits, opts.FailOnNoCommits) require.Equal(t, len(tt.want.Assets), len(opts.Assets)) for i := range tt.want.Assets { @@ -460,6 +476,100 @@ func Test_createRun(t *testing.T) { wantStdout: "https://github.com/OWNER/REPO/releases/tag/v1.2.3\n", wantStderr: ``, }, + { + name: "create a release if there are new commits and the last release does not exist", + isTTY: true, + opts: CreateOptions{ + TagName: "v1.2.3", + Name: "The Big 1.2", + Body: "* Fixed bugs", + BodyProvided: true, + Target: "", + FailOnNoCommits: true, + }, + runStubs: defaultRunStubs, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register(httpmock.REST("GET", "repos/OWNER/REPO/releases/latest"), httpmock.StatusStringResponse(404, `{ + "message": "Not Found", + "documentation_url": "https://docs.github.com/rest/releases/releases#get-the-latest-release", + "status": "404" + }`)) + reg.Register(httpmock.REST("POST", "repos/OWNER/REPO/releases"), httpmock.RESTPayload(201, `{ + "url": "https://api.github.com/releases/123", + "upload_url": "https://api.github.com/assets/upload", + "html_url": "https://github.com/OWNER/REPO/releases/tag/v1.2.3" + }`, func(params map[string]interface{}) { + assert.Equal(t, map[string]interface{}{ + "tag_name": "v1.2.3", + "name": "The Big 1.2", + "body": "* Fixed bugs", + "draft": false, + "prerelease": false, + }, params) + })) + }, + wantStdout: "https://github.com/OWNER/REPO/releases/tag/v1.2.3\n", + wantStderr: ``, + }, + { + name: "create a release if there are new commits and the last release exists", + isTTY: true, + opts: CreateOptions{ + TagName: "v1.2.3", + Name: "The Big 1.2", + Body: "* Fixed bugs", + BodyProvided: true, + Target: "", + FailOnNoCommits: true, + }, + runStubs: defaultRunStubs, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register(httpmock.REST("GET", "repos/OWNER/REPO/releases/latest"), httpmock.StatusStringResponse(200, `{ + "tag_name": "v1.2.2" + }`)) + reg.Register(httpmock.REST("GET", "repos/OWNER/REPO/compare/v1.2.2...HEAD"), httpmock.StatusStringResponse(200, `{ + "status": "ahead" + }`)) + reg.Register(httpmock.REST("POST", "repos/OWNER/REPO/releases"), httpmock.RESTPayload(201, `{ + "url": "https://api.github.com/releases/123", + "upload_url": "https://api.github.com/assets/upload", + "html_url": "https://github.com/OWNER/REPO/releases/tag/v1.2.3" + }`, func(params map[string]interface{}) { + assert.Equal(t, map[string]interface{}{ + "tag_name": "v1.2.3", + "name": "The Big 1.2", + "body": "* Fixed bugs", + "draft": false, + "prerelease": false, + }, params) + })) + }, + wantStdout: "https://github.com/OWNER/REPO/releases/tag/v1.2.3\n", + wantStderr: ``, + }, + { + name: "create a release if there are no new commits but the last release exists", + isTTY: true, + opts: CreateOptions{ + TagName: "v1.2.3", + Name: "The Big 1.2", + Body: "* Fixed bugs", + BodyProvided: true, + Target: "", + FailOnNoCommits: true, + }, + httpStubs: func(t *testing.T, reg *httpmock.Registry) { + reg.Register(httpmock.REST("GET", "repos/OWNER/REPO/releases/latest"), httpmock.StatusStringResponse(200, `{ + "tag_name": "v1.2.2" + }`)) + reg.Register(httpmock.REST("GET", "repos/OWNER/REPO/compare/v1.2.2...HEAD"), httpmock.StatusStringResponse(200, `{ + "status": "identical" + }`)) + }, + wantErr: "no new commits since the last release", + wantStdout: "", + wantStderr: ``, + }, { name: "with discussion category", isTTY: true, diff --git a/pkg/cmd/release/create/http.go b/pkg/cmd/release/create/http.go index 3bb55f39e..4311a3896 100644 --- a/pkg/cmd/release/create/http.go +++ b/pkg/cmd/release/create/http.go @@ -2,6 +2,7 @@ package create import ( "bytes" + "context" "encoding/json" "errors" "fmt" @@ -299,3 +300,31 @@ func tokenHasWorkflowScope(resp *http.Response) bool { return slices.Contains(strings.Split(scopes, ","), "workflow") } + +// isNewRelease checks if there are new commits since the latest release. +func isNewRelease(httpClient *http.Client, repo ghrepo.Interface) (bool, error) { + ctx := context.Background() + release, err := shared.FetchLatestRelease(ctx, httpClient, repo) + if err != nil { + if errors.Is(err, shared.ErrReleaseNotFound) { + return true, nil + } else { + return false, err + } + } + + tagName := release.TagName + path := fmt.Sprintf("repos/%s/%s/compare/%s...HEAD?per_page=1", repo.RepoOwner(), repo.RepoName(), tagName) + + var comparisonStatus struct { + Status string `json:"status"` + } + + apiClient := api.NewClientFromHTTP(httpClient) + if err := apiClient.REST(repo.RepoHost(), "GET", path, nil, &comparisonStatus); err != nil { + return false, err + } + + isNew := comparisonStatus.Status == "ahead" + return isNew, nil +} diff --git a/pkg/cmd/release/download/download.go b/pkg/cmd/release/download/download.go index bb0df7243..cdf6135b6 100644 --- a/pkg/cmd/release/download/download.go +++ b/pkg/cmd/release/download/download.go @@ -57,16 +57,16 @@ func NewCmdDownload(f *cmdutil.Factory, runF func(*DownloadOptions) error) *cobr is required. `, "`"), Example: heredoc.Doc(` - # download all assets from a specific release + # Download all assets from a specific release $ gh release download v1.2.3 - # download only Debian packages for the latest release + # Download only Debian packages for the latest release $ gh release download --pattern '*.deb' - # specify multiple file patterns + # Specify multiple file patterns $ gh release download -p '*.deb' -p '*.rpm' - # download the archive of the source code for a release + # Download the archive of the source code for a release $ gh release download v1.2.3 --archive=zip `), Args: cobra.MaximumNArgs(1), diff --git a/pkg/cmd/release/edit/edit.go b/pkg/cmd/release/edit/edit.go index 245041bae..95abf5e20 100644 --- a/pkg/cmd/release/edit/edit.go +++ b/pkg/cmd/release/edit/edit.go @@ -43,10 +43,10 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman Use: "edit ", Short: "Edit a release", Example: heredoc.Doc(` - Publish a release that was previously a draft + # Publish a release that was previously a draft $ gh release edit v1.0 --draft=false - Update the release notes from the content of a file + # Update the release notes from the content of a file $ gh release edit v1.0 --notes-file /path/to/release_notes.md `), Args: cobra.ExactArgs(1), diff --git a/pkg/cmd/release/shared/fetch.go b/pkg/cmd/release/shared/fetch.go index c9cf7a6a4..8db7e502a 100644 --- a/pkg/cmd/release/shared/fetch.go +++ b/pkg/cmd/release/shared/fetch.go @@ -124,7 +124,7 @@ func (rel *Release) ExportData(fields []string) map[string]interface{} { return data } -var errNotFound = errors.New("release not found") +var ErrReleaseNotFound = errors.New("release not found") type fetchResult struct { release *Release @@ -150,7 +150,7 @@ func FetchRelease(ctx context.Context, httpClient *http.Client, repo ghrepo.Inte }() res := <-results - if errors.Is(res.error, errNotFound) { + if errors.Is(res.error, ErrReleaseNotFound) { res = <-results cancel() // satisfy the linter even though no goroutines are running anymore } else { @@ -190,7 +190,7 @@ func fetchDraftRelease(ctx context.Context, httpClient *http.Client, repo ghrepo } if query.Repository.Release == nil || !query.Repository.Release.IsDraft { - return nil, errNotFound + return nil, ErrReleaseNotFound } // Then, use REST to get information about the draft release. In theory, we could have fetched @@ -213,7 +213,7 @@ func fetchReleasePath(ctx context.Context, httpClient *http.Client, host string, if resp.StatusCode == 404 { _, _ = io.Copy(io.Discard, resp.Body) - return nil, errNotFound + return nil, ErrReleaseNotFound } else if resp.StatusCode > 299 { return nil, api.HandleHTTPError(resp) } diff --git a/pkg/cmd/release/view/view_test.go b/pkg/cmd/release/view/view_test.go index 6fc747f69..1fed0fdc8 100644 --- a/pkg/cmd/release/view/view_test.go +++ b/pkg/cmd/release/view/view_test.go @@ -144,15 +144,15 @@ func Test_viewRun(t *testing.T) { wantStdout: heredoc.Doc(` v1.2.3 MonaLisa released this about 1 day ago - + • Fixed bugs - - + + Assets windows.zip 12 B linux.tgz 34 B - + View on GitHub: https://github.com/OWNER/REPO/releases/tags/v1.2.3 `), wantStderr: ``, @@ -168,15 +168,15 @@ func Test_viewRun(t *testing.T) { wantStdout: heredoc.Doc(` v1.2.3 MonaLisa released this about 1 day ago - + • Fixed bugs - - + + Assets windows.zip 12 B linux.tgz 34 B - + View on GitHub: https://github.com/OWNER/REPO/releases/tags/v1.2.3 `), wantStderr: ``, diff --git a/pkg/cmd/repo/create/create.go b/pkg/cmd/repo/create/create.go index ba33156c8..cd7c56ea8 100644 --- a/pkg/cmd/repo/create/create.go +++ b/pkg/cmd/repo/create/create.go @@ -103,17 +103,17 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co The repo is created with the configured repository default branch, see . `, "`"), Example: heredoc.Doc(` - # create a repository interactively - gh repo create + # Create a repository interactively + $ gh repo create - # create a new remote repository and clone it locally - gh repo create my-project --public --clone + # Create a new remote repository and clone it locally + $ gh repo create my-project --public --clone - # create a new remote repository in a different organization - gh repo create my-org/my-project --public + # Create a new remote repository in a different organization + $ gh repo create my-org/my-project --public - # create a remote repository from the current directory - gh repo create my-project --private --source=. --remote=upstream + # Create a remote repository from the current directory + $ gh repo create my-project --private --source=. --remote=upstream `), Args: cobra.MaximumNArgs(1), Aliases: []string{"new"}, diff --git a/pkg/cmd/repo/credits/credits.go b/pkg/cmd/repo/credits/credits.go index 7db08a231..a26b6a731 100644 --- a/pkg/cmd/repo/credits/credits.go +++ b/pkg/cmd/repo/credits/credits.go @@ -43,13 +43,13 @@ func NewCmdCredits(f *cmdutil.Factory, runF func(*CreditsOptions) error) *cobra. Short: "View credits for this tool", Long: `View animated credits for gh, the tool you are currently using :)`, Example: heredoc.Doc(` - # see a credits animation for this project + # See a credits animation for this project $ gh credits - # display a non-animated thank you + # Display a non-animated thank you $ gh credits -s - # just print the contributors, one per line + # Just print the contributors, one per line $ gh credits | cat `), Args: cobra.ExactArgs(0), @@ -79,16 +79,16 @@ func NewCmdRepoCredits(f *cmdutil.Factory, runF func(*CreditsOptions) error) *co Use: "credits []", Short: "View credits for a repository", Example: heredoc.Doc(` - # view credits for the current repository + # View credits for the current repository $ gh repo credits - # view credits for a specific repository + # View credits for a specific repository $ gh repo credits cool/repo - # print a non-animated thank you + # Print a non-animated thank you $ gh repo credits -s - # pipe to just print the contributors, one per line + # Pipe to just print the contributors, one per line $ gh repo credits | cat `), Args: cobra.MaximumNArgs(1), diff --git a/pkg/cmd/repo/delete/delete.go b/pkg/cmd/repo/delete/delete.go index 7c6476f1d..96cfe5e95 100644 --- a/pkg/cmd/repo/delete/delete.go +++ b/pkg/cmd/repo/delete/delete.go @@ -41,10 +41,10 @@ func NewCmdDelete(f *cmdutil.Factory, runF func(*DeleteOptions) error) *cobra.Co Short: "Delete a repository", Long: heredoc.Docf(` Delete a GitHub repository. - + With no argument, deletes the current repository. Otherwise, deletes the specified repository. - Deletion requires authorization with the %[1]sdelete_repo%[1]s scope. + Deletion requires authorization with the %[1]sdelete_repo%[1]s scope. To authorize, run %[1]sgh auth refresh -s delete_repo%[1]s `, "`"), Args: cobra.MaximumNArgs(1), @@ -65,9 +65,9 @@ func NewCmdDelete(f *cmdutil.Factory, runF func(*DeleteOptions) error) *cobra.Co }, } - cmd.Flags().BoolVar(&opts.Confirmed, "confirm", false, "confirm deletion without prompting") + cmd.Flags().BoolVar(&opts.Confirmed, "confirm", false, "Confirm deletion without prompting") _ = cmd.Flags().MarkDeprecated("confirm", "use `--yes` instead") - cmd.Flags().BoolVar(&opts.Confirmed, "yes", false, "confirm deletion without prompting") + cmd.Flags().BoolVar(&opts.Confirmed, "yes", false, "Confirm deletion without prompting") return cmd } diff --git a/pkg/cmd/repo/deploy-key/add/add.go b/pkg/cmd/repo/deploy-key/add/add.go index 473d42498..2466ec98a 100644 --- a/pkg/cmd/repo/deploy-key/add/add.go +++ b/pkg/cmd/repo/deploy-key/add/add.go @@ -34,13 +34,13 @@ func NewCmdAdd(f *cmdutil.Factory, runF func(*AddOptions) error) *cobra.Command Short: "Add a deploy key to a GitHub repository", Long: heredoc.Doc(` Add a deploy key to a GitHub repository. - + Note that any key added by gh will be associated with the current authentication token. If you de-authorize the GitHub CLI app or authentication token from your account, any deploy keys added by GitHub CLI will be removed as well. `), Example: heredoc.Doc(` - # generate a passwordless SSH key and add it as a deploy key to a repository + # Generate a passwordless SSH key and add it as a deploy key to a repository $ ssh-keygen -t ed25519 -C "my description" -N "" -f ~/.ssh/gh-test $ gh repo deploy-key add ~/.ssh/gh-test.pub `), diff --git a/pkg/cmd/repo/edit/edit.go b/pkg/cmd/repo/edit/edit.go index daa700cfb..94ad8868a 100644 --- a/pkg/cmd/repo/edit/edit.go +++ b/pkg/cmd/repo/edit/edit.go @@ -119,15 +119,15 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(options *EditOptions) error) *cobr When the %[1]s--visibility%[1]s flag is used, %[1]s--accept-visibility-change-consequences%[1]s flag is required. - For information on all the potential consequences, see + For information on all the potential consequences, see . `, "`"), Args: cobra.MaximumNArgs(1), Example: heredoc.Doc(` - # enable issues and wiki - gh repo edit --enable-issues --enable-wiki + # Enable issues and wiki + $ gh repo edit --enable-issues --enable-wiki - # disable projects - gh repo edit --enable-projects=false + # Disable projects + $ gh repo edit --enable-projects=false `), RunE: func(cmd *cobra.Command, args []string) error { if len(args) > 0 { diff --git a/pkg/cmd/repo/gitignore/view/view.go b/pkg/cmd/repo/gitignore/view/view.go index 46e674632..fcf83700a 100644 --- a/pkg/cmd/repo/gitignore/view/view.go +++ b/pkg/cmd/repo/gitignore/view/view.go @@ -34,23 +34,23 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman Short: "View an available repository gitignore template", Long: heredoc.Docf(` View an available repository %[1]s.gitignore%[1]s template. - + %[1]s