-
Notifications
You must be signed in to change notification settings - Fork 9.7k
feat: Multi-Directory Workspace Support (part 3: configuration in settings.json) #5354
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
feat: Multi-Directory Workspace Support (part 3: configuration in settings.json) #5354
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.
Summary of Changes
Hello @y-okt, 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 completes the multi-directory workspace support by introducing persistent configuration options via settings.json. It allows users to define includeDirectories that are loaded automatically on startup and provides a clearWorkspaceDirsOnRefresh setting to control how workspace directories behave during memory refreshes.
Highlights
- Persistent Directory Inclusion: Introduced
includeDirectoriesinsettings.jsonto allow users to specify directories that should always be included in the workspace context upongemini-clistartup. - Workspace Directory Refresh Control: Added
clearWorkspaceDirsOnRefreshtosettings.json, which, when set totrue, will clear any dynamically added workspace directories (those not specified insettings.json) during a memory refresh operation. - Robust Path Resolution: Implemented a new
resolvePathutility to correctly handle and expand common path notations like~(home directory) and%userprofile%withinincludeDirectoriesspecified insettings.json. - CLI Argument Integration: Ensured that
includeDirectoriesspecified insettings.jsonare properly combined with those provided via the--include-directoriescommand-line argument, providing a unified set of initial workspace directories. - Enhanced Test Coverage: Added comprehensive unit tests to validate the correct merging and resolution of
includeDirectoriesfrom various configuration scopes (system, user, workspace) and command-line arguments.
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 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 or fill out our survey 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request completes the multi-directory workspace support by adding configuration options in settings.json for includeDirectories and clearWorkspaceDirsOnRefresh. The changes are well-structured, introducing a new resolvePath utility and updating configuration loading across the CLI and core packages.
My review has identified a critical issue where directories provided via CLI arguments are lost during a memory refresh, as well as a bug in path resolution for tilde (~) prefixed paths. I've also included a minor suggestion to improve type safety in the new setDirectories method. Addressing these points will ensure the new features are robust and behave as expected.
|
@allenhutchison @jacob314 Hi, could you please reivew this PR to add configurations for multi-directory workspace support? This is the final PR for this multi-directory workspace feature, described in #1118. Thanks! |
75ec800 to
ed9bd21
Compare
|
Thank you for this excellent contribution! The multi-directory support is a powerful feature, and the implementation is very clean. I have one suggestion for improvement regarding the new settings. While the clearWorkspaceDirsOnRefresh setting makes sense, I believe a more useful feature for users would be to control whether GEMINI.md memory files are loaded from these additional directories. This would give users more fine-grained control over the context that gets loaded, especially when including shared libraries or other projects that might have their own GEMINI.md files. Could we replace clearWorkspaceDirsOnRefresh with a new boolean setting called loadMemoryFromIncludeDirectories? Suggested Change:
This change would make the multi-directory feature even more flexible and prevent unexpected context from being loaded. What are your thoughts on this suggestion? I'm happy to discuss it further. |
f453bd9 to
f73ba69
Compare
|
@allenhutchison Hi, thank you very much for reviewing! |
…eDirectories flag
allenhutchison
left a comment
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.
Great work on this, @y-okt! The implementation for multi-directory support is very clean and a fantastic addition to the CLI. Thank you for being so receptive to the feedback.
We're almost ready to approve the pull request. It looks like there might be a few merge conflicts to resolve with the main branch, but once that's sorted, this will be ready to go. This is an excellent contribution
80a4ffa to
f6a322f
Compare
|
@allenhutchison Thank you for reviewing! I've just resolved merge conflicts. Could you please check? |
allenhutchison
left a comment
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 is great. Thank you for all of your hard work.
…tings.json) (#5354) Co-authored-by: Allen Hutchison <[email protected]>
|
@allenhutchison Thank you very much for your reviews and approvals! I really enjoyed contributing to this issue! |
… 3: configuration in settings.json) (google-gemini#5354)
…tings.json) (google-gemini#5354) Co-authored-by: Allen Hutchison <[email protected]>
| // Handle comma-separated values | ||
| dirs.flatMap((dir) => dir.split(',').map((d) => d.trim())), | ||
| }) | ||
| .option('load-memory-from-include-directories', { |
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.
@y-okt would you mind doing a follow up to drop this from command line args (only rely on settings)? We've been trying to reduce the number of command line surface area we have
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.
@NTaylorMullen Hi, understood, let me work on this!
…tings.json) (google-gemini#5354) Co-authored-by: Allen Hutchison <[email protected]>
…tings.json) (google-gemini#5354) Co-authored-by: Allen Hutchison <[email protected]>
…tings.json) (google-gemini#5354) Co-authored-by: Allen Hutchison <[email protected]>
…tings.json) (google-gemini#5354) Co-authored-by: Allen Hutchison <[email protected]>
TLDR
In #4605 and #5241, I implemented the Multi-Directory Workspace support. These can:
--include-directoriesoption, which can include directories when starting up the gemini-cli/directorycommand, which can add/show the directory from CLIIn this PR, inspired by comments from @allenhutchison, implemented
includeDirectoriesandclearWorkspaceDirsOnRefreshfields insettings.json.includeDirectories(default: null) can add directories from the very beginning of gemini-cli launching.loadMemoryFromIncludeDirectories(default: false) can load the GEMINI.md file when "/dir add" or the memory is refreshed.This is the final PR for the multi-directory support.
Dive Deeper
The same as TLDR.
Reviewer Test Plan
loadMemoryFromIncludeDirectoriesfield enabled in settings.json)project dir's settings.json
home dir's settings.json
Verifying that directories are successfully added from settings.json
Without loadMemoryFromIncludeDirectories
With loadMemoryFromIncludeDirectories
Testing Matrix
Linked issues / bugs
Resolves #1118