fs: add --all to rclone config create/update to ask all the config questions #3455

This also factors the config questions into a state based mechanism so
a backend can be configured using the same dialog as rclone config but
remotely.

cmd/config/config.go

@@ -167,10 +167,15 @@ time as the question.
     rclone config update name --continue state "*oauth-islocal,teamdrive,," result "true"
 
 Note that when using |--continue| all passwords should be passed in
-the clear (not obscured).
+the clear (not obscured). Any default config values should be passed
+in with each invocation of |--continue|.
 
 At the end of the non interactive process, rclone will return a result
 with |State| as empty string.
+
+If |--all| is passed then rclone will ask all the config questions,
+not just the post config questions. Any parameters are used as
+defaults for questions as usual.
 `, "|", "`")
 var configCreateCommand = &cobra.Command{
 	Use:   "create `name` `type` [`key` `value`]*",
@@ -229,6 +234,7 @@ func init() {
 		flags.BoolVarP(cmdFlags, &updateRemoteOpt.NoObscure, "no-obscure", "", false, "Force any passwords not to be obscured.")
 		flags.BoolVarP(cmdFlags, &updateRemoteOpt.NonInteractive, "non-interactive", "", false, "Don't interact with user and return questions.")
 		flags.BoolVarP(cmdFlags, &updateRemoteOpt.Continue, "continue", "", false, "Continue the configuration process with an answer.")
+		flags.BoolVarP(cmdFlags, &updateRemoteOpt.All, "all", "", false, "Ask the full set of config questions.")
 	}
 }
 

fs/backend_config.go

@@ -7,6 +7,7 @@ package fs
 import (
 	"context"
 	"fmt"
+	"strconv"
 	"strings"
 
 	"github.com/pkg/errors"
@@ -16,6 +17,9 @@ import (
 const (
 	// ConfigToken is the key used to store the token under
 	ConfigToken = "token"
+
+	// ConfigKeyEphemeralPrefix marks config keys which shouldn't be stored in the config file
+	ConfigKeyEphemeralPrefix = "config_"
 )
 
 // ConfigOAuth should be called to do the OAuth
@@ -52,7 +56,10 @@ var ConfigOAuth func(ctx context.Context, name string, m configmap.Mapper, ri *R
 //
 // Where the questions ask for a name then this should start with
 // "config_" to show it is an ephemeral config input rather than the
-// actual value stored in the config file.
+// actual value stored in the config file. Names beginning with
+// "config_fs_" are reserved for internal use.
+//
+// State names starting with "*" are reserved for internal use.
 type ConfigIn struct {
 	State  string // State to run
 	Result string // Result from previous Option
@@ -258,10 +265,11 @@ func StatePop(state string) (newState string, value string) {
 
 // BackendConfig calls the config for the backend in ri
 //
-// It wraps any OAuth transactions as necessary so only straight forward config questions are emitted
-func BackendConfig(ctx context.Context, name string, m configmap.Mapper, ri *RegInfo, in ConfigIn) (out *ConfigOut, err error) {
+// It wraps any OAuth transactions as necessary so only straight
+// forward config questions are emitted
+func BackendConfig(ctx context.Context, name string, m configmap.Mapper, ri *RegInfo, choices configmap.Getter, in ConfigIn) (out *ConfigOut, err error) {
 	for {
-		out, err = backendConfigStep(ctx, name, m, ri, in)
+		out, err = backendConfigStep(ctx, name, m, ri, choices, in)
 		if err != nil {
 			break
 		}
@@ -286,24 +294,130 @@ func BackendConfig(ctx context.Context, name string, m configmap.Mapper, ri *Reg
 	return out, err
 }
 
-func backendConfigStep(ctx context.Context, name string, m configmap.Mapper, ri *RegInfo, in ConfigIn) (out *ConfigOut, err error) {
-	ci := GetConfig(ctx)
-	if ri.Config == nil {
-		return nil, nil
+// ConfigAll should be passed in as the initial state to run the
+// entire config
+const ConfigAll = "*all"
+
+// Run the config state machine for the normal config
+func configAll(ctx context.Context, name string, m configmap.Mapper, ri *RegInfo, in ConfigIn) (out *ConfigOut, err error) {
+	if len(ri.Options) == 0 {
+		return ConfigGoto("*postconfig")
 	}
+
+	// States are encoded
+	//
+	//     *all-ACTION,NUMBER,ADVANCED
+	//
+	// Where NUMBER is the curent state, ADVANCED is a flag true or false
+	// to say whether we are asking about advanced config and
+	// ACTION is what the state should be doing next.
+	stateParams, state := StatePop(in.State)
+	stateParams, stateNumber := StatePop(stateParams)
+	_, stateAdvanced := StatePop(stateParams)
+
+	optionNumber := 0
+	advanced := stateAdvanced == "true"
+	if stateNumber != "" {
+		optionNumber, err = strconv.Atoi(stateNumber)
+		if err != nil {
+			return nil, errors.Wrap(err, "internal error: bad state number")
+		}
+	}
+
+	// Detect if reached the end of the questions
+	if optionNumber == len(ri.Options) {
+		if ri.Options.HasAdvanced() {
+			return ConfigConfirm("*all-advanced", false, "config_fs_advanced", "Edit advanced config?")
+		}
+		return ConfigGoto("*postconfig")
+	} else if optionNumber < 0 || optionNumber > len(ri.Options) {
+		return nil, errors.New("internal error: option out of range")
+	}
+
+	// Make the next state
+	newState := func(state string, i int, advanced bool) string {
+		return StatePush("", state, fmt.Sprint(i), fmt.Sprint(advanced))
+	}
+
+	// Find the current option
+	option := &ri.Options[optionNumber]
+
+	switch state {
+	case "*all":
+		// If option is hidden or doesn't match advanced setting then skip it
+		if option.Hide&OptionHideConfigurator != 0 || option.Advanced != advanced {
+			return ConfigGoto(newState("*all", optionNumber+1, advanced))
+		}
+
+		// Skip this question if it isn't the correct provider
+		provider, _ := m.Get(ConfigProvider)
+		if !MatchProvider(option.Provider, provider) {
+			return ConfigGoto(newState("*all", optionNumber+1, advanced))
+		}
+
+		out = &ConfigOut{
+			State:  newState("*all-set", optionNumber, advanced),
+			Option: option,
+		}
+
+		// Filter examples by provider if necessary
+		if provider != "" && len(option.Examples) > 0 {
+			optionCopy := option.Copy()
+			optionCopy.Examples = OptionExamples{}
+			for _, example := range option.Examples {
+				if MatchProvider(example.Provider, provider) {
+					optionCopy.Examples = append(optionCopy.Examples, example)
+				}
+			}
+			out.Option = optionCopy
+		}
+
+		return out, nil
+	case "*all-set":
+		// Set the value if not different to current
+		// Note this won't set blank values in the config file
+		// if the default is blank
+		currentValue, _ := m.Get(option.Name)
+		if currentValue != in.Result {
+			m.Set(option.Name, in.Result)
+		}
+		// Find the next question
+		return ConfigGoto(newState("*all", optionNumber+1, advanced))
+	case "*all-advanced":
+		// Reply to edit advanced question
+		if in.Result == "true" {
+			return ConfigGoto(newState("*all", 0, true))
+		}
+		return ConfigGoto("*postconfig")
+	}
+	return nil, errors.Errorf("internal error: bad state %q", state)
+}
+
+func backendConfigStep(ctx context.Context, name string, m configmap.Mapper, ri *RegInfo, choices configmap.Getter, in ConfigIn) (out *ConfigOut, err error) {
+	ci := GetConfig(ctx)
 	Debugf(name, "config in: state=%q, result=%q", in.State, in.Result)
 	defer func() {
 		Debugf(name, "config out: out=%+v, err=%v", out, err)
 	}()
 
 	switch {
+	case strings.HasPrefix(in.State, ConfigAll):
+		// Do all config
+		out, err = configAll(ctx, name, m, ri, in)
 	case strings.HasPrefix(in.State, "*oauth"):
 		// Do internal oauth states
 		out, err = ConfigOAuth(ctx, name, m, ri, in)
+	case strings.HasPrefix(in.State, "*postconfig"):
+		// Do the post config starting from state ""
+		in.State = ""
+		return backendConfigStep(ctx, name, m, ri, choices, in)
 	case strings.HasPrefix(in.State, "*"):
 		err = errors.Errorf("unknown internal state %q", in.State)
 	default:
 		// Otherwise pass to backend
+		if ri.Config == nil {
+			return nil, nil
+		}
 		out, err = ri.Config(ctx, name, m, in)
 	}
 	if err != nil {
@@ -325,8 +439,8 @@ func backendConfigStep(ctx context.Context, name string, m configmap.Mapper, ri
 		if out.Option.Name == "" {
 			return nil, errors.New("internal error: no name set in Option")
 		}
-		// If override value is set in the config then use that
-		if result, ok := m.Get(out.Option.Name); ok {
+		// If override value is set in the choices then use that
+		if result, ok := choices.Get(out.Option.Name); ok {
 			Debugf(nil, "Override value found, choosing value %q for state %q", result, out.State)
 			return ConfigResult(out.State, result)
 		}
@@ -336,6 +450,52 @@ func backendConfigStep(ctx context.Context, name string, m configmap.Mapper, ri
 			Debugf(nil, "Auto confirm is set, choosing default %q for state %q, override by setting config parameter %q", result, out.State, out.Option.Name)
 			return ConfigResult(out.State, result)
 		}
+		// If fs.ConfigEdit is set then make the default value
+		// in the config the current value.
+		if result, ok := choices.Get(ConfigEdit); ok && result == "true" {
+			if value, ok := m.Get(out.Option.Name); ok {
+				newOption := out.Option.Copy()
+				oldValue := newOption.Value
+				err = newOption.Set(value)
+				if err != nil {
+					Errorf(nil, "Failed to set %q from %q - using default: %v", out.Option.Name, value, err)
+				} else {
+					newOption.Default = newOption.Value
+					newOption.Value = oldValue
+					out.Option = newOption
+				}
+			}
+		}
 	}
 	return out, nil
 }
+
+// MatchProvider returns true if provider matches the providerConfig string.
+//
+// The providerConfig string can either be a list of providers to
+// match, or if it starts with "!" it will be a list of providers not
+// to match.
+//
+// If either providerConfig or provider is blank then it will return true
+func MatchProvider(providerConfig, provider string) bool {
+	if providerConfig == "" || provider == "" {
+		return true
+	}
+	negate := false
+	if strings.HasPrefix(providerConfig, "!") {
+		providerConfig = providerConfig[1:]
+		negate = true
+	}
+	providers := strings.Split(providerConfig, ",")
+	matched := false
+	for _, p := range providers {
+		if p == provider {
+			matched = true
+			break
+		}
+	}
+	if negate {
+		return !matched
+	}
+	return matched
+}

fs/backend_config_test.go

@@ -1,6 +1,7 @@
 package fs
 
 import (
+	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -35,3 +36,24 @@ func TestStatePop(t *testing.T) {
 	assert.Equal(t, "1,2,3", value)
 	assert.Equal(t, "a", state)
 }
+
+func TestMatchProvider(t *testing.T) {
+	for _, test := range []struct {
+		config   string
+		provider string
+		want     bool
+	}{
+		{"", "", true},
+		{"one", "one", true},
+		{"one,two", "two", true},
+		{"one,two,three", "two", true},
+		{"one", "on", false},
+		{"one,two,three", "tw", false},
+		{"!one,two,three", "two", false},
+		{"!one,two,three", "four", true},
+	} {
+		what := fmt.Sprintf("%q,%q", test.config, test.provider)
+		got := MatchProvider(test.config, test.provider)
+		assert.Equal(t, test.want, got, what)
+	}
+}

fs/config.go

@@ -37,6 +37,9 @@ var (
 
 	// ConfigProvider is the config key used for provider options
 	ConfigProvider = "provider"
+
+	// ConfigEdit is the config key used to show we wish to edit existing entries
+	ConfigEdit = "config_fs_edit"
 )
 
 // ConfigInfo is filesystem config options

fs/config/config.go

@@ -19,6 +19,7 @@ import (
 
 	"github.com/rclone/rclone/fs"
 	"github.com/rclone/rclone/fs/cache"
+	"github.com/rclone/rclone/fs/config/configmap"
 	"github.com/rclone/rclone/fs/config/obscure"
 	"github.com/rclone/rclone/fs/fspath"
 	"github.com/rclone/rclone/fs/rc"
@@ -418,6 +419,8 @@ type UpdateRemoteOpt struct {
 	NonInteractive bool `json:"nonInteractive"`
 	// If set then supply state and result parameters to continue the process
 	Continue bool `json:"continue"`
+	// If set then ask all the questions, not just the post config questions
+	All bool `json:"all"`
 }
 
 // UpdateRemote adds the keyValues passed in to the remote of name.
@@ -431,7 +434,7 @@ func UpdateRemote(ctx context.Context, name string, keyValues rc.Params, opt Upd
 		return nil, err
 	}
 	interactive := !(opt.NonInteractive || opt.Continue)
-	if interactive {
+	if interactive && !opt.All {
 		ctx = suppressConfirm(ctx)
 	}
 
@@ -445,41 +448,48 @@ func UpdateRemote(ctx context.Context, name string, keyValues rc.Params, opt Upd
 		return nil, errors.Errorf("couldn't find backend for type %q", fsType)
 	}
 
-	if !opt.Continue {
-		// Work out which options need to be obscured
-		needsObscure := map[string]struct{}{}
-		if !opt.NoObscure {
-			for _, option := range ri.Options {
-				if option.IsPassword {
-					needsObscure[option.Name] = struct{}{}
+	// Work out which options need to be obscured
+	needsObscure := map[string]struct{}{}
+	if !opt.NoObscure {
+		for _, option := range ri.Options {
+			if option.IsPassword {
+				needsObscure[option.Name] = struct{}{}
+			}
+		}
+	}
+
+	choices := configmap.Simple{}
+	m := fs.ConfigMap(ri, name, nil)
+
+	// Set the config
+	for k, v := range keyValues {
+		vStr := fmt.Sprint(v)
+		// Obscure parameter if necessary
+		if _, ok := needsObscure[k]; ok {
+			_, err := obscure.Reveal(vStr)
+			if err != nil || opt.Obscure {
+				// If error => not already obscured, so obscure it
+				// or we are forced to obscure
+				vStr, err = obscure.Obscure(vStr)
+				if err != nil {
+					return nil, errors.Wrap(err, "UpdateRemote: obscure failed")
 				}
 			}
 		}
-
-		// Set the config
-		for k, v := range keyValues {
-			vStr := fmt.Sprint(v)
-			// Obscure parameter if necessary
-			if _, ok := needsObscure[k]; ok {
-				_, err := obscure.Reveal(vStr)
-				if err != nil || opt.Obscure {
-					// If error => not already obscured, so obscure it
-					// or we are forced to obscure
-					vStr, err = obscure.Obscure(vStr)
-					if err != nil {
-						return nil, errors.Wrap(err, "UpdateRemote: obscure failed")
-					}
-				}
-			}
-			LoadedData().SetValue(name, k, vStr)
+		choices.Set(k, vStr)
+		if !strings.HasPrefix(k, fs.ConfigKeyEphemeralPrefix) {
+			m.Set(k, vStr)
 		}
 	}
 
 	if interactive {
-		err = RemoteConfig(ctx, name)
+		var state = ""
+		if opt.All {
+			state = fs.ConfigAll
+		}
+		err = backendConfig(ctx, name, m, ri, choices, state)
 	} else {
 		// Start the config state machine
-		m := fs.ConfigMap(ri, name, nil)
 		in := fs.ConfigIn{}
 		if opt.Continue {
 			if state, ok := keyValues["state"]; ok {
@@ -493,7 +503,10 @@ func UpdateRemote(ctx context.Context, name string, keyValues rc.Params, opt Upd
 				return nil, errors.New("UpdateRemote: need result parameter with --continue")
 			}
 		}
-		out, err = fs.BackendConfig(ctx, name, m, ri, in)
+		if in.State == "" && opt.All {
+			in.State = fs.ConfigAll
+		}
+		out, err = fs.BackendConfig(ctx, name, m, ri, choices, in)
 	}
 	if err != nil {
 		return nil, err

fs/config/rc.go

@@ -118,6 +118,7 @@ func init() {
     - noObscure - declare passwords are already obscured and don't need obscuring
     - nonInteractive - don't interact with a user, return questions
     - continue - continue the config process with an answer
+    - all - ask all the config questions not just the post config ones
 `
 		}
 		rc.Add(rc.Call{

fs/config/ui.go

@@ -241,18 +241,15 @@ func OkRemote(name string) bool {
 	return false
 }
 
-// PostConfig configures the backend after the main config has been done
+// backendConfig configures the backend starting from the state passed in
 //
 // The is the user interface loop that drives the post configuration backend config.
-func PostConfig(ctx context.Context, name string, m configmap.Mapper, ri *fs.RegInfo) error {
-	if ri.Config == nil {
-		return errors.New("backend doesn't support reconnect or authorize")
-	}
+func backendConfig(ctx context.Context, name string, m configmap.Mapper, ri *fs.RegInfo, choices configmap.Getter, startState string) error {
 	in := fs.ConfigIn{
-		State: "",
+		State: startState,
 	}
 	for {
-		out, err := fs.BackendConfig(ctx, name, m, ri, in)
+		out, err := fs.BackendConfig(ctx, name, m, ri, choices, in)
 		if err != nil {
 			return err
 		}
@@ -297,6 +294,16 @@ func PostConfig(ctx context.Context, name string, m configmap.Mapper, ri *fs.Reg
 	return nil
 }
 
+// PostConfig configures the backend after the main config has been done
+//
+// The is the user interface loop that drives the post configuration backend config.
+func PostConfig(ctx context.Context, name string, m configmap.Mapper, ri *fs.RegInfo) error {
+	if ri.Config == nil {
+		return errors.New("backend doesn't support reconnect or authorize")
+	}
+	return backendConfig(ctx, name, m, ri, configmap.Simple{}, "")
+}
+
 // RemoteConfig runs the config helper for the remote if needed
 func RemoteConfig(ctx context.Context, name string) error {
 	fmt.Printf("Remote config\n")
@@ -308,39 +315,8 @@ func RemoteConfig(ctx context.Context, name string) error {
 	return PostConfig(ctx, name, m, ri)
 }
 
-// matchProvider returns true if provider matches the providerConfig string.
-//
-// The providerConfig string can either be a list of providers to
-// match, or if it starts with "!" it will be a list of providers not
-// to match.
-//
-// If either providerConfig or provider is blank then it will return true
-func matchProvider(providerConfig, provider string) bool {
-	if providerConfig == "" || provider == "" {
-		return true
-	}
-	negate := false
-	if strings.HasPrefix(providerConfig, "!") {
-		providerConfig = providerConfig[1:]
-		negate = true
-	}
-	providers := strings.Split(providerConfig, ",")
-	matched := false
-	for _, p := range providers {
-		if p == provider {
-			matched = true
-			break
-		}
-	}
-	if negate {
-		return !matched
-	}
-	return matched
-}
-
 // ChooseOption asks the user to choose an option
 func ChooseOption(o *fs.Option, name string) string {
-	var subProvider = getWithDefault(name, fs.ConfigProvider, "")
 	fmt.Println(o.Help)
 	if o.IsPassword {
 		actions := []string{"yYes type in my own password", "gGenerate random password"}
@@ -397,10 +373,8 @@ func ChooseOption(o *fs.Option, name string) string {
 			var values []string
 			var help []string
 			for _, example := range o.Examples {
-				if matchProvider(example.Provider, subProvider) {
-					values = append(values, example.Value)
-					help = append(help, example.Help)
-				}
+				values = append(values, example.Value)
+				help = append(help, example.Help)
 			}
 			in = Choose(o.Name, values, help, !o.Exclusive)
 		} else {
@@ -450,38 +424,13 @@ func NewRemoteName() (name string) {
 
 // editOptions edits the options.  If new is true then it just allows
 // entry and doesn't show any old values.
-func editOptions(ri *fs.RegInfo, name string, isNew bool) {
+func editOptions(ctx context.Context, ri *fs.RegInfo, name string, isNew bool) error {
 	fmt.Printf("** See help for %s backend at: https://rclone.org/%s/ **\n\n", ri.Name, ri.FileName())
-	hasAdvanced := false
-	for _, advanced := range []bool{false, true} {
-		if advanced {
-			if !hasAdvanced {
-				break
-			}
-			fmt.Printf("Edit advanced config? (y/n)\n")
-			if !Confirm(false) {
-				break
-			}
-		}
-		for _, option := range ri.Options {
-			isVisible := option.Hide&fs.OptionHideConfigurator == 0
-			hasAdvanced = hasAdvanced || (option.Advanced && isVisible)
-			if option.Advanced != advanced {
-				continue
-			}
-			subProvider := getWithDefault(name, fs.ConfigProvider, "")
-			if matchProvider(option.Provider, subProvider) && isVisible {
-				if !isNew {
-					fmt.Printf("Value %q = %q\n", option.Name, FileGet(name, option.Name))
-					fmt.Printf("Edit? (y/n)>\n")
-					if !Confirm(false) {
-						continue
-					}
-				}
-				FileSet(name, option.Name, ChooseOption(&option, name))
-			}
-		}
+	m := fs.ConfigMap(ri, name, nil)
+	choices := configmap.Simple{
+		fs.ConfigEdit: fmt.Sprint(isNew),
 	}
+	return backendConfig(ctx, name, m, ri, choices, fs.ConfigAll)
 }
 
 // NewRemote make a new remote from its name
@@ -504,8 +453,7 @@ func NewRemote(ctx context.Context, name string) error {
 	}
 	LoadedData().SetValue(name, "type", newType)
 
-	editOptions(ri, name, true)
-	err = RemoteConfig(ctx, name)
+	err = editOptions(ctx, ri, name, true)
 	if err != nil {
 		return err
 	}
@@ -521,13 +469,16 @@ func EditRemote(ctx context.Context, ri *fs.RegInfo, name string) error {
 	ShowRemote(name)
 	fmt.Printf("Edit remote\n")
 	for {
-		editOptions(ri, name, false)
+		err := editOptions(ctx, ri, name, true)
+		if err != nil {
+			return err
+		}
 		if OkRemote(name) {
 			break
 		}
 	}
 	SaveConfig()
-	return RemoteConfig(ctx, name)
+	return nil
 }
 
 // DeleteRemote gets the user to delete a remote

fs/config/ui_internal_test.go

@@ -1,29 +0,0 @@
-package config
-
-import (
-	"fmt"
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-)
-
-func TestMatchProvider(t *testing.T) {
-	for _, test := range []struct {
-		config   string
-		provider string
-		want     bool
-	}{
-		{"", "", true},
-		{"one", "one", true},
-		{"one,two", "two", true},
-		{"one,two,three", "two", true},
-		{"one", "on", false},
-		{"one,two,three", "tw", false},
-		{"!one,two,three", "two", false},
-		{"!one,two,three", "four", true},
-	} {
-		what := fmt.Sprintf("%q,%q", test.config, test.provider)
-		got := matchProvider(test.config, test.provider)
-		assert.Equal(t, test.want, got, what)
-	}
-}

fs/config/ui_test.go

@@ -148,7 +148,7 @@ func TestChooseOption(t *testing.T) {
 	}
 	require.NoError(t, config.NewRemote(ctx, "test"))
 
-	assert.Equal(t, "false", config.FileGet("test", "bool"))
+	assert.Equal(t, "", config.FileGet("test", "bool")) // this is the default now
 	assert.Equal(t, "not very random password", obscure.MustReveal(config.FileGet("test", "pass")))
 
 	// script for creating remote

fs/fs.go

@@ -157,6 +157,17 @@ func (os Options) NonDefault(m configmap.Getter) configmap.Simple {
 	return nonDefault
 }
 
+// HasAdvanced discovers if any options have an Advanced setting
+func (os Options) HasAdvanced() bool {
+	for i := range os {
+		opt := &os[i]
+		if opt.Advanced {
+			return true
+		}
+	}
+	return false
+}
+
 // OptionVisibility controls whether the options are visible in the
 // configurator or the command line.
 type OptionVisibility byte
@@ -256,6 +267,13 @@ func (o *Option) EnvVarName(prefix string) string {
 	return OptionToEnv(prefix + "-" + o.Name)
 }
 
+// Copy makes a shallow copy of the option
+func (o *Option) Copy() *Option {
+	copy := new(Option)
+	*copy = *o
+	return copy
+}
+
 // OptionExamples is a slice of examples
 type OptionExamples []OptionExample