Fix localized name search for Program plugin #4195
Merged
+23
−39
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
🥷 Code experts: jjw24, Jack251970 jjw24, Jack251970 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. |
Contributor
📝 WalkthroughWalkthroughReplaces previous resource-based localization with Windows Shell COM usage: code now calls Changes
Sequence Diagram(s)sequenceDiagram
participant App as FlowLauncher (C#)
participant PInvoke as NativeMethods (P/Invoke)
participant Shell as Windows Shell (COM)
participant OS as Kernel/COM-Malloc
Note over App,PInvoke: Request localized display name for path
App->>PInvoke: SHCreateItemFromParsingName(path, ...)
alt success
PInvoke->>Shell: returns IShellItem*
App->>Shell: IShellItem->GetDisplayName(SIGDN_NORMALDISPLAY)
Shell-->>OS: allocate display name (CoTaskMemAlloc)
Shell-->>App: returns pointer to wide string
App->>OS: CoTaskMemFree(ptr)
App->>Shell: Release IShellItem
Shell-->>App: success/display name string
else failure
PInvoke-->>App: HRESULT failure
App-->>App: return empty string
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Plugins/Flow.Launcher.Plugin.Program/Programs/ShellLocalization.cs (1)
22-33: Consider releasing the COM object deterministically.The
IShellItemCOM object should be released after use to avoid holding native resources until garbage collection. While not strictly a bug, deterministic cleanup is a best practice for COM interop.🔎 Proposed fix with COM object release
+using System.Runtime.InteropServices;int retCode = PInvoke.SHCreateItemFromParsingName(path, null, typeof(Windows.Win32.UI.Shell.IShellItem).GUID, out object shellItemObj); if (retCode != 0 || shellItemObj is not Windows.Win32.UI.Shell.IShellItem shellItem) { return string.Empty; } - PWSTR displayName; - shellItem.GetDisplayName(Windows.Win32.UI.Shell.SIGDN.SIGDN_NORMALDISPLAY, &displayName); - string filename = displayName.ToString(); - PInvoke.CoTaskMemFree(displayName); - - return filename; + try + { + PWSTR displayName; + shellItem.GetDisplayName(Windows.Win32.UI.Shell.SIGDN.SIGDN_NORMALDISPLAY, &displayName); + string filename = displayName.ToString(); + PInvoke.CoTaskMemFree(displayName); + return filename; + } + finally + { + Marshal.ReleaseComObject(shellItem); + }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Plugins/Flow.Launcher.Plugin.Program/NativeMethods.txtPlugins/Flow.Launcher.Plugin.Program/Programs/ShellLocalization.cs
🧰 Additional context used
🧠 Learnings (12)
📓 Common learnings
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3593
File: Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml.cs:143-165
Timestamp: 2025-09-13T06:04:26.977Z
Learning: In Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml, the ListView_SizeChanged handler in the C# file assumes 5 GridViewColumns but the XAML may have more columns. The user Jack251970 indicated this mismatch doesn't need to be fixed when pointed out during PR #3593 review.
📚 Learning: 2025-07-20T07:28:28.092Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3850
File: Flow.Launcher.Core/Resource/Internationalization.cs:0-0
Timestamp: 2025-07-20T07:28:28.092Z
Learning: In Flow Launcher's Internationalization class, when the Flow Launcher language directory or default language file is missing, the only viable approach is to log an error and return early - there are no fallback mechanisms or alternative recovery strategies available due to architectural constraints.
Applied to files:
Plugins/Flow.Launcher.Plugin.Program/Programs/ShellLocalization.cs
📚 Learning: 2025-07-01T05:46:13.251Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 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.Program/Programs/ShellLocalization.cs
📚 Learning: 2025-06-24T19:06:48.344Z
Learnt from: Koisu-unavailable
Repo: Flow-Launcher/Flow.Launcher PR: 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.Program/Programs/ShellLocalization.cs
📚 Learning: 2025-10-16T10:48:30.573Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3854
File: Flow.Launcher/App.xaml.cs:252-275
Timestamp: 2025-10-16T10:48:30.573Z
Learning: In Flow Launcher's App.xaml.cs, the plugin initialization block (lines 252-275) that includes PreStartPluginExecutablePathUpdate, PluginManager.LoadPlugins, PluginManager.InitializePluginsAsync, _mainVM.QueryResults(), and API.SaveAppAllSettings() does not require additional Task.Run wrappers or Dispatcher.InvokeAsync calls according to maintainer Jack251970, as the threading model is already safe as designed.
Applied to files:
Plugins/Flow.Launcher.Plugin.Program/Programs/ShellLocalization.cs
📚 Learning: 2025-07-21T09:19:49.684Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 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.Program/Programs/ShellLocalization.cs
📚 Learning: 2025-09-04T11:52:29.096Z
Learnt from: jjw24
Repo: Flow-Launcher/Flow.Launcher PR: 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.Program/Programs/ShellLocalization.cs
📚 Learning: 2025-07-21T09:19:19.012Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 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:
Plugins/Flow.Launcher.Plugin.Program/Programs/ShellLocalization.cs
📚 Learning: 2025-06-08T14:12:21.348Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 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.
Applied to files:
Plugins/Flow.Launcher.Plugin.Program/Programs/ShellLocalization.cs
📚 Learning: 2025-07-06T12:21:37.947Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 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.Program/Programs/ShellLocalization.cs
📚 Learning: 2025-06-08T14:12:12.842Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 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.
Applied to files:
Plugins/Flow.Launcher.Plugin.Program/Programs/ShellLocalization.cs
📚 Learning: 2025-09-06T05:32:51.575Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3573
File: Plugins/Flow.Launcher.Plugin.Shell/Main.cs:330-339
Timestamp: 2025-09-06T05:32:51.575Z
Learning: In Flow.Launcher Shell plugin's StartProcess method, the maintainer Jack251970 prefers not to propagate launch failures from Context.API.StartProcess or throw exceptions when the API call returns false. Silent failure handling is intentional for shell commands in this plugin.
Applied to files:
Plugins/Flow.Launcher.Plugin.Program/Programs/ShellLocalization.cs
🪛 GitHub Check: Check Spelling
Plugins/Flow.Launcher.Plugin.Program/Programs/ShellLocalization.cs
[warning] 31-31:
PInvoke is not a recognized word. (unrecognized-spelling)
[warning] 22-22:
PInvoke is not a recognized word. (unrecognized-spelling)
[warning] 31-31:
PInvoke 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). (2)
- GitHub Check: gitStream.cm
- GitHub Check: build
🔇 Additional comments (1)
Plugins/Flow.Launcher.Plugin.Program/NativeMethods.txt (1)
10-13: LGTM!The native method declarations are correctly added to support the new
IShellItem-based localization approach. These entries will generate the necessary P/Invoke wrappers via CsWin32.
Plugins/Flow.Launcher.Plugin.Program/Programs/ShellLocalization.cs
Outdated
Show resolved
Hide resolved
Jack251970
previously approved these changes
Jan 1, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
Plugins/Flow.Launcher.Plugin.Program/Programs/ShellLocalization.cs (1)
31-35: Missing error check forGetDisplayNamecall.The
GetDisplayNamemethod returns anHRESULTthat must be checked. If it fails,displayNameremains uninitialized, leading to undefined behavior inToString()andCoTaskMemFree.🔎 Proposed fix with error handling
PWSTR displayName; -shellItem.GetDisplayName(Windows.Win32.UI.Shell.SIGDN.SIGDN_NORMALDISPLAY, &displayName); +HRESULT hr = shellItem.GetDisplayName(Windows.Win32.UI.Shell.SIGDN.SIGDN_NORMALDISPLAY, &displayName); +if (hr.Failed) +{ + return string.Empty; +} + string filename = displayName.ToString(); PInvoke.CoTaskMemFree(displayName); return filename;
🧹 Nitpick comments (1)
Plugins/Flow.Launcher.Plugin.Program/Programs/ShellLocalization.cs (1)
37-40: Consider logging exceptions or catching specific exception types.The empty catch block silently swallows all exceptions, which can make debugging difficult if localization fails unexpectedly. While returning
string.Emptyis a reasonable fallback for this best-effort utility, consider logging the exception or catching specific expected exception types (e.g.,COMException,InvalidCastException).🔎 Example with logging
-catch +catch (Exception ex) { + // Log exception for debugging purposes + // e.g., Logger.Exception($"Failed to get localized name for {path}", ex, GetType()); return string.Empty; }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Plugins/Flow.Launcher.Plugin.Program/Programs/ShellLocalization.cs
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3593
File: Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml.cs:143-165
Timestamp: 2025-09-13T06:04:26.977Z
Learning: In Plugins/Flow.Launcher.Plugin.WebSearch/SettingsControl.xaml, the ListView_SizeChanged handler in the C# file assumes 5 GridViewColumns but the XAML may have more columns. The user Jack251970 indicated this mismatch doesn't need to be fixed when pointed out during PR #3593 review.
📚 Learning: 2025-07-20T07:28:28.092Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3850
File: Flow.Launcher.Core/Resource/Internationalization.cs:0-0
Timestamp: 2025-07-20T07:28:28.092Z
Learning: In Flow Launcher's Internationalization class, when the Flow Launcher language directory or default language file is missing, the only viable approach is to log an error and return early - there are no fallback mechanisms or alternative recovery strategies available due to architectural constraints.
Applied to files:
Plugins/Flow.Launcher.Plugin.Program/Programs/ShellLocalization.cs
📚 Learning: 2025-07-21T09:19:49.684Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 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.Program/Programs/ShellLocalization.cs
📚 Learning: 2025-08-13T06:12:43.382Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3897
File: Flow.Launcher/ViewModel/PluginViewModel.cs:46-51
Timestamp: 2025-08-13T06:12:43.382Z
Learning: In Flow Launcher's PluginViewModel.cs, the LoadIconAsync method does not require additional try-catch error handling according to maintainer Jack251970, as the existing error handling approach is considered sufficient for the image loading operations.
Applied to files:
Plugins/Flow.Launcher.Plugin.Program/Programs/ShellLocalization.cs
📚 Learning: 2025-07-01T05:46:13.251Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 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.Program/Programs/ShellLocalization.cs
📚 Learning: 2025-09-28T03:59:59.693Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3770
File: Plugins/Flow.Launcher.Plugin.Explorer/Views/RenameFile.xaml.cs:75-80
Timestamp: 2025-09-28T03:59:59.693Z
Learning: In Flow.Launcher's Explorer plugin rename dialog (RenameFile.xaml.cs), the dialog should close unconditionally after calling RenameThing.Rename(), even on validation failures, because RenameThing.Rename() displays error messages via popup notifications. This is the intended UX design.
Applied to files:
Plugins/Flow.Launcher.Plugin.Program/Programs/ShellLocalization.cs
📚 Learning: 2025-10-16T10:48:30.573Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3854
File: Flow.Launcher/App.xaml.cs:252-275
Timestamp: 2025-10-16T10:48:30.573Z
Learning: In Flow Launcher's App.xaml.cs, the plugin initialization block (lines 252-275) that includes PreStartPluginExecutablePathUpdate, PluginManager.LoadPlugins, PluginManager.InitializePluginsAsync, _mainVM.QueryResults(), and API.SaveAppAllSettings() does not require additional Task.Run wrappers or Dispatcher.InvokeAsync calls according to maintainer Jack251970, as the threading model is already safe as designed.
Applied to files:
Plugins/Flow.Launcher.Plugin.Program/Programs/ShellLocalization.cs
📚 Learning: 2025-07-06T12:21:37.947Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 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.Program/Programs/ShellLocalization.cs
📚 Learning: 2025-09-06T05:32:51.575Z
Learnt from: Jack251970
Repo: Flow-Launcher/Flow.Launcher PR: 3573
File: Plugins/Flow.Launcher.Plugin.Shell/Main.cs:330-339
Timestamp: 2025-09-06T05:32:51.575Z
Learning: In Flow.Launcher Shell plugin's StartProcess method, the maintainer Jack251970 prefers not to propagate launch failures from Context.API.StartProcess or throw exceptions when the API call returns false. Silent failure handling is intentional for shell commands in this plugin.
Applied to files:
Plugins/Flow.Launcher.Plugin.Program/Programs/ShellLocalization.cs
📚 Learning: 2025-09-04T11:52:29.096Z
Learnt from: jjw24
Repo: Flow-Launcher/Flow.Launcher PR: 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.Program/Programs/ShellLocalization.cs
🪛 GitHub Check: Check Spelling
Plugins/Flow.Launcher.Plugin.Program/Programs/ShellLocalization.cs
[warning] 34-34:
PInvoke is not a recognized word. (unrecognized-spelling)
[warning] 23-23:
PInvoke is not a recognized word. (unrecognized-spelling)
[warning] 34-34:
PInvoke 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). (2)
- GitHub Check: gitStream.cm
- GitHub Check: build
🔇 Additional comments (2)
Plugins/Flow.Launcher.Plugin.Program/Programs/ShellLocalization.cs (2)
23-27: LGTM: Proper error handling for SHCreateItemFromParsingName.The error check correctly validates both the return code and the output type before proceeding, with a safe fallback to
string.Empty.
41-44: LGTM: Proper COM cleanup in finally block.The finally block correctly ensures the COM object is released regardless of whether the operation succeeds or fails.
Jack251970
approved these changes
Jan 2, 2026
TBM13
pushed a commit
to TBM13/Flow.Launcher
that referenced
this pull request
Feb 8, 2026
…d-name Fix localized name search for Program plugin
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #4056.