refactor: Switch over to unified shouldIgnoreFile (#11815)

packages/cli/src/zed-integration/zedIntegration.ts

@@ -12,6 +12,7 @@ import type {
   ToolResult,
   ToolCallConfirmationDetails,
   GeminiCLIExtension,
+  FilterFilesOptions,
 } from '@google/gemini-cli-core';
 import {
   AuthType,
@@ -571,7 +572,8 @@ class Session {
 
     // Get centralized file discovery service
     const fileDiscovery = this.config.getFileService();
-    const respectGitIgnore = this.config.getFileFilteringRespectGitIgnore();
+    const fileFilteringOptions: FilterFilesOptions =
+      this.config.getFileFilteringOptions();
 
     const pathSpecsToRead: string[] = [];
     const contentLabelsForDisplay: string[] = [];
@@ -587,13 +589,10 @@ class Session {
 
     for (const atPathPart of atPathCommandParts) {
       const pathName = atPathPart.fileData!.fileUri;
-      // Check if path should be ignored by git
+      // Check if path should be ignored
-      if (fileDiscovery.shouldGitIgnoreFile(pathName)) {
+      if (fileDiscovery.shouldIgnoreFile(pathName, fileFilteringOptions)) {
         ignoredPaths.push(pathName);
-        const reason = respectGitIgnore
+        debugLogger.warn(`Path ${pathName} is ignored and will be skipped.`);
-          ? 'git-ignored and will be skipped'
-          : 'ignored by custom patterns';
-        debugLogger.warn(`Path ${pathName} is ${reason}.`);
         continue;
       }
       let currentPathSpec = pathName;
@@ -730,9 +729,8 @@ class Session {
     initialQueryText = initialQueryText.trim();
     // Inform user about ignored paths
     if (ignoredPaths.length > 0) {
-      const ignoreType = respectGitIgnore ? 'git-ignored' : 'custom-ignored';
       this.debug(
-        `Ignored ${ignoredPaths.length} ${ignoreType} files: ${ignoredPaths.join(', ')}`,
+        `Ignored ${ignoredPaths.length} files: ${ignoredPaths.join(', ')}`,
       );
     }
 
@@ -747,7 +745,6 @@ class Session {
     if (pathSpecsToRead.length > 0) {
       const toolArgs = {
         paths: pathSpecsToRead,
-        respectGitIgnore, // Use configuration setting
       };
 
       const callId = `${readManyFilesTool.name}-${Date.now()}`;

packages/core/src/services/fileDiscoveryService.test.ts

@@ -40,8 +40,8 @@ describe('FileDiscoveryService', () => {
 
       const service = new FileDiscoveryService(projectRoot);
       // Let's check the effect of the parser instead of mocking it.
-      expect(service.shouldGitIgnoreFile('node_modules/foo.js')).toBe(true);
+      expect(service.shouldIgnoreFile('node_modules/foo.js')).toBe(true);
-      expect(service.shouldGitIgnoreFile('src/foo.js')).toBe(false);
+      expect(service.shouldIgnoreFile('src/foo.js')).toBe(false);
     });
 
     it('should not load git repo patterns when not in a git repo', async () => {
@@ -50,15 +50,15 @@ describe('FileDiscoveryService', () => {
       const service = new FileDiscoveryService(projectRoot);
 
       // .gitignore is not loaded in non-git repos
-      expect(service.shouldGitIgnoreFile('node_modules/foo.js')).toBe(false);
+      expect(service.shouldIgnoreFile('node_modules/foo.js')).toBe(false);
     });
 
     it('should load .geminiignore patterns even when not in a git repo', async () => {
       await createTestFile('.geminiignore', 'secrets.txt');
       const service = new FileDiscoveryService(projectRoot);
 
-      expect(service.shouldGeminiIgnoreFile('secrets.txt')).toBe(true);
+      expect(service.shouldIgnoreFile('secrets.txt')).toBe(true);
-      expect(service.shouldGeminiIgnoreFile('src/index.js')).toBe(false);
+      expect(service.shouldIgnoreFile('src/index.js')).toBe(false);
     });
   });
 
@@ -184,7 +184,7 @@ describe('FileDiscoveryService', () => {
       const service = new FileDiscoveryService(projectRoot);
 
       expect(
-        service.shouldGitIgnoreFile(
+        service.shouldIgnoreFile(
           path.join(projectRoot, 'node_modules/package/index.js'),
         ),
       ).toBe(true);
@@ -194,7 +194,7 @@ describe('FileDiscoveryService', () => {
       const service = new FileDiscoveryService(projectRoot);
 
       expect(
-        service.shouldGitIgnoreFile(path.join(projectRoot, 'src/index.ts')),
+        service.shouldIgnoreFile(path.join(projectRoot, 'src/index.ts')),
       ).toBe(false);
     });
 
@@ -202,7 +202,7 @@ describe('FileDiscoveryService', () => {
       const service = new FileDiscoveryService(projectRoot);
 
       expect(
-        service.shouldGeminiIgnoreFile(path.join(projectRoot, 'debug.log')),
+        service.shouldIgnoreFile(path.join(projectRoot, 'debug.log')),
       ).toBe(true);
     });
 
@@ -210,7 +210,7 @@ describe('FileDiscoveryService', () => {
       const service = new FileDiscoveryService(projectRoot);
 
       expect(
-        service.shouldGeminiIgnoreFile(path.join(projectRoot, 'src/index.ts')),
+        service.shouldIgnoreFile(path.join(projectRoot, 'src/index.ts')),
       ).toBe(false);
     });
   });
@@ -224,10 +224,10 @@ describe('FileDiscoveryService', () => {
       );
 
       expect(
-        service.shouldGitIgnoreFile(path.join(projectRoot, 'ignored.txt')),
+        service.shouldIgnoreFile(path.join(projectRoot, 'ignored.txt')),
       ).toBe(true);
       expect(
-        service.shouldGitIgnoreFile(path.join(projectRoot, 'not-ignored.txt')),
+        service.shouldIgnoreFile(path.join(projectRoot, 'not-ignored.txt')),
       ).toBe(false);
     });
 

packages/core/src/services/fileDiscoveryService.ts

@@ -37,21 +37,13 @@ export class FileDiscoveryService {
   /**
    * Filters a list of file paths based on git ignore rules
    */
-  filterFiles(
+  filterFiles(filePaths: string[], options: FilterFilesOptions = {}): string[] {
-    filePaths: string[],
+    const { respectGitIgnore = true, respectGeminiIgnore = true } = options;
-    options: FilterFilesOptions = {
-      respectGitIgnore: true,
-      respectGeminiIgnore: true,
-    },
-  ): string[] {
     return filePaths.filter((filePath) => {
-      if (options.respectGitIgnore && this.shouldGitIgnoreFile(filePath)) {
+      if (respectGitIgnore && this.gitIgnoreFilter?.isIgnored(filePath)) {
         return false;
       }
-      if (
+      if (respectGeminiIgnore && this.geminiIgnoreFilter?.isIgnored(filePath)) {
-        options.respectGeminiIgnore &&
-        this.shouldGeminiIgnoreFile(filePath)
-      ) {
         return false;
       }
       return true;
@@ -78,26 +70,6 @@ export class FileDiscoveryService {
     };
   }
 
-  /**
-   * Checks if a single file should be git-ignored
-   */
-  shouldGitIgnoreFile(filePath: string): boolean {
-    if (this.gitIgnoreFilter) {
-      return this.gitIgnoreFilter.isIgnored(filePath);
-    }
-    return false;
-  }
-
-  /**
-   * Checks if a single file should be gemini-ignored
-   */
-  shouldGeminiIgnoreFile(filePath: string): boolean {
-    if (this.geminiIgnoreFilter) {
-      return this.geminiIgnoreFilter.isIgnored(filePath);
-    }
-    return false;
-  }
-
   /**
    * Unified method to check if a file should be ignored based on filtering options
    */
@@ -105,14 +77,6 @@ export class FileDiscoveryService {
     filePath: string,
     options: FilterFilesOptions = {},
   ): boolean {
-    const { respectGitIgnore = true, respectGeminiIgnore = true } = options;
+    return this.filterFiles([filePath], options).length === 0;
-
-    if (respectGitIgnore && this.shouldGitIgnoreFile(filePath)) {
-      return true;
-    }
-    if (respectGeminiIgnore && this.shouldGeminiIgnoreFile(filePath)) {
-      return true;
-    }
-    return false;
   }
 }

packages/core/src/tools/read-file.test.ts

@@ -38,6 +38,10 @@ describe('ReadFileTool', () => {
       getFileSystemService: () => new StandardFileSystemService(),
       getTargetDir: () => tempRootDir,
       getWorkspaceContext: () => createMockWorkspaceContext(tempRootDir),
+      getFileFilteringOptions: () => ({
+        respectGitIgnore: true,
+        respectGeminiIgnore: true,
+      }),
       storage: {
         getProjectTempDir: () => path.join(tempRootDir, '.temp'),
       },
@@ -462,7 +466,7 @@ describe('ReadFileTool', () => {
         const params: ReadFileToolParams = {
           absolute_path: ignoredFilePath,
         };
-        const expectedError = `File path '${ignoredFilePath}' is ignored by .geminiignore pattern(s).`;
+        const expectedError = `File path '${ignoredFilePath}' is ignored by configured ignore patterns.`;
         expect(() => tool.build(params)).toThrow(expectedError);
       });
 
@@ -474,7 +478,7 @@ describe('ReadFileTool', () => {
         const params: ReadFileToolParams = {
           absolute_path: ignoredFilePath,
         };
-        const expectedError = `File path '${ignoredFilePath}' is ignored by .geminiignore pattern(s).`;
+        const expectedError = `File path '${ignoredFilePath}' is ignored by configured ignore patterns.`;
         expect(() => tool.build(params)).toThrow(expectedError);
       });
 

packages/core/src/tools/read-file.ts

@@ -210,8 +210,11 @@ export class ReadFileTool extends BaseDeclarativeTool<
     }
 
     const fileService = this.config.getFileService();
-    if (fileService.shouldGeminiIgnoreFile(params.absolute_path)) {
+    const fileFilteringOptions = this.config.getFileFilteringOptions();
-      return `File path '${filePath}' is ignored by .geminiignore pattern(s).`;
+    if (
+      fileService.shouldIgnoreFile(params.absolute_path, fileFilteringOptions)
+    ) {
+      return `File path '${filePath}' is ignored by configured ignore patterns.`;
     }
 
     return null;

packages/core/src/utils/getFolderStructure.ts

@@ -8,7 +8,10 @@ import * as fs from 'node:fs/promises';
 import type { Dirent } from 'node:fs';
 import * as path from 'node:path';
 import { getErrorMessage, isNodeError } from './errors.js';
-import type { FileDiscoveryService } from '../services/fileDiscoveryService.js';
+import type {
+  FileDiscoveryService,
+  FilterFilesOptions,
+} from '../services/fileDiscoveryService.js';
 import type { FileFilteringOptions } from '../config/constants.js';
 import { DEFAULT_FILE_FILTERING_OPTIONS } from '../config/constants.js';
 import { debugLogger } from './debugLogger.js';
@@ -119,6 +122,10 @@ async function readFullStructure(
 
     const filesInCurrentDir: string[] = [];
     const subFoldersInCurrentDir: FullFolderInfo[] = [];
+    const filterFileOptions: FilterFilesOptions = {
+      respectGitIgnore: options.fileFilteringOptions?.respectGitIgnore,
+      respectGeminiIgnore: options.fileFilteringOptions?.respectGeminiIgnore,
+    };
 
     // Process files first in the current directory
     for (const entry of entries) {
@@ -129,15 +136,10 @@ async function readFullStructure(
         }
         const fileName = entry.name;
         const filePath = path.join(currentPath, fileName);
-        if (options.fileService) {
+        if (
-          const shouldIgnore =
+          options.fileService?.shouldIgnoreFile(filePath, filterFileOptions)
-            (options.fileFilteringOptions.respectGitIgnore &&
+        ) {
-              options.fileService.shouldGitIgnoreFile(filePath)) ||
+          continue;
-            (options.fileFilteringOptions.respectGeminiIgnore &&
-              options.fileService.shouldGeminiIgnoreFile(filePath));
-          if (shouldIgnore) {
-            continue;
-          }
         }
         if (
           !options.fileIncludePattern ||
@@ -168,14 +170,11 @@ async function readFullStructure(
         const subFolderName = entry.name;
         const subFolderPath = path.join(currentPath, subFolderName);
 
-        let isIgnored = false;
+        const isIgnored =
-        if (options.fileService) {
+          options.fileService?.shouldIgnoreFile(
-          isIgnored =
+            subFolderPath,
-            (options.fileFilteringOptions.respectGitIgnore &&
+            filterFileOptions,
-              options.fileService.shouldGitIgnoreFile(subFolderPath)) ||
+          ) ?? false;
-            (options.fileFilteringOptions.respectGeminiIgnore &&
-              options.fileService.shouldGeminiIgnoreFile(subFolderPath));
-        }
 
         if (options.ignoredFolders.has(subFolderName) || isIgnored) {
           const ignoredSubFolder: FullFolderInfo = {