BUG: Explorer plugin navigate path should only show results in current path #4053
Conversation
📝 WalkthroughWalkthroughRemoved early quick-access population inside the action-keyword branch, added an early-return path when QuickAccessActionKeyword matches, and ensured quick-access results are merged at the end for non-early-return searches; minor whitespace and a clarifying comment were added. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant SearchManager
participant PrimarySearch
participant QuickAccess
rect rgb(245,245,255)
note over SearchManager: Old flow (before)
User->>SearchManager: Query
SearchManager->>PrimarySearch: Build main results
alt ActionKeyword branch (old)
SearchManager->>QuickAccess: AccessLinkListMatched(query, links)
QuickAccess-->>SearchManager: Quick access results
SearchManager->>SearchManager: UnionWith (early)
end
SearchManager-->>User: Final results
end
sequenceDiagram
autonumber
actor User
participant SearchManager
participant PrimarySearch
participant QuickAccess
rect rgb(235,255,235)
note over SearchManager: New flow (after)
User->>SearchManager: Query
SearchManager->>PrimarySearch: Build main results
alt QuickAccessActionKeyword matched (early-return)
SearchManager->>QuickAccess: AccessLinkListMatched(query, links)
QuickAccess-->>SearchManager: Quick access results
SearchManager-->>User: Return quick-access results (early)
else Not early-return
SearchManager->>PrimarySearch: Continue other search branches
SearchManager->>QuickAccess: AccessLinkListMatched(query, links)
QuickAccess-->>SearchManager: Quick access results
SearchManager->>SearchManager: UnionWith (final step)
SearchManager-->>User: Final results
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
|
🥷 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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
Plugins/Flow.Launcher.Plugin.Explorer/Search/SearchManager.cs (2)
112-113: Consider adding a clarifying comment.This line is only reached for FileContentSearch and IndexSearch actions (both non-path searches), which might not be immediately obvious. Consider adding a comment to clarify when quick access links are merged.
+ // Merge quick access links for non-path searches (FileContentSearch and IndexSearch) results.UnionWith(QuickAccess.AccessLinkListMatched(query, Settings.QuickAccessLinks));
63-63: Optional: Comment could be slightly clearer.The comment is technically correct but requires domain knowledge to fully understand. Consider rephrasing for clarity.
- // Handle scenario where all search options have action keyword set, return to avoid running further search logic. + // No action keyword matched - plugin should not handle this query, return empty results.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Plugins/Flow.Launcher.Plugin.Explorer/Search/SearchManager.cs(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
Plugins/Flow.Launcher.Plugin.Explorer/Search/SearchManager.cs (3)
Plugins/Flow.Launcher.Plugin.Explorer/Search/QuickAccessLinks/QuickAccess.cs (3)
List(11-28)List(30-40)QuickAccess(7-41)Flow.Launcher.Plugin/SharedCommands/FilesFolders.cs (1)
IsLocationPathString(246-265)Plugins/Flow.Launcher.Plugin.Explorer/Settings.cs (1)
Settings(13-205)
⏰ 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). (2)
- GitHub Check: build
- GitHub Check: Check Spelling
🔇 Additional comments (3)
Plugins/Flow.Launcher.Plugin.Explorer/Search/SearchManager.cs (3)
77-83: LGTM! Path searches correctly exclude quick access links.The early return here ensures that path searches (with PathSearchActionKeyword or SearchActionKeyword) do not merge quick access links, which correctly implements the PR objective.
104-106: LGTM! QuickAccessActionKeyword correctly returns only filtered quick access results.This new case properly handles the QuickAccessActionKeyword for both path-like and non-path queries, ensuring users see only quick access links when using this specific keyword.
52-140: Excellent fix! Logic correctly prevents quick access links in path searches.The changes properly implement the PR objective:
- Path searches (lines 77-83) return early without merging quick access links ✓
- Non-path searches (FileContentSearch and IndexSearch) merge quick access links at line 112 ✓
- QuickAccessActionKeyword (lines 104-106) returns only quick access results ✓
All execution paths have been verified to behave correctly.
|
@01Dri the change is good, just needed to handle one more scenario where users may search through Quick Access list using path query. I have added the change, please have a look and let me know if it's ok. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
Plugins/Flow.Launcher.Plugin.Explorer/Search/SearchManager.cs (2)
69-72: Minor: cachequery.Searchand short‑circuit empty input.Small readability/perf win and avoids repeating property calls.
- bool isPathSearch = query.Search.IsLocationPathString() - || EnvironmentVariables.IsEnvironmentVariableSearch(query.Search) - || EnvironmentVariables.HasEnvironmentVar(query.Search); + var search = query.Search; + bool isPathSearch = !string.IsNullOrEmpty(search) && ( + search.IsLocationPathString() + || EnvironmentVariables.IsEnvironmentVariableSearch(search) + || EnvironmentVariables.HasEnvironmentVar(search));
112-114: Quick Access union for non‑path flows looks right; two tiny nits.
- Behavior: When File Content Search is selected and Everything content search is disabled, the early return at Lines 88‑91 bypasses this union. Confirm that you don’t want Quick Access mixed into that “enable content search” prompt.
- Micro‑opt: skip the call on empty search.
- // Merge Quick Access Link results for non-path searches. - results.UnionWith(QuickAccess.AccessLinkListMatched(query, Settings.QuickAccessLinks)); + // Merge Quick Access Link results for non-path flows (Index/FileContent searches). + if (!string.IsNullOrEmpty(query.Search)) + results.UnionWith(QuickAccess.AccessLinkListMatched(query, Settings.QuickAccessLinks));If intended to include Quick Access with the “enable content search” result, move this union before the early return at Lines 88‑91 or append a second union just before that return.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Plugins/Flow.Launcher.Plugin.Explorer/Search/SearchManager.cs(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
Plugins/Flow.Launcher.Plugin.Explorer/Search/SearchManager.cs (3)
Plugins/Flow.Launcher.Plugin.Explorer/Search/QuickAccessLinks/QuickAccess.cs (3)
List(11-28)List(30-40)QuickAccess(7-41)Flow.Launcher.Plugin/SharedCommands/FilesFolders.cs (1)
IsLocationPathString(246-265)Plugins/Flow.Launcher.Plugin.Explorer/Settings.cs (1)
Settings(13-205)
⏰ 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 (2)
Plugins/Flow.Launcher.Plugin.Explorer/Search/SearchManager.cs (2)
63-65: Good guard and clear intent.Early return on unmatched action keyword is correct for scoping the plugin. Comment reads well.
104-107: Early return for Quick Access action (path or not) — nice.This cleanly handles “search Quick Access by path‑looking queries” and matches the reviewer’s scenario.
Please sanity‑check:
- qa + empty search → full Quick Access list.
- qa + “C:\” → filtered Quick Access items only.
- search/path keywords + “C:\” → only filesystem results, no Quick Access.
I tested the new scenario and it worked correctly. Everything looks good |
3 participants
CHANGES