-
Notifications
You must be signed in to change notification settings - Fork 110
feat(cli): Comprehensive erst.toml configuration parsing #646
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 | ||||
|---|---|---|---|---|---|---|
|
|
@@ -109,12 +109,16 @@ func (EnvParser) Parse(cfg *Config) error { | |||||
| } | ||||||
| if v := os.Getenv("ERST_SIMULATOR_PATH"); v != "" { | ||||||
| cfg.SimulatorPath = v | ||||||
| } else if v := os.Getenv("ERST_SIM_PATH"); v != "" { | ||||||
| cfg.SimulatorPath = v | ||||||
| } | ||||||
| if v := os.Getenv("ERST_LOG_LEVEL"); v != "" { | ||||||
| cfg.LogLevel = v | ||||||
| } | ||||||
| if v := os.Getenv("ERST_CACHE_PATH"); v != "" { | ||||||
| cfg.CachePath = v | ||||||
| } else if v := os.Getenv("ERST_CACHE_DIR"); v != "" { | ||||||
| cfg.CachePath = v | ||||||
| } | ||||||
| if v := os.Getenv("ERST_RPC_TOKEN"); v != "" { | ||||||
| cfg.RPCToken = v | ||||||
|
|
@@ -136,24 +140,18 @@ func (EnvParser) Parse(cfg *Config) error { | |||||
|
|
||||||
| switch strings.ToLower(os.Getenv("ERST_CRASH_REPORTING")) { | ||||||
| case "": | ||||||
| case "1", "true", "yes": | ||||||
| case "1", "true", "yes", "on": | ||||||
| cfg.CrashReporting = true | ||||||
| case "0", "false", "no": | ||||||
| case "0", "false", "no", "off": | ||||||
| cfg.CrashReporting = false | ||||||
| default: | ||||||
| return errors.WrapValidationError("ERST_CRASH_REPORTING must be a boolean") | ||||||
| } | ||||||
|
|
||||||
| if urlsEnv := os.Getenv("ERST_RPC_URLS"); urlsEnv != "" { | ||||||
| cfg.RpcUrls = strings.Split(urlsEnv, ",") | ||||||
| for i := range cfg.RpcUrls { | ||||||
| cfg.RpcUrls[i] = strings.TrimSpace(cfg.RpcUrls[i]) | ||||||
| } | ||||||
| cfg.RpcUrls = splitList(urlsEnv) | ||||||
| } else if urlsEnv := os.Getenv("STELLAR_RPC_URLS"); urlsEnv != "" { | ||||||
| cfg.RpcUrls = strings.Split(urlsEnv, ",") | ||||||
| for i := range cfg.RpcUrls { | ||||||
| cfg.RpcUrls[i] = strings.TrimSpace(cfg.RpcUrls[i]) | ||||||
| } | ||||||
| cfg.RpcUrls = splitList(urlsEnv) | ||||||
| } | ||||||
|
|
||||||
| return nil | ||||||
|
|
@@ -248,42 +246,14 @@ func LoadConfig() (*Config, error) { | |||||
| // Load loads the configuration from environment variables and TOML files. | ||||||
| // The lifecycle follows three distinct phases: load, merge defaults, validate. | ||||||
| func Load() (*Config, error) { | ||||||
| // Phase 1: Load from sources (env vars, then TOML file). | ||||||
| cfg := &Config{ | ||||||
| RpcUrl: getEnv("ERST_RPC_URL", ""), | ||||||
| Network: Network(getEnv("ERST_NETWORK", "")), | ||||||
| SimulatorPath: getEnv("ERST_SIMULATOR_PATH", ""), | ||||||
| LogLevel: getEnv("ERST_LOG_LEVEL", ""), | ||||||
| CachePath: getEnv("ERST_CACHE_PATH", ""), | ||||||
| RPCToken: getEnv("ERST_RPC_TOKEN", ""), | ||||||
| CrashEndpoint: getEnv("ERST_CRASH_ENDPOINT", ""), | ||||||
| CrashSentryDSN: getEnv("ERST_SENTRY_DSN", ""), | ||||||
| RequestTimeout: defaultRequestTimeout, | ||||||
| } | ||||||
|
|
||||||
| // ERST_REQUEST_TIMEOUT is an integer env var; parse it explicitly. | ||||||
| if v := os.Getenv("ERST_REQUEST_TIMEOUT"); v != "" { | ||||||
| if n, err := strconv.Atoi(v); err == nil && n > 0 { | ||||||
| cfg.RequestTimeout = n | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| switch strings.ToLower(os.Getenv("ERST_CRASH_REPORTING")) { | ||||||
| case "1", "true", "yes": | ||||||
| cfg.CrashReporting = true | ||||||
| } | ||||||
|
|
||||||
| if urlsEnv := os.Getenv("ERST_RPC_URLS"); urlsEnv != "" { | ||||||
| cfg.RpcUrls = strings.Split(urlsEnv, ",") | ||||||
| for i := range cfg.RpcUrls { | ||||||
| cfg.RpcUrls[i] = strings.TrimSpace(cfg.RpcUrls[i]) | ||||||
| cfg := &Config{} | ||||||
| parsers := []Parser{fileParser{}, EnvParser{}} | ||||||
| for _, parser := range parsers { | ||||||
| if err := parser.Parse(cfg); err != nil { | ||||||
| return nil, err | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| if err := cfg.loadFromFile(); err != nil { | ||||||
| return nil, err | ||||||
| } | ||||||
|
|
||||||
| // Phase 2: Merge defaults for any fields still unset. | ||||||
| cfg.MergeDefaults() | ||||||
|
|
||||||
|
|
@@ -303,14 +273,20 @@ func (fileParser) Parse(cfg *Config) error { | |||||
|
|
||||||
| func (c *Config) loadFromFile() error { | ||||||
| paths := []string{ | ||||||
| ".erst.toml", | ||||||
| filepath.Join(os.ExpandEnv("$HOME"), ".erst.toml"), | ||||||
| "/etc/erst/config.toml", | ||||||
| } | ||||||
|
|
||||||
| if homeDir, err := os.UserHomeDir(); err == nil && homeDir != "" { | ||||||
| paths = append(paths, filepath.Join(homeDir, ".erst.toml")) | ||||||
| } | ||||||
| paths = append(paths, ".erst.toml") | ||||||
|
|
||||||
| for _, path := range paths { | ||||||
| if err := c.loadTOML(path); err == nil { | ||||||
| return nil | ||||||
| if err := c.loadTOML(path); err != nil { | ||||||
| if os.IsNotExist(err) { | ||||||
| continue | ||||||
| } | ||||||
| return err | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
|
|
@@ -333,7 +309,7 @@ func (c *Config) loadTOML(path string) error { | |||||
| func (c *Config) parseTOML(content string) error { | ||||||
| lines := strings.Split(content, "\n") | ||||||
| for _, line := range lines { | ||||||
| line = strings.TrimSpace(line) | ||||||
| line = strings.TrimSpace(stripInlineComment(line)) | ||||||
|
||||||
| line = strings.TrimSpace(stripInlineComment(line)) | |
| line = stripInlineComment(strings.TrimSpace(line)) |
Copilot
AI
Feb 25, 2026
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.
The stripInlineComment function lacks unit test coverage. This function handles complex quote-aware comment parsing logic, including nested quote handling for single and double quotes. Given its complexity and importance for correct TOML parsing, dedicated unit tests should be added to verify behavior with edge cases like unmatched quotes, escaped quotes, and comments within strings.
Copilot
AI
Feb 25, 2026
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.
The stripInlineComment function does not handle escaped quotes. For example, a string like key = "value with \" quote" would incorrectly toggle the inDouble state when encountering the escaped quote, potentially causing comments to be parsed incorrectly. Consider adding escape character handling by checking if the previous character is a backslash before toggling quote states.
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.
The hardcoded path "/etc/erst/config.toml" is Unix-specific and will not work correctly on Windows. On Windows, this path would be invalid and could cause issues. Consider using a platform-appropriate system config path or making this path configurable, or at least document that system-level configuration is only supported on Unix-like systems.