Add additional public extension APIs #5595
Conversation
There was a problem hiding this comment.
Summary of Changes
Hello @DanTup, 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 significantly enhances the Dart extension's public API surface, providing external extensions with more powerful programmatic access to workspace analysis data and SDK command execution. This will enable richer integrations and automation possibilities for developers building on top of the Dart/Flutter extension.
Highlights
- New Public Extension APIs: I've expanded the public extension API by introducing
workspaceandsdkproperties. These new APIs allow other extensions to programmatically interact with the Dart/Flutter workspace and execute SDK commands. - Workspace API Features: The new
workspaceAPI provides methods to retrieve the outline of a given document (getOutline) and to discover all Dart/Flutter project folders within the workspace (findProjectFolders). - SDK Command Execution API: The new
sdkAPI enables running Dart and Pub commands directly. It providesrunDartandrunPubmethods, which return a detailed result including stdout, stderr, and the exit code of the executed command. - Internal API Model Updates: The internal
DartExtensionApiModel(formerlyDartExtensionApiData) has been updated to hold references to core services like theLspAnalyzer,ProjectFinder, andSdkCommands. This allows the new public APIs to leverage existing extension functionalities. - Enhanced Command Execution Results: The underlying command execution logic in
SdkCommandshas been refactored to capture and returnstdoutandstderrstreams, along with theexitCode, providing more comprehensive results for executed commands.
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and 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 to provide feedback.
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
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces a set of public APIs for the Dart extension, allowing other extensions to interact with it. The new APIs cover workspace-related functionalities like getting document outlines and finding project folders, as well as SDK-related actions like running Dart and Pub commands.
The changes are well-structured, with clear interfaces for the public API and corresponding implementation classes. The extension's activation and deactivation logic has been correctly updated to manage the lifecycle of these new components. Additionally, new tests have been added to validate the functionality of the new APIs.
I've identified a critical issue with nested promises that needs to be addressed, along with a couple of other high and medium severity issues related to correctness and code consistency.
Once these points are addressed, the PR will be in great shape.
99a2569 to
cd020e0
Compare
72c61c1 to
b74d951
Compare
|
/gemini review |
There was a problem hiding this comment.
Pull Request Overview
This PR adds new public extension APIs for workspace and SDK interactions, updates command execution to use a structured RunProcessResult, and adjusts related tests and dependency injection.
- Tests now import and assert on
RunProcessResult.exitCodeinstead of raw exit codes. RunProcessResultis standardized to always include an exit code, and process execution pathways (including chained processes) are updated.- Extension API surface expanded with
PublicWorkspaceandPublicSdkinterfaces;ProjectFinderis injected into views and wired into the API model.
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/multi_root/commands.test.ts | Updated tests to expect RunProcessResult.exitCode for dart.getPackages. |
| src/test/multi_project_folder/commands.test.ts | Same as above for multi-project-folder tests. |
| src/test/flutter_bazel/commands/doctor.test.ts | Updated flutter.doctor test to assert on RunProcessResult.exitCode. |
| src/test/flutter/commands/outdated.test.ts | Updated flutter.packages.outdated test to assert on RunProcessResult.exitCode. |
| src/test/flutter/commands/doctor.test.ts | Updated flutter.doctor test to assert on RunProcessResult.exitCode. |
| src/test/dart/extension_api.test.ts | Added tests for new workspace and sdk APIs in the public extension. |
| src/test/dart/commands/outdated.test.ts | Updated pub.outdated test to assert on RunProcessResult.exitCode. |
| src/shared/processes.ts | Changed RunProcessResult constructor to require a numeric exit code and default. |
| src/extension/views/packages_view.ts | Injected ProjectFinder via constructor instead of instantiating internally. |
| src/extension/extension.ts | Renamed API data store to extensionApiModel, wired up new API hooks, and cleared. |
| src/extension/commands/sdk.ts | Refactored command runners to return RunProcessResult, added runDartCommand. |
| src/extension/commands/flutter.ts | Adjusted flutterClean to return RunProcessResult. |
| src/extension/api/interfaces.ts | Defined PublicWorkspace, PublicSdk, and related run option/result interfaces. |
| src/extension/api/extension_api.ts | Implemented new workspace and SDK API surfaces on the global API model. |
Comments suppressed due to low confidence (2)
src/extension/commands/sdk.ts:116
- Wrapping
vs.window.withProgressinPromise.resolvecauses the method to returnPromise<Thenable<RunProcessResult>>instead of a directPromise<RunProcessResult>. Consider returning theThenabledirectly or awaiting it so the signature matchesPromise<RunProcessResult | undefined>.
return Promise.resolve(vs.window.withProgress({
src/test/multi_root/commands.test.ts:17
- The test now expects a
RunProcessResult, but thedart.getPackagescommand registration must be updated to return aRunProcessResultobject instead of a raw exit code number. Without that change, this test will fail at runtime.
const res: RunProcessResult = await vs.commands.executeCommand("dart.getPackages", helloWorldMainFile);
There was a problem hiding this comment.
Code Review
This pull request introduces a significant set of new public APIs for the extension, allowing other extensions to interact with workspace information, outlines, and run SDK commands. A key part of this change is a refactoring of process execution to return a structured RunProcessResult object containing stdout, stderr, and the exit code, instead of just the exit code. This is a great improvement for API consumers.
The changes are well-implemented and consistently applied across the codebase, including updates to tests.
I've found one high-severity issue related to process cancellation that should be addressed.
No description provided.