-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat: show package names and file paths during indexing #6908
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -45,11 +45,11 @@ export class CodeIndexOrchestrator { | |
| if (totalInBatch > 0 && this.stateManager.state !== "Indexing") { | ||
| this.stateManager.setSystemState("Indexing", "Processing file changes...") | ||
| } | ||
| this.stateManager.reportFileQueueProgress( | ||
| processedInBatch, | ||
| totalInBatch, | ||
| currentFile ? path.basename(currentFile) : undefined, | ||
| ) | ||
|
|
||
| // Extract package name from file path if possible | ||
| let packageName = this.extractPackageName(currentFile) | ||
|
|
||
| this.stateManager.reportFileQueueProgress(processedInBatch, totalInBatch, currentFile, packageName) | ||
| if (processedInBatch === totalInBatch) { | ||
| // Covers (N/N) and (0/0) | ||
| if (totalInBatch > 0) { | ||
|
|
@@ -291,4 +291,40 @@ export class CodeIndexOrchestrator { | |
| public get state(): IndexingState { | ||
| return this.stateManager.state | ||
| } | ||
|
|
||
| /** | ||
| * Extracts package name from a file path | ||
| * @param filePath Path to extract package name from | ||
| * @returns Package name if identified, otherwise undefined | ||
| */ | ||
| private extractPackageName(filePath?: string): string | undefined { | ||
| if (!filePath) return undefined | ||
|
|
||
| // Check if path contains node_modules | ||
| const nodeModulesMatch = filePath.match(/node_modules[\\/\\\\](@[^\\/\\\\]+[\\/\\\\][^\\/\\\\]+|[^\\/\\\\]+)/) | ||
| if (nodeModulesMatch && nodeModulesMatch[1]) { | ||
| return nodeModulesMatch[1] | ||
| } | ||
|
|
||
| // Check if path contains packages directory structure (monorepo) | ||
| const packagesMatch = filePath.match(/packages[\\/\\\\]([^\\/\\\\]+)/) | ||
| if (packagesMatch && packagesMatch[1]) { | ||
| return packagesMatch[1] | ||
| } | ||
|
|
||
| // Extract npm package name from package.json if close to the file | ||
| try { | ||
| const dirPath = path.dirname(filePath) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This variable is declared but never used. Should we remove it? |
||
|
|
||
| // Check if it's part of the src/ directory structure | ||
| const srcMatch = filePath.match(/src[\\/\\\\]([^\\/\\\\]+)/) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this the intended behavior? A file at |
||
| if (srcMatch && srcMatch[1]) { | ||
| return srcMatch[1] | ||
| } | ||
| } catch (error) { | ||
| // Ignore errors in package name extraction | ||
| } | ||
|
|
||
| return undefined | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| import * as vscode from "vscode" | ||
| import * as path from "path" | ||
|
|
||
| export type IndexingState = "Standby" | "Indexing" | "Indexed" | "Error" | ||
|
|
||
|
|
@@ -78,7 +79,12 @@ export class CodeIndexStateManager { | |
| } | ||
| } | ||
|
|
||
| public reportFileQueueProgress(processedFiles: number, totalFiles: number, currentFileBasename?: string): void { | ||
| public reportFileQueueProgress( | ||
| processedFiles: number, | ||
| totalFiles: number, | ||
| currentFilePath?: string, | ||
| packageName?: string, | ||
| ): void { | ||
| const progressChanged = processedFiles !== this._processedItems || totalFiles !== this._totalItems | ||
|
|
||
| if (progressChanged || this._systemStatus !== "Indexing") { | ||
|
|
@@ -89,9 +95,16 @@ export class CodeIndexStateManager { | |
|
|
||
| let message: string | ||
| if (totalFiles > 0 && processedFiles < totalFiles) { | ||
| message = `Processing ${processedFiles} / ${totalFiles} ${this._currentItemUnit}. Current: ${ | ||
| currentFileBasename || "..." | ||
| }` | ||
| // Extract file information for display | ||
| const fileBasename = currentFilePath ? path.basename(currentFilePath) : "..." | ||
| const packageDisplay = packageName ? `${packageName}` : "" | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These template literals are redundant. Could we simplify to just: |
||
| const pathDisplay = currentFilePath ? `${currentFilePath}` : "" | ||
|
|
||
| if (packageName) { | ||
| message = `Processing ${processedFiles} / ${totalFiles} ${this._currentItemUnit}. Package: ${packageDisplay}, File: ${fileBasename}` | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Have we considered using the i18n system for these new status messages? The rest of the codebase uses |
||
| } else { | ||
| message = `Processing ${processedFiles} / ${totalFiles} ${this._currentItemUnit}. Path: ${pathDisplay}` | ||
| } | ||
| } else if (totalFiles > 0 && processedFiles === totalFiles) { | ||
| message = `Finished processing ${totalFiles} ${this._currentItemUnit} from queue.` | ||
| } else { | ||
|
|
||
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.
This new method lacks test coverage. Could we add unit tests to verify the package name extraction logic works correctly for various path patterns?