-
Notifications
You must be signed in to change notification settings - Fork 1.4k
feat(vscode): enhance mention parsing by introducing MentionType and … #4083
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
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 PR enhances mention parsing and quick pick functionality in the VSCode client by introducing a new MentionType enum and updating the parsing logic as well as the corresponding UI flows. Key changes include:
- Changing the mentions field from an array of strings to an array of objects conforming to the new Mention interface.
- Adding context-based quick pick functionality to allow users to select between file and symbol mention types.
- Minor cleanup in branchQuickPick.ts by removing outdated comments.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
clients/vscode/src/inline-edit/util.ts | Refactored mention parsing to use a Mention interface and added a mentionType field. |
clients/vscode/src/inline-edit/util.test.ts | Test code has been commented out and will need to be updated to match the new Mention type. |
clients/vscode/src/inline-edit/quickPick.ts | Updated quick pick UI to handle both file and symbol selections and introduced SymbolPick. |
clients/vscode/src/inline-edit/branchQuickPick.ts | Removed an outdated comment to simplify caching details. |
Comments suppressed due to low confidence (1)
clients/vscode/src/inline-edit/util.test.ts:1
- Test cases are commented out and do not validate the updated Mention structure. Please update and re-enable these tests to ensure that mentions are parsed as objects with both text and type.
// import { describe } from "mocha";
private resultDeffer = new Deferred<SymbolSelectionResult | 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.
Typo detected: 'resultDeffer' should be renamed to 'resultDeferred' for clarity.
private resultDeffer = new Deferred<SymbolSelectionResult | undefined>(); | |
private resultDeferred = new Deferred<SymbolSelectionResult | undefined>(); |
Copilot is powered by AI, so mistakes are possible. Review output carefully before use.
8b5cfdd
to
112efa4
Compare
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 PR enhances the mention parsing functionality in the VSCode client by introducing a new MentionType enum and a Mention interface, and updating the corresponding parse logic and tests.
- Introduces MentionType enum and Mention interface in util.ts
- Updates the parseUserCommand function to return mentions as an array of Mention objects and include a mentionType field
- Adjusts tests in util.test.ts to account for the new mention structure and removes an outdated comment from branchQuickPick.ts
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
File | Description |
---|---|
clients/vscode/src/inline-edit/util.ts | Adds MentionType enum, Mention interface, and updates parseUserCommand to use these types |
clients/vscode/src/inline-edit/util.test.ts | Updates tests to reflect the new Mention structure and additional mentionType field |
clients/vscode/src/inline-edit/branchQuickPick.ts | Removes an outdated comment regarding cache entry simplification |
…serCommandQuickpick
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 PR enhances the mention parsing for the VS Code client by introducing a new MentionType enum and updating both the parsing logic and corresponding tests. Key changes include:
- Introducing the MentionType enum and updating the Mention interface in util.ts.
- Updating tests in util.test.ts to validate the new mention model.
- Adjusting quick pick flows in quickPick.ts to support both file and symbol mentions, as well as renaming deferred variables for clarity.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
clients/vscode/src/inline-edit/util.ts | Introduces MentionType, updates Mention and parseUserCommand to use the new type model. |
clients/vscode/src/inline-edit/util.test.ts | Updates tests to reflect the new Mention interface and mentionType property. |
clients/vscode/src/inline-edit/quickPick.ts | Revises quick pick UI flows to incorporate file/symbol mentions and renames deferred variables. |
clients/vscode/src/inline-edit/branchQuickPick.ts | Removes outdated caching comment. |
clients/vscode/src/findSymbols.ts | Implements symbol lookup with enhanced processing and icon mapping. |
Comments suppressed due to low confidence (2)
clients/vscode/src/inline-edit/quickPick.ts:191
- [nitpick] Using file labels directly as keys in 'fileContextLabelToUriMap' might lead to conflicts if the same label appears for both file and symbol contexts. Consider using a composite key or a unique identifier to avoid potential collisions.
this.fileContextLabelToUriMap.set(this._pendingFileContext.label, {
clients/vscode/src/findSymbols.ts:99
- [nitpick] The computed variable 'fullName' is used only in recursive calls to update the context, but the parent symbol's containerName is not updated with it. Verify if this is the intended behavior or if using 'fullName' consistently would yield clearer symbol hierarchies.
flattenDocumentSymbols(symbol.children, fullName, uri, result);
cc @zhanba |
… for improved efficiency
…ker logic in UserCommandQuickpick
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.
LGTM
still working
screenshots