diff --git a/docs/user/04-configuration.md b/docs/user/04-configuration.md index 1af47584..04c7a3f6 100644 --- a/docs/user/04-configuration.md +++ b/docs/user/04-configuration.md @@ -504,27 +504,14 @@ Environment variables use the prefix `DOT_` followed by the section-qualified ke by single underscores, uppercased. For example `symlinks.mode` becomes `DOT_SYMLINKS_MODE`. There is no double-underscore convention. -Only the following keys are bound: - -``` -DOT_DIRECTORIES_PACKAGE DOT_DIRECTORIES_TARGET DOT_DIRECTORIES_MANIFEST -DOT_LOGGING_LEVEL DOT_LOGGING_FORMAT DOT_LOGGING_DESTINATION DOT_LOGGING_FILE -DOT_SYMLINKS_MODE DOT_SYMLINKS_FOLDING DOT_SYMLINKS_OVERWRITE -DOT_SYMLINKS_BACKUP DOT_SYMLINKS_BACKUP_SUFFIX -DOT_IGNORE_USE_DEFAULTS DOT_IGNORE_PATTERNS DOT_IGNORE_OVERRIDES -DOT_IGNORE_PER_PACKAGE_IGNORE DOT_IGNORE_MAX_FILE_SIZE DOT_IGNORE_INTERACTIVE_LARGE_FILES -DOT_DOTFILE_TRANSLATE DOT_DOTFILE_PREFIX -DOT_OUTPUT_FORMAT DOT_OUTPUT_COLOR DOT_OUTPUT_PROGRESS -DOT_OUTPUT_VERBOSITY DOT_OUTPUT_WIDTH -DOT_OPERATIONS_DRY_RUN DOT_OPERATIONS_ATOMIC DOT_OPERATIONS_MAX_PARALLEL -DOT_PACKAGES_SORT_BY DOT_PACKAGES_AUTO_DISCOVER DOT_PACKAGES_VALIDATE_NAMES -DOT_DOCTOR_AUTO_FIX DOT_DOCTOR_CHECK_MANIFEST DOT_DOCTOR_CHECK_BROKEN_LINKS -DOT_DOCTOR_CHECK_ORPHANED DOT_DOCTOR_CHECK_PERMISSIONS -DOT_EXPERIMENTAL_PARALLEL DOT_EXPERIMENTAL_PROFILING -``` - -`symlinks.backup_dir`, `dotfile.package_name_mapping`, `output.table_style`, and every key under -`update` and `network` have no environment binding; set them in the configuration file. +Every configuration key in the tables above is bound: `DOT_` plus the uppercased key with `.` +replaced by `_`. `symlinks.backup_dir` becomes `DOT_SYMLINKS_BACKUP_DIR`, +`dotfile.package_name_mapping` becomes `DOT_DOTFILE_PACKAGE_NAME_MAPPING`, and the `update` and +`network` sections follow the same rule. An environment value overrides the configuration file even +when it sets a boolean to `false`. + +Path-typed values (`directories.*`, `symlinks.backup_dir`, `logging.file`) expand `~` and `$VAR` +references exactly as file values do. `DOT_CONFIG` is handled separately and overrides the configuration file path. diff --git a/docs/user/08-troubleshooting.md b/docs/user/08-troubleshooting.md index bb9c38f9..2366bcd6 100644 --- a/docs/user/08-troubleshooting.md +++ b/docs/user/08-troubleshooting.md @@ -310,9 +310,10 @@ dot manage dot-vim # Uses config file dot -v manage dot-vim # -v overrides config ``` -Not every key is bound to an environment variable. `symlinks.backup_dir`, -`dotfile.package_name_mapping`, `output.table_style`, and the whole `update` -and `network` sections have no environment binding. +Every configuration key is bound to an environment variable: `DOT_` plus the +uppercased key with dots replaced by underscores, including `symlinks.backup_dir`, +`dotfile.package_name_mapping`, `output.table_style`, and the `update` and +`network` sections. ### Invalid Configuration diff --git a/internal/config/expand.go b/internal/config/expand.go index 2c00a3a2..9b86bd5d 100644 --- a/internal/config/expand.go +++ b/internal/config/expand.go @@ -35,16 +35,7 @@ import ( // On error the configuration is left as written, and the message names both // the offending key and the reference that could not be resolved. func (c *ExtendedConfig) ExpandPaths() error { - fields := []struct { - key string - value *string - }{ - {"directories.package", &c.Directories.Package}, - {"directories.target", &c.Directories.Target}, - {"directories.manifest", &c.Directories.Manifest}, - {"symlinks.backup_dir", &c.Symlinks.BackupDir}, - {"logging.file", &c.Logging.File}, - } + fields := c.pathFields() expanded := make([]string, len(fields)) for i, field := range fields { @@ -62,6 +53,25 @@ func (c *ExtendedConfig) ExpandPaths() error { return nil } +// pathField pairs a path-typed configuration key with its value slot. +type pathField struct { + key string + value *string +} + +// pathFields returns every path-typed configuration value in a stable order. +// It is the single list both ExpandPaths and the environment override path +// consume, so a new path-typed field is expanded everywhere or nowhere. +func (c *ExtendedConfig) pathFields() []pathField { + return []pathField{ + {"directories.package", &c.Directories.Package}, + {"directories.target", &c.Directories.Target}, + {"directories.manifest", &c.Directories.Manifest}, + {"symlinks.backup_dir", &c.Symlinks.BackupDir}, + {"logging.file", &c.Logging.File}, + } +} + // expandPath expands environment variables and a leading tilde in a single // path value. An empty value stays empty, so callers can distinguish "unset" // from "set to the home directory". diff --git a/internal/config/loader.go b/internal/config/loader.go index 9aca242e..ce3eca8f 100644 --- a/internal/config/loader.go +++ b/internal/config/loader.go @@ -2,6 +2,7 @@ package config import ( "fmt" + "reflect" "strings" "github.com/spf13/viper" @@ -54,19 +55,14 @@ func (l *Loader) LoadWithEnv() (*ExtendedConfig, error) { return nil, err } - // Load from environment (sparse config with only env-set values) - envCfg := l.loadFromEnv() - - // Environment values carry the same "~" and "$VAR" forms a config file may - // use. Expand the overlay before merging, so file values that were already - // expanded on load are not run through expansion a second time. - if err := envCfg.ExpandPaths(); err != nil { - return nil, fmt.Errorf("expand environment configuration: %w", err) + // Apply environment overrides directly onto the loaded configuration, + // keyed on viper's IsSet. A sparse overlay merged by zero-value cannot + // represent "explicitly set to false", which silently discarded env + // overrides like DOT_DOTFILE_PACKAGE_NAME_MAPPING=false (issue #86). + if err := applyEnvOverrides(l.newEnvViper(), cfg); err != nil { + return nil, fmt.Errorf("apply environment configuration: %w", err) } - // Use simple merge for env (only strings, no booleans unless tracked) - cfg = mergeConfigs(cfg, envCfg) - // Validate merged configuration if err := cfg.Validate(); err != nil { return nil, fmt.Errorf("invalid configuration: %w", err) @@ -104,230 +100,166 @@ func (l *Loader) LoadWithFlags(flags map[string]interface{}) (*ExtendedConfig, e return cfg, nil } -// loadFromEnv loads configuration from environment variables. -// Returns a sparse config with only explicitly set environment values. -func (l *Loader) loadFromEnv() *ExtendedConfig { +// newEnvViper returns a viper instance with every configuration key bound +// to its DOT_-prefixed environment variable. +func (l *Loader) newEnvViper() *viper.Viper { v := viper.New() - - // Set up environment variable handling v.SetEnvPrefix(strings.ToUpper(l.appName)) v.SetEnvKeyReplacer(strings.NewReplacer(".", "_")) v.AutomaticEnv() - // Bind all configuration keys - l.bindEnvKeys(v) - - // Create sparse config - cfg := createSparseConfig() - - // Load each section - loadDirectoriesFromEnv(v, &cfg.Directories) - loadLoggingFromEnv(v, &cfg.Logging) - loadSymlinksFromEnv(v, &cfg.Symlinks) - loadIgnoreFromEnv(v, &cfg.Ignore) - loadDotfileFromEnv(v, &cfg.Dotfile) - loadOutputFromEnv(v, &cfg.Output) - loadOperationsFromEnv(v, &cfg.Operations) - loadPackagesFromEnv(v, &cfg.Packages) - loadDoctorFromEnv(v, &cfg.Doctor) - loadExperimentalFromEnv(v, &cfg.Experimental) - - return cfg -} - -func loadDirectoriesFromEnv(v *viper.Viper, cfg *DirectoriesConfig) { - if v.IsSet("directories.package") { - cfg.Package = v.GetString("directories.package") - } - if v.IsSet("directories.target") { - cfg.Target = v.GetString("directories.target") + // Bind every key, derived from the config struct's mapstructure tags so + // a new field cannot be forgotten here (issue #86: a hand-written bind + // list silently ignored the keys it missed). + for _, key := range configKeys() { + _ = v.BindEnv(key) // BindEnv only errors when called with no key } - if v.IsSet("directories.manifest") { - cfg.Manifest = v.GetString("directories.manifest") - } -} -func loadLoggingFromEnv(v *viper.Viper, cfg *LoggingConfig) { - if v.IsSet("logging.level") { - cfg.Level = v.GetString("logging.level") - } - if v.IsSet("logging.format") { - cfg.Format = v.GetString("logging.format") - } - if v.IsSet("logging.destination") { - cfg.Destination = v.GetString("logging.destination") - } - if v.IsSet("logging.file") { - cfg.File = v.GetString("logging.file") - } + return v } -func loadSymlinksFromEnv(v *viper.Viper, cfg *SymlinksConfig) { - if v.IsSet("symlinks.mode") { - cfg.Mode = v.GetString("symlinks.mode") - } - if v.IsSet("symlinks.folding") { - cfg.Folding = v.GetBool("symlinks.folding") - } - if v.IsSet("symlinks.overwrite") { - cfg.Overwrite = v.GetBool("symlinks.overwrite") - } - if v.IsSet("symlinks.backup") { - cfg.Backup = v.GetBool("symlinks.backup") - } - if v.IsSet("symlinks.backup_suffix") { - cfg.BackupSuffix = v.GetString("symlinks.backup_suffix") - } -} - -func loadIgnoreFromEnv(v *viper.Viper, cfg *IgnoreConfig) { - if v.IsSet("ignore.use_defaults") { - cfg.UseDefaults = v.GetBool("ignore.use_defaults") - } - if v.IsSet("ignore.patterns") { - cfg.Patterns = v.GetStringSlice("ignore.patterns") - } - if v.IsSet("ignore.overrides") { - cfg.Overrides = v.GetStringSlice("ignore.overrides") - } - if v.IsSet("ignore.per_package_ignore") { - cfg.PerPackageIgnore = v.GetBool("ignore.per_package_ignore") - } - if v.IsSet("ignore.max_file_size") { - cfg.MaxFileSize = v.GetInt64("ignore.max_file_size") - } - if v.IsSet("ignore.interactive_large_files") { - cfg.InteractiveLargeFiles = v.GetBool("ignore.interactive_large_files") - } -} - -func loadDotfileFromEnv(v *viper.Viper, cfg *DotfileConfig) { - if v.IsSet("dotfile.translate") { - cfg.Translate = v.GetBool("dotfile.translate") - } - if v.IsSet("dotfile.prefix") { - cfg.Prefix = v.GetString("dotfile.prefix") - } -} - -func loadOutputFromEnv(v *viper.Viper, cfg *OutputConfig) { - if v.IsSet("output.format") { - cfg.Format = v.GetString("output.format") - } - if v.IsSet("output.color") { - cfg.Color = v.GetString("output.color") - } - if v.IsSet("output.progress") { - cfg.Progress = v.GetBool("output.progress") - } - if v.IsSet("output.verbosity") { - cfg.Verbosity = v.GetInt("output.verbosity") - } - if v.IsSet("output.width") { - cfg.Width = v.GetInt("output.width") - } -} - -func loadOperationsFromEnv(v *viper.Viper, cfg *OperationsConfig) { - if v.IsSet("operations.dry_run") { - cfg.DryRun = v.GetBool("operations.dry_run") - } - if v.IsSet("operations.atomic") { - cfg.Atomic = v.GetBool("operations.atomic") - } - if v.IsSet("operations.max_parallel") { - cfg.MaxParallel = v.GetInt("operations.max_parallel") - } -} - -func loadPackagesFromEnv(v *viper.Viper, cfg *PackagesConfig) { - if v.IsSet("packages.sort_by") { - cfg.SortBy = v.GetString("packages.sort_by") - } - if v.IsSet("packages.auto_discover") { - cfg.AutoDiscover = v.GetBool("packages.auto_discover") +// configKeys returns every section.field key in ExtendedConfig, derived by +// reflection from the mapstructure tags. Only tagged struct sections with +// tagged fields produce keys, so the walk tolerates the struct growing a +// scalar or untagged field without panicking on every command start. +func configKeys() []string { + var keys []string + root := reflect.TypeOf(ExtendedConfig{}) + for i := 0; i < root.NumField(); i++ { + sectionType := root.Field(i).Type + if sectionType.Kind() != reflect.Struct { + continue + } + section := root.Field(i).Tag.Get("mapstructure") + if section == "" { + continue + } + for j := 0; j < sectionType.NumField(); j++ { + field := sectionType.Field(j).Tag.Get("mapstructure") + if field == "" { + continue + } + keys = append(keys, section+"."+field) + } } - if v.IsSet("packages.validate_names") { - cfg.ValidateNames = v.GetBool("packages.validate_names") + return keys +} + +// applyEnvOverrides applies every environment-set key directly onto cfg. +// Path-typed values are expanded here, mirroring the expansion file values +// receive on load. The per-section lists below are kept total by +// TestLoader_EnvOverridesEveryConfigKey, which walks the config struct. +func applyEnvOverrides(v *viper.Viper, cfg *ExtendedConfig) error { + if err := applyPathEnv(v, cfg); err != nil { + return err + } + + applyEnv(v, v.GetString, map[string]*string{ + "logging.level": &cfg.Logging.Level, + "logging.format": &cfg.Logging.Format, + "logging.destination": &cfg.Logging.Destination, + "symlinks.mode": &cfg.Symlinks.Mode, + "symlinks.backup_suffix": &cfg.Symlinks.BackupSuffix, + "dotfile.prefix": &cfg.Dotfile.Prefix, + "output.format": &cfg.Output.Format, + "output.color": &cfg.Output.Color, + "output.table_style": &cfg.Output.TableStyle, + "packages.sort_by": &cfg.Packages.SortBy, + "update.package_manager": &cfg.Update.PackageManager, + "update.repository": &cfg.Update.Repository, + "network.http_proxy": &cfg.Network.HTTPProxy, + "network.https_proxy": &cfg.Network.HTTPSProxy, + "network.no_proxy": &cfg.Network.NoProxy, + }) + + applyEnv(v, v.GetBool, map[string]*bool{ + "symlinks.folding": &cfg.Symlinks.Folding, + "symlinks.overwrite": &cfg.Symlinks.Overwrite, + "symlinks.backup": &cfg.Symlinks.Backup, + "ignore.use_defaults": &cfg.Ignore.UseDefaults, + "ignore.per_package_ignore": &cfg.Ignore.PerPackageIgnore, + "ignore.interactive_large_files": &cfg.Ignore.InteractiveLargeFiles, + "dotfile.translate": &cfg.Dotfile.Translate, + "dotfile.package_name_mapping": &cfg.Dotfile.PackageNameMapping, + "output.progress": &cfg.Output.Progress, + "operations.dry_run": &cfg.Operations.DryRun, + "operations.atomic": &cfg.Operations.Atomic, + "packages.auto_discover": &cfg.Packages.AutoDiscover, + "packages.validate_names": &cfg.Packages.ValidateNames, + "doctor.auto_fix": &cfg.Doctor.AutoFix, + "doctor.check_manifest": &cfg.Doctor.CheckManifest, + "doctor.check_broken_links": &cfg.Doctor.CheckBrokenLinks, + "doctor.check_orphaned": &cfg.Doctor.CheckOrphaned, + "doctor.check_permissions": &cfg.Doctor.CheckPermissions, + "update.check_on_startup": &cfg.Update.CheckOnStartup, + "update.include_prerelease": &cfg.Update.IncludePrerelease, + "experimental.parallel": &cfg.Experimental.Parallel, + "experimental.profiling": &cfg.Experimental.Profiling, + }) + + applyEnv(v, v.GetInt, map[string]*int{ + "output.verbosity": &cfg.Output.Verbosity, + "output.width": &cfg.Output.Width, + "operations.max_parallel": &cfg.Operations.MaxParallel, + "update.check_frequency": &cfg.Update.CheckFrequency, + "network.timeout": &cfg.Network.Timeout, + "network.connect_timeout": &cfg.Network.ConnectTimeout, + "network.tls_timeout": &cfg.Network.TLSTimeout, + }) + + applyEnv(v, v.GetInt64, map[string]*int64{ + "ignore.max_file_size": &cfg.Ignore.MaxFileSize, + }) + + // Lists are comma-separated in the environment, matching the documented + // contract and the comma convention config set already uses. Viper's own + // GetStringSlice would whitespace-split instead. + applyEnv(v, func(key string) []string { return splitEnvList(v.GetString(key)) }, map[string]*[]string{ + "ignore.patterns": &cfg.Ignore.Patterns, + "ignore.overrides": &cfg.Ignore.Overrides, + }) + + return nil +} + +// applyPathEnv sets path-typed values from the environment, expanding "~" +// and "$VAR" references the way file values are expanded on load. It walks +// pathFields in order, so with several bad values the reported key is +// deterministic. +func applyPathEnv(v *viper.Viper, cfg *ExtendedConfig) error { + for _, field := range cfg.pathFields() { + if !v.IsSet(field.key) { + continue + } + expanded, err := expandPath(v.GetString(field.key)) + if err != nil { + return fmt.Errorf("%s: %w", field.key, err) + } + *field.value = expanded } + return nil } -func loadDoctorFromEnv(v *viper.Viper, cfg *DoctorConfig) { - if v.IsSet("doctor.auto_fix") { - cfg.AutoFix = v.GetBool("doctor.auto_fix") - } - if v.IsSet("doctor.check_manifest") { - cfg.CheckManifest = v.GetBool("doctor.check_manifest") - } - if v.IsSet("doctor.check_broken_links") { - cfg.CheckBrokenLinks = v.GetBool("doctor.check_broken_links") - } - if v.IsSet("doctor.check_orphaned") { - cfg.CheckOrphaned = v.GetBool("doctor.check_orphaned") - } - if v.IsSet("doctor.check_permissions") { - cfg.CheckPermissions = v.GetBool("doctor.check_permissions") +// applyEnv copies every environment-set key in targets onto its destination. +func applyEnv[T any](v *viper.Viper, get func(string) T, targets map[string]*T) { + for key, dst := range targets { + if v.IsSet(key) { + *dst = get(key) + } } } -func loadExperimentalFromEnv(v *viper.Viper, cfg *ExperimentalConfig) { - if v.IsSet("experimental.parallel") { - cfg.Parallel = v.GetBool("experimental.parallel") - } - if v.IsSet("experimental.profiling") { - cfg.Profiling = v.GetBool("experimental.profiling") +// splitEnvList splits a comma-separated environment value into its items, +// trimming surrounding whitespace and dropping empty entries. +func splitEnvList(value string) []string { + parts := strings.Split(value, ",") + items := make([]string, 0, len(parts)) + for _, part := range parts { + if trimmed := strings.TrimSpace(part); trimmed != "" { + items = append(items, trimmed) + } } -} - -// bindEnvKeys binds all configuration keys to environment variables. -func (l *Loader) bindEnvKeys(v *viper.Viper) { - v.BindEnv("directories.package") - v.BindEnv("directories.target") - v.BindEnv("directories.manifest") - - v.BindEnv("logging.level") - v.BindEnv("logging.format") - v.BindEnv("logging.destination") - v.BindEnv("logging.file") - - v.BindEnv("symlinks.mode") - v.BindEnv("symlinks.folding") - v.BindEnv("symlinks.overwrite") - v.BindEnv("symlinks.backup") - v.BindEnv("symlinks.backup_suffix") - - v.BindEnv("ignore.use_defaults") - v.BindEnv("ignore.patterns") - v.BindEnv("ignore.overrides") - v.BindEnv("ignore.per_package_ignore") - v.BindEnv("ignore.max_file_size") - v.BindEnv("ignore.interactive_large_files") - - v.BindEnv("dotfile.translate") - v.BindEnv("dotfile.prefix") - - v.BindEnv("output.format") - v.BindEnv("output.color") - v.BindEnv("output.progress") - v.BindEnv("output.verbosity") - v.BindEnv("output.width") - - v.BindEnv("operations.dry_run") - v.BindEnv("operations.atomic") - v.BindEnv("operations.max_parallel") - - v.BindEnv("packages.sort_by") - v.BindEnv("packages.auto_discover") - v.BindEnv("packages.validate_names") - - v.BindEnv("doctor.auto_fix") - v.BindEnv("doctor.check_manifest") - v.BindEnv("doctor.check_broken_links") - v.BindEnv("doctor.check_orphaned") - v.BindEnv("doctor.check_permissions") - - v.BindEnv("experimental.parallel") - v.BindEnv("experimental.profiling") + return items } // configFromFlags creates partial config from flag map. @@ -339,20 +271,10 @@ func (l *Loader) configFromFlags(flags map[string]interface{}) (*ExtendedConfig, return cfg, verbositySet } -// createSparseConfig creates an empty config for flag/env merging. +// createSparseConfig creates an empty config for flag merging. Verbosity -1 +// is the "not set" sentinel. func createSparseConfig() *ExtendedConfig { - return &ExtendedConfig{ - Directories: DirectoriesConfig{}, - Logging: LoggingConfig{}, - Symlinks: SymlinksConfig{}, - Ignore: IgnoreConfig{}, - Dotfile: DotfileConfig{}, - Output: OutputConfig{Verbosity: -1}, // Use -1 as sentinel for "not set" - Operations: OperationsConfig{}, - Packages: PackagesConfig{}, - Doctor: DoctorConfig{}, - Experimental: ExperimentalConfig{}, - } + return &ExtendedConfig{Output: OutputConfig{Verbosity: -1}} } // applyFlagsToConfig maps command-line flags to configuration fields. @@ -413,12 +335,6 @@ func applyOutputFlags(cfg *ExtendedConfig, flags map[string]interface{}) bool { return verbositySet } -// mergeConfigs merges two configs, with override taking precedence for non-zero values. -// Only merges fields that are explicitly set in override (non-empty strings, non-zero lists). -func mergeConfigs(base, override *ExtendedConfig) *ExtendedConfig { - return mergeConfigsWithVerbosity(base, override, false) -} - // mergeConfigsWithVerbosity merges configs with special handling for verbosity. func mergeConfigsWithVerbosity(base, override *ExtendedConfig, verbosityExplicit bool) *ExtendedConfig { merged := *base diff --git a/internal/config/loader_test.go b/internal/config/loader_test.go index 7ce745b9..56f7ad3e 100644 --- a/internal/config/loader_test.go +++ b/internal/config/loader_test.go @@ -3,6 +3,9 @@ package config_test import ( "os" "path/filepath" + "reflect" + "strconv" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -357,3 +360,180 @@ package = "/test/dotfiles" }) } } + +// envSampleValues holds one valid, non-default sample per configuration key, +// in the string form an operator would export. TestLoader_EnvOverridesEveryConfigKey +// walks the ExtendedConfig struct, so a new key without a sample here fails +// loudly instead of shipping as a silently ignored env var (issue #86). +var envSampleValues = map[string]string{ + "directories.package": "/env/pkgs", + "directories.target": "/env/target", + "directories.manifest": "/env/manifest", + "logging.level": "DEBUG", + "logging.format": "json", + "logging.destination": "stdout", + "logging.file": "/env/dot.log", + "symlinks.mode": "absolute", + "symlinks.folding": "false", + "symlinks.overwrite": "true", + "symlinks.backup": "true", + "symlinks.backup_suffix": ".orig", + "symlinks.backup_dir": "/env/backups", + "ignore.use_defaults": "false", + "ignore.patterns": "*.log", + "ignore.overrides": "keep.me", + "ignore.per_package_ignore": "false", + "ignore.max_file_size": "1024", + "ignore.interactive_large_files": "false", + "dotfile.translate": "false", + "dotfile.prefix": "custom-", + "dotfile.package_name_mapping": "false", + "output.format": "json", + "output.color": "always", + "output.table_style": "simple", + "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", + "update.check_on_startup": "false", + "update.check_frequency": "48", + "update.package_manager": "brew", + "update.repository": "acme/dot", + "update.include_prerelease": "true", + "network.http_proxy": "http://proxy.env:3128", + "network.https_proxy": "http://proxy.env:3129", + "network.no_proxy": "localhost", + "network.timeout": "30", + "network.connect_timeout": "9", + "network.tls_timeout": "7", + "experimental.parallel": "true", + "experimental.profiling": "true", +} + +// configLeafKeys walks the two-level ExtendedConfig shape and returns every +// section.field key together with the reflect.Value accessor for a loaded +// config, so the test cannot drift from the struct. +func configLeafKeys(t *testing.T, cfg *config.ExtendedConfig) map[string]reflect.Value { + t.Helper() + + keys := make(map[string]reflect.Value) + root := reflect.ValueOf(cfg).Elem() + rootType := root.Type() + + for i := 0; i < rootType.NumField(); i++ { + section := rootType.Field(i).Tag.Get("mapstructure") + require.NotEmpty(t, section, "field %s has no mapstructure tag", rootType.Field(i).Name) + + sectionValue := root.Field(i) + sectionType := sectionValue.Type() + for j := 0; j < sectionType.NumField(); j++ { + field := sectionType.Field(j).Tag.Get("mapstructure") + require.NotEmpty(t, field, "field %s.%s has no mapstructure tag", section, sectionType.Field(j).Name) + keys[section+"."+field] = sectionValue.Field(j) + } + } + + return keys +} + +func TestLoader_EnvOverridesEveryConfigKey(t *testing.T) { + // Issue #86: DOT_ env overrides were driven by hand-lists, so keys + // outside them (dotfile.package_name_mapping among others) were + // silently ignored. Every key in the config struct must be settable + // from the environment. + for key := range configLeafKeys(t, config.DefaultExtended()) { + t.Run(key, func(t *testing.T) { + sample, ok := envSampleValues[key] + require.True(t, ok, "config key %s has no env sample; add one to envSampleValues", key) + + envVar := "DOT_" + strings.ToUpper(strings.ReplaceAll(key, ".", "_")) + t.Setenv(envVar, sample) + + loader := config.NewLoader("dot", filepath.Join(t.TempDir(), "missing.yaml")) + cfg, err := loader.LoadWithEnv() + require.NoError(t, err) + + got := configLeafKeys(t, cfg)[key] + + var want interface{} + switch got.Kind() { + case reflect.Bool: + b, perr := strconv.ParseBool(sample) + require.NoError(t, perr) + want = b + case reflect.Int: + n, perr := strconv.Atoi(sample) + require.NoError(t, perr) + want = n + case reflect.Int64: + n, perr := strconv.ParseInt(sample, 10, 64) + require.NoError(t, perr) + want = n + case reflect.String: + want = sample + case reflect.Slice: + want = []string{sample} + default: + t.Fatalf("unhandled kind %s for %s", got.Kind(), key) + } + + // A sample equal to the default would make this subtest pass + // even if the override were silently dropped. + def := configLeafKeys(t, config.DefaultExtended())[key] + require.NotEqual(t, def.Interface(), want, + "sample for %s equals the default; pick a distinct value in envSampleValues", key) + + assert.Equal(t, want, got.Interface(), "%s not overridden by %s", key, envVar) + }) + } +} + +func TestLoader_EnvListValuesAreCommaSeparated(t *testing.T) { + // The documented contract (and the config set convention) is + // comma-separated lists; viper's own env handling would split on + // whitespace instead, turning "*.log,*.tmp" into one bogus pattern. + t.Setenv("DOT_IGNORE_PATTERNS", "*.log, *.tmp,cache") + + loader := config.NewLoader("dot", filepath.Join(t.TempDir(), "missing.yaml")) + cfg, err := loader.LoadWithEnv() + require.NoError(t, err) + + assert.Equal(t, []string{"*.log", "*.tmp", "cache"}, cfg.Ignore.Patterns) +} + +func TestLoader_EnvFalseOverridesFileTrue(t *testing.T) { + // The old sparse-config merge could only propagate true booleans, so an + // env var set to false could never override a file value of true. That + // is the exact shape of the issue #86 report. + configPath := filepath.Join(t.TempDir(), "config.yaml") + content := ` +dotfile: + translate: true + package_name_mapping: true +symlinks: + folding: true +` + require.NoError(t, os.WriteFile(configPath, []byte(content), 0o600)) + + t.Setenv("DOT_DOTFILE_PACKAGE_NAME_MAPPING", "false") + t.Setenv("DOT_DOTFILE_TRANSLATE", "false") + t.Setenv("DOT_SYMLINKS_FOLDING", "false") + + loader := config.NewLoader("dot", configPath) + cfg, err := loader.LoadWithEnv() + require.NoError(t, err) + + assert.False(t, cfg.Dotfile.PackageNameMapping, "env false must override file true") + assert.False(t, cfg.Dotfile.Translate, "env false must override file true") + assert.False(t, cfg.Symlinks.Folding, "env false must override file true") +}