Skip to content

Commit

Permalink
Upgrade spf13/cobra dep to current master (#944)
Browse files Browse the repository at this point in the history
* Pin spf13/cobra dep at b95db644ed1c0e183a90d8509ef2f9a5d51cc29b

 which includes the zsh completion fix for spf13/cobra#899
 and remove the fork of cobra with fix in `replace` section of go.mod

* Parse args without invoking a separate command

 Do not define and execute extractCommand before running actual root
 command to parse all the args without flags. This was creating issues
 with completion utils to generate additional shell completion directive.
 Now uses cobra's inbuilt utilities to parse the command args without flags.

* Return error from stripFlags if any

* Update unit tests
  • Loading branch information
navidshaikh committed Jul 22, 2020
1 parent 879e5bd commit 8232c9d
Show file tree
Hide file tree
Showing 29 changed files with 2,076 additions and 753 deletions.
65 changes: 18 additions & 47 deletions cmd/kn/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,21 +64,26 @@ func run(args []string) error {
return err
}

// Strip of all flags to get the non-flag commands only
commands, err := stripFlags(args)
pluginManager := plugin.NewManager(config.GlobalConfig.PluginsDir(), config.GlobalConfig.LookupPluginsInPath())

// Create kn root command and all sub-commands
rootCmd, err := root.NewRootCommand(pluginManager.HelpTemplateFuncs())
if err != nil {
return err
}

// Find plugin with the commands arguments
pluginManager := plugin.NewManager(config.GlobalConfig.PluginsDir(), config.GlobalConfig.LookupPluginsInPath())
plugin, err := pluginManager.FindPlugin(commands)
// temporary setting to parse all flags
rootCmd.FParseErrWhitelist = cobra.FParseErrWhitelist{UnknownFlags: true}
// Strip of all flags to get the non-flag commands only
commands, err := stripFlags(rootCmd, args)
if err != nil {
return err
}
// reset the temporary setting
rootCmd.FParseErrWhitelist = cobra.FParseErrWhitelist{UnknownFlags: false}

// Create kn root command and all sub-commands
rootCmd, err := root.NewRootCommand(pluginManager.HelpTemplateFuncs())
// Find plugin with the commands arguments
plugin, err := pluginManager.FindPlugin(commands)
if err != nil {
return err
}
Expand Down Expand Up @@ -106,46 +111,12 @@ func run(args []string) error {
}
}

