Skip to content

Commit

Permalink
fixes(#414): change plugins config to use - instead of camel case (#428)
Browse files Browse the repository at this point in the history
This solves 414 by adopting new format for config keys and thereby
bypassing the need for case sensitivity in Viper. Also changed
`lookupPluginsInPath` key to `lookup-plugins` and also same for the
persistent flag. The PATH part is implied and can be read from
help.
  • Loading branch information
maximilien authored and knative-prow-robot committed Oct 4, 2019
1 parent b27e366 commit d543b0e
Show file tree
Hide file tree
Showing 10 changed files with 40 additions and 40 deletions.
12 changes: 6 additions & 6 deletions docs/cmd/kn.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@ Manage your Knative building blocks:
### Options

```
--config string kn config file (default is $HOME/.kn/config.yaml)
-h, --help help for kn
--kubeconfig string kubectl config file (default is $HOME/.kube/config)
--log-http log http traffic
--lookup-plugins-in-path look for kn plugins in $PATH
--plugins-dir string kn plugins directory (default "~/.kn/plugins")
--config string kn config file (default is $HOME/.kn/config.yaml)
-h, --help help for kn
--kubeconfig string kubectl config file (default is $HOME/.kube/config)
--log-http log http traffic
--lookup-plugins look for kn plugins in $PATH
--plugins-dir string kn plugins directory (default "~/.kn/plugins")
```

### SEE ALSO
Expand Down
6 changes: 3 additions & 3 deletions docs/cmd/kn_plugin.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ kn plugin [flags]
### Options

```
-h, --help help for plugin
--lookup-plugins-in-path look for kn plugins in $PATH
--plugins-dir string kn plugins directory (default "~/.kn/plugins")
-h, --help help for plugin
--lookup-plugins look for kn plugins in $PATH
--plugins-dir string kn plugins directory (default "~/.kn/plugins")
```

### Options inherited from parent commands
Expand Down
10 changes: 5 additions & 5 deletions docs/cmd/kn_plugin_list.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,11 @@ kn plugin list [flags]
### Options

```
-h, --help help for list
--lookup-plugins-in-path look for kn plugins in $PATH
--name-only If true, display only the binary name of each plugin, rather than its full path
--plugins-dir string kn plugins directory (default "~/.kn/plugins")
--verbose verbose output
-h, --help help for list
--lookup-plugins look for kn plugins in $PATH
--name-only If true, display only the binary name of each plugin, rather than its full path
--plugins-dir string kn plugins directory (default "~/.kn/plugins")
--verbose verbose output
```

### Options inherited from parent commands
Expand Down
4 changes: 2 additions & 2 deletions pkg/kn/commands/plugin/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func listPlugins(cmd *cobra.Command, flags pluginListFlags) error {
if err != nil {
return err
}
if commands.Cfg.LookupPluginsInPath {
if commands.Cfg.LookupPlugins {
pluginPath = pluginPath + string(os.PathListSeparator) + os.Getenv("PATH")
}

Expand All @@ -74,7 +74,7 @@ func listPlugins(cmd *cobra.Command, flags pluginListFlags) error {
if flags.verbose {
fmt.Fprintf(out, "The following plugins are available, using options:\n")
fmt.Fprintf(out, " - plugins dir: '%s'%s\n", commands.Cfg.PluginsDir, extraLabelIfPathNotExists(pluginPath))
fmt.Fprintf(out, " - lookup plugins in path: '%t'\n", commands.Cfg.LookupPluginsInPath)
fmt.Fprintf(out, " - lookup plugins in $PATH: '%t'\n", commands.Cfg.LookupPlugins)
}

if len(pluginsFound) == 0 {
Expand Down
18 changes: 9 additions & 9 deletions pkg/kn/commands/plugin/list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,22 +107,22 @@ func TestPluginList(t *testing.T) {
assert.Assert(t, pluginListCmd.RunE != nil)
})

t.Run("when pluginsDir does not include any plugins", func(t *testing.T) {
t.Run("when --lookup-plugins-in-path is true", func(t *testing.T) {
t.Run("when plugins-dir does not include any plugins", func(t *testing.T) {
t.Run("when --lookup-plugins is true", func(t *testing.T) {
t.Run("no plugins installed", func(t *testing.T) {

t.Run("warns user that no plugins found in verbose mode", func(t *testing.T) {
ctx := setup(t)
defer ctx.cleanup()
err := ctx.execute("plugin", "list", "--lookup-plugins-in-path=true", "--verbose")
err := ctx.execute("plugin", "list", "--lookup-plugins=true", "--verbose")
assert.NilError(t, err)
assert.Assert(t, util.ContainsAll(ctx.output(), "No plugins found"))
})

t.Run("no output when no plugins found", func(t *testing.T) {
ctx := setup(t)
defer ctx.cleanup()
err := ctx.execute("plugin", "list", "--lookup-plugins-in-path=true")
err := ctx.execute("plugin", "list", "--lookup-plugins=true")
assert.NilError(t, err)
assert.Equal(t, ctx.output(), "No plugins found.\n")
})
Expand All @@ -138,7 +138,7 @@ func TestPluginList(t *testing.T) {
err := ctx.createTestPlugin(KnTestPluginName, FileModeExecutable, true)
assert.NilError(t, err)

err = ctx.execute("plugin", "list", "--lookup-plugins-in-path=true")
err = ctx.execute("plugin", "list", "--lookup-plugins=true")
assert.NilError(t, err)
assert.Assert(t, util.ContainsAll(ctx.output(), KnTestPluginName))
})
Expand All @@ -152,7 +152,7 @@ func TestPluginList(t *testing.T) {
err := ctx.createTestPlugin(KnTestPluginName, FileModeReadable, false)
assert.NilError(t, err)

err = ctx.execute("plugin", "list", "--lookup-plugins-in-path=false")
err = ctx.execute("plugin", "list", "--lookup-plugins=false")
assert.NilError(t, err)
assert.Assert(t, util.ContainsAll(ctx.output(), "WARNING", "not executable", "current user"))
})
Expand All @@ -177,7 +177,7 @@ func TestPluginList(t *testing.T) {
err = ctx.createTestPluginWithPath(KnTestPluginName, FileModeExecutable, tmpPathDir2)
assert.NilError(t, err)

err = ctx.execute("plugin", "list", "--lookup-plugins-in-path=true")
err = ctx.execute("plugin", "list", "--lookup-plugins=true")
assert.NilError(t, err)
assert.Assert(t, util.ContainsAll(ctx.output(), "WARNING", "shadowed", "ignored"))
})
Expand All @@ -197,7 +197,7 @@ func TestPluginList(t *testing.T) {
err := ctx.createTestPlugin("kn-fake", FileModeExecutable, true)
assert.NilError(t, err)

err = ctx.execute("plugin", "list", "--lookup-plugins-in-path=true")
err = ctx.execute("plugin", "list", "--lookup-plugins=true")
assert.ErrorContains(t, err, "overwrite", "built-in")
assert.Assert(t, util.ContainsAll(ctx.output(), "ERROR", "overwrite", "built-in"))
})
Expand All @@ -206,7 +206,7 @@ func TestPluginList(t *testing.T) {
})
})

t.Run("when pluginsDir has plugins", func(t *testing.T) {
t.Run("when plugins-dir has plugins", func(t *testing.T) {
t.Run("list plugins in --plugins-dir", func(t *testing.T) {
ctx := setup(t)
defer ctx.cleanup()
Expand Down
10 changes: 5 additions & 5 deletions pkg/kn/commands/plugin/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,14 @@ Please refer to the documentation and examples for more information about how wr
// AddPluginFlags plugins-dir and lookup-plugins-in-path to cmd
func AddPluginFlags(cmd *cobra.Command) {
cmd.Flags().StringVar(&commands.Cfg.PluginsDir, "plugins-dir", "~/.kn/plugins", "kn plugins directory")
cmd.Flags().BoolVar(&commands.Cfg.LookupPluginsInPath, "lookup-plugins-in-path", false, "look for kn plugins in $PATH")
cmd.Flags().BoolVar(&commands.Cfg.LookupPlugins, "lookup-plugins", false, "look for kn plugins in $PATH")
}

// BindPluginsFlagToViper bind and set default with viper for plugins flags
func BindPluginsFlagToViper(cmd *cobra.Command) {
viper.BindPFlag("pluginsDir", cmd.Flags().Lookup("plugins-dir"))
viper.BindPFlag("lookupPluginsInPath", cmd.Flags().Lookup("lookup-plugins-in-path"))
viper.BindPFlag("plugins-dir", cmd.Flags().Lookup("plugins-dir"))
viper.BindPFlag("lookup-plugins", cmd.Flags().Lookup("lookup-plugins"))

viper.SetDefault("pluginsDir", "~/.kn/plugins")
viper.SetDefault("lookupPluginsInPath", false)
viper.SetDefault("plugins-dir", "~/.kn/plugins")
viper.SetDefault("lookup-plugins", false)
}
4 changes: 2 additions & 2 deletions pkg/kn/commands/plugin/plugin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ Available Commands:
Flags:
-h, --help help for plugin
--lookup-plugins-in-path look for kn plugins in $PATH
--lookup-plugins look for kn plugins in $PATH
--plugins-dir string kn plugins directory (default "~/.kn/plugins")
Global Flags:
Expand Down Expand Up @@ -68,7 +68,7 @@ func TestNewPluginCommand(t *testing.T) {
assert.Assert(t, pluginCmd.Short == "Plugin command group")
assert.Assert(t, strings.Contains(pluginCmd.Long, "Provides utilities for interacting and managing with kn plugins."))
assert.Assert(t, pluginCmd.Flags().Lookup("plugins-dir") != nil)
assert.Assert(t, pluginCmd.Flags().Lookup("lookup-plugins-in-path") != nil)
assert.Assert(t, pluginCmd.Flags().Lookup("lookup-plugins") != nil)
assert.Assert(t, pluginCmd.Args == nil)
})
}
10 changes: 5 additions & 5 deletions pkg/kn/commands/testing_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,13 +119,13 @@ Eventing: Manage event subscriptions and channels. Connect up event sources.`,
rootCmd.PersistentFlags().StringVar(&params.KubeCfgPath, "kubeconfig", "", "kubectl config file (default is $HOME/.kube/config)")

rootCmd.Flags().StringVar(&Cfg.PluginsDir, "plugins-dir", "~/.kn/plugins", "kn plugins directory")
rootCmd.Flags().BoolVar(&Cfg.LookupPluginsInPath, "lookup-plugins-in-path", false, "look for kn plugins in $PATH")
rootCmd.Flags().BoolVar(&Cfg.LookupPlugins, "lookup-plugins", false, "look for kn plugins in $PATH")

viper.BindPFlag("pluginsDir", rootCmd.Flags().Lookup("plugins-dir"))
viper.BindPFlag("lookupPluginsInPath", rootCmd.Flags().Lookup("lookup-plugins-in-path"))
viper.BindPFlag("plugins-dir", rootCmd.Flags().Lookup("plugins-dir"))
viper.BindPFlag("lookup-plugins", rootCmd.Flags().Lookup("lookup-plugins"))

viper.SetDefault("pluginsDir", "~/.kn/plugins")
viper.SetDefault("lookupPluginsInPath", false)
viper.SetDefault("plugins-dir", "~/.kn/plugins")
viper.SetDefault("lookup-plugins", false)

rootCmd.AddCommand(subCommand)

Expand Down
4 changes: 2 additions & 2 deletions pkg/kn/commands/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ var Cfg Config

// Config contains the variables for the Kn config
type Config struct {
PluginsDir string
LookupPluginsInPath bool
PluginsDir string
LookupPlugins bool
}

// Parameters for creating commands. Useful for inserting mocks for testing.
Expand Down
2 changes: 1 addition & 1 deletion pkg/kn/core/root_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ func checkRootCmd(t *testing.T, rootCmd *cobra.Command) {
assert.Assert(t, rootCmd.SilenceErrors)

assert.Assert(t, rootCmd.Flags().Lookup("plugins-dir") != nil)
assert.Assert(t, rootCmd.Flags().Lookup("lookup-plugins-in-path") != nil)
assert.Assert(t, rootCmd.Flags().Lookup("lookup-plugins") != nil)

assert.Assert(t, rootCmd.RunE == nil)
}
Expand Down

0 comments on commit d543b0e

Please sign in to comment.