Unverified Commit a1f55094 authored by Sam Foo's avatar Sam Foo Committed by GitHub
Browse files

ks version and -h flag should not findRoot (#671)



* Explicitly skipRoot if cmd is version
Signed-off-by: default avatarGuessWhoSamFoo <sfoohei@gmail.com>

* Catch case for help flags; rename ErrNotInApp
Signed-off-by: default avatarGuessWhoSamFoo <sfoohei@gmail.com>

* Refactored skipRoot logic; wrapped error message
Signed-off-by: default avatarGuessWhoSamFoo <sfoohei@gmail.com>

* Added tests; change back to original ErrNotInApp
Signed-off-by: default avatarGuessWhoSamFoo <sfoohei@gmail.com>

* Address cases without cmd;fix test param
Signed-off-by: default avatarGuessWhoSamFoo <sfoohei@gmail.com>
parent e2ef2b8e
......@@ -67,7 +67,7 @@ func runTestCmd(t *testing.T, cases []cmdTestCase) {
fs := afero.NewMemMapFs()
wd := "/"
cmdName, err := parseCommand(tc.args)
cmdName, _, err := parseCommand(tc.args)
require.NoError(t, err)
if len(tc.args) > 0 && cmdName != "init" {
wd = "/app"
......
......@@ -23,6 +23,7 @@ import (
"github.com/ksonnet/ksonnet/pkg/app"
"github.com/ksonnet/ksonnet/pkg/log"
"github.com/ksonnet/ksonnet/pkg/plugin"
"github.com/ksonnet/ksonnet/pkg/util/strings"
"github.com/pkg/errors"
"github.com/shomron/pflag"
"github.com/spf13/afero"
......@@ -79,19 +80,20 @@ func appRoot() (string, error) {
}
// parseCommand does an early parse of the command line and returns
// what will ultimately be recognized as the command by cobra.
func parseCommand(args []string) (string, error) {
// what will ultimately be recognized as the command by cobra
// and a boolean for calling help flags.
func parseCommand(args []string) (string, bool, error) {
fset := pflag.NewFlagSet("", pflag.ContinueOnError)
fset.ParseErrorsWhitelist.UnknownFlags = true
fset.BoolP("help", "h", false, "") // Needed to avoid pflag.ErrHelp
containsHelp := fset.BoolP("help", "h", false, "") // Needed to avoid pflag.ErrHelp
if err := fset.Parse(args); err != nil {
return "", err
return "", false, err
}
if len(fset.Args()) == 0 {
return "", nil
return "", true, nil
}
return fset.Args()[0], nil
return fset.Args()[0], *containsHelp, nil
}
// checkUpgrade runs upgrade validations unless the user is running an excluded command.
......@@ -101,6 +103,7 @@ func checkUpgrade(a app.App, cmd string) error {
"init": struct{}{},
"upgrade": struct{}{},
"help": struct{}{},
"version": struct{}{},
"": struct{}{},
}
if _, ok := skip[cmd]; ok {
......@@ -122,17 +125,24 @@ func NewRoot(appFs afero.Fs, wd string, args []string) (*cobra.Command, error) {
var a app.App
var err error
cmdName, err := parseCommand(args)
cmdName, hasHelpFlag, err := parseCommand(args)
if err != nil {
return nil, err
}
if len(args) > 0 && cmdName != "init" {
cmds := []string{"init", "version", "help"}
switch {
// Commands that do not require a ksonnet application
case strings.InSlice(cmdName, cmds), hasHelpFlag:
a, err = app.Load(appFs, wd, true)
case len(args) > 0:
a, err = app.Load(appFs, wd, false)
if err != nil {
return nil, err
}
default:
// noop
}
if err != nil {
return nil, err
}
if err := checkUpgrade(a, cmdName); err != nil {
......
......@@ -27,28 +27,45 @@ func Test_parseCommand(t *testing.T) {
name string
args []string
expected string
helpFlag bool
}{
{
name: "normal",
args: []string{"init", "-abc", "123", "--foo", "--bar", ""},
expected: "init",
helpFlag: false,
},
{
name: "flags before command",
args: []string{"-abc", "123", "--foo", "--empty", "", "--bar", "arg", "init"},
expected: "init",
helpFlag: false,
},
{
name: "no command",
args: []string{"-abc", "123", "--foo", "--empty", "", "--bar", "arg"},
expected: "",
helpFlag: true,
},
{
name: "help flag",
args: []string{"init", "arg", "--help", "-h"},
expected: "init",
helpFlag: true,
},
{
name: "help command",
args: []string{"help", "-abc", "--foo", "--bar", ""},
expected: "help",
helpFlag: false,
},
}
for _, tc := range tests {
cmd, err := parseCommand(tc.args)
cmd, hasHelpFlag, err := parseCommand(tc.args)
require.NoError(t, err)
assert.Equal(t, tc.expected, cmd, tc.name)
assert.Equal(t, tc.helpFlag, hasHelpFlag, tc.name)
}
}
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment