-
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
Conversation
- Enhanced state-manager.ts to accept full file path and package name parameters - Added package name extraction logic to orchestrator.ts - Updated status messages to display package names and file paths during indexing - Improved user visibility into what is being indexed in multi-package workspaces Fixes #6907
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 reviewed my own code and found issues. Shocking, I know.
| }` | ||
| // Extract file information for display | ||
| const fileBasename = currentFilePath ? path.basename(currentFilePath) : "..." | ||
| const packageDisplay = packageName ? `${packageName}` : "" |
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.
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}` |
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.
Have we considered using the i18n system for these new status messages? The rest of the codebase uses t() for user-facing strings, and it would maintain consistency.
|
|
||
| // Extract npm package name from package.json if close to the file | ||
| try { | ||
| const dirPath = path.dirname(filePath) |
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 variable is declared but never used. Should we remove it?
| const dirPath = path.dirname(filePath) | ||
|
|
||
| // Check if it's part of the src/ directory structure | ||
| const srcMatch = filePath.match(/src[\\/\\\\]([^\\/\\\\]+)/) |
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.
Is this the intended behavior? A file at src/components/Button.tsx would return "components" as the package name, which seems misleading. Should we reconsider this logic or perhaps only use this as a last resort?
| * @param filePath Path to extract package name from | ||
| * @returns Package name if identified, otherwise undefined | ||
| */ | ||
| private extractPackageName(filePath?: string): string | undefined { |
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?
|
Closing, the author of the issue is interesten in implementing this |
This PR implements the enhancement described in #6907 to show package names and file paths during the indexing process, providing better visibility into what is being indexed, especially in multi-package workspaces.
Changes Made
1. Enhanced state-manager.ts
2. Enhanced orchestrator.ts
3. File Watcher Integration
Benefits
Testing
Example Output
Before: Processing 5 / 10 files. Current: index.ts
After: Processing 5 / 10 files. Package: @myorg/package, File: index.ts
Or: Processing 5 / 10 files. Path: /workspace/src/utils/index.ts
Fixes #6907
Important
Enhance indexing process to display package names and file paths, improving visibility and debugging in multi-package workspaces.
reportFileQueueProgressinstate-manager.tsnow accepts full file paths and optional package names.extractPackageNameinorchestrator.tsextracts package names from file paths, handlingnode_modules, monorepo packages, andsrcdirectories.state-manager.ts.file-watcher.tsalready provides full file paths, no changes needed.pathmodule import instate-manager.tsfor basename extraction.This description was created by
for da160fe. You can customize this summary. It will automatically update as commits are pushed.