-
Notifications
You must be signed in to change notification settings - Fork 37.5k
Git - use findFiles2() to expand glob patterns
#287238
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| const filesToCheckResults = await this.checkIgnore(Array.from(filesToCheck)); | ||
| filesToCheckResults.forEach(ignoredFile => gitIgnoredFiles.add(ignoredFile)); | ||
| // Get files matching the globs with git ignore files applied | ||
| const nonIgnoredFiles = await workspace.findFiles2(filePattern, { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suspect this would be a good place to apply the new ignoreGlobCase option, but perhaps internally at the API entry point vs. passing as an argument...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that we would have to better understand the use cases to make that choice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request refactors the Git extension's worktree include files logic to use the new findFiles2() proposed API instead of manual glob expansion with Node.js fsPromises.glob().
Changes:
- Replaces custom file globbing and git ignore checking logic with two
findFiles2()calls to determine git-ignored files - Removes manual directory traversal and
checkIgnore()calls in favor of built-in ignore file handling - Improves error logging in
_copyWorktreeIncludeFiles()to show individual error details
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| extensions/git/package.json | Adds findFiles2 to the list of enabled proposed APIs |
| extensions/git/tsconfig.json | Includes the TypeScript definitions for the findFiles2 proposed API |
| extensions/git/src/repository.ts | Refactors _getWorktreeIncludeFiles() to use findFiles2() API; simplifies error handling in _copyWorktreeIncludeFiles() by removing directory-specific error filtering and adding detailed error logging |
| const filePattern = worktreeIncludeFiles | ||
| .map(pattern => new RelativePattern(this.root, pattern)); | ||
|
|
||
| if (isUnderIgnoredDir) { | ||
| gitIgnoredFiles.add(fullPath); | ||
| } else { | ||
| filesToCheck.push(fullPath); | ||
| } | ||
| } | ||
| // Get all files matching the globs (no ignore files applied) | ||
| const allFiles = await workspace.findFiles2(filePattern, { | ||
| useExcludeSettings: ExcludeSettingOptions.None, | ||
| useIgnoreFiles: { local: false, parent: false, global: false } | ||
| }); | ||
|
|
||
| // Check the files that are not under a git ignored directories | ||
| const filesToCheckResults = await this.checkIgnore(Array.from(filesToCheck)); | ||
| filesToCheckResults.forEach(ignoredFile => gitIgnoredFiles.add(ignoredFile)); | ||
| // Get files matching the globs with git ignore files applied | ||
| const nonIgnoredFiles = await workspace.findFiles2(filePattern, { | ||
| useExcludeSettings: ExcludeSettingOptions.None, | ||
| useIgnoreFiles: { local: true, parent: true, global: true } | ||
| }); | ||
|
|
||
| return gitIgnoredFiles; | ||
| } catch (err) { | ||
| this.logger.warn(`[Repository][_getWorktreeIncludeFiles] Failed to get worktree include files: ${err}`); | ||
| return new Set<string>(); | ||
| // Files that are git ignored = all files - non-ignored files | ||
| const gitIgnoredFiles = new Set(allFiles.map(uri => uri.fsPath)); | ||
| for (const uri of nonIgnoredFiles) { | ||
| gitIgnoredFiles.delete(uri.fsPath); | ||
| } | ||
|
|
||
| return gitIgnoredFiles; |
Copilot
AI
Jan 13, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The refactored method no longer has error handling that was present in the original implementation. The original code wrapped the glob expansion and git ignore checking in a try-catch block, logging any errors and returning an empty Set on failure. Consider adding similar error handling to ensure robustness, especially since this method is called asynchronously without awaiting in createWorktree.
See below for a potential fix:
try {
const filePattern = worktreeIncludeFiles
.map(pattern => new RelativePattern(this.root, pattern));
// Get all files matching the globs (no ignore files applied)
const allFiles = await workspace.findFiles2(filePattern, {
useExcludeSettings: ExcludeSettingOptions.None,
useIgnoreFiles: { local: false, parent: false, global: false }
});
// Get files matching the globs with git ignore files applied
const nonIgnoredFiles = await workspace.findFiles2(filePattern, {
useExcludeSettings: ExcludeSettingOptions.None,
useIgnoreFiles: { local: true, parent: true, global: true }
});
// Files that are git ignored = all files - non-ignored files
const gitIgnoredFiles = new Set(allFiles.map(uri => uri.fsPath));
for (const uri of nonIgnoredFiles) {
gitIgnoredFiles.delete(uri.fsPath);
}
return gitIgnoredFiles;
} catch (err) {
this.logger.warn(`[Repository][_getWorktreeIncludeFiles] Failed to resolve worktree include files: ${err}`);
return new Set<string>();
}
No description provided.