community
/
rclone
3
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

config: fix handling of default, exclusive and required properties of multiple-choice options 

Previously an empty input (just pressing enter) was only allowed for multiple-choice
options that did not have the Exclusive property set. With this change the existing
Required property is introduced into the multiple choice handling, so that one can have
Exclusive and Required options where only a value from the list is allowed, and one can
have Exclusive but not Required options where an empty value is accepted but any
non-empty value must still be matching an item from the list.

Fixes #5549

See #5551
This commit is contained in:
albertony 2021-08-22 22:43:51 +02:00
parent bd4bbed592
commit 38e2f835ed
4 changed files with 470 additions and 109 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
fs/config/config.go
View File

@ -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{

208
fs/config/ui.go
View File

@ -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)

343
fs/config/ui_test.go
View File

@ -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"))
}

21
fs/registry.go
View File

@ -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