Support MultiVars (gopasspw#2476)

* Support MultiVars Fixes gopasspw#2457 RELEASE_NOTES=[ENHANCEMENT] gitconfig: Support MultiVars Signed-off-by: Dominik Schulz <[email protected]> * Add getter and fix tests Signed-off-by: Dominik Schulz <[email protected]> * Add GetAll getters Signed-off-by: Dominik Schulz <[email protected]> * Add multiple-insert tests Signed-off-by: Dominik Schulz <[email protected]> * Fix multi get/set handling Signed-off-by: Dominik Schulz <[email protected]> Signed-off-by: Dominik Schulz <[email protected]>
sergey-suslov · Dec 28, 2022 · 5845b2e · 5845b2e
1 parent 1ba67f7
commit 5845b2e
Show file tree

Hide file tree

Showing 6 changed files with 180 additions and 49 deletions.
diff --git a/internal/config/config.go b/internal/config/config.go
@@ -107,6 +107,11 @@ func (c *Config) Get(key string) string {
 	return c.root.Get(key)
 }
 
+// GetAll returns all values for the given key.
+func (c *Config) GetAll(key string) []string {
+	return c.root.GetAll(key)
+}
+
 // GetM returns the given key from the mount or the root config if mount is empty.
 func (c *Config) GetM(mount, key string) string {
 	if mount == "" {

diff --git a/pkg/gitconfig/config.go b/pkg/gitconfig/config.go
@@ -21,8 +21,7 @@ type Config struct {
 	readonly bool // do not allow modifying values (even in memory)
 	noWrites bool // do not persist changes to disk (e.g. for tests)
 	raw      strings.Builder
-	// TODO(#2457): support multi-vars
-	vars map[string]string
+	vars     map[string][]string
 }
 
 // Unset deletes a key.
@@ -38,11 +37,31 @@ func (c *Config) Unset(key string) error {
 
 	delete(c.vars, key)
 
-	return c.rewriteRaw(key, "", func(fKey, key, value, comment string) (string, bool) {
+	return c.rewriteRaw(key, "", func(fKey, key, value, comment, _ string) (string, bool) {
 		return "", true
 	})
 }
 
+// Get returns the first value of the key.
+func (c *Config) Get(key string) (string, bool) {
+	vs, found := c.vars[key]
+	if !found || len(vs) < 1 {
+		return "", false
+	}
+
+	return vs[0], true
+}
+
+// GetAll returns all values of the key.
+func (c *Config) GetAll(key string) ([]string, bool) {
+	vs, found := c.vars[key]
+	if !found {
+		return nil, false
+	}
+
+	return vs, true
+}
+
 // IsSet returns true if the key was set in this config.
 func (c *Config) IsSet(key string) bool {
 	_, present := c.vars[key]
@@ -66,18 +85,26 @@ func (c *Config) Set(key, value string) error {
 	}
 
 	if c.vars == nil {
-		c.vars = make(map[string]string, 16)
+		c.vars = make(map[string][]string, 16)
 	}
 
 	// already present at the same value, no need to rewrite the config
-	if v, found := c.vars[key]; found && v == value {
-		debug.Log("key %q with value %q already present. Not re-writing.", key, value)
+	if vs, found := c.vars[key]; found {
+		for _, v := range vs {
+			if v == value {
+				debug.Log("key %q with value %q already present. Not re-writing.", key, value)
 
-		return nil
+				return nil
+			}
+		}
 	}
 
-	_, present := c.vars[key]
-	c.vars[key] = value
+	vs, present := c.vars[key]
+	if vs == nil {
+		vs = make([]string, 1)
+	}
+	vs[0] = value
+	c.vars[key] = vs
 
 	debug.Log("set %q to %q", key, value)
 
@@ -90,7 +117,14 @@ func (c *Config) Set(key, value string) error {
 
 	debug.Log("updating value")
 
-	return c.rewriteRaw(key, value, func(fKey, sKey, value, comment string) (string, bool) {
+	var updated bool
+
+	return c.rewriteRaw(key, value, func(fKey, sKey, value, comment, line string) (string, bool) {
+		if updated {
+			return line, false
+		}
+		updated = true
+
 		return fmt.Sprintf(keyValueTpl, sKey, value, comment), false
 	})
 }
@@ -214,7 +248,7 @@ func (c *Config) flushRaw() error {
 	return nil
 }
 
-type parseFunc func(fqkn, skn, value, comment string) (newLine string, skipLine bool)
+type parseFunc func(fqkn, skn, value, comment, fullLine string) (newLine string, skipLine bool)
 
 // parseConfig implements a simple parser for the gitconfig subset we support.
 // The idea is to save all lines unaltered so we can reproduce the config
@@ -232,11 +266,11 @@ func parseConfig(in io.Reader, key, value string, cb parseFunc) []string {
 	var section string
 	var subsection string
 	for s.Scan() {
-		line := s.Text()
+		fullLine := s.Text()
 
-		lines = append(lines, line)
+		lines = append(lines, fullLine)
 
-		line = strings.TrimSpace(line)
+		line := strings.TrimSpace(fullLine)
 		if strings.HasPrefix(line, "#") {
 			continue
 		}
@@ -292,7 +326,7 @@ func parseConfig(in io.Reader, key, value string, cb parseFunc) []string {
 			oValue = value
 		}
 
-		newLine, skip := cb(fKey, wKey, oValue, comment)
+		newLine, skip := cb(fKey, wKey, oValue, comment, fullLine)
 		if skip {
 			// remove the last line
 			lines = lines[:len(lines)-1]
@@ -309,11 +343,11 @@ func parseConfig(in io.Reader, key, value string, cb parseFunc) []string {
 func NewFromMap(data map[string]string) *Config {
 	c := &Config{
 		readonly: true,
-		vars:     make(map[string]string, len(data)),
+		vars:     make(map[string][]string, len(data)),
 	}
 
 	for k, v := range data {
-		c.vars[k] = v
+		c.vars[k] = []string{v}
 	}
 
 	return c
@@ -337,11 +371,11 @@ func LoadConfig(fn string) (*Config, error) {
 // Invalid configs will be silently rejceted.
 func ParseConfig(r io.Reader) *Config {
 	c := &Config{
-		vars: make(map[string]string, 42),
+		vars: make(map[string][]string, 42),
 	}
 
-	lines := parseConfig(r, "", "", func(fk, k, v, comment string) (string, bool) {
-		c.vars[fk] = v
+	lines := parseConfig(r, "", "", func(fk, k, v, comment, _ string) (string, bool) {
+		c.vars[fk] = append(c.vars[fk], v)
 
 		return fmt.Sprintf(keyValueTpl, k, v, comment), false
 	})
@@ -369,7 +403,7 @@ func LoadConfigFromEnv(envPrefix string) *Config {
 		}
 	}
 
-	c.vars = make(map[string]string, count)
+	c.vars = make(map[string][]string, count)
 
 	for i := 0; i < count; i++ {
 		keyVar := fmt.Sprintf("%s%d", envPrefix+"_CONFIG_KEY_", i)
@@ -384,7 +418,7 @@ func LoadConfigFromEnv(envPrefix string) *Config {
 			}
 		}
 
-		c.vars[key] = value
+		c.vars[key] = append(c.vars[key], value)
 		debug.Log("added %s from env", key)
 	}
 

diff --git a/pkg/gitconfig/config_test.go b/pkg/gitconfig/config_test.go
@@ -1,6 +1,7 @@
 package gitconfig
 
 import (
+	"bytes"
 	"fmt"
 	"io/ioutil"
 	"math/rand"
@@ -29,6 +30,50 @@ func TestInsertOnce(t *testing.T) {
 `, c.raw.String())
 }
 
+func TestInsertMultipleSameKey(t *testing.T) {
+	t.Parallel()
+
+	c := &Config{
+		noWrites: true,
+	}
+
+	assert.NoError(t, c.Set("foo.bar", "baz"))
+	assert.Equal(t, `[foo]
+	bar = baz
+`, c.raw.String())
+	assert.NoError(t, c.Set("foo.bar", "zab"))
+	assert.Equal(t, `[foo]
+	bar = zab
+`, c.raw.String())
+}
+
+func TestGetAll(t *testing.T) {
+	t.Parallel()
+
+	r := bytes.NewReader([]byte(`[core]
+	foo = bar
+	foo = zab
+	foo = 123
+`))
+
+	c := ParseConfig(r)
+	require.NotNil(t, c)
+	vs, found := c.GetAll("core.foo")
+	assert.True(t, found)
+	assert.Equal(t, []string{"bar", "zab", "123"}, vs)
+
+	assert.NoError(t, c.Set("core.foo", "456"))
+	vs, found = c.GetAll("core.foo")
+	assert.True(t, found)
+	assert.Equal(t, []string{"456", "zab", "123"}, vs)
+
+	assert.Equal(t, `[core]
+	foo = 456
+	foo = zab
+	foo = 123
+`, c.raw.String())
+}
+
 func TestSubsection(t *testing.T) {
 	t.Parallel()
 
@@ -41,7 +86,7 @@ func TestSubsection(t *testing.T) {
 	c := ParseConfig(strings.NewReader(in))
 	c.noWrites = true
 
-	assert.Equal(t, c.vars["aliases.subsection with spaces.foo"], "bar")
+	assert.Equal(t, []string{"bar"}, c.vars["aliases.subsection with spaces.foo"])
 }
 
 func TestParseSection(t *testing.T) {
@@ -184,7 +229,7 @@ func TestNewFromMap(t *testing.T) {
 
 	cfg := NewFromMap(tc)
 	for k, v := range tc {
-		assert.Equal(t, v, cfg.vars[k])
+		assert.Equal(t, []string{v}, cfg.vars[k])
 	}
 
 	assert.True(t, cfg.IsSet("core.foo"))
@@ -206,9 +251,17 @@ func TestLoadConfig(t *testing.T) {
 	cfg, err := LoadConfig(fn)
 	require.NoError(t, err)
 
-	assert.Equal(t, "7", cfg.vars["core.int"])
-	assert.Equal(t, "foo", cfg.vars["core.string"])
-	assert.Equal(t, "false", cfg.vars["core.bar"])
+	v, ok := cfg.Get("core.int")
+	assert.True(t, ok)
+	assert.Equal(t, "7", v)
+
+	v, ok = cfg.Get("core.string")
+	assert.True(t, ok)
+	assert.Equal(t, "foo", v)
+
+	v, ok = cfg.Get("core.bar")
+	assert.True(t, ok)
+	assert.Equal(t, "false", v)
 }
 
 func TestLoadFromEnv(t *testing.T) {
@@ -231,6 +284,8 @@ func TestLoadFromEnv(t *testing.T) {
 
 	cfg := LoadConfigFromEnv(prefix)
 	for k, v := range tc {
-		assert.Equal(t, v, cfg.vars[k])
+		got, ok := cfg.Get(k)
+		assert.True(t, ok)
+		assert.Equal(t, v, got)
 	}
 }
diff --git a/pkg/gitconfig/configs.go b/pkg/gitconfig/configs.go
@@ -188,7 +188,7 @@ func (c *Configs) Get(key string) string {
 		if cfg == nil || cfg.vars == nil {
 			continue
 		}
-		if v, found := cfg.vars[key]; found {
+		if v, found := cfg.Get(key); found {
 			return v
 		}
 	}
@@ -198,13 +198,37 @@ func (c *Configs) Get(key string) string {
 	return ""
 }
 
+// GetAll returns all values for the given key from the first location that is found.
+// See the description of Get for more details.
+func (c *Configs) GetAll(key string) []string {
+	for _, cfg := range []*Config{
+		c.env,
+		c.worktree,
+		c.local,
+		c.global,
+		c.system,
+		c.Preset,
+	} {
+		if cfg == nil || cfg.vars == nil {
+			continue
+		}
+		if vs, found := cfg.GetAll(key); found {
+			return vs
+		}
+	}
+
+	debug.Log("no value for %s found", key)
+
+	return nil
+}
+
 // GetGlobal specifically ask the per-user (global) config for a key.
 func (c *Configs) GetGlobal(key string) string {
 	if c.global == nil {
 		return ""
 	}
 
-	if v, found := c.global.vars[key]; found {
+	if v, found := c.global.Get(key); found {
 		return v
 	}
 
@@ -219,7 +243,7 @@ func (c *Configs) GetLocal(key string) string {
 		return ""
 	}
 
-	if v, found := c.local.vars[key]; found {
+	if v, found := c.local.Get(key); found {
 		return v
 	}