-
Notifications
You must be signed in to change notification settings - Fork 2
Simplify update notification flow #159
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,22 +1,29 @@ | ||
| package config | ||
|
|
||
| import ( | ||
| "bufio" | ||
| _ "embed" | ||
| "errors" | ||
| "fmt" | ||
| "os" | ||
| "path/filepath" | ||
| "strings" | ||
|
|
||
| "github.com/spf13/viper" | ||
| ) | ||
|
|
||
| //go:embed default_config.toml | ||
| var defaultConfigTemplate string | ||
|
|
||
| type CLIConfig struct { | ||
| UpdatePrompt bool `mapstructure:"update_prompt"` | ||
| UpdateSkippedVersion string `mapstructure:"update_skipped_version"` | ||
| } | ||
|
|
||
| type Config struct { | ||
| Containers []ContainerConfig `mapstructure:"containers"` | ||
| Env map[string]map[string]string `mapstructure:"env"` | ||
| UpdatePrompt bool `mapstructure:"update_prompt"` | ||
| Containers []ContainerConfig `mapstructure:"containers"` | ||
| Env map[string]map[string]string `mapstructure:"env"` | ||
| CLI CLIConfig `mapstructure:"cli"` | ||
| } | ||
|
|
||
| func setDefaults() { | ||
|
|
@@ -27,7 +34,7 @@ func setDefaults() { | |
| "port": "4566", | ||
| }, | ||
| }) | ||
| viper.SetDefault("update_prompt", true) | ||
| viper.SetDefault("cli.update_prompt", true) | ||
| } | ||
|
|
||
| func loadConfig(path string) error { | ||
|
|
@@ -109,18 +116,119 @@ func resolvedConfigPath() string { | |
|
|
||
| func Set(key string, value any) error { | ||
| viper.Set(key, value) | ||
| return viper.WriteConfig() | ||
| return setInFile(viper.ConfigFileUsed(), key, value) | ||
| } | ||
|
|
||
| // setInFile updates a single key in the TOML config file without | ||
| // rewriting unrelated keys (avoids Viper dumping all defaults). | ||
| func setInFile(path, key string, value any) error { | ||
| // Split "cli.update_skipped_version" into section "cli" and field "update_skipped_version". | ||
| parts := strings.SplitN(key, ".", 2) | ||
| if len(parts) != 2 { | ||
| // Top-level keys: fall back to full rewrite. | ||
| return viper.WriteConfig() | ||
| } | ||
| section, field := parts[0], parts[1] | ||
|
|
||
| formatted := formatTOMLValue(value) | ||
| targetLine := field + " = " + formatted | ||
|
|
||
| data, err := os.ReadFile(path) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to read config file: %w", err) | ||
| } | ||
|
|
||
| var result []string | ||
| scanner := bufio.NewScanner(strings.NewReader(string(data))) | ||
| inSection := false | ||
| replaced := false | ||
| sectionHeader := "[" + section + "]" | ||
|
|
||
| for scanner.Scan() { | ||
| line := scanner.Text() | ||
| trimmed := strings.TrimSpace(line) | ||
|
|
||
| // Detect section headers. | ||
| if strings.HasPrefix(trimmed, "[") { | ||
| if trimmed == sectionHeader { | ||
| inSection = true | ||
| } else if inSection { | ||
| // Leaving our section without having replaced — insert before the new section. | ||
| if !replaced { | ||
| result = append(result, targetLine) | ||
| replaced = true | ||
| } | ||
| inSection = false | ||
| } | ||
| } | ||
|
|
||
| // Replace existing key in the target section. | ||
| if inSection && strings.HasPrefix(trimmed, field+" ") || inSection && strings.HasPrefix(trimmed, field+"=") { | ||
| result = append(result, targetLine) | ||
| replaced = true | ||
| continue | ||
| } | ||
|
|
||
| result = append(result, line) | ||
| } | ||
|
|
||
| // Section exists but key was not found — append to end of file (still in section). | ||
| if !replaced && inSection { | ||
| result = append(result, targetLine) | ||
| replaced = true | ||
| } | ||
|
|
||
| // Section doesn't exist at all — append section and key. | ||
| if !replaced { | ||
| if len(result) > 0 && strings.TrimSpace(result[len(result)-1]) != "" { | ||
| result = append(result, "") | ||
| } | ||
| result = append(result, sectionHeader) | ||
| result = append(result, targetLine) | ||
| } | ||
|
|
||
| output := strings.Join(result, "\n") | ||
| if !strings.HasSuffix(output, "\n") { | ||
| output += "\n" | ||
| } | ||
|
|
||
| return os.WriteFile(path, []byte(output), 0644) | ||
| } | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: can we use the |
||
|
|
||
| func formatTOMLValue(v any) string { | ||
| switch val := v.(type) { | ||
| case string: | ||
| return fmt.Sprintf("%q", val) | ||
| case bool: | ||
| if val { | ||
| return "true" | ||
| } | ||
| return "false" | ||
| default: | ||
| return fmt.Sprintf("%v", val) | ||
| } | ||
| } | ||
|
|
||
| func DisableUpdatePrompt() error { | ||
| return Set("update_prompt", false) | ||
| return Set("cli.update_prompt", false) | ||
| } | ||
|
|
||
| func SetUpdateSkippedVersion(version string) error { | ||
| return Set("cli.update_skipped_version", version) | ||
| } | ||
|
|
||
| func GetUpdateSkippedVersion() string { | ||
| return viper.GetString("cli.update_skipped_version") | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue: this is not used. We don't really need getters for config, since we access directly the values from the |
||
| } | ||
|
|
||
| func Get() (*Config, error) { | ||
| var cfg Config | ||
| if err := viper.Unmarshal(&cfg); err != nil { | ||
| return nil, fmt.Errorf("failed to unmarshal config: %w", err) | ||
| } | ||
| if !viper.InConfig("cli.update_prompt") && viper.InConfig("update_prompt") { | ||
| cfg.CLI.UpdatePrompt = viper.GetBool("update_prompt") | ||
| } | ||
| for i := range cfg.Containers { | ||
gtsiolis marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| if err := cfg.Containers[i].Validate(); err != nil { | ||
| return nil, fmt.Errorf("invalid container config: %w", err) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: as far as I understand, we only left this here for backward-compatibility. The new version will not let the users choose to never be prompted again for update from the UI. But we have this code here to make sure that users that have
update_promptset to true in their config, will indeed never be prompted again.My suggestion is: can we remove this code completely and ignore if they have that setting? That would clean up the code quite a bit. That would also allow getting rid of the migration logic lines 229-231.