@file don't respect config respectGitIgnore=false (#3382) (#3387)

Co-authored-by: Ryan Fang <ryan.fang@gllue.com>
This commit is contained in:
zfflxx 2025-07-07 13:48:39 +08:00 committed by GitHub
parent bb8f6b376d
commit 97d9386e3f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 109 additions and 33 deletions

View File

@ -90,7 +90,7 @@ describe('handleAtCommand', () => {
// Mock FileDiscoveryService // Mock FileDiscoveryService
mockFileDiscoveryService = { mockFileDiscoveryService = {
initialize: vi.fn(), initialize: vi.fn(),
shouldGitIgnoreFile: vi.fn(() => false), shouldIgnoreFile: vi.fn(() => false),
filterFiles: vi.fn((files) => files), filterFiles: vi.fn((files) => files),
getIgnoreInfo: vi.fn(() => ({ gitIgnored: [] })), getIgnoreInfo: vi.fn(() => ({ gitIgnored: [] })),
isGitRepository: vi.fn(() => true), isGitRepository: vi.fn(() => true),
@ -171,7 +171,7 @@ describe('handleAtCommand', () => {
125, 125,
); );
expect(mockReadManyFilesExecute).toHaveBeenCalledWith( expect(mockReadManyFilesExecute).toHaveBeenCalledWith(
{ paths: [filePath], respectGitIgnore: true }, { paths: [filePath], respect_git_ignore: true },
abortController.signal, abortController.signal,
); );
expect(mockAddItem).toHaveBeenCalledWith( expect(mockAddItem).toHaveBeenCalledWith(
@ -217,7 +217,7 @@ describe('handleAtCommand', () => {
126, 126,
); );
expect(mockReadManyFilesExecute).toHaveBeenCalledWith( expect(mockReadManyFilesExecute).toHaveBeenCalledWith(
{ paths: [resolvedGlob], respectGitIgnore: true }, { paths: [resolvedGlob], respect_git_ignore: true },
abortController.signal, abortController.signal,
); );
expect(mockOnDebugMessage).toHaveBeenCalledWith( expect(mockOnDebugMessage).toHaveBeenCalledWith(
@ -318,7 +318,7 @@ describe('handleAtCommand', () => {
signal: abortController.signal, signal: abortController.signal,
}); });
expect(mockReadManyFilesExecute).toHaveBeenCalledWith( expect(mockReadManyFilesExecute).toHaveBeenCalledWith(
{ paths: [unescapedPath], respectGitIgnore: true }, { paths: [unescapedPath], respect_git_ignore: true },
abortController.signal, abortController.signal,
); );
}); });
@ -347,7 +347,7 @@ describe('handleAtCommand', () => {
signal: abortController.signal, signal: abortController.signal,
}); });
expect(mockReadManyFilesExecute).toHaveBeenCalledWith( expect(mockReadManyFilesExecute).toHaveBeenCalledWith(
{ paths: [file1, file2], respectGitIgnore: true }, { paths: [file1, file2], respect_git_ignore: true },
abortController.signal, abortController.signal,
); );
expect(result.processedQuery).toEqual([ expect(result.processedQuery).toEqual([
@ -389,7 +389,7 @@ describe('handleAtCommand', () => {
signal: abortController.signal, signal: abortController.signal,
}); });
expect(mockReadManyFilesExecute).toHaveBeenCalledWith( expect(mockReadManyFilesExecute).toHaveBeenCalledWith(
{ paths: [file1, file2], respectGitIgnore: true }, { paths: [file1, file2], respect_git_ignore: true },
abortController.signal, abortController.signal,
); );
expect(result.processedQuery).toEqual([ expect(result.processedQuery).toEqual([
@ -454,7 +454,7 @@ describe('handleAtCommand', () => {
}); });
expect(mockReadManyFilesExecute).toHaveBeenCalledWith( expect(mockReadManyFilesExecute).toHaveBeenCalledWith(
{ paths: [file1, resolvedFile2], respectGitIgnore: true }, { paths: [file1, resolvedFile2], respect_git_ignore: true },
abortController.signal, abortController.signal,
); );
expect(result.processedQuery).toEqual([ expect(result.processedQuery).toEqual([
@ -556,7 +556,7 @@ describe('handleAtCommand', () => {
// If the mock is simpler, it might use queryPath if stat(queryPath) succeeds. // If the mock is simpler, it might use queryPath if stat(queryPath) succeeds.
// The most important part is that *some* version of the path that leads to the content is used. // The most important part is that *some* version of the path that leads to the content is used.
// Let's assume it uses the path from the query if stat confirms it exists (even if different case on disk) // Let's assume it uses the path from the query if stat confirms it exists (even if different case on disk)
{ paths: [queryPath], respectGitIgnore: true }, { paths: [queryPath], respect_git_ignore: true },
abortController.signal, abortController.signal,
); );
expect(mockAddItem).toHaveBeenCalledWith( expect(mockAddItem).toHaveBeenCalledWith(
@ -582,8 +582,9 @@ describe('handleAtCommand', () => {
const query = `@${gitIgnoredFile}`; const query = `@${gitIgnoredFile}`;
// Mock the file discovery service to report this file as git-ignored // Mock the file discovery service to report this file as git-ignored
mockFileDiscoveryService.shouldGitIgnoreFile.mockImplementation( mockFileDiscoveryService.shouldIgnoreFile.mockImplementation(
(path: string) => path === gitIgnoredFile, (path: string, options?: { respectGitIgnore?: boolean }) =>
path === gitIgnoredFile && options?.respectGitIgnore !== false,
); );
const result = await handleAtCommand({ const result = await handleAtCommand({
@ -595,8 +596,9 @@ describe('handleAtCommand', () => {
signal: abortController.signal, signal: abortController.signal,
}); });
expect(mockFileDiscoveryService.shouldGitIgnoreFile).toHaveBeenCalledWith( expect(mockFileDiscoveryService.shouldIgnoreFile).toHaveBeenCalledWith(
gitIgnoredFile, gitIgnoredFile,
{ respectGitIgnore: true },
); );
expect(mockOnDebugMessage).toHaveBeenCalledWith( expect(mockOnDebugMessage).toHaveBeenCalledWith(
`Path ${gitIgnoredFile} is git-ignored and will be skipped.`, `Path ${gitIgnoredFile} is git-ignored and will be skipped.`,
@ -614,7 +616,7 @@ describe('handleAtCommand', () => {
const query = `@${validFile}`; const query = `@${validFile}`;
const fileContent = 'console.log("Hello world");'; const fileContent = 'console.log("Hello world");';
mockFileDiscoveryService.shouldGitIgnoreFile.mockReturnValue(false); mockFileDiscoveryService.shouldIgnoreFile.mockReturnValue(false);
mockReadManyFilesExecute.mockResolvedValue({ mockReadManyFilesExecute.mockResolvedValue({
llmContent: [`--- ${validFile} ---\n\n${fileContent}\n\n`], llmContent: [`--- ${validFile} ---\n\n${fileContent}\n\n`],
returnDisplay: 'Read 1 file.', returnDisplay: 'Read 1 file.',
@ -629,11 +631,12 @@ describe('handleAtCommand', () => {
signal: abortController.signal, signal: abortController.signal,
}); });
expect(mockFileDiscoveryService.shouldGitIgnoreFile).toHaveBeenCalledWith( expect(mockFileDiscoveryService.shouldIgnoreFile).toHaveBeenCalledWith(
validFile, validFile,
{ respectGitIgnore: true },
); );
expect(mockReadManyFilesExecute).toHaveBeenCalledWith( expect(mockReadManyFilesExecute).toHaveBeenCalledWith(
{ paths: [validFile], respectGitIgnore: true }, { paths: [validFile], respect_git_ignore: true },
abortController.signal, abortController.signal,
); );
expect(result.processedQuery).toEqual([ expect(result.processedQuery).toEqual([
@ -652,8 +655,9 @@ describe('handleAtCommand', () => {
const query = `@${validFile} @${gitIgnoredFile}`; const query = `@${validFile} @${gitIgnoredFile}`;
const fileContent = '# Project README'; const fileContent = '# Project README';
mockFileDiscoveryService.shouldGitIgnoreFile.mockImplementation( mockFileDiscoveryService.shouldIgnoreFile.mockImplementation(
(path: string) => path === gitIgnoredFile, (path: string, options?: { respectGitIgnore?: boolean }) =>
path === gitIgnoredFile && options?.respectGitIgnore !== false,
); );
mockReadManyFilesExecute.mockResolvedValue({ mockReadManyFilesExecute.mockResolvedValue({
llmContent: [`--- ${validFile} ---\n\n${fileContent}\n\n`], llmContent: [`--- ${validFile} ---\n\n${fileContent}\n\n`],
@ -669,11 +673,13 @@ describe('handleAtCommand', () => {
signal: abortController.signal, signal: abortController.signal,
}); });
expect(mockFileDiscoveryService.shouldGitIgnoreFile).toHaveBeenCalledWith( expect(mockFileDiscoveryService.shouldIgnoreFile).toHaveBeenCalledWith(
validFile, validFile,
{ respectGitIgnore: true },
); );
expect(mockFileDiscoveryService.shouldGitIgnoreFile).toHaveBeenCalledWith( expect(mockFileDiscoveryService.shouldIgnoreFile).toHaveBeenCalledWith(
gitIgnoredFile, gitIgnoredFile,
{ respectGitIgnore: true },
); );
expect(mockOnDebugMessage).toHaveBeenCalledWith( expect(mockOnDebugMessage).toHaveBeenCalledWith(
`Path ${gitIgnoredFile} is git-ignored and will be skipped.`, `Path ${gitIgnoredFile} is git-ignored and will be skipped.`,
@ -682,7 +688,7 @@ describe('handleAtCommand', () => {
'Ignored 1 git-ignored files: .env', 'Ignored 1 git-ignored files: .env',
); );
expect(mockReadManyFilesExecute).toHaveBeenCalledWith( expect(mockReadManyFilesExecute).toHaveBeenCalledWith(
{ paths: [validFile], respectGitIgnore: true }, { paths: [validFile], respect_git_ignore: true },
abortController.signal, abortController.signal,
); );
expect(result.processedQuery).toEqual([ expect(result.processedQuery).toEqual([
@ -699,7 +705,7 @@ describe('handleAtCommand', () => {
const gitFile = '.git/config'; const gitFile = '.git/config';
const query = `@${gitFile}`; const query = `@${gitFile}`;
mockFileDiscoveryService.shouldGitIgnoreFile.mockReturnValue(true); mockFileDiscoveryService.shouldIgnoreFile.mockReturnValue(true);
const result = await handleAtCommand({ const result = await handleAtCommand({
query, query,
@ -710,8 +716,9 @@ describe('handleAtCommand', () => {
signal: abortController.signal, signal: abortController.signal,
}); });
expect(mockFileDiscoveryService.shouldGitIgnoreFile).toHaveBeenCalledWith( expect(mockFileDiscoveryService.shouldIgnoreFile).toHaveBeenCalledWith(
gitFile, gitFile,
{ respectGitIgnore: true },
); );
expect(mockOnDebugMessage).toHaveBeenCalledWith( expect(mockOnDebugMessage).toHaveBeenCalledWith(
`Path ${gitFile} is git-ignored and will be skipped.`, `Path ${gitFile} is git-ignored and will be skipped.`,

View File

@ -181,12 +181,10 @@ export async function handleAtCommand({
return { processedQuery: null, shouldProceed: false }; return { processedQuery: null, shouldProceed: false };
} }
// Check if path should be ignored by git // Check if path should be ignored based on filtering options
if (fileDiscovery.shouldGitIgnoreFile(pathName)) { if (fileDiscovery.shouldIgnoreFile(pathName, { respectGitIgnore })) {
const reason = respectGitIgnore const reason = respectGitIgnore ? 'git-ignored' : 'custom-ignored';
? 'git-ignored and will be skipped' onDebugMessage(`Path ${pathName} is ${reason} and will be skipped.`);
: 'ignored by custom patterns';
onDebugMessage(`Path ${pathName} is ${reason}.`);
ignoredPaths.push(pathName); ignoredPaths.push(pathName);
continue; continue;
} }
@ -349,7 +347,7 @@ export async function handleAtCommand({
const toolArgs = { const toolArgs = {
paths: pathSpecsToRead, paths: pathSpecsToRead,
respectGitIgnore, // Use configuration setting respect_git_ignore: respectGitIgnore, // Use configuration setting
}; };
let toolCallDisplay: IndividualToolCallDisplay; let toolCallDisplay: IndividualToolCallDisplay;

View File

@ -41,7 +41,10 @@ describe('useCompletion git-aware filtering integration', () => {
beforeEach(() => { beforeEach(() => {
mockFileDiscoveryService = { mockFileDiscoveryService = {
shouldGitIgnoreFile: vi.fn(), shouldGitIgnoreFile: vi.fn(),
shouldGeminiIgnoreFile: vi.fn(),
shouldIgnoreFile: vi.fn(),
filterFiles: vi.fn(), filterFiles: vi.fn(),
getGeminiIgnorePatterns: vi.fn(() => []),
}; };
mockConfig = { mockConfig = {
@ -68,6 +71,14 @@ describe('useCompletion git-aware filtering integration', () => {
mockFileDiscoveryService.shouldGitIgnoreFile.mockImplementation( mockFileDiscoveryService.shouldGitIgnoreFile.mockImplementation(
(path: string) => path.includes('dist'), (path: string) => path.includes('dist'),
); );
mockFileDiscoveryService.shouldIgnoreFile.mockImplementation(
(path: string, options) => {
if (options?.respectGitIgnore !== false) {
return mockFileDiscoveryService.shouldGitIgnoreFile(path);
}
return false;
},
);
const { result } = renderHook(() => const { result } = renderHook(() =>
useCompletion('@d', testCwd, true, slashCommands, mockConfig), useCompletion('@d', testCwd, true, slashCommands, mockConfig),
@ -102,6 +113,14 @@ describe('useCompletion git-aware filtering integration', () => {
path.includes('dist') || path.includes('dist') ||
path.includes('.env'), path.includes('.env'),
); );
mockFileDiscoveryService.shouldIgnoreFile.mockImplementation(
(path: string, options) => {
if (options?.respectGitIgnore !== false) {
return mockFileDiscoveryService.shouldGitIgnoreFile(path);
}
return false;
},
);
const { result } = renderHook(() => const { result } = renderHook(() =>
useCompletion('@', testCwd, true, slashCommands, mockConfig), useCompletion('@', testCwd, true, slashCommands, mockConfig),
@ -153,6 +172,14 @@ describe('useCompletion git-aware filtering integration', () => {
mockFileDiscoveryService.shouldGitIgnoreFile.mockImplementation( mockFileDiscoveryService.shouldGitIgnoreFile.mockImplementation(
(path: string) => path.includes('node_modules') || path.includes('temp'), (path: string) => path.includes('node_modules') || path.includes('temp'),
); );
mockFileDiscoveryService.shouldIgnoreFile.mockImplementation(
(path: string, options) => {
if (options?.respectGitIgnore !== false) {
return mockFileDiscoveryService.shouldGitIgnoreFile(path);
}
return false;
},
);
const { result } = renderHook(() => const { result } = renderHook(() =>
useCompletion('@t', testCwd, true, slashCommands, mockConfig), useCompletion('@t', testCwd, true, slashCommands, mockConfig),
@ -261,6 +288,14 @@ describe('useCompletion git-aware filtering integration', () => {
mockFileDiscoveryService.shouldGitIgnoreFile.mockImplementation( mockFileDiscoveryService.shouldGitIgnoreFile.mockImplementation(
(path: string) => path.includes('.log'), (path: string) => path.includes('.log'),
); );
mockFileDiscoveryService.shouldIgnoreFile.mockImplementation(
(path: string, options) => {
if (options?.respectGitIgnore !== false) {
return mockFileDiscoveryService.shouldGitIgnoreFile(path);
}
return false;
},
);
const { result } = renderHook(() => const { result } = renderHook(() =>
useCompletion('@src/comp', testCwd, true, slashCommands, mockConfig), useCompletion('@src/comp', testCwd, true, slashCommands, mockConfig),

View File

@ -217,7 +217,11 @@ export function useCompletion(
const findFilesRecursively = async ( const findFilesRecursively = async (
startDir: string, startDir: string,
searchPrefix: string, searchPrefix: string,
fileDiscovery: { shouldGitIgnoreFile: (path: string) => boolean } | null, fileDiscovery: FileDiscoveryService | null,
filterOptions: {
respectGitIgnore?: boolean;
respectGeminiIgnore?: boolean;
},
currentRelativePath = '', currentRelativePath = '',
depth = 0, depth = 0,
maxDepth = 10, // Limit recursion depth maxDepth = 10, // Limit recursion depth
@ -245,10 +249,10 @@ export function useCompletion(
continue; continue;
} }
// Check if this entry should be ignored by git-aware filtering // Check if this entry should be ignored by filtering options
if ( if (
fileDiscovery && fileDiscovery &&
fileDiscovery.shouldGitIgnoreFile(entryPathFromRoot) fileDiscovery.shouldIgnoreFile(entryPathFromRoot, filterOptions)
) { ) {
continue; continue;
} }
@ -272,6 +276,7 @@ export function useCompletion(
path.join(startDir, entry.name), path.join(startDir, entry.name),
searchPrefix, // Pass original searchPrefix for recursive calls searchPrefix, // Pass original searchPrefix for recursive calls
fileDiscovery, fileDiscovery,
filterOptions,
entryPathRelative, entryPathRelative,
depth + 1, depth + 1,
maxDepth, maxDepth,
@ -290,6 +295,10 @@ export function useCompletion(
const findFilesWithGlob = async ( const findFilesWithGlob = async (
searchPrefix: string, searchPrefix: string,
fileDiscoveryService: FileDiscoveryService, fileDiscoveryService: FileDiscoveryService,
filterOptions: {
respectGitIgnore?: boolean;
respectGeminiIgnore?: boolean;
},
maxResults = 50, maxResults = 50,
): Promise<Suggestion[]> => { ): Promise<Suggestion[]> => {
const globPattern = `**/${searchPrefix}*`; const globPattern = `**/${searchPrefix}*`;
@ -309,7 +318,10 @@ export function useCompletion(
}) })
.filter((s) => { .filter((s) => {
if (fileDiscoveryService) { if (fileDiscoveryService) {
return !fileDiscoveryService.shouldGitIgnoreFile(s.label); // relative path return !fileDiscoveryService.shouldIgnoreFile(
s.label,
filterOptions,
); // relative path
} }
return true; return true;
}) })
@ -325,6 +337,10 @@ export function useCompletion(
const fileDiscoveryService = config ? config.getFileService() : null; const fileDiscoveryService = config ? config.getFileService() : null;
const enableRecursiveSearch = const enableRecursiveSearch =
config?.getEnableRecursiveFileSearch() ?? true; config?.getEnableRecursiveFileSearch() ?? true;
const filterOptions = {
respectGitIgnore: config?.getFileFilteringRespectGitIgnore() ?? true,
respectGeminiIgnore: true,
};
try { try {
// If there's no slash, or it's the root, do a recursive search from cwd // If there's no slash, or it's the root, do a recursive search from cwd
@ -337,12 +353,14 @@ export function useCompletion(
fetchedSuggestions = await findFilesWithGlob( fetchedSuggestions = await findFilesWithGlob(
prefix, prefix,
fileDiscoveryService, fileDiscoveryService,
filterOptions,
); );
} else { } else {
fetchedSuggestions = await findFilesRecursively( fetchedSuggestions = await findFilesRecursively(
cwd, cwd,
prefix, prefix,
fileDiscoveryService, fileDiscoveryService,
filterOptions,
); );
} }
} else { } else {
@ -367,7 +385,7 @@ export function useCompletion(
); );
if ( if (
fileDiscoveryService && fileDiscoveryService &&
fileDiscoveryService.shouldGitIgnoreFile(relativePath) fileDiscoveryService.shouldIgnoreFile(relativePath, filterOptions)
) { ) {
continue; continue;
} }

View File

@ -84,6 +84,24 @@ export class FileDiscoveryService {
return false; return false;
} }
/**
* Unified method to check if a file should be ignored based on filtering options
*/
shouldIgnoreFile(
filePath: string,
options: FilterFilesOptions = {},
): boolean {
const { respectGitIgnore = true, respectGeminiIgnore = true } = options;
if (respectGitIgnore && this.shouldGitIgnoreFile(filePath)) {
return true;
}
if (respectGeminiIgnore && this.shouldGeminiIgnoreFile(filePath)) {
return true;
}
return false;
}
/** /**
* Returns loaded patterns from .geminiignore * Returns loaded patterns from .geminiignore
*/ */