feat: Add explore subcommand for extension #11846
Conversation
Summary of ChangesHello @JayadityaGit, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the CLI by adding an Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a new explore subcommand for extensions, which opens the extensions documentation page in the browser. The implementation follows the existing pattern of the /docs command and includes unit tests. My review focuses on improving the robustness of the new feature by adding error handling for the browser-opening operation and suggesting a corresponding test case to ensure correctness. Overall, the changes are well-structured and clear.
| // Ensure 'open' was not called in test environment | ||
| expect(open).not.toHaveBeenCalled(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
While the existing tests cover the success paths well, a key scenario is missing. It's important to also test the failure case where open() might reject (e.g., if a browser cannot be opened). Adding a test for this scenario would ensure your new error handling logic works as expected and prevents unhandled promise rejections.
| }, | ||
| Date.now(), | ||
| ); | ||
| await open(extensionsUrl); |
There was a problem hiding this comment.
The call to open() can fail for various reasons (e.g., no default browser configured, permissions issues). An unhandled promise rejection here could lead to an uncaught exception, potentially providing a poor user experience. It's better to wrap this call in a try...catch block to gracefully handle any errors and inform the user.
try {
await open(extensionsUrl);
} catch (error) {
context.ui.addItem(
{
type: MessageType.ERROR,
text: `Failed to open browser. Please open this URL manually: ${extensionsUrl}`,
},
Date.now()
);
}| import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; | ||
| import { type ExtensionUpdateAction } from '../state/extensions.js'; | ||
|
|
||
| // Mock the 'open' library similar to docsCommand.test.ts |
| beforeEach(() => { | ||
| vi.resetAllMocks(); | ||
| mockGetExtensions.mockReturnValue([]); | ||
| // Reset the `open` mock as done in docsCommand.test.ts |
| context.ui.addItem( | ||
| { | ||
| type: MessageType.INFO, | ||
| text: `Please open the following URL in your browser to explore extensions:\\n${extensionsUrl}`, |
There was a problem hiding this comment.
"View available extensions at ${extensionsUrl}"
| { | ||
| type: MessageType.ERROR, | ||
| text: `Failed to open browser. Please open this URL manually: ${extensionsUrl}`, | ||
| }, |
There was a problem hiding this comment.
"Failed to open browser. Check out the extensions gallery at ${extensions Url}
chrstnb
left a comment
There was a problem hiding this comment.
Couple of nits but overall looks good!
|
Thank you so much for the review, @chrstnb! I’ll address these right away. |
|
Also, I’m participating in the Hacktoberfest event. Could you please add the hacktoberfest-accepted label to this PR when you get a chance? Thank you so much, @chrstnb! 🙏 |
TL;DR
This PR introduces a new
exploresubcommand under the/extensioncommand that opens the extensions page in the user's default browser.It follows the same design and implementation pattern as the existing
/docscommand.Dive Deeper
The purpose of this addition is to make it easier for users to discover and explore available extensions directly from the CLI.
The implementation reuses the existing open-library mechanism used by
/docs, ensuring consistent behavior and proper handling in both production and test environments.Comprehensive unit tests are included to validate all key scenarios, ensuring full alignment with the project’s coding conventions and test standards.
Testing Matrix