diff --git a/fs/config/config.go b/fs/config/config.go index a4f8a1644..e17902054 100644 --- a/fs/config/config.go +++ b/fs/config/config.go @@ -570,9 +570,10 @@ func JSONListProviders() error { // fsOption returns an Option describing the possible remotes func fsOption() *fs.Option { o := &fs.Option{ - Name: "Storage", - Help: "Type of storage to configure.", - Default: "", + Name: "Storage", + Help: "Type of storage to configure.", + Default: "", + Required: true, } for _, item := range fs.Registry { example := fs.OptionExample{ diff --git a/fs/config/ui.go b/fs/config/ui.go index 5e2ded4af..0bb0ba1b7 100644 --- a/fs/config/ui.go +++ b/fs/config/ui.go @@ -61,15 +61,19 @@ func CommandDefault(commands []string, defaultIndex int) byte { for { fmt.Printf("%s> ", optHelp) result := strings.ToLower(ReadLine()) - if len(result) == 0 && defaultIndex >= 0 { - return optString[defaultIndex] - } - if len(result) != 1 { - continue - } - i := strings.Index(optString, string(result[0])) - if i >= 0 { - return result[0] + if len(result) == 0 { + if defaultIndex >= 0 { + return optString[defaultIndex] + } + fmt.Printf("This value is required and it has no default.\n") + } else if len(result) == 1 { + i := strings.Index(optString, string(result[0])) + if i >= 0 { + return result[0] + } + fmt.Printf("This value must be one of the following characters: %s.\n", strings.Join(opts, ", ")) + } else { + fmt.Printf("This value must be a single character, one of the following: %s.\n", strings.Join(opts, ", ")) } } } @@ -90,15 +94,20 @@ func Confirm(Default bool) bool { return CommandDefault([]string{"yYes", "nNo"}, defaultIndex) == 'y' } -// Choose one of the defaults or type a new string if newOk is set -func Choose(what string, defaults, help []string, newOk bool) string { +// Choose one of the choices, or default, or type a new string if newOk is set +func Choose(what string, kind string, choices, help []string, defaultValue string, required bool, newOk bool) string { valueDescription := "an existing" if newOk { valueDescription = "your own" } - fmt.Printf("Choose a number from below, or type in %s value.\n", valueDescription) + fmt.Printf("Choose a number from below, or type in %s %s.\n", valueDescription, kind) + if !required || defaultValue != "" { + // Empty input is allowed if not required is set, or if + // required is set but there is a default value to use. + fmt.Printf("Press Enter for the default (%q).\n", defaultValue) + } attributes := []string{terminal.HiRedFg, terminal.HiGreenFg} - for i, text := range defaults { + for i, text := range choices { var lines []string if help != nil { parts := strings.Split(help[i], "\n") @@ -135,22 +144,101 @@ func Choose(what string, defaults, help []string, newOk bool) string { result := ReadLine() i, err := strconv.Atoi(result) if err != nil { - if newOk { - return result - } - for _, v := range defaults { + for _, v := range choices { if result == v { return result } } - continue - } - if i >= 1 && i <= len(defaults) { - return defaults[i-1] + if result == "" { + // If empty string is in the predefined list of choices it has already been returned above. + // If parameter required is not set, then empty string is always a valid value. + if !required { + return result + } + // If parameter required is set, but there is a default, then empty input means default. + if defaultValue != "" { + return defaultValue + } + // If parameter required is set, and there is no default, then an input value is required. + fmt.Printf("This value is required and it has no default.\n") + } else if newOk { + // If legal input is not restricted to defined choices, then any nonzero input string is accepted. + return result + } else { + // A nonzero input string was specified but it did not match any of the strictly defined choices. + fmt.Printf("This value must match %s value.\n", valueDescription) + } + } else { + if i >= 1 && i <= len(choices) { + return choices[i-1] + } + fmt.Printf("No choices with this number.\n") } } } +// Enter prompts for an input value of a specified type +func Enter(what string, kind string, defaultValue string, required bool) string { + if !required || defaultValue != "" { + // Empty input is allowed if not required is set, or if + // required is set but there is a default value to use. + fmt.Printf("Enter a %s. Press Enter for the default (%q).\n", kind, defaultValue) + } else { + fmt.Printf("Enter a %s.\n", kind) + } + for { + fmt.Printf("%s> ", what) + result := ReadLine() + if !required || result != "" { + return result + } + if defaultValue != "" { + return defaultValue + } + fmt.Printf("This value is required and it has no default.\n") + } +} + +// ChoosePassword asks the user for a password +func ChoosePassword(required bool) string { + fmt.Printf("Choose an alternative below.") + actions := []string{"yYes type in my own password", "gGenerate random password"} + defaultAction := -1 + if !required { + defaultAction = len(actions) + actions = append(actions, "nNo leave this optional password blank") + fmt.Printf(" Press Enter for the default (%s).", string(actions[defaultAction][0])) + } + fmt.Println() + var password string + var err error + switch i := CommandDefault(actions, defaultAction); i { + case 'y': + password = ChangePassword("the") + case 'g': + for { + fmt.Printf("Password strength in bits.\n64 is just about memorable\n128 is secure\n1024 is the maximum\n") + bits := ChooseNumber("Bits", 64, 1024) + password, err = Password(bits) + if err != nil { + log.Fatalf("Failed to make password: %v", err) + } + fmt.Printf("Your password is: %s\n", password) + fmt.Printf("Use this password? Please note that an obscured version of this \npassword (and not the " + + "password itself) will be stored under your \nconfiguration file, so keep this generated password " + + "in a safe place.\n") + if Confirm(true) { + break + } + } + case 'n': + return "" + default: + fs.Errorf(nil, "Bad choice %c", i) + } + return obscure.MustObscure(password) +} + // ChooseNumber asks the user to enter a number between min and max // inclusive prompting them with what. func ChooseNumber(what string, min, max int) int { @@ -188,7 +276,7 @@ func ShowRemotes() { func ChooseRemote() string { remotes := LoadedData().GetSectionList() sort.Strings(remotes) - return Choose("remote", remotes, nil, false) + return Choose("remote", "value", remotes, nil, "", true, false) } // mustFindByName finds the RegInfo for the remote name passed in or @@ -277,7 +365,7 @@ func backendConfig(ctx context.Context, name string, m configmap.Mapper, ri *fs. fmt.Println(out.Option.Help) in.Result = fmt.Sprint(Confirm(Default)) } else { - value := ChooseOption(out.Option) + value := ChooseOption(out.Option, name) if value != "" { err := out.Option.Set(value) if err != nil { @@ -316,51 +404,18 @@ func RemoteConfig(ctx context.Context, name string) error { } // ChooseOption asks the user to choose an option -func ChooseOption(o *fs.Option) string { +func ChooseOption(o *fs.Option, name string) string { fmt.Printf("Option %s.\n", o.Name) if o.Help != "" { // Show help string without empty lines. help := strings.Replace(strings.TrimSpace(o.Help), "\n\n", "\n", -1) fmt.Println(help) } + if o.IsPassword { - fmt.Printf("Choose an alternative below.") - actions := []string{"yYes type in my own password", "gGenerate random password"} - defaultAction := -1 - if !o.Required { - defaultAction = len(actions) - actions = append(actions, "nNo leave this optional password blank") - fmt.Printf(" Press Enter for the default (%s).", string(actions[defaultAction][0])) - } - fmt.Println() - var password string - var err error - switch i := CommandDefault(actions, defaultAction); i { - case 'y': - password = ChangePassword("the") - case 'g': - for { - fmt.Printf("Password strength in bits.\n64 is just about memorable\n128 is secure\n1024 is the maximum\n") - bits := ChooseNumber("Bits", 64, 1024) - password, err = Password(bits) - if err != nil { - log.Fatalf("Failed to make password: %v", err) - } - fmt.Printf("Your password is: %s\n", password) - fmt.Printf("Use this password? Please note that an obscured version of this \npassword (and not the " + - "password itself) will be stored under your \nconfiguration file, so keep this generated password " + - "in a safe place.\n") - if Confirm(true) { - break - } - } - case 'n': - return "" - default: - fs.Errorf(nil, "Bad choice %c", i) - } - return obscure.MustObscure(password) + return ChoosePassword(o.Required) } + what := fmt.Sprintf("%T value", o.Default) switch o.Default.(type) { case bool: @@ -375,8 +430,14 @@ func ChooseOption(o *fs.Option) string { what = "unsigned integer" } var in string + var defaultValue string + if o.Default == nil { + defaultValue = "" + } else { + defaultValue = fmt.Sprint(o.Default) + } + for { - fmt.Printf("Enter a %s. Press Enter for the default (%q).\n", what, fmt.Sprint(o.Default)) if len(o.Examples) > 0 { var values []string var help []string @@ -384,27 +445,20 @@ func ChooseOption(o *fs.Option) string { values = append(values, example.Value) help = append(help, example.Help) } - in = Choose(o.Name, values, help, !o.Exclusive) + in = Choose(o.Name, what, values, help, defaultValue, o.Required, !o.Exclusive) } else { - fmt.Printf("%s> ", o.Name) - in = ReadLine() + in = Enter(o.Name, what, defaultValue, o.Required) } - if in == "" { - if o.Required && fmt.Sprint(o.Default) == "" { - fmt.Printf("This value is required and it has no default.\n") + if in != "" { + newIn, err := configstruct.StringToInterface(o.Default, in) + if err != nil { + fmt.Printf("Failed to parse %q: %v\n", in, err) continue } - break + in = fmt.Sprint(newIn) // canonicalise } - newIn, err := configstruct.StringToInterface(o.Default, in) - if err != nil { - fmt.Printf("Failed to parse %q: %v\n", in, err) - continue - } - in = fmt.Sprint(newIn) // canonicalise - break + return in } - return in } // NewRemoteName asks the user for a name for a new remote @@ -440,7 +494,7 @@ func NewRemote(ctx context.Context, name string) error { // Set the type first for { - newType = ChooseOption(fsOption()) + newType = ChooseOption(fsOption(), name) ri, err = fs.Find(newType) if err != nil { fmt.Printf("Bad remote %q: %v\n", newType, err) diff --git a/fs/config/ui_test.go b/fs/config/ui_test.go index 550383914..5aec20904 100644 --- a/fs/config/ui_test.go +++ b/fs/config/ui_test.go @@ -20,7 +20,17 @@ import ( "github.com/stretchr/testify/require" ) -func testConfigFile(t *testing.T, configFileName string) func() { +var simpleOptions = []fs.Option{{ + Name: "bool", + Default: false, + IsPassword: false, +}, { + Name: "pass", + Default: "", + IsPassword: true, +}} + +func testConfigFile(t *testing.T, options []fs.Option, configFileName string) func() { ctx := context.Background() ci := fs.GetConfig(ctx) config.ClearConfigPassword() @@ -46,24 +56,18 @@ func testConfigFile(t *testing.T, configFileName string) func() { configfile.Install() assert.Equal(t, []string{}, config.Data().GetSectionList()) - // Fake a remote - fs.Register(&fs.RegInfo{ - Name: "config_test_remote", - Options: fs.Options{ - { - Name: "bool", - Default: false, - IsPassword: false, - }, - { - Name: "pass", - Default: "", - IsPassword: true, - }, - }, - }) + // Fake a filesystem/backend + backendName := "config_test_remote" + if regInfo, _ := fs.Find(backendName); regInfo != nil { + regInfo.Options = options + } else { + fs.Register(&fs.RegInfo{ + Name: backendName, + Options: options, + }) + } - // Undo the above + // Undo the above (except registered backend, unfortunately) return func() { err := os.Remove(path) assert.NoError(t, err) @@ -91,7 +95,7 @@ func makeReadLine(answers []string) func() string { } func TestCRUD(t *testing.T) { - defer testConfigFile(t, "crud.conf")() + defer testConfigFile(t, simpleOptions, "crud.conf")() ctx := context.Background() // script for creating remote @@ -129,7 +133,7 @@ func TestCRUD(t *testing.T) { } func TestChooseOption(t *testing.T) { - defer testConfigFile(t, "crud.conf")() + defer testConfigFile(t, simpleOptions, "crud.conf")() ctx := context.Background() // script for creating remote @@ -165,7 +169,7 @@ func TestChooseOption(t *testing.T) { } func TestNewRemoteName(t *testing.T) { - defer testConfigFile(t, "crud.conf")() + defer testConfigFile(t, simpleOptions, "crud.conf")() ctx := context.Background() // script for creating remote @@ -189,7 +193,7 @@ func TestNewRemoteName(t *testing.T) { func TestCreateUpdatePasswordRemote(t *testing.T) { ctx := context.Background() - defer testConfigFile(t, "update.conf")() + defer testConfigFile(t, simpleOptions, "update.conf")() for _, doObscure := range []bool{false, true} { for _, noObscure := range []bool{false, true} { @@ -244,5 +248,298 @@ func TestCreateUpdatePasswordRemote(t *testing.T) { }) } } - +} + +func TestDefaultRequired(t *testing.T) { + // By default options are optional (sic), regardless if a default value is defined. + // Setting Required=true means empty string is no longer allowed, except when + // a default value is set: Default value means empty string is always allowed! + options := []fs.Option{{ + Name: "string_required", + Required: true, + }, { + Name: "string_default", + Default: "AAA", + }, { + Name: "string_required_default", + Default: "BBB", + Required: true, + }} + + defer testConfigFile(t, options, "crud.conf")() + ctx := context.Background() + + // script for creating remote + config.ReadLine = makeReadLine([]string{ + "config_test_remote", // type + "111", // string_required + "222", // string_default + "333", // string_required_default + "y", // looks good, save + }) + require.NoError(t, config.NewRemote(ctx, "test")) + + assert.Equal(t, []string{"test"}, config.Data().GetSectionList()) + assert.Equal(t, "config_test_remote", config.FileGet("test", "type")) + assert.Equal(t, "111", config.FileGet("test", "string_required")) + assert.Equal(t, "222", config.FileGet("test", "string_default")) + assert.Equal(t, "333", config.FileGet("test", "string_required_default")) + + // delete remote + config.DeleteRemote("test") + assert.Equal(t, []string{}, config.Data().GetSectionList()) + + // script for creating remote + config.ReadLine = makeReadLine([]string{ + "config_test_remote", // type + "", // string_required - invalid (empty string not allowed) + "111", // string_required - valid + "", // string_default (empty string allowed, means use default) + "", // string_required_default (empty string allowed, means use default) + "y", // looks good, save + }) + require.NoError(t, config.NewRemote(ctx, "test")) + + assert.Equal(t, []string{"test"}, config.Data().GetSectionList()) + assert.Equal(t, "config_test_remote", config.FileGet("test", "type")) + assert.Equal(t, "111", config.FileGet("test", "string_required")) + assert.Equal(t, "", config.FileGet("test", "string_default")) + assert.Equal(t, "", config.FileGet("test", "string_required_default")) +} + +func TestMultipleChoice(t *testing.T) { + // Multiple-choice options can be set to the number of a predefined choice, or + // its text. Unless Exclusive=true, tested later, any free text input is accepted. + // + // By default options are optional, regardless if a default value is defined. + // Setting Required=true means empty string is no longer allowed, except when + // a default value is set: Default value means empty string is always allowed! + options := []fs.Option{{ + Name: "multiple_choice", + Examples: []fs.OptionExample{{ + Value: "AAA", + Help: "This is value AAA", + }, { + Value: "BBB", + Help: "This is value BBB", + }, { + Value: "CCC", + Help: "This is value CCC", + }}, + }, { + Name: "multiple_choice_required", + Required: true, + Examples: []fs.OptionExample{{ + Value: "AAA", + Help: "This is value AAA", + }, { + Value: "BBB", + Help: "This is value BBB", + }, { + Value: "CCC", + Help: "This is value CCC", + }}, + }, { + Name: "multiple_choice_default", + Default: "BBB", + Examples: []fs.OptionExample{{ + Value: "AAA", + Help: "This is value AAA", + }, { + Value: "BBB", + Help: "This is value BBB", + }, { + Value: "CCC", + Help: "This is value CCC", + }}, + }, { + Name: "multiple_choice_required_default", + Required: true, + Default: "BBB", + Examples: []fs.OptionExample{{ + Value: "AAA", + Help: "This is value AAA", + }, { + Value: "BBB", + Help: "This is value BBB", + }, { + Value: "CCC", + Help: "This is value CCC", + }}, + }} + + defer testConfigFile(t, options, "crud.conf")() + ctx := context.Background() + + // script for creating remote + config.ReadLine = makeReadLine([]string{ + "config_test_remote", // type + "3", // multiple_choice + "3", // multiple_choice_required + "3", // multiple_choice_default + "3", // multiple_choice_required_default + "y", // looks good, save + }) + require.NoError(t, config.NewRemote(ctx, "test")) + + assert.Equal(t, []string{"test"}, config.Data().GetSectionList()) + assert.Equal(t, "config_test_remote", config.FileGet("test", "type")) + assert.Equal(t, "CCC", config.FileGet("test", "multiple_choice")) + assert.Equal(t, "CCC", config.FileGet("test", "multiple_choice_required")) + assert.Equal(t, "CCC", config.FileGet("test", "multiple_choice_default")) + assert.Equal(t, "CCC", config.FileGet("test", "multiple_choice_required_default")) + + // delete remote + config.DeleteRemote("test") + assert.Equal(t, []string{}, config.Data().GetSectionList()) + + // script for creating remote + config.ReadLine = makeReadLine([]string{ + "config_test_remote", // type + "XXX", // multiple_choice + "XXX", // multiple_choice_required + "XXX", // multiple_choice_default + "XXX", // multiple_choice_required_default + "y", // looks good, save + }) + require.NoError(t, config.NewRemote(ctx, "test")) + + assert.Equal(t, []string{"test"}, config.Data().GetSectionList()) + assert.Equal(t, "config_test_remote", config.FileGet("test", "type")) + assert.Equal(t, "XXX", config.FileGet("test", "multiple_choice")) + assert.Equal(t, "XXX", config.FileGet("test", "multiple_choice_required")) + assert.Equal(t, "XXX", config.FileGet("test", "multiple_choice_default")) + assert.Equal(t, "XXX", config.FileGet("test", "multiple_choice_required_default")) + + // delete remote + config.DeleteRemote("test") + assert.Equal(t, []string{}, config.Data().GetSectionList()) + + // script for creating remote + config.ReadLine = makeReadLine([]string{ + "config_test_remote", // type + "", // multiple_choice (empty string allowed) + "", // multiple_choice_required - invalid (empty string not allowed) + "XXX", // multiple_choice_required - valid (value not restricted to examples) + "", // multiple_choice_default (empty string allowed) + "", // multiple_choice_required_default (required does nothing when default is set) + "y", // looks good, save + }) + require.NoError(t, config.NewRemote(ctx, "test")) + + assert.Equal(t, []string{"test"}, config.Data().GetSectionList()) + assert.Equal(t, "config_test_remote", config.FileGet("test", "type")) + assert.Equal(t, "", config.FileGet("test", "multiple_choice")) + assert.Equal(t, "XXX", config.FileGet("test", "multiple_choice_required")) + assert.Equal(t, "", config.FileGet("test", "multiple_choice_default")) + assert.Equal(t, "", config.FileGet("test", "multiple_choice_required_default")) +} + +func TestMultipleChoiceExclusive(t *testing.T) { + // Setting Exclusive=true on multiple-choice option means any input + // value must be from the predefined list, but empty string is allowed. + // Setting a default value makes no difference. + options := []fs.Option{{ + Name: "multiple_choice_exclusive", + Exclusive: true, + Examples: []fs.OptionExample{{ + Value: "AAA", + Help: "This is value AAA", + }, { + Value: "BBB", + Help: "This is value BBB", + }, { + Value: "CCC", + Help: "This is value CCC", + }}, + }, { + Name: "multiple_choice_exclusive_default", + Exclusive: true, + Default: "CCC", + Examples: []fs.OptionExample{{ + Value: "AAA", + Help: "This is value AAA", + }, { + Value: "BBB", + Help: "This is value BBB", + }, { + Value: "CCC", + Help: "This is value CCC", + }}, + }} + + defer testConfigFile(t, options, "crud.conf")() + ctx := context.Background() + + // script for creating remote + config.ReadLine = makeReadLine([]string{ + "config_test_remote", // type + "XXX", // multiple_choice_exclusive - invalid (not a value from examples) + "", // multiple_choice_exclusive - valid (empty string allowed) + "YYY", // multiple_choice_exclusive_default - invalid (not a value from examples) + "", // multiple_choice_exclusive_default - valid (empty string allowed) + "y", // looks good, save + }) + require.NoError(t, config.NewRemote(ctx, "test")) + + assert.Equal(t, []string{"test"}, config.Data().GetSectionList()) + assert.Equal(t, "config_test_remote", config.FileGet("test", "type")) + assert.Equal(t, "", config.FileGet("test", "multiple_choice_exclusive")) + assert.Equal(t, "", config.FileGet("test", "multiple_choice_exclusive_default")) +} + +func TestMultipleChoiceExclusiveRequired(t *testing.T) { + // Setting Required=true together with Exclusive=true on multiple-choice option + // means empty string is no longer allowed, except when a default value is set + // (default value means empty string is always allowed). + options := []fs.Option{{ + Name: "multiple_choice_exclusive_required", + Exclusive: true, + Required: true, + Examples: []fs.OptionExample{{ + Value: "AAA", + Help: "This is value AAA", + }, { + Value: "BBB", + Help: "This is value BBB", + }, { + Value: "CCC", + Help: "This is value CCC", + }}, + }, { + Name: "multiple_choice_exclusive_required_default", + Exclusive: true, + Required: true, + Default: "CCC", + Examples: []fs.OptionExample{{ + Value: "AAA", + Help: "This is value AAA", + }, { + Value: "BBB", + Help: "This is value BBB", + }, { + Value: "CCC", + Help: "This is value CCC", + }}, + }} + + defer testConfigFile(t, options, "crud.conf")() + ctx := context.Background() + + // script for creating remote + config.ReadLine = makeReadLine([]string{ + "config_test_remote", // type + "XXX", // multiple_choice_exclusive_required - invalid (not a value from examples) + "", // multiple_choice_exclusive_required - invalid (empty string not allowed) + "CCC", // multiple_choice_exclusive_required - valid + "XXX", // multiple_choice_exclusive_required_default - invalid (not a value from examples) + "", // multiple_choice_exclusive_required_default - valid (empty string allowed) + "y", // looks good, save + }) + require.NoError(t, config.NewRemote(ctx, "test")) + + assert.Equal(t, []string{"test"}, config.Data().GetSectionList()) + assert.Equal(t, "config_test_remote", config.FileGet("test", "type")) + assert.Equal(t, "CCC", config.FileGet("test", "multiple_choice_exclusive_required")) + assert.Equal(t, "", config.FileGet("test", "multiple_choice_exclusive_required_default")) } diff --git a/fs/registry.go b/fs/registry.go index 7ca447372..4f51062e3 100644 --- a/fs/registry.go +++ b/fs/registry.go @@ -124,20 +124,29 @@ const ( // Option is describes an option for the config wizard // // This also describes command line options and environment variables +// +// To create a multiple-choice option, specify the possible values +// in the Examples property. Whether the option's value is required +// to be one of these depends on other properties: +// - Default is to allow any value, either from specified examples, +// or any other value. To restrict exclusively to the specified +// examples, also set Exclusive=true. +// - If empty string should not be allowed then set Required=true, +// and do not set Default. type Option struct { Name string // name of the option in snake_case - Help string // Help, the first line only is used for the command line help - Provider string // Set to filter on provider - Default interface{} // default value, nil => "" + Help string // help, start with a single sentence on a single line that will be extracted for command line help + Provider string // set to filter on provider + Default interface{} // default value, nil => "", if set (and not to nil or "") then Required does nothing Value interface{} // value to be set by flags - Examples OptionExamples `json:",omitempty"` // config examples + Examples OptionExamples `json:",omitempty"` // predefined values that can be selected from list (multiple-choice option) ShortOpt string // the short option for this if required Hide OptionVisibility // set this to hide the config from the configurator or the command line - Required bool // this option is required + Required bool // this option is required, meaning value cannot be empty unless there is a default IsPassword bool // set if the option is a password NoPrefix bool // set if the option for this should not use the backend prefix Advanced bool // set if this is an advanced config option - Exclusive bool // set if the answer can only be one of the examples + Exclusive bool // set if the answer can only be one of the examples (empty string allowed unless Required or Default is set) } // BaseOption is an alias for Option used internally