Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

add RunWithContext + remove signal cancellation #975

Merged
merged 2 commits into from Dec 6, 2019
Merged

add RunWithContext + remove signal cancellation #975

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
322fc3b
add RunWithContext + remove signal cancellation
marwan-at-work Dec 4, 2019
7daa2d1
RunWithContext -> RunContext
marwan-at-work Dec 6, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 9 additions & 1 deletion app.go
View file
Open in desktop
@@ -1,6 +1,7 @@
package cli

import (
"context"
"flag"
"fmt"
"io"
Expand Down Expand Up @@ -207,6 +208,13 @@ func (a *App) useShortOptionHandling() bool {
// Run is the entry point to the cli app. Parses the arguments slice and routes
// to the proper flag/args combination
func (a *App) Run(arguments []string) (err error) {
return a.RunWithContext(context.Background(), arguments)
}

// RunWithContext is like Run except it takes a Context that will be
// passed to its commands and sub-commands. Through this, you can
// propagate timeouts and cancellation requests
func (a *App) RunWithContext(ctx context.Context, arguments []string) (err error) {
marwan-at-work marked this conversation as resolved.
Show resolved Hide resolved
a.Setup()

// handle the completion flag separately from the flagset since
Expand All @@ -224,7 +232,7 @@ func (a *App) Run(arguments []string) (err error) {

err = parseIter(set, a, arguments[1:], shellComplete)
nerr := normalizeFlags(a.Flags, set)
context := NewContext(a, set, nil)
context := NewContext(a, set, &Context{Context: ctx})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this still work of user does RunWithContext(nil, args)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Passing a nil Context is always a developer error.

From the docs:

Do not pass a nil Context, even if a function permits it. Pass context.TODO if you are unsure about which Context to use.

We shouldn't need to support that

if nerr != nil {
_, _ = fmt.Fprintln(a.Writer, nerr)
_ = ShowAppHelp(context)
Expand Down
12 changes: 1 addition & 11 deletions context.go
View file
Open in desktop
Expand Up @@ -5,10 +5,7 @@ import (
"errors"
"flag"
"fmt"
"os"
"os/signal"
"strings"
"syscall"
)

// Context is a type that is passed through to
Expand Down Expand Up @@ -36,14 +33,7 @@ func NewContext(app *App, set *flag.FlagSet, parentCtx *Context) *Context {
c.Command = &Command{}

if c.Context == nil {
ctx, cancel := context.WithCancel(context.Background())
go func() {
defer cancel()
sigs := make(chan os.Signal, 1)
signal.Notify(sigs, syscall.SIGINT, syscall.SIGTERM)
<-sigs
}()
c.Context = ctx
c.Context = context.Background()
}

return c
Expand Down