-
Notifications
You must be signed in to change notification settings - Fork 0
feat(dotfile): add advanced merge support for config files #104
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
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,6 +1,15 @@ | ||
| package model | ||
|
|
||
| import "context" | ||
| import ( | ||
| "bufio" | ||
| "context" | ||
| "fmt" | ||
| "os" | ||
| "path/filepath" | ||
| "strings" | ||
|
|
||
| "github.com/sirupsen/logrus" | ||
| ) | ||
|
|
||
| // GhosttyApp handles Ghostty terminal configuration files | ||
| type GhosttyApp struct { | ||
|
|
@@ -18,11 +27,188 @@ func (g *GhosttyApp) Name() string { | |
| func (g *GhosttyApp) GetConfigPaths() []string { | ||
| return []string{ | ||
| "~/.config/ghostty/config", | ||
| "~/.config/ghostty", | ||
| } | ||
| } | ||
|
|
||
| func (g *GhosttyApp) CollectDotfiles(ctx context.Context) ([]DotfileItem, error) { | ||
| skipIgnored := true | ||
| return g.CollectFromPaths(ctx, g.Name(), g.GetConfigPaths(), &skipIgnored) | ||
| } | ||
| } | ||
|
|
||
| // configLine represents a line in the Ghostty config file | ||
| type configLine struct { | ||
| isComment bool | ||
| isBlank bool | ||
| key string | ||
| value string | ||
| raw string // for comments and blank lines | ||
| } | ||
|
|
||
| // parseGhosttyConfig parses Ghostty config content into structured lines | ||
| func (g *GhosttyApp) parseGhosttyConfig(content string) []configLine { | ||
| var lines []configLine | ||
| scanner := bufio.NewScanner(strings.NewReader(content)) | ||
|
|
||
| for scanner.Scan() { | ||
| line := scanner.Text() | ||
| trimmed := strings.TrimSpace(line) | ||
|
|
||
| if trimmed == "" { | ||
| lines = append(lines, configLine{ | ||
| isBlank: true, | ||
| raw: line, | ||
| }) | ||
| } else if strings.HasPrefix(trimmed, "#") { | ||
| lines = append(lines, configLine{ | ||
| isComment: true, | ||
| raw: line, | ||
| }) | ||
| } else if strings.Contains(line, "=") { | ||
| parts := strings.SplitN(line, "=", 2) | ||
| key := strings.TrimSpace(parts[0]) | ||
| value := "" | ||
| if len(parts) > 1 { | ||
| value = strings.TrimSpace(parts[1]) | ||
| } | ||
| lines = append(lines, configLine{ | ||
| key: key, | ||
| value: value, | ||
| raw: line, | ||
| }) | ||
| } else { | ||
| // Treat as a comment if it doesn't match key=value format | ||
| lines = append(lines, configLine{ | ||
| isComment: true, | ||
| raw: line, | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| return lines | ||
| } | ||
|
|
||
| // mergeGhosttyConfigs merges remote config with local config, local has priority | ||
| func (g *GhosttyApp) mergeGhosttyConfigs(localLines, remoteLines []configLine) []configLine { | ||
| // Create a map of local keys for quick lookup | ||
| localKeys := make(map[string]bool) | ||
| for _, line := range localLines { | ||
| if !line.isComment && !line.isBlank && line.key != "" { | ||
| localKeys[line.key] = true | ||
| } | ||
| } | ||
|
|
||
| // Start with local config | ||
| merged := make([]configLine, len(localLines)) | ||
| copy(merged, localLines) | ||
|
|
||
| // Add keys from remote that don't exist in local | ||
| for _, remoteLine := range remoteLines { | ||
| if !remoteLine.isComment && !remoteLine.isBlank && remoteLine.key != "" { | ||
| if !localKeys[remoteLine.key] { | ||
| merged = append(merged, remoteLine) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return merged | ||
| } | ||
|
|
||
| // formatGhosttyConfig converts config lines back to string | ||
| func (g *GhosttyApp) formatGhosttyConfig(lines []configLine) string { | ||
| var result []string | ||
| for _, line := range lines { | ||
| if line.isComment || line.isBlank { | ||
| result = append(result, line.raw) | ||
| } else { | ||
| result = append(result, fmt.Sprintf("%s = %s", line.key, line.value)) | ||
| } | ||
| } | ||
| return strings.Join(result, "\n") | ||
| } | ||
|
|
||
| // Save overrides the base Save method to handle Ghostty's specific config format | ||
| func (g *GhosttyApp) Save(ctx context.Context, files map[string]string, isDryRun bool) error { | ||
| for path, remoteContent := range files { | ||
| expandedPath, err := g.expandPath(path) | ||
| if err != nil { | ||
| logrus.Warnf("Failed to expand path %s: %v", path, err) | ||
| continue | ||
| } | ||
|
|
||
| // Read existing local content if file exists | ||
| var localContent string | ||
| if existingBytes, err := os.ReadFile(expandedPath); err == nil { | ||
| localContent = string(existingBytes) | ||
| } else if !os.IsNotExist(err) { | ||
| logrus.Warnf("Failed to read existing file %s: %v", expandedPath, err) | ||
| continue | ||
| } | ||
|
|
||
| // Parse both configs | ||
| localLines := g.parseGhosttyConfig(localContent) | ||
| remoteLines := g.parseGhosttyConfig(remoteContent) | ||
|
|
||
| // Merge configs (local has priority) | ||
| mergedLines := g.mergeGhosttyConfigs(localLines, remoteLines) | ||
| mergedContent := g.formatGhosttyConfig(mergedLines) | ||
|
|
||
| // Check if there are any differences | ||
| if localContent == mergedContent { | ||
| logrus.Infof("No changes needed for %s", expandedPath) | ||
| continue | ||
| } | ||
|
|
||
| if isDryRun { | ||
| // In dry-run mode, show the diff | ||
| fmt.Printf("\n📄 %s:\n", expandedPath) | ||
| fmt.Println("--- Changes to be applied ---") | ||
|
|
||
| // Show added keys (from remote) | ||
| remoteKeys := make(map[string]string) | ||
| for _, line := range remoteLines { | ||
| if !line.isComment && !line.isBlank && line.key != "" { | ||
| remoteKeys[line.key] = line.value | ||
| } | ||
| } | ||
|
|
||
| localKeys := make(map[string]string) | ||
| for _, line := range localLines { | ||
| if !line.isComment && !line.isBlank && line.key != "" { | ||
| localKeys[line.key] = line.value | ||
| } | ||
| } | ||
|
|
||
| // Show new keys from remote | ||
| hasChanges := false | ||
| for key, value := range remoteKeys { | ||
| if _, exists := localKeys[key]; !exists { | ||
| fmt.Printf("+ %s = %s (from remote)\n", key, value) | ||
| hasChanges = true | ||
| } | ||
| } | ||
|
Comment on lines
+167
to
+188
Contributor
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. The dry-run block re-creates To improve efficiency and reduce code duplication, you could refactor Example refactoring: // In mergeGhosttyConfigs
func (g *GhosttyApp) mergeGhosttyConfigs(...) ([]configLine, []configLine) {
// ...
var addedLines []configLine
for _, remoteLine := range remoteLines {
if !localKeys[remoteLine.key] {
merged = append(merged, remoteLine)
addedLines = append(addedLines, remoteLine)
}
}
return merged, addedLines
}
// In Save method
mergedLines, addedLines := g.mergeGhosttyConfigs(localLines, remoteLines)
// ...
if isDryRun {
// ...
for _, line := range addedLines {
fmt.Printf("+ %s = %s (from remote)\n", line.key, line.value)
}
// ...
}This change would make the code more DRY (Don't Repeat Yourself) and slightly more performant. |
||
|
|
||
| if !hasChanges { | ||
| fmt.Println("No new keys from remote") | ||
| } | ||
|
|
||
| fmt.Println("--- End of changes ---") | ||
| continue | ||
| } | ||
|
|
||
| // Ensure directory exists | ||
| dir := filepath.Dir(expandedPath) | ||
| if err := os.MkdirAll(dir, 0755); err != nil { | ||
| logrus.Warnf("Failed to create directory %s: %v", dir, err) | ||
| continue | ||
| } | ||
|
|
||
| // Write merged content | ||
| if err := os.WriteFile(expandedPath, []byte(mergedContent), 0644); err != nil { | ||
| logrus.Warnf("Failed to save file %s: %v", expandedPath, err) | ||
| } else { | ||
| logrus.Infof("Saved merged config to %s", expandedPath) | ||
| } | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
Comment on lines
+130
to
+214
Contributor
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. The To provide more accurate feedback on the operation's success, it would be better to collect any errors that occur within the loop and return a consolidated error at the end. This ensures that failures are propagated up and can be handled appropriately (e.g., by exiting with a non-zero status code). Example of aggregating errors: func (g *GhosttyApp) Save(...) error {
var errs []error
for path, remoteContent := range files {
// ... on error
if err != nil {
errs = append(errs, fmt.Errorf("failed to process %s: %w", path, err))
logrus.Warnf("Failed to process %s: %v", path, err)
continue
}
// ...
}
if len(errs) > 0 {
// Using a modern approach to join errors
return errors.Join(errs...)
}
return nil
} |
||
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 current implementation re-formats all key-value pairs using
fmt.Sprintf("%s = %s", line.key, line.value). This alters the original formatting (e.g., spacing around=) and could strip inline comments. The PR description states that a goal is to "Maintain original formatting including whitespace and comments".To better adhere to this goal, you can leverage the
rawfield of theconfigLinestruct, which stores the original line from the file. By usingline.rawfor all lines, you ensure that formatting is perfectly preserved for existing lines. This also simplifies the function.