mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-04-13 23:01:09 -07:00
refactor(core): improve ignore resolution and fix directory-matching bug (#23816)
This commit is contained in:
@@ -221,7 +221,7 @@ describe('FileDiscoveryService', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('shouldGitIgnoreFile & shouldGeminiIgnoreFile', () => {
|
||||
describe('shouldIgnoreFile & shouldIgnoreDirectory', () => {
|
||||
beforeEach(async () => {
|
||||
await fs.mkdir(path.join(projectRoot, '.git'));
|
||||
await createTestFile('.gitignore', 'node_modules/');
|
||||
@@ -238,6 +238,13 @@ describe('FileDiscoveryService', () => {
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it('should return true for git-ignored directories', () => {
|
||||
const service = new FileDiscoveryService(projectRoot);
|
||||
expect(
|
||||
service.shouldIgnoreDirectory(path.join(projectRoot, 'node_modules')),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it('should return false for non-git-ignored files', () => {
|
||||
const service = new FileDiscoveryService(projectRoot);
|
||||
|
||||
@@ -293,6 +300,7 @@ describe('FileDiscoveryService', () => {
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('precedence (.geminiignore over .gitignore)', () => {
|
||||
beforeEach(async () => {
|
||||
await fs.mkdir(path.join(projectRoot, '.git'));
|
||||
@@ -495,4 +503,99 @@ describe('FileDiscoveryService', () => {
|
||||
expect(paths[0]).toBe(path.join(projectRoot, '.gitignore'));
|
||||
});
|
||||
});
|
||||
|
||||
describe('getIgnoredPaths', () => {
|
||||
beforeEach(async () => {
|
||||
await fs.mkdir(path.join(projectRoot, '.git'));
|
||||
});
|
||||
|
||||
it('should return all ignored paths that exist on disk', async () => {
|
||||
await createTestFile(
|
||||
'.gitignore',
|
||||
'ignored-dir/\nignored-file.txt\n*.log',
|
||||
);
|
||||
await createTestFile('ignored-dir/inside.txt');
|
||||
await createTestFile('ignored-file.txt');
|
||||
await createTestFile('keep.log');
|
||||
await createTestFile('src/index.ts');
|
||||
await createTestFile(GEMINI_IGNORE_FILE_NAME, 'secrets/');
|
||||
await createTestFile('secrets/passwords.txt');
|
||||
|
||||
const service = new FileDiscoveryService(projectRoot);
|
||||
const ignoredPaths = await service.getIgnoredPaths();
|
||||
|
||||
const expectedPaths = [
|
||||
path.join(projectRoot, '.git'),
|
||||
path.join(projectRoot, 'ignored-dir'),
|
||||
path.join(projectRoot, 'ignored-file.txt'),
|
||||
path.join(projectRoot, 'keep.log'),
|
||||
path.join(projectRoot, 'secrets'),
|
||||
].sort();
|
||||
|
||||
expect(ignoredPaths.sort()).toEqual(expectedPaths);
|
||||
});
|
||||
|
||||
it('should optimize by not traversing into ignored directories', async () => {
|
||||
await createTestFile('.gitignore', 'ignored-dir/');
|
||||
const ignoredDir = path.join(projectRoot, 'ignored-dir');
|
||||
await fs.mkdir(ignoredDir);
|
||||
await createTestFile('ignored-dir/large-file-1.txt');
|
||||
|
||||
const service = new FileDiscoveryService(projectRoot);
|
||||
const ignoredPaths = await service.getIgnoredPaths();
|
||||
|
||||
expect(ignoredPaths.sort()).toEqual(
|
||||
[path.join(projectRoot, '.git'), ignoredDir].sort(),
|
||||
);
|
||||
});
|
||||
|
||||
it('should handle un-ignore patterns correctly', async () => {
|
||||
await createTestFile(
|
||||
'.gitignore',
|
||||
'ignored-dir/*\n!ignored-dir/keep.txt',
|
||||
);
|
||||
await createTestFile('ignored-dir/ignored.txt');
|
||||
await createTestFile('ignored-dir/keep.txt');
|
||||
|
||||
const service = new FileDiscoveryService(projectRoot);
|
||||
const ignoredPaths = await service.getIgnoredPaths();
|
||||
|
||||
expect(ignoredPaths).toContain(
|
||||
path.join(projectRoot, 'ignored-dir/ignored.txt'),
|
||||
);
|
||||
expect(ignoredPaths).not.toContain(
|
||||
path.join(projectRoot, 'ignored-dir/keep.txt'),
|
||||
);
|
||||
expect(ignoredPaths).not.toContain(path.join(projectRoot, 'ignored-dir'));
|
||||
});
|
||||
|
||||
it('should respect FilterFilesOptions when provided', async () => {
|
||||
await createTestFile('.gitignore', 'ignored-by-git.txt');
|
||||
await createTestFile(GEMINI_IGNORE_FILE_NAME, 'ignored-by-gemini.txt');
|
||||
await createTestFile('ignored-by-git.txt');
|
||||
await createTestFile('ignored-by-gemini.txt');
|
||||
|
||||
const service = new FileDiscoveryService(projectRoot);
|
||||
|
||||
const onlyGemini = await service.getIgnoredPaths({
|
||||
respectGitIgnore: false,
|
||||
respectGeminiIgnore: true,
|
||||
});
|
||||
expect(onlyGemini).toContain(
|
||||
path.join(projectRoot, 'ignored-by-gemini.txt'),
|
||||
);
|
||||
expect(onlyGemini).not.toContain(
|
||||
path.join(projectRoot, 'ignored-by-git.txt'),
|
||||
);
|
||||
|
||||
const onlyGit = await service.getIgnoredPaths({
|
||||
respectGitIgnore: true,
|
||||
respectGeminiIgnore: false,
|
||||
});
|
||||
expect(onlyGit).toContain(path.join(projectRoot, 'ignored-by-git.txt'));
|
||||
expect(onlyGit).not.toContain(
|
||||
path.join(projectRoot, 'ignored-by-gemini.txt'),
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -14,6 +14,8 @@ import {
|
||||
} from '../utils/ignoreFileParser.js';
|
||||
import { isGitRepository } from '../utils/gitUtils.js';
|
||||
import { GEMINI_IGNORE_FILE_NAME } from '../config/constants.js';
|
||||
import { isNodeError } from '../utils/errors.js';
|
||||
import { debugLogger } from '../utils/debugLogger.js';
|
||||
import fs from 'node:fs';
|
||||
import * as path from 'node:path';
|
||||
|
||||
@@ -83,6 +85,60 @@ export class FileDiscoveryService {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns all absolute paths (files and directories) within the project root that should be ignored.
|
||||
*/
|
||||
async getIgnoredPaths(options: FilterFilesOptions = {}): Promise<string[]> {
|
||||
const ignoredPaths: string[] = [];
|
||||
|
||||
/**
|
||||
* Recursively walks the directory tree to find ignored paths.
|
||||
*/
|
||||
const walk = async (currentDir: string) => {
|
||||
let dirEntries: fs.Dirent[];
|
||||
try {
|
||||
dirEntries = await fs.promises.readdir(currentDir, {
|
||||
withFileTypes: true,
|
||||
});
|
||||
} catch (error: unknown) {
|
||||
if (
|
||||
isNodeError(error) &&
|
||||
(error.code === 'EACCES' || error.code === 'ENOENT')
|
||||
) {
|
||||
// Stop if the directory is inaccessible or doesn't exist
|
||||
debugLogger.debug(
|
||||
`Skipping directory ${currentDir} due to ${error.code}`,
|
||||
);
|
||||
return;
|
||||
}
|
||||
throw error;
|
||||
}
|
||||
|
||||
// Traverse sibling directories concurrently to improve performance.
|
||||
await Promise.all(
|
||||
dirEntries.map(async (entry) => {
|
||||
const fullPath = path.join(currentDir, entry.name);
|
||||
|
||||
if (entry.isDirectory()) {
|
||||
// Optimization: If a directory is ignored, its contents are not traversed.
|
||||
if (this.shouldIgnoreDirectory(fullPath, options)) {
|
||||
ignoredPaths.push(fullPath);
|
||||
} else {
|
||||
await walk(fullPath);
|
||||
}
|
||||
} else {
|
||||
if (this.shouldIgnoreFile(fullPath, options)) {
|
||||
ignoredPaths.push(fullPath);
|
||||
}
|
||||
}
|
||||
}),
|
||||
);
|
||||
};
|
||||
|
||||
await walk(this.projectRoot);
|
||||
return ignoredPaths;
|
||||
}
|
||||
|
||||
private applyFilterFilesOptions(options?: FilterFilesOptions): void {
|
||||
if (!options) return;
|
||||
|
||||
@@ -100,34 +156,16 @@ export class FileDiscoveryService {
|
||||
}
|
||||
|
||||
/**
|
||||
* Filters a list of file paths based on ignore rules
|
||||
* Filters a list of file paths based on ignore rules.
|
||||
*
|
||||
* NOTE: Directory paths must include a trailing slash to be correctly identified and
|
||||
* matched against directory-specific ignore patterns (e.g., 'dist/').
|
||||
*/
|
||||
filterFiles(filePaths: string[], options: FilterFilesOptions = {}): string[] {
|
||||
const {
|
||||
respectGitIgnore = this.defaultFilterFileOptions.respectGitIgnore,
|
||||
respectGeminiIgnore = this.defaultFilterFileOptions.respectGeminiIgnore,
|
||||
} = options;
|
||||
return filePaths.filter((filePath) => {
|
||||
if (
|
||||
respectGitIgnore &&
|
||||
respectGeminiIgnore &&
|
||||
this.combinedIgnoreFilter
|
||||
) {
|
||||
return !this.combinedIgnoreFilter.isIgnored(filePath);
|
||||
}
|
||||
|
||||
// Always respect custom ignore filter if provided
|
||||
if (this.customIgnoreFilter?.isIgnored(filePath)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
if (respectGitIgnore && this.gitIgnoreFilter?.isIgnored(filePath)) {
|
||||
return false;
|
||||
}
|
||||
if (respectGeminiIgnore && this.geminiIgnoreFilter?.isIgnored(filePath)) {
|
||||
return false;
|
||||
}
|
||||
return true;
|
||||
// Infer directory status from the string format
|
||||
const isDir = filePath.endsWith('/') || filePath.endsWith('\\');
|
||||
return !this._shouldIgnore(filePath, isDir, options);
|
||||
});
|
||||
}
|
||||
|
||||
@@ -152,13 +190,61 @@ export class FileDiscoveryService {
|
||||
}
|
||||
|
||||
/**
|
||||
* Unified method to check if a file should be ignored based on filtering options
|
||||
* Checks if a specific file should be ignored based on project ignore rules.
|
||||
*/
|
||||
shouldIgnoreFile(
|
||||
filePath: string,
|
||||
options: FilterFilesOptions = {},
|
||||
): boolean {
|
||||
return this.filterFiles([filePath], options).length === 0;
|
||||
return this._shouldIgnore(filePath, false, options);
|
||||
}
|
||||
|
||||
/**
|
||||
* Checks if a specific directory should be ignored based on project ignore rules.
|
||||
*/
|
||||
shouldIgnoreDirectory(
|
||||
dirPath: string,
|
||||
options: FilterFilesOptions = {},
|
||||
): boolean {
|
||||
return this._shouldIgnore(dirPath, true, options);
|
||||
}
|
||||
|
||||
/**
|
||||
* Internal unified check for paths.
|
||||
*/
|
||||
private _shouldIgnore(
|
||||
filePath: string,
|
||||
isDirectory: boolean,
|
||||
options: FilterFilesOptions = {},
|
||||
): boolean {
|
||||
const {
|
||||
respectGitIgnore = this.defaultFilterFileOptions.respectGitIgnore,
|
||||
respectGeminiIgnore = this.defaultFilterFileOptions.respectGeminiIgnore,
|
||||
} = options;
|
||||
|
||||
if (respectGitIgnore && respectGeminiIgnore && this.combinedIgnoreFilter) {
|
||||
return this.combinedIgnoreFilter.isIgnored(filePath, isDirectory);
|
||||
}
|
||||
|
||||
if (this.customIgnoreFilter?.isIgnored(filePath, isDirectory)) {
|
||||
return true;
|
||||
}
|
||||
|
||||
if (
|
||||
respectGitIgnore &&
|
||||
this.gitIgnoreFilter?.isIgnored(filePath, isDirectory)
|
||||
) {
|
||||
return true;
|
||||
}
|
||||
|
||||
if (
|
||||
respectGeminiIgnore &&
|
||||
this.geminiIgnoreFilter?.isIgnored(filePath, isDirectory)
|
||||
) {
|
||||
return true;
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
Reference in New Issue
Block a user