-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
feat(core): Add .ignore file support #938
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
bb7fae2
8365478
44d172b
263cf48
438b79f
52e50e2
2b5df06
d2ed827
e003ec6
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 |
|---|---|---|
|
|
@@ -127,6 +127,10 @@ export const buildCliConfig = (options: CliOptions): RepomixConfigCli => { | |
| if (options.gitignore === false) { | ||
| cliConfig.ignore = { ...cliConfig.ignore, useGitignore: options.gitignore }; | ||
| } | ||
| // Only apply dotIgnore setting if explicitly set to false | ||
| if (options.dotIgnore === false) { | ||
|
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. Good: Consistent with existing patterns This implementation correctly mirrors the handling of Minor suggestion: Consider adding a comment explaining this behavior for future maintainers: // Only apply dotIgnore setting if explicitly set to false
// This allows config file values to take precedence when the flag is not specified
if (options.dotIgnore === false) {
cliConfig.ignore = { ...cliConfig.ignore, useDotIgnore: options.dotIgnore };
} |
||
| cliConfig.ignore = { ...cliConfig.ignore, useDotIgnore: options.dotIgnore }; | ||
| } | ||
| // Only apply defaultPatterns setting if explicitly set to false | ||
| if (options.defaultPatterns === false) { | ||
| cliConfig.ignore = { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -295,10 +295,21 @@ export const parseIgnoreContent = (content: string): string[] => { | |
| export const getIgnoreFilePatterns = async (config: RepomixConfigMerged): Promise<string[]> => { | ||
| const ignoreFilePatterns: string[] = []; | ||
|
|
||
| // Note: When ignore files are found in nested directories, files in deeper | ||
| // directories have higher priority, following the behavior of ripgrep and fd. | ||
| // For example, `src/.ignore` patterns override `./.ignore` patterns. | ||
| // | ||
| // Multiple ignore files in the same directory (.gitignore, .ignore, .repomixignore) | ||
| // are all merged together. The order in this array does not affect priority. | ||
|
|
||
| if (config.ignore.useGitignore) { | ||
| ignoreFilePatterns.push('**/.gitignore'); | ||
| } | ||
|
|
||
| if (config.ignore.useDotIgnore) { | ||
| ignoreFilePatterns.push('**/.ignore'); | ||
|
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 array order here determines how globby processes ignore files. Please verify that this order matches the documented priority in README.md: Documented priority (highest to lowest):
Current implementation adds:
If globby processes Please verify and add a comment explaining the order, e.g.: // Order matters: globby processes these files in order,
// with later files overriding earlier ones |
||
| } | ||
|
|
||
| ignoreFilePatterns.push('**/.repomixignore'); | ||
|
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. Critical Issue: Incorrect Priority Order The current implementation adds
Why this is a problem: Actually, I need to verify this: Looking at globby's behavior, ignore files are typically additive (patterns accumulate), not override-based. The priority should actually be controlled by the order patterns are added to the main The Real Issue: However, there's still a documentation inconsistency: The code adds Recommendation: // Add in reverse priority order (highest priority last for clarity)
if (config.ignore.useGitignore) {
ignoreFilePatterns.push('**/.gitignore');
}
if (config.ignore.useDotIgnore) {
ignoreFilePatterns.push('**/.ignore');
}
ignoreFilePatterns.push('**/.repomixignore');This way, the code visually reflects the intended priority order even though globby treats all ignoreFiles equally by reading and merging their patterns.
yamadashy marked this conversation as resolved.
Show resolved
Hide resolved
yamadashy marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| return ignoreFilePatterns; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -53,30 +53,46 @@ describe('fileSearch', () => { | |
| }); | ||
|
|
||
| describe('getIgnoreFilePaths', () => { | ||
| test('should return correct paths when .gitignore and .repomixignore exist', async () => { | ||
| test('should return correct paths when .gitignore, .ignore and .repomixignore exist', async () => { | ||
yamadashy marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| vi.mocked(fs.access).mockResolvedValue(undefined); | ||
| const mockConfig = createMockConfig({ | ||
| ignore: { | ||
| useGitignore: true, | ||
| useDotIgnore: true, | ||
| useDefaultPatterns: true, | ||
| customPatterns: [], | ||
| }, | ||
| }); | ||
| const filePatterns = await getIgnoreFilePatterns(mockConfig); | ||
| expect(filePatterns).toEqual(['**/.gitignore', '**/.repomixignore']); | ||
| expect(filePatterns).toEqual(['**/.gitignore', '**/.ignore', '**/.repomixignore']); | ||
|
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. Test Coverage Issue: Order Matters While this test verifies that all three ignore file patterns are included, it should also verify the order to ensure the priority is correct. Consider updating to: expect(filePatterns).toEqual(['**/.gitignore', '**/.ignore', '**/.repomixignore']);
// Or better yet, add a comment explaining why order matters:
// Order should reflect priority: .gitignore (lowest) -> .ignore -> .repomixignore (highest)However, I need to verify if globby's |
||
| }); | ||
|
|
||
| test('should not include .gitignore when useGitignore is false', async () => { | ||
| vi.mocked(fs.access).mockResolvedValue(undefined); | ||
| const mockConfig = createMockConfig({ | ||
| ignore: { | ||
| useGitignore: false, | ||
| useDotIgnore: true, | ||
| useDefaultPatterns: true, | ||
| customPatterns: [], | ||
| }, | ||
| }); | ||
| const filePatterns = await getIgnoreFilePatterns(mockConfig); | ||
| expect(filePatterns).toEqual(['**/.repomixignore']); | ||
| expect(filePatterns).toEqual(['**/.ignore', '**/.repomixignore']); | ||
| }); | ||
|
|
||
| test('should not include .ignore when useDotIgnore is false', async () => { | ||
| vi.mocked(fs.access).mockResolvedValue(undefined); | ||
| const mockConfig = createMockConfig({ | ||
| ignore: { | ||
| useGitignore: true, | ||
| useDotIgnore: false, | ||
| useDefaultPatterns: true, | ||
| customPatterns: [], | ||
| }, | ||
| }); | ||
| const filePatterns = await getIgnoreFilePatterns(mockConfig); | ||
| expect(filePatterns).toEqual(['**/.gitignore', '**/.repomixignore']); | ||
| }); | ||
|
|
||
| test('should handle empty directories when enabled', async () => { | ||
|
|
||
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.
Documentation Enhancement Suggestion
Great addition of
.ignorefile support documentation! Consider adding a note about compatibility/precedence when files conflict:This clarifies what happens when the same pattern appears in multiple files.