Fix hide item option not working in Program plugin #4141
Conversation
Co-authored-by: Jack251970 <[email protected]>
Co-authored-by: Jack251970 <[email protected]>
Introduced an optional `resetCache` parameter to `IndexWin32ProgramsAsync` and `IndexUwpProgramsAsync` to allow finer control over cache resets during indexing. Updated `IndexProgramsAsync` to explicitly pass `true` for cache resets. Modified `DisableProgramAsync` to return a `bool` indicating success and adjusted its logic to prevent unnecessary cache resets when reindexing. Ensured it returns `false` if the program is already disabled. Replaced nullable `PackageCatalog?` with non-nullable `PackageCatalog` in `UWPPackage.cs` to ensure proper initialization. Updated `WatchPackageChangeAsync` and `WatchDirectory` to call indexing methods with `resetCache` set to `true` when changes are detected. Improved cache reset logic across methods by conditionally invoking `ResetCache` based on the `resetCache` parameter, enhancing performance and maintainability.
prlabeler
bot
added
bug
|
🥷 Code experts: no user but you matched threshold 10 Jack251970 has most 👩💻 activity in the files. See details
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
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. |
|
Warning Rate limit exceeded@Jack251970 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 20 minutes and 39 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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. 📒 Files selected for processing (3)
Note Other AI code review bot(s) detectedCodeRabbit 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. 📝 WalkthroughWalkthroughThis PR refactors the program disabling and cache management workflow in the Program plugin. The Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
Plugins/Flow.Launcher.Plugin.Program/Main.cs(7 hunks)Plugins/Flow.Launcher.Plugin.Program/Programs/UWPPackage.cs(2 hunks)Plugins/Flow.Launcher.Plugin.Program/Programs/Win32.cs(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-07-06T12:21:37.947Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 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.
Applied to files:
Plugins/Flow.Launcher.Plugin.Program/Programs/UWPPackage.csPlugins/Flow.Launcher.Plugin.Program/Programs/Win32.csPlugins/Flow.Launcher.Plugin.Program/Main.cs
📚 Learning: 2025-10-16T10:48:30.573Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3854
File: Flow.Launcher/App.xaml.cs:252-275
Timestamp: 2025-10-16T10:48:30.573Z
Learning: In Flow Launcher's App.xaml.cs, the plugin initialization block (lines 252-275) that includes PreStartPluginExecutablePathUpdate, PluginManager.LoadPlugins, PluginManager.InitializePluginsAsync, _mainVM.QueryResults(), and API.SaveAppAllSettings() does not require additional Task.Run wrappers or Dispatcher.InvokeAsync calls according to maintainer Jack251970, as the threading model is already safe as designed.
Applied to files:
Plugins/Flow.Launcher.Plugin.Program/Programs/UWPPackage.csPlugins/Flow.Launcher.Plugin.Program/Programs/Win32.csPlugins/Flow.Launcher.Plugin.Program/Main.cs
📚 Learning: 2025-07-21T09:19:49.684Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3854
File: Flow.Launcher/App.xaml.cs:246-262
Timestamp: 2025-07-21T09:19:49.684Z
Learning: In Flow Launcher's App.xaml.cs, the asynchronous plugin initialization task (containing AbstractPluginEnvironment.PreStartPluginExecutablePathUpdate, PluginManager.LoadPlugins, PluginManager.InitializePluginsAsync, and AutoPluginUpdates) does not require additional try-catch error handling according to maintainer Jack251970, as these operations are designed to handle exceptions internally.
Applied to files:
Plugins/Flow.Launcher.Plugin.Program/Programs/UWPPackage.csPlugins/Flow.Launcher.Plugin.Program/Programs/Win32.csPlugins/Flow.Launcher.Plugin.Program/Main.cs
📚 Learning: 2025-09-04T11:52:29.096Z
Learnt from: jjw24
Repo: Flow-Launcher/Flow.Launcher PR: 3932
File: Flow.Launcher.Core/ExternalPlugins/PluginsManifest.cs:48-55
Timestamp: 2025-09-04T11:52:29.096Z
Learning: In Flow Launcher's PluginsManifest.cs, when dealing with version parsing for the MinimumAppVersion feature, maintainer jjw24 prefers to keep the solution simple rather than implementing comprehensive helper methods for SemVer parsing normalization.
Applied to files:
Plugins/Flow.Launcher.Plugin.Program/Programs/UWPPackage.cs
📚 Learning: 2025-07-01T05:46:13.251Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 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.
Applied to files:
Plugins/Flow.Launcher.Plugin.Program/Programs/UWPPackage.csPlugins/Flow.Launcher.Plugin.Program/Programs/Win32.csPlugins/Flow.Launcher.Plugin.Program/Main.cs
🧬 Code graph analysis (1)
Plugins/Flow.Launcher.Plugin.Program/Programs/Win32.cs (2)
Plugins/Flow.Launcher.Plugin.Program/Main.cs (8)
Task(81-162)Task(214-335)Task(337-366)Task(368-397)Task(399-412)Task(480-522)Task(539-542)Main(18-548)Plugins/Flow.Launcher.Plugin.Program/Programs/UWPPackage.cs (1)
Task(295-323)
⏰ 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). (10)
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
- GitHub Check: CodeQL analysis (csharp)
- GitHub Check: Agent
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
- GitHub Check: build
🔇 Additional comments (4)
Plugins/Flow.Launcher.Plugin.Program/Programs/UWPPackage.cs (1)
293-293: LGTM!The nullability change is appropriate since
catalogis always initialized via null-coalescing assignment (catalog ??= PackageCatalog.OpenForCurrentUser()) before any access withinWatchPackageChangeAsync.Plugins/Flow.Launcher.Plugin.Program/Main.cs (3)
337-366: LGTM!The conditional cache reset based on the
resetCacheparameter is well-implemented. The method properly acquires/releases the lock and handles exceptions.
368-397: LGTM!Consistent implementation with
IndexWin32ProgramsAsync. The conditional cache reset pattern is correct.
403-408: Timing measurement is correct; no issue found.The
StopwatchLogInfoAsyncmethod correctly awaits theFunc<Task>parameter at line 53 inFlow.Launcher.Infrastructure/Stopwatch.cs. Both() => IndexWin32ProgramsAsync(true)and the suggestedasync () => await IndexWin32ProgramsAsync(true)are functionally equivalent—they both return aTaskthat is properly awaited byInfoAsync, and the stopwatch accurately measures the actual indexing duration. The timing measurement works correctly as-is.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Pull request overview
This pull request refactors the program disabling and indexing logic in the Flow.Launcher Program plugin to fix an issue where the "hide item" option wasn't working properly. The changes introduce a resetCache parameter to indexing methods for better cache control, refactor the DisableProgramAsync method to return a boolean and trigger targeted reindexing, and improve error handling in the context menu action.
- Added a
resetCacheparameter to indexing methods to control when query cache is reset - Refactored
DisableProgramAsyncto return a boolean indicating success and trigger type-specific reindexing - Wrapped the disable program action in try-catch with proper error logging
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| Plugins/Flow.Launcher.Plugin.Program/Main.cs | Added resetCache parameter to indexing methods; refactored DisableProgramAsync to return bool and trigger targeted reindexing; improved error handling in context menu action for disabling programs |
| Plugins/Flow.Launcher.Plugin.Program/Programs/Win32.cs | Updated directory change monitor to explicitly pass resetCache: true when triggering Win32 program reindexing |
| Plugins/Flow.Launcher.Plugin.Program/Programs/UWPPackage.cs | Updated package change watcher to explicitly pass resetCache: true when triggering UWP program reindexing; changed catalog field from nullable to non-nullable |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
The default value `resetCache = true` was removed from the method signatures of `IndexWin32ProgramsAsync` and `IndexUwpProgramsAsync` in the `Flow.Launcher.Plugin.Program` namespace. These methods now require the `resetCache` parameter to be explicitly provided by the caller. This change improves clarity and reduces ambiguity in method invocation.
This pull request refactors and improves the program indexing and disabling logic in the
Flow.Launcher.Plugin.Programplugin. The main changes introduce more robust cache management, better handling of program disabling, and improved asynchrony and error handling.Improvements to indexing and cache management:
resetCacheparameter (defaulting totrue) to bothIndexWin32ProgramsAsyncandIndexUwpProgramsAsync, allowing more control over when the cache is reset. The cache is only reset if this parameter istrue. [1] [2] [3] [4]resetCache: truefor correct cache invalidation. [1] [2] [3]Improvements to disabling programs:
DisableProgramAsyncto return aboolindicating whether a program was actually disabled, and to only trigger reindexing for the relevant program type (UWP or Win32).Code quality and safety:
catalogfield inUWPPackage.csfrom nullable to non-nullable, reflecting that it is always initialized before use.These changes improve the efficiency, reliability, and user experience of program management in the plugin.
Fixes #4139
Tested