feat: add starship config support to dotfiles push/pull#88
Conversation
- Add StarshipApp implementation in model/dotfile_starship.go - Update dotfiles_pull.go to include starship in available apps - Update dotfiles_push.go to include starship in available apps - Starship config file supported: ~/.config/starship.toml Fixes #87 🤖 Generated with [Claude Code](https://claude.ai/code) Co-authored-by: Le He <AnnatarHe@users.noreply.github.com>
There was a problem hiding this comment.
Summary of Changes
Hello @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 extends the dotfiles management system to include support for Starship, a popular cross-shell prompt. It enables users to seamlessly push and pull their Starship configuration files, ensuring their prompt settings are synchronized across different environments.
Highlights
- Starship Configuration Support: Implemented support for managing Starship shell prompt configurations within the dotfiles system.
- New Starship App Handler: A new StarshipApp handler has been added in model/dotfile_starship.go to define how Starship dotfiles are handled.
- Integration with Push/Pull Commands: The dotfiles_pull.go and dotfiles_push.go commands have been updated to recognize and include Starship in the list of supported applications for dotfile management.
- Supported Configuration Path: The system now supports the Starship configuration file located at ~/.config/starship.toml.
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.
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
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
|
Claude finished @AnnatarHe's task —— View job Pull Request ReviewReview Tasks:
✅ Code Quality & ArchitectureThe implementation follows the established patterns perfectly:
✅ Implementation QualityStarshipApp (model/dotfile_starship.go:1-27)
Push/Pull Integration
🔍 Security & Safety
⚡ Performance
🚨 Issues FoundMissing Test CoverageThe implementation lacks unit tests. All existing dotfile apps appear to lack dedicated tests as well, but this would be a good opportunity to establish testing patterns. Recommendation: Consider adding tests for: func TestStarshipApp_Name(t *testing.T)
func TestStarshipApp_GetConfigPaths(t *testing.T)
func TestStarshipApp_CollectDotfiles(t *testing.T)Minor Consistency ConsiderationThe config path 📋 Recommendations
🎯 Overall AssessmentThis is a well-implemented, low-risk addition that:
The implementation is ready for merge with the minor recommendation to add tests in a follow-up PR. Files analyzed:
|
There was a problem hiding this comment.
Code Review
This pull request adds support for Starship configuration to the dotfiles push and pull commands. The implementation is straightforward, adding a new StarshipApp model and integrating it into the existing commands. My review includes suggestions to make the Starship configuration discovery more robust by respecting standard environment variables, and to improve the file collection logic to handle edge cases and ensure configuration paths are stored in a portable format.
| func (s *StarshipApp) CollectDotfiles(ctx context.Context) ([]DotfileItem, error) { | ||
| return s.CollectFromPaths(ctx, s.Name(), s.GetConfigPaths()) | ||
| } |
There was a problem hiding this comment.
The generic CollectFromPaths function has two issues that affect this implementation:
- It will traverse and collect all files if the config path points to a directory. Starship's config is a single file, so this could lead to unexpected behavior.
- It stores the absolute, user-specific path (e.g.,
/home/user/...) instead of the portable path with a tilde (~/.config/...). This breaks synchronization across different machines or users.
A custom CollectDotfiles implementation can address both issues by explicitly checking for files and ensuring the original, portable path is stored.
Note: This change requires adding "os" and "github.com/sirupsen/logrus" to the file's imports.
func (s *StarshipApp) CollectDotfiles(ctx context.Context) ([]DotfileItem, error) {
hostname, _ := os.Hostname()
var dotfiles []DotfileItem
for _, path := range s.GetConfigPaths() {
expandedPath, err := s.expandPath(path)
if err != nil {
logrus.Debugf("Failed to expand path %s: %v", path, err)
continue
}
fileInfo, err := os.Stat(expandedPath)
if err != nil {
if !os.IsNotExist(err) {
logrus.Debugf("Failed to stat path %s: %v", expandedPath, err)
}
continue
}
if fileInfo.IsDir() {
logrus.Warnf("Starship config path is a directory, but should be a file: %s", expandedPath)
continue
}
content, modTime, err := s.readFileContent(path)
if err != nil {
logrus.Debugf("Failed to read file content for %s: %v", path, err)
continue
}
dotfiles = append(dotfiles, DotfileItem{
App: s.Name(),
Path: path, // Use original path to preserve '~'
Content: content,
FileModifiedAt: modTime,
FileType: "file",
Hostname: hostname,
})
}
return dotfiles, nil
}| func (s *StarshipApp) GetConfigPaths() []string { | ||
| return []string{ | ||
| "~/.config/starship.toml", | ||
| } | ||
| } |
There was a problem hiding this comment.
The current implementation for GetConfigPaths only considers the default ~/.config/starship.toml path. Starship configuration is more flexible, respecting the STARSHIP_CONFIG environment variable, and falling back to $XDG_CONFIG_HOME/starship.toml if XDG_CONFIG_HOME is set. To make this more robust and align with starship's behavior, consider checking for these environment variables.
This will require adding "os" and "path/filepath" to your imports.
| func (s *StarshipApp) GetConfigPaths() []string { | |
| return []string{ | |
| "~/.config/starship.toml", | |
| } | |
| } | |
| func (s *StarshipApp) GetConfigPaths() []string { | |
| // Starship config can be set via STARSHIP_CONFIG env var. | |
| if configPath := os.Getenv("STARSHIP_CONFIG"); configPath != "" { | |
| return []string{configPath} | |
| } | |
| // Default to XDG_CONFIG_HOME if set. | |
| if xdgConfigHome := os.Getenv("XDG_CONFIG_HOME"); xdgConfigHome != "" { | |
| return []string{filepath.Join(xdgConfigHome, "starship.toml")} | |
| } | |
| // Fallback to default location. | |
| return []string{ | |
| "~/.config/starship.toml", | |
| } | |
| } |
Adds starship configuration support to the dotfiles push and pull commands.
Changes
Fixes #87
🤖 Generated with Claude Code