forked from microsoft/vscode
-
Notifications
You must be signed in to change notification settings - Fork 135
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
Fixed iconbar sync state #197
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
👍 Looks good to me! Reviewed everything up to bd8f61f in 2 minutes and 57 seconds
More details
- Looked at
178
lines of code in4
files - Skipped
0
files when reviewing. - Skipped posting
13
drafted comments based on config settings.
1. src/vs/workbench/browser/parts/editor/editor.contribution.ts:76
- Draft comment:
Good addition: importing and registering the new SwitchToPearAIIntegrationAction ensures that the PearAI iconbar state synchronizes with the rest of the UI. Make sure its integration is tested across views. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
2. src/vs/workbench/browser/parts/paneCompositePart.ts:121
- Draft comment:
The InventoryIcons update for the 'agent' icon now uses the updated command string ('pearai-roo-cline.SidebarProvider.focus'), ensuring consistency with the shared constants. Confirm that this aligns with the extension’s implementation. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
3. src/vs/workbench/browser/parts/panel/panelActions.ts:32
- Draft comment:
SwitchToPearAIIntegrationAction’s implementation is straightforward. Its use of default 'chat' view and toggling the auxiliary bar visibility looks good. Ensure that the asynchronous openView call behaves as expected in all cases. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
4. src/vs/workbench/browser/parts/panel/panelActions.ts:418
- Draft comment:
In MovePearExtensionToAuxBarAction, replacing hard-coded view container IDs with shared constants (e.g., PearAIChatExtensionId, etc.) improves maintainability and consistency. Confirm that no legacy references are missed. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
5. src/vs/workbench/services/views/pearai/pearaiViewsShared.ts:8
- Draft comment:
The declaration of PEARAI_VIEWS using 'as const' provides better type safety. This approach nicely centralizes the view identifiers for later reuse. Ensure that any changes in IDs propagate correctly across consumers. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
6. src/vs/workbench/browser/parts/editor/editor.contribution.ts:76
- Draft comment:
Added registration of SwitchToPearAIIntegrationAction. Ensure the localization keys and command registration are consistent. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50%
The comment asks to verify localization keys and command registration, but:
- This is a simple action registration that follows the standard pattern used throughout the file
- The localization would be handled in the action class itself, not here
- The command registration is done via registerAction2() which is the standard way
- The comment is asking for verification rather than pointing out a specific issue
- Without seeing the action class implementation, we can't verify these aspects anyway
I could be wrong if there are special localization requirements for PearAI integration that I'm not aware of. The action class might need specific command registration patterns.
Even if there are special requirements, those would be handled in the action class itself, not at the registration point. The registration follows the standard pattern used throughout this file.
Delete this comment as it asks for verification without pointing out any specific issues, and the registration follows standard patterns.
7. src/vs/workbench/browser/parts/paneCompositePart.ts:120
- Draft comment:
Updated the 'agent' icon's command from 'roo-cline.SidebarProvider.focus' to 'pearai-roo-cline.SidebarProvider.focus'. Verify that this new ID is correctly handled elsewhere. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
The comment is asking the PR author to verify that the new ID is correctly handled elsewhere, which violates the rule against asking the author to ensure behavior is intended or to double-check things. The comment does not provide a specific suggestion or ask for a specific test to be written.
8. src/vs/workbench/browser/parts/panel/panelActions.ts:431
- Draft comment:
Refactored MovePearExtensionToAuxBarAction to use imported constants from pearaiViewsShared.ts and renamed the variable from 'creatorViewContainer' to 'agentViewContainer' for clarity. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
9. src/vs/workbench/services/views/pearai/pearaiViewsShared.ts:1
- Draft comment:
Defined PearAIView type and corresponding extension IDs and view mapping (PEARAI_VIEWS) to improve maintainability and consistency for PearAI integration. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
This comment is purely informative, describing what was done without providing any actionable feedback or suggestions. It does not align with the rules for useful comments.
10. src/vs/workbench/browser/parts/editor/editor.contribution.ts:307
- Draft comment:
Typo: The identifier 'CopyEditorToNewindowAction' appears to be misspelled. It should likely be renamed to 'CopyEditorToNewWindowAction' for consistency. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
11. src/vs/workbench/browser/parts/paneCompositePart.ts:155
- Draft comment:
Typo: The variable name 'headerFooterCompositeBarDispoables' appears to be misspelled. It should be 'headerFooterCompositeBarDisposables'. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
12. src/vs/workbench/browser/parts/panel/panelActions.ts:417
- Draft comment:
Typo in comment: 'loaction' should be corrected to 'location'. - Reason this comment was not posted:
Comment was on unchanged code.
13. src/vs/workbench/services/views/pearai/pearaiViewsShared.ts:6
- Draft comment:
Typo noticed: The extension ID 'workbench.view.extension.pearai-roo-cline' on line 6 contains 'cline'. If this was intended to be 'client' (to match typical naming conventions), please update it accordingly. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
Extension IDs are typically very intentional and carefully chosen identifiers. The consistent use of 'cline' in multiple places suggests this is intentional. Without more context about what 'roo-cline' represents, assuming it's a typo is speculative. The comment is asking for verification rather than pointing out a definite issue.
The extension ID could genuinely be a typo that was copy-pasted multiple times. The unusual naming could cause confusion for future developers.
Even if it were a typo, extension IDs are critical identifiers that would be carefully reviewed. The consistent usage and the specific nature of 'roo-cline' suggests this is an intentional name.
Delete the comment. It's speculative, asks for verification, and lacks strong evidence that there's actually an issue to fix.
Workflow ID: wflow_6ksM09X3WLHQflRo
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
#189
Important
Synchronizes PearAI icon bar state by adding a new action and updating commands for PearAI views.
SwitchToPearAIIntegrationAction
inpanelActions.ts
to switch to PearAI integration, defaulting to 'chat' view.SwitchToPearAIIntegrationAction
ineditor.contribution.ts
.paneCompositePart.ts
topearai-roo-cline.SidebarProvider.focus
.PearAIView
type andPEARAI_VIEWS
mapping inpearaiViewsShared.ts
.MovePearExtensionToAuxBarAction
to use constants frompearaiViewsShared.ts
.MovePearExtensionToAuxBarAction
inpanelActions.ts
.This description was created by
for bd8f61f. It will automatically update as commits are pushed.