IPluginHotkey Interface for Global Hotkey & Window Hotkey / Support Rename File & Folder by Hotkey or Context Menu #3770
Conversation
|
🥷 Code experts: Jack251970, onesounds 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:
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:
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 |
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.
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.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
![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 (1)
Flow.Launcher.Infrastructure/DialogJump/DialogJump.cs (1)
700-700: Fix typo in log messageThere's a typo in the log message that was flagged by the pipeline spelling check.
- Log.Debug(ClassName, $"Destory dialog: {hwnd}"); + Log.Debug(ClassName, $"Destroy dialog: {hwnd}");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
Flow.Launcher.Core/Plugin/PluginManager.cs(8 hunks)Flow.Launcher.Core/Resource/Internationalization.cs(1 hunks)Flow.Launcher.Infrastructure/DialogJump/DialogJump.cs(2 hunks)Flow.Launcher.Infrastructure/Hotkey/RegisteredHotkeyData.cs(7 hunks)Flow.Launcher.Infrastructure/UserSettings/Settings.cs(4 hunks)Flow.Launcher/Helper/HotKeyMapper.cs(4 hunks)Flow.Launcher/HotkeyControl.xaml.cs(4 hunks)Flow.Launcher/Languages/en.xaml(1 hunks)Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs(1 hunks)Flow.Launcher/SettingPages/Views/SettingsPaneHotkey.xaml(5 hunks)Flow.Launcher/ViewModel/MainViewModel.cs(2 hunks)Plugins/Flow.Launcher.Plugin.Explorer/ContextMenu.cs(2 hunks)Plugins/Flow.Launcher.Plugin.Explorer/Helper/RenameThing.cs(1 hunks)Plugins/Flow.Launcher.Plugin.Explorer/Languages/en.xaml(2 hunks)Plugins/Flow.Launcher.Plugin.Explorer/Main.cs(3 hunks)Plugins/Flow.Launcher.Plugin.Explorer/Search/ResultManager.cs(6 hunks)Plugins/Flow.Launcher.Plugin.Program/Main.cs(2 hunks)Plugins/Flow.Launcher.Plugin.Program/Programs/UWPPackage.cs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (13)
- Flow.Launcher.Core/Resource/Internationalization.cs
- Flow.Launcher/Languages/en.xaml
- Flow.Launcher/ViewModel/MainViewModel.cs
- Plugins/Flow.Launcher.Plugin.Program/Programs/UWPPackage.cs
- Plugins/Flow.Launcher.Plugin.Program/Main.cs
- Flow.Launcher/HotkeyControl.xaml.cs
- Plugins/Flow.Launcher.Plugin.Explorer/ContextMenu.cs
- Plugins/Flow.Launcher.Plugin.Explorer/Languages/en.xaml
- Flow.Launcher/SettingPages/Views/SettingsPaneHotkey.xaml
- Plugins/Flow.Launcher.Plugin.Explorer/Search/ResultManager.cs
- Plugins/Flow.Launcher.Plugin.Explorer/Helper/RenameThing.cs
- Flow.Launcher.Infrastructure/Hotkey/RegisteredHotkeyData.cs
- Flow.Launcher.Core/Plugin/PluginManager.cs
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: onesounds
PR: Flow-Launcher/Flow.Launcher#3394
File: Flow.Launcher/Themes/Darker Glass.xaml:134-141
Timestamp: 2025-03-28T21:12:13.386Z
Learning: In Flow.Launcher, hotkey styling is implemented with a two-component structure: a Border element with style `ItemHotkeyBGStyle` that provides background and border styling, containing a TextBlock with style `ItemHotkeyStyle` that handles the text styling.
Learnt from: Yusyuriv
PR: Flow-Launcher/Flow.Launcher#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.
Learnt from: Koisu-unavailable
PR: Flow-Launcher/Flow.Launcher#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.
Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs (2)
Learnt from: Yusyuriv
PR: Flow-Launcher/Flow.Launcher#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.
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#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.
Plugins/Flow.Launcher.Plugin.Explorer/Main.cs (9)
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#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.
Learnt from: onesounds
PR: Flow-Launcher/Flow.Launcher#3394
File: Flow.Launcher/Themes/Darker Glass.xaml:134-141
Timestamp: 2025-03-28T21:12:13.386Z
Learning: In Flow.Launcher, hotkey styling is implemented with a two-component structure: a Border element with style `ItemHotkeyBGStyle` that provides background and border styling, containing a TextBlock with style `ItemHotkeyStyle` that handles the text styling.
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#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.
Learnt from: Koisu-unavailable
PR: Flow-Launcher/Flow.Launcher#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.
Learnt from: Yusyuriv
PR: Flow-Launcher/Flow.Launcher#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.
Learnt from: taooceros
PR: Flow-Launcher/Flow.Launcher#2616
File: Flow.Launcher/Flow.Launcher.csproj:7-7
Timestamp: 2024-10-08T15:52:58.573Z
Learning: In the Flow Launcher project, the version number in the `Flow.Launcher.csproj` file is dynamically updated during the CI/CD process.
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3672
File: Flow.Launcher/MainWindow.xaml.cs:318-318
Timestamp: 2025-06-08T14:12:12.842Z
Learning: In Flow.Launcher, the App.NotifyIcon static property is initialized in the App class before MainWindow creation, so null checks are not needed when accessing App.NotifyIcon in MainWindow lifecycle methods.
Learnt from: jjw24
PR: Flow-Launcher/Flow.Launcher#2448
File: Plugins/Flow.Launcher.Plugin.Sys/ThemeSelector.cs:16-20
Timestamp: 2025-01-18T10:10:18.414Z
Learning: In Flow Launcher's plugin system, the PluginInitContext parameter passed to plugin constructors is guaranteed to be non-null by the plugin initialization system, making null checks unnecessary.
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3672
File: Flow.Launcher/MainWindow.xaml.cs:244-247
Timestamp: 2025-06-08T14:12:21.348Z
Learning: In Flow.Launcher, App.NotifyIcon is created before MainWindow creation, so null checks for App.NotifyIcon are not necessary when accessing it from MainWindow code.
Flow.Launcher.Infrastructure/DialogJump/DialogJump.cs (6)
Learnt from: onesounds
PR: Flow-Launcher/Flow.Launcher#3394
File: Flow.Launcher/Themes/Darker Glass.xaml:134-141
Timestamp: 2025-03-28T21:12:13.386Z
Learning: In Flow.Launcher, hotkey styling is implemented with a two-component structure: a Border element with style `ItemHotkeyBGStyle` that provides background and border styling, containing a TextBlock with style `ItemHotkeyStyle` that handles the text styling.
Learnt from: Yusyuriv
PR: Flow-Launcher/Flow.Launcher#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.
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#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.
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3672
File: Flow.Launcher/MainWindow.xaml.cs:244-247
Timestamp: 2025-06-08T14:12:21.348Z
Learning: In Flow.Launcher, App.NotifyIcon is created before MainWindow creation, so null checks for App.NotifyIcon are not necessary when accessing it from MainWindow code.
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3672
File: Flow.Launcher/MainWindow.xaml.cs:318-318
Timestamp: 2025-06-08T14:12:12.842Z
Learning: In Flow.Launcher, the App.NotifyIcon static property is initialized in the App class before MainWindow creation, so null checks are not needed when accessing App.NotifyIcon in MainWindow lifecycle methods.
Learnt from: onesounds
PR: Flow-Launcher/Flow.Launcher#0
File: :0-0
Timestamp: 2025-04-23T15:14:49.986Z
Learning: In WPF applications like Flow.Launcher, font styling should be applied using implicit styles instead of setting the FontFamily property on individual controls. Define implicit styles in a ResourceDictionary using <Style TargetType="{x:Type Button}"> format and merge it into App.xaml, which automatically applies the font to all instances of the control type while still allowing explicit overrides where needed.
Flow.Launcher/Helper/HotKeyMapper.cs (3)
Learnt from: onesounds
PR: Flow-Launcher/Flow.Launcher#3394
File: Flow.Launcher/Themes/Darker Glass.xaml:134-141
Timestamp: 2025-03-28T21:12:13.386Z
Learning: In Flow.Launcher, hotkey styling is implemented with a two-component structure: a Border element with style `ItemHotkeyBGStyle` that provides background and border styling, containing a TextBlock with style `ItemHotkeyStyle` that handles the text styling.
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#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.
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#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.
Flow.Launcher.Infrastructure/UserSettings/Settings.cs (3)
Learnt from: onesounds
PR: Flow-Launcher/Flow.Launcher#3394
File: Flow.Launcher/Themes/Darker Glass.xaml:134-141
Timestamp: 2025-03-28T21:12:13.386Z
Learning: In Flow.Launcher, hotkey styling is implemented with a two-component structure: a Border element with style `ItemHotkeyBGStyle` that provides background and border styling, containing a TextBlock with style `ItemHotkeyStyle` that handles the text styling.
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#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.
Learnt from: Yusyuriv
PR: Flow-Launcher/Flow.Launcher#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.
🪛 GitHub Check: Check Spelling
Plugins/Flow.Launcher.Plugin.Explorer/Main.cs
[failure] 21-21:
Settings Settings matches a line_forbidden.patterns entry: \s([A-Z]{3,}|[A-Z][a-z]{2,}|[a-z]{3,})\s\g{-1}\s. (forbidden-pattern)
🪛 GitHub Actions: Check Spelling
Plugins/Flow.Launcher.Plugin.Explorer/Main.cs
[warning] 138-138: opencontainingfolder is not a recognized word. (unrecognized-spelling)
[warning] 139-139: opencontainingfolder is not a recognized word. (unrecognized-spelling)
[warning] 227-227: opendir is not a recognized word. (unrecognized-spelling)
[error] 21-21: Settings Settings matches a line_forbidden.patterns entry: \s([A-Z]{3,}|[A-Z][a-z]{2,}|[a-z]{3,})\s\g{-1}\s. (forbidden-pattern)
Flow.Launcher.Infrastructure/DialogJump/DialogJump.cs
[warning] 165-165: PInvoke is not a recognized word. (unrecognized-spelling)
[warning] 170-170: PInvoke is not a recognized word. (unrecognized-spelling)
[warning] 175-175: PInvoke is not a recognized word. (unrecognized-spelling)
[warning] 180-180: PInvoke is not a recognized word. (unrecognized-spelling)
[warning] 321-321: Wnd is not a recognized word. (unrecognized-spelling)
[warning] 160-160: PInvoke is not a recognized word. (unrecognized-spelling)
[warning] 314-314: Wnd is not a recognized word. (unrecognized-spelling)
[warning] 700-700: Destory is not a recognized word. (unrecognized-spelling)
Flow.Launcher/Helper/HotKeyMapper.cs
[warning] 432-432: LWin is not a recognized word. (unrecognized-spelling)
[warning] 432-432: RWin is not a recognized word. (unrecognized-spelling)
[warning] 530-530: LWin is not a recognized word. (unrecognized-spelling)
[warning] 530-530: RWin is not a recognized word. (unrecognized-spelling)
[warning] 696-696: nullability is not a recognized word. (unrecognized-spelling)
[warning] 17-17: NHotkey is not a recognized word. (unrecognized-spelling)
[warning] 683-683: nullability 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: gitStream.cm
🔇 Additional comments (23)
Flow.Launcher.Infrastructure/UserSettings/Settings.cs (4)
74-86: LGTM: Hotkey property refactoring follows consistent patternThe conversion from auto-implemented to full properties with backing fields and
OnPropertyChanged()calls enables proper change notification for reactive UI updates. This pattern is consistently applied across all hotkey properties.
671-673: LGTM: Target-typed new expressions improve readabilityThe use of target-typed
new()expressions for collection properties is a good modernization that reduces redundancy while maintaining type safety.
744-744: LGTM: Target-typed new expression for PluginSettingsConsistent with the other collection property updates, this improves readability.
747-747: RegisteredHotkeys population verified in HotKeyMapper
The emptyRegisteredHotkeysinitializer inSettings.csis intentional—all hotkey entries are added centrally inFlow.Launcher/Helper/HotKeyMapper.cs. No further changes are required.Key locations where items are added/removed:
HotKeyMapper.InitializeRegisteredHotkeys()– adds default and editable hotkeysCustomPluginHotkeys_CollectionChangedhandler – handles Add/Remove/Replace for custom plugin hotkeysPluginManager_PluginHotkeyChangedhandler – updates plugin window/global hotkeys- Various
NotifyCollectionChangedActioncases – maintain_settings.RegisteredHotkeysand callSetHotkey/RemoveHotkeyaccordinglyFlow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs (1)
150-161: LGTM: Simplified hotkey management delegationThe removal of direct
HotKeyMapper.SetHotkeyandHotKeyMapper.RemoveHotkeycalls simplifies the setter and properly delegates all dialog jump setup toDialogJump.SetupDialogJump(). This aligns with the architectural shift to centralized hotkey management and command-based hotkey handling.Flow.Launcher.Infrastructure/DialogJump/DialogJump.cs (4)
15-15: LGTM: Added RelayCommand support for command-based hotkey handlingThe addition of
CommunityToolkit.Mvvm.Inputimport enables the new command-based hotkey architecture, replacing the previous event-driven approach.
456-478: LGTM: Well-implemented command-based hotkey systemThe new
DialogJumpCommandproperly implements the command pattern:
- Lazy initialization of the RelayCommand
- Proper guard clause checking
EnableDialogJumpsetting- Asynchronous execution with exception handling
- Clean separation of concerns
This design aligns well with the architectural shift to centralized, command-based hotkey management.
160-180: Pipeline spelling warnings for PInvoke are false positivesThe pipeline flagged "PInvoke" as an unrecognized word, but this is a legitimate API name in the Windows SDK. These warnings can be safely ignored as they represent proper Win32 API usage.
314-321: Pipeline spelling warnings for "Wnd" are false positivesThe pipeline flagged "Wnd" as unrecognized, but this is part of standard Win32 API naming conventions (e.g.,
EnumWindows,hWnd). These warnings can be safely ignored.Plugins/Flow.Launcher.Plugin.Explorer/Main.cs (6)
1-23: LGTM! Clean interface implementation and imports.The addition of the
IPluginHotkeyinterface and supporting imports are well-structured. TheClassNamefield for logging follows good practices.
131-133: LGTM! Correct interface implementation.The method signature correctly implements the
IPluginHotkeyinterface requirement.
135-172: LGTM! Well-implemented open containing folder hotkey.The implementation correctly handles both files and directories with appropriate error handling, logging, and user feedback. The type checking and return values are appropriate.
173-198: LGTM! Proper context menu hotkey implementation.The implementation correctly excludes Volume types and includes appropriate error handling. Returning
falseis correct since showing a context menu shouldn't hide the main window.
199-236: LGTM! Correct run as administrator implementation.The implementation properly handles elevated execution for files and normal opening for directories, with consistent error handling and appropriate return values.
237-272: LGTM! Excellent rename hotkey implementation.The implementation uses the standard F2 hotkey and correctly handles different file system types. The use of
ShowDialog()makes the rename operation modal, which addresses previous feedback and provides better UX by preventing multiple dialogs or conflicting actions.Flow.Launcher/Helper/HotKeyMapper.cs (8)
21-151: LGTM! Well-architected hotkey management system.The initialization is comprehensive and well-structured, covering all hotkey types (system, Flow Launcher, custom, and plugin hotkeys). The event-driven approach and dependency injection usage are appropriate for this complex system.
157-321: LGTM! Comprehensive event handling implementation.The event handlers properly manage hotkey lifecycle changes across all supported scenarios. The logic correctly handles collection changes and plugin hotkey updates with appropriate cleanup and registration.
387-597: LGTM! Robust hotkey registration and removal system.The implementation provides comprehensive error handling, supports multiple hotkey managers (HotkeyManager and ChefKeysManager), and includes proper duplicate prevention. The user feedback on registration failures is excellent.
645-710: LGTM! Well-implemented command pattern with proper state validation.The command implementations correctly validate plugin states, handle result selection, and include appropriate checks for disabled/modified plugins. The logic for invoking plugin hotkey actions is sound.
737-807: LGTM! Excellent backward compatibility approach.The ActionContext support is properly deprecated with clear migration guidance while maintaining existing functionality. The Obsolete attributes with descriptive messages guide developers toward the new
IPluginHotkeyinterface.
603-641: LGTM! Well-designed helper methods for modularity.The helper methods for changing and searching registered hotkeys improve code modularity and maintainability. The CheckAvailability method includes proper cleanup in the finally block.
809-838: LGTM! Appropriate data container classes.The helper classes provide clean encapsulation for command parameters. The Obsolete attribute on the WindowPluginHotkeyPair.Hotkey property correctly aligns with the ActionContext deprecation strategy.
1-840: Excellent comprehensive hotkey management system.This refactor establishes a robust, extensible hotkey management framework that handles system, application, and plugin hotkeys with proper event-driven updates, comprehensive error handling, and thoughtful backward compatibility. The architecture supports future enhancements while maintaining existing functionality.
This comment has been minimized.
This comment has been minimized.
|
Please resolve conflict. |
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (1)
Plugins/Flow.Launcher.Plugin.Explorer/Languages/en.xaml (1)
227-228: Spelling fix acknowledged“occurred” spelling looks correct now. Thanks for addressing the prior feedback.
🧹 Nitpick comments (6)
Plugins/Flow.Launcher.Plugin.Explorer/Languages/en.xaml (2)
140-140: Use Windows-standard casing: "Run as administrator"Align with existing "Run as different user" and Windows UX wording.
- <system:String x:Key="plugin_explorer_run_as_administrator">Run As Administrator</system:String> + <system:String x:Key="plugin_explorer_run_as_administrator">Run as administrator</system:String>
215-226: Polish rename dialog strings for clarity and consistencyTighten grammar, avoid awkward colons, and use consistent punctuation and terminology.
- <system:String x:Key="plugin_explorer_not_a_new_name">The given name: {0} was not new.</system:String> + <system:String x:Key="plugin_explorer_not_a_new_name">The name '{0}' is the same as the current name.</system:String> - <system:String x:Key="plugin_explorer_field_may_not_be_empty">{0} may not be empty.</system:String> + <system:String x:Key="plugin_explorer_field_may_not_be_empty">{0} must not be empty.</system:String> - <system:String x:Key="plugin_explorer_invalid_name">{0} is an invalid name.</system:String> + <system:String x:Key="plugin_explorer_invalid_name">{0} is not a valid name.</system:String> - <system:String x:Key="plugin_explorer_item_not_found">The specified item: {0} was not found</system:String> + <system:String x:Key="plugin_explorer_item_not_found">The specified item '{0}' was not found.</system:String> - <system:String x:Key="plugin_explorer_rename_subtitle">Open a dialog to rename file or folder</system:String> + <system:String x:Key="plugin_explorer_rename_subtitle">Open the rename dialog for the selected file or folder</system:String> - <system:String x:Key="plugin_explorer_cannot_rename">This cannot be renamed.</system:String> + <system:String x:Key="plugin_explorer_cannot_rename">This item cannot be renamed.</system:String> - <system:String x:Key="plugin_explorer_successful_rename">Successfully renamed it to: {0}</system:String> + <system:String x:Key="plugin_explorer_successful_rename">Successfully renamed to '{0}'.</system:String> - <system:String x:Key="plugin_explorer_element_already_exists">There is already a file with the name: {0} in this location</system:String> + <system:String x:Key="plugin_explorer_element_already_exists">An item named '{0}' already exists in this location.</system:String>Optional (matches onesounds’ UX suggestion): add a hint line for Enter/Esc.
+ <system:String x:Key="plugin_explorer_rename_dialog_hint">Press Enter to rename • Esc to cancel</system:String>Flow.Launcher/SettingPages/Views/SettingsPaneHotkey.xaml (2)
238-240: Nit: use uniform key namingElsewhere you use literal symbols ([ and ]). Consider “Ctrl++” instead of “Ctrl+Plus” for consistency, or keep as-is if deliberate.
309-314: Plugin hotkey section: ensure consistent styling and disposal
- Apply the known ItemHotkeyBGStyle/ItemHotkeyStyle styling pattern (per onesounds’ theming) when populating items, to match the rest of the page.
- Ensure dynamic handlers (e.g., ChangePluginHotkey) don’t leak when the page unloads.
Flow.Launcher.Core/Plugin/PluginManager.cs (2)
589-596: InitializePluginHotkeyInfo: consider idempotenceIf InitializePluginsAsync can run more than once in-session (edge cases), guard against duplicate _pluginHotkeyInfo entries.
Apply:
- var hotkeys = ((IPluginHotkey)plugin.Plugin).GetPluginHotkeys(); - _pluginHotkeyInfo.Add(plugin, hotkeys); + var hotkeys = ((IPluginHotkey)plugin.Plugin).GetPluginHotkeys(); + if (!_pluginHotkeyInfo.ContainsKey(plugin)) + _pluginHotkeyInfo.Add(plugin, hotkeys);
622-636: Global ChangePluginHotkey: mirror the hardeningApply the same FirstOrDefault/setting null checks as above for symmetry and robustness.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (13)
Flow.Launcher.Core/Plugin/PluginManager.cs(8 hunks)Flow.Launcher.Core/Resource/Internationalization.cs(1 hunks)Flow.Launcher.Plugin/Result.cs(2 hunks)Flow.Launcher/Languages/en.xaml(1 hunks)Flow.Launcher/MainWindow.xaml(0 hunks)Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs(0 hunks)Flow.Launcher/SettingPages/Views/SettingsPaneHotkey.xaml(5 hunks)Flow.Launcher/ViewModel/MainViewModel.cs(2 hunks)Plugins/Flow.Launcher.Plugin.Explorer/ContextMenu.cs(2 hunks)Plugins/Flow.Launcher.Plugin.Explorer/Languages/en.xaml(2 hunks)Plugins/Flow.Launcher.Plugin.Explorer/Main.cs(3 hunks)Plugins/Flow.Launcher.Plugin.Explorer/Search/ResultManager.cs(6 hunks)Plugins/Flow.Launcher.Plugin.Program/Main.cs(2 hunks)
💤 Files with no reviewable changes (2)
- Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs
- Flow.Launcher/MainWindow.xaml
🚧 Files skipped from review as they are similar to previous changes (7)
- Flow.Launcher/Languages/en.xaml
- Flow.Launcher/ViewModel/MainViewModel.cs
- Flow.Launcher.Core/Resource/Internationalization.cs
- Flow.Launcher.Plugin/Result.cs
- Plugins/Flow.Launcher.Plugin.Explorer/ContextMenu.cs
- Plugins/Flow.Launcher.Plugin.Explorer/Main.cs
- Plugins/Flow.Launcher.Plugin.Explorer/Search/ResultManager.cs
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-07-01T05:46:13.251Z
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#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:
Flow.Launcher.Core/Plugin/PluginManager.cs
📚 Learning: 2025-07-21T09:19:19.012Z
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#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:
Flow.Launcher.Core/Plugin/PluginManager.cs
📚 Learning: 2025-03-28T21:12:13.386Z
Learnt from: onesounds
PR: Flow-Launcher/Flow.Launcher#3394
File: Flow.Launcher/Themes/Darker Glass.xaml:134-141
Timestamp: 2025-03-28T21:12:13.386Z
Learning: In Flow.Launcher, hotkey styling is implemented with a two-component structure: a Border element with style `ItemHotkeyBGStyle` that provides background and border styling, containing a TextBlock with style `ItemHotkeyStyle` that handles the text styling.
Applied to files:
Flow.Launcher/SettingPages/Views/SettingsPaneHotkey.xaml
🧬 Code graph analysis (2)
Plugins/Flow.Launcher.Plugin.Program/Main.cs (3)
Plugins/Flow.Launcher.Plugin.Program/Programs/UWPPackage.cs (1)
List(471-505)Flow.Launcher.Plugin/Interfaces/IPluginHotkey.cs (1)
List(14-14)Plugins/Flow.Launcher.Plugin.Program/Programs/Win32.cs (8)
List(213-272)List(828-849)Win32(296-329)Win32(331-387)Win32(389-427)Win32(429-453)Win32(591-606)Win32(667-722)
Flow.Launcher.Core/Plugin/PluginManager.cs (6)
Flow.Launcher.Plugin/Interfaces/IPluginHotkey.cs (1)
List(14-14)Flow.Launcher.Plugin/PluginHotkey.cs (7)
BasePluginHotkey(11-61)BasePluginHotkey(17-20)SearchWindowPluginHotkey(84-97)SearchWindowPluginHotkey(89-91)GlobalPluginHotkey(66-79)GlobalPluginHotkey(71-73)PluginHotkey(118-134)Flow.Launcher.Plugin/PluginMetadata.cs (2)
PluginMetadata(10-168)ToString(164-167)Flow.Launcher.Infrastructure/UserSettings/PluginSettings.cs (4)
UpdatePluginHotkeyInfo(96-198)Plugin(200-207)Plugin(209-213)Plugin(216-244)Flow.Launcher/SettingPages/Views/SettingsPaneHotkey.xaml.cs (1)
ChangePluginHotkey(106-116)Flow.Launcher/Helper/HotKeyMapper.cs (1)
GlobalPluginHotkey(663-676)
🔇 Additional comments (9)
Plugins/Flow.Launcher.Plugin.Program/Main.cs (1)
18-18: Interface adoption looks goodImplementing IPluginHotkey here is consistent with the new hotkey architecture.
Flow.Launcher/SettingPages/Views/SettingsPaneHotkey.xaml (4)
134-141: History hotkey change LGTMSwitch to “OpenHistoryHotkey” with Ctrl+H looks consistent.
152-157: Reload plugin hotkey as non-editable F5Reasonable choice. Please verify this doesn’t collide with any in-window refresh actions.
159-169: New first/last result presets LGTMClear, non-conflicting keys.
207-223: Requery/GameMode/CopyPath presets LGTMStatic displays are fine given recent preset policy.
Flow.Launcher.Core/Plugin/PluginManager.cs (4)
289-292: Order and initialization LGTMHotkey info init → settings sync → window mapping is a sane sequence.
556-561: Return a defensive copy LGTMReturning a copy of _windowPluginHotkeys avoids external mutation.
973-994: Event payload class LGTMShape and fields are sufficient.
556-561: HotkeyModel correctly implements value equality for dictionary keys – it implements IEquatable.Equals(CharKey, ModifierKeys) and overrides GetHashCode (via HashCode.Combine(ModifierKeys, CharKey)), and Dictionary<TKey, TValue>’s default comparer uses IEquatable.Equals. No changes needed.
Context
Windows system has a default context menu for doing some operations to one specific object like exe file, pdf file, etc. And some operations can have some hotkeys like ctrl-c for copy, etc. Since Flow provides many results like them, we should add context menu for them.
Flow designed context menu is good, but currently, for hotkey event binding in context menu, Flow introduces action context (
ActionContextparameter inActionandAsyncActioninResult class) like ctrl, alt, win key with the enter hotkey event (this is the key to open the action in one result) which means Flow need to regard ctrl+enter key event as enter event so that ctrl key will be passed with enter key.This can lead many possible problems:
Considering these issues, I would like to deprecate the action context API and introduce
IPluginHotkeyinterface which can help plugins to register any hotkeys for their results. Additionally, plugins can return results with their supported hotkeys. And we need to find a workaround to ensure the compatibility for action context and I think we can fully deprecate it in future.In future, if plugins can register their hotkeys and specific supported hotkeys for their results. And we can look forward to a command bar like what does as Raycast. It will tell users what commands are available for the selected result. And it looks very nice for me, and I really enjoy this design.
Changes:
IPluginHotkeyto register global & search window hotkeys for one plugins.Hotkey registration refactor for both global hotkeys and main window hotkeys (KeyBinding): Now we are using call back function to register / unregister global and main window hotkeys which can improve code quality (Put all codes related to hotkey in
HotkeyMapper.cs)Redesign welcome page 3: Removed Ctrl+Enter & Ctrl+Shift+Enter hotkeys since it will be deprated API and added more preset hotkeys.
Resolve #1614.
TODOS:
HotKeyMapper.Initialize();(Asynchronous Loading & Initialization Plugin Model to Improve Window Startup Speed #3854).Future:
Test: