fix(config):Reviving CustomWitty feature - #8432
Conversation
There was a problem hiding this comment.
Summary of Changes
Hello @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 introduces a crucial improvement to how user settings are updated and migrated, specifically addressing an issue where parts of the 'ui' object were being overwritten instead of merged. By implementing a smart merging strategy for objects and maintaining replacement behavior for arrays, the system now handles settings updates more robustly, ensuring user configurations like custom witty phrases are preserved while allowing other settings to be updated safely. This makes the settings management more intuitive and prevents data loss.
Highlights
- Redemption PR: This pull request is a redemption for a previous attempt (#7782) to fix a settings bug, addressing issues that arose from an unsynced branch.
- Smart Object Merging: Implements logic to merge properties of the 'ui' object when updating settings, preventing accidental overwrites of existing configurations like 'customWittyPhrases'.
- Array Replacement: Ensures that array-type settings (e.g., 'customWittyPhrases') are still replaced entirely when updated, maintaining expected behavior for lists.
- Improved Settings Migration: Enhances the 'migrateSettingsToV2' function to intelligently merge objects and replace arrays during settings migration, making updates safer and more intuitive.
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. 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
-
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 addresses a bug where parts of the ui settings object were being overwritten during settings migration. The fix introduces a deep merge for objects when migrating settings from v1 to v2 format, ensuring that existing nested settings are preserved. The changes are well-contained within the migration logic and are accompanied by a new test case that verifies the fix for customWittyPhrases. The implementation correctly handles the merging of objects while continuing to replace arrays, as intended. Overall, the changes look solid and effectively resolve the described issue.
|
@richieforeman, please take a look when you get a chance — I’m happy to refine further if needed |
|
All preflight checks are passing locally. I'm not sure why one of the windows test is failing. |
|
This PR has already been reviewed and approved. Could we please get it merged when convenient? It restores the customWittyFeature, which is currently not working. Thanks! |
Co-authored-by: Richie Foreman <richie.foreman@gmail.com> Co-authored-by: Arya Gummadi <aryagummadi@google.com>
Co-authored-by: Richie Foreman <richie.foreman@gmail.com> Co-authored-by: Arya Gummadi <aryagummadi@google.com>
Co-authored-by: Richie Foreman <richie.foreman@gmail.com> Co-authored-by: Arya Gummadi <aryagummadi@google.com>
TLDR
This PR is a redemption PR for #7782.
In my earlier attempt, I forgot to sync the latest changes into my branch before making edits. That led to merge conflicts and complex issues, so I’ve created a fresh branch and brought the corrected changes here.
The core change:
Fixes the
uiobject overwrite bug by teaching the app to merge objects instead of replacing them blindly.Arrays are still replaced as expected (so user lists like
customWittyPhrasesbehave correctly).Dive Deeper
Previously, when a user updated their settings, the app overwrote the entire
uiobject in memory. For example:Adding a new theme would erase any
customWittyPhrases.This PR introduces logic that checks:
If both old and new values are plain objects → merge them.
If either value is an array → replace them.
This ensures:
Updating themes preserves custom phrases.
Updating arrays (like phrases) correctly replaces old values.
This makes settings updates safer, smarter, and more intuitive for users.
Testing Matrix
Linked issues / bugs
Redemption PR for fix(config): Correct settings migration to prevent overwrite #7782