-
Notifications
You must be signed in to change notification settings - Fork 9.1k
fix(cli): Do not refresh pending settings after setting a setting. #11167
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
Conversation
When a setting is reset, the pending settings were being refreshed from the saved state, which would cause the UI to not reflect the reset value. This change removes the refresh, so that the pending settings now reflect the reset value.
Summary of ChangesHello @galz10, 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 resolves a user interface inconsistency within the settings dialog. Previously, when a user reset a setting to its default, the UI would not correctly display this change, especially if another setting was subsequently modified. The core of the fix involves removing a redundant state refresh operation that was inadvertently causing the UI to revert the display of the reset setting, thereby enhancing the reliability and accuracy of the settings management experience. 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.
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 aims to fix a bug in the settings dialog where resetting a setting would discard other pending changes. The change correctly removes the line that was resetting the entire pendingSettings state. However, this fix is incomplete and introduces a new bug: the UI no longer updates to show that a setting has been reset to its default value. My review includes a critical comment with a suggested fix to correctly update the pendingSettings state for only the reset setting, which resolves both issues.
|
Size Change: -122 B (0%) Total Size: 17.8 MB ℹ️ View Unchanged
|
| // Refresh pending settings from the saved state | ||
| setPendingSettings( | ||
| structuredClone(settings.forScope(selectedScope).settings), | ||
| ); |
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.
lets add a test so we don't regress this. allowing users to be confident that their settings aren't corrupted is crucial.
TLDR
This PR fixes a bug in the settings dialog where resetting a setting to its default value did not update the UI to reflect the change.
Dive Deeper
When a user modified a setting and then changed the setting bellow it, the UI would reset the previous setting to it's default state.
This change removes the unnecessary refresh of pending settings after a reset, ensuring that the UI correctly displays the set setting.
Reviewer Test Plan
gemini settingsTesting Matrix
Linked issues / bugs