From 53a9db984e92a96e0ad8005434c68162446e2ed5 Mon Sep 17 00:00:00 2001 From: Raúl Benencia Date: Fri, 5 Jun 2026 16:21:18 -0300 Subject: Preserve empty flag values --- internal/environment/flags.go | 37 +++++++++++---------- internal/environment/flags_test.go | 67 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+), 17 deletions(-) diff --git a/internal/environment/flags.go b/internal/environment/flags.go index 2ffda31..dcb03fc 100644 --- a/internal/environment/flags.go +++ b/internal/environment/flags.go @@ -26,9 +26,9 @@ import ( func (env *Environment) setFlags(args []string, environ []string) (*flag.FlagSet, error) { env.setFlagDefaults() - configFile := configFileFromArgs(args) - if configFile == "" { - configFile = envValue(environ, "CONFIG") + configFile, configFromArgs := configFileFromArgs(args) + if !configFromArgs { + configFile, _ = envValue(environ, "CONFIG") } if configFile != "" { env.ConfigFile = configFile @@ -70,22 +70,22 @@ func (env *Environment) registerFlags() *flag.FlagSet { return flags } -func configFileFromArgs(args []string) string { +func configFileFromArgs(args []string) (string, bool) { for i, arg := range args { if arg == "-config" || arg == "--config" { if i+1 < len(args) { - return args[i+1] + return args[i+1], true } - return "" + return "", true } if strings.HasPrefix(arg, "-config=") { - return strings.TrimPrefix(arg, "-config=") + return strings.TrimPrefix(arg, "-config="), true } if strings.HasPrefix(arg, "--config=") { - return strings.TrimPrefix(arg, "--config=") + return strings.TrimPrefix(arg, "--config="), true } } - return "" + return "", false } func (env *Environment) loadConfigFile(configFile string) error { @@ -155,24 +155,27 @@ func (env *Environment) applyEnvVars(environ []string) error { } func (env *Environment) applyEnvVar(environ []string, key, name string) error { - return env.setConfigValue(key, envValue(environ, name)) + value, ok := envValue(environ, name) + if !ok { + return nil + } + if key == "debug" && value == "" { + value = "true" + } + return env.setConfigValue(key, value) } -func envValue(environ []string, name string) string { +func envValue(environ []string, name string) (string, bool) { for _, entry := range environ { key, value, found := strings.Cut(entry, "=") if found && key == name { - return value + return value, true } } - return "" + return "", false } func (env *Environment) setConfigValue(key, value string) error { - if value == "" { - return nil - } - switch key { case "config": env.ConfigFile = value diff --git a/internal/environment/flags_test.go b/internal/environment/flags_test.go index 2c2750d..782a336 100644 --- a/internal/environment/flags_test.go +++ b/internal/environment/flags_test.go @@ -101,6 +101,72 @@ func TestSetFlagsCLIConfigOverridesEnvConfig(t *testing.T) { } } +func TestSetFlagsEmptyEnvOverridesConfigValue(t *testing.T) { + configFile := writeConfig(t, "shoelaces.conf", "base-url=config-base-url\n") + + env := defaultEnvironment() + if _, err := env.setFlags(nil, []string{"CONFIG=" + configFile, "BASE_URL="}); err != nil { + t.Fatal(err) + } + + if env.BaseURL != "" { + t.Errorf("Expected empty base URL from env, got %q", env.BaseURL) + } +} + +func TestSetFlagsEmptyEnvOverridesDefaultValue(t *testing.T) { + env := defaultEnvironment() + if _, err := env.setFlags(nil, []string{"ENV_DIR="}); err != nil { + t.Fatal(err) + } + + if env.EnvDir != "" { + t.Errorf("Expected empty env dir from env, got %q", env.EnvDir) + } +} + +func TestSetFlagsEmptyConfigValueOverridesDefault(t *testing.T) { + configFile := writeConfig(t, "shoelaces.conf", "env-dir=\n") + + env := defaultEnvironment() + if _, err := env.setFlags([]string{"-config", configFile}, nil); err != nil { + t.Fatal(err) + } + + if env.EnvDir != "" { + t.Errorf("Expected empty env dir from config, got %q", env.EnvDir) + } +} + +func TestSetFlagsEmptyCLIConfigSkipsEnvConfig(t *testing.T) { + envConfig := writeConfig(t, "env.conf", "base-url=env-config-base-url\n") + + env := defaultEnvironment() + if _, err := env.setFlags([]string{"-config="}, []string{"CONFIG=" + envConfig}); err != nil { + t.Fatal(err) + } + + if env.BaseURL != "" { + t.Errorf("Expected env config file to be skipped, got base URL %q", env.BaseURL) + } + if env.ConfigFile != "" { + t.Errorf("Expected empty config file from CLI, got %q", env.ConfigFile) + } +} + +func TestSetFlagsEmptyBoolEnvMeansTrue(t *testing.T) { + configFile := writeConfig(t, "shoelaces.conf", "debug=false\n") + + env := defaultEnvironment() + if _, err := env.setFlags(nil, []string{"CONFIG=" + configFile, "DEBUG="}); err != nil { + t.Fatal(err) + } + + if !env.Debug { + t.Error("Expected empty DEBUG env var to set debug true") + } +} + func TestSetFlagsReturnsConfigErrors(t *testing.T) { tests := []struct { name string @@ -108,6 +174,7 @@ func TestSetFlagsReturnsConfigErrors(t *testing.T) { }{ {name: "unknown key", config: "unknown=value\n"}, {name: "invalid bool", config: "debug=maybe\n"}, + {name: "empty bool", config: "debug=\n"}, } for _, tt := range tests { -- cgit v1.2.3