Skip to content

Conversation

Jack251970
Copy link
Member

Added a call to Context.API.SaveAppAllSettings() before executing system shutdown, restart, or advanced restart operations. This ensures that any unsaved settings are persisted, reducing the risk of data loss during these actions.

Added a call to `Context.API.SaveAppAllSettings()` before executing system shutdown, restart, or advanced restart operations. This ensures that any unsaved settings are persisted, reducing the risk of data loss during these actions.
@Jack251970 Jack251970 enabled auto-merge October 2, 2025 11:35
@github-actions github-actions bot added this to the 2.1.0 milestone Oct 2, 2025
Copy link

gitstream-cm bot commented Oct 2, 2025

🥷 Code experts: no user but you matched threshold 10

Jack251970 has most 👩‍💻 activity in the files.
Jack251970 has most 🧠 knowledge in the files.

See details

Plugins/Flow.Launcher.Plugin.Sys/Main.cs

Activity based on git-commit:

Jack251970
OCT
SEP 60 additions & 62 deletions
AUG
JUL 26 additions & 18 deletions
JUN
MAY

Knowledge based on git-blame:
Jack251970: 46%

✨ Comment /gs review for LinearB AI review. Learn how to automate it here.

Copy link

gitstream-cm bot commented Oct 2, 2025

Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX.

@Jack251970 Jack251970 requested a review from Copilot October 2, 2025 11:36
@Jack251970 Jack251970 added the bug Something isn't working label Oct 2, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds data loss prevention to system operations by saving application settings before executing shutdown, restart, and advanced restart commands. The change ensures that any unsaved application settings are persisted before the system powers down or reboots.

Key changes:

  • Added Context.API.SaveAppAllSettings() calls before system operations
  • Applied the fix to all three power operations: shutdown, restart, and advanced restart

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link
Contributor

coderabbitai bot commented Oct 2, 2025

Warning

Rate limit exceeded

@Jack251970 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 19 minutes and 30 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between e376da4 and 5b0a307.

📒 Files selected for processing (1)
  • Plugins/Flow.Launcher.Plugin.Sys/Main.cs (3 hunks)

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

📝 Walkthrough

Walkthrough

Adds calls to save all application settings after user confirmation and before executing system actions (shutdown, restart, restart with advanced boot) in the system plugin’s main command handler.

Changes

Cohort / File(s) Summary
Pre-action settings save for system operations
Plugins/Flow.Launcher.Plugin.Sys/Main.cs
After confirmation dialogs, inserts Context.API.SaveAppAllSettings() before privilege checks and invoking shutdown/restart/advanced boot actions. Comments added to explain the pre-action save rationale.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor U as User
  participant P as Sys Plugin (Main.cs)
  participant OS as Windows API

  U->>P: Trigger shutdown/restart/advanced boot
  P->>U: Show confirmation dialog
  U-->>P: Confirm
  Note right of P: New step added
  P->>P: SaveAppAllSettings()
  P->>P: Check privileges/elevation
  alt Shutdown/Restart
    P->>OS: ExitWindowsEx(...)
    OS-->>P: Action initiated
  else Advanced boot
    P->>OS: Shutdown with advanced options
    OS-->>P: Action initiated
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

bug

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The pull request title succinctly describes the primary change of saving settings before shutdown and restart to prevent data loss, directly reflecting the main modifications in the changeset.
Description Check ✅ Passed The pull request description clearly outlines adding a call to SaveAppAllSettings before shutdown, restart, and advanced restart operations to prevent data loss, which directly matches the actual changes in the code.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (2)
Plugins/Flow.Launcher.Plugin.Sys/Main.cs (2)

286-287: Consider saving settings before log off for consistency.

Unlike shutdown/restart, this command doesn't save settings before executing. For consistency with the other system state-changing commands, consider adding SaveAppAllSettings() after the user confirms.

                     if (result == MessageBoxResult.Yes)
+                    {
+                        // Save settings before log off to avoid data loss
+                        Context.API.SaveAppAllSettings();
                         PInvoke.ExitWindowsEx(EXIT_WINDOWS_FLAGS.EWX_LOGOFF, REASON);
+                    }

377-379: Consider saving settings before exiting the application.

The "Exit" command closes the application without saving settings, which could lead to data loss if the user has made configuration changes. Consider adding SaveAppAllSettings() before closing.

                     Action = c =>
                     {
+                        Context.API.SaveAppAllSettings();
                         Context.API.HideMainWindow();
                         Application.Current.MainWindow.Close();
                         return true;
                     }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1b7a475 and e376da4.

📒 Files selected for processing (1)
  • Plugins/Flow.Launcher.Plugin.Sys/Main.cs (3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
Plugins/Flow.Launcher.Plugin.Sys/Main.cs (2)
Flow.Launcher/PublicAPIInstance.cs (1)
  • SaveAppAllSettings (108-117)
Flow.Launcher.Plugin/Interfaces/IPublicAPI.cs (1)
  • SaveAppAllSettings (60-60)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: gitStream.cm
  • GitHub Check: gitStream.cm
  • GitHub Check: build
🔇 Additional comments (1)
Plugins/Flow.Launcher.Plugin.Sys/Main.cs (1)

402-403: RestartApp() already saves settings before restart
RestartApp() in PublicAPIInstance.cs calls SaveAppAllSettings() before invoking UpdateManager.RestartApp(), so no explicit save is required.

Previously, `Context.API.SaveAppAllSettings()` was called unconditionally before user confirmation for shutdown, restart, and advanced restart actions. This change ensures settings are only saved if the user confirms the action by clicking "Yes" in the confirmation dialog.

For all three functionalities:
- Moved the settings save call inside the `if (result == MessageBoxResult.Yes)` block.
- Retained the existing logic for executing the respective system commands, with checks for `EnableShutdownPrivilege()` to determine whether to use `PInvoke.ExitWindowsEx` or the `shutdown` command.

This change prevents unnecessary settings saves when the user cancels the action.
Removed logoff operation logic and associated return statement.
Eliminated return statement after recycle bin error handling.
Removed async plugin data reload and success message logic.
Simplified theme selector query handling by removing `return false`.
These changes streamline the code and improve maintainability.
@Jack251970 Jack251970 requested a review from Copilot October 2, 2025 11:44
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@Jack251970 Jack251970 requested a review from Copilot October 2, 2025 11:45
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@Jack251970 Jack251970 merged commit 8c98aed into dev Oct 4, 2025
11 checks passed
@Jack251970 Jack251970 deleted the hide_window_before_sleep branch October 4, 2025 11:52
@jjw24 jjw24 removed the 1 min review label Oct 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants