Use Flow.Launcher.Localization to improve code quality #3997
Conversation
|
🥷 Code experts: jjw24 Jack251970, VictoriousRaptor 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:
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: ✨ Comment |
|
Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX. |
📝 WalkthroughWalkthroughDependency version bumps for Flow.Launcher.Localization across three plugins. Explorer plugin migrates all user-facing strings from Context.API.GetTranslation to Localize helpers, adds a new en.xaml key, and makes one method static. Explorer settings remove a public property, adjust collection initializations, and change ActionKeywordSetting constructor usage/signature. Changes
Sequence Diagram(s)Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Plugins/Flow.Launcher.Plugin.Explorer/ContextMenu.cs (1)
463-476: Quote rundll32 arguments to handle paths with spacesOpen With... fails on paths containing spaces. Quote shell32.dll and the target path.
- Process.Start("rundll32.exe", $"{Path.Combine(Environment.SystemDirectory, "shell32.dll")},OpenAs_RunDLL {record.FullPath}"); + var shell32 = Path.Combine(Environment.SystemDirectory, "shell32.dll"); + Process.Start("rundll32.exe", $"\"{shell32}\",OpenAs_RunDLL \"{record.FullPath}\"");Plugins/Flow.Launcher.Plugin.Explorer/Views/PreviewPanel.xaml.cs (1)
74-79: Raise property changes on UI threadOnPropertyChanged is invoked from a background Task; WPF bindings may not update reliably. Dispatch back to UI thread.
- _ = Task.Run(() => - { - FileSize = GetFolderSize(filePath); - OnPropertyChanged(nameof(FileSize)); - }).ConfigureAwait(false); + _ = Task.Run(() => GetFolderSize(filePath)) + .ContinueWith(t => + { + FileSize = t.Result; + OnPropertyChanged(nameof(FileSize)); + }, TaskScheduler.FromCurrentSynchronizationContext());
🧹 Nitpick comments (9)
Plugins/Flow.Launcher.Plugin.Explorer/Views/QuickAccessLinkSettings.xaml.cs (2)
98-103: Message says “folder” even when a file is selected.
This dialog supports files; the warning should be path-agnostic.Update the resource text to be neutral:
- <system:String x:Key="plugin_explorer_quick_access_link_no_folder_selected">Please select a folder path.</system:String> + <system:String x:Key="plugin_explorer_quick_access_link_no_folder_selected">Please select a path.</system:String>
110-113: “folder path” wording doesn’t fit file links.
Prefer “name or path” so it applies to both files and folders.- <system:String x:Key="plugin_explorer_quick_access_link_path_already_exists">Please choose a different name or folder path.</system:String> + <system:String x:Key="plugin_explorer_quick_access_link_path_already_exists">Please choose a different name or path.</system:String>Plugins/Flow.Launcher.Plugin.Explorer/Languages/en.xaml (2)
27-27: Tighten microcopy for action keyword collision.
Drop “new” and end with a period.- <system:String x:Key="plugin_explorer_new_action_keyword_assigned">This new action keyword is already assigned to another plugin, please choose a different one</system:String> + <system:String x:Key="plugin_explorer_new_action_keyword_assigned">This action keyword is already assigned to another plugin. Please choose a different one.</system:String>
8-9: Make quick access messages path-agnostic.
Matches the file/folder capability of the dialog.- <system:String x:Key="plugin_explorer_quick_access_link_no_folder_selected">Please select a folder path.</system:String> + <system:String x:Key="plugin_explorer_quick_access_link_no_folder_selected">Please select a path.</system:String> - <system:String x:Key="plugin_explorer_quick_access_link_path_already_exists">Please choose a different name or folder path.</system:String> + <system:String x:Key="plugin_explorer_quick_access_link_path_already_exists">Please choose a different name or path.</system:String>Plugins/Flow.Launcher.Plugin.Explorer/Settings.cs (1)
17-20: Collection expressions for ObservableCollection — likely fine; confirm C# version.
[] requires the modern collection expression support; net9 defaults to C# 13, so this should compile. If targeting older compilers in some environments, fallback is:- public ObservableCollection<AccessLink> QuickAccessLinks { get; set; } = []; + public ObservableCollection<AccessLink> QuickAccessLinks { get; set; } = new(); - public ObservableCollection<AccessLink> IndexSearchExcludedSubdirectoryPaths { get; set; } = []; + public ObservableCollection<AccessLink> IndexSearchExcludedSubdirectoryPaths { get; set; } = new();Plugins/Flow.Launcher.Plugin.Explorer/Views/ActionKeywordSetting.xaml.cs (1)
47-49: Treat whitespace-only input as emptyUse IsNullOrWhiteSpace so " " becomes the wildcard too.
- if (string.IsNullOrEmpty(ActionKeyword)) + if (string.IsNullOrWhiteSpace(ActionKeyword)) ActionKeyword = Query.GlobalPluginWildcardSign;Plugins/Flow.Launcher.Plugin.Explorer/Views/PreviewPanel.xaml.cs (1)
308-329: Future dates edge caseIf fileDateTime is in the future, DaysAgo/MonthsAgo/YearsAgo can be negative. Clamp at 0 or return “Today”.
- var difference = now - fileDateTime; + var difference = now - fileDateTime; + if (difference.TotalSeconds < 0) + return Localize.Today();Plugins/Flow.Launcher.Plugin.Explorer/Search/ResultManager.cs (1)
242-255: Readable size units: tiny polishIf you want, consider “KiB/MiB” vs “KB/MB” or add “PB”. Not blocking.
Plugins/Flow.Launcher.Plugin.Explorer/ViewModels/SettingsViewModel.cs (1)
379-394: Null check is redundantIndexSearchExcludedSubdirectoryPaths is initialized; the null guard can be dropped. Low priority.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Flow.Launcher.Plugin.BrowserBookmark.csproj(1 hunks)Plugins/Flow.Launcher.Plugin.Calculator/Flow.Launcher.Plugin.Calculator.csproj(1 hunks)Plugins/Flow.Launcher.Plugin.Explorer/ContextMenu.cs(23 hunks)Plugins/Flow.Launcher.Plugin.Explorer/Flow.Launcher.Plugin.Explorer.csproj(1 hunks)Plugins/Flow.Launcher.Plugin.Explorer/Languages/en.xaml(1 hunks)Plugins/Flow.Launcher.Plugin.Explorer/Main.cs(1 hunks)Plugins/Flow.Launcher.Plugin.Explorer/Search/Everything/EverythingDownloadHelper.cs(2 hunks)Plugins/Flow.Launcher.Plugin.Explorer/Search/Everything/EverythingSearchManager.cs(5 hunks)Plugins/Flow.Launcher.Plugin.Explorer/Search/ResultManager.cs(11 hunks)Plugins/Flow.Launcher.Plugin.Explorer/Search/SearchManager.cs(1 hunks)Plugins/Flow.Launcher.Plugin.Explorer/Search/WindowsIndex/WindowsIndexSearchManager.cs(1 hunks)Plugins/Flow.Launcher.Plugin.Explorer/Settings.cs(2 hunks)Plugins/Flow.Launcher.Plugin.Explorer/ViewModels/SettingsViewModel.cs(4 hunks)Plugins/Flow.Launcher.Plugin.Explorer/Views/ActionKeywordSetting.xaml.cs(2 hunks)Plugins/Flow.Launcher.Plugin.Explorer/Views/PreviewPanel.xaml.cs(9 hunks)Plugins/Flow.Launcher.Plugin.Explorer/Views/QuickAccessLinkSettings.xaml.cs(2 hunks)
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2024-10-08T15:52:58.573Z
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.
Applied to files:
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Flow.Launcher.Plugin.BrowserBookmark.csprojPlugins/Flow.Launcher.Plugin.Explorer/Flow.Launcher.Plugin.Explorer.csprojPlugins/Flow.Launcher.Plugin.Calculator/Flow.Launcher.Plugin.Calculator.csproj
📚 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:
Plugins/Flow.Launcher.Plugin.Explorer/ContextMenu.csPlugins/Flow.Launcher.Plugin.Explorer/Views/PreviewPanel.xaml.cs
📚 Learning: 2025-06-24T19:06:48.344Z
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.
Applied to files:
Plugins/Flow.Launcher.Plugin.Explorer/ContextMenu.csPlugins/Flow.Launcher.Plugin.Explorer/Search/ResultManager.cs
📚 Learning: 2025-09-04T11:52:29.096Z
Learnt from: jjw24
PR: Flow-Launcher/Flow.Launcher#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/Flow.Launcher.Plugin.Explorer.csprojPlugins/Flow.Launcher.Plugin.Calculator/Flow.Launcher.Plugin.Calculator.csproj
📚 Learning: 2025-07-06T12:21:37.947Z
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.
Applied to files:
Plugins/Flow.Launcher.Plugin.Explorer/Search/WindowsIndex/WindowsIndexSearchManager.cs
📚 Learning: 2025-07-21T09:19:49.684Z
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#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/WindowsIndex/WindowsIndexSearchManager.cs
🧬 Code graph analysis (6)
Plugins/Flow.Launcher.Plugin.Explorer/ContextMenu.cs (2)
Plugins/Flow.Launcher.Plugin.Explorer/Settings.cs (1)
Settings(13-205)Plugins/Flow.Launcher.Plugin.Explorer/Search/ResultManager.cs (7)
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/Search/Everything/EverythingSearchManager.cs (1)
Plugins/Flow.Launcher.Plugin.Explorer/Main.cs (1)
Main(18-131)
Plugins/Flow.Launcher.Plugin.Explorer/Views/ActionKeywordSetting.xaml.cs (3)
Plugins/Flow.Launcher.Plugin.Explorer/ViewModels/ActionKeywordModel.cs (2)
ActionKeywordModel(5-49)ActionKeywordModel(14-18)Plugins/Flow.Launcher.Plugin.Explorer/Main.cs (1)
Main(18-131)Plugins/Flow.Launcher.Plugin.Explorer/Settings.cs (1)
Settings(13-205)
Plugins/Flow.Launcher.Plugin.Explorer/Views/PreviewPanel.xaml.cs (2)
Plugins/Flow.Launcher.Plugin.Explorer/Main.cs (1)
Main(18-131)Plugins/Flow.Launcher.Plugin.Explorer/ContextMenu.cs (1)
LogException(479-482)
Plugins/Flow.Launcher.Plugin.Explorer/ViewModels/SettingsViewModel.cs (1)
Plugins/Flow.Launcher.Plugin.Explorer/Views/ActionKeywordSetting.xaml.cs (1)
ActionKeywordSetting(34-43)
Plugins/Flow.Launcher.Plugin.Explorer/Settings.cs (1)
Plugins/Flow.Launcher.Plugin.Explorer/Search/QuickAccessLinks/AccessLink.cs (1)
AccessLink(3-10)
🪛 GitHub Check: Check Spelling
Plugins/Flow.Launcher.Plugin.Explorer/ContextMenu.cs
[warning] 70-70:
quickaccess is not a recognized word. (unrecognized-spelling)
[warning] 87-87:
titletooltip is not a recognized word. (unrecognized-spelling)
[warning] 87-87:
contextmenu is not a recognized word. (unrecognized-spelling)
[warning] 81-81:
addfilefoldersuccess is not a recognized word. (unrecognized-spelling)
[warning] 109-109:
contextmenu is not a recognized word. (unrecognized-spelling)
[warning] 108-108:
titletooltip is not a recognized word. (unrecognized-spelling)
[warning] 108-108:
contextmenu is not a recognized word. (unrecognized-spelling)
[warning] 103-103:
removefilefoldersuccess is not a recognized word. (unrecognized-spelling)
[warning] 97-97:
quickaccess is not a recognized word. (unrecognized-spelling)
[warning] 96-96:
quickaccess 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). (4)
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
- GitHub Check: build
🔇 Additional comments (16)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Flow.Launcher.Plugin.BrowserBookmark.csproj (1)
105-108: Aligned Localization bump — LGTM.
No further action from this file.Plugins/Flow.Launcher.Plugin.Explorer/Settings.cs (1)
71-74: AI summary inconsistency: PreviewPanelDateFormat still exists.
The summary mentions removal, but the property is present.Plugins/Flow.Launcher.Plugin.Explorer/Search/SearchManager.cs (1)
164-166: Localized Everything content-search prompt — LGTM.
Keys exist; behavior unchanged.Plugins/Flow.Launcher.Plugin.Calculator/Flow.Launcher.Plugin.Calculator.csproj (1)
65-67: Approve — Flow.Launcher.Localization bumped to 0.0.6; references are consistent.Confirmed PackageReference entries in:
- Plugins/Flow.Launcher.Plugin.Explorer/Flow.Launcher.Plugin.Explorer.csproj (line 51)
- Plugins/Flow.Launcher.Plugin.BrowserBookmark/Flow.Launcher.Plugin.BrowserBookmark.csproj (line 107)
- Plugins/Flow.Launcher.Plugin.Calculator/Flow.Launcher.Plugin.Calculator.csproj (line 66)
Run
dotnet restore(or CI) to confirm package restore succeeds.Plugins/Flow.Launcher.Plugin.Explorer/Views/ActionKeywordSetting.xaml.cs (2)
34-43: Constructor signature simplification looks goodRemoving IPublicAPI from the ctor keeps the view self-contained. Call sites were updated.
68-77: Confirm Localize keys exist and parameter counts matchPlease verify keys plugin_explorer_globalActionKeywordInvalid, plugin_explorer_quickaccess_globalActionKeywordInvalid, and plugin_explorer_new_action_keyword_assigned exist and accept the provided args (none).
Would you like a quick repo scan script to check for missing Localize members?
Plugins/Flow.Launcher.Plugin.Explorer/Search/Everything/EverythingDownloadHelper.cs (2)
23-27: Dialog text migration LGTMSwapping to Localize.* for content/title aligns with the PR goal.
44-51: Subtitle/title keys: verify placeholdersEnsure flowlauncher_plugin_everything_installing_subtitle and flowlauncher_plugin_everything_installationsuccess_subtitle expect zero args; otherwise, mismatched placeholders will surface at runtime.
Plugins/Flow.Launcher.Plugin.Explorer/ContextMenu.cs (1)
69-83: Localization refactor reads wellAll user-facing strings now use Localize.*, including tooltips and confirmations. Static CreateOpenWithMenu is appropriate.
Consider running a build with a non-English culture to sanity-check formatting and placeholders at these call sites.
Also applies to: 96-110, 117-130, 139-157, 161-179, 184-207, 227-239, 245-265, 314-333, 338-339, 371-396, 403-418, 434-455
Plugins/Flow.Launcher.Plugin.Explorer/Search/Everything/EverythingSearchManager.cs (3)
27-34: Good: EngineNotAvailableException now uses Localize messagesNo behavior change; clearer strings.
37-42: Constructor overload ordering: please confirmDifferent overloads are used here vs. Line 27 (4 vs 5 args). Verify parameter order semantics (message vs. resolution) are still correct for UI.
If helpful, I can grep the constructors and validate usage.
53-69: Error paths migrated to LocalizeNotFound/install-issue/content-search messages look consistent.
Also applies to: 99-102
Plugins/Flow.Launcher.Plugin.Explorer/Views/PreviewPanel.xaml.cs (1)
28-28: Localization swap is consistent across all tooltip/labelsUnknown/error strings and age labels now centralize through Localize.
Also applies to: 115-125, 146-156, 177-187, 209-243, 264-305, 315-329
Plugins/Flow.Launcher.Plugin.Explorer/Search/ResultManager.cs (1)
127-173: Localize migration throughout Result creation is solidError messages, tooltips, and disk space strings are consistently localized.
Also applies to: 191-197, 265-283, 333-341, 374-405
Plugins/Flow.Launcher.Plugin.Explorer/ViewModels/SettingsViewModel.cs (2)
299-306: Ctor callsite updated correctlynew ActionKeywordSetting(actionKeyword) matches the simplified ctor.
435-448: Delete confirmations migrated to LocalizeOK. Verify the keys accept zero args.
Plugins/Flow.Launcher.Plugin.Explorer/Flow.Launcher.Plugin.Explorer.csproj
Show resolved
Hide resolved
Plugins/Flow.Launcher.Plugin.Explorer/Search/WindowsIndex/WindowsIndexSearchManager.cs
Show resolved
Hide resolved
…r_localization Use Flow.Launcher.Localization to improve code quality
Use Flow.Launcher.Localization to improve code quality
Use
Localizeclass fromFlow.Launcher.Localizationto improve code quality. No feature updates.Test
The solution is compiled successfully.