From b12d2dcc8835fe32d89d1b0c39a96551767c7398 Mon Sep 17 00:00:00 2001 From: Arik Kfir Date: Tue, 11 Jun 2024 13:22:23 +0300 Subject: [PATCH] feat(command): support non-executable commands This change provides support for fully-intermediate commands, that have no execution logic of their own - and only function as groups of other actions - though they can still have pre-run hooks, and also potential configurations (that may or may not be inherited). --- command.go | 56 +++++++++++++++++++++++----- command_test.go | 97 ++++++++++++++++++++++++++----------------------- execute.go | 53 +++++++++------------------ execute_test.go | 65 +++++++++++++++++---------------- flag_set.go | 16 ++++---- 5 files changed, 157 insertions(+), 130 deletions(-) diff --git a/command.go b/command.go index d15576a..c297139 100644 --- a/command.go +++ b/command.go @@ -1,6 +1,7 @@ package command import ( + "context" "errors" "fmt" "io" @@ -18,13 +19,42 @@ type HelpConfig struct { Help bool `inherited:"true" desc:"Show this help screen and exit."` } +type Action interface { + Run(context.Context) error +} + +type ActionFunc func(context.Context) error + +func (i ActionFunc) Run(ctx context.Context) error { + if i != nil { + return i(ctx) + } else { + return nil + } +} + +type PreRunHook interface { + PreRun(context.Context) error +} + +type PreRunHookFunc func(context.Context) error + +func (i PreRunHookFunc) PreRun(ctx context.Context) error { + if i != nil { + return i(ctx) + } else { + return nil + } +} + // Command is a command instance, created by [New] and can be composed with more Command instances to form a CLI command // hierarchy. type Command struct { name string shortDescription string longDescription string - executor Executor + preRunHooks []PreRunHook + action Action flags *flagSet parent *Command subCommands []*Command @@ -34,8 +64,8 @@ type Command struct { // MustNew creates a new command using [New], but will panic if it returns an error. // //goland:noinspection GoUnusedExportedFunction -func MustNew(name, shortDescription, longDescription string, executor Executor, subCommands ...*Command) *Command { - cmd, err := New(name, shortDescription, longDescription, executor, subCommands...) +func MustNew(name, shortDescription, longDescription string, action Action, preRunHooks []PreRunHook, subCommands ...*Command) *Command { + cmd, err := New(name, shortDescription, longDescription, action, preRunHooks, subCommands...) if err != nil { panic(err) } @@ -44,13 +74,11 @@ func MustNew(name, shortDescription, longDescription string, executor Executor, // New creates a new command with the given name, short & long descriptions, and the given executor. The executor object // is also scanned for configuration structs via reflection. -func New(name, shortDescription, longDescription string, executor Executor, subCommands ...*Command) (*Command, error) { +func New(name, shortDescription, longDescription string, action Action, preRunHooks []PreRunHook, subCommands ...*Command) (*Command, error) { if name == "" { return nil, fmt.Errorf("%w: empty name", ErrInvalidCommand) } else if shortDescription == "" { return nil, fmt.Errorf("%w: empty short description", ErrInvalidCommand) - } else if executor == nil { - return nil, fmt.Errorf("%w: nil executor", ErrInvalidCommand) } // Create the command instance @@ -58,7 +86,8 @@ func New(name, shortDescription, longDescription string, executor Executor, subC name: name, shortDescription: shortDescription, longDescription: longDescription, - executor: executor, + action: action, + preRunHooks: preRunHooks, HelpConfig: &HelpConfig{}, } @@ -84,14 +113,21 @@ func (c *Command) setParent(parent *Command) error { var parentFlags *flagSet if parent != nil { parentFlags = parent.flags - } else if fs, err := newFlagSet(nil, reflect.ValueOf(c).Elem().FieldByName("HelpConfig")); err != nil { + } else if parentFlagSet, err := newFlagSet(nil, reflect.ValueOf(c).Elem().FieldByName("HelpConfig")); err != nil { return fmt.Errorf("failed creating Help flag set: %w", err) } else { - parentFlags = fs + parentFlags = parentFlagSet } // Create the flag-set - if fs, err := newFlagSet(parentFlags, reflect.ValueOf(c.executor)); err != nil { + var configObjects []reflect.Value + if c.action != nil { + configObjects = append(configObjects, reflect.ValueOf(c.action)) + } + for _, hook := range c.preRunHooks { + configObjects = append(configObjects, reflect.ValueOf(hook)) + } + if fs, err := newFlagSet(parentFlags, configObjects...); err != nil { return fmt.Errorf("failed creating flag-set for command '%s': %w", c.name, err) } else { c.parent = parent diff --git a/command_test.go b/command_test.go index 07af71d..718af4c 100644 --- a/command_test.go +++ b/command_test.go @@ -25,25 +25,19 @@ func TestNew(t *testing.T) { testCases := map[string]testCase{ "empty name": { commandFactory: func(t T, tc *testCase) (*Command, error) { - return New("", "short desc", "long desc", InlineExecutor{}) + return New("", "short desc", "long desc", nil, nil) }, expectedError: `^invalid command: empty name$`, }, "empty short description": { commandFactory: func(t T, tc *testCase) (*Command, error) { - return New("cmd", "", "long desc", InlineExecutor{}) + return New("cmd", "", "long desc", nil, nil) }, expectedError: `^invalid command: empty short description$`, }, - "nil executor": { - commandFactory: func(t T, tc *testCase) (*Command, error) { - return New("cmd", "desc", "long desc", nil) - }, - expectedError: `^invalid command: nil executor$`, - }, "no flags": { commandFactory: func(t T, tc *testCase) (*Command, error) { - return New("cmd", "desc", "long desc", InlineExecutor{}) + return New("cmd", "desc", "long desc", nil, nil) }, expectedName: "cmd", expectedShortDescription: "desc", @@ -56,9 +50,10 @@ func TestNew(t *testing.T) { "desc", "long desc", &struct { - InlineExecutor + Action MyFlag string `flag:"true"` }{}, + nil, ) }, expectedFlagSet: &flagSet{ @@ -101,13 +96,13 @@ func TestNew(t *testing.T) { func TestAddSubCommand(t *testing.T) { t.Parallel() - root, err := New("root", "desc", "description", &InlineExecutor{}) + root, err := New("root", "desc", "description", nil, nil) With(t).Verify(err).Will(BeNil()).OrFail() - sub1, err := New("sub1", "sub1 desc", "sub1 description", &InlineExecutor{}) + sub1, err := New("sub1", "sub1 desc", "sub1 description", nil, nil) With(t).Verify(err).Will(BeNil()).OrFail() - sub2, err := New("sub2", "sub2 desc", "sub2 description", &InlineExecutor{}) + sub2, err := New("sub2", "sub2 desc", "sub2 description", nil, nil) With(t).Verify(err).Will(BeNil()).OrFail() With(t).Verify(root.AddSubCommand(sub1)).Will(BeNil()).OrFail() @@ -128,10 +123,10 @@ func Test_inferCommandAndArgs(t *testing.T) { testCases := map[string]testCase{ "No arguments": { root: MustNew( - "root", "desc", "description", &InlineExecutor{}, - MustNew("sub1", "sub1 desc", "sub1 description", &InlineExecutor{}, - MustNew("sub2", "sub2 desc", "sub2 description", &InlineExecutor{}, - MustNew("sub3", "sub3 desc", "sub3 description", &InlineExecutor{}), + "root", "desc", "description", nil, nil, + MustNew("sub1", "sub1 desc", "sub1 description", nil, nil, + MustNew("sub2", "sub2 desc", "sub2 description", nil, nil, + MustNew("sub3", "sub3 desc", "sub3 description", nil, nil), ), ), ), @@ -142,9 +137,9 @@ func Test_inferCommandAndArgs(t *testing.T) { }, "Flags for root command": { root: MustNew( - "root", "desc", "description", &InlineExecutor{}, - MustNew("sub1", "sub1 desc", "sub1 description", &InlineExecutor{}, - MustNew("sub2", "sub2 desc", "sub2 description", &InlineExecutor{}), + "root", "desc", "description", nil, nil, + MustNew("sub1", "sub1 desc", "sub1 description", nil, nil, + MustNew("sub2", "sub2 desc", "sub2 description", nil, nil), ), ), args: strings.Split("-f1 -f2", " "), @@ -154,9 +149,9 @@ func Test_inferCommandAndArgs(t *testing.T) { }, "Flags and positionals for root command": { root: MustNew( - "root", "desc", "description", &InlineExecutor{}, - MustNew("sub1", "sub1 desc", "sub1 description", &InlineExecutor{}, - MustNew("sub2", "sub2 desc", "sub2 description", &InlineExecutor{}), + "root", "desc", "description", nil, nil, + MustNew("sub1", "sub1 desc", "sub1 description", nil, nil, + MustNew("sub2", "sub2 desc", "sub2 description", nil, nil), ), ), args: strings.Split("-f1 a -f2 b", " "), @@ -166,9 +161,9 @@ func Test_inferCommandAndArgs(t *testing.T) { }, "Flags and positionals for sub1 command": { root: MustNew( - "root", "desc", "description", &InlineExecutor{}, - MustNew("sub1", "sub1 desc", "sub1 description", &InlineExecutor{}, - MustNew("sub2", "sub2 desc", "sub2 description", &InlineExecutor{}), + "root", "desc", "description", nil, nil, + MustNew("sub1", "sub1 desc", "sub1 description", nil, nil, + MustNew("sub2", "sub2 desc", "sub2 description", nil, nil), ), ), args: strings.Split("-f1 sub1 -f2 a b", " "), @@ -178,9 +173,9 @@ func Test_inferCommandAndArgs(t *testing.T) { }, "Flags and positionals for sub2 command": { root: MustNew( - "root", "desc", "description", &InlineExecutor{}, - MustNew("sub1", "sub1 desc", "sub1 description", &InlineExecutor{}, - MustNew("sub2", "sub2 desc", "sub2 description", &InlineExecutor{}), + "root", "desc", "description", nil, nil, + MustNew("sub1", "sub1 desc", "sub1 description", nil, nil, + MustNew("sub2", "sub2 desc", "sub2 description", nil, nil), ), ), args: strings.Split("-f1 sub1 -f2 a b sub2 c", " "), @@ -205,10 +200,10 @@ func Test_getFullName(t *testing.T) { cmd *Command expectedFullName string } - sub3 := MustNew("sub3", "sub3 desc", "sub3 description", &InlineExecutor{}) - sub2 := MustNew("sub2", "sub2 desc", "sub2 description", &InlineExecutor{}, sub3) - sub1 := MustNew("sub1", "sub1 desc", "sub1 description", &InlineExecutor{}, sub2) - root := MustNew("root", "desc", "description", &InlineExecutor{}, sub1) + sub3 := MustNew("sub3", "sub3 desc", "sub3 description", nil, nil) + sub2 := MustNew("sub2", "sub2 desc", "sub2 description", nil, nil, sub3) + sub1 := MustNew("sub1", "sub1 desc", "sub1 description", nil, nil, sub2) + root := MustNew("root", "desc", "description", nil, nil, sub1) testCases := map[string]testCase{ "root": { cmd: root, @@ -240,10 +235,10 @@ func Test_getChain(t *testing.T) { cmd *Command expectedChain []string } - sub3 := MustNew("sub3", "sub3 desc", "sub3 description", &InlineExecutor{}) - sub2 := MustNew("sub2", "sub2 desc", "sub2 description", &InlineExecutor{}, sub3) - sub1 := MustNew("sub1", "sub1 desc", "sub1 description", &InlineExecutor{}, sub2) - root := MustNew("root", "desc", "description", &InlineExecutor{}, sub1) + sub3 := MustNew("sub3", "sub3 desc", "sub3 description", nil, nil) + sub2 := MustNew("sub2", "sub2 desc", "sub2 description", nil, nil, sub3) + sub1 := MustNew("sub1", "sub1 desc", "sub1 description", nil, nil, sub2) + root := MustNew("root", "desc", "description", nil, nil, sub1) testCases := map[string]testCase{ "root": { cmd: root, @@ -286,7 +281,7 @@ func TestPrintHelp(t *testing.T) { "no flags & no positionals": { commandFactory: func(*testCase) *Command { ligen := loremipsum.NewWithSeed(4321) - return MustNew("cmd", ligen.Sentence(), ligen.Sentences(2), InlineExecutor{}) + return MustNew("cmd", ligen.Sentence(), ligen.Sentences(2), nil, nil) }, expectedHelpUsageOutput: ` Usage: cmd [--help] @@ -320,11 +315,16 @@ Flags: "with flags, args": { commandFactory: func(*testCase) *Command { ligen := loremipsum.NewWithSeed(4321) - return MustNew("cmd", ligen.Sentence(), ligen.Sentences(2), &struct { - InlineExecutor - MyFlag string `desc:"flag description"` - Args []string `args:"true"` - }{}) + return MustNew( + "cmd", + ligen.Sentence(), + ligen.Sentences(2), + &struct { + Action + MyFlag string `desc:"flag description"` + Args []string `args:"true"` + }{}, + nil) }, expectedHelpUsageOutput: ` Usage: cmd [--help] @@ -369,16 +369,21 @@ Flags: ligen.Sentence(), ligen.Sentences(2), &struct { - InlineExecutor + Action MyFlag string `desc:"flag description"` Args []string `args:"true"` }{}, + nil, MustNew( - "child1", ligen.Sentence(), ligen.Sentences(2), &struct { - InlineExecutor + "child1", + ligen.Sentence(), + ligen.Sentences(2), + &struct { + Action SubFlag string `desc:"sub flag description"` Args []string `args:"true"` }{}, + nil, ), ) }, diff --git a/execute.go b/execute.go index 78a9a62..4869431 100644 --- a/execute.go +++ b/execute.go @@ -15,33 +15,6 @@ const ( ExitCodeMisconfiguration ExitCode = 2 ) -// Executor is the interface to be implemented by custom commands. -type Executor interface { - PreRun(ctx context.Context) error - Run(ctx context.Context) error -} - -type InlineExecutor struct { - PreRunFunc func(context.Context) error - RunFunc func(context.Context) error -} - -func (i InlineExecutor) PreRun(ctx context.Context) error { - if i.PreRunFunc != nil { - return i.PreRunFunc(ctx) - } else { - return nil - } -} - -func (i InlineExecutor) Run(ctx context.Context) error { - if i.PreRunFunc != nil { - return i.RunFunc(ctx) - } else { - return nil - } -} - // Execute the correct command in the given command hierarchy (starting at "root"), configured from the given CLI args // and environment variables. The command will be executed with the given context after all pre-RunFunc hooks have been // successfully executed in the command hierarchy. @@ -75,17 +48,27 @@ func Execute(ctx context.Context, w io.Writer, root *Command, args []string, env // Invoke all "PreRun" hooks on the whole chain of commands (starting at the root) for _, c := range cmd.getChain() { - if err := c.executor.PreRun(ctx); err != nil { - _, _ = fmt.Fprintln(w, err) - return ExitCodeError + for _, hook := range c.preRunHooks { + if err := hook.PreRun(ctx); err != nil { + _, _ = fmt.Fprintln(w, err) + return ExitCodeError + } } } - // Run the command - if err := cmd.executor.Run(ctx); err != nil { - _, _ = fmt.Fprintln(w, err) - return ExitCodeError + // Run the command or print help screen if it's not a command + if cmd.action != nil { + if err := cmd.action.Run(ctx); err != nil { + _, _ = fmt.Fprintln(w, err) + return ExitCodeError + } + } else { + // Command is not a runner - print help + if err := cmd.PrintHelp(w, getTerminalWidth()); err != nil { + _, _ = fmt.Fprintf(w, "%s\n", err) + return ExitCodeError + } } - return ExitCodeSuccess + } diff --git a/execute_test.go b/execute_test.go index 16019a0..0301d3d 100644 --- a/execute_test.go +++ b/execute_test.go @@ -10,35 +10,36 @@ import ( . "github.com/arikkfir/justest" ) -type TrackingExecutor struct { - preRunCalled *time.Time - preRunErrorToReturn error - runCalled *time.Time - runErrorToReturn error +type TrackingAction struct { + callTime *time.Time + errorToReturnOnCall error } -func (te *TrackingExecutor) PreRun(_ context.Context) error { - te.preRunCalled = ptrOf(time.Now()) +func (a *TrackingAction) Run(_ context.Context) error { + a.callTime = ptrOf(time.Now()) time.Sleep(100 * time.Millisecond) - return te.preRunErrorToReturn + return a.errorToReturnOnCall } -func (te *TrackingExecutor) Run(_ context.Context) error { - te.runCalled = ptrOf(time.Now()) - time.Sleep(100 * time.Millisecond) - return te.runErrorToReturn +type TrackingPreRunHook struct { + callTime *time.Time + errorToReturnOnCall error } -type ExecutorWithFlag struct { - MyFlag string `name:"my-flag"` +func (a *TrackingPreRunHook) PreRun(_ context.Context) error { + a.callTime = ptrOf(time.Now()) + time.Sleep(100 * time.Millisecond) + return a.errorToReturnOnCall } -func (e *ExecutorWithFlag) PreRun(_ context.Context) error { - return nil +type ActionWithConfig struct { + TrackingAction + MyFlag string `name:"my-flag"` } -func (e *ExecutorWithFlag) Run(_ context.Context) error { - return nil +type PreRunHookWithConfig struct { + TrackingPreRunHook + MyFlag string `name:"my-flag"` } func TestExecute(t *testing.T) { @@ -46,8 +47,8 @@ func TestExecute(t *testing.T) { t.Run("command must be root", func(t *testing.T) { ctx := context.Background() - child := MustNew("child", "desc", "long desc", InlineExecutor{}) - _ = MustNew("root", "desc", "long desc", InlineExecutor{}, child) + child := MustNew("child", "desc", "long desc", nil, nil) + _ = MustNew("root", "desc", "long desc", nil, nil, child) b := &bytes.Buffer{} With(t).Verify(Execute(ctx, b, child, nil, nil)).Will(EqualTo(ExitCodeError)).OrFail() With(t).Verify(b).Will(Say(`^unsupported operation: command must be the root command$`)).OrFail() @@ -55,23 +56,23 @@ func TestExecute(t *testing.T) { t.Run("applies configuration", func(t *testing.T) { ctx := context.Background() - cmd := MustNew("cmd", "desc", "long desc", &ExecutorWithFlag{}) + cmd := MustNew("cmd", "desc", "long desc", &ActionWithConfig{}, nil) With(t).Verify(Execute(ctx, os.Stderr, cmd, []string{"--my-flag=V1"}, nil)).Will(EqualTo(ExitCodeSuccess)).OrFail() - With(t).Verify(cmd.executor.(*ExecutorWithFlag).MyFlag).Will(EqualTo("V1")).OrFail() + With(t).Verify(cmd.action.(*ActionWithConfig).MyFlag).Will(EqualTo("V1")).OrFail() }) t.Run("prints usage on CLI parse errors", func(t *testing.T) { ctx := context.Background() - cmd := MustNew("cmd", "desc", "long desc", &ExecutorWithFlag{}) + cmd := MustNew("cmd", "desc", "long desc", &ActionWithConfig{}, nil) b := &bytes.Buffer{} With(t).Verify(Execute(ctx, b, cmd, []string{"--bad-flag=V1"}, nil)).Will(EqualTo(ExitCodeMisconfiguration)).OrFail() - With(t).Verify(cmd.executor.(*ExecutorWithFlag).MyFlag).Will(BeEmpty()).OrFail() + With(t).Verify(cmd.action.(*ActionWithConfig).MyFlag).Will(BeEmpty()).OrFail() With(t).Verify(b.String()).Will(EqualTo("unknown flag: --bad-flag\nUsage: cmd [--help] [--my-flag=VALUE]\n")).OrFail() }) t.Run("prints help on --help flag", func(t *testing.T) { ctx := context.Background() - cmd := MustNew("cmd", "desc", "long desc", &ExecutorWithFlag{}) + cmd := MustNew("cmd", "desc", "long desc", &ActionWithConfig{}, nil) b := &bytes.Buffer{} With(t).Verify(Execute(ctx, b, cmd, []string{"--help"}, nil)).Will(EqualTo(ExitCodeSuccess)).OrFail() With(t).Verify(b.String()).Will(EqualTo(` @@ -92,23 +93,23 @@ Flags: t.Run("preRun called for command chain", func(t *testing.T) { ctx := context.Background() - sub2 := MustNew("sub2", "desc", "long desc", &TrackingExecutor{}) - sub1 := MustNew("sub1", "desc", "long desc", &TrackingExecutor{}, sub2) - root := MustNew("cmd", "desc", "long desc", &TrackingExecutor{}, sub1) + sub2 := MustNew("sub2", "desc", "long desc", &ActionWithConfig{}, []PreRunHook{&PreRunHookWithConfig{}}) + sub1 := MustNew("sub1", "desc", "long desc", &ActionWithConfig{}, []PreRunHook{&PreRunHookWithConfig{}}, sub2) + root := MustNew("cmd", "desc", "long desc", &ActionWithConfig{}, []PreRunHook{&PreRunHookWithConfig{}}, sub1) With(t).Verify(Execute(ctx, os.Stderr, root, []string{"sub1", "sub2"}, nil)).Will(EqualTo(ExitCodeSuccess)).OrFail() - sub2PreRunTime := sub2.executor.(*TrackingExecutor).preRunCalled + sub2PreRunTime := sub2.preRunHooks[0].(*PreRunHookWithConfig).callTime With(t).Verify(sub2PreRunTime).Will(Not(BeNil())).OrFail() - sub1PreRunTime := sub1.executor.(*TrackingExecutor).preRunCalled + sub1PreRunTime := sub1.preRunHooks[0].(*PreRunHookWithConfig).callTime With(t).Verify(sub1PreRunTime).Will(Not(BeNil())).OrFail() With(t).Verify(sub1PreRunTime.Before(*sub2PreRunTime)).Will(EqualTo(true)).OrFail() - rootPreRunTime := root.executor.(*TrackingExecutor).preRunCalled + rootPreRunTime := root.preRunHooks[0].(*PreRunHookWithConfig).callTime With(t).Verify(rootPreRunTime).Will(Not(BeNil())).OrFail() With(t).Verify(rootPreRunTime.Before(*sub1PreRunTime)).Will(EqualTo(true)).OrFail() - sub2RunTime := sub2.executor.(*TrackingExecutor).runCalled + sub2RunTime := sub2.action.(*ActionWithConfig).callTime With(t).Verify(sub2RunTime).Will(Not(BeNil())).OrFail() With(t).Verify(sub2RunTime.After(*sub2PreRunTime)).Will(EqualTo(true)).OrFail() }) diff --git a/flag_set.go b/flag_set.go index c04da1d..f868d60 100644 --- a/flag_set.go +++ b/flag_set.go @@ -72,14 +72,16 @@ type flagSet struct { positionalsTargets []*[]string } -func newFlagSet(parent *flagSet, valueOfConfig reflect.Value) (*flagSet, error) { +func newFlagSet(parent *flagSet, objects ...reflect.Value) (*flagSet, error) { fs := &flagSet{parent: parent} - if valueOfConfig.Kind() == reflect.Ptr && valueOfConfig.Type().Elem().Kind() == reflect.Struct { - if valueOfConfig.IsNil() { - valueOfConfig.Set(reflect.New(valueOfConfig.Type().Elem())) - } - if err := fs.readFlagsFromStruct(valueOfConfig.Elem(), false); err != nil { - return nil, err + for _, c := range objects { + if c.Kind() == reflect.Ptr && c.Type().Elem().Kind() == reflect.Struct { + if c.IsNil() { + c.Set(reflect.New(c.Type().Elem())) + } + if err := fs.readFlagsFromStruct(c.Elem(), false); err != nil { + return nil, err + } } } return fs, nil