Skip to content

Conversation

@01Dri
Copy link
Contributor

@01Dri 01Dri commented Nov 5, 2025

CHANGES

  • Added new File and Folder action keywords
  • Implemented action keyword–based result filtering
  • Refactored the Search Manager for better maintainability
  • Added option to exclude Quick Access items from action keyword results

Details

Exclude Quick Access

Quick Access items currently don’t respect the action keyword result filters.
For example, the Folder Search action should only display folders, but if there’s a .txt file in Quick Access, it still appears in the results.
My idea introduces a new option to include or exclude Quick Access items when an action keyword is used.

Note

The Search Manager in Explorer was refactored to simplify the addition of new action keywords.
Previously, each new keyword required code changes in multiple places.
Now, the logic for action keywords and filters is much easier to understand and extend.

@github-actions github-actions bot added this to the 2.1.0 milestone Nov 5, 2025
@gitstream-cm
Copy link

gitstream-cm bot commented Nov 5, 2025

🥷 Code experts: Jack251970, VictoriousRaptor

Jack251970, VictoriousRaptor have most 👩‍💻 activity in the files.
Jack251970, jjw24 have most 🧠 knowledge in the files.

See details

Plugins/Flow.Launcher.Plugin.Explorer/Languages/en.xaml

Activity based on git-commit:

Jack251970 VictoriousRaptor
NOV
OCT
SEP 1 additions & 0 deletions
AUG
JUL 7 additions & 0 deletions 28 additions & 15 deletions
JUN 7 additions & 0 deletions

Knowledge based on git-blame:
jjw24: 20%
Jack251970: 12%

Plugins/Flow.Launcher.Plugin.Explorer/Search/SearchManager.cs

Activity based on git-commit:

Jack251970 VictoriousRaptor
NOV
OCT
SEP 2 additions & 2 deletions
AUG
JUL
JUN 12 additions & 0 deletions

Knowledge based on git-blame:
jjw24: 18%
Jack251970: 6%

Plugins/Flow.Launcher.Plugin.Explorer/Settings.cs

Activity based on git-commit:

Jack251970 VictoriousRaptor
NOV
OCT
SEP 3 additions & 6 deletions
AUG
JUL 2 additions & 6 deletions
JUN 2 additions & 0 deletions

Knowledge based on git-blame:
jjw24: 8%
Jack251970: 2%

Plugins/Flow.Launcher.Plugin.Explorer/ViewModels/SettingsViewModel.cs

Activity based on git-commit:

Jack251970 VictoriousRaptor
NOV
OCT
SEP 9 additions & 9 deletions
AUG
JUL 17 additions & 13 deletions 33 additions & 6 deletions
JUN 51 additions & 27 deletions

Knowledge based on git-blame:
Jack251970: 13%
jjw24: 5%

Plugins/Flow.Launcher.Plugin.Explorer/Views/ExplorerSettings.xaml

Activity based on git-commit:

Jack251970 VictoriousRaptor
NOV
OCT 7 additions & 8 deletions
SEP
AUG
JUL 6 additions & 15 deletions
JUN 867 additions & 821 deletions

Knowledge based on git-blame:
Jack251970: 83%
jjw24: 1%

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

@gitstream-cm
Copy link

gitstream-cm bot commented Nov 5, 2025

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

@coderabbitai coderabbitai bot added the enhancement New feature or request label Nov 5, 2025
@01Dri
Copy link
Contributor Author

01Dri commented Nov 5, 2025

image

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 5, 2025

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 folder- and file-search action keywords and an option to exclude Quick Access from action-keyword searches; updates English localization and settings UI; refactors SearchManager to use GetActiveActionKeyword, centralize path/content/quick-access branching, add ShouldSkip filtering and conditional quick-access merging.

Changes

Cohort / File(s) Summary
Localization Resources
Plugins/Flow.Launcher.Plugin.Explorer/Languages/en.xaml
Added three resource strings: plugin_explorer_exclude_quickaccess_from_actionkeywords, plugin_explorer_actionkeywordview_foldersearch, plugin_explorer_actionkeywordview_filesearch.
Settings & Configuration
Plugins/Flow.Launcher.Plugin.Explorer/Settings.cs
Made ActionKeyword enum public and extended it with FolderSearchActionKeyword and FileSearchActionKeyword; added properties FolderSearchActionKeyword, FolderSearchKeywordEnabled, FileSearchActionKeyword, FileSearchKeywordEnabled, ExcludeQuickAccessFromActionKeywords; added GetActiveActionKeyword(string).
Search Logic
Plugins/Flow.Launcher.Plugin.Explorer/Search/SearchManager.cs
Replaced legacy ActionKeywordMatch flow with GetActiveActionKeyword usage; introduced ShouldSkip() filtering and MergeQuickAccessInResultsIfQueryMatch(); centralized branching for path vs content/index vs quick-access and adjusted result merging/return flow.
UI & ViewModel
Plugins/Flow.Launcher.Plugin.Explorer/ViewModels/SettingsViewModel.cs, Plugins/Flow.Launcher.Plugin.Explorer/Views/ExplorerSettings.xaml
Added Folder/File action keyword entries in ViewModel; added CheckBox bound to ExcludeQuickAccessFromActionKeywords and restructured grid rows/ListView border in ExplorerSettings.xaml.

Sequence Diagram(s)

sequenceDiagram
    participant UI as UI / Query
    participant Settings as Settings.GetActiveActionKeyword
    participant SearchMgr as SearchManager
    participant PathSearch as PathSearchAsync
    participant ContentSearch as Content/Index Search
    participant QuickAccess as QuickAccessLookup

    UI->>SearchMgr: Submit Query
    SearchMgr->>Settings: GetActiveActionKeyword(query.ActionKeywordStr)
    Settings-->>SearchMgr: activeActionKeyword (nullable)

    alt no active keyword
        SearchMgr-->>UI: return empty results
    else active is QuickAccessActionKeyword
        SearchMgr->>QuickAccess: Lookup quick access matches
        QuickAccess-->>SearchMgr: quick access results
        SearchMgr-->>UI: return quick access results
    else active is PathSearchActionKeyword
        SearchMgr->>PathSearch: PathSearchAsync(query)
        PathSearch-->>SearchMgr: path results
        SearchMgr->>SearchMgr: MergeQuickAccessInResultsIfQueryMatch(...)
        SearchMgr->>SearchMgr: filter with ShouldSkip(...)
        SearchMgr-->>UI: return filtered results
    else
        SearchMgr->>ContentSearch: Content/index search (file content handled specially)
        ContentSearch-->>SearchMgr: content results
        SearchMgr->>SearchMgr: MergeQuickAccessInResultsIfQueryMatch(...)
        SearchMgr->>SearchMgr: filter with ShouldSkip(...)
        SearchMgr-->>UI: return filtered results
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Areas to focus:
    • Settings.cs: public API changes (enum visibility and new properties) and correctness of GetActiveActionKeyword behavior.
    • SearchManager.cs: correctness of new branching, ShouldSkip filtering, MergeQuickAccessInResultsIfQueryMatch, and edge-case handling for empty/quick-access flows.
    • ExplorerSettings.xaml & SettingsViewModel.cs: binding correctness for new checkbox and added action-keyword entries.

Possibly related PRs

Suggested reviewers

  • jjw24
  • Jack251970
  • onesounds

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 title '[FEATURE] Folder and File Action Keywords' directly and clearly summarizes the main change in the changeset, which adds new Folder and File action keywords to the Explorer plugin.
Description check ✅ Passed The description is well-related to the changeset, explaining the purpose of the new features, the refactoring rationale, and the Quick Access filtering option that addresses a real problem in the implementation.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

📝 Customizable high-level summaries are now available in beta!

You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.

  • Provide your own instructions using the high_level_summary_instructions setting.
  • Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
  • Use high_level_summary_in_walkthrough to move the summary from the description to the walkthrough section.

Example instruction:

"Divide the high-level summary into five sections:

  1. 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
  2. 📓 References — List relevant issues, discussions, documentation, or related PRs.
  3. 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
  4. 📊 Contributor Summary — Include a Markdown table showing contributions:
    | Contributor | Lines Added | Lines Removed | Files Changed |
  5. ✔️ Additional Notes — Add any extra reviewer context.
    Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."

Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.


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: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e2ac321 and 4e5378b.

📒 Files selected for processing (5)
  • Plugins/Flow.Launcher.Plugin.Explorer/Languages/en.xaml (2 hunks)
  • Plugins/Flow.Launcher.Plugin.Explorer/Search/SearchManager.cs (4 hunks)
  • Plugins/Flow.Launcher.Plugin.Explorer/Settings.cs (7 hunks)
  • Plugins/Flow.Launcher.Plugin.Explorer/ViewModels/SettingsViewModel.cs (2 hunks)
  • Plugins/Flow.Launcher.Plugin.Explorer/Views/ExplorerSettings.xaml (2 hunks)
🧰 Additional context used
🧠 Learnings (14)
📓 Common learnings
Learnt from: Koisu-unavailable
Repo: Flow-Launcher/Flow.Launcher PR: 3770
File: Flow.Launcher/ViewModel/MainViewModel.cs:0-0
Timestamp: 2025-06-24T19:06:48.344Z
Learning: In Flow.Launcher's Explorer plugin results, the SubTitle property always contains the directory containing the file. For file results, Title contains the filename and SubTitle contains the parent directory. For directory results, SubTitle contains the directory path itself.
📚 Learning: 2025-09-05T11:56:27.267Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3593
File: Flow.Launcher/HotkeyControlDialog.xaml:6-6
Timestamp: 2025-09-05T11:56:27.267Z
Learning: In Flow.Launcher's migration to iNKORE.UI.WPF.Modern UI framework, dialog resource keys like PopuBGColor, PopupButtonAreaBGColor, PopupButtonAreaBorderColor, and PopupTextColor are provided by the iNKORE.UI.WPF.Modern framework itself, not defined locally in the codebase theme files.

Applied to files:

  • Plugins/Flow.Launcher.Plugin.Explorer/Languages/en.xaml
📚 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.Explorer/Search/SearchManager.cs
  • Plugins/Flow.Launcher.Plugin.Explorer/Settings.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.Explorer/Search/SearchManager.cs
  • Plugins/Flow.Launcher.Plugin.Explorer/Settings.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.Explorer/Search/SearchManager.cs
  • Plugins/Flow.Launcher.Plugin.Explorer/Settings.cs
📚 Learning: 2025-06-24T19:06:48.344Z
Learnt from: Koisu-unavailable
Repo: Flow-Launcher/Flow.Launcher PR: 3770
File: Flow.Launcher/ViewModel/MainViewModel.cs:0-0
Timestamp: 2025-06-24T19:06:48.344Z
Learning: In Flow.Launcher's Explorer plugin results, the SubTitle property always contains the directory containing the file. For file results, Title contains the filename and SubTitle contains the parent directory. For directory results, SubTitle contains the directory path itself.

Applied to files:

  • Plugins/Flow.Launcher.Plugin.Explorer/Search/SearchManager.cs
  • Plugins/Flow.Launcher.Plugin.Explorer/Settings.cs
📚 Learning: 2025-07-21T09:19:19.012Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3854
File: Flow.Launcher.Core/Plugin/PluginManager.cs:280-292
Timestamp: 2025-07-21T09:19:19.012Z
Learning: In Flow Launcher's PluginManager.cs, the post-initialization operations (RegisterResultsUpdatedEvent, UpdatePluginMetadataTranslation, RegisterPluginActionKeywords, DialogJump.InitializeDialogJumpPlugin, and AddPluginToLists) are designed to be exception-safe and do not require additional try-catch error handling according to the maintainer Jack251970.

Applied to files:

  • Plugins/Flow.Launcher.Plugin.Explorer/Search/SearchManager.cs
  • Plugins/Flow.Launcher.Plugin.Explorer/Settings.cs
📚 Learning: 2024-11-03T07:40:11.014Z
Learnt from: Yusyuriv
Repo: Flow-Launcher/Flow.Launcher PR: 3057
File: Flow.Launcher.Core/Plugin/JsonRPCPluginSettings.cs:0-0
Timestamp: 2024-11-03T07:40:11.014Z
Learning: In Flow Launcher, when using Windows Forms dialogs (e.g., in `JsonRPCPluginSettings.cs`), path validation is enabled by default in `OpenFileDialog` and `FolderBrowserDialog`, preventing users from selecting invalid paths, but it's possible to opt out of this validation on individual dialogs.

Applied to files:

  • Plugins/Flow.Launcher.Plugin.Explorer/Search/SearchManager.cs
  • Plugins/Flow.Launcher.Plugin.Explorer/Settings.cs
📚 Learning: 2025-10-16T09:29:19.653Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3854
File: Flow.Launcher.Core/Plugin/PluginManager.cs:519-523
Timestamp: 2025-10-16T09:29:19.653Z
Learning: In Flow Launcher's PluginManager.cs QueryDialogJumpForPluginAsync method, when a DialogJump plugin is still initializing, returning null is intentional behavior. This allows the query to fall through to the default Explorer plugin, which serves as a fallback handler. This is different from QueryForPluginAsync and QueryHomeForPluginAsync, which show "still initializing" messages because they don't have fallback mechanisms.

Applied to files:

  • Plugins/Flow.Launcher.Plugin.Explorer/Search/SearchManager.cs
  • Plugins/Flow.Launcher.Plugin.Explorer/Settings.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.Explorer/Search/SearchManager.cs
  • Plugins/Flow.Launcher.Plugin.Explorer/Settings.cs
📚 Learning: 2025-09-13T06:04:26.977Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3593
File: Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml.cs:143-165
Timestamp: 2025-09-13T06:04:26.977Z
Learning: In Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml, the ListView_SizeChanged handler in the C# file assumes 5 GridViewColumns but the XAML may have more columns. The user Jack251970 indicated this mismatch doesn't need to be fixed when pointed out during PR #3593 review.

Applied to files:

  • Plugins/Flow.Launcher.Plugin.Explorer/Views/ExplorerSettings.xaml
  • Plugins/Flow.Launcher.Plugin.Explorer/ViewModels/SettingsViewModel.cs
📚 Learning: 2025-09-11T08:30:29.487Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3593
File: Flow.Launcher/SettingPages/Views/SettingsPaneTheme.xaml:0-0
Timestamp: 2025-09-11T08:30:29.487Z
Learning: In Flow.Launcher's SettingsPaneTheme.xaml theme editor panel, the inner ui:ScrollViewerEx within the Border (Grid.Column="1") is intentionally kept because its height cannot be expanded to the whole page due to layout constraints. This serves a different scrolling purpose than the outer ScrollViewerEx and is necessary for the theme editor panel functionality.

Applied to files:

  • Plugins/Flow.Launcher.Plugin.Explorer/Views/ExplorerSettings.xaml
📚 Learning: 2025-03-28T21:20:54.978Z
Learnt from: onesounds
Repo: Flow-Launcher/Flow.Launcher PR: 3394
File: Flow.Launcher/Themes/Darker Glass.xaml:134-141
Timestamp: 2025-03-28T21:20:54.978Z
Learning: In WPF applications like Flow.Launcher, Border elements cannot directly display text content and require a child element like TextBlock to handle text rendering. This separation of concerns (Border for visual container styling, TextBlock for text display) follows WPF best practices and provides greater styling flexibility.

Applied to files:

  • Plugins/Flow.Launcher.Plugin.Explorer/Views/ExplorerSettings.xaml
📚 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.Explorer/Settings.cs
🧬 Code graph analysis (2)
Plugins/Flow.Launcher.Plugin.Explorer/Search/SearchManager.cs (4)
Plugins/Flow.Launcher.Plugin.Explorer/Settings.cs (2)
  • Settings (14-242)
  • ActionKeyword (229-240)
Flow.Launcher.Plugin/SharedCommands/FilesFolders.cs (1)
  • IsLocationPathString (246-265)
Plugins/Flow.Launcher.Plugin.Explorer/Search/EnvironmentVariables.cs (3)
  • EnvironmentVariables (10-109)
  • IsEnvironmentVariableSearch (25-31)
  • HasEnvironmentVar (33-41)
Plugins/Flow.Launcher.Plugin.Explorer/Search/QuickAccessLinks/QuickAccess.cs (3)
  • List (11-28)
  • List (30-40)
  • QuickAccess (7-41)
Plugins/Flow.Launcher.Plugin.Explorer/ViewModels/SettingsViewModel.cs (1)
Plugins/Flow.Launcher.Plugin.Explorer/Settings.cs (2)
  • Settings (14-242)
  • ActionKeyword (229-240)
🪛 GitHub Check: Check Spelling
Plugins/Flow.Launcher.Plugin.Explorer/Views/ExplorerSettings.xaml

[warning] 690-690:
quickaccess is not a recognized word. (unrecognized-spelling)

Plugins/Flow.Launcher.Plugin.Explorer/ViewModels/SettingsViewModel.cs

[warning] 282-282:
actionkeywordview is not a recognized word. (unrecognized-spelling)

⏰ 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). (1)
  • GitHub Check: build

@Jack251970 Jack251970 requested a review from Copilot November 6, 2025 07:36
Copy link
Contributor

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 two new action keywords for folder-only and file-only search functionality, along with an option to exclude Quick Access results from appearing when other action keywords are used. The changes enable more targeted search experiences within the Explorer plugin.

Key changes:

  • Added FolderSearchActionKeyword and FileSearchActionKeyword to the ActionKeyword enum with associated settings
  • Implemented filtering logic in SearchManager to skip results based on active action keyword type
  • Added UI checkbox to control Quick Access result merging behavior

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
Settings.cs Adds new action keyword properties and enum values for folder/file search, implements GetActiveActionKeyword method, reorganizes using statements
SearchManager.cs Refactors search logic to use GetActiveActionKeyword, adds filtering for folder/file-only searches, reorganizes using statements
SettingsViewModel.cs Adds new action keywords to UI model initialization, has trailing whitespace on one line
ExplorerSettings.xaml Adds checkbox for Quick Access exclusion option, adjusts grid layout with improper indentation
en.xaml Adds localization strings for new action keywords and checkbox

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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: 0

♻️ Duplicate comments (1)
Plugins/Flow.Launcher.Plugin.Explorer/Search/SearchManager.cs (1)

281-289: Quick Access results bypass folder/file action keyword filters.

When ExcludeQuickAccessFromActionKeywords is false (the default), MergeQuickAccessInResultsIfQueryMatch merges all matched Quick Access entries regardless of their type. This means:

  • FolderSearchActionKeyword will show Quick Access files
  • FileSearchActionKeyword will show Quick Access folders

This contradicts the PR's goal of respecting action keyword constraints.

Apply this diff to filter Quick Access results by type:

 private void MergeQuickAccessInResultsIfQueryMatch(HashSet<Result> results, Query query, ActionKeyword? activeActionKeyword)
 {
     if (activeActionKeyword != null 
         && activeActionKeyword != ActionKeyword.QuickAccessActionKeyword
         && Settings.ExcludeQuickAccessFromActionKeywords)
         return;
-    var quickAccessMatched = QuickAccess.AccessLinkListMatched(query, Settings.QuickAccessLinks);
+    
+    IEnumerable<AccessLink> quickAccessLinks = Settings.QuickAccessLinks;
+    
+    if (activeActionKeyword == ActionKeyword.FolderSearchActionKeyword)
+        quickAccessLinks = quickAccessLinks.Where(link => link.Type != ResultType.File);
+    else if (activeActionKeyword == ActionKeyword.FileSearchActionKeyword)
+        quickAccessLinks = quickAccessLinks.Where(link => link.Type == ResultType.File);
+    
+    var quickAccessMatched = QuickAccess.AccessLinkListMatched(query, quickAccessLinks);
     if (quickAccessMatched != null && quickAccessMatched.Count > 0) results.UnionWith(quickAccessMatched);
 }
🧹 Nitpick comments (1)
Plugins/Flow.Launcher.Plugin.Explorer/Search/SearchManager.cs (1)

80-104: Consider simplifying the switch pattern.

The switch on a boolean with pattern matching works but is non-idiomatic. An if/else if chain would be more readable.

Apply this diff for clearer logic flow:

-            switch (activeActionKeyword!.Equals(ActionKeyword.PathSearchActionKeyword))
+            if (activeActionKeyword!.Equals(ActionKeyword.PathSearchActionKeyword))
             {
-                case true:
-                    results.UnionWith(await PathSearchAsync(query, token).ConfigureAwait(false));
-                    return results.ToList();
-                case false
-                    // Intentionally require enabling of Everything's content search due to its slowness
-                    when activeActionKeyword.Equals(ActionKeyword.FileContentSearchActionKeyword):
-                    if (Settings.ContentIndexProvider is EverythingSearchManager && !Settings.EnableEverythingContentSearch)
-                        return EverythingContentSearchResult(query);
-
-                    searchResults = Settings.ContentIndexProvider.ContentSearchAsync("", query.Search, token);
-                    engineName = Enum.GetName(Settings.ContentSearchEngine);
-                    break;
-
-                case false
-                    when activeActionKeyword.Equals(ActionKeyword.QuickAccessActionKeyword):
-                    return QuickAccess.AccessLinkListMatched(query, Settings.QuickAccessLinks);
-
-
-                default:
-                    searchResults = Settings.IndexProvider.SearchAsync(query.Search, token);
-                    engineName = Enum.GetName(Settings.IndexSearchEngine);
-                    break;
+                results.UnionWith(await PathSearchAsync(query, token).ConfigureAwait(false));
+                return results.ToList();
+            }
+            else if (activeActionKeyword.Equals(ActionKeyword.FileContentSearchActionKeyword))
+            {
+                // Intentionally require enabling of Everything's content search due to its slowness
+                if (Settings.ContentIndexProvider is EverythingSearchManager && !Settings.EnableEverythingContentSearch)
+                    return EverythingContentSearchResult(query);
+
+                searchResults = Settings.ContentIndexProvider.ContentSearchAsync("", query.Search, token);
+                engineName = Enum.GetName(Settings.ContentSearchEngine);
+            }
+            else if (activeActionKeyword.Equals(ActionKeyword.QuickAccessActionKeyword))
+            {
+                return QuickAccess.AccessLinkListMatched(query, Settings.QuickAccessLinks);
+            }
+            else
+            {
+                searchResults = Settings.IndexProvider.SearchAsync(query.Search, token);
+                engineName = Enum.GetName(Settings.IndexSearchEngine);
             }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between dbf4491 and 1d60cb7.

📒 Files selected for processing (3)
  • Plugins/Flow.Launcher.Plugin.Explorer/Search/SearchManager.cs (5 hunks)
  • Plugins/Flow.Launcher.Plugin.Explorer/ViewModels/SettingsViewModel.cs (1 hunks)
  • Plugins/Flow.Launcher.Plugin.Explorer/Views/ExplorerSettings.xaml (2 hunks)
🧰 Additional context used
🧠 Learnings (11)
📚 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.Explorer/Search/SearchManager.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.Explorer/Search/SearchManager.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.Explorer/Search/SearchManager.cs
📚 Learning: 2025-06-24T19:06:48.344Z
Learnt from: Koisu-unavailable
Repo: Flow-Launcher/Flow.Launcher PR: 3770
File: Flow.Launcher/ViewModel/MainViewModel.cs:0-0
Timestamp: 2025-06-24T19:06:48.344Z
Learning: In Flow.Launcher's Explorer plugin results, the SubTitle property always contains the directory containing the file. For file results, Title contains the filename and SubTitle contains the parent directory. For directory results, SubTitle contains the directory path itself.

Applied to files:

  • Plugins/Flow.Launcher.Plugin.Explorer/Search/SearchManager.cs
📚 Learning: 2025-07-21T09:19:19.012Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3854
File: Flow.Launcher.Core/Plugin/PluginManager.cs:280-292
Timestamp: 2025-07-21T09:19:19.012Z
Learning: In Flow Launcher's PluginManager.cs, the post-initialization operations (RegisterResultsUpdatedEvent, UpdatePluginMetadataTranslation, RegisterPluginActionKeywords, DialogJump.InitializeDialogJumpPlugin, and AddPluginToLists) are designed to be exception-safe and do not require additional try-catch error handling according to the maintainer Jack251970.

Applied to files:

  • Plugins/Flow.Launcher.Plugin.Explorer/Search/SearchManager.cs
📚 Learning: 2024-11-03T07:40:11.014Z
Learnt from: Yusyuriv
Repo: Flow-Launcher/Flow.Launcher PR: 3057
File: Flow.Launcher.Core/Plugin/JsonRPCPluginSettings.cs:0-0
Timestamp: 2024-11-03T07:40:11.014Z
Learning: In Flow Launcher, when using Windows Forms dialogs (e.g., in `JsonRPCPluginSettings.cs`), path validation is enabled by default in `OpenFileDialog` and `FolderBrowserDialog`, preventing users from selecting invalid paths, but it's possible to opt out of this validation on individual dialogs.

