Add new api Result.QuerySuggestionText #3899
Conversation
|
🥷 Code experts: onesounds, Yusyuriv Jack251970, onesounds have 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: ✨ Comment |
|
Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX. |
📝 WalkthroughWalkthroughAdds Changes
Sequence Diagram(s)sequenceDiagram
participant User as User Input
participant Converter as QuerySuggestionBoxConverter
participant Result as Result
User->>Converter: provide current query text
Converter->>Result: read QuerySuggestionText (or Title)
Converter->>Converter: validate suggestion starts-with query (case-insensitive)
Converter->>Converter: assemble suggestion using span-based concat
Converter-->>User: return suggestion (or none)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
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). (2)
✨ Finishing Touches
🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
Flow.Launcher.Plugin/Result.cs (2)
260-265: Clarify docs: don’t include action keyword; treat whitespace as “not set”Given the converter prepends ActionKeywordAssigned + " ", plugin authors should not include the action keyword in QuerySuggestionText. Also, “not set” should typically include whitespace.
Update the XML comments:
- /// <remarks>When a value is not set, the <see cref="Title"/> will be used.</remarks> + /// <remarks> + /// When not set or whitespace, the <see cref="Title"/> will be used. + /// Do not include the action keyword prefix; Flow prepends <see cref="ActionKeywordAssigned"/> + " " automatically. + /// </remarks>
260-265: API addition checklist (versioning/docs)Nice additive API. Please ensure:
- Flow.Launcher.Plugin package version and changelog reflect the new property.
- Plugin author docs mention the new property and usage (exclude action keyword, must align with the typed prefix).
I can draft the CHANGELOG/docs snippet if helpful.
Flow.Launcher/Converters/QuerySuggestionBoxConverter.cs (4)
36-39: Treat whitespace as “not set” to avoid blank suggestionsIf a plugin sets QuerySuggestionText to spaces, it currently overrides Title. Using IsNullOrWhiteSpace better matches “not set” semantics and prevents empty-looking suggestions.
Apply this minimal change:
- var selectedResultPossibleSuggestion = selectedResultActionKeyword + - (string.IsNullOrEmpty(selectedResult.QuerySuggestionText) ? - selectedResult.Title : - selectedResult.QuerySuggestionText); + var selectedResultPossibleSuggestion = selectedResultActionKeyword + + (string.IsNullOrWhiteSpace(selectedResult.QuerySuggestionText) ? + selectedResult.Title : + selectedResult.QuerySuggestionText);
36-39: Optional: guard against double-prepending the action keywordIf a plugin mistakenly includes the action keyword in QuerySuggestionText, this can lead to duplicated prefixes. We can avoid that without breaking existing behavior.
Alternative refactor for these lines:
- var selectedResultPossibleSuggestion = selectedResultActionKeyword + - (string.IsNullOrEmpty(selectedResult.QuerySuggestionText) ? - selectedResult.Title : - selectedResult.QuerySuggestionText); + var suggestionCore = string.IsNullOrWhiteSpace(selectedResult.QuerySuggestionText) + ? selectedResult.Title + : selectedResult.QuerySuggestionText; + + var selectedResultPossibleSuggestion = + string.IsNullOrEmpty(selectedResultActionKeyword) + || suggestionCore.StartsWith(selectedResultActionKeyword, StringComparison.CurrentCultureIgnoreCase) + ? suggestionCore + : selectedResultActionKeyword + suggestionCore;This keeps the auto-prefixing but avoids doubling it when the plugin already prefixed.
46-46: Use AsSpan() on both arguments for broader overload supportUsing string.Concat(queryText.AsSpan(), ...) ensures the compiler selects the ReadOnlySpan overload on all supported TFMs and keeps the no-substring-allocation benefit.
Apply:
- selectedItem.QuerySuggestionText = string.Concat(queryText, selectedResultPossibleSuggestion.AsSpan(queryText.Length)); + selectedItem.QuerySuggestionText = string.Concat(queryText.AsSpan(), selectedResultPossibleSuggestion.AsSpan(queryText.Length));
36-39: Add unit tests for new behaviorPlease add tests covering:
- Prefers QuerySuggestionText over Title.
- Falls back when QuerySuggestionText is null/whitespace.
- Action keyword is auto-prefixed and not duplicated.
- Case-insensitive StartsWith handling.
- No suggestion when prefix doesn’t match.
I can draft test cases targeting QuerySuggestionBoxConverter to validate these scenarios. Let me know your preferred test framework/project layout.
Also applies to: 46-46
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Flow.Launcher.Plugin/Result.cs(1 hunks)Flow.Launcher/Converters/QuerySuggestionBoxConverter.cs(1 hunks)
⏰ 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
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
Flow.Launcher/Converters/QuerySuggestionBoxConverter.cs (1)
51-61: Fix misleading comment and guard against null/whitespace TitleThe comment mentions QuerySuggestionText while this branch handles Title. Also, guard against null/whitespace Title to avoid generating a malformed candidate.
- // Then check Title as suggestion + // Then check Title as suggestion if (string.IsNullOrEmpty(selectedResultPossibleSuggestion)) { - selectedResultPossibleSuggestion = selectedResultActionKeyword + selectedResult.Title; - - // If this QuerySuggestionText does not start with the queryText, set it to null - if (!selectedResultPossibleSuggestion.StartsWith(queryText, StringComparison.CurrentCultureIgnoreCase)) - { - selectedResultPossibleSuggestion = null; - } + if (!string.IsNullOrWhiteSpace(selectedResult.Title)) + { + selectedResultPossibleSuggestion = selectedResultActionKeyword + selectedResult.Title; + // If this Title does not start with the queryText, set it to null + if (!selectedResultPossibleSuggestion.StartsWith(queryText, StringComparison.CurrentCultureIgnoreCase)) + { + selectedResultPossibleSuggestion = null; + } + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Flow.Launcher.Plugin/Result.cs(2 hunks)Flow.Launcher/Converters/QuerySuggestionBoxConverter.cs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- Flow.Launcher.Plugin/Result.cs
⏰ 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)
Flow.Launcher/Converters/QuerySuggestionBoxConverter.cs (1)
68-68: No changes needed: TFM supports the overload and span slicing is already safeAll projects target .NET 9.0, which includes
string.Concat(string, ReadOnlySpan<char>), and the precedingStartsWith(queryText)check guaranteesselectedResultPossibleSuggestion.Length >= queryText.Length, soAsSpan(queryText.Length)cannot go out of range. Keeping the existing code is correct.
| /// <remarks>When a value is not set, the <see cref="Title"/> will be used.</remarks> | ||
| /// <remarks> | ||
| /// When a value is not set, the <see cref="Title"/> will be used. | ||
| /// Please include the action keyword prefix when necessary because Flow does not prepend it automatically. |
…ionText_API Add new api Result.QuerySuggestionText
Add new API Result.QuerySuggestionText
This can help developers change query suggestion text for each result.
Test