-
-
Notifications
You must be signed in to change notification settings - Fork 445
Support removing duplicated programs #3259
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
Support removing duplicated programs #3259
Conversation
This comment has been minimized.
This comment has been minimized.
🥷 Code experts: VictoriousRaptor, onesounds Jack251970, taooceros 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: To learn more about /:\ gitStream - Visit our Docs |
📝 WalkthroughWalkthroughThe changes introduce a feature to hide duplicated Windows applications in the Flow Launcher program plugin. This includes the addition of new string resources for user interface labels and tooltips, a constant and filtering method in the main logic to detect duplicates, and a boolean property in the settings to control this feature. The user interface is also updated with a new checkbox and adjusted margin formatting to accommodate the new functionality. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as ProgramSetting UI
participant Settings
participant Main
User->>UI: Toggle "Hide duplicated apps" CheckBox
UI->>Settings: Update HideDuplicatedWindowsApp property
Settings->>UI: Reflect new settings value
UI->>Main: Call ResetCache()
Main->>Main: Filter programs using HideDuplicatedWindowsAppFilter
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
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: 6
🧹 Nitpick comments (2)
Plugins/Flow.Launcher.Plugin.Program/Main.cs (1)
171-171
: Fix comment in code.The comment "Ensure trailing slash" is incorrect as the code actually removes the trailing slash.
Apply this diff to fix the comment:
- var location = program.Location.TrimEnd('\\'); // Ensure trailing slash + var location = program.Location.TrimEnd('\\'); // Remove trailing slashPlugins/Flow.Launcher.Plugin.Program/Views/ProgramSetting.xaml.cs (1)
60-68
: Fix typo in property name: "Dulplicated" should be "Duplicated".The implementation follows the correct pattern for visibility settings, but there's a typo in the property name.
Apply this diff to fix the typo:
- public bool HideDulplicatedWindowsApp + public bool HideDuplicatedWindowsApp
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
Plugins/Flow.Launcher.Plugin.Program/Languages/en.xaml
(1 hunks)Plugins/Flow.Launcher.Plugin.Program/Main.cs
(3 hunks)Plugins/Flow.Launcher.Plugin.Program/Settings.cs
(1 hunks)Plugins/Flow.Launcher.Plugin.Program/Views/ProgramSetting.xaml
(8 hunks)Plugins/Flow.Launcher.Plugin.Program/Views/ProgramSetting.xaml.cs
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: gitStream.cm
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: 0
🧹 Nitpick comments (2)
Plugins/Flow.Launcher.Plugin.Program/Main.cs (2)
95-101
: Extract UWP directories collection into a separate method.Consider extracting this logic into a dedicated method for better maintainability and reusability. This would also make the
QueryAsync
method more focused and easier to read.+ private static string[] GetUwpDirectories(UWPApp[] uwps, bool hideDuplicates) + { + if (!hideDuplicates) + return null; + + return uwps + .Where(uwp => !string.IsNullOrEmpty(uwp.Location)) + .Where(uwp => uwp.Location.StartsWith(WindowsAppPath, StringComparison.OrdinalIgnoreCase)) + .Select(uwp => uwp.Location.TrimEnd('\\')) + .Distinct(StringComparer.OrdinalIgnoreCase) + .ToArray(); + } public async Task<List<Result>> QueryAsync(Query query, CancellationToken token) { var result = await cache.GetOrCreateAsync(query.Search, async entry => { var resultList = await Task.Run(() => { try { - // Collect all UWP Windows app directories - var uwpsDirectories = _settings.HideDuplicatedWindowsApp ? _uwps - .Where(uwp => !string.IsNullOrEmpty(uwp.Location)) // Exclude invalid paths - .Where(uwp => uwp.Location.StartsWith(WindowsAppPath, StringComparison.OrdinalIgnoreCase)) // Keep system apps - .Select(uwp => uwp.Location.TrimEnd('\\')) // Remove trailing slash - .Distinct(StringComparer.OrdinalIgnoreCase) - .ToArray() : null; + var uwpsDirectories = GetUwpDirectories(_uwps, _settings.HideDuplicatedWindowsApp);
171-171
: Fix misleading comments.The comments need correction:
- Line 171: The comment "Ensure trailing slash" is incorrect as the code is removing the trailing slash.
- Line 178: The comment has grammatical issues.
- var location = program.Location.TrimEnd('\\'); // Ensure trailing slash + var location = program.Location.TrimEnd('\\'); // Remove trailing slash - // Check if the any Win32 executable directory contains UWP Windows app location matches + // Check if any Win32 executable directory matches a UWP Windows app location return !uwpsDirectories.Any(uwpDirectory => location.StartsWith(uwpDirectory, StringComparison.OrdinalIgnoreCase));Also applies to: 178-180
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
Plugins/Flow.Launcher.Plugin.Program/Languages/en.xaml
(1 hunks)Plugins/Flow.Launcher.Plugin.Program/Main.cs
(3 hunks)Plugins/Flow.Launcher.Plugin.Program/Settings.cs
(1 hunks)Plugins/Flow.Launcher.Plugin.Program/Views/ProgramSetting.xaml
(8 hunks)Plugins/Flow.Launcher.Plugin.Program/Views/ProgramSetting.xaml.cs
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- Plugins/Flow.Launcher.Plugin.Program/Settings.cs
- Plugins/Flow.Launcher.Plugin.Program/Views/ProgramSetting.xaml.cs
- Plugins/Flow.Launcher.Plugin.Program/Views/ProgramSetting.xaml
- Plugins/Flow.Launcher.Plugin.Program/Languages/en.xaml
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
🔇 Additional comments (1)
Plugins/Flow.Launcher.Plugin.Program/Main.cs (1)
75-75
: Use system environment for Windows app path.The hardcoded path might not work on all systems. Consider using
Environment.GetFolderPath
or similar system APIs to get the correct path.
@check-spelling-bot Report🔴 Please reviewSee the 📂 files view, the 📜action log, or 📝 job summary for details.
See ❌ Event descriptions for more information. Pattern suggestions ✂️ (1)You could add these patterns to
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: 1
🧹 Nitpick comments (2)
Plugins/Flow.Launcher.Plugin.Program/Main.cs (2)
95-102
: Improve readability of the LINQ chain.While the logic is correct, the LINQ chain could be more readable by:
- Extracting the filtering conditions into descriptive variables
- Adding more detailed comments explaining the purpose of each filter
Consider this refactoring:
- // Collect all UWP Windows app directories + // Collect unique UWP Windows app directories for duplicate detection + // Only consider valid system apps from the WindowsApps directory var uwpsDirectories = _settings.HideDuplicatedWindowsApp ? _uwps - .Where(uwp => !string.IsNullOrEmpty(uwp.Location)) // Exclude invalid paths - .Where(uwp => uwp.Location.StartsWith(WindowsAppPath, StringComparison.OrdinalIgnoreCase)) // Keep system apps - .Select(uwp => uwp.Location.TrimEnd('\\')) // Remove trailing slash + .Where(uwp => !string.IsNullOrEmpty(uwp.Location)) + .Where(uwp => + uwp.Location.StartsWith(WindowsAppPath, StringComparison.OrdinalIgnoreCase)) + .Select(uwp => uwp.Location.TrimEnd('\\')) .Distinct(StringComparer.OrdinalIgnoreCase) .ToArray() : null;
334-347
: Improve error handling in StartProcess method.The catch block catches all exceptions but only shows a generic error message. Consider:
- Logging the specific exception
- Providing more detailed error information to help diagnose issues
public static void StartProcess(Func<ProcessStartInfo, Process> runProcess, ProcessStartInfo info) { try { runProcess(info); } - catch (Exception) + catch (Exception ex) { + Log.Exception("Failed to start process", ex, typeof(Main)); var title = Context.API.GetTranslation("flowlauncher_plugin_program_disable_dlgtitle_error"); var message = string.Format(Context.API.GetTranslation("flowlauncher_plugin_program_run_failed"), info.FileName); - Context.API.ShowMsg(title, string.Format(message, info.FileName), string.Empty); + Context.API.ShowMsg(title, string.Format(message, info.FileName), ex.Message); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Plugins/Flow.Launcher.Plugin.Program/Main.cs
(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
🔇 Additional comments (1)
Plugins/Flow.Launcher.Plugin.Program/Main.cs (1)
75-75
: LGTM! Robust path handling implementation.The use of
Path.Combine
withEnvironment.GetFolderPath
ensures correct path handling across different Windows installations.
Support removing duplicated Win32 apps in Program plugin
For Program plugin, we can get results with duplicated apps. (One is Win32 app, another is UWP app.)
So I add option to let us remove the duplicated apps.
I remove the Win32 apps instead of UWP apps because the name, description and some other properties of Win32 apps are always wrong if these apps are duplicated.
As you can see above, the top item is from Win32 app and its name is
olk
but actually this name should beOutlook (Classic)
(same as bottom item which is from UWP app).Test
Settings Panel:
Search: