From 44efc2952d4de377141c058c334e8a379967ec48 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Castan=CC=83e=CC=81?= Date: Tue, 18 Nov 2014 23:44:21 +0100 Subject: [PATCH 1/7] Added Before method to command and app --- app.go | 23 ++++++++++++++---- app_test.go | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++++- command.go | 6 ++++- 3 files changed, 91 insertions(+), 6 deletions(-) diff --git a/app.go b/app.go index f4c4af8..a125afc 100644 --- a/app.go +++ b/app.go @@ -31,6 +31,9 @@ type App struct { // An action to execute before any subcommands are run, but after the context is ready // If a non-nil error is returned, no subcommands are run Before func(context *Context) error + // An action to execute after any subcommands are run, but after the subcommand has finished + // It is run regardless of Because() result + After func(context *Context) error // The action to execute when no subcommands are specified Action func(context *Context) // Execute this function if the proper command cannot be found @@ -66,7 +69,7 @@ func NewApp() *App { } // Entry point to the cli app. Parses the arguments slice and routes to the proper flag/args combination -func (a *App) Run(arguments []string) error { +func (a *App) Run(arguments []string) (err error) { // append help to commands if a.Command(helpCommand.Name) == nil && !a.HideHelp { a.Commands = append(a.Commands, helpCommand) @@ -85,7 +88,7 @@ func (a *App) Run(arguments []string) error { // parse flags set := flagSet(a.Name, a.Flags) set.SetOutput(ioutil.Discard) - err := set.Parse(arguments[1:]) + err = set.Parse(arguments[1:]) nerr := normalizeFlags(a.Flags, set) if nerr != nil { fmt.Println(nerr) @@ -115,6 +118,12 @@ func (a *App) Run(arguments []string) error { return nil } + if a.After != nil { + defer func() { + err = a.After(context) + }() + } + if a.Before != nil { err := a.Before(context) if err != nil { @@ -145,7 +154,7 @@ func (a *App) RunAndExitOnError() { } // Invokes the subcommand given the context, parses ctx.Args() to generate command-specific flags -func (a *App) RunAsSubcommand(ctx *Context) error { +func (a *App) RunAsSubcommand(ctx *Context) (err error) { // append help to commands if len(a.Commands) > 0 { if a.Command(helpCommand.Name) == nil && !a.HideHelp { @@ -162,7 +171,7 @@ func (a *App) RunAsSubcommand(ctx *Context) error { // parse flags set := flagSet(a.Name, a.Flags) set.SetOutput(ioutil.Discard) - err := set.Parse(ctx.Args().Tail()) + err = set.Parse(ctx.Args().Tail()) nerr := normalizeFlags(a.Flags, set) context := NewContext(a, set, ctx.globalSet) @@ -197,6 +206,12 @@ func (a *App) RunAsSubcommand(ctx *Context) error { } } + if a.After != nil { + defer func() { + err = a.After(context) + }() + } + if a.Before != nil { err := a.Before(context) if err != nil { diff --git a/app_test.go b/app_test.go index 81d1174..1e0b638 100644 --- a/app_test.go +++ b/app_test.go @@ -5,7 +5,7 @@ import ( "os" "testing" - "github.com/codegangsta/cli" + "github.com/imdario/cli" ) func ExampleApp() { @@ -331,6 +331,72 @@ func TestApp_BeforeFunc(t *testing.T) { } +func TestApp_AfterFunc(t *testing.T) { + afterRun, subcommandRun := false, false + afterError := fmt.Errorf("fail") + var err error + + app := cli.NewApp() + + app.After = func(c *cli.Context) error { + afterRun = true + s := c.String("opt") + if s == "fail" { + return afterError + } + + return nil + } + + app.Commands = []cli.Command{ + cli.Command{ + Name: "sub", + Action: func(c *cli.Context) { + subcommandRun = true + }, + }, + } + + app.Flags = []cli.Flag{ + cli.StringFlag{Name: "opt"}, + } + + // run with the After() func succeeding + err = app.Run([]string{"command", "--opt", "succeed", "sub"}) + + if err != nil { + t.Fatalf("Run error: %s", err) + } + + if afterRun == false { + t.Errorf("After() not executed when expected") + } + + if subcommandRun == false { + t.Errorf("Subcommand not executed when expected") + } + + // reset + afterRun, subcommandRun = false, false + + // run with the Before() func failing + err = app.Run([]string{"command", "--opt", "fail", "sub"}) + + // should be the same error produced by the Before func + if err != afterError { + t.Errorf("Run error expected, but not received") + } + + if afterRun == false { + t.Errorf("After() not executed when expected") + } + + if subcommandRun == false { + t.Errorf("Subcommand not executed when expected") + } +} + + func TestAppHelpPrinter(t *testing.T) { oldPrinter := cli.HelpPrinter defer func() { diff --git a/command.go b/command.go index 5622b38..f85bc5d 100644 --- a/command.go +++ b/command.go @@ -21,6 +21,9 @@ type Command struct { // An action to execute before any sub-subcommands are run, but after the context is ready // If a non-nil error is returned, no sub-subcommands are run Before func(context *Context) error + // An action to execute after any subcommands are run, but after the subcommand has finished + // It is run regardless of Because() result + After func(context *Context) error // The function to call when this command is invoked Action func(context *Context) // List of child commands @@ -36,7 +39,7 @@ type Command struct { // Invokes the command given the context, parses ctx.Args() to generate command-specific flags func (c Command) Run(ctx *Context) error { - if len(c.Subcommands) > 0 || c.Before != nil { + if len(c.Subcommands) > 0 || c.Before != nil || c.After != nil { return c.startApp(ctx) } @@ -134,6 +137,7 @@ func (c Command) startApp(ctx *Context) error { // set the actions app.Before = c.Before + app.After = c.After if c.Action != nil { app.Action = c.Action } else { From 3c4b583feee724d8be4eee9ccda9f57443218143 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Castan=CC=83e=CC=81?= Date: Tue, 18 Nov 2014 23:54:27 +0100 Subject: [PATCH 2/7] Action error shadowing avoided on After --- app.go | 12 ++++++++++-- app_test.go | 1 - 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/app.go b/app.go index a125afc..25f49b0 100644 --- a/app.go +++ b/app.go @@ -120,7 +120,11 @@ func (a *App) Run(arguments []string) (err error) { if a.After != nil { defer func() { - err = a.After(context) + aferr := a.After(context) + // check Action error to avoid shadowing + if err == nil { + err = aferr + } }() } @@ -208,7 +212,11 @@ func (a *App) RunAsSubcommand(ctx *Context) (err error) { if a.After != nil { defer func() { - err = a.After(context) + aferr := a.After(context) + // check Action error to avoid shadowing + if err == nil { + err = aferr + } }() } diff --git a/app_test.go b/app_test.go index 1e0b638..20978bd 100644 --- a/app_test.go +++ b/app_test.go @@ -396,7 +396,6 @@ func TestApp_AfterFunc(t *testing.T) { } } - func TestAppHelpPrinter(t *testing.T) { oldPrinter := cli.HelpPrinter defer func() { From ba22f2189a4bcbc0ce827fcca71835bb6878eb1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Casta=C3=B1=C3=A9?= Date: Wed, 19 Nov 2014 00:28:57 +0100 Subject: [PATCH 3/7] Changing back cli import in app_test.go --- app_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app_test.go b/app_test.go index 20978bd..d3cc1c5 100644 --- a/app_test.go +++ b/app_test.go @@ -5,7 +5,7 @@ import ( "os" "testing" - "github.com/imdario/cli" + "github.com/codegangsta/cli" ) func ExampleApp() { From 4ab12cd639d80ed43e78a02ef62a8073f597192f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Castan=CC=83e=CC=81?= Date: Sat, 10 Jan 2015 00:44:37 +0100 Subject: [PATCH 4/7] jszwedko's suggestion done --- app.go | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/app.go b/app.go index f400ab9..69aa95b 100644 --- a/app.go +++ b/app.go @@ -78,6 +78,7 @@ func NewApp() *App { // Entry point to the cli app. Parses the arguments slice and routes to the proper flag/args combination func (a *App) Run(arguments []string) error { + var err error if HelpPrinter == nil { defer func() { HelpPrinter = nil @@ -146,11 +147,10 @@ func (a *App) Run(arguments []string) error { if a.After != nil { defer func() { - aferr := a.After(context) - // check Action error to avoid shadowing - if err == nil { - err = aferr - } + // err is always nil here. + // There is a check to see if it is non-nil + // just few lines before. + err = a.After(context) }() } @@ -184,7 +184,8 @@ func (a *App) RunAndExitOnError() { } // Invokes the subcommand given the context, parses ctx.Args() to generate command-specific flags -func (a *App) RunAsSubcommand(ctx *Context) (err error) { +func (a *App) RunAsSubcommand(ctx *Context) error { + var err error // append help to commands if len(a.Commands) > 0 { if a.Command(helpCommand.Name) == nil && !a.HideHelp { @@ -240,11 +241,10 @@ func (a *App) RunAsSubcommand(ctx *Context) (err error) { if a.After != nil { defer func() { - aferr := a.After(context) - // check Action error to avoid shadowing - if err == nil { - err = aferr - } + // err is always nil here. + // There is a check to see if it is non-nil + // just few lines before. + err = a.After(context) }() } From dca177c384c35776b499012316b5363475499cc4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dario=20Castan=CC=83e=CC=81?= Date: Sat, 10 Jan 2015 00:51:06 +0100 Subject: [PATCH 5/7] After handler brought up to speed. As discussed in issue #151, I modified the implementation to be simpler. I checked how to propagate err, being done with a scope trick. From our deferred function we cannot return a new value, so we override the "default" return value (err), declared on functions' declarations. Check this StackOverflow for more info: http://stackoverflow.com/a/19934989 --- app.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/app.go b/app.go index 69aa95b..f01ae42 100644 --- a/app.go +++ b/app.go @@ -77,8 +77,7 @@ func NewApp() *App { } // Entry point to the cli app. Parses the arguments slice and routes to the proper flag/args combination -func (a *App) Run(arguments []string) error { - var err error +func (a *App) Run(arguments []string) (err error) { if HelpPrinter == nil { defer func() { HelpPrinter = nil @@ -184,8 +183,7 @@ func (a *App) RunAndExitOnError() { } // Invokes the subcommand given the context, parses ctx.Args() to generate command-specific flags -func (a *App) RunAsSubcommand(ctx *Context) error { - var err error +func (a *App) RunAsSubcommand(ctx *Context) (err error) { // append help to commands if len(a.Commands) > 0 { if a.Command(helpCommand.Name) == nil && !a.HideHelp { From 2a9c617652b1f8a8e5db915dbe85cc0c0b1967ca Mon Sep 17 00:00:00 2001 From: Vishal Sodani Date: Fri, 30 Jan 2015 16:32:58 +0530 Subject: [PATCH 6/7] Fix sentence --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 0e8327b..c0bb338 100644 --- a/README.md +++ b/README.md @@ -293,6 +293,6 @@ setting the `PROG` variable to the name of your program: ## Contribution Guidelines Feel free to put up a pull request to fix a bug or maybe add a feature. I will give it a code review and make sure that it does not break backwards compatibility. If I or any other collaborators agree that it is in line with the vision of the project, we will work with you to get the code into a mergeable state and merge it into the master branch. -If you are have contributed something significant to the project, I will most likely add you as a collaborator. As a collaborator you are given the ability to merge others pull requests. It is very important that new code does not break existing code, so be careful about what code you do choose to merge. If you have any questions feel free to link @codegangsta to the issue in question and we can review it together. +If you have contributed something significant to the project, I will most likely add you as a collaborator. As a collaborator you are given the ability to merge others pull requests. It is very important that new code does not break existing code, so be careful about what code you do choose to merge. If you have any questions feel free to link @codegangsta to the issue in question and we can review it together. If you feel like you have contributed to the project but have not yet been added as a collaborator, I probably forgot to add you. Hit @codegangsta up over email and we will get it figured out. From 50c77ecec0068c9aef9d90ae0fd0fdf410041da3 Mon Sep 17 00:00:00 2001 From: jszwedko Date: Fri, 20 Feb 2015 16:21:27 -0500 Subject: [PATCH 7/7] Fix comments for .After field on Command and App --- app.go | 2 +- command.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app.go b/app.go index ab49ec0..928983e 100644 --- a/app.go +++ b/app.go @@ -35,7 +35,7 @@ type App struct { // If a non-nil error is returned, no subcommands are run Before func(context *Context) error // An action to execute after any subcommands are run, but after the subcommand has finished - // It is run regardless of Because() result + // It is run even if Action() panics After func(context *Context) error // The action to execute when no subcommands are specified Action func(context *Context) diff --git a/command.go b/command.go index 7c02e47..5747e52 100644 --- a/command.go +++ b/command.go @@ -22,7 +22,7 @@ type Command struct { // If a non-nil error is returned, no sub-subcommands are run Before func(context *Context) error // An action to execute after any subcommands are run, but after the subcommand has finished - // It is run regardless of Because() result + // It is run even if Action() panics After func(context *Context) error // The function to call when this command is invoked Action func(context *Context)