Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
192 changes: 189 additions & 3 deletions model/dotfile_ghostty.go
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 {
Expand All @@ -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")
Comment on lines +118 to +126
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

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 raw field of the configLine struct, which stores the original line from the file. By using line.raw for all lines, you ensure that formatting is perfectly preserved for existing lines. This also simplifies the function.

 var result []string
 for _, line := range lines {
  result = append(result, line.raw)
 }
 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
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The dry-run block re-creates localKeys and remoteKeys maps and iterates through them to find new keys. This logic is redundant because mergeGhosttyConfigs already performs a similar operation to identify which remote keys are new.

To improve efficiency and reduce code duplication, you could refactor mergeGhosttyConfigs to also return the list of configLines that were added from the remote config. The Save method could then use this list directly for the dry-run output.

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
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The Save function iterates through a map of files, but if an error occurs (e.g., path expansion, file read, file write), it logs a warning and uses continue to proceed to the next file. The function then returns nil at the end, regardless of whether errors occurred. This can cause the calling command to report success even if some or all files failed to save.

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
}