From bb67d317394ba4be7b55bdc7175c5d432a40ae53 Mon Sep 17 00:00:00 2001 From: "Anas H. Sulaiman" Date: Fri, 13 Jun 2025 17:17:08 -0400 Subject: [PATCH] reuse `GitIgnoreParser` for loading `.geminiignore` (#1025) --- packages/cli/src/gemini.tsx | 2 +- .../cli/src/utils/loadIgnorePatterns.test.ts | 93 ++----------------- packages/cli/src/utils/loadIgnorePatterns.ts | 42 +++------ .../core/src/utils/gitIgnoreParser.test.ts | 28 +++++- packages/core/src/utils/gitIgnoreParser.ts | 53 ++++++----- 5 files changed, 82 insertions(+), 136 deletions(-) diff --git a/packages/cli/src/gemini.tsx b/packages/cli/src/gemini.tsx index 35c94214..6cd246db 100644 --- a/packages/cli/src/gemini.tsx +++ b/packages/cli/src/gemini.tsx @@ -41,7 +41,7 @@ export async function main() { const settings = loadSettings(workspaceRoot); setWindowTitle(basename(workspaceRoot), settings); - const geminiIgnorePatterns = loadGeminiIgnorePatterns(workspaceRoot); + const geminiIgnorePatterns = await loadGeminiIgnorePatterns(workspaceRoot); await cleanupCheckpoints(); if (settings.errors.length > 0) { for (const error of settings.errors) { diff --git a/packages/cli/src/utils/loadIgnorePatterns.test.ts b/packages/cli/src/utils/loadIgnorePatterns.test.ts index 9bcddf34..5ff89c4d 100644 --- a/packages/cli/src/utils/loadIgnorePatterns.test.ts +++ b/packages/cli/src/utils/loadIgnorePatterns.test.ts @@ -42,9 +42,6 @@ describe('loadGeminiIgnorePatterns', () => { let consoleLogSpy: Mock< (message?: unknown, ...optionalParams: unknown[]) => void >; - let consoleWarnSpy: Mock< - (message?: unknown, ...optionalParams: unknown[]) => void - >; beforeAll(async () => { actualFs = await vi.importActual('node:fs'); @@ -62,11 +59,6 @@ describe('loadGeminiIgnorePatterns', () => { .mockImplementation(() => {}) as Mock< (message?: unknown, ...optionalParams: unknown[]) => void >; - consoleWarnSpy = vi - .spyOn(console, 'warn') - .mockImplementation(() => {}) as Mock< - (message?: unknown, ...optionalParams: unknown[]) => void - >; mockedFsReadFileSync.mockReset(); }); @@ -77,7 +69,7 @@ describe('loadGeminiIgnorePatterns', () => { vi.restoreAllMocks(); }); - it('should load and parse patterns from .geminiignore, ignoring comments and empty lines', () => { + it('should load and parse patterns from .geminiignore, ignoring comments and empty lines', async () => { const ignoreContent = [ '# This is a comment', 'pattern1', @@ -90,14 +82,7 @@ describe('loadGeminiIgnorePatterns', () => { const ignoreFilePath = path.join(tempDir, '.geminiignore'); actualFs.writeFileSync(ignoreFilePath, ignoreContent); - mockedFsReadFileSync.mockImplementation((p: string, encoding: string) => { - if (p === ignoreFilePath && encoding === 'utf-8') return ignoreContent; - throw new Error( - `Mock fs.readFileSync: Unexpected call with path: ${p}, encoding: ${encoding}`, - ); - }); - - const patterns = loadGeminiIgnorePatterns(tempDir); + const patterns = await loadGeminiIgnorePatterns(tempDir); expect(patterns).toEqual([ 'pattern1', @@ -109,39 +94,19 @@ describe('loadGeminiIgnorePatterns', () => { expect(consoleLogSpy).toHaveBeenCalledWith( expect.stringContaining('Loaded 5 patterns from .geminiignore'), ); - expect(mockedFsReadFileSync).toHaveBeenCalledWith(ignoreFilePath, 'utf-8'); }); - it('should return an empty array and log info if .geminiignore is not found', () => { - const ignoreFilePath = path.join(tempDir, '.geminiignore'); - mockedFsReadFileSync.mockImplementation((p: string, encoding: string) => { - if (p === ignoreFilePath && encoding === 'utf-8') { - const error = new Error('File not found') as NodeJS.ErrnoException; - error.code = 'ENOENT'; - throw error; - } - throw new Error( - `Mock fs.readFileSync: Unexpected call with path: ${p}, encoding: ${encoding}`, - ); - }); - - const patterns = loadGeminiIgnorePatterns(tempDir); + it('should return an empty array and log info if .geminiignore is not found', async () => { + const patterns = await loadGeminiIgnorePatterns(tempDir); expect(patterns).toEqual([]); expect(consoleLogSpy).not.toHaveBeenCalled(); - expect(mockedFsReadFileSync).toHaveBeenCalledWith(ignoreFilePath, 'utf-8'); }); - it('should return an empty array if .geminiignore is empty', () => { + it('should return an empty array if .geminiignore is empty', async () => { const ignoreFilePath = path.join(tempDir, '.geminiignore'); actualFs.writeFileSync(ignoreFilePath, ''); - mockedFsReadFileSync.mockImplementation((p: string, encoding: string) => { - if (p === ignoreFilePath && encoding === 'utf-8') return ''; // Return string for empty file - throw new Error( - `Mock fs.readFileSync: Unexpected call with path: ${p}, encoding: ${encoding}`, - ); - }); - const patterns = loadGeminiIgnorePatterns(tempDir); + const patterns = await loadGeminiIgnorePatterns(tempDir); expect(patterns).toEqual([]); expect(consoleLogSpy).not.toHaveBeenCalledWith( expect.stringContaining('Loaded 0 patterns from .geminiignore'), @@ -149,10 +114,9 @@ describe('loadGeminiIgnorePatterns', () => { expect(consoleLogSpy).not.toHaveBeenCalledWith( expect.stringContaining('No .geminiignore file found'), ); - expect(mockedFsReadFileSync).toHaveBeenCalledWith(ignoreFilePath, 'utf-8'); }); - it('should return an empty array if .geminiignore contains only comments and empty lines', () => { + it('should return an empty array if .geminiignore contains only comments and empty lines', async () => { const ignoreContent = [ '# Comment 1', ' # Comment 2 with leading spaces', @@ -161,14 +125,8 @@ describe('loadGeminiIgnorePatterns', () => { ].join('\n'); const ignoreFilePath = path.join(tempDir, '.geminiignore'); actualFs.writeFileSync(ignoreFilePath, ignoreContent); - mockedFsReadFileSync.mockImplementation((p: string, encoding: string) => { - if (p === ignoreFilePath && encoding === 'utf-8') return ignoreContent; - throw new Error( - `Mock fs.readFileSync: Unexpected call with path: ${p}, encoding: ${encoding}`, - ); - }); - const patterns = loadGeminiIgnorePatterns(tempDir); + const patterns = await loadGeminiIgnorePatterns(tempDir); expect(patterns).toEqual([]); expect(consoleLogSpy).not.toHaveBeenCalledWith( expect.stringContaining('Loaded 0 patterns from .geminiignore'), @@ -176,48 +134,17 @@ describe('loadGeminiIgnorePatterns', () => { expect(consoleLogSpy).not.toHaveBeenCalledWith( expect.stringContaining('No .geminiignore file found'), ); - expect(mockedFsReadFileSync).toHaveBeenCalledWith(ignoreFilePath, 'utf-8'); }); - it('should handle read errors (other than ENOENT) and log a warning', () => { - const ignoreFilePath = path.join(tempDir, '.geminiignore'); - mockedFsReadFileSync.mockImplementation((p: string, encoding: string) => { - if (p === ignoreFilePath && encoding === 'utf-8') { - const error = new Error('Test read error') as NodeJS.ErrnoException; - error.code = 'EACCES'; - throw error; - } - throw new Error( - `Mock fs.readFileSync: Unexpected call with path: ${p}, encoding: ${encoding}`, - ); - }); - - const patterns = loadGeminiIgnorePatterns(tempDir); - expect(patterns).toEqual([]); - expect(consoleWarnSpy).toHaveBeenCalledWith( - expect.stringContaining( - `[WARN] Could not read .geminiignore file at ${ignoreFilePath}: Test read error`, - ), - ); - expect(mockedFsReadFileSync).toHaveBeenCalledWith(ignoreFilePath, 'utf-8'); - }); - - it('should correctly handle patterns with inline comments if not starting with #', () => { + it('should correctly handle patterns with inline comments if not starting with #', async () => { const ignoreContent = 'src/important # but not this part'; const ignoreFilePath = path.join(tempDir, '.geminiignore'); actualFs.writeFileSync(ignoreFilePath, ignoreContent); - mockedFsReadFileSync.mockImplementation((p: string, encoding: string) => { - if (p === ignoreFilePath && encoding === 'utf-8') return ignoreContent; - throw new Error( - `Mock fs.readFileSync: Unexpected call with path: ${p}, encoding: ${encoding}`, - ); - }); - const patterns = loadGeminiIgnorePatterns(tempDir); + const patterns = await loadGeminiIgnorePatterns(tempDir); expect(patterns).toEqual(['src/important # but not this part']); expect(consoleLogSpy).toHaveBeenCalledWith( expect.stringContaining('Loaded 1 patterns from .geminiignore'), ); - expect(mockedFsReadFileSync).toHaveBeenCalledWith(ignoreFilePath, 'utf-8'); }); }); diff --git a/packages/cli/src/utils/loadIgnorePatterns.ts b/packages/cli/src/utils/loadIgnorePatterns.ts index 1910942f..34efc8c8 100644 --- a/packages/cli/src/utils/loadIgnorePatterns.ts +++ b/packages/cli/src/utils/loadIgnorePatterns.ts @@ -4,46 +4,28 @@ * SPDX-License-Identifier: Apache-2.0 */ -import * as fs from 'node:fs'; import * as path from 'node:path'; +import { GitIgnoreParser } from '@gemini-cli/core'; const GEMINI_IGNORE_FILE_NAME = '.geminiignore'; /** * Loads and parses a .geminiignore file from the given workspace root. - * The .geminiignore file follows a format similar to .gitignore: - * - Each line specifies a glob pattern. - * - Lines are trimmed of leading and trailing whitespace. - * - Blank lines (after trimming) are ignored. - * - Lines starting with a pound sign (#) (after trimming) are treated as comments and ignored. - * - Patterns are case-sensitive and follow standard glob syntax. - * - If a # character appears elsewhere in a line (not at the start after trimming), - * it is considered part of the glob pattern. + * The .geminiignore file follows a format similar to .gitignore. * * @param workspaceRoot The absolute path to the workspace root where the .geminiignore file is expected. * @returns An array of glob patterns extracted from the .geminiignore file. Returns an empty array * if the file does not exist or contains no valid patterns. */ -export function loadGeminiIgnorePatterns(workspaceRoot: string): string[] { - const ignoreFilePath = path.join(workspaceRoot, GEMINI_IGNORE_FILE_NAME); - const patterns: string[] = []; +export async function loadGeminiIgnorePatterns( + workspaceRoot: string, +): Promise { + const parser = new GitIgnoreParser(workspaceRoot); try { - const fileContent = fs.readFileSync(ignoreFilePath, 'utf-8'); - const lines = fileContent.split(/\r?\n/); - - for (const line of lines) { - const trimmedLine = line.trim(); - if (trimmedLine && !trimmedLine.startsWith('#')) { - patterns.push(trimmedLine); - } - } - if (patterns.length > 0) { - console.log( - `[INFO] Loaded ${patterns.length} patterns from .geminiignore`, - ); - } + await parser.loadPatterns(GEMINI_IGNORE_FILE_NAME); } catch (error: unknown) { + const ignoreFilePath = path.join(workspaceRoot, GEMINI_IGNORE_FILE_NAME); if ( error instanceof Error && 'code' in error && @@ -64,5 +46,11 @@ export function loadGeminiIgnorePatterns(workspaceRoot: string): string[] { ); } } - return patterns; + const loadedPatterns = parser.getPatterns(); + if (loadedPatterns.length > 0) { + console.log( + `[INFO] Loaded ${loadedPatterns.length} patterns from .geminiignore`, + ); + } + return loadedPatterns; } diff --git a/packages/core/src/utils/gitIgnoreParser.test.ts b/packages/core/src/utils/gitIgnoreParser.test.ts index 1646a5b9..12084266 100644 --- a/packages/core/src/utils/gitIgnoreParser.test.ts +++ b/packages/core/src/utils/gitIgnoreParser.test.ts @@ -8,14 +8,13 @@ import { describe, it, expect, beforeEach, vi, afterEach } from 'vitest'; import { GitIgnoreParser } from './gitIgnoreParser.js'; import * as fs from 'fs/promises'; import * as path from 'path'; +import { isGitRepository } from './gitUtils.js'; // Mock fs module vi.mock('fs/promises'); // Mock gitUtils module -vi.mock('./gitUtils.js', () => ({ - isGitRepository: vi.fn(() => true), -})); +vi.mock('./gitUtils.js'); describe('GitIgnoreParser', () => { let parser: GitIgnoreParser; @@ -26,6 +25,7 @@ describe('GitIgnoreParser', () => { // Reset mocks before each test vi.mocked(fs.readFile).mockClear(); vi.mocked(fs.readFile).mockRejectedValue(new Error('ENOENT')); // Default to no file + vi.mocked(isGitRepository).mockReturnValue(true); }); afterEach(() => { @@ -51,6 +51,13 @@ node_modules/ await parser.initialize(); + expect(parser.getPatterns()).toEqual([ + '.git', + 'node_modules/', + '*.log', + '/dist', + '.env', + ]); expect(parser.isIgnored('node_modules/some-lib')).toBe(true); expect(parser.isIgnored('src/app.log')).toBe(true); expect(parser.isIgnored('dist/index.js')).toBe(true); @@ -68,7 +75,22 @@ node_modules/ }); await parser.initialize(); + expect(parser.getPatterns()).toEqual(['.git', 'temp/', '*.tmp']); + expect(parser.isIgnored('temp/file.txt')).toBe(true); + expect(parser.isIgnored('src/file.tmp')).toBe(true); + }); + it('should handle custom patterns file name', async () => { + vi.mocked(isGitRepository).mockReturnValue(false); + vi.mocked(fs.readFile).mockImplementation(async (filePath) => { + if (filePath === path.join(mockProjectRoot, '.geminiignore')) { + return 'temp/\n*.tmp'; + } + throw new Error('ENOENT'); + }); + + await parser.initialize('.geminiignore'); + expect(parser.getPatterns()).toEqual(['temp/', '*.tmp']); expect(parser.isIgnored('temp/file.txt')).toBe(true); expect(parser.isIgnored('src/file.tmp')).toBe(true); }); diff --git a/packages/core/src/utils/gitIgnoreParser.ts b/packages/core/src/utils/gitIgnoreParser.ts index d5d013a8..eeee9f48 100644 --- a/packages/core/src/utils/gitIgnoreParser.ts +++ b/packages/core/src/utils/gitIgnoreParser.ts @@ -17,43 +17,53 @@ export class GitIgnoreParser implements GitIgnoreFilter { private projectRoot: string; private isGitRepo: boolean = false; private ig: Ignore = ignore(); + private patterns: string[] = []; constructor(projectRoot: string) { this.projectRoot = path.resolve(projectRoot); } - async initialize(): Promise { + async initialize(patternsFileName?: string): Promise { + const patternFiles = []; + if (patternsFileName && patternsFileName !== '') { + patternFiles.push(patternsFileName); + } + this.isGitRepo = isGitRepository(this.projectRoot); if (this.isGitRepo) { - const gitIgnoreFiles = [ - path.join(this.projectRoot, '.gitignore'), - path.join(this.projectRoot, '.git', 'info', 'exclude'), - ]; + patternFiles.push('.gitignore'); + patternFiles.push(path.join('.git', 'info', 'exclude')); // Always ignore .git directory regardless of .gitignore content this.addPatterns(['.git']); - - for (const gitIgnoreFile of gitIgnoreFiles) { - try { - const content = await fs.readFile(gitIgnoreFile, 'utf-8'); - const patterns = content.split('\n').map((p) => p.trim()); - this.addPatterns(patterns); - } catch (_error) { - // File doesn't exist or can't be read, continue silently - } + } + for (const pf of patternFiles) { + try { + await this.loadPatterns(pf); + } catch (_error) { + // File doesn't exist or can't be read, continue silently } } } + async loadPatterns(patternsFileName: string): Promise { + const content = await fs.readFile( + path.join(this.projectRoot, patternsFileName), + 'utf-8', + ); + const patterns = content + .split('\n') + .map((p) => p.trim()) + .filter((p) => p !== '' && !p.startsWith('#')); + this.addPatterns(patterns); + } + private addPatterns(patterns: string[]) { this.ig.add(patterns); + this.patterns.push(...patterns); } isIgnored(filePath: string): boolean { - if (!this.isGitRepo) { - return false; - } - const relativePath = path.isAbsolute(filePath) ? path.relative(this.projectRoot, filePath) : filePath; @@ -67,11 +77,10 @@ export class GitIgnoreParser implements GitIgnoreFilter { normalizedPath = normalizedPath.substring(2); } - const ignored = this.ig.ignores(normalizedPath); - return ignored; + return this.ig.ignores(normalizedPath); } - getGitRepoRoot(): string { - return this.projectRoot; + getPatterns(): string[] { + return this.patterns; } }