Applied to files:

  • Plugins/Flow.Launcher.Plugin.Explorer/Search/SearchManager.cs
📚 Learning: 2025-10-16T09:29:19.653Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3854
File: Flow.Launcher.Core/Plugin/PluginManager.cs:519-523
Timestamp: 2025-10-16T09:29:19.653Z
Learning: In Flow Launcher's PluginManager.cs QueryDialogJumpForPluginAsync method, when a DialogJump plugin is still initializing, returning null is intentional behavior. This allows the query to fall through to the default Explorer plugin, which serves as a fallback handler. This is different from QueryForPluginAsync and QueryHomeForPluginAsync, which show "still initializing" messages because they don't have fallback mechanisms.

Applied to files:

  • Plugins/Flow.Launcher.Plugin.Explorer/Search/SearchManager.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.Explorer/Search/SearchManager.cs
📚 Learning: 2025-09-11T08:30:29.487Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3593
File: Flow.Launcher/SettingPages/Views/SettingsPaneTheme.xaml:0-0
Timestamp: 2025-09-11T08:30:29.487Z
Learning: In Flow.Launcher's SettingsPaneTheme.xaml theme editor panel, the inner ui:ScrollViewerEx within the Border (Grid.Column="1") is intentionally kept because its height cannot be expanded to the whole page due to layout constraints. This serves a different scrolling purpose than the outer ScrollViewerEx and is necessary for the theme editor panel functionality.

Applied to files:

  • Plugins/Flow.Launcher.Plugin.Explorer/Views/ExplorerSettings.xaml
📚 Learning: 2025-09-13T06:04:26.977Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3593
File: Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml.cs:143-165
Timestamp: 2025-09-13T06:04:26.977Z
Learning: In Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml, the ListView_SizeChanged handler in the C# file assumes 5 GridViewColumns but the XAML may have more columns. The user Jack251970 indicated this mismatch doesn't need to be fixed when pointed out during PR #3593 review.

Applied to files:

  • Plugins/Flow.Launcher.Plugin.Explorer/Views/ExplorerSettings.xaml
📚 Learning: 2025-03-28T21:20:54.978Z
Learnt from: onesounds
Repo: Flow-Launcher/Flow.Launcher PR: 3394
File: Flow.Launcher/Themes/Darker Glass.xaml:134-141
Timestamp: 2025-03-28T21:20:54.978Z
Learning: In WPF applications like Flow.Launcher, Border elements cannot directly display text content and require a child element like TextBlock to handle text rendering. This separation of concerns (Border for visual container styling, TextBlock for text display) follows WPF best practices and provides greater styling flexibility.

Applied to files:

  • Plugins/Flow.Launcher.Plugin.Explorer/Views/ExplorerSettings.xaml
🧬 Code graph analysis (2)
Plugins/Flow.Launcher.Plugin.Explorer/Search/SearchManager.cs (3)
Plugins/Flow.Launcher.Plugin.Explorer/Settings.cs (2)
  • Settings (14-241)
  • ActionKeyword (228-239)
Plugins/Flow.Launcher.Plugin.Explorer/Search/EnvironmentVariables.cs (3)
  • EnvironmentVariables (10-109)
  • IsEnvironmentVariableSearch (25-31)
  • HasEnvironmentVar (33-41)
Plugins/Flow.Launcher.Plugin.Explorer/Search/QuickAccessLinks/QuickAccess.cs (3)
  • List (11-28)
  • List (30-40)
  • QuickAccess (7-41)
Plugins/Flow.Launcher.Plugin.Explorer/ViewModels/SettingsViewModel.cs (1)
Plugins/Flow.Launcher.Plugin.Explorer/Settings.cs (2)
  • Settings (14-241)
  • ActionKeyword (228-239)
🪛 GitHub Check: Check Spelling
Plugins/Flow.Launcher.Plugin.Explorer/Views/ExplorerSettings.xaml

[warning] 690-690:
quickaccess is not a recognized word. (unrecognized-spelling)

Plugins/Flow.Launcher.Plugin.Explorer/ViewModels/SettingsViewModel.cs

[warning] 282-282:
actionkeywordview is not a recognized word. (unrecognized-spelling)

⏰ 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 (7)
Plugins/Flow.Launcher.Plugin.Explorer/ViewModels/SettingsViewModel.cs (1)

282-286: LGTM!

The new action keyword entries follow the established pattern and are properly integrated with the underlying Settings API.

Plugins/Flow.Launcher.Plugin.Explorer/Views/ExplorerSettings.xaml (2)

677-677: LGTM!

The new checkbox for excluding Quick Access from action keywords is properly bound and follows the established UI patterns.

Also applies to: 684-691


693-705: LGTM!

The grid layout updates correctly accommodate the new checkbox row, and the ListView wrapping maintains proper visual hierarchy.

Also applies to: 708-708

Plugins/Flow.Launcher.Plugin.Explorer/Search/SearchManager.cs (4)

54-66: LGTM!

The active keyword detection and path search inference logic is well-structured and handles edge cases appropriately.


70-74: LGTM!

Returning all Quick Access links for empty searches with the QuickAccess action keyword provides intuitive user experience.


107-107: LGTM!

The result filtering logic properly applies the ShouldSkip check to exclude results that don't match the active action keyword.

Also applies to: 112-118


261-279: LGTM!

The ShouldSkip method correctly filters results based on the active action keyword, handling both excluded file types and keyword-specific type constraints.

Jack251970
Jack251970 previously approved these changes Nov 9, 2025
Copy link
Member

@Jack251970 Jack251970 left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution! LGTM.

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: 1

♻️ Duplicate comments (1)
Plugins/Flow.Launcher.Plugin.Explorer/Search/SearchManager.cs (1)

283-294: Quick Access entries still bypass folder/file filters

This helper adds every Quick Access hit even when activeActionKeyword is FolderSearchActionKeyword or FileSearchActionKeyword, so folders leak into file-only queries and vice versa. That defeats the new per-type filtering and matches the earlier unresolved feedback. Please run each Quick Access result through the same ShouldSkip logic (or pre-filter the collection) before unioning so action keywords remain authoritative.

You can reuse ShouldSkip:

