feat(config): add YAML config support with JSON schema generation#174
feat(config): add YAML config support with JSON schema generation#174
Conversation
- Add YAML (.yaml, .yml) config file support alongside TOML - Config priority: config.local.yaml > config.local.yml > config.yaml > config.yml > config.local.toml > config.toml - Add `shelltime schema` command for JSON schema generation (IDE autocompletion) - Add yaml/json struct tags to all config types - Fix typo in Endpoint struct (token:"token" -> toml:"token") - Maintain full backward compatibility with existing TOML configs 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary of ChangesHello @AnnatarHe, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the configuration management capabilities of the application by introducing support for YAML configuration files alongside the existing TOML format. It establishes a clear priority order for loading configuration files, ensuring flexibility and ease of use. Furthermore, a new command has been added to generate a JSON schema, which will greatly improve the developer experience by enabling IDE autocompletion and validation for configuration files. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Codecov Report❌ Patch coverage is
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes 🚀 New features to boost your workflow:
|
Update auth command to create config.yaml instead of config.toml when initializing shelltime for new users. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for YAML configuration files, alongside the existing TOML format, and adds a new shelltime schema command to generate a JSON schema for IDE autocompletion. The changes are well-structured and the new functionality is a great addition for user experience.
My review focuses on ensuring backward compatibility, improving code clarity and maintainability, and promoting robust error handling. I've identified a critical issue regarding backward compatibility with existing TOML configurations, which contradicts one of the goals of this PR. I've also found opportunities to improve platform-independence of path handling, clarify comments, reduce code duplication, and make error handling more explicit. Additionally, I've pointed out some potentially unused code and a minor inconsistency in struct tags.
| Token string `toml:"token" yaml:"token" json:"token"` | ||
| APIEndpoint string `toml:"apiEndpoint" yaml:"apiEndpoint" json:"apiEndpoint"` | ||
| WebEndpoint string `toml:"webEndpoint" yaml:"webEndpoint" json:"webEndpoint"` | ||
| // how often sync to server | ||
| FlushCount int | ||
| FlushCount int `toml:"flushCount" yaml:"flushCount" json:"flushCount"` | ||
| // how long the synced data would keep in db: | ||
| // unit is days | ||
| GCTime int | ||
| GCTime int `toml:"gcTime" yaml:"gcTime" json:"gcTime"` | ||
|
|
||
| // is data should be masking? | ||
| // @default true | ||
| DataMasking *bool `toml:"dataMasking"` | ||
| DataMasking *bool `toml:"dataMasking" yaml:"dataMasking" json:"dataMasking"` | ||
|
|
||
| // for debug purpose | ||
| Endpoints []Endpoint `toml:"ENDPOINTS"` | ||
| Endpoints []Endpoint `toml:"ENDPOINTS" yaml:"endpoints" json:"endpoints"` | ||
|
|
||
| // WARNING | ||
| // This config will track each command metrics you run in current shell. | ||
| // Use this config only the developer asked you to do so. | ||
| // This could be very slow on each command you run. | ||
| EnableMetrics *bool `toml:"enableMetrics"` | ||
| EnableMetrics *bool `toml:"enableMetrics" yaml:"enableMetrics" json:"enableMetrics"` | ||
|
|
||
| Encrypted *bool `toml:"encrypted"` | ||
| Encrypted *bool `toml:"encrypted" yaml:"encrypted" json:"encrypted"` | ||
|
|
||
| // AI configuration | ||
| AI *AIConfig `toml:"ai"` | ||
| AI *AIConfig `toml:"ai" yaml:"ai" json:"ai"` | ||
|
|
||
| // Exclude patterns - regular expressions to exclude commands from being saved | ||
| // Commands matching any of these patterns will not be synced to the server | ||
| Exclude []string `toml:"exclude"` | ||
| Exclude []string `toml:"exclude" yaml:"exclude" json:"exclude"` | ||
|
|
||
| // CCUsage configuration for Claude Code usage tracking (v1 - ccusage CLI based) | ||
| CCUsage *CCUsage `toml:"ccusage"` | ||
| CCUsage *CCUsage `toml:"ccusage" yaml:"ccusage" json:"ccusage"` | ||
|
|
||
| // CCOtel configuration for OTEL-based Claude Code tracking (v2 - gRPC passthrough) | ||
| CCOtel *CCOtel `toml:"ccotel"` | ||
| CCOtel *CCOtel `toml:"ccotel" yaml:"ccotel" json:"ccotel"` | ||
|
|
||
| // CodeTracking configuration for coding activity heartbeat tracking | ||
| CodeTracking *CodeTracking `toml:"codeTracking"` | ||
| CodeTracking *CodeTracking `toml:"codeTracking" yaml:"codeTracking" json:"codeTracking"` | ||
|
|
||
| // LogCleanup configuration for automatic log file cleanup in daemon | ||
| LogCleanup *LogCleanup `toml:"logCleanup"` | ||
| LogCleanup *LogCleanup `toml:"logCleanup" yaml:"logCleanup" json:"logCleanup"` | ||
|
|
||
| // SocketPath is the path to the Unix domain socket used for communication | ||
| // between the CLI and the daemon. | ||
| SocketPath string `toml:"socketPath"` | ||
| SocketPath string `toml:"socketPath" yaml:"socketPath" json:"socketPath"` |
There was a problem hiding this comment.
Adding toml struct tags with camelCase names (e.g., toml:"token") is a breaking change for existing users who have Token (PascalCase) in their config.toml files. The PR description states 'Maintain full backward compatibility with existing TOML configs'.
To maintain backward compatibility, you should remove the toml tags from all fields in ShellTimeConfig. The go-toml library matches field names case-insensitively by default, so it will correctly unmarshal both Token (old format) and token (new convention) from TOML files into the corresponding struct fields. The yaml and json tags can remain for the new config formats.
For example, Token string toml:"token" yaml:"token" json:"token"should becomeToken string yaml:"token" json:"token".
| } | ||
| configFile := os.ExpandEnv(fmt.Sprintf("%s/%s/%s", "$HOME", model.COMMAND_BASE_STORAGE_FOLDER, "config.toml")) | ||
| configService := model.NewConfigService(configFile) | ||
| configDir := os.ExpandEnv(fmt.Sprintf("%s/%s", "$HOME", model.COMMAND_BASE_STORAGE_FOLDER)) |
There was a problem hiding this comment.
The path construction is not platform-independent. It uses a hardcoded path separator / and relies on shell expansion of $HOME. It's better to use os.UserHomeDir() and filepath.Join for creating paths to ensure cross-platform compatibility. The model package already provides a helper function GetBaseStoragePath for this.
| configDir := os.ExpandEnv(fmt.Sprintf("%s/%s", "$HOME", model.COMMAND_BASE_STORAGE_FOLDER)) | |
| configDir := model.GetBaseStoragePath() |
References
- For platform-independent paths, use
filepath.Jointo combine segments andos.UserHomeDir()to get the home directory, rather than hardcoding path separators or environment variables like$HOME.
| ctx := context.Background() | ||
| configFile := os.ExpandEnv(fmt.Sprintf("%s/%s/%s", "$HOME", model.COMMAND_BASE_STORAGE_FOLDER, "config.toml")) | ||
| daemonConfigService := model.NewConfigService(configFile) | ||
| configDir := os.ExpandEnv(fmt.Sprintf("%s/%s", "$HOME", model.COMMAND_BASE_STORAGE_FOLDER)) |
There was a problem hiding this comment.
The path construction is not platform-independent. It uses a hardcoded path separator / and relies on shell expansion of $HOME. It's better to use os.UserHomeDir() and filepath.Join for creating paths to ensure cross-platform compatibility. The model package already provides a helper function GetBaseStoragePath for this.
| configDir := os.ExpandEnv(fmt.Sprintf("%s/%s", "$HOME", model.COMMAND_BASE_STORAGE_FOLDER)) | |
| configDir := model.GetBaseStoragePath() |
References
- For platform-independent paths, use
filepath.Jointo combine segments andos.UserHomeDir()to get the home directory, rather than hardcoding path separators or environment variables like$HOME.
| if unmarshalErr := unmarshalConfig(localConfig, files.localFormat, &localSettings); unmarshalErr == nil { | ||
| mergeConfig(&config, &localSettings) | ||
| } |
There was a problem hiding this comment.
Errors from unmarshalling the local config file are silently ignored. If a user has a malformed local config, they would likely expect an error message instead of their settings being silently disregarded. It's better to propagate this error.
if unmarshalErr := unmarshalConfig(localConfig, files.localFormat, &localSettings); unmarshalErr != nil {
err = fmt.Errorf("failed to parse local config file %s: %w", files.localFile, unmarshalErr)
return
}
mergeConfig(&config, &localSettings)| // findConfigFiles discovers config files in priority order | ||
| // Priority: config.local.yaml > config.local.yml > config.yaml > config.yml > config.local.toml > config.toml |
There was a problem hiding this comment.
The comment describing the priority order is misleading and doesn't accurately reflect what the function does. The function finds the highest-priority base file and local file separately, rather than using a single flat priority list. The comment should be updated to clarify this.
| // findConfigFiles discovers config files in priority order | |
| // Priority: config.local.yaml > config.local.yml > config.yaml > config.yml > config.local.toml > config.toml | |
| // findConfigFiles discovers config files based on a priority order. | |
| // It finds the highest-priority base file and local file separately. |
| // Check for base config files (YAML first, then TOML) | ||
| baseFiles := []string{"config.yaml", "config.yml", "config.toml"} | ||
| for _, name := range baseFiles { | ||
| path := filepath.Join(configDir, name) | ||
| if _, err := os.Stat(path); err == nil { | ||
| result.baseFile = path | ||
| result.baseFormat = detectFormat(name) | ||
| break | ||
| } | ||
| } | ||
|
|
||
| // Check for local config files (YAML first, then TOML) | ||
| localFiles := []string{"config.local.yaml", "config.local.yml", "config.local.toml"} | ||
| for _, name := range localFiles { | ||
| path := filepath.Join(configDir, name) | ||
| if _, err := os.Stat(path); err == nil { | ||
| result.localFile = path | ||
| result.localFormat = detectFormat(name) | ||
| break | ||
| } | ||
| } |
There was a problem hiding this comment.
The logic for finding base and local config files is duplicated. Consider extracting this logic into a helper function to improve code clarity and reduce duplication. A single function could handle iterating through a list of potential filenames and returning the first one that exists.
| // It supports both YAML (.yaml, .yml) and TOML (.toml) formats with priority: | ||
| // config.local.yaml > config.local.yml > config.yaml > config.yml > config.local.toml > config.toml |
There was a problem hiding this comment.
This priority list in the comment is misleading. The service supports a base configuration file and a local override file, which can have different formats. The priority is applied separately for base files and for local files. It would be clearer to document these priorities separately.
| // It supports both YAML (.yaml, .yml) and TOML (.toml) formats with priority: | |
| // config.local.yaml > config.local.yml > config.yaml > config.yml > config.local.toml > config.toml | |
| // It supports both YAML (.yaml, .yml) and TOML (.toml) formats. | |
| // Priorities are: Base(config.yaml > .yml > .toml), Local(config.local.yaml > .yml > .toml) |
| // GetYAMLConfigFilePath returns the path to the main config file (YAML) | ||
| func GetYAMLConfigFilePath() string { | ||
| return GetStoragePath("config.yaml") | ||
| } | ||
|
|
||
| // GetYMLConfigFilePath returns the path to the main config file (YML) | ||
| func GetYMLConfigFilePath() string { | ||
| return GetStoragePath("config.yml") | ||
| } | ||
|
|
||
| // GetLocalYAMLConfigFilePath returns the path to the local config file (YAML) | ||
| func GetLocalYAMLConfigFilePath() string { | ||
| return GetStoragePath("config.local.yaml") | ||
| } | ||
|
|
||
| // GetLocalYMLConfigFilePath returns the path to the local config file (YML) | ||
| func GetLocalYMLConfigFilePath() string { | ||
| return GetStoragePath("config.local.yml") | ||
| } |
There was a problem hiding this comment.
These new helper functions (GetYAMLConfigFilePath, GetYMLConfigFilePath, GetLocalYAMLConfigFilePath, GetLocalYMLConfigFilePath) seem to be unused. The new configuration loading logic in model/config.go dynamically discovers config files in a directory, making these specific path helpers redundant. If they are not used elsewhere, they should be removed to avoid dead code.
|
|
||
| // for debug purpose | ||
| Endpoints []Endpoint `toml:"ENDPOINTS"` | ||
| Endpoints []Endpoint `toml:"ENDPOINTS" yaml:"endpoints" json:"endpoints"` |
There was a problem hiding this comment.
The toml tag ENDPOINTS is inconsistent with the yaml and json tags (endpoints). For consistency, it's better to use the same name across all formats. Consider changing toml:"ENDPOINTS" to toml:"endpoints".
| Endpoints []Endpoint `toml:"ENDPOINTS" yaml:"endpoints" json:"endpoints"` | |
| Endpoints []Endpoint `toml:"endpoints" yaml:"endpoints" json:"endpoints"` |
Add yaml-language-server $schema comment to generated config.yaml for IDE autocompletion support. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Generate config.schema.json locally at ~/.shelltime/ during init/auth and reference it in the generated config.yaml for IDE autocompletion. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Update TOML struct tags to use capitalized field names (Token, APIEndpoint, etc.) to maintain compatibility with existing TOML config files while keeping yaml/json tags lowercase for new YAML configs. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 participant
Summary
config.local.yaml>config.local.yml>config.yaml>config.yml>config.local.toml>config.tomlshelltime schemacommand to generate JSON schema for IDE autocompletionUsage
YAML Config
Users can now use YAML format for config files:
JSON Schema for IDE
Generate schema for IDE autocompletion:
shelltime schema -o ~/.shelltime/config-schema.jsonThen in your YAML config:
Test plan
🤖 Generated with Claude Code