Skip to content

fix(config): coerce config set values and keep config get in lockstep #96

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jamesainslie merged 1 commit into from
Aug 12, 2026
Merged

fix(config): coerce config set values and keep config get in lockstep #96

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 67 additions & 38 deletions cmd/dot/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import (
"fmt"
"os"
"path/filepath"
"sort"
"strconv"
"strings"

"github.com/spf13/cobra"
Expand Down Expand Up @@ -217,49 +219,76 @@ func runConfigGet(key string) error {

// getValidConfigKeys returns all valid configuration keys for completion.
func getValidConfigKeys() []string {
return []string{
"directories.package",
"directories.target",
"directories.manifest",
"logging.level",
"logging.format",
"logging.destination",
"symlinks.mode",
"symlinks.backup_suffix",
"symlinks.backup_dir",
"dotfile.prefix",
"dotfile.translate",
"dotfile.package_name_mapping",
"output.format",
"output.color",
"packages.sort_by",
keys := make([]string, 0, len(configGetters))
for key := range configGetters {
keys = append(keys, key)
}
sort.Strings(keys)
return keys
}

// configGetters maps every gettable key to its renderer. Kept in lockstep
// with the setters in internal/config: every key config set accepts must
// resolve here, or a freshly set key reads back as unknown.
var configGetters = map[string]func(cfg *dot.ExtendedConfig) string{
"directories.package": func(c *dot.ExtendedConfig) string { return c.Directories.Package },
"directories.target": func(c *dot.ExtendedConfig) string { return c.Directories.Target },
"directories.manifest": func(c *dot.ExtendedConfig) string { return c.Directories.Manifest },

"logging.level": func(c *dot.ExtendedConfig) string { return c.Logging.Level },
"logging.format": func(c *dot.ExtendedConfig) string { return c.Logging.Format },
"logging.destination": func(c *dot.ExtendedConfig) string { return c.Logging.Destination },
"logging.file": func(c *dot.ExtendedConfig) string { return c.Logging.File },

"symlinks.mode": func(c *dot.ExtendedConfig) string { return c.Symlinks.Mode },
"symlinks.folding": func(c *dot.ExtendedConfig) string { return strconv.FormatBool(c.Symlinks.Folding) },
"symlinks.overwrite": func(c *dot.ExtendedConfig) string { return strconv.FormatBool(c.Symlinks.Overwrite) },
"symlinks.backup": func(c *dot.ExtendedConfig) string { return strconv.FormatBool(c.Symlinks.Backup) },
"symlinks.backup_suffix": func(c *dot.ExtendedConfig) string { return c.Symlinks.BackupSuffix },
"symlinks.backup_dir": func(c *dot.ExtendedConfig) string { return c.Symlinks.BackupDir },

"ignore.use_defaults": func(c *dot.ExtendedConfig) string { return strconv.FormatBool(c.Ignore.UseDefaults) },
"ignore.patterns": func(c *dot.ExtendedConfig) string { return strings.Join(c.Ignore.Patterns, ",") },
"ignore.overrides": func(c *dot.ExtendedConfig) string { return strings.Join(c.Ignore.Overrides, ",") },

"dotfile.translate": func(c *dot.ExtendedConfig) string { return strconv.FormatBool(c.Dotfile.Translate) },
"dotfile.prefix": func(c *dot.ExtendedConfig) string { return c.Dotfile.Prefix },
"dotfile.package_name_mapping": func(c *dot.ExtendedConfig) string {
return strconv.FormatBool(c.Dotfile.PackageNameMapping)
},

"output.format": func(c *dot.ExtendedConfig) string { return c.Output.Format },
"output.color": func(c *dot.ExtendedConfig) string { return c.Output.Color },
"output.progress": func(c *dot.ExtendedConfig) string { return strconv.FormatBool(c.Output.Progress) },
"output.verbosity": func(c *dot.ExtendedConfig) string { return strconv.Itoa(c.Output.Verbosity) },
"output.width": func(c *dot.ExtendedConfig) string { return strconv.Itoa(c.Output.Width) },

"operations.dry_run": func(c *dot.ExtendedConfig) string { return strconv.FormatBool(c.Operations.DryRun) },
"operations.atomic": func(c *dot.ExtendedConfig) string { return strconv.FormatBool(c.Operations.Atomic) },
"operations.max_parallel": func(c *dot.ExtendedConfig) string { return strconv.Itoa(c.Operations.MaxParallel) },

"packages.sort_by": func(c *dot.ExtendedConfig) string { return c.Packages.SortBy },
"packages.auto_discover": func(c *dot.ExtendedConfig) string { return strconv.FormatBool(c.Packages.AutoDiscover) },
"packages.validate_names": func(c *dot.ExtendedConfig) string { return strconv.FormatBool(c.Packages.ValidateNames) },

"doctor.auto_fix": func(c *dot.ExtendedConfig) string { return strconv.FormatBool(c.Doctor.AutoFix) },
"doctor.check_manifest": func(c *dot.ExtendedConfig) string { return strconv.FormatBool(c.Doctor.CheckManifest) },
"doctor.check_broken_links": func(c *dot.ExtendedConfig) string {
return strconv.FormatBool(c.Doctor.CheckBrokenLinks)
},
"doctor.check_orphaned": func(c *dot.ExtendedConfig) string { return strconv.FormatBool(c.Doctor.CheckOrphaned) },
"doctor.check_permissions": func(c *dot.ExtendedConfig) string {
return strconv.FormatBool(c.Doctor.CheckPermissions)
},

"experimental.parallel": func(c *dot.ExtendedConfig) string { return strconv.FormatBool(c.Experimental.Parallel) },
"experimental.profiling": func(c *dot.ExtendedConfig) string { return strconv.FormatBool(c.Experimental.Profiling) },
}

// getConfigValue retrieves a value from config by key path.
func getConfigValue(cfg *dot.ExtendedConfig, key string) (string, error) {
getters := map[string]func() string{
"directories.package": func() string { return cfg.Directories.Package },
"directories.target": func() string { return cfg.Directories.Target },
"directories.manifest": func() string { return cfg.Directories.Manifest },
"logging.level": func() string { return cfg.Logging.Level },
"logging.format": func() string { return cfg.Logging.Format },
"logging.destination": func() string { return cfg.Logging.Destination },
"symlinks.mode": func() string { return cfg.Symlinks.Mode },
"symlinks.backup_suffix": func() string { return cfg.Symlinks.BackupSuffix },
"symlinks.backup_dir": func() string { return cfg.Symlinks.BackupDir },
"dotfile.prefix": func() string { return cfg.Dotfile.Prefix },
"dotfile.translate": func() string { return fmt.Sprintf("%t", cfg.Dotfile.Translate) },
"dotfile.package_name_mapping": func() string {
return fmt.Sprintf("%t", cfg.Dotfile.PackageNameMapping)
},
"output.format": func() string { return cfg.Output.Format },
"output.color": func() string { return cfg.Output.Color },
"packages.sort_by": func() string { return cfg.Packages.SortBy },
}

if getter, ok := getters[key]; ok {
return getter(), nil
if getter, ok := configGetters[key]; ok {
return getter(cfg), nil
}
return "", fmt.Errorf("unknown config key: %s", key)
}
Expand Down
92 changes: 92 additions & 0 deletions cmd/dot/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -726,3 +726,95 @@ func TestRunConfigListCmd(t *testing.T) {
err = runConfigListCmd(cmd, []string{})
assert.NoError(t, err)
}

func TestConfigCommand_EveryCompletedKeyRoundTrips(t *testing.T) {
// Shell completion offers getValidConfigKeys() for BOTH get and set, so
// every completed key must survive set -> load -> get (issue #94: the
// getter and setter hand-lists had drifted, so freshly set keys read
// back as unknown). Driving the loop from getValidConfigKeys keeps the
// invariant self-maintaining: a new getter without a matching setter
// (or sample here) fails this test instead of failing users.
samples := map[string]string{
"directories.package": "/set/pkgs",
"directories.target": "/set/target",
"directories.manifest": "/set/manifest",
"logging.level": "DEBUG",
"logging.format": "json",
"logging.destination": "stdout",
"logging.file": "/set/dot.log",
"symlinks.mode": "absolute",
"symlinks.folding": "false",
"symlinks.overwrite": "true",
"symlinks.backup": "true",
"symlinks.backup_suffix": ".orig",
"symlinks.backup_dir": "/set/backups",
"ignore.use_defaults": "false",
"ignore.patterns": "*.log,cache",
"ignore.overrides": "keep,also",
"dotfile.translate": "false",
"dotfile.prefix": "custom-",
"dotfile.package_name_mapping": "false",
"output.format": "json",
"output.color": "always",
"output.progress": "false",
"output.verbosity": "2",
"output.width": "120",
"operations.dry_run": "true",
"operations.atomic": "false",
"operations.max_parallel": "4",
"packages.sort_by": "links",
"packages.auto_discover": "false",
"packages.validate_names": "false",
"doctor.auto_fix": "true",
"doctor.check_manifest": "false",
"doctor.check_broken_links": "false",
"doctor.check_orphaned": "false",
"doctor.check_permissions": "false",
"experimental.parallel": "true",
"experimental.profiling": "true",
}

for _, key := range getValidConfigKeys() {
t.Run(key, func(t *testing.T) {
sample, ok := samples[key]
require.True(t, ok, "key %s is offered for completion but has no sample here; add one and make sure config set accepts it", key)

configPath := filepath.Join(t.TempDir(), "config.yaml")
writer := config.NewWriter(configPath)
require.NoError(t, writer.WriteDefault(config.WriteOptions{Format: "yaml"}))

require.NoError(t, writer.Update(key, sample), "config set rejects a key it offers for completion")

loaded, err := config.LoadExtendedFromFile(configPath)
require.NoError(t, err)

got, err := getConfigValue(loaded, key)
require.NoError(t, err, "config get rejects a key it offers for completion")
assert.Equal(t, sample, got)
})
}
}

func TestConfigCommand_GetRendersTypedValues(t *testing.T) {
cfg := config.DefaultExtended()
cfg.Symlinks.Backup = true
cfg.Output.Verbosity = 2
cfg.Ignore.Patterns = []string{"*.log", "cache"}

tests := []struct {
key string
expected string
}{
{"symlinks.backup", "true"},
{"output.verbosity", "2"},
{"ignore.patterns", "*.log,cache"},
}

for _, tt := range tests {
t.Run(tt.key, func(t *testing.T) {
value, err := getConfigValue(cfg, tt.key)
require.NoError(t, err)
assert.Equal(t, tt.expected, value)
})
}
}
Loading
Loading