-
-
Notifications
You must be signed in to change notification settings - Fork 344
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
Use Command Keyword to Search System Command #3344
Use Command Keyword to Search System Command #3344
Conversation
This comment has been minimized.
This comment has been minimized.
Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX. |
📝 WalkthroughWalkthroughThe changes involve significant updates to the Changes
Suggested reviewers
Suggested labels
Poem
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
Pull request was converted to draft
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
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: 1
🧹 Nitpick comments (5)
Plugins/Flow.Launcher.Plugin.Sys/SettingsViewModel.cs (1)
1-12
: Simple View Model for System Command SettingsThis is a clean implementation of a ViewModel for the System Commands settings. It follows the MVVM pattern by providing a way to bind the UI to the Settings object.
Consider implementing
INotifyPropertyChanged
if you need to support UI updates when properties change, though it's not strictly necessary for this simple case sinceSettings
is a read-only property.Plugins/Flow.Launcher.Plugin.Sys/CommandKeywordSetting.xaml (2)
91-98
: Fix misleading control nameThe button is named
btnTestActionKeyword
but its function is to reset the keyword (as indicated by the click handlerOnResetButtonClick
and the "Reset" text content). This naming inconsistency could cause confusion for future maintenance.- <Button - x:Name="btnTestActionKeyword" + <Button + x:Name="btnResetKeyword" Grid.Row="1" Grid.Column="2" Margin="0 0 10 0" Padding="10 5 10 5" Click="OnResetButtonClick" Content="{DynamicResource flowlauncher_plugin_sys_reset}" />
1-121
: Well-structured UI for Command Keyword editingThe UI is well-designed with proper layout and styling. The window provides a good user experience for editing command keywords, with clear labels and intuitive controls.
Consider adding a tooltip or help text to explain the purpose of the keyword feature to users, especially since this is a new feature.
Plugins/Flow.Launcher.Plugin.Sys/Main.cs (2)
23-46
: Consolidate or clarify overlapping command labels.You have commands like "Restart" and "Restart Flow Launcher" in the same dictionary. While functionally distinct, it may be worth distinguishing them more clearly or ensuring their distinction is evident in UI and logs.
96-104
: Check for null command keywords.Using
_context.API.FuzzySearch
withcommand.Keyword
might throw an error ifcommand.Keyword
is null. If user input or older settings lead to empty keywords, this could cause issues.Consider defaulting
command.Keyword
to an empty string in theSettings
model or defending against null here.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
Plugins/Flow.Launcher.Plugin.Sys/Command.cs
(1 hunks)Plugins/Flow.Launcher.Plugin.Sys/CommandKeywordSetting.xaml
(1 hunks)Plugins/Flow.Launcher.Plugin.Sys/CommandKeywordSetting.xaml.cs
(1 hunks)Plugins/Flow.Launcher.Plugin.Sys/Languages/en.xaml
(3 hunks)Plugins/Flow.Launcher.Plugin.Sys/Main.cs
(11 hunks)Plugins/Flow.Launcher.Plugin.Sys/Settings.cs
(1 hunks)Plugins/Flow.Launcher.Plugin.Sys/SettingsViewModel.cs
(1 hunks)Plugins/Flow.Launcher.Plugin.Sys/SysSettings.xaml
(1 hunks)Plugins/Flow.Launcher.Plugin.Sys/SysSettings.xaml.cs
(2 hunks)Plugins/Flow.Launcher.Plugin.Sys/ThemeSelector.cs
(2 hunks)Plugins/Flow.Launcher.Plugin.WebSearch/SettingsViewModel.cs
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
🔇 Additional comments (22)
Plugins/Flow.Launcher.Plugin.WebSearch/SettingsViewModel.cs (1)
1-12
:✅ Verification successful
Code simplification looks good
The simplified ViewModel for WebSearch plugin follows the same pattern as the new Sys plugin ViewModel, which provides consistency across the codebase.
The removal of the storage-related code suggests that saving functionality is now handled elsewhere, which should be verified to ensure settings persistence still works correctly.
🏁 Script executed:
#!/bin/bash # Verify if saving functionality is implemented elsewhere echo "Checking for save functionality in related files..." rg -A 5 "Save.*Settings" --type csLength of output: 9992
Simplified ViewModel verified – Global saving mechanism confirmed
The refactored
SettingsViewModel
in the WebSearch plugin now omits storage logic, which is consistent with the refactoring seen in the Sys plugin. The repository-wide search confirms that saving functionality (e.g., viaAPI.SaveAppAllSettings()
and related methods) is implemented elsewhere. This global approach ensures settings persistence without requiring each plugin ViewModel to manage its own storage.No further changes are needed here, but please ensure that integration tests confirm that settings continue to persist correctly.
Plugins/Flow.Launcher.Plugin.Sys/ThemeSelector.cs (2)
5-5
: Good use of type aliasing to avoid naming conflictsUsing the alias
FLSettings
forFlow.Launcher.Infrastructure.UserSettings.Settings
is a good approach to avoid naming conflicts with the newSettings
class in the same namespace.
13-13
: Consistent update of type referencesThe update of the field type from
Settings
toFLSettings
and its corresponding initialization is properly implemented.Also applies to: 46-46
Plugins/Flow.Launcher.Plugin.Sys/Command.cs (1)
1-44
: Well-structured model class for command customizationThis Command class is well-designed with:
- Clear property structure with proper change notification
- Appropriate use of JsonIgnore to prevent serialization of UI-specific properties
- Consistent implementation of property change notifications
The class effectively supports the new command keyword customization feature.
Plugins/Flow.Launcher.Plugin.Sys/CommandKeywordSetting.xaml.cs (3)
10-17
: Well-implemented dialog initializationGood implementation of the dialog constructor with proper initialization of UI elements and translation support. The use of string formatting for the tip text enhances user understanding.
24-37
: Proper validation and error handling for keyword inputThe confirmation logic correctly validates against empty keywords and displays appropriate error messages using the translation system. This ensures users understand the requirement for non-empty keywords.
39-43
: Useful reset functionalityThe reset button implementation provides a convenient way for users to revert to the default keyword. The comment explaining that Key is the default value adds clarity to the code.
Plugins/Flow.Launcher.Plugin.Sys/SysSettings.xaml (6)
7-8
: Good design-time supportAdding the ViewModel namespace and DataContext for design-time support improves development experience by providing IntelliSense and preview capabilities in the XAML designer.
12-16
: Well-structured layoutThe updated Grid with proper row definitions creates a clean separation between the command list and action buttons, improving the UI organization.
24-26
: Proper data binding implementationThe ListView now correctly binds to the Commands collection and tracks the selected command, providing the foundation for the keyword customization feature.
31-35
: Updated column bindings for clarityThe changes to column headers and bindings from Title/SubTitle to Name/Description improve clarity and better align with the Command object properties.
Also applies to: 39-45
47-53
: New column for command keywordsThe addition of a dedicated column for displaying command keywords gives users clear visibility of the customized keywords.
58-67
: Well-positioned action buttonThe Edit button is appropriately positioned in a separate row with proper alignment and spacing, providing a clear call-to-action for customizing command keywords.
Plugins/Flow.Launcher.Plugin.Sys/SysSettings.xaml.cs (4)
8-16
: Validate constructor dependencies.The constructor now accepts and stores both
_context
and_settings
for the control. Generally, storing the plugin context in a UI control is acceptable if it’s truly needed, but ensure that no null reference exceptions occur if_context
orviewModel
is not properly initialized.Please verify upstream usage to confirm
_context
is never null at construction time; otherwise, consider defensive checks (e.g.,ArgumentNullException
).
25-27
: Column resizing logic looks consistent.The adjusted column widths (
0.2
,0.6
,0.2
) sum to 1.0 and should evenly fill the availableListView
space. This design is straightforward and should not pose performance or layout issues.Also applies to: 36-37
39-43
: Consider null-check before opening the CommandKeywordSettingWindow.When calling the window constructor (
CommandKeywordSettingWindow(_context, _settings.SelectedCommand)
), it’s assumed_settings.SelectedCommand
is valid. If_settings.SelectedCommand
could be null, guard against that case to prevent exceptions.Please confirm that
_settings.SelectedCommand
is always set before a user can click "Edit".
45-51
: Double-click handling logic looks clean.The double-click event properly verifies the data context is a
Command
and checks_settings.SelectedCommand
for null. This helps avoid accidental null dereferences or exceptions. No issues found.Plugins/Flow.Launcher.Plugin.Sys/Main.cs (2)
77-78
: Guard against empty or missing commands.
_settings.Commands.First(x => x.Key == c.Title)
will throw an exception if no command is found, which could happen if_settings.Commands
is missing the corresponding command. Consider usingFirstOrDefault
with a null check or validating that the key exists.Also applies to: 79-79
144-154
: Ensure collection safety inUpdateLocalizedNameDescription
.
_settings.Commands[0]
could lead to an index out-of-range exception if_settings.Commands
is empty. Add a defensive check to avoid potential runtime errors.Plugins/Flow.Launcher.Plugin.Sys/Languages/en.xaml (3)
7-9
: Confirm resource usage and naming consistency.“Name” (
flowlauncher_plugin_sys_name
) vs. “Command” (flowlauncher_plugin_sys_command
) might cause confusion if both appear in similar contexts. Verify that these are used in clearly distinct UI columns or labels.
33-33
: New 'Edit' resource is consistent with UI.Defining
flowlauncher_plugin_sys_edit
for future usage is a good extension. No further issues identified.
67-75
: Useful additions for customizing command keywords.The newly introduced resource strings clarify user actions for command keyword configuration and error handling. This should enhance user experience when customizing search keywords.
@onesounds I have completed my work and please review when you have a chance. Regarding the tips for changing the command keyword, I would like your feedback on whether they are clear enough for users. |
🥷 Code experts: onesounds 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:
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: To learn more about /:\ gitStream - Visit our Docs |
@check-spelling-bot Report🔴 Please reviewSee the 📂 files view, the 📜action log, or 📝 job summary for details.
See ❌ Event descriptions for more information. If the flagged items are 🤯 false positivesIf items relate to a ...
|
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.Sys/Main.cs (1)
337-338
: Consider localizing error messageWhile most messages use localization, this error message for recycling bin operation is still hardcoded in English.
Consider using the translation API for this error message as well:
- _context.API.ShowMsgBox("Failed to empty the recycle bin. This might happen if:\n" + + _context.API.ShowMsgBox(_context.API.GetTranslation("flowlauncher_plugin_sys_failed_empty_recycle_bin") + "- A file in the recycle bin is in use\n" + "- You don't have permission to delete some items\n" + "Please close any applications that might be using these files and try again.", "Error", MessageBoxButton.OK, MessageBoxImage.Error);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Plugins/Flow.Launcher.Plugin.Sys/Main.cs
(11 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Report (PR)
🔇 Additional comments (13)
Plugins/Flow.Launcher.Plugin.Sys/Main.cs (13)
23-46
: Good organizational structure for command mappingThe introduction of a structured dictionary for command mappings provides a clear and maintainable way to manage system commands and their translations. This approach eliminates potential string inconsistencies and centralizes command definitions.
47-47
: KeywordDescriptionMappings empty initialization is appropriateThe empty initialization of KeywordDescriptionMappings is correct as it will be populated later in the Init method, providing a clean separation between declaration and population logic.
49-54
: Improved code documentationAdding clear comments for the shutdown reason constants improves code understanding and maintainability.
55-58
: ViewModel pattern implementationThe addition of the SettingsViewModel field implements the MVVM pattern, which is a good practice for separating UI logic from data models.
62-63
: Initialization before UI creationCalling UpdateLocalizedNameDescription before creating the settings panel ensures the UI will display properly localized content from the start.
77-80
: Core feature implementation for command keyword searchingThis code effectively implements the command keyword feature by retrieving command names and descriptions from settings, allowing users to search by their preferred keywords instead of relying solely on English titles.
81-89
: Comprehensive search matching logicThe search logic now checks across title, subtitle, and keyword fields, which provides users with more flexible ways to find commands. The implementation correctly calculates and compares match scores to determine the best result.
106-115
: Simplified title retrieval methodThe renamed and streamlined GetTitle method improves code readability while maintaining proper error handling with helpful log messages.
117-126
: Parallel implementation for descriptionsThe new GetDescription method follows the same pattern as GetTitle, maintaining consistency in the codebase while extending functionality to include command descriptions.
130-138
: Clever description mapping logicThe implementation cleverly derives description translation keys by removing the "_cmd" suffix from title keys, avoiding the need to maintain two separate mapping dictionaries.
141-151
: Efficient localization update mechanismThe UpdateLocalizedNameDescription method efficiently handles command localization by only updating when necessary (empty names or forced update), which is a good performance optimization.
206-209
: Consistently localized dialog messagesThe dialog messages are now properly localized using the translation API, which improves the user experience for non-English users.
Also applies to: 227-230, 248-251, 269-272
505-508
: Culture change handlingThe implementation of OnCultureInfoChanged ensures that command names and descriptions are properly updated when the user changes the language settings.
Good. there are no issues. (It works fine even when no command is entered or when there are duplicate inputs.) There was a request before to allow certain items to be disabled (since options like "Restart with Advanced" aren’t used often), but I’m not sure if that should be included. For now, this PR is good to go, so I'll approve it. |
Use Command Keyword to Search System Command
Previously, FL uses the English title & translated title & subtitle to search the commands. If English title is matched, the result title will be in English which I think can cause confusion for users.
This PR Introduces command keyword to replace the original function of searching with English title. (Users can still search with translated title & subtitle) And users can change this command keyword.
By default, the command keywords are the same as the English title. And users can set whatever they like for faster query.
Demo
2025-03-13.23-58-58.mp4
Test
Close #3343