-            var quickAccessMatched = QuickAccess.AccessLinkListMatched(query, Settings.QuickAccessLinks);
-            if (quickAccessMatched != null && quickAccessMatched.Count > 0) results.UnionWith(quickAccessMatched);
+            var quickAccessMatched = QuickAccess.AccessLinkListMatched(query, Settings.QuickAccessLinks);
+            if (quickAccessMatched is { Count: > 0 })
+            {
+                foreach (var qa in quickAccessMatched.Where(r =>
+                         !ShouldSkip(activeActionKeyword!.Value, (SearchResult)r.ContextData)))
+                {
+                    results.Add(qa);
+                }
+            }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1d60cb7 and 592f60a.

📒 Files selected for processing (2)
  • Plugins/Flow.Launcher.Plugin.Explorer/Search/SearchManager.cs (10 hunks)
  • Plugins/Flow.Launcher.Plugin.Explorer/Settings.cs (7 hunks)
🧰 Additional context used
🧠 Learnings (10)
📚 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.Explorer/Settings.cs
  • Plugins/Flow.Launcher.Plugin.Explorer/Search/SearchManager.cs
📚 Learning: 2025-07-21T09:19:19.012Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3854
File: Flow.Launcher.Core/Plugin/PluginManager.cs:280-292
Timestamp: 2025-07-21T09:19:19.012Z
Learning: In Flow Launcher's PluginManager.cs, the post-initialization operations (RegisterResultsUpdatedEvent, UpdatePluginMetadataTranslation, RegisterPluginActionKeywords, DialogJump.InitializeDialogJumpPlugin, and AddPluginToLists) are designed to be exception-safe and do not require additional try-catch error handling according to the maintainer Jack251970.

Applied to files:

  • Plugins/Flow.Launcher.Plugin.Explorer/Settings.cs
  • Plugins/Flow.Launcher.Plugin.Explorer/Search/SearchManager.cs
📚 Learning: 2025-10-16T09:29:19.653Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3854
File: Flow.Launcher.Core/Plugin/PluginManager.cs:519-523
Timestamp: 2025-10-16T09:29:19.653Z
Learning: In Flow Launcher's PluginManager.cs QueryDialogJumpForPluginAsync method, when a DialogJump plugin is still initializing, returning null is intentional behavior. This allows the query to fall through to the default Explorer plugin, which serves as a fallback handler. This is different from QueryForPluginAsync and QueryHomeForPluginAsync, which show "still initializing" messages because they don't have fallback mechanisms.

Applied to files:

  • Plugins/Flow.Launcher.Plugin.Explorer/Settings.cs
  • Plugins/Flow.Launcher.Plugin.Explorer/Search/SearchManager.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.Explorer/Settings.cs
  • Plugins/Flow.Launcher.Plugin.Explorer/Search/SearchManager.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.Explorer/Settings.cs
📚 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.Explorer/Settings.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.Explorer/Settings.cs
  • Plugins/Flow.Launcher.Plugin.Explorer/Search/SearchManager.cs
📚 Learning: 2025-06-24T19:06:48.344Z
Learnt from: Koisu-unavailable
Repo: Flow-Launcher/Flow.Launcher PR: 3770
File: Flow.Launcher/ViewModel/MainViewModel.cs:0-0
Timestamp: 2025-06-24T19:06:48.344Z
Learning: In Flow.Launcher's Explorer plugin results, the SubTitle property always contains the directory containing the file. For file results, Title contains the filename and SubTitle contains the parent directory. For directory results, SubTitle contains the directory path itself.

Applied to files:

  • Plugins/Flow.Launcher.Plugin.Explorer/Settings.cs
  • Plugins/Flow.Launcher.Plugin.Explorer/Search/SearchManager.cs
📚 Learning: 2024-11-03T07:40:11.014Z
Learnt from: Yusyuriv
Repo: Flow-Launcher/Flow.Launcher PR: 3057
File: Flow.Launcher.Core/Plugin/JsonRPCPluginSettings.cs:0-0
Timestamp: 2024-11-03T07:40:11.014Z
Learning: In Flow Launcher, when using Windows Forms dialogs (e.g., in `JsonRPCPluginSettings.cs`), path validation is enabled by default in `OpenFileDialog` and `FolderBrowserDialog`, preventing users from selecting invalid paths, but it's possible to opt out of this validation on individual dialogs.

Applied to files:

  • Plugins/Flow.Launcher.Plugin.Explorer/Settings.cs
  • Plugins/Flow.Launcher.Plugin.Explorer/Search/SearchManager.cs
📚 Learning: 2025-11-06T12:57:11.574Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 4072
File: Plugins/Flow.Launcher.Plugin.Program/Main.cs:100-111
Timestamp: 2025-11-06T12:57:11.574Z
Learning: In Flow Launcher's Program plugin Main.cs QueryAsync method, the semaphore pattern where WaitAsync(token) is inside a try-catch that only catches OperationCanceledException, followed by an explicit Release() call after the try-catch block, is safe and doesn't cause semaphore leaks. When WaitAsync throws OperationCanceledException, the semaphore is not acquired, so no release is needed. When WaitAsync succeeds, the Release() call after the try-catch executes normally.

Applied to files:

  • Plugins/Flow.Launcher.Plugin.Explorer/Search/SearchManager.cs
🧬 Code graph analysis (1)
Plugins/Flow.Launcher.Plugin.Explorer/Search/SearchManager.cs (5)
Plugins/Flow.Launcher.Plugin.Explorer/Settings.cs (2)
  • Settings (14-240)
  • ActionKeyword (228-239)
Plugins/Flow.Launcher.Plugin.Explorer/Search/EnvironmentVariables.cs (3)
  • EnvironmentVariables (10-109)
  • IsEnvironmentVariableSearch (25-31)
  • HasEnvironmentVar (33-41)
Plugins/Flow.Launcher.Plugin.Explorer/Search/QuickAccessLinks/QuickAccess.cs (3)
  • QuickAccess (7-41)
  • List (11-28)
  • List (30-40)
Plugins/Flow.Launcher.Plugin.Explorer/Search/ResultManager.cs (8)
  • ResultManager (16-412)
  • Result (62-72)
  • Result (95-173)
  • Result (175-178)
  • Result (180-183)
  • Result (185-227)
  • Result (257-283)
  • Result (285-343)
Plugins/Flow.Launcher.Plugin.Explorer/Main.cs (2)
  • List (54-57)
  • Main (18-131)
⏰ 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). (1)
  • GitHub Check: build

@Jack251970
Copy link
Member

@jjw24 It works well for me generally. But I am not particularly sure about the refactoring within the SearchAsync function, please review this for me, thanks!

@Jack251970 Jack251970 requested a review from jjw24 November 9, 2025 05:42
@jjw24
Copy link
Member

jjw24 commented Nov 9, 2025

Yeah no worries I will take a look too.

Copy link
Member

@jjw24 jjw24 left a comment

Choose a reason for hiding this comment

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

Just doing some initial testing, I am getting this with the file/folder action keyword when no query is inputted:
{A193A1F7-1DAA-4B5C-8AA0-F31C40811B46}

Copy link
Member

@jjw24 jjw24 left a comment

Choose a reason for hiding this comment

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

Searching with actual path should still be triggered with a general search:
{E74CA1F1-A0EC-4C48-9E7D-5B9E02F1D940}

@01Dri 01Dri requested a review from jjw24 November 22, 2025 19:14
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: 0

♻️ Duplicate comments (1)
Plugins/Flow.Launcher.Plugin.Explorer/Search/SearchManager.cs (1)

290-301: Quick Access results still bypass action keyword filtering.

This is the same critical issue flagged in the previous review: AccessLinkListMatched at line 299 returns all matching Quick Access entries regardless of activeActionKeyword. When FolderSearchActionKeyword is active, Quick Access files are incorrectly included; when FileSearchActionKeyword is active, Quick Access folders leak through. This contradicts the PR's goal of respecting action keyword scopes.

Apply the previously suggested fix to filter Quick Access by type:

-            var quickAccessMatched = QuickAccess.AccessLinkListMatched(query, Settings.QuickAccessLinks);
+            IEnumerable<AccessLink> quickAccessLinks = Settings.QuickAccessLinks;
+
+            if (activeActionKeyword == ActionKeyword.FolderSearchActionKeyword)
+                quickAccessLinks = quickAccessLinks.Where(link => link.Type != ResultType.File);
+            else if (activeActionKeyword == ActionKeyword.FileSearchActionKeyword)
+                quickAccessLinks = quickAccessLinks.Where(link => link.Type == ResultType.File);
+
+            var quickAccessMatched = QuickAccess.AccessLinkListMatched(query, quickAccessLinks);
             if (quickAccessMatched != null && quickAccessMatched.Count > 0) results.UnionWith(quickAccessMatched);

Minor style note: The if conditions at lines 292-294 can be combined into a single expression as suggested by Copilot:

-            if (activeActionKeyword != null 
-                && activeActionKeyword != ActionKeyword.QuickAccessActionKeyword
-                && Settings.ExcludeQuickAccessFromActionKeywords)
+            if (activeActionKeyword != null 
+                && activeActionKeyword != ActionKeyword.QuickAccessActionKeyword
+                && Settings.ExcludeQuickAccessFromActionKeywords)

(Though the current formatting is acceptable.)

🧹 Nitpick comments (1)
Plugins/Flow.Launcher.Plugin.Explorer/Search/SearchManager.cs (1)

86-110: Consider refactoring to if-else for clarity.

The switch-on-boolean pattern is unconventional and less readable than an if-else chain. While functionally correct, standard conditional branching would improve maintainability.

Consider this refactor:

-            switch (activeActionKeyword.Equals(ActionKeyword.PathSearchActionKeyword))
-            {
-                case true:
-                    results.UnionWith(await PathSearchAsync(query, token).ConfigureAwait(false));
-                    return [.. results];
-
-                case false
-                    // Intentionally require enabling of Everything's content search due to its slowness
-                    when activeActionKeyword.Equals(ActionKeyword.FileContentSearchActionKeyword):
-                    if (Settings.ContentIndexProvider is EverythingSearchManager && !Settings.EnableEverythingContentSearch)
-                        return EverythingContentSearchResult(query);
-
-                    searchResults = Settings.ContentIndexProvider.ContentSearchAsync("", query.Search, token);
-                    engineName = Enum.GetName(Settings.ContentSearchEngine);
-                    break;
-
-                case false
-                    when activeActionKeyword.Equals(ActionKeyword.QuickAccessActionKeyword):
-                    return QuickAccess.AccessLinkListMatched(query, Settings.QuickAccessLinks);
-
-                default:
-                    searchResults = Settings.IndexProvider.SearchAsync(query.Search, token);
-                    engineName = Enum.GetName(Settings.IndexSearchEngine);
-                    break;
-            }
+            if (activeActionKeyword.Equals(ActionKeyword.PathSearchActionKeyword))
+            {
+                results.UnionWith(await PathSearchAsync(query, token).ConfigureAwait(false));
+                return [.. results];
+            }
+            
+            if (activeActionKeyword.Equals(ActionKeyword.FileContentSearchActionKeyword))
+            {
+                // Intentionally require enabling of Everything's content search due to its slowness
+                if (Settings.ContentIndexProvider is EverythingSearchManager && !Settings.EnableEverythingContentSearch)
+                    return EverythingContentSearchResult(query);
+
+                searchResults = Settings.ContentIndexProvider.ContentSearchAsync("", query.Search, token);
+                engineName = Enum.GetName(Settings.ContentSearchEngine);
+            }
+            else if (activeActionKeyword.Equals(ActionKeyword.QuickAccessActionKeyword))
+            {
+                return QuickAccess.AccessLinkListMatched(query, Settings.QuickAccessLinks);
+            }
+            else
+            {
+                searchResults = Settings.IndexProvider.SearchAsync(query.Search, token);
+                engineName = Enum.GetName(Settings.IndexSearchEngine);
+            }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 592f60a and fc573bd.

📒 Files selected for processing (1)
  • Plugins/Flow.Launcher.Plugin.Explorer/Search/SearchManager.cs (10 hunks)
🧰 Additional context used
🧠 Learnings (8)
📚 Learning: 2025-07-21T09:19:19.012Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3854
File: Flow.Launcher.Core/Plugin/PluginManager.cs:280-292
Timestamp: 2025-07-21T09:19:19.012Z
Learning: In Flow Launcher's PluginManager.cs, the post-initialization operations (RegisterResultsUpdatedEvent, UpdatePluginMetadataTranslation, RegisterPluginActionKeywords, DialogJump.InitializeDialogJumpPlugin, and AddPluginToLists) are designed to be exception-safe and do not require additional try-catch error handling according to the maintainer Jack251970.

