From 72437a9ca2deb709bac646428250b60b0e27c57a Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Tue, 30 Sep 2025 11:59:40 +0100 Subject: [PATCH] config: add more human readable configmap.Simple output Before this, String() quoted every part of the config map even if it wasn't necessary. The new Human() method removes the quoting and adds the special case for "true" values. --- fs/config/configmap/configmap.go | 45 +++++-- .../configmap/configmap_external_test.go | 121 ++++++++++++++++++ fs/config/configmap/configmap_test.go | 24 ---- 3 files changed, 155 insertions(+), 35 deletions(-) create mode 100644 fs/config/configmap/configmap_external_test.go diff --git a/fs/config/configmap/configmap.go b/fs/config/configmap/configmap.go index ee8d4918f..d52514ef2 100644 --- a/fs/config/configmap/configmap.go +++ b/fs/config/configmap/configmap.go @@ -136,9 +136,11 @@ func (c Simple) Set(key, value string) { c[key] = value } -// String the map value the same way the config parser does, but with +// string the map value the same way the config parser does, but with // sorted keys for reproducibility. -func (c Simple) String() string { +// +// If human is set then use fewer quotes. +func (c Simple) string(human bool) string { var ks = make([]string, 0, len(c)) for k := range c { ks = append(ks, k) @@ -150,20 +152,41 @@ func (c Simple) String() string { out.WriteRune(',') } out.WriteString(k) - out.WriteRune('=') - out.WriteRune('\'') - for _, ch := range c[k] { - out.WriteRune(ch) - // Escape ' as '' - if ch == '\'' { - out.WriteRune(ch) - } + v := c[k] + if human && v == "true" { + continue + } + out.WriteRune('=') + if !human || strings.ContainsAny(v, `'":=,`) { + out.WriteRune('\'') + for _, ch := range v { + out.WriteRune(ch) + // Escape ' as '' + if ch == '\'' { + out.WriteRune(ch) + } + } + out.WriteRune('\'') + } else { + out.WriteString(v) } - out.WriteRune('\'') } return out.String() } +// Human converts the map value the same way the config parser does, +// but with sorted keys for reproducibility. This does it in human +// readable form with fewer quotes. +func (c Simple) Human() string { + return c.string(true) +} + +// String the map value the same way the config parser does, but with +// sorted keys for reproducibility. +func (c Simple) String() string { + return c.string(false) +} + // Encode from c into a string suitable for putting on the command line func (c Simple) Encode() (string, error) { if len(c) == 0 { diff --git a/fs/config/configmap/configmap_external_test.go b/fs/config/configmap/configmap_external_test.go new file mode 100644 index 000000000..236f02933 --- /dev/null +++ b/fs/config/configmap/configmap_external_test.go @@ -0,0 +1,121 @@ +package configmap_test + +import ( + "fmt" + "testing" + + "github.com/rclone/rclone/fs/config/configmap" + "github.com/rclone/rclone/fs/fspath" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestSimpleString(t *testing.T) { + for _, tt := range []struct { + name string + want string + in configmap.Simple + }{ + {name: "Nil", want: "", in: configmap.Simple(nil)}, + {name: "Empty", want: "", in: configmap.Simple{}}, + {name: "Basic", want: "config1='one'", in: configmap.Simple{ + "config1": "one", + }}, + {name: "Truthy", want: "config1='true',config2='true'", in: configmap.Simple{ + "config1": "true", + "config2": "true", + }}, + {name: "Quotable", want: `config1='"one"',config2=':two:',config3='''three''',config4='=four=',config5=',five,'`, in: configmap.Simple{ + "config1": `"one"`, + "config2": `:two:`, + "config3": `'three'`, + "config4": `=four=`, + "config5": `,five,`, + }}, + {name: "Order", want: "config1='one',config2='two',config3='three',config4='four',config5='five'", in: configmap.Simple{ + "config5": "five", + "config4": "four", + "config3": "three", + "config2": "two", + "config1": "one", + }}, + {name: "Escaping", want: "apple='',config1='o''n''e'", in: configmap.Simple{ + "config1": "o'n'e", + "apple": "", + }}, + } { + t.Run(tt.name, func(t *testing.T) { + // Check forwards + params := tt.in.String() + assert.Equal(t, tt.want, params) + + // Check config round trips through config parser + remote := ":local," + params + ":" + if params == "" { + remote = ":local:" + } + what := fmt.Sprintf("remote = %q", remote) + parsed, err := fspath.Parse(remote) + require.NoError(t, err, what) + if len(parsed.Config) != 0 || len(tt.in) != 0 { + assert.Equal(t, tt.in, parsed.Config, what) + } + }) + } + +} + +func TestSimpleHuman(t *testing.T) { + for _, tt := range []struct { + name string + want string + in configmap.Simple + }{ + {name: "Nil", want: "", in: configmap.Simple(nil)}, + {name: "Empty", want: "", in: configmap.Simple{}}, + {name: "Basic", want: "config1=one", in: configmap.Simple{ + "config1": "one", + }}, + {name: "Truthy", want: "config1,config2", in: configmap.Simple{ + "config1": "true", + "config2": "true", + }}, + {name: "Quotable", want: `config1='"one"',config2=':two:',config3='''three''',config4='=four=',config5=',five,'`, in: configmap.Simple{ + "config1": `"one"`, + "config2": `:two:`, + "config3": `'three'`, + "config4": `=four=`, + "config5": `,five,`, + }}, + {name: "Order", want: "config1=one,config2=two,config3=three,config4=four,config5=five", in: configmap.Simple{ + "config5": "five", + "config4": "four", + "config3": "three", + "config2": "two", + "config1": "one", + }}, + {name: "Escaping", want: "apple=,config1='o''n''e'", in: configmap.Simple{ + "config1": "o'n'e", + "apple": "", + }}, + } { + t.Run(tt.name, func(t *testing.T) { + // Check forwards + params := tt.in.Human() + assert.Equal(t, tt.want, params) + + // Check config round trips through config parser + remote := ":local," + params + ":" + if params == "" { + remote = ":local:" + } + what := fmt.Sprintf("remote = %q", remote) + parsed, err := fspath.Parse(remote) + require.NoError(t, err, what) + if len(parsed.Config) != 0 || len(tt.in) != 0 { + assert.Equal(t, tt.in, parsed.Config, what) + } + }) + } + +} diff --git a/fs/config/configmap/configmap_test.go b/fs/config/configmap/configmap_test.go index 85d4686f1..b68847360 100644 --- a/fs/config/configmap/configmap_test.go +++ b/fs/config/configmap/configmap_test.go @@ -246,30 +246,6 @@ func TestConfigMapClearSetters(t *testing.T) { assert.Equal(t, []Setter(nil), m.setters) } -func TestSimpleString(t *testing.T) { - // Basic - assert.Equal(t, "", Simple(nil).String()) - assert.Equal(t, "", Simple{}.String()) - assert.Equal(t, "config1='one'", Simple{ - "config1": "one", - }.String()) - - // Check ordering - assert.Equal(t, "config1='one',config2='two',config3='three',config4='four',config5='five'", Simple{ - "config5": "five", - "config4": "four", - "config3": "three", - "config2": "two", - "config1": "one", - }.String()) - - // Check escaping - assert.Equal(t, "apple='',config1='o''n''e'", Simple{ - "config1": "o'n'e", - "apple": "", - }.String()) -} - func TestSimpleEncode(t *testing.T) { for _, test := range []struct { in Simple