-
-
Notifications
You must be signed in to change notification settings - Fork 445
Fix the issue where UWP changes are not monitored #3821
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
🥷 Code experts: Jack251970 Jack251970 has most 👩💻 activity in the files. See details
Activity based on git-commit:
Knowledge based on git-blame: ✨ Comment |
Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX. |
📝 Walkthrough""" WalkthroughThe update refactors the initialization of the Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant UWPPackage as UWPPackage Class
participant Catalog as PackageCatalog
App->>UWPPackage: Class load
UWPPackage->>Catalog: OpenForCurrentUser() (static init)
App->>UWPPackage: Call WatchPackageChangeAsync()
UWPPackage->>Catalog: Attach event handlers (install, uninstall, update)
Catalog-->>UWPPackage: Event triggered (e.g., package installed)
UWPPackage->>App: Signal reindex via channel
Assessment against linked issues
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Plugins/Flow.Launcher.Plugin.Program/Programs/UWPPackage.cs (1)
293-293
: Good fix for the UWP monitoring issue, but consider adding protection against multiple calls.The static catalog correctly ensures event handlers persist for the application lifetime, fixing the monitoring issue where new catalog instances would lose their event handlers.
However, if
WatchPackageChangeAsync
is called multiple times, it will register duplicate event handlers on the same catalog, causing multiple events for each package change.Consider adding a guard to prevent duplicate registrations:
+private static bool _isWatchingPackageChanges = false; public static async Task WatchPackageChangeAsync() { if (Environment.OSVersion.Version.Major >= 10) { + if (_isWatchingPackageChanges) return; + _isWatchingPackageChanges = true; + catalog.PackageInstalling += (_, args) =>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Plugins/Flow.Launcher.Plugin.Program/Programs/UWPPackage.cs
(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: taooceros
PR: Flow-Launcher/Flow.Launcher#2616
File: Flow.Launcher/Flow.Launcher.csproj:7-7
Timestamp: 2024-10-08T15:52:58.573Z
Learning: In the Flow Launcher project, the version number in the `Flow.Launcher.csproj` file is dynamically updated during the CI/CD process.
Plugins/Flow.Launcher.Plugin.Program/Programs/UWPPackage.cs (5)
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3791
File: Flow.Launcher.Core/Plugin/PluginManager.cs:293-295
Timestamp: 2025-07-01T05:46:13.251Z
Learning: In Flow.Launcher.Core/Plugin/PluginManager.cs, when checking if a plugin is modified within the PluginManager class itself, prefer using the internal static PluginModified(string id) method directly rather than going through API.PluginModified() for better performance and architectural design.
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3572
File: Flow.Launcher/App.xaml.cs:214-216
Timestamp: 2025-07-06T12:21:37.947Z
Learning: In Flow Launcher, the UpdatePluginManifestAsync method in PluginsManifest.cs already has comprehensive internal try-catch handling that logs exceptions and returns false on failure rather than throwing, making external try-catch wrappers unnecessary.
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3672
File: Flow.Launcher/MainWindow.xaml.cs:318-318
Timestamp: 2025-06-08T14:12:12.842Z
Learning: In Flow.Launcher, the App.NotifyIcon static property is initialized in the App class before MainWindow creation, so null checks are not needed when accessing App.NotifyIcon in MainWindow lifecycle methods.
Learnt from: taooceros
PR: Flow-Launcher/Flow.Launcher#2616
File: Flow.Launcher/Flow.Launcher.csproj:7-7
Timestamp: 2024-10-08T15:52:58.573Z
Learning: In the Flow Launcher project, the version number in the `Flow.Launcher.csproj` file is dynamically updated during the CI/CD process.
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3672
File: Flow.Launcher/MainWindow.xaml.cs:244-247
Timestamp: 2025-06-08T14:12:21.348Z
Learning: In Flow.Launcher, App.NotifyIcon is created before MainWindow creation, so null checks for App.NotifyIcon are not necessary when accessing it from MainWindow code.
⏰ 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). (5)
- GitHub Check: Report (PR)
- GitHub Check: gitStream workflow automation
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
- GitHub Check: build
Co-authored-by: Copilot <[email protected]>
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.
Pull Request Overview
This PR addresses the issue where UWP package change events were not being monitored by reusing a single PackageCatalog instance instead of recreating it each time.
- Introduce a static
catalog
field to cache the catalog instance. - Change
WatchPackageChangeAsync
to open the catalog only once using null-coalescing assignment.
Comments suppressed due to low confidence (1)
Plugins/Flow.Launcher.Plugin.Program/Programs/UWPPackage.cs:293
- [nitpick] Private static fields in C# typically use the _camelCase naming convention. Consider renaming 'catalog' to '_catalog' to align with project style guidelines.
private static PackageCatalog? catalog;
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.
Can confirm installing / uninstalling can lead to reindexing successfully and icons are reloaded
@jjw24 Please include this in 1.20.2 since it is just a small fix |
Fix the issue that UWP changes that can't be monitored (#2345)
…age-monitoring Fix the issue that UWP changes that can't be monitored (Flow-Launcher#2345)
Fix #2345 #3747.