jcarr
/
gemini-cli
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

feat(git): Refactor gitignore handling to use `ignore` library instead of `minimatch` (#864)

This commit is contained in:
N. Taylor Mullen 2025-06-08 18:32:19 -07:00 committed by GitHub
parent d061419452
commit 72fa01f62d
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 65 additions and 238 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
package-lock.json generated
View File

@ -3100,6 +3100,7 @@
"version": "1.0.2", "version": "1.0.2",
"resolved": "https://registry.npmjs.org/balanced-match/-/balanced-match-1.0.2.tgz", "resolved": "https://registry.npmjs.org/balanced-match/-/balanced-match-1.0.2.tgz",
"integrity": "sha512-3oSeUO0TMV67hN1AmbXsK4yaqU7tjiHlbxRDZOpH0KW9+CeX4bRAaX0Anxt0tx2MrpRpWwQaPwIlISEJhYU5Pw==", "integrity": "sha512-3oSeUO0TMV67hN1AmbXsK4yaqU7tjiHlbxRDZOpH0KW9+CeX4bRAaX0Anxt0tx2MrpRpWwQaPwIlISEJhYU5Pw==",
"dev": true,
"license": "MIT" "license": "MIT"
}, },
"node_modules/base64-js": { "node_modules/base64-js": {
@ -10408,7 +10409,7 @@
"diff": "^7.0.0", "diff": "^7.0.0",
"dotenv": "^16.4.7", "dotenv": "^16.4.7",
"fast-glob": "^3.3.3", "fast-glob": "^3.3.3",
"minimatch": "^10.0.0", "ignore": "^7.0.0",
"shell-quote": "^1.8.2", "shell-quote": "^1.8.2",
"strip-ansi": "^7.1.0" "strip-ansi": "^7.1.0"
}, },
@ -10423,28 +10424,13 @@
"node": ">=18" "node": ">=18"
} }
}, },
"packages/core/node_modules/brace-expansion": { "packages/core/node_modules/ignore": {
"version": "2.0.1", "version": "7.0.5",
"resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-2.0.1.tgz", "resolved": "https://registry.npmjs.org/ignore/-/ignore-7.0.5.tgz",
"integrity": "sha512-XnAIvQ8eM+kC6aULx6wuQiwVsnzsi9d3WxzV3FpWTGA19F621kwdbsAcFKXgKUHZWsy+mY6iL1sHTxWEFCytDA==", "integrity": "sha512-Hs59xBNfUIunMFgWAbGX5cq6893IbWg4KnrjbYwX3tx0ztorVgTDA6B2sxf8ejHJ4wz8BqGUMYlnzNBer5NvGg==",
"license": "MIT", "license": "MIT",
"dependencies": {
"balanced-match": "^1.0.0"
}
},
"packages/core/node_modules/minimatch": {
"version": "10.0.1",
"resolved": "https://registry.npmjs.org/minimatch/-/minimatch-10.0.1.tgz",
"integrity": "sha512-ethXTt3SGGR+95gudmqJ1eNhRO7eGEGIgYA9vnPatK4/etz2MEVDno5GMCibdMTuBMyElzIlgxMna3K94XDIDQ==",
"license": "ISC",
"dependencies": {
"brace-expansion": "^2.0.1"
},
"engines": { "engines": {
"node": "20 || >=22" "node": ">= 4"
},
"funding": {
"url": "https://github.com/sponsors/isaacs"
} }
} }
} }

2
packages/core/package.json
View File

@ -26,7 +26,7 @@
"diff": "^7.0.0", "diff": "^7.0.0",
"dotenv": "^16.4.7", "dotenv": "^16.4.7",
"fast-glob": "^3.3.3", "fast-glob": "^3.3.3",
"minimatch": "^10.0.0", "ignore": "^7.0.0",
"shell-quote": "^1.8.2", "shell-quote": "^1.8.2",
"strip-ansi": "^7.1.0", "strip-ansi": "^7.1.0",
"@opentelemetry/api": "^1.9.0", "@opentelemetry/api": "^1.9.0",

27
packages/core/src/services/fileDiscoveryService.test.ts
View File

@ -140,33 +140,6 @@ describe('FileDiscoveryService', () => {
}); });
}); });
describe('getIgnoreInfo', () => {
beforeEach(async () => {
await service.initialize();
});
it('should return git ignored patterns', () => {
const info = service.getIgnoreInfo();
expect(info.gitIgnored).toEqual(['.git/**', 'node_modules/**']);
});
it('should return empty arrays when git ignore parser is not initialized', async () => {
const uninitializedService = new FileDiscoveryService(mockProjectRoot);
const info = uninitializedService.getIgnoreInfo();
expect(info.gitIgnored).toEqual([]);
});
it('should handle git ignore parser returning null patterns', async () => {
mockGitIgnoreParser.getIgnoredPatterns.mockReturnValue([] as string[]);
const info = service.getIgnoreInfo();
expect(info.gitIgnored).toEqual([]);
});
});
describe('isGitRepository', () => { describe('isGitRepository', () => {
it('should return true when isGitRepo is explicitly set to true in options', () => { it('should return true when isGitRepo is explicitly set to true in options', () => {
const result = service.isGitRepository({ isGitRepo: true }); const result = service.isGitRepository({ isGitRepo: true });

9
packages/core/src/services/fileDiscoveryService.ts
View File

@ -51,15 +51,6 @@ export class FileDiscoveryService {
}); });
} }
/**
* Gets patterns that would be ignored for debugging/transparency
*/
getIgnoreInfo(): { gitIgnored: string[] } {
return {
gitIgnored: this.gitIgnoreFilter?.getIgnoredPatterns() || [],
};
}
/** /**
* Checks if a single file should be ignored * Checks if a single file should be ignored
*/ */

160
packages/core/src/utils/gitIgnoreParser.test.ts
View File

@ -15,7 +15,6 @@ vi.mock('fs/promises');
// Mock gitUtils module // Mock gitUtils module
vi.mock('./gitUtils.js', () => ({ vi.mock('./gitUtils.js', () => ({
isGitRepository: vi.fn(() => true), isGitRepository: vi.fn(() => true),
findGitRoot: vi.fn(() => '/test/project'),
})); }));
describe('GitIgnoreParser', () => { describe('GitIgnoreParser', () => {
@ -24,7 +23,9 @@ describe('GitIgnoreParser', () => {
beforeEach(() => { beforeEach(() => {
parser = new GitIgnoreParser(mockProjectRoot); parser = new GitIgnoreParser(mockProjectRoot);
vi.clearAllMocks(); // Reset mocks before each test
vi.mocked(fs.readFile).mockClear();
vi.mocked(fs.readFile).mockRejectedValue(new Error('ENOENT')); // Default to no file
}); });
afterEach(() => { afterEach(() => {
@ -33,8 +34,6 @@ describe('GitIgnoreParser', () => {
describe('initialization', () => { describe('initialization', () => {
it('should initialize without errors when no .gitignore exists', async () => { it('should initialize without errors when no .gitignore exists', async () => {
vi.mocked(fs.readFile).mockRejectedValue(new Error('ENOENT'));
await expect(parser.initialize()).resolves.not.toThrow(); await expect(parser.initialize()).resolves.not.toThrow();
}); });
@ -43,106 +42,35 @@ describe('GitIgnoreParser', () => {
# Comment # Comment
node_modules/ node_modules/
*.log *.log
dist /dist
.env .env
`; `;
vi.mocked(fs.readFile).mockResolvedValue(gitignoreContent); vi.mocked(fs.readFile)
.mockResolvedValueOnce(gitignoreContent)
.mockRejectedValue(new Error('ENOENT'));
await parser.initialize(); await parser.initialize();
const patterns = parser.getIgnoredPatterns();
expect(patterns).toContain('.git/**'); expect(parser.isIgnored('node_modules/some-lib')).toBe(true);
expect(patterns).toContain('.git'); expect(parser.isIgnored('src/app.log')).toBe(true);
expect(patterns).toContain('node_modules/**'); expect(parser.isIgnored('dist/index.js')).toBe(true);
expect(patterns).toContain('**/*.log'); expect(parser.isIgnored('.env')).toBe(true);
expect(patterns).toContain('**/dist');
expect(patterns).toContain('**/.env');
}); });
it('should handle git exclude file', async () => { it('should handle git exclude file', async () => {
vi.mocked(fs.readFile).mockImplementation(async (filePath) => { vi.mocked(fs.readFile).mockImplementation(async (filePath) => {
if (filePath === path.join(mockProjectRoot, '.gitignore')) {
throw new Error('ENOENT');
}
if ( if (
filePath === path.join(mockProjectRoot, '.git', 'info', 'exclude') filePath === path.join(mockProjectRoot, '.git', 'info', 'exclude')
) { ) {
return 'temp/\n*.tmp'; return 'temp/\n*.tmp';
} }
throw new Error('Unexpected file'); throw new Error('ENOENT');
}); });
await parser.initialize(); await parser.initialize();
const patterns = parser.getIgnoredPatterns();
expect(patterns).toContain('temp/**'); expect(parser.isIgnored('temp/file.txt')).toBe(true);
expect(patterns).toContain('**/*.tmp'); expect(parser.isIgnored('src/file.tmp')).toBe(true);
});
});
describe('pattern parsing', () => {
it('should handle directory patterns correctly', async () => {
const gitignoreContent = 'node_modules/\nbuild/\n';
vi.mocked(fs.readFile).mockResolvedValue(gitignoreContent);
await parser.initialize();
const patterns = parser.getIgnoredPatterns();
expect(patterns).toContain('node_modules/**');
expect(patterns).toContain('build/**');
});
it('should handle file patterns correctly', async () => {
const gitignoreContent = '*.log\n.env\nconfig.json\n';
vi.mocked(fs.readFile).mockResolvedValue(gitignoreContent);
await parser.initialize();
const patterns = parser.getIgnoredPatterns();
expect(patterns).toContain('**/*.log');
expect(patterns).toContain('**/.env');
expect(patterns).toContain('**/config.json');
});
it('should skip comments and empty lines', async () => {
const gitignoreContent = `
# This is a comment
*.log
# Another comment
.env
`;
vi.mocked(fs.readFile).mockResolvedValue(gitignoreContent);
await parser.initialize();
const patterns = parser.getIgnoredPatterns();
expect(patterns).not.toContain('# This is a comment');
expect(patterns).not.toContain('# Another comment');
expect(patterns).toContain('**/*.log');
expect(patterns).toContain('**/.env');
});
it('should skip negation patterns for now', async () => {
const gitignoreContent = '*.log\n!important.log\n';
vi.mocked(fs.readFile).mockResolvedValue(gitignoreContent);
await parser.initialize();
const patterns = parser.getIgnoredPatterns();
expect(patterns).toContain('**/*.log');
expect(patterns).not.toContain('!important.log');
});
it('should handle paths with slashes correctly', async () => {
const gitignoreContent = 'src/*.log\ndocs/temp/\n';
vi.mocked(fs.readFile).mockResolvedValue(gitignoreContent);
await parser.initialize();
const patterns = parser.getIgnoredPatterns();
expect(patterns).toContain('src/*.log');
expect(patterns).toContain('docs/temp/**');
}); });
}); });
@ -151,28 +79,32 @@ dist
const gitignoreContent = ` const gitignoreContent = `
node_modules/ node_modules/
*.log *.log
dist /dist
.env /.env
src/*.tmp src/*.tmp
!src/important.tmp
`; `;
vi.mocked(fs.readFile).mockResolvedValue(gitignoreContent); vi.mocked(fs.readFile)
.mockResolvedValueOnce(gitignoreContent)
.mockRejectedValue(new Error('ENOENT'));
await parser.initialize(); await parser.initialize();
}); });
it('should always ignore .git directory', () => { it('should always ignore .git directory', () => {
expect(parser.isIgnored('.git')).toBe(true); expect(parser.isIgnored('.git')).toBe(true);
expect(parser.isIgnored('.git/config')).toBe(true); expect(parser.isIgnored('.git/config')).toBe(true);
expect(parser.isIgnored('.git/objects/abc123')).toBe(true); expect(parser.isIgnored(path.join(mockProjectRoot, '.git', 'HEAD'))).toBe(
true,
);
}); });
it('should ignore files matching patterns', () => { it('should ignore files matching patterns', () => {
expect(parser.isIgnored('node_modules')).toBe(true); expect(parser.isIgnored('node_modules/package/index.js')).toBe(true);
expect(parser.isIgnored('node_modules/package')).toBe(true);
expect(parser.isIgnored('app.log')).toBe(true); expect(parser.isIgnored('app.log')).toBe(true);
expect(parser.isIgnored('logs/app.log')).toBe(true); expect(parser.isIgnored('logs/app.log')).toBe(true);
expect(parser.isIgnored('dist')).toBe(true); expect(parser.isIgnored('dist/bundle.js')).toBe(true);
expect(parser.isIgnored('.env')).toBe(true); expect(parser.isIgnored('.env')).toBe(true);
expect(parser.isIgnored('config/.env')).toBe(true); expect(parser.isIgnored('config/.env')).toBe(false); // .env is anchored to root
}); });
it('should ignore files with path-specific patterns', () => { it('should ignore files with path-specific patterns', () => {
@ -180,29 +112,28 @@ src/*.tmp
expect(parser.isIgnored('other/temp.tmp')).toBe(false); expect(parser.isIgnored('other/temp.tmp')).toBe(false);
}); });
it('should handle negation patterns', () => {
expect(parser.isIgnored('src/important.tmp')).toBe(false);
});
it('should not ignore files that do not match patterns', () => { it('should not ignore files that do not match patterns', () => {
expect(parser.isIgnored('src/index.ts')).toBe(false); expect(parser.isIgnored('src/index.ts')).toBe(false);
expect(parser.isIgnored('README.md')).toBe(false); expect(parser.isIgnored('README.md')).toBe(false);
expect(parser.isIgnored('package.json')).toBe(false);
}); });
it('should handle absolute paths correctly', () => { it('should handle absolute paths correctly', () => {
const absolutePath = path.join( const absolutePath = path.join(mockProjectRoot, 'node_modules', 'lib');
mockProjectRoot,
'node_modules',
'package',
);
expect(parser.isIgnored(absolutePath)).toBe(true); expect(parser.isIgnored(absolutePath)).toBe(true);
}); });
it('should handle paths outside project root', () => { it('should handle paths outside project root by not ignoring them', () => {
const outsidePath = '/other/project/file.txt'; const outsidePath = path.resolve(mockProjectRoot, '../other/file.txt');
expect(parser.isIgnored(outsidePath)).toBe(false); expect(parser.isIgnored(outsidePath)).toBe(false);
}); });
it('should handle relative paths correctly', () => { it('should handle relative paths correctly', () => {
expect(parser.isIgnored('./node_modules')).toBe(true); expect(parser.isIgnored('node_modules/some-package')).toBe(true);
expect(parser.isIgnored('../file.txt')).toBe(false); expect(parser.isIgnored('../some/other/file.txt')).toBe(false);
}); });
it('should normalize path separators on Windows', () => { it('should normalize path separators on Windows', () => {
@ -212,26 +143,11 @@ src/*.tmp
}); });
describe('getIgnoredPatterns', () => { describe('getIgnoredPatterns', () => {
it('should return a copy of patterns array', async () => { it('should return the raw patterns added', async () => {
const gitignoreContent = '*.log\n'; const gitignoreContent = '*.log\n!important.log';
vi.mocked(fs.readFile).mockResolvedValue(gitignoreContent); vi.mocked(fs.readFile).mockResolvedValueOnce(gitignoreContent);
await parser.initialize(); await parser.initialize();
const patterns1 = parser.getIgnoredPatterns();
const patterns2 = parser.getIgnoredPatterns();
expect(patterns1).not.toBe(patterns2); // Different array instances
expect(patterns1).toEqual(patterns2); // Same content
});
it('should always include .git patterns', async () => {
vi.mocked(fs.readFile).mockRejectedValue(new Error('ENOENT'));
await parser.initialize();
const patterns = parser.getIgnoredPatterns();
expect(patterns).toContain('.git/**');
expect(patterns).toContain('.git');
}); });
}); });
}); });

77
packages/core/src/utils/gitIgnoreParser.ts
View File

@ -6,18 +6,17 @@
import * as fs from 'fs/promises'; import * as fs from 'fs/promises';
import * as path from 'path'; import * as path from 'path';
import { minimatch } from 'minimatch'; import ignore, { Ignore } from 'ignore';
import { isGitRepository } from './gitUtils.js'; import { isGitRepository } from './gitUtils.js';
export interface GitIgnoreFilter { export interface GitIgnoreFilter {
isIgnored(filePath: string): boolean; isIgnored(filePath: string): boolean;
getIgnoredPatterns(): string[];
} }
export class GitIgnoreParser implements GitIgnoreFilter { export class GitIgnoreParser implements GitIgnoreFilter {
private ignorePatterns: string[] = [];
private projectRoot: string; private projectRoot: string;
private isGitRepo: boolean = false; private isGitRepo: boolean = false;
private ig: Ignore = ignore();
constructor(projectRoot: string) { constructor(projectRoot: string) {
this.projectRoot = path.resolve(projectRoot); this.projectRoot = path.resolve(projectRoot);
@ -32,13 +31,13 @@ export class GitIgnoreParser implements GitIgnoreFilter {
]; ];
// Always ignore .git directory regardless of .gitignore content // Always ignore .git directory regardless of .gitignore content
this.ignorePatterns = ['.git/**', '.git']; this.addPatterns(['.git']);
for (const gitIgnoreFile of gitIgnoreFiles) { for (const gitIgnoreFile of gitIgnoreFiles) {
try { try {
const content = await fs.readFile(gitIgnoreFile, 'utf-8'); const content = await fs.readFile(gitIgnoreFile, 'utf-8');
const patterns = this.parseGitIgnoreContent(content); const patterns = content.split('\n').map((p) => p.trim());
this.ignorePatterns.push(...patterns); this.addPatterns(patterns);
} catch (_error) { } catch (_error) {
// File doesn't exist or can't be read, continue silently // File doesn't exist or can't be read, continue silently
} }
@ -46,71 +45,33 @@ export class GitIgnoreParser implements GitIgnoreFilter {
} }
} }
private parseGitIgnoreContent(content: string): string[] { private addPatterns(patterns: string[]) {
return content this.ig.add(patterns);
.split('\n')
.map((line) => line.trim())
.filter((line) => line && !line.startsWith('#'))
.map((pattern) => {
// Handle negation patterns (!) - for now we'll skip them
if (pattern.startsWith('!')) {
return null;
}
// Convert gitignore patterns to minimatch-compatible patterns
if (pattern.endsWith('/')) {
// Directory pattern - match directory and all contents
const dirPattern = pattern.slice(0, -1); // Remove trailing slash
return [dirPattern, dirPattern + '/**'];
}
// If pattern doesn't contain /, it should match at any level
if (!pattern.includes('/') && !pattern.startsWith('**/')) {
return '**/' + pattern;
}
return pattern;
})
.filter((pattern) => pattern !== null)
.flat() as string[];
} }
isIgnored(filePath: string): boolean { isIgnored(filePath: string): boolean {
// If not a git repository, nothing is ignored
if (!this.isGitRepo) { if (!this.isGitRepo) {
return false; return false;
} }
// Normalize the input path (handle ./ prefixes) const relativePath = path.isAbsolute(filePath)
let cleanPath = filePath; ? path.relative(this.projectRoot, filePath)
if (cleanPath.startsWith('./')) { : filePath;
cleanPath = cleanPath.slice(2);
}
// Convert to relative path from project root if (relativePath === '' || relativePath.startsWith('..')) {
const relativePath = path.relative(
this.projectRoot,
path.resolve(this.projectRoot, cleanPath),
);
// Handle paths that go outside project root
if (relativePath.startsWith('..')) {
return false; return false;
} }
// Normalize path separators for cross-platform compatibility let normalizedPath = relativePath.replace(/\\/g, '/');
const normalizedPath = relativePath.replace(/\\/g, '/'); if (normalizedPath.startsWith('./')) {
normalizedPath = normalizedPath.substring(2);
}
return this.ignorePatterns.some((pattern) => const ignored = this.ig.ignores(normalizedPath);
minimatch(normalizedPath, pattern, { return ignored;
dot: true,
matchBase: false,
flipNegate: false,
}),
);
} }
getIgnoredPatterns(): string[] { getGitRepoRoot(): string {
return [...this.ignorePatterns]; return this.projectRoot;
} }
} }