// Get only the args provided but no options. The extraction
// process is a bit tricky as Cobra doesn't provide such
// functionality out of the box
func stripFlags(args []string) ([]string, error) {
// Store all command
commandsFound := &[]string{}

// Use a canary command that allows all options and only extracts
// commands. Doesn't work with arbitrary boolean flags but is good enough
// for us here
extractCommand := cobra.Command{
Run: func(cmd *cobra.Command, args []string) {
for _, arg := range args {
*commandsFound = append(*commandsFound, arg)
}
},
SilenceErrors: true,
SilenceUsage: true,
}

// No help and usage functions to prin
extractCommand.SetHelpFunc(func(*cobra.Command, []string) {})
extractCommand.SetUsageFunc(func(*cobra.Command) error { return nil })

// Filter out --help and -h options to avoid special treatment which we don't
// need here
extractCommand.SetArgs(filterHelpOptions(args))

// Adding all global flags here
config.AddBootstrapFlags(extractCommand.Flags())

// Allow all options
extractCommand.FParseErrWhitelist = cobra.FParseErrWhitelist{UnknownFlags: true}

// Execute to get to the command args
err := extractCommand.Execute()
if err != nil {
return nil, err
// Get only the args provided but no options
func stripFlags(rootCmd *cobra.Command, args []string) ([]string, error) {
if err := rootCmd.ParseFlags(filterHelpOptions(args)); err != nil {
return []string{}, fmt.Errorf("error while parsing flags from args %v: %s", args, err.Error())
}
return *commandsFound, nil
return rootCmd.Flags().Args(), nil
}

// Strip all plugin commands before calling out to the plugin
Expand Down Expand Up @@ -192,7 +163,7 @@ func validatePlugin(root *cobra.Command, plugin plugin.Plugin) error {
func validateRootCommand(cmd *cobra.Command) error {
foundCmd, innerArgs, err := cmd.Find(os.Args[1:])
if err == nil && foundCmd.HasSubCommands() && len(innerArgs) > 0 {
argsWithoutFlags, err := stripFlags(innerArgs)
argsWithoutFlags, err := stripFlags(cmd, innerArgs)
if len(argsWithoutFlags) > 0 || err != nil {
return errors.Errorf("unknown sub-command '%s' for '%s'. Available sub-commands: %s", innerArgs[0], foundCmd.CommandPath(), strings.Join(root.ExtractSubCommandNames(foundCmd.Commands()), ", "))
}
Expand Down
12 changes: 11 additions & 1 deletion cmd/kn/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"gotest.tools/assert"

"knative.dev/client/lib/test"
"knative.dev/client/pkg/kn/config"
"knative.dev/client/pkg/kn/root"
"knative.dev/client/pkg/util"
)
Expand Down Expand Up @@ -159,6 +160,7 @@ func TestUnknownCommands(t *testing.T) {
for _, d := range data {
args := append([]string{"kn"}, d.givenCmdArgs...)
rootCmd, err := root.NewRootCommand(nil)
rootCmd.FParseErrWhitelist = cobra.FParseErrWhitelist{UnknownFlags: true}
os.Args = args
assert.NilError(t, err)
err = validateRootCommand(rootCmd)
Expand Down Expand Up @@ -218,14 +220,22 @@ func TestStripFlags(t *testing.T) {

for i, f := range data {
step := fmt.Sprintf("Check %d", i)
commands, err := stripFlags(f.givenArgs)
cmd := &cobra.Command{FParseErrWhitelist: cobra.FParseErrWhitelist{UnknownFlags: true}}
config.AddBootstrapFlags(cmd.Flags())
commands, err := stripFlags(cmd, f.givenArgs)
assert.DeepEqual(t, commands, f.expectedCommands)
if f.expectedError != "" {
assert.ErrorContains(t, err, f.expectedError, step)
} else {
assert.NilError(t, err, step)
}
}

t.Log("checking error case for stripFlags")
cmd, err := root.NewRootCommand(nil)
assert.NilError(t, err)
_, err = stripFlags(cmd, []string{"--config"})
assert.ErrorContains(t, err, "needs an argument")
}

func TestRunWithError(t *testing.T) {
Expand Down
6 changes: 1 addition & 5 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ go 1.14
require (
github.com/mitchellh/go-homedir v1.1.0
github.com/pkg/errors v0.9.1
github.com/spf13/cobra v1.0.0
github.com/spf13/cobra v1.0.1-0.20200715031239-b95db644ed1c
github.com/spf13/pflag v1.0.5
github.com/spf13/viper v1.7.0
golang.org/x/crypto v0.0.0-20200709230013-948cd5f35899
Expand All @@ -23,11 +23,7 @@ require (

// Temporary pinning certain libraries. Please check periodically, whether these are still needed
// ----------------------------------------------------------------------------------------------

// Fix for `[` in help messages and shell completion code
// See https://github.com/spf13/cobra/pull/899
replace (
github.com/spf13/cobra => github.com/chmouel/cobra v0.0.0-20191021105835-a78788917390
k8s.io/api => k8s.io/api v0.17.6
k8s.io/apiextensions-apiserver => k8s.io/apiextensions-apiserver v0.17.6
k8s.io/apimachinery => k8s.io/apimachinery v0.17.6
Expand Down
16 changes: 14 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -225,8 +225,6 @@ github.com/cespare/xxhash/v2 v2.1.0/go.mod h1:dgIUBU3pDso/gPgZ1osOZ0iQf77oPR28Tj
github.com/cespare/xxhash/v2 v2.1.1 h1:6MnRN8NT7+YBpUIWxHtefFZOKTAPgGjpQSxqLNn0+qY=
github.com/cespare/xxhash/v2 v2.1.1/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs=
github.com/chai2010/gettext-go v0.0.0-20160711120539-c6fed771bfd5/go.mod h1:/iP1qXHoty45bqomnu2LM+VVyAEdWN+vtSHGlQgyxbw=
github.com/chmouel/cobra v0.0.0-20191021105835-a78788917390 h1:r2QCtXeuDAgROGLHUyLAnC4ju/o9zGIZ+kZLaNBKgqI=
github.com/chmouel/cobra v0.0.0-20191021105835-a78788917390/go.mod h1:/6GTrnGXV9HjY+aR4k0oJ5tcvakLuG6EuKReYlHNrgE=
github.com/chzyer/logex v1.1.10/go.mod h1:+Ywpsq7O8HXn0nuIou7OrIPyXbp3wmkHB+jjWRnGsAI=
github.com/chzyer/readline v0.0.0-20180603132655-2972be24d48e/go.mod h1:nSuG5e5PlCu98SY8svDHJxuZscDgtXS6KTTbou5AhLI=
github.com/chzyer/test v0.0.0-20180213035817-a1ea475d72b1/go.mod h1:Q3SI9o4m/ZMnBNeIyt5eFwwo7qiLfzFZmjNmxjkiQlU=
Expand All @@ -252,6 +250,7 @@ github.com/containerd/typeurl v0.0.0-20180627222232-a93fcdb778cd/go.mod h1:Cm3kw
github.com/coreos/bbolt v1.3.2/go.mod h1:iRUV2dpdMOn7Bo10OQBFzIJO9kkE559Wcmn+qkEiiKk=
github.com/coreos/etcd v3.3.10+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE=
github.com/coreos/etcd v3.3.13+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE=
github.com/coreos/go-etcd v2.0.0+incompatible/go.mod h1:Jez6KQU2B/sWsbdaef3ED8NzMklzPG4d5KIOhIy30Tk=
github.com/coreos/go-oidc v2.1.0+incompatible/go.mod h1:CgnwVTmzoESiwO9qyAFEMiHoZ1nMCKZlZ9V6mm3/LKc=
github.com/coreos/go-semver v0.2.0/go.mod h1:nnelYz7RCh+5ahJtPPxZlU+153eP4D4r3EedlOD2RNk=
github.com/coreos/go-semver v0.3.0/go.mod h1:nnelYz7RCh+5ahJtPPxZlU+153eP4D4r3EedlOD2RNk=
Expand All @@ -260,6 +259,8 @@ github.com/coreos/go-systemd v0.0.0-20190321100706-95778dfbb74e/go.mod h1:F5haX7
github.com/coreos/pkg v0.0.0-20160727233714-3ac0863d7acf/go.mod h1:E3G3o1h8I7cfcXa63jLwjI0eiQQMgzzUDFVpN/nH/eA=
github.com/coreos/pkg v0.0.0-20180108230652-97fdf19511ea/go.mod h1:E3G3o1h8I7cfcXa63jLwjI0eiQQMgzzUDFVpN/nH/eA=
github.com/coreos/pkg v0.0.0-20180928190104-399ea9e2e55f/go.mod h1:E3G3o1h8I7cfcXa63jLwjI0eiQQMgzzUDFVpN/nH/eA=
github.com/cpuguy83/go-md2man v1.0.10 h1:BSKMNlYxDvnunlTymqtgONjNnaRV1sTpcovwwjF22jk=
github.com/cpuguy83/go-md2man v1.0.10/go.mod h1:SmD6nW6nTyfqj6ABTjUi3V3JVMnlJmwcJI5acqYI6dE=
github.com/cpuguy83/go-md2man/v2 v2.0.0 h1:EoUDS0afbrsXAZ9YQ9jdu/mZ2sXgT1/2yyNng4PGlyM=
github.com/cpuguy83/go-md2man/v2 v2.0.0/go.mod h1:maD7wRr/U5Z6m/iR4s+kqSMx2CaBsrgA7czyZG/E6dU=
github.com/creack/pty v1.1.7/go.mod h1:lj5s0c3V2DBrqTV7llrYr5NG6My20zk30Fl46Y7DoTY=
Expand Down Expand Up @@ -978,6 +979,13 @@ github.com/spf13/afero v1.2.2/go.mod h1:9ZxEEn6pIJ8Rxe320qSDBk6AsU0r9pR7Q4OcevTd
github.com/spf13/cast v1.3.0/go.mod h1:Qx5cxh0v+4UWYiBimWS+eyWzqEqokIECu5etghLkUJE=
github.com/spf13/cast v1.3.1 h1:nFm6S0SMdyzrzcmThSipiEubIDy8WEXKNZ0UOgiRpng=
github.com/spf13/cast v1.3.1/go.mod h1:Qx5cxh0v+4UWYiBimWS+eyWzqEqokIECu5etghLkUJE=
github.com/spf13/cobra v0.0.2-0.20171109065643-2da4a54c5cee/go.mod h1:1l0Ry5zgKvJasoi3XT1TypsSe7PqH0Sj9dhYf7v3XqQ=
github.com/spf13/cobra v0.0.3/go.mod h1:1l0Ry5zgKvJasoi3XT1TypsSe7PqH0Sj9dhYf7v3XqQ=
github.com/spf13/cobra v0.0.5/go.mod h1:3K3wKZymM7VvHMDS9+Akkh4K60UwM26emMESw8tLCHU=
github.com/spf13/cobra v0.0.6/go.mod h1:/6GTrnGXV9HjY+aR4k0oJ5tcvakLuG6EuKReYlHNrgE=
github.com/spf13/cobra v1.0.0/go.mod h1:/6GTrnGXV9HjY+aR4k0oJ5tcvakLuG6EuKReYlHNrgE=
github.com/spf13/cobra v1.0.1-0.20200715031239-b95db644ed1c h1:nt+V0u9zwtHQLk4OrQxVDMwJ7ifwoUzqcrcxySbmxxY=
github.com/spf13/cobra v1.0.1-0.20200715031239-b95db644ed1c/go.mod h1:yk5b0mALVusDL5fMM6Rd1wgnoO5jUPhwsQ6LQAJTidQ=
github.com/spf13/jwalterweatherman v1.0.0/go.mod h1:cQK4TGJAtQXfYWX+Ddv3mKDzgVb68N+wFjFa4jdeBTo=
github.com/spf13/jwalterweatherman v1.1.0 h1:ue6voC5bR5F8YxI5S67j9i582FU4Qvo2bmqnqMYADFk=
github.com/spf13/jwalterweatherman v1.1.0/go.mod h1:aNWZUN0dPAAO/Ljvb5BEdw96iTZ0EXowPYD95IqWIGo=
Expand All @@ -987,6 +995,7 @@ github.com/spf13/pflag v1.0.1/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnIn
github.com/spf13/pflag v1.0.3/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnInEg4=
github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA=
github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg=
github.com/spf13/viper v1.3.2/go.mod h1:ZiWeW+zYFKm7srdB9IoDzzZXaJaI5eL9QjNiN/DMA2s=
github.com/spf13/viper v1.4.0/go.mod h1:PTJ7Z/lr49W6bUbkmS1V3by4uWynFiR9p7+dSq/yZzE=
github.com/spf13/viper v1.6.1/go.mod h1:t3iDnF5Jlj76alVNuyFBk5oUMCvsrkbvZK0WQdfDi5k=
github.com/spf13/viper v1.6.2 h1:7aKfF+e8/k68gda3LOjo5RxiUqddoFxVq4BKBPrxk5E=
Expand Down Expand Up @@ -1029,6 +1038,7 @@ github.com/tommy-muehle/go-mnd v1.3.1-0.20200224220436-e6f9a994e8fa/go.mod h1:dS
github.com/tsenart/go-tsz v0.0.0-20180814235614-0bd30b3df1c3/go.mod h1:SWZznP1z5Ki7hDT2ioqiFKEse8K9tU2OUvaRI0NeGQo=
github.com/tsenart/vegeta v12.7.1-0.20190725001342-b5f4fca92137+incompatible/go.mod h1:Smz/ZWfhKRcyDDChZkG3CyTHdj87lHzio/HOCkbndXM=
github.com/ugorji/go v1.1.4/go.mod h1:uQMGLiO92mf5W77hV/PUCpI3pbzQx3CRekS0kk+RGrc=
github.com/ugorji/go/codec v0.0.0-20181204163529-d75b2dcb6bc8/go.mod h1:VFNgLljTbGfSG7qAOspJ7OScBnGdDN/yBr0sguwnwf0=
github.com/ulikunitz/xz v0.5.6/go.mod h1:2bypXElzHzzJZwzH67Y6wb67pO62Rzfn7BSiF4ABRW8=
github.com/ulikunitz/xz v0.5.7/go.mod h1:nbz6k7qbPmH4IRqmfOplQw/tblSgqTqBwxkY0oWt/14=
github.com/ultraware/funlen v0.0.2/go.mod h1:Dp4UiAus7Wdb9KUZsYWZEWiRzGuM2kXM1lPbfaF6xhA=
Expand Down Expand Up @@ -1105,6 +1115,7 @@ gocloud.dev v0.19.0/go.mod h1:SmKwiR8YwIMMJvQBKLsC3fHNyMwXLw3PMDO+VVteJMI=
golang.org/x/crypto v0.0.0-20171113213409-9f005a07e0d3/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4=
golang.org/x/crypto v0.0.0-20180904163835-0709b304e793/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4=
golang.org/x/crypto v0.0.0-20181029021203-45a5f77698d3/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4=
golang.org/x/crypto v0.0.0-20181203042331-505ab145d0a9/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4=
golang.org/x/crypto v0.0.0-20190211182817-74369b46fc67/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4=
golang.org/x/crypto v0.0.0-20190219172222-a4c6cb3142f2/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4=
golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
Expand Down Expand Up @@ -1247,6 +1258,7 @@ golang.org/x/sys v0.0.0-20181026203630-95b1ffbd15a5/go.mod h1:STP8DvDyc/dI5b8T5h
golang.org/x/sys v0.0.0-20181107165924-66b7b1311ac8/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20181116152217-5ac8a444bdc5/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20181122145206-62eef0e2fa9b/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20181205085412-a5c9d58dba9a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20190209173611-3b5209105503/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20190221075227-b4e8571b14e0/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
Expand Down
2 changes: 1 addition & 1 deletion vendor/github.com/spf13/cobra/.gitignore

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

25 changes: 13 additions & 12 deletions vendor/github.com/spf13/cobra/.travis.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

22 changes: 22 additions & 0 deletions vendor/github.com/spf13/cobra/CHANGELOG.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

36 changes: 36 additions & 0 deletions vendor/github.com/spf13/cobra/Makefile

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 8232c9d

Please sign in to comment.