Skip to content
Merged
Show file tree
Hide file tree
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
43 changes: 42 additions & 1 deletion model/dotfile_apps.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,12 @@ func (b *BaseApp) readFileContent(path string) (string, *time.Time, error) {
return string(content), &modTime, nil
}

func (b *BaseApp) CollectFromPaths(_ context.Context, appName string, paths []string) ([]DotfileItem, error) {
func (b *BaseApp) CollectFromPaths(_ context.Context, appName string, paths []string, skipIgnoredSections *bool) ([]DotfileItem, error) {
// Default to true if not specified
shouldSkipIgnored := true
if skipIgnoredSections != nil {
shouldSkipIgnored = *skipIgnoredSections
}
hostname, _ := os.Hostname()
var dotfiles []DotfileItem

Expand Down Expand Up @@ -143,6 +148,11 @@ func (b *BaseApp) CollectFromPaths(_ context.Context, appName string, paths []st
continue
}

// Filter ignored sections if requested
if shouldSkipIgnored {
content = b.filterIgnoredSections(content)
}

dotfiles = append(dotfiles, DotfileItem{
App: appName,
Path: file,
Expand All @@ -160,6 +170,11 @@ func (b *BaseApp) CollectFromPaths(_ context.Context, appName string, paths []st
continue
}

// Filter ignored sections if requested
if shouldSkipIgnored {
content = b.filterIgnoredSections(content)
}

dotfiles = append(dotfiles, DotfileItem{
App: appName,
Path: expandedPath,
Expand Down Expand Up @@ -188,6 +203,32 @@ func (b *BaseApp) collectFromDirectory(dir string) ([]string, error) {
return files, err
}

// filterIgnoredSections removes content between SHELLTIME IGNORE BEGIN and SHELLTIME IGNORE END markers
func (b *BaseApp) filterIgnoredSections(content string) string {
lines := strings.Split(content, "\n")
var filteredLines []string
var inIgnoreBlock bool

for _, line := range lines {
// Check for ignore markers (can be in comments)
if strings.Contains(line, "SHELLTIME IGNORE BEGIN") {
inIgnoreBlock = true
continue
}
if strings.Contains(line, "SHELLTIME IGNORE END") {
inIgnoreBlock = false
continue
}

// Only include lines that are not in an ignore block
if !inIgnoreBlock {
filteredLines = append(filteredLines, line)
}
}

return strings.Join(filteredLines, "\n")
}
Comment on lines +207 to +230
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 implementation of filterIgnoredSections can be improved for readability and efficiency:

  1. Use constants for markers: The strings "SHELLTIME IGNORE BEGIN" and "SHELLTIME IGNORE END" are used as magic strings. Defining them as constants at the package or function level would improve readability and make them easier to manage.
  2. Pre-allocate slice capacity: You can pre-allocate the capacity for filteredLines to avoid multiple re-allocations as the slice grows. This is a good practice for performance, especially with larger files.
func (b *BaseApp) filterIgnoredSections(content string) string {
	const ignoreBeginMarker = "SHELLTIME IGNORE BEGIN"
	const ignoreEndMarker = "SHELLTIME IGNORE END"

	lines := strings.Split(content, "\n")
	filteredLines := make([]string, 0, len(lines))
	var inIgnoreBlock bool

	for _, line := range lines {
		// Check for ignore markers (can be in comments)
		if strings.Contains(line, ignoreBeginMarker) {
			inIgnoreBlock = true
			continue
		}
		if strings.Contains(line, ignoreEndMarker) {
			inIgnoreBlock = false
			continue
		}

		// Only include lines that are not in an ignore block
		if !inIgnoreBlock {
			filteredLines = append(filteredLines, line)
		}
	}

	return strings.Join(filteredLines, "\n")
}


// IsEqual checks if the provided files match the local files by comparing SHA256 hashes
func (b *BaseApp) IsEqual(_ context.Context, files map[string]string) (map[string]bool, error) {
result := make(map[string]bool)
Expand Down
54 changes: 49 additions & 5 deletions model/dotfile_apps_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,8 @@ func TestBaseApp_CollectFromPaths(t *testing.T) {
require.NoError(t, err)

t.Run("collect from single file", func(t *testing.T) {
dotfiles, err := app.CollectFromPaths(ctx, "testapp", []string{configFile})
skipIgnored := true
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 variable skipIgnored is repeatedly declared with := true inside each t.Run block (e.g., here, and on lines 152, 174, 182). To reduce repetition, you can declare it once before all the sub-tests.

dotfiles, err := app.CollectFromPaths(ctx, "testapp", []string{configFile}, &skipIgnored)
require.NoError(t, err)
assert.Len(t, dotfiles, 1)

Expand All @@ -148,7 +149,8 @@ func TestBaseApp_CollectFromPaths(t *testing.T) {
})

t.Run("collect from directory", func(t *testing.T) {
dotfiles, err := app.CollectFromPaths(ctx, "testapp", []string{subDir})
skipIgnored := true
dotfiles, err := app.CollectFromPaths(ctx, "testapp", []string{subDir}, &skipIgnored)
require.NoError(t, err)

// Should find 2 files (hidden files are ignored)
Expand All @@ -169,17 +171,58 @@ func TestBaseApp_CollectFromPaths(t *testing.T) {
})

t.Run("collect from mixed paths", func(t *testing.T) {
dotfiles, err := app.CollectFromPaths(ctx, "testapp", []string{configFile, subDir})
skipIgnored := true
dotfiles, err := app.CollectFromPaths(ctx, "testapp", []string{configFile, subDir}, &skipIgnored)
require.NoError(t, err)
assert.Len(t, dotfiles, 3) // 1 file + 2 files from directory
})

t.Run("collect from non-existent path", func(t *testing.T) {
nonExistentPath := filepath.Join(tmpDir, "does-not-exist")
dotfiles, err := app.CollectFromPaths(ctx, "testapp", []string{nonExistentPath})
skipIgnored := true
dotfiles, err := app.CollectFromPaths(ctx, "testapp", []string{nonExistentPath}, &skipIgnored)
require.NoError(t, err)
assert.Empty(t, dotfiles) // Should skip non-existent paths
})

t.Run("collect with ignored sections", func(t *testing.T) {
// Create a file with ignored sections
configWithIgnore := filepath.Join(tmpDir, "config_with_ignore.conf")
configContentWithIgnore := `line1
# SHELLTIME IGNORE BEGIN
secret_key=123456
password=hidden
# SHELLTIME IGNORE END
line2
visible_key=value`
err = os.WriteFile(configWithIgnore, []byte(configContentWithIgnore), 0644)
require.NoError(t, err)

// Test with skipIgnored = true (default behavior)
skipIgnored := true
dotfiles, err := app.CollectFromPaths(ctx, "testapp", []string{configWithIgnore}, &skipIgnored)
require.NoError(t, err)
require.Len(t, dotfiles, 1)

// Should not contain ignored sections
assert.NotContains(t, dotfiles[0].Content, "secret_key")
assert.NotContains(t, dotfiles[0].Content, "password=hidden")
assert.NotContains(t, dotfiles[0].Content, "SHELLTIME IGNORE")
assert.Contains(t, dotfiles[0].Content, "line1")
assert.Contains(t, dotfiles[0].Content, "line2")
assert.Contains(t, dotfiles[0].Content, "visible_key=value")
Comment on lines +208 to +213
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 assertions here check for the presence or absence of certain substrings. This is good, but it can be made more robust by asserting the exact expected content. This ensures that no extra lines or characters are present and that the line order is correct.

For the skipIgnored = true case, you could assert:

expectedContent := "line1\nline2\nvisible_key=value"
assert.Equal(t, expectedContent, dotfiles[0].Content)

Similarly, for the skipIgnored = false case, you can assert that the content is identical to the original configContentWithIgnore.


// Test with skipIgnored = false
skipIgnored = false
dotfiles, err = app.CollectFromPaths(ctx, "testapp", []string{configWithIgnore}, &skipIgnored)
require.NoError(t, err)
require.Len(t, dotfiles, 1)

// Should contain all content including ignored sections
assert.Contains(t, dotfiles[0].Content, "secret_key")
assert.Contains(t, dotfiles[0].Content, "password=hidden")
assert.Contains(t, dotfiles[0].Content, "SHELLTIME IGNORE")
})
}

func TestBaseApp_collectFromDirectory(t *testing.T) {
Expand Down Expand Up @@ -524,7 +567,8 @@ func TestBaseApp_Integration(t *testing.T) {

t.Run("full workflow", func(t *testing.T) {
// 1. Collect dotfiles
dotfiles, err := app.CollectFromPaths(ctx, "testapp", []string{configFile, subDir})
skipIgnored := true
dotfiles, err := app.CollectFromPaths(ctx, "testapp", []string{configFile, subDir}, &skipIgnored)
require.NoError(t, err)
assert.Len(t, dotfiles, 2)

Expand Down
3 changes: 2 additions & 1 deletion model/dotfile_bash.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,6 @@ func (b *BashApp) GetConfigPaths() []string {
}

func (b *BashApp) CollectDotfiles(ctx context.Context) ([]DotfileItem, error) {
return b.CollectFromPaths(ctx, b.Name(), b.GetConfigPaths())
skipIgnored := true
return b.CollectFromPaths(ctx, b.Name(), b.GetConfigPaths(), &skipIgnored)
Comment on lines +28 to +29
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

This implementation can be simplified. The CollectFromPaths function is designed to handle a nil value for skipIgnoredSections by defaulting to true. Therefore, you can just pass nil directly.

This simplification applies to all other CollectDotfiles methods in the other dotfile_*.go files as well, which would remove a lot of boilerplate code.

Suggested change
skipIgnored := true
return b.CollectFromPaths(ctx, b.Name(), b.GetConfigPaths(), &skipIgnored)
return b.CollectFromPaths(ctx, b.Name(), b.GetConfigPaths(), nil)

}
3 changes: 2 additions & 1 deletion model/dotfile_claude.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,6 @@ func (c *ClaudeApp) GetConfigPaths() []string {
}

func (c *ClaudeApp) CollectDotfiles(ctx context.Context) ([]DotfileItem, error) {
return c.CollectFromPaths(ctx, c.Name(), c.GetConfigPaths())
skipIgnored := true
return c.CollectFromPaths(ctx, c.Name(), c.GetConfigPaths(), &skipIgnored)
}
3 changes: 2 additions & 1 deletion model/dotfile_fish.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,6 @@ func (f *FishApp) GetConfigPaths() []string {
}

func (f *FishApp) CollectDotfiles(ctx context.Context) ([]DotfileItem, error) {
return f.CollectFromPaths(ctx, f.Name(), f.GetConfigPaths())
skipIgnored := true
return f.CollectFromPaths(ctx, f.Name(), f.GetConfigPaths(), &skipIgnored)
}
3 changes: 2 additions & 1 deletion model/dotfile_ghostty.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,6 @@ func (g *GhosttyApp) GetConfigPaths() []string {
}

func (g *GhosttyApp) CollectDotfiles(ctx context.Context) ([]DotfileItem, error) {
return g.CollectFromPaths(ctx, g.Name(), g.GetConfigPaths())
skipIgnored := true
return g.CollectFromPaths(ctx, g.Name(), g.GetConfigPaths(), &skipIgnored)
}
3 changes: 2 additions & 1 deletion model/dotfile_git.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,6 @@ func (g *GitApp) GetConfigPaths() []string {
}

func (g *GitApp) CollectDotfiles(ctx context.Context) ([]DotfileItem, error) {
return g.CollectFromPaths(ctx, g.Name(), g.GetConfigPaths())
skipIgnored := true
return g.CollectFromPaths(ctx, g.Name(), g.GetConfigPaths(), &skipIgnored)
}
3 changes: 2 additions & 1 deletion model/dotfile_kitty.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,6 @@ func (k *KittyApp) GetConfigPaths() []string {
}

func (k *KittyApp) CollectDotfiles(ctx context.Context) ([]DotfileItem, error) {
return k.CollectFromPaths(ctx, k.Name(), k.GetConfigPaths())
skipIgnored := true
return k.CollectFromPaths(ctx, k.Name(), k.GetConfigPaths(), &skipIgnored)
}
3 changes: 2 additions & 1 deletion model/dotfile_kubernetes.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,6 @@ func (k *KubernetesApp) GetConfigPaths() []string {
}

func (k *KubernetesApp) CollectDotfiles(ctx context.Context) ([]DotfileItem, error) {
return k.CollectFromPaths(ctx, k.Name(), k.GetConfigPaths())
skipIgnored := true
return k.CollectFromPaths(ctx, k.Name(), k.GetConfigPaths(), &skipIgnored)
}
3 changes: 2 additions & 1 deletion model/dotfile_npm.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,6 @@ func (n *NpmApp) GetConfigPaths() []string {
}

func (n *NpmApp) CollectDotfiles(ctx context.Context) ([]DotfileItem, error) {
return n.CollectFromPaths(ctx, n.Name(), n.GetConfigPaths())
skipIgnored := true
return n.CollectFromPaths(ctx, n.Name(), n.GetConfigPaths(), &skipIgnored)
}
3 changes: 2 additions & 1 deletion model/dotfile_nvim.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,6 @@ func (n *NvimApp) GetConfigPaths() []string {
}

func (n *NvimApp) CollectDotfiles(ctx context.Context) ([]DotfileItem, error) {
return n.CollectFromPaths(ctx, n.Name(), n.GetConfigPaths())
skipIgnored := true
return n.CollectFromPaths(ctx, n.Name(), n.GetConfigPaths(), &skipIgnored)
}
3 changes: 2 additions & 1 deletion model/dotfile_ssh.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,6 @@ func (s *SshApp) GetConfigPaths() []string {
}

func (s *SshApp) CollectDotfiles(ctx context.Context) ([]DotfileItem, error) {
return s.CollectFromPaths(ctx, s.Name(), s.GetConfigPaths())
skipIgnored := true
return s.CollectFromPaths(ctx, s.Name(), s.GetConfigPaths(), &skipIgnored)
}
3 changes: 2 additions & 1 deletion model/dotfile_starship.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,5 +22,6 @@ func (s *StarshipApp) GetConfigPaths() []string {
}

func (s *StarshipApp) CollectDotfiles(ctx context.Context) ([]DotfileItem, error) {
return s.CollectFromPaths(ctx, s.Name(), s.GetConfigPaths())
skipIgnored := true
return s.CollectFromPaths(ctx, s.Name(), s.GetConfigPaths(), &skipIgnored)
}
3 changes: 2 additions & 1 deletion model/dotfile_zsh.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,6 @@ func (z *ZshApp) GetConfigPaths() []string {
}

func (z *ZshApp) CollectDotfiles(ctx context.Context) ([]DotfileItem, error) {
return z.CollectFromPaths(ctx, z.Name(), z.GetConfigPaths())
skipIgnored := true
return z.CollectFromPaths(ctx, z.Name(), z.GetConfigPaths(), &skipIgnored)
}