Issue lock early arg parsing
This commit is contained in:
William Martin
parent
7744e0564f
commit
60f248458c
2 changed files with 111 additions and 34 deletions
|
|
@ -19,6 +19,7 @@ import (
|
|||
"github.com/cli/cli/v2/api"
|
||||
"github.com/cli/cli/v2/internal/gh"
|
||||
"github.com/cli/cli/v2/internal/ghrepo"
|
||||
"github.com/cli/cli/v2/pkg/cmd/issue/shared"
|
||||
issueShared "github.com/cli/cli/v2/pkg/cmd/issue/shared"
|
||||
"github.com/cli/cli/v2/pkg/cmdutil"
|
||||
"github.com/cli/cli/v2/pkg/iostreams"
|
||||
|
|
@ -99,20 +100,33 @@ type LockOptions struct {
|
|||
|
||||
ParentCmd string
|
||||
Reason string
|
||||
SelectorArg string
|
||||
IssueNumber int
|
||||
Interactive bool
|
||||
}
|
||||
|
||||
func (opts *LockOptions) setCommonOptions(f *cmdutil.Factory, args []string) {
|
||||
func (opts *LockOptions) setCommonOptions(f *cmdutil.Factory, args []string) error {
|
||||
opts.IO = f.IOStreams
|
||||
opts.HttpClient = f.HttpClient
|
||||
opts.Config = f.Config
|
||||
|
||||
// support `-R, --repo` override
|
||||
opts.BaseRepo = f.BaseRepo
|
||||
issueNumber, baseRepo, err := shared.ParseIssueFromArg(args[0])
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
opts.SelectorArg = args[0]
|
||||
// If the args provided the base repo then use that directly.
|
||||
if baseRepo, present := baseRepo.Value(); present {
|
||||
opts.BaseRepo = func() (ghrepo.Interface, error) {
|
||||
return baseRepo, nil
|
||||
}
|
||||
} else {
|
||||
// support `-R, --repo` override
|
||||
opts.BaseRepo = f.BaseRepo
|
||||
}
|
||||
|
||||
opts.IssueNumber = issueNumber
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
func NewCmdLock(f *cmdutil.Factory, parentName string, runF func(string, *LockOptions) error) *cobra.Command {
|
||||
|
|
@ -129,7 +143,9 @@ func NewCmdLock(f *cmdutil.Factory, parentName string, runF func(string, *LockOp
|
|||
Short: short,
|
||||
Args: cobra.ExactArgs(1),
|
||||
RunE: func(cmd *cobra.Command, args []string) error {
|
||||
opts.setCommonOptions(f, args)
|
||||
if err := opts.setCommonOptions(f, args); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
reasonProvided := cmd.Flags().Changed("reason")
|
||||
if reasonProvided {
|
||||
|
|
@ -172,7 +188,9 @@ func NewCmdUnlock(f *cmdutil.Factory, parentName string, runF func(string, *Lock
|
|||
Short: short,
|
||||
Args: cobra.ExactArgs(1),
|
||||
RunE: func(cmd *cobra.Command, args []string) error {
|
||||
opts.setCommonOptions(f, args)
|
||||
if err := opts.setCommonOptions(f, args); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
if runF != nil {
|
||||
return runF(Unlock, opts)
|
||||
|
|
@ -214,13 +232,18 @@ func lockRun(state string, opts *LockOptions) error {
|
|||
return err
|
||||
}
|
||||
|
||||
issuePr, baseRepo, err := issueShared.IssueFromArgWithFields(httpClient, opts.BaseRepo, opts.SelectorArg, fields())
|
||||
|
||||
parent := alias[opts.ParentCmd]
|
||||
|
||||
baseRepo, err := opts.BaseRepo()
|
||||
if err != nil {
|
||||
return err
|
||||
} else if parent.Typename != issuePr.Typename {
|
||||
}
|
||||
|
||||
issuePr, err := issueShared.FindIssueOrPR(httpClient, baseRepo, opts.IssueNumber, fields())
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
parent := alias[opts.ParentCmd]
|
||||
if parent.Typename != issuePr.Typename {
|
||||
currentType := alias[parent.Typename]
|
||||
correctType := alias[issuePr.Typename]
|
||||
|
||||
|
|
|
|||
|
|
@ -30,7 +30,7 @@ func Test_NewCmdLock(t *testing.T) {
|
|||
args: "--reason off_topic 451",
|
||||
want: LockOptions{
|
||||
Reason: "off_topic",
|
||||
SelectorArg: "451",
|
||||
IssueNumber: 451,
|
||||
},
|
||||
},
|
||||
{
|
||||
|
|
@ -41,9 +41,36 @@ func Test_NewCmdLock(t *testing.T) {
|
|||
name: "no flags",
|
||||
args: "451",
|
||||
want: LockOptions{
|
||||
SelectorArg: "451",
|
||||
IssueNumber: 451,
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "issue number argument",
|
||||
args: "451 --repo owner/repo",
|
||||
want: LockOptions{
|
||||
IssueNumber: 451,
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "argument is hash prefixed number",
|
||||
// Escaping is required here to avoid what I think is shellex treating it as a comment.
|
||||
args: "\\#451 --repo owner/repo",
|
||||
want: LockOptions{
|
||||
IssueNumber: 451,
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "argument is a URL",
|
||||
args: "https://github.com/cli/cli/issues/451",
|
||||
want: LockOptions{
|
||||
IssueNumber: 451,
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "argument cannot be parsed to an issue",
|
||||
args: "unparseable",
|
||||
wantErr: "invalid issue format: \"unparseable\"",
|
||||
},
|
||||
{
|
||||
name: "bad reason",
|
||||
args: "--reason bad 451",
|
||||
|
|
@ -60,7 +87,7 @@ func Test_NewCmdLock(t *testing.T) {
|
|||
args: "451",
|
||||
tty: true,
|
||||
want: LockOptions{
|
||||
SelectorArg: "451",
|
||||
IssueNumber: 451,
|
||||
Interactive: true,
|
||||
},
|
||||
},
|
||||
|
|
@ -99,7 +126,7 @@ func Test_NewCmdLock(t *testing.T) {
|
|||
}
|
||||
|
||||
assert.Equal(t, tt.want.Reason, opts.Reason)
|
||||
assert.Equal(t, tt.want.SelectorArg, opts.SelectorArg)
|
||||
assert.Equal(t, tt.want.IssueNumber, opts.IssueNumber)
|
||||
assert.Equal(t, tt.want.Interactive, opts.Interactive)
|
||||
})
|
||||
}
|
||||
|
|
@ -121,9 +148,36 @@ func Test_NewCmdUnlock(t *testing.T) {
|
|||
name: "no flags",
|
||||
args: "451",
|
||||
want: LockOptions{
|
||||
SelectorArg: "451",
|
||||
IssueNumber: 451,
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "issue number argument",
|
||||
args: "451 --repo owner/repo",
|
||||
want: LockOptions{
|
||||
IssueNumber: 451,
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "argument is hash prefixed number",
|
||||
// Escaping is required here to avoid what I think is shellex treating it as a comment.
|
||||
args: "\\#451 --repo owner/repo",
|
||||
want: LockOptions{
|
||||
IssueNumber: 451,
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "argument is a URL",
|
||||
args: "https://github.com/cli/cli/issues/451",
|
||||
want: LockOptions{
|
||||
IssueNumber: 451,
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "argument cannot be parsed to an issue",
|
||||
args: "unparseable",
|
||||
wantErr: "invalid issue format: \"unparseable\"",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range cases {
|
||||
|
|
@ -158,7 +212,7 @@ func Test_NewCmdUnlock(t *testing.T) {
|
|||
assert.NoError(t, err)
|
||||
}
|
||||
|
||||
assert.Equal(t, tt.want.SelectorArg, opts.SelectorArg)
|
||||
assert.Equal(t, tt.want.IssueNumber, opts.IssueNumber)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
@ -179,7 +233,7 @@ func Test_runLock(t *testing.T) {
|
|||
name: "lock issue nontty",
|
||||
state: Lock,
|
||||
opts: LockOptions{
|
||||
SelectorArg: "451",
|
||||
IssueNumber: 451,
|
||||
ParentCmd: "issue",
|
||||
},
|
||||
httpStubs: func(t *testing.T, reg *httpmock.Registry) {
|
||||
|
|
@ -203,7 +257,7 @@ func Test_runLock(t *testing.T) {
|
|||
tty: true,
|
||||
opts: LockOptions{
|
||||
Interactive: true,
|
||||
SelectorArg: "451",
|
||||
IssueNumber: 451,
|
||||
ParentCmd: "issue",
|
||||
},
|
||||
state: Lock,
|
||||
|
|
@ -241,7 +295,7 @@ func Test_runLock(t *testing.T) {
|
|||
tty: true,
|
||||
state: Lock,
|
||||
opts: LockOptions{
|
||||
SelectorArg: "451",
|
||||
IssueNumber: 451,
|
||||
ParentCmd: "issue",
|
||||
Reason: "off_topic",
|
||||
},
|
||||
|
|
@ -268,7 +322,7 @@ func Test_runLock(t *testing.T) {
|
|||
tty: true,
|
||||
state: Unlock,
|
||||
opts: LockOptions{
|
||||
SelectorArg: "451",
|
||||
IssueNumber: 451,
|
||||
ParentCmd: "issue",
|
||||
},
|
||||
httpStubs: func(t *testing.T, reg *httpmock.Registry) {
|
||||
|
|
@ -294,7 +348,7 @@ func Test_runLock(t *testing.T) {
|
|||
name: "unlock issue nontty",
|
||||
state: Unlock,
|
||||
opts: LockOptions{
|
||||
SelectorArg: "451",
|
||||
IssueNumber: 451,
|
||||
ParentCmd: "issue",
|
||||
},
|
||||
httpStubs: func(t *testing.T, reg *httpmock.Registry) {
|
||||
|
|
@ -319,7 +373,7 @@ func Test_runLock(t *testing.T) {
|
|||
name: "lock issue with explicit reason nontty",
|
||||
state: Lock,
|
||||
opts: LockOptions{
|
||||
SelectorArg: "451",
|
||||
IssueNumber: 451,
|
||||
ParentCmd: "issue",
|
||||
Reason: "off_topic",
|
||||
},
|
||||
|
|
@ -344,7 +398,7 @@ func Test_runLock(t *testing.T) {
|
|||
name: "relock issue tty",
|
||||
state: Lock,
|
||||
opts: LockOptions{
|
||||
SelectorArg: "451",
|
||||
IssueNumber: 451,
|
||||
ParentCmd: "issue",
|
||||
Reason: "off_topic",
|
||||
},
|
||||
|
|
@ -388,7 +442,7 @@ func Test_runLock(t *testing.T) {
|
|||
name: "relock issue nontty",
|
||||
state: Lock,
|
||||
opts: LockOptions{
|
||||
SelectorArg: "451",
|
||||
IssueNumber: 451,
|
||||
ParentCmd: "issue",
|
||||
Reason: "off_topic",
|
||||
},
|
||||
|
|
@ -409,7 +463,7 @@ func Test_runLock(t *testing.T) {
|
|||
name: "lock pr nontty",
|
||||
state: Lock,
|
||||
opts: LockOptions{
|
||||
SelectorArg: "451",
|
||||
IssueNumber: 451,
|
||||
ParentCmd: "pr",
|
||||
},
|
||||
httpStubs: func(t *testing.T, reg *httpmock.Registry) {
|
||||
|
|
@ -433,7 +487,7 @@ func Test_runLock(t *testing.T) {
|
|||
tty: true,
|
||||
opts: LockOptions{
|
||||
Interactive: true,
|
||||
SelectorArg: "451",
|
||||
IssueNumber: 451,
|
||||
ParentCmd: "pr",
|
||||
},
|
||||
state: Lock,
|
||||
|
|
@ -469,7 +523,7 @@ func Test_runLock(t *testing.T) {
|
|||
tty: true,
|
||||
state: Lock,
|
||||
opts: LockOptions{
|
||||
SelectorArg: "451",
|
||||
IssueNumber: 451,
|
||||
ParentCmd: "pr",
|
||||
Reason: "off_topic",
|
||||
},
|
||||
|
|
@ -495,7 +549,7 @@ func Test_runLock(t *testing.T) {
|
|||
name: "lock pr with explicit nontty",
|
||||
state: Lock,
|
||||
opts: LockOptions{
|
||||
SelectorArg: "451",
|
||||
IssueNumber: 451,
|
||||
ParentCmd: "pr",
|
||||
Reason: "off_topic",
|
||||
},
|
||||
|
|
@ -520,7 +574,7 @@ func Test_runLock(t *testing.T) {
|
|||
name: "unlock pr tty",
|
||||
state: Unlock,
|
||||
opts: LockOptions{
|
||||
SelectorArg: "451",
|
||||
IssueNumber: 451,
|
||||
ParentCmd: "pr",
|
||||
},
|
||||
httpStubs: func(t *testing.T, reg *httpmock.Registry) {
|
||||
|
|
@ -546,7 +600,7 @@ func Test_runLock(t *testing.T) {
|
|||
tty: true,
|
||||
state: Unlock,
|
||||
opts: LockOptions{
|
||||
SelectorArg: "451",
|
||||
IssueNumber: 451,
|
||||
ParentCmd: "pr",
|
||||
},
|
||||
httpStubs: func(t *testing.T, reg *httpmock.Registry) {
|
||||
|
|
@ -572,7 +626,7 @@ func Test_runLock(t *testing.T) {
|
|||
name: "relock pr tty",
|
||||
state: Lock,
|
||||
opts: LockOptions{
|
||||
SelectorArg: "451",
|
||||
IssueNumber: 451,
|
||||
ParentCmd: "pr",
|
||||
Reason: "off_topic",
|
||||
},
|
||||
|
|
@ -616,7 +670,7 @@ func Test_runLock(t *testing.T) {
|
|||
name: "relock pr nontty",
|
||||
state: Lock,
|
||||
opts: LockOptions{
|
||||
SelectorArg: "451",
|
||||
IssueNumber: 451,
|
||||
ParentCmd: "pr",
|
||||
Reason: "off_topic",
|
||||
},
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue