-
Notifications
You must be signed in to change notification settings - Fork 0
feat(dotfile): add support for ignoring sections in collected files #103
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 |
|---|---|---|
|
|
@@ -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 | ||
|
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. |
||
| dotfiles, err := app.CollectFromPaths(ctx, "testapp", []string{configFile}, &skipIgnored) | ||
| require.NoError(t, err) | ||
| assert.Len(t, dotfiles, 1) | ||
|
|
||
|
|
@@ -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) | ||
|
|
@@ -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
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 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 expectedContent := "line1\nline2\nvisible_key=value"
assert.Equal(t, expectedContent, dotfiles[0].Content)Similarly, for the |
||
|
|
||
| // 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) { | ||
|
|
@@ -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) | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -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
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. This implementation can be simplified. The This simplification applies to all other
Suggested change
|
||||||||
| } | ||||||||
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 implementation of
filterIgnoredSectionscan be improved for readability and efficiency:"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.filteredLinesto avoid multiple re-allocations as the slice grows. This is a good practice for performance, especially with larger files.