diff --git a/.github/workflows/deployment.yml b/.github/workflows/deployment.yml index 0470a6644..67ea742f6 100644 --- a/.github/workflows/deployment.yml +++ b/.github/workflows/deployment.yml @@ -50,7 +50,7 @@ jobs: with: go-version-file: 'go.mod' - name: Install GoReleaser - uses: goreleaser/goreleaser-action@e435ccd777264be153ace6237001ef4d979d3a7a # v6.4.0 + uses: goreleaser/goreleaser-action@ec59f474b9834571250b370d4735c50f8e2d1e29 # v7.0.0 with: # The version is pinned not only for security purposes, but also to avoid breaking # our scripts, which rely on the specific file names generated by GoReleaser. @@ -70,7 +70,7 @@ jobs: run: | go run ./cmd/gen-docs --website --doc-path dist/manual tar -czvf dist/manual.tar.gz -C dist -- manual - - uses: actions/upload-artifact@v6 + - uses: actions/upload-artifact@v7 with: name: linux if-no-files-found: error @@ -111,7 +111,7 @@ jobs: security set-key-partition-list -S "apple-tool:,apple:,codesign:" -s -k "$keychain_password" "$keychain" rm "$RUNNER_TEMP/cert.p12" - name: Install GoReleaser - uses: goreleaser/goreleaser-action@e435ccd777264be153ace6237001ef4d979d3a7a # v6.4.0 + uses: goreleaser/goreleaser-action@ec59f474b9834571250b370d4735c50f8e2d1e29 # v7.0.0 with: # The version is pinned not only for security purposes, but also to avoid breaking # our scripts, which rely on the specific file names generated by GoReleaser. @@ -150,7 +150,7 @@ jobs: run: | shopt -s failglob script/pkgmacos "$TAG_NAME" - - uses: actions/upload-artifact@v6 + - uses: actions/upload-artifact@v7 with: name: macos if-no-files-found: error @@ -173,7 +173,7 @@ jobs: with: go-version-file: 'go.mod' - name: Install GoReleaser - uses: goreleaser/goreleaser-action@e435ccd777264be153ace6237001ef4d979d3a7a # v6.4.0 + uses: goreleaser/goreleaser-action@ec59f474b9834571250b370d4735c50f8e2d1e29 # v7.0.0 with: # The version is pinned not only for security purposes, but also to avoid breaking # our scripts, which rely on the specific file names generated by GoReleaser. @@ -263,7 +263,7 @@ jobs: Get-ChildItem -Path .\dist -Filter *.msi | ForEach-Object { .\script\sign.ps1 $_.FullName } - - uses: actions/upload-artifact@v6 + - uses: actions/upload-artifact@v7 with: name: windows if-no-files-found: error @@ -281,7 +281,7 @@ jobs: - name: Checkout cli/cli uses: actions/checkout@v6 - name: Merge built artifacts - uses: actions/download-artifact@v7 + uses: actions/download-artifact@v8 - name: Checkout documentation site uses: actions/checkout@v6 with: @@ -334,7 +334,7 @@ jobs: rpmsign --addsign dist/*.rpm - name: Attest release artifacts if: inputs.environment == 'production' - uses: actions/attest-build-provenance@96278af6caaf10aea03fd8d33a09a777ca52d62f # v3.2.0 + uses: actions/attest-build-provenance@a2bbfa25375fe432b6a289bc6b6cd05ecd0c4c32 # v4.1.0 with: subject-path: "dist/gh_*" create-storage-record: false # (default: true) diff --git a/.gitignore b/.gitignore index a4b73ac7a..b82a00c72 100644 --- a/.gitignore +++ b/.gitignore @@ -18,6 +18,10 @@ # Windows resource files /cmd/gh/*.syso +# Third-party licenses +/internal/licenses/embed/*/* +!/internal/licenses/embed/*/PLACEHOLDER + # VS Code .vscode diff --git a/api/client.go b/api/client.go index 207fd86d3..895f29692 100644 --- a/api/client.go +++ b/api/client.go @@ -10,6 +10,7 @@ import ( "regexp" "strings" + "github.com/cli/cli/v2/pkg/set" ghAPI "github.com/cli/go-gh/v2/pkg/api" ghauth "github.com/cli/go-gh/v2/pkg/auth" ) @@ -180,6 +181,10 @@ func handleResponse(err error) error { var gqlErr *ghAPI.GraphQLError if errors.As(err, &gqlErr) { + scopeErr := GenerateScopeErrorForGQL(gqlErr) + if scopeErr != nil { + return scopeErr + } return GraphQLError{ GraphQLError: gqlErr, } @@ -188,6 +193,40 @@ func handleResponse(err error) error { return err } +func GenerateScopeErrorForGQL(gqlErr *ghAPI.GraphQLError) error { + missing := set.NewStringSet() + for _, e := range gqlErr.Errors { + if e.Type != "INSUFFICIENT_SCOPES" { + continue + } + missing.AddValues(requiredScopesFromServerMessage(e.Message)) + } + if missing.Len() > 0 { + s := missing.ToSlice() + // TODO: this duplicates parts of generateScopesSuggestion + return fmt.Errorf( + "error: your authentication token is missing required scopes %v\n"+ + "To request it, run: gh auth refresh -s %s", + s, + strings.Join(s, ",")) + } + return nil +} + +var scopesRE = regexp.MustCompile(`one of the following scopes: \[(.+?)]`) + +func requiredScopesFromServerMessage(msg string) []string { + m := scopesRE.FindStringSubmatch(msg) + if m == nil { + return nil + } + var scopes []string + for _, mm := range strings.Split(m[1], ",") { + scopes = append(scopes, strings.Trim(mm, "' ")) + } + return scopes +} + // ScopesSuggestion is an error messaging utility that prints the suggestion to request additional OAuth // scopes in case a server response indicates that there are missing scopes. func ScopesSuggestion(resp *http.Response) string { diff --git a/api/client_test.go b/api/client_test.go index f988e090c..ad75c1889 100644 --- a/api/client_test.go +++ b/api/client_test.go @@ -10,6 +10,7 @@ import ( "github.com/cli/cli/v2/pkg/httpmock" "github.com/cli/cli/v2/pkg/iostreams" + "github.com/cli/go-gh/v2/pkg/api" "github.com/stretchr/testify/assert" ) @@ -256,3 +257,79 @@ func TestHTTPHeaders(t *testing.T) { } assert.Equal(t, "", stderr.String()) } + +func TestGenerateScopeErrorForGQL(t *testing.T) { + tests := []struct { + name string + gqlError *api.GraphQLError + wantErr bool + expected string + }{ + { + name: "missing scope", + gqlError: &api.GraphQLError{ + Errors: []api.GraphQLErrorItem{ + { + Type: "INSUFFICIENT_SCOPES", + Message: "The 'addProjectV2ItemById' field requires one of the following scopes: ['project']", + }, + }, + }, + wantErr: true, + expected: "error: your authentication token is missing required scopes [project]\n" + + "To request it, run: gh auth refresh -s project", + }, + + { + name: "ignore non-scope errors", + gqlError: &api.GraphQLError{ + Errors: []api.GraphQLErrorItem{ + { + Type: "NOT_FOUND", + Message: "Could not resolve to a Repository", + }, + }, + }, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := GenerateScopeErrorForGQL(tt.gqlError) + if tt.wantErr { + assert.NotNil(t, err) + assert.Equal(t, tt.expected, err.Error()) + } else { + assert.Nil(t, err) + } + }) + } +} + +func TestRequiredScopesFromServerMessage(t *testing.T) { + tests := []struct { + msg string + expected []string + }{ + { + msg: "requires one of the following scopes: ['project']", + expected: []string{"project"}, + }, + { + msg: "requires one of the following scopes: ['repo', 'read:org']", + expected: []string{"repo", "read:org"}, + }, + { + msg: "no match here", + expected: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.msg, func(t *testing.T) { + output := requiredScopesFromServerMessage(tt.msg) + assert.Equal(t, tt.expected, output) + }) + } +} diff --git a/api/queries_pr.go b/api/queries_pr.go index 1bc5ddb55..c38018a68 100644 --- a/api/queries_pr.go +++ b/api/queries_pr.go @@ -296,9 +296,10 @@ type PullRequestCommitCommit struct { } type PullRequestFile struct { - Path string `json:"path"` - Additions int `json:"additions"` - Deletions int `json:"deletions"` + Path string `json:"path"` + Additions int `json:"additions"` + Deletions int `json:"deletions"` + ChangeType string `json:"changeType"` } type ReviewRequests struct { diff --git a/api/query_builder.go b/api/query_builder.go index cb80b595f..c3e1e9ba3 100644 --- a/api/query_builder.go +++ b/api/query_builder.go @@ -148,7 +148,8 @@ var prFiles = shortenQuery(` nodes { additions, deletions, - path + path, + changeType } } `) diff --git a/api/query_builder_test.go b/api/query_builder_test.go index f854cd36a..f0b5789a3 100644 --- a/api/query_builder_test.go +++ b/api/query_builder_test.go @@ -26,7 +26,7 @@ func TestPullRequestGraphQL(t *testing.T) { { name: "compressed query", fields: []string{"files"}, - want: "files(first: 100) {nodes {additions,deletions,path}}", + want: "files(first: 100) {nodes {additions,deletions,path,changeType}}", }, { name: "invalid fields", @@ -72,7 +72,7 @@ func TestIssueGraphQL(t *testing.T) { { name: "compressed query", fields: []string{"files"}, - want: "files(first: 100) {nodes {additions,deletions,path}}", + want: "files(first: 100) {nodes {additions,deletions,path,changeType}}", }, { name: "projectItems", diff --git a/go.mod b/go.mod index 68cf9d6ff..5e6d07247 100644 --- a/go.mod +++ b/go.mod @@ -21,7 +21,7 @@ require ( github.com/creack/pty v1.1.24 github.com/digitorus/timestamp v0.0.0-20250524132541-c45532741eea github.com/distribution/reference v0.6.0 - github.com/gabriel-vasile/mimetype v1.4.11 + github.com/gabriel-vasile/mimetype v1.4.13 github.com/gdamore/tcell/v2 v2.13.8 github.com/golang/snappy v1.0.0 github.com/google/go-cmp v0.7.0 @@ -54,7 +54,7 @@ require ( golang.org/x/sync v0.19.0 golang.org/x/term v0.40.0 golang.org/x/text v0.34.0 - google.golang.org/grpc v1.78.0 + google.golang.org/grpc v1.79.1 google.golang.org/protobuf v1.36.11 gopkg.in/h2non/gock.v1 v1.1.2 gopkg.in/yaml.v3 v3.0.1 @@ -72,6 +72,7 @@ require ( github.com/aymerick/douceur v0.2.0 // indirect github.com/blang/semver v3.5.1+incompatible // indirect github.com/catppuccin/go v0.3.0 // indirect + github.com/cespare/xxhash/v2 v2.3.0 // indirect github.com/charmbracelet/bubbles v0.21.1-0.20250623103423-23b8fd6302d7 // indirect github.com/charmbracelet/bubbletea v1.3.10 // indirect github.com/charmbracelet/colorprofile v0.3.1 // indirect @@ -173,9 +174,9 @@ require ( github.com/yuin/goldmark-emoji v1.0.6 // indirect go.mongodb.org/mongo-driver v1.17.6 // indirect go.opentelemetry.io/auto/sdk v1.2.1 // indirect - go.opentelemetry.io/otel v1.38.0 // indirect - go.opentelemetry.io/otel/metric v1.38.0 // indirect - go.opentelemetry.io/otel/trace v1.38.0 // indirect + go.opentelemetry.io/otel v1.39.0 // indirect + go.opentelemetry.io/otel/metric v1.39.0 // indirect + go.opentelemetry.io/otel/trace v1.39.0 // indirect go.yaml.in/yaml/v3 v3.0.4 // indirect golang.org/x/mod v0.32.0 // indirect golang.org/x/net v0.49.0 // indirect diff --git a/go.sum b/go.sum index e9460e53b..576d84c65 100644 --- a/go.sum +++ b/go.sum @@ -102,6 +102,8 @@ github.com/cenkalti/backoff/v4 v4.3.0 h1:MyRJ/UdXutAwSAT+s3wNd7MfTIcy71VQueUuFK3 github.com/cenkalti/backoff/v4 v4.3.0/go.mod h1:Y3VNntkOUPxTVeUxJ/G5vcM//AlwfmyYozVcomhLiZE= github.com/cenkalti/backoff/v5 v5.0.3 h1:ZN+IMa753KfX5hd8vVaMixjnqRZ3y8CuJKRKj1xcsSM= github.com/cenkalti/backoff/v5 v5.0.3/go.mod h1:rkhZdG3JZukswDf7f0cwqPNk4K0sa+F97BxZthm/crw= +github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs= +github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= github.com/charmbracelet/bubbles v0.21.1-0.20250623103423-23b8fd6302d7 h1:JFgG/xnwFfbezlUnFMJy0nusZvytYysV4SCS2cYbvws= github.com/charmbracelet/bubbles v0.21.1-0.20250623103423-23b8fd6302d7/go.mod h1:ISC1gtLcVilLOf23wvTfoQuYbW2q0JevFxPfUzZ9Ybw= github.com/charmbracelet/bubbletea v1.3.10 h1:otUDHWMMzQSB0Pkc87rm691KZ3SWa4KUlvF9nRvCICw= @@ -193,8 +195,8 @@ github.com/felixge/httpsnoop v1.0.4 h1:NFTV2Zj1bL4mc9sqWACXbQFVBBg2W3GPvqp8/ESS2 github.com/felixge/httpsnoop v1.0.4/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U= github.com/frankban/quicktest v1.14.6 h1:7Xjx+VpznH+oBnejlPUj8oUpdxnVs4f8XU8WnHkI4W8= github.com/frankban/quicktest v1.14.6/go.mod h1:4ptaffx2x8+WTWXmUCuVU6aPUX1/Mz7zb5vbUoiM6w0= -github.com/gabriel-vasile/mimetype v1.4.11 h1:AQvxbp830wPhHTqc1u7nzoLT+ZFxGY7emj5DR5DYFik= -github.com/gabriel-vasile/mimetype v1.4.11/go.mod h1:d+9Oxyo1wTzWdyVUPMmXFvp4F9tea18J8ufA774AB3s= +github.com/gabriel-vasile/mimetype v1.4.13 h1:46nXokslUBsAJE/wMsp5gtO500a4F3Nkz9Ufpk2AcUM= +github.com/gabriel-vasile/mimetype v1.4.13/go.mod h1:d+9Oxyo1wTzWdyVUPMmXFvp4F9tea18J8ufA774AB3s= github.com/gdamore/encoding v1.0.1 h1:YzKZckdBL6jVt2Gc+5p82qhrGiqMdG/eNs6Wy0u3Uhw= github.com/gdamore/encoding v1.0.1/go.mod h1:0Z0cMFinngz9kS1QfMjCP8TY7em3bZYeeklsSDPivEo= github.com/gdamore/tcell/v2 v2.13.8 h1:Mys/Kl5wfC/GcC5Cx4C2BIQH9dbnhnkPgS9/wF3RlfU= @@ -548,16 +550,16 @@ go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.6 go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.63.0/go.mod h1:fvPi2qXDqFs8M4B4fmJhE92TyQs9Ydjlg3RvfUp+NbQ= go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.63.0 h1:RbKq8BG0FI8OiXhBfcRtqqHcZcka+gU3cskNuf05R18= go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.63.0/go.mod h1:h06DGIukJOevXaj/xrNjhi/2098RZzcLTbc0jDAUbsg= -go.opentelemetry.io/otel v1.38.0 h1:RkfdswUDRimDg0m2Az18RKOsnI8UDzppJAtj01/Ymk8= -go.opentelemetry.io/otel v1.38.0/go.mod h1:zcmtmQ1+YmQM9wrNsTGV/q/uyusom3P8RxwExxkZhjM= -go.opentelemetry.io/otel/metric v1.38.0 h1:Kl6lzIYGAh5M159u9NgiRkmoMKjvbsKtYRwgfrA6WpA= -go.opentelemetry.io/otel/metric v1.38.0/go.mod h1:kB5n/QoRM8YwmUahxvI3bO34eVtQf2i4utNVLr9gEmI= -go.opentelemetry.io/otel/sdk v1.38.0 h1:l48sr5YbNf2hpCUj/FoGhW9yDkl+Ma+LrVl8qaM5b+E= -go.opentelemetry.io/otel/sdk v1.38.0/go.mod h1:ghmNdGlVemJI3+ZB5iDEuk4bWA3GkTpW+DOoZMYBVVg= -go.opentelemetry.io/otel/sdk/metric v1.38.0 h1:aSH66iL0aZqo//xXzQLYozmWrXxyFkBJ6qT5wthqPoM= -go.opentelemetry.io/otel/sdk/metric v1.38.0/go.mod h1:dg9PBnW9XdQ1Hd6ZnRz689CbtrUp0wMMs9iPcgT9EZA= -go.opentelemetry.io/otel/trace v1.38.0 h1:Fxk5bKrDZJUH+AMyyIXGcFAPah0oRcT+LuNtJrmcNLE= -go.opentelemetry.io/otel/trace v1.38.0/go.mod h1:j1P9ivuFsTceSWe1oY+EeW3sc+Pp42sO++GHkg4wwhs= +go.opentelemetry.io/otel v1.39.0 h1:8yPrr/S0ND9QEfTfdP9V+SiwT4E0G7Y5MO7p85nis48= +go.opentelemetry.io/otel v1.39.0/go.mod h1:kLlFTywNWrFyEdH0oj2xK0bFYZtHRYUdv1NklR/tgc8= +go.opentelemetry.io/otel/metric v1.39.0 h1:d1UzonvEZriVfpNKEVmHXbdf909uGTOQjA0HF0Ls5Q0= +go.opentelemetry.io/otel/metric v1.39.0/go.mod h1:jrZSWL33sD7bBxg1xjrqyDjnuzTUB0x1nBERXd7Ftcs= +go.opentelemetry.io/otel/sdk v1.39.0 h1:nMLYcjVsvdui1B/4FRkwjzoRVsMK8uL/cj0OyhKzt18= +go.opentelemetry.io/otel/sdk v1.39.0/go.mod h1:vDojkC4/jsTJsE+kh+LXYQlbL8CgrEcwmt1ENZszdJE= +go.opentelemetry.io/otel/sdk/metric v1.39.0 h1:cXMVVFVgsIf2YL6QkRF4Urbr/aMInf+2WKg+sEJTtB8= +go.opentelemetry.io/otel/sdk/metric v1.39.0/go.mod h1:xq9HEVH7qeX69/JnwEfp6fVq5wosJsY1mt4lLfYdVew= +go.opentelemetry.io/otel/trace v1.39.0 h1:2d2vfpEDmCJ5zVYz7ijaJdOF59xLomrvj7bjt6/qCJI= +go.opentelemetry.io/otel/trace v1.39.0/go.mod h1:88w4/PnZSazkGzz/w84VHpQafiU4EtqqlVdxWy+rNOA= go.step.sm/crypto v0.74.0 h1:/APBEv45yYR4qQFg47HA8w1nesIGcxh44pGyQNw6JRA= go.step.sm/crypto v0.74.0/go.mod h1:UoXqCAJjjRgzPte0Llaqen7O9P7XjPmgjgTHQGkKCDk= go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto= @@ -638,8 +640,8 @@ google.golang.org/genproto/googleapis/api v0.0.0-20251202230838-ff82c1b0f217 h1: google.golang.org/genproto/googleapis/api v0.0.0-20251202230838-ff82c1b0f217/go.mod h1:+rXWjjaukWZun3mLfjmVnQi18E1AsFbDN9QdJ5YXLto= google.golang.org/genproto/googleapis/rpc v0.0.0-20251222181119-0a764e51fe1b h1:Mv8VFug0MP9e5vUxfBcE3vUkV6CImK3cMNMIDFjmzxU= google.golang.org/genproto/googleapis/rpc v0.0.0-20251222181119-0a764e51fe1b/go.mod h1:j9x/tPzZkyxcgEFkiKEEGxfvyumM01BEtsW8xzOahRQ= -google.golang.org/grpc v1.78.0 h1:K1XZG/yGDJnzMdd/uZHAkVqJE+xIDOcmdSFZkBUicNc= -google.golang.org/grpc v1.78.0/go.mod h1:I47qjTo4OKbMkjA/aOOwxDIiPSBofUtQUI5EfpWvW7U= +google.golang.org/grpc v1.79.1 h1:zGhSi45ODB9/p3VAawt9a+O/MULLl9dpizzNNpq7flY= +google.golang.org/grpc v1.79.1/go.mod h1:KmT0Kjez+0dde/v2j9vzwoAScgEPx/Bw1CYChhHLrHQ= google.golang.org/protobuf v1.36.11 h1:fV6ZwhNocDyBLK0dj+fg8ektcVegBBuEolpbTQyBNVE= google.golang.org/protobuf v1.36.11/go.mod h1:HTf+CrKn2C3g5S8VImy6tdcUvCska2kB7j23XfzDpco= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= diff --git a/internal/licenses/embed/darwin-amd64/PLACEHOLDER b/internal/licenses/embed/darwin-amd64/PLACEHOLDER new file mode 100644 index 000000000..e69de29bb diff --git a/internal/licenses/embed/darwin-arm64/PLACEHOLDER b/internal/licenses/embed/darwin-arm64/PLACEHOLDER new file mode 100644 index 000000000..e69de29bb diff --git a/internal/licenses/embed/linux-386/PLACEHOLDER b/internal/licenses/embed/linux-386/PLACEHOLDER new file mode 100644 index 000000000..e69de29bb diff --git a/internal/licenses/embed/linux-amd64/PLACEHOLDER b/internal/licenses/embed/linux-amd64/PLACEHOLDER new file mode 100644 index 000000000..e69de29bb diff --git a/internal/licenses/embed/linux-arm/PLACEHOLDER b/internal/licenses/embed/linux-arm/PLACEHOLDER new file mode 100644 index 000000000..e69de29bb diff --git a/internal/licenses/embed/linux-arm64/PLACEHOLDER b/internal/licenses/embed/linux-arm64/PLACEHOLDER new file mode 100644 index 000000000..e69de29bb diff --git a/internal/licenses/embed/report.txt b/internal/licenses/embed/report.txt deleted file mode 100644 index 002dfeeec..000000000 --- a/internal/licenses/embed/report.txt +++ /dev/null @@ -1 +0,0 @@ -License information is only available in official release builds. diff --git a/internal/licenses/embed/third-party/PLACEHOLDER b/internal/licenses/embed/third-party/PLACEHOLDER deleted file mode 100644 index 48cdce852..000000000 --- a/internal/licenses/embed/third-party/PLACEHOLDER +++ /dev/null @@ -1 +0,0 @@ -placeholder diff --git a/internal/licenses/embed/windows-386/PLACEHOLDER b/internal/licenses/embed/windows-386/PLACEHOLDER new file mode 100644 index 000000000..e69de29bb diff --git a/internal/licenses/embed/windows-amd64/PLACEHOLDER b/internal/licenses/embed/windows-amd64/PLACEHOLDER new file mode 100644 index 000000000..e69de29bb diff --git a/internal/licenses/embed/windows-arm64/PLACEHOLDER b/internal/licenses/embed/windows-arm64/PLACEHOLDER new file mode 100644 index 000000000..e69de29bb diff --git a/internal/licenses/embed_darwin_amd64.go b/internal/licenses/embed_darwin_amd64.go new file mode 100644 index 000000000..9da7398c6 --- /dev/null +++ b/internal/licenses/embed_darwin_amd64.go @@ -0,0 +1,8 @@ +package licenses + +import "embed" + +const rootDir = "embed/darwin-amd64" + +//go:embed all:embed/darwin-amd64 +var embedFS embed.FS diff --git a/internal/licenses/embed_darwin_arm64.go b/internal/licenses/embed_darwin_arm64.go new file mode 100644 index 000000000..844a51ab9 --- /dev/null +++ b/internal/licenses/embed_darwin_arm64.go @@ -0,0 +1,8 @@ +package licenses + +import "embed" + +const rootDir = "embed/darwin-arm64" + +//go:embed all:embed/darwin-arm64 +var embedFS embed.FS diff --git a/internal/licenses/embed_default.go b/internal/licenses/embed_default.go new file mode 100644 index 000000000..387f4285f --- /dev/null +++ b/internal/licenses/embed_default.go @@ -0,0 +1,15 @@ +// This file is necessary to allow building on platforms that we do not have +// official release builds for. Without this, `go build` or `go install` calls +// would fail due to undefined symbols that are expected to be included in the +// build. + +//go:build !(darwin && (amd64 || arm64)) && !(linux && (386 || amd64 || arm || arm64)) && !(windows && (386 || amd64 || arm64)) + +package licenses + +import "embed" + +const rootDir = "" + +// embedFS is left empty to indicate there's no embedded content. +var embedFS embed.FS diff --git a/internal/licenses/embed_linux_386.go b/internal/licenses/embed_linux_386.go new file mode 100644 index 000000000..f6f34313e --- /dev/null +++ b/internal/licenses/embed_linux_386.go @@ -0,0 +1,8 @@ +package licenses + +import "embed" + +const rootDir = "embed/linux-386" + +//go:embed all:embed/linux-386 +var embedFS embed.FS diff --git a/internal/licenses/embed_linux_amd64.go b/internal/licenses/embed_linux_amd64.go new file mode 100644 index 000000000..8c944d613 --- /dev/null +++ b/internal/licenses/embed_linux_amd64.go @@ -0,0 +1,8 @@ +package licenses + +import "embed" + +const rootDir = "embed/linux-amd64" + +//go:embed all:embed/linux-amd64 +var embedFS embed.FS diff --git a/internal/licenses/embed_linux_arm.go b/internal/licenses/embed_linux_arm.go new file mode 100644 index 000000000..61ba21d7d --- /dev/null +++ b/internal/licenses/embed_linux_arm.go @@ -0,0 +1,8 @@ +package licenses + +import "embed" + +const rootDir = "embed/linux-arm" + +//go:embed all:embed/linux-arm +var embedFS embed.FS diff --git a/internal/licenses/embed_linux_arm64.go b/internal/licenses/embed_linux_arm64.go new file mode 100644 index 000000000..99013dc98 --- /dev/null +++ b/internal/licenses/embed_linux_arm64.go @@ -0,0 +1,8 @@ +package licenses + +import "embed" + +const rootDir = "embed/linux-arm64" + +//go:embed all:embed/linux-arm64 +var embedFS embed.FS diff --git a/internal/licenses/embed_windows_386.go b/internal/licenses/embed_windows_386.go new file mode 100644 index 000000000..1976ab9f1 --- /dev/null +++ b/internal/licenses/embed_windows_386.go @@ -0,0 +1,8 @@ +package licenses + +import "embed" + +const rootDir = "embed/windows-386" + +//go:embed all:embed/windows-386 +var embedFS embed.FS diff --git a/internal/licenses/embed_windows_amd64.go b/internal/licenses/embed_windows_amd64.go new file mode 100644 index 000000000..3e9fb0b5d --- /dev/null +++ b/internal/licenses/embed_windows_amd64.go @@ -0,0 +1,8 @@ +package licenses + +import "embed" + +const rootDir = "embed/windows-amd64" + +//go:embed all:embed/windows-amd64 +var embedFS embed.FS diff --git a/internal/licenses/embed_windows_arm64.go b/internal/licenses/embed_windows_arm64.go new file mode 100644 index 000000000..4afd13825 --- /dev/null +++ b/internal/licenses/embed_windows_arm64.go @@ -0,0 +1,8 @@ +package licenses + +import "embed" + +const rootDir = "embed/windows-arm64" + +//go:embed all:embed/windows-arm64 +var embedFS embed.FS diff --git a/internal/licenses/licenses.go b/internal/licenses/licenses.go index 09c8058f7..1499a0722 100644 --- a/internal/licenses/licenses.go +++ b/internal/licenses/licenses.go @@ -1,28 +1,31 @@ package licenses import ( - "embed" "fmt" "io/fs" - "path/filepath" + "path" "sort" "strings" ) -//go:embed embed/report.txt -var report string - -//go:embed all:embed/third-party -var thirdParty embed.FS - +// Content returns the full license report, including the main report and all +// third-party licenses. func Content() string { - return content(report, thirdParty, "embed/third-party") + return content(embedFS, rootDir) } -func content(report string, thirdPartyFS fs.ReadFileFS, root string) string { +func content(embedFS fs.ReadFileFS, rootDir string) string { var b strings.Builder - b.WriteString(report) + reportPath := path.Join(rootDir, "report.txt") + thirdPartyPath := path.Join(rootDir, "third-party") + + report, err := fs.ReadFile(embedFS, reportPath) + if err != nil { + return "License information is only available in official release builds.\n" + } + + b.Write(report) b.WriteString("\n") // Walk the third-party directory and output each license/notice file @@ -32,8 +35,13 @@ func content(report string, thirdPartyFS fs.ReadFileFS, root string) string { files []string } + thirdPartyFS, err := fs.Sub(embedFS, thirdPartyPath) + if err != nil { + return b.String() + } + modules := map[string]*moduleFiles{} - fs.WalkDir(thirdPartyFS, root, func(filePath string, d fs.DirEntry, err error) error { + fs.WalkDir(thirdPartyFS, ".", func(filePath string, d fs.DirEntry, err error) error { if err != nil { return fmt.Errorf("failed to read embedded file %s: %w", filePath, err) } @@ -42,18 +50,11 @@ func content(report string, thirdPartyFS fs.ReadFileFS, root string) string { return nil } - name := d.Name() - if name == "PLACEHOLDER" { - return nil + dir := path.Dir(filePath) + if _, ok := modules[dir]; !ok { + modules[dir] = &moduleFiles{path: dir} } - - // Module path is the directory relative to root - dir := filepath.Dir(filepath.FromSlash(filePath)) - rel, _ := filepath.Rel(filepath.FromSlash(root), dir) - if _, ok := modules[rel]; !ok { - modules[rel] = &moduleFiles{path: rel} - } - modules[rel].files = append(modules[rel].files, filePath) + modules[dir].files = append(modules[dir].files, filePath) return nil }) @@ -71,7 +72,7 @@ func content(report string, thirdPartyFS fs.ReadFileFS, root string) string { b.WriteString("================================================================================\n\n") for _, filePath := range mod.files { - data, err := thirdPartyFS.ReadFile(filePath) + data, err := fs.ReadFile(thirdPartyFS, filePath) if err != nil { continue } diff --git a/internal/licenses/licenses_test.go b/internal/licenses/licenses_test.go index b3b81d666..befb03e5f 100644 --- a/internal/licenses/licenses_test.go +++ b/internal/licenses/licenses_test.go @@ -1,85 +1,160 @@ package licenses import ( - "path/filepath" - "strings" + "io/fs" "testing" "testing/fstest" + "github.com/MakeNowJust/heredoc" "github.com/stretchr/testify/require" ) -func TestContent_reportOnly(t *testing.T) { - report := "dep1 (v1.0.0) - MIT - https://example.com\n" - fsys := fstest.MapFS{ - "third-party/PLACEHOLDER": &fstest.MapFile{Data: []byte("placeholder")}, - } - - actualContent := content(report, fsys, "third-party") - - require.True(t, strings.HasPrefix(actualContent, report), "expected output to start with report") - require.NotContains(t, actualContent, "PLACEHOLDER") - require.NotContains(t, actualContent, "====") +func TestContent(t *testing.T) { + // This test is to ensure that we don't accidentally commit actual license + // files in the repo. The embedded content is only included in release builds, + // so in a normal test build we should get a default message. + require.Equal(t, "License information is only available in official release builds.\n", Content()) } -func TestContent_singleModule(t *testing.T) { - report := "example.com/mod (v1.0.0) - MIT - https://example.com\n" - fsys := fstest.MapFS{ - "third-party/example.com/mod/LICENSE": &fstest.MapFile{ - Data: []byte("MIT License\n\nCopyright (c) 2024"), +func TestContent_tableTests(t *testing.T) { + tests := []struct { + name string + fsys fstest.MapFS + expected string + }{ + { + name: "report only", + fsys: fstest.MapFS{ + "embed/os-arch/PLACEHOLDER": &fstest.MapFile{}, // Checked-in placeholder, so it's always there. + "embed/os-arch/report.txt": &fstest.MapFile{Data: []byte("dep1 (v1.0.0) - MIT - https://example.com\n")}, + }, + expected: heredoc.Doc(` + dep1 (v1.0.0) - MIT - https://example.com + + `), + }, + { + name: "empty third-party dir", + fsys: fstest.MapFS{ + "embed/os-arch/PLACEHOLDER": &fstest.MapFile{}, // Checked-in placeholder, so it's always there. + "embed/os-arch/report.txt": &fstest.MapFile{Data: []byte("dep1 (v1.0.0) - MIT - https://example.com\n")}, + "embed/os-arch/third-party": &fstest.MapFile{Data: []byte{}, Mode: fs.ModeDir}, + }, + expected: heredoc.Doc(` + dep1 (v1.0.0) - MIT - https://example.com + + `), + }, + { + name: "unknown file at root ignored", + fsys: fstest.MapFS{ + "embed/os-arch/PLACEHOLDER": &fstest.MapFile{}, // Checked-in placeholder, so it's always there. + "embed/os-arch/report.txt": &fstest.MapFile{Data: []byte("dep1 (v1.0.0) - MIT - https://example.com\n")}, + "embed/os-arch/unknown": &fstest.MapFile{ + Data: []byte("MIT License\n\nCopyright (c) 2024"), + }, + }, + expected: heredoc.Doc(` + dep1 (v1.0.0) - MIT - https://example.com + + `), + }, + { + name: "unknown directory at root ignored", + fsys: fstest.MapFS{ + "embed/os-arch/PLACEHOLDER": &fstest.MapFile{}, // Checked-in placeholder, so it's always there. + "embed/os-arch/report.txt": &fstest.MapFile{Data: []byte("dep1 (v1.0.0) - MIT - https://example.com\n")}, + "embed/os-arch/unknown/example.com/mod/LICENSE": &fstest.MapFile{ + Data: []byte("MIT License\n\nCopyright (c) 2024"), + }, + }, + expected: heredoc.Doc(` + dep1 (v1.0.0) - MIT - https://example.com + + `), + }, + { + name: "single module", + fsys: fstest.MapFS{ + "embed/os-arch/PLACEHOLDER": &fstest.MapFile{}, // Checked-in placeholder, so it's always there. + "embed/os-arch/report.txt": &fstest.MapFile{Data: []byte("example.com/mod (v1.0.0) - MIT - https://example.com\n")}, + "embed/os-arch/third-party/example.com/mod/LICENSE": &fstest.MapFile{ + Data: []byte("MIT License\n\nCopyright (c) 2024"), + }, + }, + expected: heredoc.Doc(` + example.com/mod (v1.0.0) - MIT - https://example.com + + ================================================================================ + example.com/mod + ================================================================================ + + MIT License + + Copyright (c) 2024 + + `), + }, + { + name: "multiple modules sorted alphabetically", + fsys: fstest.MapFS{ + "embed/os-arch/PLACEHOLDER": &fstest.MapFile{}, // Checked-in placeholder, so it's always there. + "embed/os-arch/report.txt": &fstest.MapFile{Data: []byte("example.com/mod (v1.0.0) - MIT - https://example.com\n")}, + "embed/os-arch/third-party/github.com/zzz/pkg/LICENSE": &fstest.MapFile{ + Data: []byte("ZZZ License"), + }, + "embed/os-arch/third-party/github.com/aaa/pkg/LICENSE": &fstest.MapFile{ + Data: []byte("AAA License"), + }, + }, + expected: heredoc.Doc(` + example.com/mod (v1.0.0) - MIT - https://example.com + + ================================================================================ + github.com/aaa/pkg + ================================================================================ + + AAA License + + ================================================================================ + github.com/zzz/pkg + ================================================================================ + + ZZZ License + + `), + }, + { + name: "license and notice files", + fsys: fstest.MapFS{ + "embed/os-arch/PLACEHOLDER": &fstest.MapFile{}, // Checked-in placeholder, so it's always there. + "embed/os-arch/report.txt": &fstest.MapFile{Data: []byte("example.com/mod (v1.0.0) - MIT - https://example.com\n")}, + "embed/os-arch/third-party/example.com/mod/LICENSE": &fstest.MapFile{ + Data: []byte("Apache License 2.0"), + }, + "embed/os-arch/third-party/example.com/mod/NOTICE": &fstest.MapFile{ + Data: []byte("Copyright 2024 Example Corp"), + }, + }, + expected: heredoc.Doc(` + example.com/mod (v1.0.0) - MIT - https://example.com + + ================================================================================ + example.com/mod + ================================================================================ + + Apache License 2.0 + + Copyright 2024 Example Corp + + `), }, } - actualContent := content(report, fsys, "third-party") - - require.Contains(t, actualContent, filepath.FromSlash("example.com/mod")) - require.Contains(t, actualContent, "MIT License") -} - -func TestContent_multipleModulesSortedAlphabetically(t *testing.T) { - report := "header\n" - fsys := fstest.MapFS{ - "third-party/github.com/zzz/pkg/LICENSE": &fstest.MapFile{ - Data: []byte("ZZZ License"), - }, - "third-party/github.com/aaa/pkg/LICENSE": &fstest.MapFile{ - Data: []byte("AAA License"), - }, + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := content(tt.fsys, "embed/os-arch") + require.Equal(t, tt.expected, got) + }) } - - actualContent := content(report, fsys, "third-party") - - aIdx := strings.Index(actualContent, filepath.FromSlash("github.com/aaa/pkg")) - zIdx := strings.Index(actualContent, filepath.FromSlash("github.com/zzz/pkg")) - require.NotEqual(t, -1, aIdx, "expected aaa module in output") - require.NotEqual(t, -1, zIdx, "expected zzz module in output") - require.Less(t, aIdx, zIdx, "expected modules to be sorted alphabetically") -} - -func TestContent_licenseAndNoticeFiles(t *testing.T) { - report := "header\n" - fsys := fstest.MapFS{ - "third-party/example.com/mod/LICENSE": &fstest.MapFile{ - Data: []byte("Apache License 2.0"), - }, - "third-party/example.com/mod/NOTICE": &fstest.MapFile{ - Data: []byte("Copyright 2024 Example Corp"), - }, - } - - actualContent := content(report, fsys, "third-party") - - require.Contains(t, actualContent, "Apache License 2.0") - require.Contains(t, actualContent, "Copyright 2024 Example Corp") -} - -func TestContent_emptyThirdPartyDir(t *testing.T) { - report := "header\n" - fsys := fstest.MapFS{ - "third-party/empty": &fstest.MapFile{Data: []byte("")}, - } - - actualContent := content(report, fsys, "third-party") - - require.True(t, strings.HasPrefix(actualContent, "header\n"), "expected output to start with report header") } diff --git a/pkg/cmd/agent-task/capi/sessions.go b/pkg/cmd/agent-task/capi/sessions.go index 8e3969d69..4b457d799 100644 --- a/pkg/cmd/agent-task/capi/sessions.go +++ b/pkg/cmd/agent-task/capi/sessions.go @@ -102,6 +102,94 @@ type SessionError struct { Message string } +// SessionFields defines the available fields for JSON export of a Session. +var SessionFields = []string{ + "id", + "name", + "state", + "repository", + "user", + "createdAt", + "updatedAt", + "completedAt", + "pullRequestNumber", + "pullRequestUrl", + "pullRequestTitle", + "pullRequestState", +} + +// ExportData implements the exportable interface for JSON output. +func (s *Session) ExportData(fields []string) map[string]interface{} { + data := make(map[string]interface{}, len(fields)) + for _, f := range fields { + switch f { + case "id": + data[f] = s.ID + case "name": + data[f] = s.Name + case "state": + data[f] = s.State + case "repository": + if s.PullRequest != nil && s.PullRequest.Repository != nil { + data[f] = s.PullRequest.Repository.NameWithOwner + } else { + data[f] = nil + } + case "user": + if s.User != nil { + data[f] = s.User.Login + } else { + data[f] = nil + } + case "createdAt": + if s.CreatedAt.IsZero() { + data[f] = nil + } else { + data[f] = s.CreatedAt + } + case "updatedAt": + if s.LastUpdatedAt.IsZero() { + data[f] = nil + } else { + data[f] = s.LastUpdatedAt + } + case "completedAt": + if s.CompletedAt.IsZero() { + data[f] = nil + } else { + data[f] = s.CompletedAt + } + case "pullRequestNumber": + if s.PullRequest != nil { + data[f] = s.PullRequest.Number + } else { + data[f] = nil + } + case "pullRequestUrl": + if s.PullRequest != nil { + data[f] = s.PullRequest.URL + } else { + data[f] = nil + } + case "pullRequestTitle": + if s.PullRequest != nil { + data[f] = s.PullRequest.Title + } else { + data[f] = nil + } + case "pullRequestState": + if s.PullRequest != nil { + data[f] = s.PullRequest.State + } else { + data[f] = nil + } + default: + data[f] = nil + } + } + return data +} + type resource struct { ID string `json:"id"` UserID uint64 `json:"user_id"` diff --git a/pkg/cmd/agent-task/list/list.go b/pkg/cmd/agent-task/list/list.go index 211dc07ea..559389b5c 100644 --- a/pkg/cmd/agent-task/list/list.go +++ b/pkg/cmd/agent-task/list/list.go @@ -25,6 +25,7 @@ type ListOptions struct { CapiClient func() (capi.CapiClient, error) Web bool Browser browser.Browser + Exporter cmdutil.Exporter } // NewCmdList creates the list command @@ -54,6 +55,8 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman cmd.Flags().IntVarP(&opts.Limit, "limit", "L", defaultLimit, "Maximum number of agent tasks to fetch") cmd.Flags().BoolVarP(&opts.Web, "web", "w", false, "Open agent tasks in the browser") + cmdutil.AddJSONFlags(cmd, &opts.Exporter, capi.SessionFields) + return cmd } @@ -87,10 +90,14 @@ func listRun(opts *ListOptions) error { opts.IO.StopProgressIndicator() - if len(sessions) == 0 { + if len(sessions) == 0 && opts.Exporter == nil { return cmdutil.NewNoResultsError("no agent tasks found") } + if opts.Exporter != nil { + return opts.Exporter.Write(opts.IO, sessions) + } + if err := opts.IO.StartPager(); err == nil { defer opts.IO.StopPager() } else { diff --git a/pkg/cmd/agent-task/list/list_test.go b/pkg/cmd/agent-task/list/list_test.go index 747650283..d46240b59 100644 --- a/pkg/cmd/agent-task/list/list_test.go +++ b/pkg/cmd/agent-task/list/list_test.go @@ -99,6 +99,7 @@ func Test_listRun(t *testing.T) { capiStubs func(*testing.T, *capi.CapiClientMock) limit int web bool + jsonFields []string wantOut string wantErr error wantStderr string @@ -286,6 +287,68 @@ func Test_listRun(t *testing.T) { wantStderr: "Opening https://github.com/copilot/agents in your browser.\n", wantBrowserURL: "https://github.com/copilot/agents", }, + { + name: "json output", + tty: false, + capiStubs: func(t *testing.T, m *capi.CapiClientMock) { + m.ListLatestSessionsForViewerFunc = func(ctx context.Context, limit int) ([]*capi.Session, error) { + return []*capi.Session{ + { + ID: "abc-123", + Name: "s1", + State: "completed", + CreatedAt: sampleDate, + LastUpdatedAt: sampleDate, + CompletedAt: sampleDate, + ResourceType: "pull", + User: &api.GitHubUser{Login: "monalisa"}, + PullRequest: &api.PullRequest{ + Number: 101, + Title: "Fix login bug", + State: "MERGED", + URL: "https://github.com/OWNER/REPO/pull/101", + Repository: &api.PRRepository{ + NameWithOwner: "OWNER/REPO", + }, + }, + }, + }, nil + } + }, + jsonFields: []string{"id", "name", "state", "repository", "user", "pullRequestNumber", "pullRequestUrl", "pullRequestTitle", "pullRequestState"}, + wantOut: "[{\"id\":\"abc-123\",\"name\":\"s1\",\"pullRequestNumber\":101,\"pullRequestState\":\"MERGED\",\"pullRequestTitle\":\"Fix login bug\",\"pullRequestUrl\":\"https://github.com/OWNER/REPO/pull/101\",\"repository\":\"OWNER/REPO\",\"state\":\"completed\",\"user\":\"monalisa\"}]\n", + }, + { + name: "json output with no sessions returns empty array", + tty: false, + capiStubs: func(t *testing.T, m *capi.CapiClientMock) { + m.ListLatestSessionsForViewerFunc = func(ctx context.Context, limit int) ([]*capi.Session, error) { + return nil, nil + } + }, + jsonFields: []string{"id", "name", "state"}, + wantOut: "[]\n", + }, + { + name: "json output with nil pull request", + tty: false, + capiStubs: func(t *testing.T, m *capi.CapiClientMock) { + m.ListLatestSessionsForViewerFunc = func(ctx context.Context, limit int) ([]*capi.Session, error) { + return []*capi.Session{ + { + ID: "abc-456", + Name: "s2", + State: "in_progress", + CreatedAt: sampleDate, + LastUpdatedAt: sampleDate, + ResourceType: "pull", + }, + }, nil + } + }, + jsonFields: []string{"id", "name", "state", "repository", "user", "pullRequestNumber", "pullRequestUrl", "pullRequestTitle", "pullRequestState"}, + wantOut: "[{\"id\":\"abc-456\",\"name\":\"s2\",\"pullRequestNumber\":null,\"pullRequestState\":null,\"pullRequestTitle\":null,\"pullRequestUrl\":null,\"repository\":null,\"state\":\"in_progress\",\"user\":null}]\n", + }, } for _, tt := range tests { @@ -316,6 +379,12 @@ func Test_listRun(t *testing.T) { }, } + if tt.jsonFields != nil { + exporter := cmdutil.NewJSONExporter() + exporter.SetFields(tt.jsonFields) + opts.Exporter = exporter + } + err := listRun(opts) if tt.wantErr != nil { assert.Error(t, err) diff --git a/pkg/cmd/agent-task/view/view.go b/pkg/cmd/agent-task/view/view.go index 38c1e0e12..854faa73d 100644 --- a/pkg/cmd/agent-task/view/view.go +++ b/pkg/cmd/agent-task/view/view.go @@ -37,6 +37,7 @@ type ViewOptions struct { Finder prShared.PRFinder Prompter prompter.Prompter Browser browser.Browser + Exporter cmdutil.Exporter LogRenderer func() shared.LogRenderer Sleep func(d time.Duration) @@ -125,6 +126,8 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman cmd.Flags().BoolVar(&opts.Log, "log", false, "Show agent session logs") cmd.Flags().BoolVar(&opts.Follow, "follow", false, "Follow agent session logs") + cmdutil.AddJSONFlags(cmd, &opts.Exporter, capi.SessionFields) + return cmd } @@ -285,6 +288,10 @@ func viewRun(opts *ViewOptions) error { opts.IO.StopProgressIndicator() } + if opts.Exporter != nil { + return opts.Exporter.Write(opts.IO, session) + } + if opts.Log { return printLogs(opts, capiClient, session.ID) } diff --git a/pkg/cmd/agent-task/view/view_test.go b/pkg/cmd/agent-task/view/view_test.go index 68cc377e3..34036cfa5 100644 --- a/pkg/cmd/agent-task/view/view_test.go +++ b/pkg/cmd/agent-task/view/view_test.go @@ -168,6 +168,7 @@ func Test_viewRun(t *testing.T) { promptStubs func(*testing.T, *prompter.MockPrompter) capiStubs func(*testing.T, *capi.CapiClientMock) logRendererStubs func(*testing.T, *shared.LogRendererMock) + jsonFields []string wantOut string wantErr error wantStderr string @@ -1209,6 +1210,63 @@ func Test_viewRun(t *testing.T) { (rendered:) `), }, + { + name: "json output (tty)", + tty: true, + opts: ViewOptions{ + SelectorArg: "some-session-id", + SessionID: "some-session-id", + }, + capiStubs: func(t *testing.T, m *capi.CapiClientMock) { + m.GetSessionFunc = func(_ context.Context, id string) (*capi.Session, error) { + return &capi.Session{ + ID: "some-session-id", + Name: "Fix login bug", + State: "completed", + CreatedAt: sampleDate, + LastUpdatedAt: sampleDate, + CompletedAt: sampleCompletedAt, + ResourceType: "pull", + PullRequest: &api.PullRequest{ + Number: 42, + URL: "https://github.com/OWNER/REPO/pull/42", + Title: "Fix login bug", + State: "MERGED", + Repository: &api.PRRepository{ + NameWithOwner: "OWNER/REPO", + }, + }, + User: &api.GitHubUser{ + Login: "testuser", + }, + }, nil + } + }, + wantOut: "{\"id\":\"some-session-id\",\"name\":\"Fix login bug\",\"pullRequestNumber\":42,\"pullRequestState\":\"MERGED\",\"pullRequestTitle\":\"Fix login bug\",\"pullRequestUrl\":\"https://github.com/OWNER/REPO/pull/42\",\"repository\":\"OWNER/REPO\",\"state\":\"completed\",\"user\":\"testuser\"}\n", + jsonFields: []string{"id", "name", "state", "repository", "user", "pullRequestNumber", "pullRequestUrl", "pullRequestTitle", "pullRequestState"}, + }, + { + name: "json output with nil pull request", + tty: false, + opts: ViewOptions{ + SelectorArg: "some-session-id", + SessionID: "some-session-id", + }, + capiStubs: func(t *testing.T, m *capi.CapiClientMock) { + m.GetSessionFunc = func(_ context.Context, id string) (*capi.Session, error) { + return &capi.Session{ + ID: "some-session-id", + Name: "New task", + State: "in_progress", + CreatedAt: sampleDate, + LastUpdatedAt: sampleDate, + ResourceType: "pull", + }, nil + } + }, + wantOut: "{\"id\":\"some-session-id\",\"name\":\"New task\",\"pullRequestNumber\":null,\"pullRequestUrl\":null,\"repository\":null,\"state\":\"in_progress\",\"user\":null}\n", + jsonFields: []string{"id", "name", "state", "repository", "user", "pullRequestNumber", "pullRequestUrl"}, + }, } for _, tt := range tests { @@ -1244,6 +1302,12 @@ func Test_viewRun(t *testing.T) { return logRenderer } + if tt.jsonFields != nil { + exporter := cmdutil.NewJSONExporter() + exporter.SetFields(tt.jsonFields) + opts.Exporter = exporter + } + err := viewRun(&opts) if tt.wantErr != nil { assert.Error(t, err) diff --git a/pkg/cmd/api/api.go b/pkg/cmd/api/api.go index 53c07f3f0..8e4b2edd4 100644 --- a/pkg/cmd/api/api.go +++ b/pkg/cmd/api/api.go @@ -456,6 +456,8 @@ func apiRun(opts *ApiOptions) error { return tmpl.Flush() } +var jsonContentTypeRE = regexp.MustCompile(`[/+]json(;|$)`) + func processResponse(resp *http.Response, opts *ApiOptions, bodyWriter, headersWriter io.Writer, template *template.Template, isFirstPage, isLastPage bool) (endCursor string, err error) { if opts.ShowResponseHeaders { fmt.Fprintln(headersWriter, resp.Proto, resp.Status) @@ -469,7 +471,7 @@ func processResponse(resp *http.Response, opts *ApiOptions, bodyWriter, headersW var responseBody io.Reader = resp.Body defer resp.Body.Close() - isJSON, _ := regexp.MatchString(`[/+]json(;|$)`, resp.Header.Get("Content-Type")) + isJSON := jsonContentTypeRE.MatchString(resp.Header.Get("Content-Type")) var serverError string if isJSON && (opts.RequestPath == "graphql" || resp.StatusCode >= 400) { diff --git a/pkg/cmd/browse/browse.go b/pkg/cmd/browse/browse.go index a85b8ab7d..d05713956 100644 --- a/pkg/cmd/browse/browse.go +++ b/pkg/cmd/browse/browse.go @@ -44,6 +44,7 @@ type BrowseOptions struct { SettingsFlag bool WikiFlag bool ActionsFlag bool + BlameFlag bool NoBrowserFlag bool HasRepoOverride bool } @@ -91,6 +92,9 @@ func NewCmdBrowse(f *cmdutil.Factory, runF func(*BrowseOptions) error) *cobra.Co # Open main.go at line 312 $ gh browse main.go:312 + # Open blame view for main.go at line 312 + $ gh browse main.go:312 --blame + # Open main.go with the repository at head of bug-fix branch $ gh browse main.go --branch bug-fix @@ -141,6 +145,10 @@ func NewCmdBrowse(f *cmdutil.Factory, runF func(*BrowseOptions) error) *cobra.Co return err } + if opts.BlameFlag && opts.SelectorArg == "" { + return cmdutil.FlagErrorf("`--blame` requires a file path argument") + } + if (isNumber(opts.SelectorArg) || isCommit(opts.SelectorArg)) && (opts.Branch != "" || opts.Commit != "") { return cmdutil.FlagErrorf("%q is an invalid argument when using `--branch` or `--commit`", opts.SelectorArg) } @@ -163,6 +171,7 @@ func NewCmdBrowse(f *cmdutil.Factory, runF func(*BrowseOptions) error) *cobra.Co cmd.Flags().BoolVarP(&opts.WikiFlag, "wiki", "w", false, "Open repository wiki") cmd.Flags().BoolVarP(&opts.ActionsFlag, "actions", "a", false, "Open repository actions") cmd.Flags().BoolVarP(&opts.SettingsFlag, "settings", "s", false, "Open repository settings") + cmd.Flags().BoolVar(&opts.BlameFlag, "blame", false, "Open blame view for a file") cmd.Flags().BoolVarP(&opts.NoBrowserFlag, "no-browser", "n", false, "Print destination URL instead of opening the browser") cmd.Flags().StringVarP(&opts.Commit, "commit", "c", "", "Select another commit by passing in the commit SHA, default is the last commit") cmd.Flags().StringVarP(&opts.Branch, "branch", "b", "", "Select another branch by passing in the branch name") @@ -272,9 +281,16 @@ func parseSection(baseRepo ghrepo.Interface, opts *BrowseOptions) (string, error } else { rangeFragment = fmt.Sprintf("L%d", rangeStart) } + if opts.BlameFlag { + return fmt.Sprintf("blame/%s/%s#%s", escapePath(ref), escapePath(filePath), rangeFragment), nil + } return fmt.Sprintf("blob/%s/%s?plain=1#%s", escapePath(ref), escapePath(filePath), rangeFragment), nil } + if opts.BlameFlag { + return fmt.Sprintf("blame/%s/%s", escapePath(ref), escapePath(filePath)), nil + } + return strings.TrimSuffix(fmt.Sprintf("tree/%s/%s", escapePath(ref), escapePath(filePath)), "/"), nil } diff --git a/pkg/cmd/browse/browse_test.go b/pkg/cmd/browse/browse_test.go index 6d4476f3a..c321fbdbb 100644 --- a/pkg/cmd/browse/browse_test.go +++ b/pkg/cmd/browse/browse_test.go @@ -207,6 +207,29 @@ func TestNewCmdBrowse(t *testing.T) { cli: "de07febc26e19000f8c9e821207f3bc34a3c8038 --commit=12a4", wantsErr: true, }, + { + name: "blame flag", + cli: "main.go --blame", + wants: BrowseOptions{ + BlameFlag: true, + SelectorArg: "main.go", + }, + wantsErr: false, + }, + { + name: "blame flag without file argument", + cli: "--blame", + wantsErr: true, + }, + { + name: "blame flag with line number", + cli: "main.go:312 --blame", + wants: BrowseOptions{ + BlameFlag: true, + SelectorArg: "main.go:312", + }, + wantsErr: false, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -239,6 +262,7 @@ func TestNewCmdBrowse(t *testing.T) { assert.Equal(t, tt.wants.SettingsFlag, opts.SettingsFlag) assert.Equal(t, tt.wants.ActionsFlag, opts.ActionsFlag) assert.Equal(t, tt.wants.Commit, opts.Commit) + assert.Equal(t, tt.wants.BlameFlag, opts.BlameFlag) }) } } @@ -595,6 +619,61 @@ func Test_runBrowse(t *testing.T) { expectedURL: "https://github.com/bchadwic/test/tree/trunk/77507cd94ccafcf568f8560cfecde965fcfa63e7.txt", wantsErr: false, }, + { + name: "file with blame flag", + opts: BrowseOptions{ + SelectorArg: "path/to/file.txt", + BlameFlag: true, + }, + baseRepo: ghrepo.New("owner", "repo"), + defaultBranch: "main", + expectedURL: "https://github.com/owner/repo/blame/main/path/to/file.txt", + wantsErr: false, + }, + { + name: "file with blame flag and line number", + opts: BrowseOptions{ + SelectorArg: "path/to/file.txt:42", + BlameFlag: true, + }, + baseRepo: ghrepo.New("owner", "repo"), + defaultBranch: "main", + expectedURL: "https://github.com/owner/repo/blame/main/path/to/file.txt#L42", + wantsErr: false, + }, + { + name: "file with blame flag and line range", + opts: BrowseOptions{ + SelectorArg: "path/to/file.txt:10-20", + BlameFlag: true, + }, + baseRepo: ghrepo.New("owner", "repo"), + defaultBranch: "main", + expectedURL: "https://github.com/owner/repo/blame/main/path/to/file.txt#L10-L20", + wantsErr: false, + }, + { + name: "file with blame flag and branch", + opts: BrowseOptions{ + SelectorArg: "main.go:100", + BlameFlag: true, + Branch: "feature-branch", + }, + baseRepo: ghrepo.New("owner", "repo"), + expectedURL: "https://github.com/owner/repo/blame/feature-branch/main.go#L100", + wantsErr: false, + }, + { + name: "file with blame flag and commit", + opts: BrowseOptions{ + SelectorArg: "src/app.js:50", + BlameFlag: true, + Commit: "abc123", + }, + baseRepo: ghrepo.New("owner", "repo"), + expectedURL: "https://github.com/owner/repo/blame/abc123/src/app.js#L50", + wantsErr: false, + }, } for _, tt := range tests { diff --git a/pkg/cmd/copilot/copilot.go b/pkg/cmd/copilot/copilot.go index 1195accc4..4ab840709 100644 --- a/pkg/cmd/copilot/copilot.go +++ b/pkg/cmd/copilot/copilot.go @@ -170,6 +170,7 @@ func runCopilot(opts *CopilotOptions) error { externalCmd.Stdin = opts.IO.In externalCmd.Stdout = opts.IO.Out externalCmd.Stderr = opts.IO.ErrOut + externalCmd.Env = append(os.Environ(), "COPILOT_GH=true") if err := externalCmd.Run(); err != nil { if exitErr, ok := err.(*exec.ExitError); ok { diff --git a/pkg/cmd/issue/develop/develop.go b/pkg/cmd/issue/develop/develop.go index 04bf14ebe..90c52dff6 100644 --- a/pkg/cmd/issue/develop/develop.go +++ b/pkg/cmd/issue/develop/develop.go @@ -4,6 +4,8 @@ import ( ctx "context" "fmt" "net/http" + "net/url" + "strings" "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/api" @@ -150,21 +152,22 @@ func developRun(opts *DevelopOptions) error { return err } - opts.IO.StartProgressIndicator() + opts.IO.StartProgressIndicatorWithLabel(fmt.Sprintf("Fetching issue #%d", opts.IssueNumber)) + defer opts.IO.StopProgressIndicator() + issue, err := shared.FindIssueOrPR(httpClient, baseRepo, opts.IssueNumber, []string{"id", "number"}) - opts.IO.StopProgressIndicator() if err != nil { return err } apiClient := api.NewClientFromHTTP(httpClient) - opts.IO.StartProgressIndicator() + opts.IO.StartProgressIndicatorWithLabel("Checking linked branch support") err = api.CheckLinkedBranchFeature(apiClient, baseRepo.RepoHost()) - opts.IO.StopProgressIndicator() if err != nil { return err } + opts.IO.StopProgressIndicator() if opts.List { return developRunList(opts, apiClient, baseRepo, issue) @@ -174,7 +177,6 @@ func developRun(opts *DevelopOptions) error { func developRunCreate(opts *DevelopOptions, apiClient *api.Client, issueRepo ghrepo.Interface, issue *api.Issue) error { branchRepo := issueRepo - var repoID string if opts.BranchRepo != "" { var err error branchRepo, err = ghrepo.FromFullName(opts.BranchRepo) @@ -183,24 +185,67 @@ func developRunCreate(opts *DevelopOptions, apiClient *api.Client, issueRepo ghr } } - opts.IO.StartProgressIndicator() - repoID, branchID, err := api.FindRepoBranchID(apiClient, branchRepo, opts.BaseBranch) - opts.IO.StopProgressIndicator() - if err != nil { - return err + opts.IO.StartProgressIndicatorWithLabel("Preparing linked branch") + defer opts.IO.StopProgressIndicator() + + branchName := "" + reusedExisting := false + if opts.Name != "" { + opts.IO.StartProgressIndicatorWithLabel("Checking existing linked branches") + branches, err := api.ListLinkedBranches(apiClient, issueRepo, issue.Number) + if err != nil { + return err + } + branchName = findExistingLinkedBranchName(branches, branchRepo, opts.Name) + reusedExisting = branchName != "" + } + + repoID := "" + branchID := "" + baseValidated := false + if opts.BaseBranch != "" { + opts.IO.StartProgressIndicatorWithLabel(fmt.Sprintf("Validating base branch %q", opts.BaseBranch)) + foundRepoID, foundBranchID, err := api.FindRepoBranchID(apiClient, branchRepo, opts.BaseBranch) + if err != nil { + return err + } + repoID = foundRepoID + branchID = foundBranchID + baseValidated = true + } + + if branchName == "" { + if !baseValidated { + opts.IO.StartProgressIndicatorWithLabel("Resolving base branch") + foundRepoID, foundBranchID, err := api.FindRepoBranchID(apiClient, branchRepo, opts.BaseBranch) + if err != nil { + return err + } + repoID = foundRepoID + branchID = foundBranchID + } + + opts.IO.StartProgressIndicatorWithLabel("Creating linked branch") + createdBranchName, err := api.CreateLinkedBranch(apiClient, branchRepo.RepoHost(), repoID, issue.ID, branchID, opts.Name) + if err != nil { + return err + } + branchName = createdBranchName + } + + if branchName == "" { + return fmt.Errorf("failed to create linked branch: API returned empty branch name") } - opts.IO.StartProgressIndicator() - branchName, err := api.CreateLinkedBranch(apiClient, branchRepo.RepoHost(), repoID, issue.ID, branchID, opts.Name) opts.IO.StopProgressIndicator() - if err != nil { - return err + + if reusedExisting && opts.IO.IsStdoutTTY() { + fmt.Fprintf(opts.IO.ErrOut, "Using existing linked branch %q\n", branchName) } // Remember which branch to target when creating a PR. if opts.BaseBranch != "" { - err = opts.GitClient.SetBranchConfig(ctx.Background(), branchName, git.MergeBaseConfig, opts.BaseBranch) - if err != nil { + if err := opts.GitClient.SetBranchConfig(ctx.Background(), branchName, git.MergeBaseConfig, opts.BaseBranch); err != nil { return err } } @@ -210,13 +255,44 @@ func developRunCreate(opts *DevelopOptions, apiClient *api.Client, issueRepo ghr return checkoutBranch(opts, branchRepo, branchName) } +func findExistingLinkedBranchName(branches []api.LinkedBranch, branchRepo ghrepo.Interface, branchName string) string { + for _, branch := range branches { + if branch.BranchName != branchName { + continue + } + linkedRepo, err := linkedBranchRepoFromURL(branch.URL) + if err != nil { + continue + } + if ghrepo.IsSame(linkedRepo, branchRepo) { + return branch.BranchName + } + } + return "" +} + +func linkedBranchRepoFromURL(branchURL string) (ghrepo.Interface, error) { + u, err := url.Parse(branchURL) + if err != nil { + return nil, err + } + pathParts := strings.SplitN(strings.Trim(u.Path, "/"), "/", 3) + if len(pathParts) < 2 { + return nil, fmt.Errorf("invalid linked branch URL: %q", branchURL) + } + u.Path = "/" + strings.Join(pathParts[0:2], "/") + return ghrepo.FromURL(u) +} + func developRunList(opts *DevelopOptions, apiClient *api.Client, issueRepo ghrepo.Interface, issue *api.Issue) error { - opts.IO.StartProgressIndicator() + opts.IO.StartProgressIndicatorWithLabel("Fetching linked branches") + defer opts.IO.StopProgressIndicator() + branches, err := api.ListLinkedBranches(apiClient, issueRepo, issue.Number) - opts.IO.StopProgressIndicator() if err != nil { return err } + opts.IO.StopProgressIndicator() if len(branches) == 0 { return cmdutil.NewNoResultsError(fmt.Sprintf("no linked branches found for %s#%d", ghrepo.FullName(issueRepo), issue.Number)) diff --git a/pkg/cmd/issue/develop/develop_test.go b/pkg/cmd/issue/develop/develop_test.go index 2485c8cc4..fe984df79 100644 --- a/pkg/cmd/issue/develop/develop_test.go +++ b/pkg/cmd/issue/develop/develop_test.go @@ -353,6 +353,16 @@ func TestDevelopRun(t *testing.T) { reg.Register( httpmock.GraphQL(`query FindRepoBranchID\b`), httpmock.StringResponse(`{"data":{"repository":{"id":"REPOID","ref":{"target":{"oid":"OID"}}}}}`)) + reg.Register( + httpmock.GraphQL(`query ListLinkedBranches\b`), + httpmock.GraphQLQuery(` + {"data":{"repository":{"issue":{"linkedBranches":{"nodes":[]}}}}} + `, func(query string, inputs map[string]interface{}) { + assert.Equal(t, float64(123), inputs["number"]) + assert.Equal(t, "OWNER", inputs["owner"]) + assert.Equal(t, "REPO", inputs["name"]) + }), + ) reg.Register( httpmock.GraphQL(`mutation CreateLinkedBranch\b`), httpmock.GraphQLMutation(`{"data":{"createLinkedBranch":{"linkedBranch":{"id":"2","ref":{"name":"my-branch"}}}}}`, @@ -370,6 +380,165 @@ func TestDevelopRun(t *testing.T) { }, expectedOut: "github.com/OWNER/REPO/tree/my-branch\n", }, + { + name: "develop existing linked branch with name and checkout", + opts: &DevelopOptions{ + Name: "my-branch", + BaseBranch: "main", + IssueNumber: 123, + Checkout: true, + }, + remotes: map[string]string{ + "origin": "OWNER/REPO", + }, + httpStubs: func(reg *httpmock.Registry, t *testing.T) { + reg.Register( + httpmock.GraphQL(`query LinkedBranchFeature\b`), + httpmock.StringResponse(featureEnabledPayload), + ) + reg.Register( + httpmock.GraphQL(`query IssueByNumber\b`), + httpmock.StringResponse(`{"data":{"repository":{ "hasIssuesEnabled":true,"issue":{"id":"SOMEID","number":123,"title":"my issue"}}}}`), + ) + reg.Register( + httpmock.GraphQL(`query ListLinkedBranches\b`), + httpmock.GraphQLQuery(` + {"data":{"repository":{"issue":{"linkedBranches":{"nodes":[{"ref":{"name":"my-branch","repository":{"url":"https://github.com/OWNER/REPO"}}}]}}}}} + `, func(query string, inputs map[string]interface{}) { + assert.Equal(t, float64(123), inputs["number"]) + assert.Equal(t, "OWNER", inputs["owner"]) + assert.Equal(t, "REPO", inputs["name"]) + }), + ) + reg.Register( + httpmock.GraphQL(`query FindRepoBranchID\b`), + httpmock.StringResponse(`{"data":{"repository":{"id":"REPOID","ref":{"target":{"oid":"OID"}}}}}`)) + }, + runStubs: func(cs *run.CommandStubber) { + cs.Register(`git config branch\.my-branch\.gh-merge-base main`, 0, "") + cs.Register(`git fetch origin \+refs/heads/my-branch:refs/remotes/origin/my-branch`, 0, "") + cs.Register(`git rev-parse --verify refs/heads/my-branch`, 0, "") + cs.Register(`git checkout my-branch`, 0, "") + cs.Register(`git pull --ff-only origin my-branch`, 0, "") + }, + expectedOut: "github.com/OWNER/REPO/tree/my-branch\n", + }, + { + name: "develop existing linked branch with name in tty shows reuse message", + opts: &DevelopOptions{ + Name: "my-branch", + BaseBranch: "main", + IssueNumber: 123, + }, + tty: true, + remotes: map[string]string{ + "origin": "OWNER/REPO", + }, + httpStubs: func(reg *httpmock.Registry, t *testing.T) { + reg.Register( + httpmock.GraphQL(`query LinkedBranchFeature\b`), + httpmock.StringResponse(featureEnabledPayload), + ) + reg.Register( + httpmock.GraphQL(`query IssueByNumber\b`), + httpmock.StringResponse(`{"data":{"repository":{ "hasIssuesEnabled":true,"issue":{"id":"SOMEID","number":123,"title":"my issue"}}}}`), + ) + reg.Register( + httpmock.GraphQL(`query ListLinkedBranches\b`), + httpmock.GraphQLQuery(` + {"data":{"repository":{"issue":{"linkedBranches":{"nodes":[{"ref":{"name":"my-branch","repository":{"url":"https://github.com/OWNER/REPO"}}}]}}}}} + `, func(query string, inputs map[string]interface{}) { + assert.Equal(t, float64(123), inputs["number"]) + assert.Equal(t, "OWNER", inputs["owner"]) + assert.Equal(t, "REPO", inputs["name"]) + }), + ) + reg.Register( + httpmock.GraphQL(`query FindRepoBranchID\b`), + httpmock.StringResponse(`{"data":{"repository":{"id":"REPOID","ref":{"target":{"oid":"OID"}}}}}`)) + }, + runStubs: func(cs *run.CommandStubber) { + cs.Register(`git config branch\.my-branch\.gh-merge-base main`, 0, "") + cs.Register(`git fetch origin \+refs/heads/my-branch:refs/remotes/origin/my-branch`, 0, "") + }, + expectedOut: "github.com/OWNER/REPO/tree/my-branch\n", + expectedErrOut: "Using existing linked branch \"my-branch\"\n", + }, + { + name: "develop existing linked branch with invalid base branch returns an error", + opts: &DevelopOptions{ + Name: "my-branch", + BaseBranch: "does-not-exist-branch", + IssueNumber: 123, + }, + httpStubs: func(reg *httpmock.Registry, t *testing.T) { + reg.Register( + httpmock.GraphQL(`query LinkedBranchFeature\b`), + httpmock.StringResponse(featureEnabledPayload), + ) + reg.Register( + httpmock.GraphQL(`query IssueByNumber\b`), + httpmock.StringResponse(`{"data":{"repository":{ "hasIssuesEnabled":true,"issue":{"id":"SOMEID","number":123,"title":"my issue"}}}}`), + ) + reg.Register( + httpmock.GraphQL(`query ListLinkedBranches\b`), + httpmock.GraphQLQuery(` + {"data":{"repository":{"issue":{"linkedBranches":{"nodes":[{"ref":{"name":"my-branch","repository":{"url":"https://github.com/OWNER/REPO"}}}]}}}}} + `, func(query string, inputs map[string]interface{}) { + assert.Equal(t, float64(123), inputs["number"]) + assert.Equal(t, "OWNER", inputs["owner"]) + assert.Equal(t, "REPO", inputs["name"]) + }), + ) + reg.Register( + httpmock.GraphQL(`query FindRepoBranchID\b`), + httpmock.StringResponse(`{"data":{"repository":{"id":"REPOID","defaultBranchRef":{"target":{"oid":"DEFAULTOID"}},"ref":null}}}`), + ) + }, + wantErr: `could not find branch "does-not-exist-branch" in OWNER/REPO`, + }, + { + name: "develop with empty linked branch name response returns an error", + opts: &DevelopOptions{ + Name: "my-branch", + BaseBranch: "main", + IssueNumber: 123, + }, + httpStubs: func(reg *httpmock.Registry, t *testing.T) { + reg.Register( + httpmock.GraphQL(`query LinkedBranchFeature\b`), + httpmock.StringResponse(featureEnabledPayload), + ) + reg.Register( + httpmock.GraphQL(`query IssueByNumber\b`), + httpmock.StringResponse(`{"data":{"repository":{ "hasIssuesEnabled":true,"issue":{"id":"SOMEID","number":123,"title":"my issue"}}}}`), + ) + reg.Register( + httpmock.GraphQL(`query ListLinkedBranches\b`), + httpmock.GraphQLQuery(` + {"data":{"repository":{"issue":{"linkedBranches":{"nodes":[]}}}}} + `, func(query string, inputs map[string]interface{}) { + assert.Equal(t, float64(123), inputs["number"]) + assert.Equal(t, "OWNER", inputs["owner"]) + assert.Equal(t, "REPO", inputs["name"]) + }), + ) + reg.Register( + httpmock.GraphQL(`query FindRepoBranchID\b`), + httpmock.StringResponse(`{"data":{"repository":{"id":"REPOID","ref":{"target":{"oid":"OID"}}}}}`)) + reg.Register( + httpmock.GraphQL(`mutation CreateLinkedBranch\b`), + httpmock.GraphQLMutation(`{"data":{"createLinkedBranch":{"linkedBranch":{"id":"2","ref":{"name":""}}}}}`, + func(inputs map[string]interface{}) { + assert.Equal(t, "REPOID", inputs["repositoryId"]) + assert.Equal(t, "SOMEID", inputs["issueId"]) + assert.Equal(t, "OID", inputs["oid"]) + assert.Equal(t, "my-branch", inputs["name"]) + }), + ) + }, + wantErr: "failed to create linked branch: API returned empty branch name", + }, { name: "develop new branch outside of local git repo", opts: &DevelopOptions{ @@ -426,6 +595,16 @@ func TestDevelopRun(t *testing.T) { httpmock.GraphQL(`query FindRepoBranchID\b`), httpmock.StringResponse(`{"data":{"repository":{"id":"REPOID","ref":{"target":{"oid":"OID"}}}}}`), ) + reg.Register( + httpmock.GraphQL(`query ListLinkedBranches\b`), + httpmock.GraphQLQuery(` + {"data":{"repository":{"issue":{"linkedBranches":{"nodes":[]}}}}} + `, func(query string, inputs map[string]interface{}) { + assert.Equal(t, float64(123), inputs["number"]) + assert.Equal(t, "OWNER", inputs["owner"]) + assert.Equal(t, "REPO", inputs["name"]) + }), + ) reg.Register( httpmock.GraphQL(`mutation CreateLinkedBranch\b`), httpmock.GraphQLMutation(`{"data":{"createLinkedBranch":{"linkedBranch":{"id":"2","ref":{"name":"my-branch"}}}}}`, @@ -468,6 +647,16 @@ func TestDevelopRun(t *testing.T) { httpmock.GraphQL(`query FindRepoBranchID\b`), httpmock.StringResponse(`{"data":{"repository":{"id":"REPOID","ref":{"target":{"oid":"OID"}}}}}`), ) + reg.Register( + httpmock.GraphQL(`query ListLinkedBranches\b`), + httpmock.GraphQLQuery(` + {"data":{"repository":{"issue":{"linkedBranches":{"nodes":[]}}}}} + `, func(query string, inputs map[string]interface{}) { + assert.Equal(t, float64(123), inputs["number"]) + assert.Equal(t, "OWNER", inputs["owner"]) + assert.Equal(t, "REPO", inputs["name"]) + }), + ) reg.Register( httpmock.GraphQL(`mutation CreateLinkedBranch\b`), httpmock.GraphQLMutation(`{"data":{"createLinkedBranch":{"linkedBranch":{"id":"2","ref":{"name":"my-branch"}}}}}`, diff --git a/pkg/cmd/issue/list/http.go b/pkg/cmd/issue/list/http.go index 49e316db6..0657637bf 100644 --- a/pkg/cmd/issue/list/http.go +++ b/pkg/cmd/issue/list/http.go @@ -2,6 +2,7 @@ package list import ( "fmt" + "regexp" "github.com/cli/cli/v2/api" fd "github.com/cli/cli/v2/internal/featuredetection" @@ -9,6 +10,8 @@ import ( prShared "github.com/cli/cli/v2/pkg/cmd/pr/shared" ) +var pullRequestSearchQualifierRE = regexp.MustCompile(`(?i)\b(?:is|type):(?:pr|pull-?request)\b`) + func listIssues(client *api.Client, repo ghrepo.Interface, filters prShared.FilterOptions, limit int) (*api.IssuesAndTotalCount, error) { var states []string switch filters.State { @@ -114,6 +117,10 @@ loop: } func searchIssues(client *api.Client, detector fd.Detector, repo ghrepo.Interface, filters prShared.FilterOptions, limit int) (*api.IssuesAndTotalCount, error) { + if pullRequestSearchQualifierRE.MatchString(filters.Search) { + return nil, fmt.Errorf("cannot use pull request search qualifiers with `gh issue list`; use `gh pr list` instead") + } + // TODO advancedIssueSearchCleanup // We won't need feature detection when GHES 3.17 support ends, since // the advanced issue search is the only available search backend for diff --git a/pkg/cmd/issue/list/http_test.go b/pkg/cmd/issue/list/http_test.go index 747bd0a4b..d313da90b 100644 --- a/pkg/cmd/issue/list/http_test.go +++ b/pkg/cmd/issue/list/http_test.go @@ -214,3 +214,56 @@ func TestSearchIssuesAndAdvancedSearch(t *testing.T) { }) } } + +func TestSearchIssues_rejectsPullRequestQualifiers(t *testing.T) { + tests := []struct { + name string + search string + }{ + { + name: "is:pr", + search: "is:pr", + }, + { + name: "type:pr", + search: "type:pr", + }, + { + name: "type:pull-request", + search: "type:pull-request", + }, + { + name: "type:pullrequest", + search: "type:pullrequest", + }, + { + name: "case-insensitive is:PR", + search: "is:PR", + }, + { + name: "case-insensitive TYPE:Pull-Request", + search: "TYPE:Pull-Request", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + reg := &httpmock.Registry{} + defer reg.Verify(t) + + httpClient := &http.Client{Transport: reg} + client := api.NewClientFromHTTP(httpClient) + + _, err := searchIssues( + client, + fd.AdvancedIssueSearchSupportedAsOnlyBackend(), + ghrepo.New("OWNER", "REPO"), + prShared.FilterOptions{Search: tt.search}, + 30, + ) + + assert.EqualError(t, err, "cannot use pull request search qualifiers with `gh issue list`; use `gh pr list` instead") + assert.Len(t, reg.Requests, 0) + }) + } +} diff --git a/pkg/cmd/issue/view/fixtures/issueView_previewSingleComment.json b/pkg/cmd/issue/view/fixtures/issueView_previewSingleComment.json index be099c14b..8959acec6 100644 --- a/pkg/cmd/issue/view/fixtures/issueView_previewSingleComment.json +++ b/pkg/cmd/issue/view/fixtures/issueView_previewSingleComment.json @@ -138,10 +138,14 @@ ] } ], - "totalCount": 6 + "totalCount": 6, + "pageInfo": { + "hasNextPage": true, + "endCursor": "Y3Vyc29yOnYyOjg5" + } }, "url": "https://github.com/OWNER/REPO/issues/123" } } } -} +} \ No newline at end of file diff --git a/pkg/cmd/issue/view/http.go b/pkg/cmd/issue/view/http.go index 4adc71802..2982fbbe3 100644 --- a/pkg/cmd/issue/view/http.go +++ b/pkg/cmd/issue/view/http.go @@ -20,14 +20,13 @@ func preloadIssueComments(client *http.Client, repo ghrepo.Interface, issue *api } `graphql:"node(id: $id)"` } + if !issue.Comments.PageInfo.HasNextPage { + return nil + } + variables := map[string]interface{}{ "id": githubv4.ID(issue.ID), - "endCursor": (*githubv4.String)(nil), - } - if issue.Comments.PageInfo.HasNextPage { - variables["endCursor"] = githubv4.String(issue.Comments.PageInfo.EndCursor) - } else { - issue.Comments.Nodes = issue.Comments.Nodes[0:0] + "endCursor": githubv4.String(issue.Comments.PageInfo.EndCursor), } gql := api.NewClientFromHTTP(client) diff --git a/pkg/cmd/issue/view/view.go b/pkg/cmd/issue/view/view.go index 41c01ef40..e41ad6acf 100644 --- a/pkg/cmd/issue/view/view.go +++ b/pkg/cmd/issue/view/view.go @@ -144,8 +144,6 @@ func viewRun(opts *ViewOptions) error { } if lookupFields.Contains("comments") { - // FIXME: this re-fetches the comments connection even though the initial set of 100 were - // fetched in the previous request. err := preloadIssueComments(httpClient, baseRepo, issue) if err != nil { return err diff --git a/pkg/cmd/pr/diff/diff.go b/pkg/cmd/pr/diff/diff.go index acf17462c..6d37bf1e5 100644 --- a/pkg/cmd/pr/diff/diff.go +++ b/pkg/cmd/pr/diff/diff.go @@ -190,7 +190,7 @@ func fetchDiff(httpClient *http.Client, baseRepo ghrepo.Interface, prNumber int, const lineBufferSize = 4096 var ( - colorHeader = []byte("\x1b[1;38m") + colorHeader = []byte("\x1b[1;37m") colorAddition = []byte("\x1b[32m") colorRemoval = []byte("\x1b[31m") colorReset = []byte("\x1b[m") diff --git a/pkg/cmd/pr/diff/diff_test.go b/pkg/cmd/pr/diff/diff_test.go index 28a83bfc4..ceb93a187 100644 --- a/pkg/cmd/pr/diff/diff_test.go +++ b/pkg/cmd/pr/diff/diff_test.go @@ -179,7 +179,7 @@ func Test_diffRun(t *testing.T) { Patch: false, }, wantFields: []string{"number"}, - wantStdout: fmt.Sprintf(testDiff, "\x1b[m", "\x1b[1;38m", "\x1b[32m", "\x1b[31m"), + wantStdout: fmt.Sprintf(testDiff, "\x1b[m", "\x1b[1;37m", "\x1b[32m", "\x1b[31m"), httpStubs: func(reg *httpmock.Registry) { stubDiffRequest(reg, "application/vnd.github.v3.diff", fmt.Sprintf(testDiff, "", "", "", "")) }, @@ -313,7 +313,7 @@ func Test_colorDiffLines(t *testing.T) { "%[4]s+foo%[2]s\n%[5]s-b%[1]sr%[2]s\n%[3]s+++ baz%[2]s\n", strings.Repeat("a", 2*lineBufferSize), "\x1b[m", - "\x1b[1;38m", + "\x1b[1;37m", "\x1b[32m", "\x1b[31m", ), diff --git a/pkg/cmd/project/item-edit/item_edit.go b/pkg/cmd/project/item-edit/item_edit.go index 43aff835a..6eb44caf4 100644 --- a/pkg/cmd/project/item-edit/item_edit.go +++ b/pkg/cmd/project/item-edit/item_edit.go @@ -16,9 +16,11 @@ import ( type editItemOpts struct { // updateDraftIssue - title string - body string - itemID string + title string + titleChanged bool + body string + bodyChanged bool + itemID string // updateItem fieldID string projectID string @@ -45,6 +47,12 @@ type EditProjectDraftIssue struct { } `graphql:"updateProjectV2DraftIssue(input:$input)"` } +type DraftIssueQuery struct { + DraftIssueNode struct { + DraftIssue queries.DraftIssue `graphql:"... on DraftIssue"` + } `graphql:"node(id: $id)"` +} + type UpdateProjectV2FieldValue struct { Update struct { Item queries.ProjectItem `graphql:"projectV2Item"` @@ -78,6 +86,8 @@ func NewCmdEditItem(f *cmdutil.Factory, runF func(config editItemConfig) error) `), RunE: func(cmd *cobra.Command, args []string) error { opts.numberChanged = cmd.Flags().Changed("number") + opts.titleChanged = cmd.Flags().Changed("title") + opts.bodyChanged = cmd.Flags().Changed("body") if err := cmdutil.MutuallyExclusive( "only one of `--text`, `--number`, `--date`, `--single-select-option-id` or `--iteration-id` may be used", opts.text != "", @@ -143,7 +153,7 @@ func runEditItem(config editItemConfig) error { } // update draft issue - if config.opts.title != "" || config.opts.body != "" { + if config.opts.titleChanged || config.opts.bodyChanged { return updateDraftIssue(config) } @@ -158,13 +168,41 @@ func runEditItem(config editItemConfig) error { return cmdutil.SilentError } -func buildEditDraftIssue(config editItemConfig) (*EditProjectDraftIssue, map[string]interface{}) { +func fetchDraftIssueByID(config editItemConfig, draftIssueID string) (*queries.DraftIssue, error) { + var query DraftIssueQuery + variables := map[string]interface{}{ + "id": githubv4.ID(draftIssueID), + } + + err := config.client.Query("DraftIssueByID", &query, variables) + if err != nil { + return nil, err + } + + return &query.DraftIssueNode.DraftIssue, nil +} + +func buildEditDraftIssue(config editItemConfig, currentDraftIssue *queries.DraftIssue) (*EditProjectDraftIssue, map[string]interface{}) { + input := githubv4.UpdateProjectV2DraftIssueInput{ + DraftIssueID: githubv4.ID(config.opts.itemID), + } + + if config.opts.titleChanged { + input.Title = githubv4.NewString(githubv4.String(config.opts.title)) + } else if currentDraftIssue != nil { + // Preserve existing if title is not provided + input.Title = githubv4.NewString(githubv4.String(currentDraftIssue.Title)) + } + + if config.opts.bodyChanged { + input.Body = githubv4.NewString(githubv4.String(config.opts.body)) + } else if currentDraftIssue != nil { + // Preserve existing if body is not provided + input.Body = githubv4.NewString(githubv4.String(currentDraftIssue.Body)) + } + return &EditProjectDraftIssue{}, map[string]interface{}{ - "input": githubv4.UpdateProjectV2DraftIssueInput{ - Body: githubv4.NewString(githubv4.String(config.opts.body)), - DraftIssueID: githubv4.ID(config.opts.itemID), - Title: githubv4.NewString(githubv4.String(config.opts.title)), - }, + "input": input, } } @@ -250,9 +288,19 @@ func updateDraftIssue(config editItemConfig) error { return cmdutil.FlagErrorf("ID must be the ID of the draft issue content which is prefixed with `DI_`") } - query, variables := buildEditDraftIssue(config) + // Fetch current draft issue to preserve fields that aren't being updated + var currentDraftIssue *queries.DraftIssue + var err error + if !config.opts.titleChanged || !config.opts.bodyChanged { + currentDraftIssue, err = fetchDraftIssueByID(config, config.opts.itemID) + if err != nil { + return err + } + } - err := config.client.Mutate("EditDraftIssueItem", query, variables) + query, variables := buildEditDraftIssue(config, currentDraftIssue) + + err = config.client.Mutate("EditDraftIssueItem", query, variables) if err != nil { return err } diff --git a/pkg/cmd/project/item-edit/item_edit_test.go b/pkg/cmd/project/item-edit/item_edit_test.go index 916bb5899..64852bad5 100644 --- a/pkg/cmd/project/item-edit/item_edit_test.go +++ b/pkg/cmd/project/item-edit/item_edit_test.go @@ -129,6 +129,15 @@ func TestNewCmdeditItem(t *testing.T) { }, wantsExporter: true, }, + { + name: "draft issue body only", + cli: "--id 123 --body foobar", + wants: editItemOpts{ + itemID: "123", + body: "foobar", + bodyChanged: true, + }, + }, } t.Setenv("GH_TOKEN", "auth-token") @@ -170,6 +179,9 @@ func TestNewCmdeditItem(t *testing.T) { assert.Equal(t, tt.wants.singleSelectOptionID, gotOpts.singleSelectOptionID) assert.Equal(t, tt.wants.iterationID, gotOpts.iterationID) assert.Equal(t, tt.wants.clear, gotOpts.clear) + assert.Equal(t, tt.wants.titleChanged, gotOpts.titleChanged) + assert.Equal(t, tt.wants.bodyChanged, gotOpts.bodyChanged) + assert.Equal(t, tt.wants.body, gotOpts.body) }) } } @@ -202,9 +214,11 @@ func TestRunItemEdit_Draft(t *testing.T) { config := editItemConfig{ io: ios, opts: editItemOpts{ - title: "a title", - body: "a new body", - itemID: "DI_item_id", + title: "a title", + titleChanged: true, + body: "a new body", + bodyChanged: true, + itemID: "DI_item_id", }, client: client, } @@ -217,6 +231,154 @@ func TestRunItemEdit_Draft(t *testing.T) { stdout.String()) } +func TestRunItemEdit_DraftTitleOnly(t *testing.T) { + defer gock.Off() + + gock.New("https://api.github.com"). + Post("/graphql"). + BodyString(`{"query":"query DraftIssueByID.*","variables":{"id":"DI_item_id"}}`). + Reply(200). + JSON(map[string]interface{}{ + "data": map[string]interface{}{ + "node": map[string]interface{}{ + "id": "DI_item_id", + "title": "existing title", + "body": "existing body", + }, + }, + }) + + gock.New("https://api.github.com"). + Post("/graphql"). + BodyString(`{"query":"mutation EditDraftIssueItem.*","variables":{"input":{"draftIssueId":"DI_item_id","title":"new title","body":"existing body"}}}`). + Reply(200). + JSON(map[string]interface{}{ + "data": map[string]interface{}{ + "updateProjectV2DraftIssue": map[string]interface{}{ + "draftIssue": map[string]interface{}{ + "title": "new title", + "body": "existing body", + }, + }, + }, + }) + + client := queries.NewTestClient() + + ios, _, stdout, _ := iostreams.Test() + ios.SetStdoutTTY(true) + + config := editItemConfig{ + io: ios, + opts: editItemOpts{ + title: "new title", + titleChanged: true, + bodyChanged: false, + itemID: "DI_item_id", + }, + client: client, + } + + err := runEditItem(config) + assert.NoError(t, err) + assert.Equal( + t, + "Edited draft issue \"new title\"\n", + stdout.String()) +} + +func TestRunItemEdit_DraftBodyOnly(t *testing.T) { + defer gock.Off() + + gock.New("https://api.github.com"). + Post("/graphql"). + BodyString(`{"query":"query DraftIssueByID.*","variables":{"id":"DI_item_id"}}`). + Reply(200). + JSON(map[string]interface{}{ + "data": map[string]interface{}{ + "node": map[string]interface{}{ + "id": "DI_item_id", + "title": "existing title", + "body": "existing body", + }, + }, + }) + + gock.New("https://api.github.com"). + Post("/graphql"). + BodyString(`{"query":"mutation EditDraftIssueItem.*","variables":{"input":{"draftIssueId":"DI_item_id","title":"existing title","body":"new body"}}}`). + Reply(200). + JSON(map[string]interface{}{ + "data": map[string]interface{}{ + "updateProjectV2DraftIssue": map[string]interface{}{ + "draftIssue": map[string]interface{}{ + "title": "existing title", + "body": "new body", + }, + }, + }, + }) + + client := queries.NewTestClient() + + ios, _, stdout, _ := iostreams.Test() + ios.SetStdoutTTY(true) + + config := editItemConfig{ + io: ios, + opts: editItemOpts{ + titleChanged: false, + body: "new body", + bodyChanged: true, + itemID: "DI_item_id", + }, + client: client, + } + + err := runEditItem(config) + assert.NoError(t, err) + assert.Equal( + t, + "Edited draft issue \"existing title\"\n", + stdout.String()) +} + +func TestRunItemEdit_DraftFetchError(t *testing.T) { + defer gock.Off() + + gock.New("https://api.github.com"). + Post("/graphql"). + BodyString(`{"query":"query DraftIssueByID.*","variables":{"id":"DI_item_id"}}`). + Reply(200). + JSON(map[string]interface{}{ + "errors": []map[string]interface{}{ + { + "type": "NOT_FOUND", + "message": "Could not resolve to a node with the global id of 'DI_item_id' (node)", + }, + }, + }) + + client := queries.NewTestClient() + + ios, _, _, _ := iostreams.Test() + + config := editItemConfig{ + io: ios, + opts: editItemOpts{ + title: "new title", + titleChanged: true, + bodyChanged: false, + itemID: "DI_item_id", + }, + client: client, + } + + err := runEditItem(config) + assert.Error(t, err) + assert.Contains(t, err.Error(), "Could not resolve to a node") +} + func TestRunItemEdit_Text(t *testing.T) { defer gock.Off() // gock.Observe(gock.DumpRequest) @@ -232,10 +394,9 @@ func TestRunItemEdit_Text(t *testing.T) { "projectV2Item": map[string]interface{}{ "ID": "item_id", "content": map[string]interface{}{ - "__typename": "Issue", - "body": "body", - "title": "title", - "number": 1, + "body": "body", + "title": "title", + "number": 1, "repository": map[string]interface{}{ "nameWithOwner": "my-repo", }, @@ -544,9 +705,11 @@ func TestRunItemEdit_InvalidID(t *testing.T) { client := queries.NewTestClient() config := editItemConfig{ opts: editItemOpts{ - title: "a title", - body: "a new body", - itemID: "item_id", + title: "a title", + titleChanged: true, + body: "a new body", + bodyChanged: true, + itemID: "item_id", }, client: client, } @@ -630,10 +793,12 @@ func TestRunItemEdit_JSON(t *testing.T) { config := editItemConfig{ io: ios, opts: editItemOpts{ - title: "a title", - body: "a new body", - itemID: "DI_item_id", - exporter: cmdutil.NewJSONExporter(), + title: "a title", + titleChanged: true, + body: "a new body", + bodyChanged: true, + itemID: "DI_item_id", + exporter: cmdutil.NewJSONExporter(), }, client: client, } diff --git a/pkg/cmd/project/shared/queries/queries.go b/pkg/cmd/project/shared/queries/queries.go index d56d61107..9a3bd4909 100644 --- a/pkg/cmd/project/shared/queries/queries.go +++ b/pkg/cmd/project/shared/queries/queries.go @@ -103,6 +103,11 @@ func (c *Client) Mutate(operationName string, query interface{}, variables map[s return handleError(err) } +func (c *Client) Query(operationName string, query interface{}, variables map[string]interface{}) error { + err := c.apiClient.Query(operationName, query, variables) + return handleError(err) +} + // PageInfo is a PageInfo GraphQL object https://docs.github.com/en/graphql/reference/objects#pageinfo. type PageInfo struct { EndCursor githubv4.String diff --git a/pkg/cmd/repo/clone/clone.go b/pkg/cmd/repo/clone/clone.go index 1466cd96a..b29b25038 100644 --- a/pkg/cmd/repo/clone/clone.go +++ b/pkg/cmd/repo/clone/clone.go @@ -27,6 +27,7 @@ type CloneOptions struct { GitArgs []string Repository string UpstreamName string + NoUpstream bool } func NewCmdClone(f *cmdutil.Factory, runF func(*CloneOptions) error) *cobra.Command { @@ -60,6 +61,7 @@ func NewCmdClone(f *cmdutil.Factory, runF func(*CloneOptions) error) *cobra.Comm the remote after the owner of the parent repository. If the repository is a fork, its parent repository will be set as the default remote repository. + To skip this behavior, use %[1]s--no-upstream%[1]s. `, "`"), Example: heredoc.Doc(` # Clone a repository from a specific org @@ -77,6 +79,9 @@ func NewCmdClone(f *cmdutil.Factory, runF func(*CloneOptions) error) *cobra.Comm # Clone a repository with additional git clone flags $ gh repo clone cli/cli -- --depth=1 + + # Clone a fork without adding an upstream remote + $ gh repo clone myfork --no-upstream `), RunE: func(cmd *cobra.Command, args []string) error { opts.Repository = args[0] @@ -91,6 +96,8 @@ func NewCmdClone(f *cmdutil.Factory, runF func(*CloneOptions) error) *cobra.Comm } cmd.Flags().StringVarP(&opts.UpstreamName, "upstream-remote-name", "u", "upstream", "Upstream remote name when cloning a fork") + cmd.Flags().BoolVar(&opts.NoUpstream, "no-upstream", false, "Do not add an upstream remote when cloning a fork") + cmd.MarkFlagsMutuallyExclusive("upstream-remote-name", "no-upstream") cmd.SetFlagErrorFunc(func(cmd *cobra.Command, err error) error { if err == pflag.ErrHelp { return err @@ -187,37 +194,43 @@ func cloneRun(opts *CloneOptions) error { // If the repo is a fork, add the parent as an upstream remote and set the parent as the default repo. if canonicalRepo.Parent != nil { - protocol := cfg.GitProtocol(canonicalRepo.Parent.RepoHost()).Value - upstreamURL := ghrepo.FormatRemoteURL(canonicalRepo.Parent, protocol) - - upstreamName := opts.UpstreamName - if opts.UpstreamName == "@owner" { - upstreamName = canonicalRepo.Parent.RepoOwner() - } - gc := gitClient.Copy() gc.RepoDir = cloneDir - if _, err := gc.AddRemote(ctx, upstreamName, upstreamURL, []string{canonicalRepo.Parent.DefaultBranchRef.Name}); err != nil { - return err - } + if opts.NoUpstream { + if err := gc.SetRemoteResolution(ctx, "origin", "base"); err != nil { + return err + } + } else { + protocol := cfg.GitProtocol(canonicalRepo.Parent.RepoHost()).Value + upstreamURL := ghrepo.FormatRemoteURL(canonicalRepo.Parent, protocol) - if err := gc.Fetch(ctx, upstreamName, ""); err != nil { - return err - } + upstreamName := opts.UpstreamName + if opts.UpstreamName == "@owner" { + upstreamName = canonicalRepo.Parent.RepoOwner() + } - if err := gc.SetRemoteBranches(ctx, upstreamName, `*`); err != nil { - return err - } + if _, err := gc.AddRemote(ctx, upstreamName, upstreamURL, []string{canonicalRepo.Parent.DefaultBranchRef.Name}); err != nil { + return err + } - if err = gc.SetRemoteResolution(ctx, upstreamName, "base"); err != nil { - return err - } + if err := gc.Fetch(ctx, upstreamName, ""); err != nil { + return err + } - connectedToTerminal := opts.IO.IsStdoutTTY() - if connectedToTerminal { - 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(canonicalRepo.Parent))) + if err := gc.SetRemoteBranches(ctx, upstreamName, `*`); err != nil { + return err + } + + if err := gc.SetRemoteResolution(ctx, upstreamName, "base"); err != nil { + return err + } + + connectedToTerminal := opts.IO.IsStdoutTTY() + if connectedToTerminal { + 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(canonicalRepo.Parent))) + } } } return nil diff --git a/pkg/cmd/repo/clone/clone_test.go b/pkg/cmd/repo/clone/clone_test.go index 471ed05dd..bab2bd670 100644 --- a/pkg/cmd/repo/clone/clone_test.go +++ b/pkg/cmd/repo/clone/clone_test.go @@ -54,6 +54,20 @@ func TestNewCmdClone(t *testing.T) { GitArgs: []string{"--depth", "1", "--recurse-submodules"}, }, }, + { + name: "no-upstream flag", + args: "OWNER/REPO --no-upstream", + wantOpts: CloneOptions{ + Repository: "OWNER/REPO", + GitArgs: []string{}, + NoUpstream: true, + }, + }, + { + name: "no-upstream with upstream-remote-name", + args: "OWNER/REPO --no-upstream --upstream-remote-name test", + wantErr: "if any flags in the group [upstream-remote-name no-upstream] are set none of the others can be; [no-upstream upstream-remote-name] were all set", + }, { name: "unknown argument", args: "OWNER/REPO --depth 1", @@ -92,6 +106,7 @@ func TestNewCmdClone(t *testing.T) { assert.Equal(t, tt.wantOpts.Repository, opts.Repository) assert.Equal(t, tt.wantOpts.GitArgs, opts.GitArgs) + assert.Equal(t, tt.wantOpts.NoUpstream, opts.NoUpstream) }) } } @@ -344,6 +359,70 @@ func Test_RepoClone_withoutUsername(t *testing.T) { assert.Equal(t, "", output.Stderr()) } +func Test_RepoClone_hasParent_noUpstream(t *testing.T) { + reg := &httpmock.Registry{} + defer reg.Verify(t) + reg.Register( + httpmock.GraphQL(`query RepositoryInfo\b`), + httpmock.StringResponse(` + { "data": { "repository": { + "name": "REPO", + "owner": { + "login": "OWNER" + }, + "parent": { + "name": "ORIG", + "owner": { + "login": "hubot" + }, + "defaultBranchRef": { + "name": "trunk" + } + } + } } } + `)) + + httpClient := &http.Client{Transport: reg} + + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + cs.Register(`git clone https://github.com/OWNER/REPO.git`, 0, "") + cs.Register(`git -C REPO config --add remote.origin.gh-resolved base`, 0, "") + + _, err := runCloneCommand(httpClient, "OWNER/REPO --no-upstream") + if err != nil { + t.Fatalf("error running command `repo clone`: %v", err) + } +} + +func Test_RepoClone_noParent_noUpstream(t *testing.T) { + reg := &httpmock.Registry{} + defer reg.Verify(t) + reg.Register( + httpmock.GraphQL(`query RepositoryInfo\b`), + httpmock.StringResponse(` + { "data": { "repository": { + "name": "REPO", + "owner": { + "login": "OWNER" + } + } } } + `)) + + httpClient := &http.Client{Transport: reg} + + cs, cmdTeardown := run.Stub() + defer cmdTeardown(t) + + cs.Register(`git clone https://github.com/OWNER/REPO.git`, 0, "") + + _, err := runCloneCommand(httpClient, "OWNER/REPO --no-upstream") + if err != nil { + t.Fatalf("error running command `repo clone`: %v", err) + } +} + func TestSimplifyURL(t *testing.T) { tests := []struct { name string diff --git a/pkg/cmd/repo/fork/fork.go b/pkg/cmd/repo/fork/fork.go index b620291d6..3ebb02413 100644 --- a/pkg/cmd/repo/fork/fork.go +++ b/pkg/cmd/repo/fork/fork.go @@ -113,6 +113,10 @@ func NewCmdFork(f *cmdutil.Factory, runF func(*ForkOptions) error) *cobra.Comman opts.Rename = true // Any existing 'origin' will be renamed to upstream } + if opts.Repository != "" && cmd.Flags().Changed("remote") { + return cmdutil.FlagErrorf("the `--remote` flag is unsupported when a repository argument is provided") + } + if promptOk { // We can prompt for these if they were not specified. opts.PromptClone = !cmd.Flags().Changed("clone") diff --git a/pkg/cmd/repo/fork/fork_test.go b/pkg/cmd/repo/fork/fork_test.go index 1f0b9cef1..edf5f2763 100644 --- a/pkg/cmd/repo/fork/fork_test.go +++ b/pkg/cmd/repo/fork/fork_test.go @@ -144,6 +144,12 @@ func TestNewCmdFork(t *testing.T) { Rename: false, }, }, + { + name: "remote with repo argument", + cli: "foo/bar --remote", + wantErr: true, + errMsg: "the `--remote` flag is unsupported when a repository argument is provided", + }, } for _, tt := range tests { diff --git a/pkg/jsoncolor/jsoncolor.go b/pkg/jsoncolor/jsoncolor.go index 8e20a1161..b9ff95253 100644 --- a/pkg/jsoncolor/jsoncolor.go +++ b/pkg/jsoncolor/jsoncolor.go @@ -9,7 +9,7 @@ import ( ) const ( - colorDelim = "1;38" // bright white + colorDelim = "1;37" // bold white colorKey = "1;34" // bright blue colorNull = "36" // cyan colorString = "32" // green diff --git a/pkg/jsoncolor/jsoncolor_test.go b/pkg/jsoncolor/jsoncolor_test.go index 9e2eda343..d2a22b90b 100644 --- a/pkg/jsoncolor/jsoncolor_test.go +++ b/pkg/jsoncolor/jsoncolor_test.go @@ -34,7 +34,7 @@ func TestWrite(t *testing.T) { r: bytes.NewBufferString(`{}`), indent: "", }, - wantW: "\x1b[1;38m{\x1b[m\x1b[1;38m}\x1b[m\n", + wantW: "\x1b[1;37m{\x1b[m\x1b[1;37m}\x1b[m\n", wantErr: false, }, { @@ -43,9 +43,9 @@ func TestWrite(t *testing.T) { r: bytes.NewBufferString(`{"hash":{"a":1,"b":2},"array":[3,4]}`), indent: "\t", }, - wantW: "\x1b[1;38m{\x1b[m\n\t\x1b[1;34m\"hash\"\x1b[m\x1b[1;38m:\x1b[m " + - "\x1b[1;38m{\x1b[m\n\t\t\x1b[1;34m\"a\"\x1b[m\x1b[1;38m:\x1b[m 1\x1b[1;38m,\x1b[m\n\t\t\x1b[1;34m\"b\"\x1b[m\x1b[1;38m:\x1b[m 2\n\t\x1b[1;38m}\x1b[m\x1b[1;38m,\x1b[m" + - "\n\t\x1b[1;34m\"array\"\x1b[m\x1b[1;38m:\x1b[m \x1b[1;38m[\x1b[m\n\t\t3\x1b[1;38m,\x1b[m\n\t\t4\n\t\x1b[1;38m]\x1b[m\n\x1b[1;38m}\x1b[m\n", + wantW: "\x1b[1;37m{\x1b[m\n\t\x1b[1;34m\"hash\"\x1b[m\x1b[1;37m:\x1b[m " + + "\x1b[1;37m{\x1b[m\n\t\t\x1b[1;34m\"a\"\x1b[m\x1b[1;37m:\x1b[m 1\x1b[1;37m,\x1b[m\n\t\t\x1b[1;34m\"b\"\x1b[m\x1b[1;37m:\x1b[m 2\n\t\x1b[1;37m}\x1b[m\x1b[1;37m,\x1b[m" + + "\n\t\x1b[1;34m\"array\"\x1b[m\x1b[1;37m:\x1b[m \x1b[1;37m[\x1b[m\n\t\t3\x1b[1;37m,\x1b[m\n\t\t4\n\t\x1b[1;37m]\x1b[m\n\x1b[1;37m}\x1b[m\n", wantErr: false, }, { @@ -63,7 +63,7 @@ func TestWrite(t *testing.T) { r: bytes.NewBufferString(`{{`), indent: "", }, - wantW: "\x1b[1;38m{\x1b[m\n", + wantW: "\x1b[1;37m{\x1b[m\n", wantErr: true, }, } diff --git a/script/licenses b/script/licenses index ecdcbcdf4..1c9debf61 100755 --- a/script/licenses +++ b/script/licenses @@ -3,7 +3,7 @@ # Generate third-party license information for embedding in the binary. # # Usage: -# ./script/licenses Generate licenses for a single platform +# ./script/licenses Generate licenses for a single platform # ./script/licenses --check Verify generation works for all release platforms # # The single-platform mode is called by goreleaser pre-build hooks to generate @@ -75,8 +75,8 @@ if [ "$1" = "--check" ]; then echo "License generation verified for all platforms." elif [ $# -eq 2 ]; then - generate_licenses "$1" "$2" "internal/licenses/embed" - echo "Licenses written to internal/licenses/embed/" + generate_licenses "$1" "$2" "internal/licenses/embed/${1}-${2}" + echo "Licenses written to internal/licenses/embed/${1}-${2}" else echo "Usage: $0 " echo " $0 --check"