diff --git a/internal/io/pending_command.go b/internal/io/pending_command.go index aa1d7ee9..846faabc 100644 --- a/internal/io/pending_command.go +++ b/internal/io/pending_command.go @@ -12,10 +12,11 @@ type PendingOutput string // Pending is used to show a loading spinner while a long running function runs to perform some action and output // information type Pending struct { - spinner spinner.Model - quitting bool + Err error fn tea.Cmd output string + quitting bool + spinner spinner.Model } // Init implements tea.Model. @@ -46,6 +47,7 @@ func (p Pending) Update(msg tea.Msg) (tea.Model, tea.Cmd) { case error: // an error occurred somewhere. show the error and exit p.quitting = true p.output = "Error: " + msg.Error() + p.Err = msg return p, tea.Quit default: // start/update the spinner var cmd tea.Cmd diff --git a/internal/pipeline/resolver/cli.go b/internal/pipeline/resolver/cli.go new file mode 100644 index 00000000..6ec11104 --- /dev/null +++ b/internal/pipeline/resolver/cli.go @@ -0,0 +1,66 @@ +package resolver + +import ( + "fmt" + "net/url" + "strings" + + "github.com/buildkite/cli/v3/internal/config" + "github.com/buildkite/cli/v3/internal/pipeline" +) + +func ResolveFromPositionalArgument(args []string, index int, conf *config.Config) PipelineResolverFn { + return func() (*pipeline.Pipeline, error) { + // if args does not have values, skip this resolver + if len(args) < 1 { + return nil, nil + } + // if the index is out of bounds + if len(args) < index { + return nil, nil + } + + org, name := parsePipelineArg(args[index], conf) + + // if we get here, we should be able to parse the value and return an error if not + // this is because a user has explicitly given an input value for us to use - we shoulnt ignore it on error + if org == "" || name == "" { + return nil, fmt.Errorf("Not able to parse the input pipeline argument: \"%s\"", args[index]) + } + + return &pipeline.Pipeline{Name: name, Org: org}, nil + } +} + +// parsePipelineArg resolve an input string in varying formats to an organization and pipeline pair +// some example input formats are: +// - a web URL: https://buildkite.com///builds/... +// - a slug: / +// - a pipeline slug by itself +func parsePipelineArg(arg string, conf *config.Config) (org, pipeline string) { + pipelineIsURL := strings.Contains(arg, ":") + pipelineIsSlug := !pipelineIsURL && strings.Contains(arg, "/") + + if pipelineIsURL { + url, err := url.Parse(arg) + if err != nil { + return "", "" + } + // eg: url.Path = /buildkite/buildkite-cli + part := strings.Split(url.Path, "/") + if len(part) < 3 { + return "", "" + } + org, pipeline = part[1], part[2] + } else if pipelineIsSlug { + part := strings.Split(arg, "/") + if len(part) < 2 { + return "", "" + } + org, pipeline = part[0], part[1] + } else { + org = conf.Organization + pipeline = arg + } + return org, pipeline +} diff --git a/pkg/cmd/build/build_test.go b/internal/pipeline/resolver/cli_test.go similarity index 54% rename from pkg/cmd/build/build_test.go rename to internal/pipeline/resolver/cli_test.go index 4099ade4..ff2a7664 100644 --- a/pkg/cmd/build/build_test.go +++ b/internal/pipeline/resolver/cli_test.go @@ -1,9 +1,10 @@ -package build +package resolver_test import ( "testing" "github.com/buildkite/cli/v3/internal/config" + "github.com/buildkite/cli/v3/internal/pipeline/resolver" ) func TestParsePipelineArg(t *testing.T) { @@ -37,13 +38,33 @@ func TestParsePipelineArg(t *testing.T) { c := config.Config{ Organization: "testing", } - org, agent := parsePipelineArg(testcase.url, &c) - if org != testcase.org { + f := resolver.ResolveFromPositionalArgument([]string{testcase.url}, 0, &c) + pipeline, err := f() + if err != nil { + t.Error(err) + } + if pipeline.Org != testcase.org { t.Error("parsed organization slug did not match expected") } - if agent != testcase.pipeline { + if pipeline.Name != testcase.pipeline { t.Error("parsed pipeline name did not match expected") } }) } + + t.Run("Returns error if failed parsing", func(t *testing.T) { + t.Parallel() + + c := config.Config{ + Organization: "testing", + } + f := resolver.ResolveFromPositionalArgument([]string{"https://buildkite.com/"}, 0, &c) + pipeline, err := f() + if err == nil { + t.Error("Should have failed parsing pipeline") + } + if pipeline != nil { + t.Error("No pipeline should be returned") + } + }) } diff --git a/pkg/cmd/build/build.go b/pkg/cmd/build/build.go index 77073357..8a6b804c 100644 --- a/pkg/cmd/build/build.go +++ b/pkg/cmd/build/build.go @@ -2,11 +2,8 @@ package build import ( "fmt" - "net/url" - "strings" "github.com/MakeNowJust/heredoc" - "github.com/buildkite/cli/v3/internal/config" "github.com/buildkite/cli/v3/pkg/cmd/factory" "github.com/buildkite/cli/v3/pkg/cmd/validation" "github.com/charmbracelet/lipgloss" @@ -21,14 +18,14 @@ func NewCmdBuild(f *factory.Factory) *cobra.Command { Long: "Work with Buildkite pipeline builds.", Example: heredoc.Doc(` # To create a new build - $ bk build new -m "Build from cli" -c "HEAD" -b "main" + $ bk build new -m "Build from cli" -c "HEAD" -b "main" `), PersistentPreRunE: validation.CheckValidConfiguration(f.Config), Annotations: map[string]string{ "help:arguments": heredoc.Doc(` A pipeline is passed as an argument. It can be supplied in any of the following formats: - "PIPELINE_SLUG" - - "ORGANIZATION_SLUG/PIPELINE_SLUG" + - "ORGANIZATION_SLUG/PIPELINE_SLUG" `), }, } @@ -41,29 +38,6 @@ func NewCmdBuild(f *factory.Factory) *cobra.Command { return &cmd } -func parsePipelineArg(arg string, conf *config.Config) (string, string) { - var org, pipeline string - pipelineIsURL := strings.Contains(arg, ":") - pipelineIsSlug := !pipelineIsURL && strings.Contains(arg, "/") - - if pipelineIsURL { - url, err := url.Parse(arg) - if err != nil { - return "", "" - } - // eg: url.Path = /buildkite/buildkite-cli - part := strings.Split(url.Path, "/") - org, pipeline = part[1], part[2] - } else if pipelineIsSlug { - part := strings.Split(arg, "/") - org, pipeline = part[0], part[1] - } else { - org = conf.Organization - pipeline = arg - } - return org, pipeline -} - func openBuildInBrowser(openInWeb bool, webUrl string) error { if openInWeb { diff --git a/pkg/cmd/build/cancel.go b/pkg/cmd/build/cancel.go index d7f50bfa..d2d42c86 100644 --- a/pkg/cmd/build/cancel.go +++ b/pkg/cmd/build/cancel.go @@ -30,7 +30,7 @@ func NewCmdBuildCancel(f *factory.Factory) *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { buildId := args[0] resolvers := resolver.NewAggregateResolver( - pipelineResolverPositionArg(args[1:], f.Config), + resolver.ResolveFromPositionalArgument(args, 1, f.Config), resolver.ResolveFromPath("", f.Config.Organization, f.RestAPIClient), ) var pipeline pipeline.Pipeline @@ -44,10 +44,13 @@ func NewCmdBuildCancel(f *factory.Factory) *cobra.Command { return io.PendingOutput(fmt.Sprintf("Resolved pipeline to: %s", pipeline.Name)) }, "Resolving pipeline") p := tea.NewProgram(r) - _, err := p.Run() + finalModel, err := p.Run() if err != nil { return err } + if finalModel.(io.Pending).Err != nil { + return finalModel.(io.Pending).Err + } return cancelBuild(pipeline.Org, pipeline.Name, buildId, web, f) }, } diff --git a/pkg/cmd/build/new.go b/pkg/cmd/build/new.go index bb0fb785..33d41eec 100644 --- a/pkg/cmd/build/new.go +++ b/pkg/cmd/build/new.go @@ -4,7 +4,6 @@ import ( "fmt" "github.com/MakeNowJust/heredoc" - "github.com/buildkite/cli/v3/internal/config" "github.com/buildkite/cli/v3/internal/io" "github.com/buildkite/cli/v3/internal/pipeline" "github.com/buildkite/cli/v3/internal/pipeline/resolver" @@ -34,7 +33,7 @@ func NewCmdBuildNew(f *factory.Factory) *cobra.Command { `), RunE: func(cmd *cobra.Command, args []string) error { resolvers := resolver.NewAggregateResolver( - pipelineResolverPositionArg(args, f.Config), + resolver.ResolveFromPositionalArgument(args, 0, f.Config), resolver.ResolveFromPath("", f.Config.Organization, f.RestAPIClient), ) var pipeline pipeline.Pipeline @@ -48,10 +47,13 @@ func NewCmdBuildNew(f *factory.Factory) *cobra.Command { return io.PendingOutput(fmt.Sprintf("Resolved pipeline to: %s", pipeline.Name)) }, "Resolving pipeline") p := tea.NewProgram(r) - _, err := p.Run() + finalModel, err := p.Run() if err != nil { return err } + if finalModel.(io.Pending).Err != nil { + return finalModel.(io.Pending).Err + } return newBuild(pipeline.Org, pipeline.Name, f, message, commit, branch, web) }, } @@ -64,24 +66,6 @@ func NewCmdBuildNew(f *factory.Factory) *cobra.Command { return &cmd } -func pipelineResolverPositionArg(args []string, conf *config.Config) resolver.PipelineResolverFn { - return func() (*pipeline.Pipeline, error) { - // if args does not have values, skip this resolver - if len(args) < 1 { - return nil, nil - } - - org, name := parsePipelineArg(args[0], conf) - // if we could not parse the pipeline from the arg then return no pipeline or error, to pass indicate to pass to - // the next resolver in the chain - if org == "" || name == "" { - return nil, nil - } - - return &pipeline.Pipeline{Name: name, Org: org}, nil - } -} - func newBuild(org string, pipeline string, f *factory.Factory, message string, commit string, branch string, web bool) error { l := io.NewPendingCommand(func() tea.Msg { diff --git a/pkg/cmd/build/rebuild.go b/pkg/cmd/build/rebuild.go index 6c5c2ba4..779ae039 100644 --- a/pkg/cmd/build/rebuild.go +++ b/pkg/cmd/build/rebuild.go @@ -30,7 +30,7 @@ func NewCmdBuildRebuild(f *factory.Factory) *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { buildId := args[0] resolvers := resolver.NewAggregateResolver( - pipelineResolverPositionArg(args[1:], f.Config), + resolver.ResolveFromPositionalArgument(args, 1, f.Config), resolver.ResolveFromPath("", f.Config.Organization, f.RestAPIClient), ) var pipeline pipeline.Pipeline @@ -44,10 +44,13 @@ func NewCmdBuildRebuild(f *factory.Factory) *cobra.Command { return io.PendingOutput(fmt.Sprintf("Resolved pipeline to: %s", pipeline.Name)) }, "Resolving pipeline") p := tea.NewProgram(r) - _, err := p.Run() + finalModel, err := p.Run() if err != nil { return err } + if finalModel.(io.Pending).Err != nil { + return finalModel.(io.Pending).Err + } return rebuild(pipeline.Org, pipeline.Name, buildId, web, f) }, } diff --git a/pkg/cmd/build/view.go b/pkg/cmd/build/view.go index 7bf0b670..7876fbb5 100644 --- a/pkg/cmd/build/view.go +++ b/pkg/cmd/build/view.go @@ -39,7 +39,7 @@ func NewCmdBuildView(f *factory.Factory) *cobra.Command { var buildAnnotations = make([]buildkite.Annotation, 0) buildId := args[0] resolvers := resolver.NewAggregateResolver( - pipelineResolverPositionArg(args[1:], f.Config), + resolver.ResolveFromPositionalArgument(args, 1, f.Config), resolver.ResolveFromPath("", f.Config.Organization, f.RestAPIClient), ) @@ -55,10 +55,13 @@ func NewCmdBuildView(f *factory.Factory) *cobra.Command { return io.PendingOutput(fmt.Sprintf("Resolved pipeline to: %s", pipeline.Name)) }, "Resolving pipeline") p := tea.NewProgram(r) - _, err := p.Run() + finalModel, err := p.Run() if err != nil { return err } + if finalModel.(io.Pending).Err != nil { + return finalModel.(io.Pending).Err + } l := io.NewPendingCommand(func() tea.Msg { var buildUrl string