STACKITGit/cli
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Merge branch 'trunk' into n1lesh-remove-scope

Browse source
This commit is contained in:
Nilesh Singh 2023-06-25 00:03:12 +05:30 • committed by GitHub
commit 2a436cde3d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
32 changed files with 402 additions and 131 deletions
Download patch file Download diff file Expand all files Collapse all files

7
cmd/gh/main.go
View file

@ -109,7 +109,7 @@ func mainRun() exitCode {
if cmd, err := rootCmd.ExecuteContextC(ctx); err != nil {
var pagerPipeError *iostreams.ErrClosedPagerPipe
var noResultsError cmdutil.NoResultsError
var execError *exec.ExitError
var extError *root.ExternalCommandExitError
var authError *root.AuthError
if err == cmdutil.SilentError {
return exitError
@ -130,8 +130,9 @@ func mainRun() exitCode {
}
// no results is not a command failure
return exitOK
} else if errors.As(err, &execError) {
return exitCode(execError.ExitCode())
} else if errors.As(err, &extError) {
// pass on exit codes from extensions and shell aliases
return exitCode(extError.ExitCode())
}
printError(stderr, err, cmd, hasDebug)

2
internal/config/config.go
View file

@ -4,9 +4,9 @@ import (
"os"
"path/filepath"
"github.com/cli/cli/v2/internal/keyring"
ghAuth "github.com/cli/go-gh/v2/pkg/auth"
ghConfig "github.com/cli/go-gh/v2/pkg/config"
"github.com/zalando/go-keyring"
)
const (

68
internal/keyring/keyring.go Normal file
View file

@ -0,0 +1,68 @@
// Package keyring is a simple wrapper that adds timeouts to the zalando/go-keyring package.
package keyring
import (
"time"
"github.com/zalando/go-keyring"
)
type TimeoutError struct {
message string
}
func (e *TimeoutError) Error() string {
return e.message
}
// Set secret in keyring for user.
func Set(service, user, secret string) error {
ch := make(chan error, 1)
go func() {
defer close(ch)
ch <- keyring.Set(service, user, secret)
}()
select {
case err := <-ch:
return err
case <-time.After(3 * time.Second):
return &TimeoutError{"timeout while trying to set secret in keyring"}
}
}
// Get secret from keyring given service and user name.
func Get(service, user string) (string, error) {
ch := make(chan struct {
val string
err error
}, 1)
go func() {
defer close(ch)
val, err := keyring.Get(service, user)
ch <- struct {
val string
err error
}{val, err}
}()
select {
case res := <-ch:
return res.val, res.err
case <-time.After(3 * time.Second):
return "", &TimeoutError{"timeout while trying to get secret from keyring"}
}
}
// Delete secret from keyring.
func Delete(service, user string) error {
ch := make(chan error, 1)
go func() {
defer close(ch)
ch <- keyring.Delete(service, user)
}()
select {
case err := <-ch:
return err
case <-time.After(3 * time.Second):
return &TimeoutError{"timeout while trying to delete secret from keyring"}
}
}

60
pkg/cmd/alias/imports/import.go
View file

@ -122,15 +122,31 @@ func importRun(opts *ImportOptions) error {
var msg strings.Builder
for _, alias := range getSortedKeys(aliasMap) {
if !opts.validAliasName(alias) {
msg.WriteString(
fmt.Sprintf("%s Could not import alias %s: already a gh command, extension, or alias\n",
cs.FailureIcon(),
cs.Bold(alias),
),
)
var existingAlias bool
if _, err := aliasCfg.Get(alias); err == nil {
existingAlias = true
}
continue
if !opts.validAliasName(alias) {
if !existingAlias {
msg.WriteString(
fmt.Sprintf("%s Could not import alias %s: already a gh command or extension\n",
cs.FailureIcon(),
cs.Bold(alias),
),
)
continue
}
if existingAlias && !opts.OverwriteExisting {
msg.WriteString(
fmt.Sprintf("%s Could not import alias %s: name already taken\n",
cs.FailureIcon(),
cs.Bold(alias),
),
)
continue
}
}
expansion := aliasMap[alias]
@ -142,31 +158,19 @@ func importRun(opts *ImportOptions) error {
cs.Bold(alias),
),
)
continue
}
if _, err := aliasCfg.Get(alias); err == nil {
if opts.OverwriteExisting {
aliasCfg.Add(alias, expansion)
aliasCfg.Add(alias, expansion)
msg.WriteString(
fmt.Sprintf("%s Changed alias %s\n",
cs.WarningIcon(),
cs.Bold(alias),
),
)
} else {
msg.WriteString(
fmt.Sprintf("%s Could not import alias %s: name already taken\n",
cs.FailureIcon(),
cs.Bold(alias),
),
)
}
if existingAlias && opts.OverwriteExisting {
msg.WriteString(
fmt.Sprintf("%s Changed alias %s\n",
cs.WarningIcon(),
cs.Bold(alias),
),
)
} else {
aliasCfg.Add(alias, expansion)
msg.WriteString(
fmt.Sprintf("%s Added alias %s\n",
cs.SuccessIcon(),

33
pkg/cmd/alias/imports/import_test.go
View file

@ -112,13 +112,14 @@ func TestImportRun(t *testing.T) {
importStdinMsg := "- Importing aliases from standard input"
tests := []struct {
name string
opts *ImportOptions
stdin string
fileContents string
initConfig string
wantConfig string
wantStderr string
name string
opts *ImportOptions
stdin string
fileContents string
initConfig string
aliasCommands []*cobra.Command
wantConfig string
wantStderr string
}{
{
name: "with no existing aliases",
@ -158,6 +159,9 @@ func TestImportRun(t *testing.T) {
igrep: '!gh issue list --label="$1" | grep "$2"'
editor: vim
`),
aliasCommands: []*cobra.Command{
{Use: "igrep"},
},
wantConfig: heredoc.Doc(`
aliases:
igrep: '!gh issue list --label="$1" | grep "$2"'
@ -211,6 +215,9 @@ func TestImportRun(t *testing.T) {
co: pr checkout
editor: vim
`),
aliasCommands: []*cobra.Command{
{Use: "co"},
},
wantConfig: heredoc.Doc(`
aliases:
co: pr checkout
@ -234,6 +241,9 @@ func TestImportRun(t *testing.T) {
co: pr checkout
editor: vim
`),
aliasCommands: []*cobra.Command{
{Use: "co"},
},
wantConfig: heredoc.Doc(`
aliases:
co: pr checkout -R cool/repo
@ -256,9 +266,9 @@ func TestImportRun(t *testing.T) {
wantStderr: strings.Join(
[]string{
importFileMsg,
"X Could not import alias api: already a gh command, extension, or alias",
"X Could not import alias issue: already a gh command, extension, or alias",
"X Could not import alias pr: already a gh command, extension, or alias\n\n",
"X Could not import alias api: already a gh command or extension",
"X Could not import alias issue: already a gh command or extension",
"X Could not import alias pr: already a gh command or extension\n\n",
},
"\n",
),
@ -315,6 +325,9 @@ func TestImportRun(t *testing.T) {
apiCmd := &cobra.Command{Use: "api"}
apiCmd.AddCommand(&cobra.Command{Use: "graphql"})
rootCmd.AddCommand(apiCmd)
for _, cmd := range tt.aliasCommands {
rootCmd.AddCommand(cmd)
}
tt.opts.validAliasName = shared.ValidAliasNameFunc(rootCmd)
tt.opts.validAliasExpansion = shared.ValidAliasExpansionFunc(rootCmd)

19
pkg/cmd/auth/status/status.go
View file

@ -66,7 +66,7 @@ func statusRun(opts *StatusOptions) error {
// TODO check tty
stderr := opts.IO.ErrOut
stdout := opts.IO.Out
cs := opts.IO.ColorScheme()
statusInfo := map[string][]string{}
@ -166,13 +166,22 @@ func statusRun(opts *StatusOptions) error {
if !ok {
continue
}
if prevEntry {
if prevEntry && failed {
fmt.Fprint(stderr, "\n")
} else if prevEntry && !failed {
fmt.Fprint(stdout, "\n")
}
prevEntry = true
fmt.Fprintf(stderr, "%s\n", cs.Bold(hostname))
for _, line := range lines {
fmt.Fprintf(stderr, " %s\n", line)
if failed {
fmt.Fprintf(stderr, "%s\n", cs.Bold(hostname))
for _, line := range lines {
fmt.Fprintf(stderr, " %s\n", line)
}
} else {
fmt.Fprintf(stdout, "%s\n", cs.Bold(hostname))
for _, line := range lines {
fmt.Fprintf(stdout, " %s\n", line)
}
}
}

30
pkg/cmd/auth/status/status_test.go
View file

@ -76,12 +76,13 @@ func Test_statusRun(t *testing.T) {
readConfigs := config.StubWriteConfig(t)
tests := []struct {
name string
opts *StatusOptions
httpStubs func(*httpmock.Registry)
cfgStubs func(*config.ConfigMock)
wantErr string
wantOut string
name string
opts *StatusOptions
httpStubs func(*httpmock.Registry)
cfgStubs func(*config.ConfigMock)
wantErr string
wantOut string
wantErrOut string
}{
{
name: "hostname set",
@ -126,7 +127,7 @@ func Test_statusRun(t *testing.T) {
httpmock.StringResponse(`{"data":{"viewer":{"login":"tess"}}}`))
},
wantErr: "SilentError",
wantOut: heredoc.Doc(`
wantErrOut: heredoc.Doc(`
joel.miller
X joel.miller: the token in GH_CONFIG_DIR/hosts.yml is missing required scope 'read:org'
- To request missing scopes, run: gh auth refresh -h joel.miller
@ -156,7 +157,7 @@ func Test_statusRun(t *testing.T) {
httpmock.StringResponse(`{"data":{"viewer":{"login":"tess"}}}`))
},
wantErr: "SilentError",
wantOut: heredoc.Doc(`
wantErrOut: heredoc.Doc(`
joel.miller
X joel.miller: authentication failed
- The joel.miller token in GH_CONFIG_DIR/hosts.yml is no longer valid.
@ -298,9 +299,9 @@ func Test_statusRun(t *testing.T) {
cfgStubs: func(c *config.ConfigMock) {
c.Set("github.com", "oauth_token", "abc123")
},
httpStubs: func(reg *httpmock.Registry) {},
wantErr: "SilentError",
wantOut: "Hostname \"github.example.com\" not found among authenticated GitHub hosts\n",
httpStubs: func(reg *httpmock.Registry) {},
wantErr: "SilentError",
wantErrOut: "Hostname \"github.example.com\" not found among authenticated GitHub hosts\n",
},
}
@ -310,13 +311,12 @@ func Test_statusRun(t *testing.T) {
tt.opts = &StatusOptions{}
}
ios, _, _, stderr := iostreams.Test()
ios, _, stdout, stderr := iostreams.Test()
ios.SetStdinTTY(true)
ios.SetStderrTTY(true)
ios.SetStdoutTTY(true)
tt.opts.IO = ios
cfg := config.NewFromString("")
if tt.cfgStubs != nil {
tt.cfgStubs(cfg)
@ -340,8 +340,10 @@ func Test_statusRun(t *testing.T) {
} else {
assert.NoError(t, err)
}
output := strings.ReplaceAll(stdout.String(), config.ConfigDir()+string(filepath.Separator), "GH_CONFIG_DIR/")
errorOutput := strings.ReplaceAll(stderr.String(), config.ConfigDir()+string(filepath.Separator), "GH_CONFIG_DIR/")
output := strings.ReplaceAll(stderr.String(), config.ConfigDir()+string(filepath.Separator), "GH_CONFIG_DIR/")
assert.Equal(t, tt.wantErrOut, errorOutput)
assert.Equal(t, tt.wantOut, output)
mainBuf := bytes.Buffer{}

12
pkg/cmd/browse/browse.go
View file

@ -163,14 +163,12 @@ func runBrowse(opts *BrowseOptions) error {
return fmt.Errorf("unable to determine base repository: %w", err)
}
if opts.Commit != "" {
if opts.Commit == emptyCommitFlag {
commit, err := opts.GitClient.LastCommit()
if err != nil {
return err
}
opts.Commit = commit.Sha
if opts.Commit != "" && opts.Commit == emptyCommitFlag {
commit, err := opts.GitClient.LastCommit()
if err != nil {
return err
}
opts.Commit = commit.Sha
}
section, err := parseSection(baseRepo, opts)

35
pkg/cmd/gist/edit/edit.go
View file

@ -32,11 +32,12 @@ type EditOptions struct {
Edit func(string, string, string, *iostreams.IOStreams) (string, error)
Selector string
EditFilename string
AddFilename string
SourceFile string
Description string
Selector string
EditFilename string
AddFilename string
RemoveFilename string
SourceFile string
Description string
}
func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Command {
@ -82,6 +83,10 @@ func NewCmdEdit(f *cmdutil.Factory, runF func(*EditOptions) error) *cobra.Comman
cmd.Flags().StringVarP(&opts.AddFilename, "add", "a", "", "Add a new file to the gist")
cmd.Flags().StringVarP(&opts.Description, "desc", "d", "", "New description for the gist")
cmd.Flags().StringVarP(&opts.EditFilename, "filename", "f", "", "Select a file to edit")
cmd.Flags().StringVarP(&opts.RemoveFilename, "remove", "r", "", "Remove a file from the gist")
cmd.MarkFlagsMutuallyExclusive("add", "remove")
cmd.MarkFlagsMutuallyExclusive("remove", "filename")
return cmd
}
@ -187,6 +192,16 @@ func editRun(opts *EditOptions) error {
return updateGist(apiClient, host, gist)
}
// Remove a file from the gist
if opts.RemoveFilename != "" {
err := removeFile(gist, opts.RemoveFilename)
if err != nil {
return err
}
return updateGist(apiClient, host, gist)
}
filesToUpdate := map[string]string{}
for {
@ -337,3 +352,13 @@ func getFilesToAdd(file string, content []byte) (map[string]*shared.GistFile, er
},
}, nil
}
func removeFile(gist *shared.Gist, filename string) error {
if _, found := gist.Files[filename]; !found {
return fmt.Errorf("gist has no file %q", filename)
}
gist.Files[filename] = nil
return nil
}

96
pkg/cmd/gist/edit/edit_test.go
View file

@ -36,9 +36,10 @@ func Test_getFilesToAdd(t *testing.T) {
func TestNewCmdEdit(t *testing.T) {
tests := []struct {
name string
cli string
wants EditOptions
name string
cli string
wants EditOptions
wantsErr bool
}{
{
name: "no flags",
@ -80,6 +81,24 @@ func TestNewCmdEdit(t *testing.T) {
Description: "my new description",
},
},
{
name: "remove",
cli: "123 --remove cool.md",
wants: EditOptions{
Selector: "123",
RemoveFilename: "cool.md",
},
},
{
name: "add and remove are mutually exclusive",
cli: "123 --add cool.md --remove great.md",
wantsErr: true,
},
{
name: "filename and remove are mutually exclusive",
cli: "123 --filename cool.md --remove great.md",
wantsErr: true,
},
}
for _, tt := range tests {
@ -100,11 +119,17 @@ func TestNewCmdEdit(t *testing.T) {
cmd.SetErr(&bytes.Buffer{})
_, err = cmd.ExecuteC()
assert.NoError(t, err)
if tt.wantsErr {
require.Error(t, err)
return
}
assert.Equal(t, tt.wants.EditFilename, gotOpts.EditFilename)
assert.Equal(t, tt.wants.AddFilename, gotOpts.AddFilename)
assert.Equal(t, tt.wants.Selector, gotOpts.Selector)
require.NoError(t, err)
require.Equal(t, tt.wants.EditFilename, gotOpts.EditFilename)
require.Equal(t, tt.wants.AddFilename, gotOpts.AddFilename)
require.Equal(t, tt.wants.Selector, gotOpts.Selector)
require.Equal(t, tt.wants.RemoveFilename, gotOpts.RemoveFilename)
})
}
}
@ -394,6 +419,63 @@ func Test_editRun(t *testing.T) {
},
},
},
{
name: "remove file, file does not exist",
gist: &shared.Gist{
ID: "1234",
Files: map[string]*shared.GistFile{
"sample.txt": {
Filename: "sample.txt",
Content: "bwhiizzzbwhuiiizzzz",
Type: "text/plain",
},
},
Owner: &shared.GistOwner{Login: "octocat"},
},
opts: &EditOptions{
RemoveFilename: "sample2.txt",
},
wantErr: "gist has no file \"sample2.txt\"",
},
{
name: "remove file from existing gist",
gist: &shared.Gist{
ID: "1234",
Files: map[string]*shared.GistFile{
"sample.txt": {
Filename: "sample.txt",
Content: "bwhiizzzbwhuiiizzzz",
Type: "text/plain",
},
"sample2.txt": {
Filename: "sample2.txt",
Content: "bwhiizzzbwhuiiizzzz",
Type: "text/plain",
},
},
Owner: &shared.GistOwner{Login: "octocat"},
},
httpStubs: func(reg *httpmock.Registry) {
reg.Register(httpmock.REST("POST", "gists/1234"),
httpmock.StatusStringResponse(201, "{}"))
},
opts: &EditOptions{
RemoveFilename: "sample2.txt",
},
wantParams: map[string]interface{}{
"description": "",
"updated_at": "0001-01-01T00:00:00Z",
"public": false,
"files": map[string]interface{}{
"sample.txt": map[string]interface{}{
"filename": "sample.txt",
"content": "bwhiizzzbwhuiiizzzz",
"type": "text/plain",
},
"sample2.txt": nil,
},
},
},
{
name: "edit gist using file from source parameter",
gist: &shared.Gist{

10
pkg/cmd/issue/view/fixtures/issueView_previewWithMetadata.json
View file

@ -25,19 +25,19 @@
"labels": {
"nodes": [
{
"name": "one"
"name": "Closed: Won't Fix"
},
{
"name": "two"
"name": "Status: In Progress"
},
{
"name": "three"
"name": "Type: Bug"
},
{
"name": "four"
"name": "help wanted"
},
{
"name": "five"
"name": "Closed: Duplicate"
}
],
"totalCount": 5

6
pkg/cmd/issue/view/view.go
View file

@ -5,6 +5,7 @@ import (
"fmt"
"io"
"net/http"
"sort"
"strings"
"time"
@ -306,6 +307,11 @@ func issueLabelList(issue *api.Issue, cs *iostreams.ColorScheme) string {
return ""
}
// ignore case sort
sort.SliceStable(issue.Labels.Nodes, func(i, j int) bool {
return strings.ToLower(issue.Labels.Nodes[i].Name) < strings.ToLower(issue.Labels.Nodes[j].Name)
})
labelNames := make([]string, len(issue.Labels.Nodes))
for i, label := range issue.Labels.Nodes {
if cs == nil {

4
pkg/cmd/issue/view/view_test.go
View file

@ -125,7 +125,7 @@ func TestIssueView_nontty_Preview(t *testing.T) {
`author:\tmarseilles`,
`state:\tOPEN`,
`comments:\t9`,
`labels:\tone, two, three, four, five`,
`labels:\tClosed: Duplicate, Closed: Won't Fix, help wanted, Status: In Progress, Type: Bug`,
`projects:\tProject 1 \(column A\), Project 2 \(column B\), Project 3 \(column C\), Project 4 \(Awaiting triage\)\n`,
`milestone:\tuluru\n`,
`number:\t123\n`,
@ -196,7 +196,7 @@ func TestIssueView_tty_Preview(t *testing.T) {
`Open.*marseilles opened about 9 years ago.*9 comments`,
`8 \x{1f615} • 7 \x{1f440} • 6 \x{2764}\x{fe0f} • 5 \x{1f389} • 4 \x{1f604} • 3 \x{1f680} • 2 \x{1f44e} • 1 \x{1f44d}`,
`Assignees:.*marseilles, monaco\n`,
`Labels:.*one, two, three, four, five\n`,
`Labels:.*Closed: Duplicate, Closed: Won't Fix, help wanted, Status: In Progress, Type: Bug\n`,
`Projects:.*Project 1 \(column A\), Project 2 \(column B\), Project 3 \(column C\), Project 4 \(Awaiting triage\)\n`,
`Milestone:.*uluru\n`,
`bold story`,

10
pkg/cmd/pr/view/fixtures/prViewPreviewWithMetadataByNumber.json
View file

@ -37,19 +37,19 @@
"labels": {
"nodes": [
{
"name": "one"
"name": "Closed: Won't Fix"
},
{
"name": "two"
"name": "Status: In Progress"
},
{
"name": "three"
"name": "Type: Bug"
},
{
"name": "four"
"name": "help wanted"
},
{
"name": "five"
"name": "Closed: Duplicate"
}
],
"totalcount": 5

5
pkg/cmd/pr/view/view.go
View file

@ -415,6 +415,11 @@ func prLabelList(pr api.PullRequest, cs *iostreams.ColorScheme) string {
return ""
}
// ignore case sort
sort.SliceStable(pr.Labels.Nodes, func(i, j int) bool {
return strings.ToLower(pr.Labels.Nodes[i].Name) < strings.ToLower(pr.Labels.Nodes[j].Name)
})
labelNames := make([]string, 0, len(pr.Labels.Nodes))
for _, label := range pr.Labels.Nodes {
labelNames = append(labelNames, cs.HexToRGB(label.Color, label.Name))

4
pkg/cmd/pr/view/view_test.go
View file

@ -229,7 +229,7 @@ func TestPRView_Preview_nontty(t *testing.T) {
`title:\tBlueberries are from a fork\n`,
`reviewers:\t1 \(Requested\)\n`,
`assignees:\tmarseilles, monaco\n`,
`labels:\tone, two, three, four, five\n`,
`labels:\tClosed: Duplicate, Closed: Won't Fix, help wanted, Status: In Progress, Type: Bug\n`,
`projects:\tProject 1 \(column A\), Project 2 \(column B\), Project 3 \(column C\), Project 4 \(Awaiting triage\)\n`,
`milestone:\tuluru\n`,
`\*\*blueberries taste good\*\*`,
@ -390,7 +390,7 @@ func TestPRView_Preview(t *testing.T) {
`.+100.-10`,
`Reviewers:.*1 \(.*Requested.*\)\n`,
`Assignees:.*marseilles, monaco\n`,
`Labels:.*one, two, three, four, five\n`,
`Labels:.*Closed: Duplicate, Closed: Won't Fix, help wanted, Status: In Progress, Type: Bug\n`,
`Projects:.*Project 1 \(column A\), Project 2 \(column B\), Project 3 \(column C\), Project 4 \(Awaiting triage\)\n`,
`Milestone:.*uluru\n`,
`blueberries taste good`,

9
pkg/cmd/project/close/close.go
View file

@ -128,13 +128,8 @@ func printResults(config closeConfig, project queries.Project) error {
if !config.io.IsStdoutTTY() {
return nil
}
var action string
if config.opts.reopen {
action = "Reopened"
} else {
action = "Closed"
}
_, err := fmt.Fprintf(config.io.Out, "%s project %s\n", action, project.URL)
_, err := fmt.Fprintf(config.io.Out, "%s\n", project.URL)
return err
}

4
pkg/cmd/project/close/close_test.go
View file

@ -183,7 +183,7 @@ func TestRunClose_User(t *testing.T) {
assert.NoError(t, err)
assert.Equal(
t,
"Closed project http://a-url.com\n",
"http://a-url.com\n",
stdout.String())
}
@ -452,6 +452,6 @@ func TestRunClose_Reopen(t *testing.T) {
assert.NoError(t, err)
assert.Equal(
t,
"Reopened project http://a-url.com\n",
"http://a-url.com\n",
stdout.String())
}

2
pkg/cmd/project/copy/copy.go
View file

@ -139,7 +139,7 @@ func printResults(config copyConfig, project queries.Project) error {
if !config.io.IsStdoutTTY() {
return nil
}
_, err := fmt.Fprintf(config.io.Out, "Copied project to %s\n", project.URL)
_, err := fmt.Fprintf(config.io.Out, "%s\n", project.URL)
return err
}

2
pkg/cmd/project/copy/copy_test.go
View file

@ -457,6 +457,6 @@ func TestRunCopy_Me(t *testing.T) {
assert.NoError(t, err)
assert.Equal(
t,
"Copied project to http://a-url.com\n",
"http://a-url.com\n",
stdout.String())
}

2
pkg/cmd/project/create/create.go
View file

@ -110,7 +110,7 @@ func printResults(config createConfig, project queries.Project) error {
return nil
}
_, err := fmt.Fprintf(config.io.Out, "Created project '%s'\n%s\n", project.Title, project.URL)
_, err := fmt.Fprintf(config.io.Out, "%s\n", project.URL)
return err
}

6
pkg/cmd/project/create/create_test.go
View file

@ -146,7 +146,7 @@ func TestRunCreate_User(t *testing.T) {
assert.NoError(t, err)
assert.Equal(
t,
"Created project 'a title'\nhttp://a-url.com\n",
"http://a-url.com\n",
stdout.String())
}
@ -214,7 +214,7 @@ func TestRunCreate_Org(t *testing.T) {
assert.NoError(t, err)
assert.Equal(
t,
"Created project 'a title'\nhttp://a-url.com\n",
"http://a-url.com\n",
stdout.String())
}
@ -273,6 +273,6 @@ func TestRunCreate_Me(t *testing.T) {
assert.NoError(t, err)
assert.Equal(
t,
"Created project 'a title'\nhttp://a-url.com\n",
"http://a-url.com\n",
stdout.String())
}

6
pkg/cmd/project/delete/delete.go
View file

@ -100,7 +100,7 @@ func runDelete(config deleteConfig) error {
return printJSON(config, *project)
}
return printResults(config)
return printResults(config, query.DeleteProject.Project)
}
@ -116,12 +116,12 @@ func deleteItemArgs(config deleteConfig) (*deleteProjectMutation, map[string]int
}
}
func printResults(config deleteConfig) error {
func printResults(config deleteConfig, project queries.Project) error {
if !config.io.IsStdoutTTY() {
return nil
}
_, err := fmt.Fprintf(config.io.Out, "Deleted project\n")
_, err := fmt.Fprintf(config.io.Out, "Deleted project %d\n", project.Number)
return err
}

15
pkg/cmd/project/delete/delete_test.go
View file

@ -148,7 +148,8 @@ func TestRunDelete_User(t *testing.T) {
"data": map[string]interface{}{
"deleteProjectV2": map[string]interface{}{
"projectV2": map[string]interface{}{
"id": "project ID",
"id": "project ID",
"number": 1,
},
},
},
@ -171,7 +172,7 @@ func TestRunDelete_User(t *testing.T) {
assert.NoError(t, err)
assert.Equal(
t,
"Deleted project\n",
"Deleted project 1\n",
stdout.String())
}
@ -239,7 +240,8 @@ func TestRunDelete_Org(t *testing.T) {
"data": map[string]interface{}{
"deleteProjectV2": map[string]interface{}{
"projectV2": map[string]interface{}{
"id": "project ID",
"id": "project ID",
"number": 1,
},
},
},
@ -262,7 +264,7 @@ func TestRunDelete_Org(t *testing.T) {
assert.NoError(t, err)
assert.Equal(
t,
"Deleted project\n",
"Deleted project 1\n",
stdout.String())
}
@ -320,7 +322,8 @@ func TestRunDelete_Me(t *testing.T) {
"data": map[string]interface{}{
"deleteProjectV2": map[string]interface{}{
"projectV2": map[string]interface{}{
"id": "project ID",
"id": "project ID",
"number": 1,
},
},
},
@ -343,6 +346,6 @@ func TestRunDelete_Me(t *testing.T) {
assert.NoError(t, err)
assert.Equal(
t,
"Deleted project\n",
"Deleted project 1\n",
stdout.String())
}

2
pkg/cmd/project/edit/edit.go
View file

@ -150,7 +150,7 @@ func printResults(config editConfig, project queries.Project) error {
return nil
}
_, err := fmt.Fprintf(config.io.Out, "Updated project %s\n", project.URL)
_, err := fmt.Fprintf(config.io.Out, "%s\n", project.URL)
return err
}

8
pkg/cmd/project/edit/edit_test.go
View file

@ -226,7 +226,7 @@ func TestRunUpdate_User(t *testing.T) {
assert.NoError(t, err)
assert.Equal(
t,
"Updated project http://a-url.com\n",
"http://a-url.com\n",
stdout.String())
}
@ -324,7 +324,7 @@ func TestRunUpdate_Org(t *testing.T) {
assert.NoError(t, err)
assert.Equal(
t,
"Updated project http://a-url.com\n",
"http://a-url.com\n",
stdout.String())
}
@ -412,7 +412,7 @@ func TestRunUpdate_Me(t *testing.T) {
assert.NoError(t, err)
assert.Equal(
t,
"Updated project http://a-url.com\n",
"http://a-url.com\n",
stdout.String())
}
@ -507,6 +507,6 @@ func TestRunUpdate_OmitParams(t *testing.T) {
assert.NoError(t, err)
assert.Equal(
t,
"Updated project http://a-url.com\n",
"http://a-url.com\n",
stdout.String())
}

9
pkg/cmd/repo/sync/http.go
View file

@ -6,6 +6,7 @@ import (
"errors"
"fmt"
"net/http"
"regexp"
"github.com/cli/cli/v2/api"
"github.com/cli/cli/v2/internal/ghrepo"
@ -31,7 +32,8 @@ func latestCommit(client *api.Client, repo ghrepo.Interface, branch string) (com
type upstreamMergeErr struct{ error }
var upstreamMergeUnavailableErr = upstreamMergeErr{errors.New("upstream merge API is unavailable")}
var missingWorkflowScopeRE = regexp.MustCompile("refusing to allow.*without `workflow` scope")
var missingWorkflowScopeErr = errors.New("Upstream commits contain workflow changes, which require the `workflow` scope to merge. To request it, run: gh auth refresh -s workflow")
func triggerUpstreamMerge(client *api.Client, repo ghrepo.Interface, branch string) (string, error) {
var payload bytes.Buffer
@ -52,9 +54,10 @@ func triggerUpstreamMerge(client *api.Client, repo ghrepo.Interface, branch stri
if errors.As(err, &httpErr) {
switch httpErr.StatusCode {
case http.StatusUnprocessableEntity, http.StatusConflict:
if missingWorkflowScopeRE.MatchString(httpErr.Message) {
return "", missingWorkflowScopeErr
}
return "", upstreamMergeErr{errors.New(httpErr.Message)}
case http.StatusNotFound:
return "", upstreamMergeUnavailableErr
}
}
return "", err

12
pkg/cmd/repo/sync/sync.go
View file

@ -284,6 +284,13 @@ func executeLocalRepoSync(srcRepo ghrepo.Interface, remote string, opts *SyncOpt
return nil
}
// ExecuteRemoteRepoSync will take several steps to sync the source and destination repositories.
// First it will try to use the merge-upstream API endpoint. If this fails due to merge conflicts
// or unknown merge issues then it will fallback to using the low level git references API endpoint.
// The reason the fallback is necessary is to better support these error cases. The git references API
// endpoint allows us to sync repositories that are not fast-forward merge compatible. Additionally,
// the git references API endpoint gives more detailed error responses as to why the sync failed.
// Unless the --force flag is specified we will not perform non-fast-forward merges.
func executeRemoteRepoSync(client *api.Client, destRepo, srcRepo ghrepo.Interface, opts *SyncOptions) (string, error) {
branchName := opts.Branch
if branchName == "" {
@ -317,8 +324,9 @@ func executeRemoteRepoSync(client *api.Client, destRepo, srcRepo ghrepo.Interfac
return "", err
}
// This is not a great way to detect the error returned by the API
// Unfortunately API returns 422 for multiple reasons
// Using string comparison is a brittle way to determine the error returned by the API
// endpoint but unfortunately the API returns 422 for many reasons so we must
// interpret the message provide better error messaging for our users.
err = syncFork(client, destRepo, branchName, commit.Object.SHA, opts.Force)
var httpErr api.HTTPError
if err != nil {

20
pkg/cmd/repo/sync/sync_test.go
View file

@ -292,7 +292,7 @@ func Test_SyncRun(t *testing.T) {
httpmock.StringResponse(`{"data":{"repository":{"defaultBranchRef":{"name": "trunk"}}}}`))
reg.Register(
httpmock.REST("POST", "repos/FORKOWNER/REPO-FORK/merge-upstream"),
httpmock.StatusStringResponse(404, `{}`))
httpmock.StatusStringResponse(422, `{}`))
reg.Register(
httpmock.REST("GET", "repos/OWNER/REPO/git/refs/heads/trunk"),
httpmock.StringResponse(`{"object":{"sha":"0xDEADBEEF"}}`))
@ -457,6 +457,24 @@ func Test_SyncRun(t *testing.T) {
wantErr: true,
errMsg: "trunk branch does not exist on OWNER/REPO-FORK repository",
},
{
name: "sync remote fork with missing workflow scope on token",
opts: &SyncOptions{
DestArg: "FORKOWNER/REPO-FORK",
},
httpStubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.GraphQL(`query RepositoryInfo\b`),
httpmock.StringResponse(`{"data":{"repository":{"defaultBranchRef":{"name": "trunk"}}}}`))
reg.Register(
httpmock.REST("POST", "repos/FORKOWNER/REPO-FORK/merge-upstream"),
httpmock.StatusJSONResponse(422, struct {
Message string `json:"message"`
}{Message: "refusing to allow an OAuth App to create or update workflow `.github/workflows/unimportant.yml` without `workflow` scope"}))
},
wantErr: true,
errMsg: "Upstream commits contain workflow changes, which require the `workflow` scope to merge. To request it, run: gh auth refresh -s workflow",
},
}
for _, tt := range tests {
reg := &httpmock.Registry{}

4
pkg/cmd/root/alias.go
View file

@ -31,6 +31,10 @@ func NewCmdShellAlias(io *iostreams.IOStreams, aliasName, aliasValue string) *co
externalCmd.Stdin = io.In
preparedCmd := run.PrepareCmd(externalCmd)
if err = preparedCmd.Run(); err != nil {
var execError *exec.ExitError
if errors.As(err, &execError) {
return &ExternalCommandExitError{execError}
}
return fmt.Errorf("failed to run external command: %w\n", err)
}
return nil

10
pkg/cmd/root/extension.go
View file

@ -1,13 +1,19 @@
package root
import (
"errors"
"fmt"
"os/exec"
"github.com/cli/cli/v2/pkg/extensions"
"github.com/cli/cli/v2/pkg/iostreams"
"github.com/spf13/cobra"
)
type ExternalCommandExitError struct {
*exec.ExitError
}
func NewCmdExtension(io *iostreams.IOStreams, em extensions.ExtensionManager, ext extensions.Extension) *cobra.Command {
return &cobra.Command{
Use: ext.Name(),
@ -15,6 +21,10 @@ func NewCmdExtension(io *iostreams.IOStreams, em extensions.ExtensionManager, ex
RunE: func(c *cobra.Command, args []string) error {
args = append([]string{ext.Name()}, args...)
if _, err := em.Dispatch(args, io.In, io.Out, io.ErrOut); err != nil {
var execError *exec.ExitError
if errors.As(err, &execError) {
return &ExternalCommandExitError{execError}
}
return fmt.Errorf("failed to run extension: %w\n", err)
}
return nil

21
pkg/httpmock/stub.go
View file

@ -143,7 +143,20 @@ func StatusStringResponse(status int, body string) Responder {
func JSONResponse(body interface{}) Responder {
return func(req *http.Request) (*http.Response, error) {
b, _ := json.Marshal(body)
return httpResponse(200, req, bytes.NewBuffer(b)), nil
header := http.Header{
"Content-Type": []string{"application/json"},
}
return httpResponseWithHeader(200, req, bytes.NewBuffer(b), header), nil
}
}
func StatusJSONResponse(status int, body interface{}) Responder {
return func(req *http.Request) (*http.Response, error) {
b, _ := json.Marshal(body)
header := http.Header{
"Content-Type": []string{"application/json"},
}
return httpResponseWithHeader(status, req, bytes.NewBuffer(b), header), nil
}
}
@ -215,10 +228,14 @@ func ScopesResponder(scopes string) func(*http.Request) (*http.Response, error)
}
func httpResponse(status int, req *http.Request, body io.Reader) *http.Response {
return httpResponseWithHeader(status, req, body, http.Header{})
}
func httpResponseWithHeader(status int, req *http.Request, body io.Reader, header http.Header) *http.Response {
return &http.Response{
StatusCode: status,
Request: req,
Body: io.NopCloser(body),
Header: http.Header{},
Header: header,
}
}