-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Feat: Record recently opened windows #8202
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?
Conversation
Summary of ChangesHello @SiriusXT, 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 multi-window experience by introducing robust state persistence and recovery mechanisms. It ensures that all application windows, especially extra ones, can maintain their open note contexts across sessions and refreshes. Furthermore, it adds a user-friendly feature to restore recently closed windows from the tray, making the application more resilient and convenient for users who frequently work with multiple note windows. 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 introduces a great feature for tracking and restoring recently opened notes across multiple windows. The implementation is mostly solid, but I've identified a few critical and high-severity issues that should be addressed before merging. These include a data corruption bug that could lead to circular JSON structures, potential crashes from unsafe array operations, and an HTML validation error. I've provided specific comments and suggestions to fix these problems.
apps/server/src/migrations/0234__migrate_open_note_contexts_format.ts
Outdated
Show resolved
Hide resolved
|
/gemini review |
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 introduces a mechanism to track and restore recently opened notes across multiple windows. This is achieved by assigning a unique ID to each window and storing its tab context. The changes span across the client and server, including a database schema migration, frontend context management, and backend window tracking. The implementation also adds a "Recently closed windows" feature to the tray menu.
My feedback focuses on improving code robustness and maintainability. I've pointed out a potential bug due to missing logic in one of two duplicated code blocks, suggested extracting magic numbers into constants, and recommended adding error handling for JSON parsing to prevent crashes.
|
/gemini review |
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 introduces a great feature for tracking and restoring recently opened windows. The implementation is mostly solid, but there are a few areas that need improvement for robustness and consistency. I've identified potential crashes related to unsafe array operations and JSON parsing, as well as inconsistent logic for managing the number of saved windows. I've also suggested using a more reliable method for generating unique window IDs. Addressing these points will make the feature more stable and maintainable.
|
/gemini review |
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 introduces a valuable feature for recording and restoring recently opened windows, which significantly improves the user experience by preserving browsing states across sessions. The implementation is comprehensive, touching both client and server-side code to manage window states. I've identified a few potential issues concerning null safety and the logic for managing the maximum number of saved windows, which could lead to runtime errors or unintended behavior. My review comments include specific suggestions to rectify these issues. Overall, this is a well-executed feature addition.
|
/gemini review |
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 introduces a great feature for tracking and restoring recently opened windows. The implementation is comprehensive, covering state saving for extra windows, UI for restoring from the tray, and a database migration for the new data structure. The changes are well-structured across the client, server, and desktop applications.
My main feedback is regarding some code duplication and inconsistency in apps/client/src/components/tab_manager.ts for saving window state, which I've detailed in a specific comment. Addressing this will improve the maintainability and robustness of the new feature.
Overall, this is a solid contribution that will significantly improve the user experience for multi-window usage.
@rom1dep Desktop and web clients maintain independent tab states. The tabs of the most recent 10 windows are stored in the |
|
/gemini review |
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 introduces a great new feature for tracking and restoring recently opened windows. The implementation is comprehensive, touching both client and server-side code, including a database migration. My review focuses on improving correctness, maintainability, and robustness. I've identified a couple of potential bugs: one in the window state management logic that could affect the eviction of old windows, and another in an event handler that could lead to data loss on window close. I've also pointed out a potential issue with handling URL query parameters and a minor maintainability improvement. Overall, these are solid changes, and with a few adjustments, they will be even better.
Uh oh!
There was an error while loading. Please reload this page.