-
Notifications
You must be signed in to change notification settings - Fork 10.2k
fix(cli): add enableShellOutputEfficiency to settings schema #15560
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
base: main
Are you sure you want to change the base?
fix(cli): add enableShellOutputEfficiency to settings schema #15560
Conversation
Summary of ChangesHello @Mapleeeeeeeeeee, 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 introduces the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. 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.
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 correctly adds the enableShellOutputEfficiency setting to the settings schema and wires it up in the configuration loading. However, there is a small issue with how the default value for this new setting is handled. I've provided a suggestion to ensure the default behavior is correctly applied when the setting isn't explicitly configured by the user.
packages/cli/src/config/config.ts
Outdated
| enableShellOutputEfficiency: | ||
| settings.tools?.shell?.enableShellOutputEfficiency, |
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.
The enableShellOutputEfficiency setting is not being defaulted correctly. If a user hasn't configured this value in their settings files, settings.tools?.shell?.enableShellOutputEfficiency will be undefined. This will likely be treated as false by downstream code, disabling the feature, which contradicts the intended default of true.
To ensure the default value is applied correctly, you should use the nullish coalescing operator (??), similar to how other boolean settings like enableInteractiveShell are handled.
| enableShellOutputEfficiency: | |
| settings.tools?.shell?.enableShellOutputEfficiency, | |
| enableShellOutputEfficiency: | |
| settings.tools?.shell?.enableShellOutputEfficiency ?? true, |
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.
Good catch! I've pushed a fix in commit b160aef - added ?? true to maintain the default behavior when the setting is undefined.
Head branch was pushed to by a user without write access
b160aef to
641c151
Compare
|
Pushed CI fixes. Could you please approve the workflow run? Thank you! |
What
Add
enableShellOutputEfficiencysetting to the settings schema so users can configure it via settings file.Why
Fixes #12546
The
enableShellOutputEfficiencyoption was introduced in PR #10651 but was not added to the settings schema. This means users cannot configure this setting through their settings file - it can only be controlled programmatically.How
enableShellOutputEfficiencyboolean setting totools.shellsection insettingsSchema.tscli/src/config/config.tsto pass the setting value toConfigParametersNotes
trueto maintain existing behaviortools.shell.enableShellOutputEfficiencyto be consistent with other shell-related settingsnpm run preflightcompleted successfully (exit code 0)