Applied to files:

  • Plugins/Flow.Launcher.Plugin.Explorer/Search/SearchManager.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.Explorer/Search/SearchManager.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.Explorer/Search/SearchManager.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.Explorer/Search/SearchManager.cs
📚 Learning: 2025-06-24T19:06:48.344Z
Learnt from: Koisu-unavailable
Repo: Flow-Launcher/Flow.Launcher PR: 3770
File: Flow.Launcher/ViewModel/MainViewModel.cs:0-0
Timestamp: 2025-06-24T19:06:48.344Z
Learning: In Flow.Launcher's Explorer plugin results, the SubTitle property always contains the directory containing the file. For file results, Title contains the filename and SubTitle contains the parent directory. For directory results, SubTitle contains the directory path itself.

Applied to files:

  • Plugins/Flow.Launcher.Plugin.Explorer/Search/SearchManager.cs
📚 Learning: 2025-10-16T09:29:19.653Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3854
File: Flow.Launcher.Core/Plugin/PluginManager.cs:519-523
Timestamp: 2025-10-16T09:29:19.653Z
Learning: In Flow Launcher's PluginManager.cs QueryDialogJumpForPluginAsync method, when a DialogJump plugin is still initializing, returning null is intentional behavior. This allows the query to fall through to the default Explorer plugin, which serves as a fallback handler. This is different from QueryForPluginAsync and QueryHomeForPluginAsync, which show "still initializing" messages because they don't have fallback mechanisms.

Applied to files:

  • Plugins/Flow.Launcher.Plugin.Explorer/Search/SearchManager.cs
📚 Learning: 2024-11-03T07:40:11.014Z
Learnt from: Yusyuriv
Repo: Flow-Launcher/Flow.Launcher PR: 3057
File: Flow.Launcher.Core/Plugin/JsonRPCPluginSettings.cs:0-0
Timestamp: 2024-11-03T07:40:11.014Z
Learning: In Flow Launcher, when using Windows Forms dialogs (e.g., in `JsonRPCPluginSettings.cs`), path validation is enabled by default in `OpenFileDialog` and `FolderBrowserDialog`, preventing users from selecting invalid paths, but it's possible to opt out of this validation on individual dialogs.

Applied to files:

  • Plugins/Flow.Launcher.Plugin.Explorer/Search/SearchManager.cs
📚 Learning: 2025-11-06T12:57:11.574Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 4072
File: Plugins/Flow.Launcher.Plugin.Program/Main.cs:100-111
Timestamp: 2025-11-06T12:57:11.574Z
Learning: In Flow Launcher's Program plugin Main.cs QueryAsync method, the semaphore pattern where WaitAsync(token) is inside a try-catch that only catches OperationCanceledException, followed by an explicit Release() call after the try-catch block, is safe and doesn't cause semaphore leaks. When WaitAsync throws OperationCanceledException, the semaphore is not acquired, so no release is needed. When WaitAsync succeeds, the Release() call after the try-catch executes normally.

Applied to files:

  • Plugins/Flow.Launcher.Plugin.Explorer/Search/SearchManager.cs
🧬 Code graph analysis (1)
Plugins/Flow.Launcher.Plugin.Explorer/Search/SearchManager.cs (7)
Plugins/Flow.Launcher.Plugin.Explorer/Settings.cs (2)
  • Settings (14-240)
  • ActionKeyword (228-239)
Flow.Launcher.Plugin/SharedCommands/FilesFolders.cs (1)
  • IsLocationPathString (246-265)
Plugins/Flow.Launcher.Plugin.Explorer/Search/EnvironmentVariables.cs (3)
  • EnvironmentVariables (10-109)
  • IsEnvironmentVariableSearch (25-31)
  • HasEnvironmentVar (33-41)
Plugins/Flow.Launcher.Plugin.Explorer/Search/QuickAccessLinks/QuickAccess.cs (3)
  • QuickAccess (7-41)
  • List (11-28)
  • List (30-40)
Plugins/Flow.Launcher.Plugin.Explorer/Search/WindowsIndex/WindowsIndexSearchManager.cs (7)
  • IAsyncEnumerable (24-44)
  • IAsyncEnumerable (46-63)
  • IAsyncEnumerable (65-84)
  • IAsyncEnumerable (86-89)
  • IAsyncEnumerable (91-94)
  • IAsyncEnumerable (96-99)
  • IAsyncEnumerable (101-120)
Plugins/Flow.Launcher.Plugin.Explorer/Search/Everything/EverythingSearchManager.cs (3)
  • IAsyncEnumerable (75-90)
  • IAsyncEnumerable (92-126)
  • IAsyncEnumerable (128-145)
Plugins/Flow.Launcher.Plugin.Explorer/Search/ResultManager.cs (8)
  • ResultManager (16-412)
  • Result (62-72)
  • Result (95-173)
  • Result (175-178)
  • Result (180-183)
  • Result (185-227)
  • Result (257-283)
  • Result (285-343)
⏰ 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). (1)
  • GitHub Check: build
🔇 Additional comments (5)
Plugins/Flow.Launcher.Plugin.Explorer/Search/SearchManager.cs (5)

1-11: LGTM! Clean imports for the refactored code.

The new using statements appropriately support the refactored action keyword logic and helper methods. The static using for Settings enables cleaner enum access.


51-73: LGTM! Action keyword determination is sound.

The logic correctly distinguishes path searches from keyword-based searches and appropriately returns empty results when no action keyword matches, preventing the plugin from handling queries it shouldn't.


265-288: LGTM! Clean filtering logic.

The ShouldSkip method appropriately centralizes result filtering based on action keywords and file type exclusions. The logic correctly filters folders vs. files for their respective action keywords.


116-123: LGTM! Filtering correctly applied to search results.

The ShouldSkip filtering is properly integrated into the result processing loop. The null-forgiving operator on activeActionKeyword!.Value is safe since line 70 ensures it's non-null at this point.


72-73: LGTM! Modern collection expression syntax.

The consistent use of C# collection expressions ([], [.. results]) throughout the refactored code is idiomatic and improves readability.

Also applies to: 90-90, 127-127, 141-142, 146-160

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

10 min review enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants