fix: design polish — Bulk Installer, Context Menu, elevation badges#500
Merged
Conversation
📝 WalkthroughWalkthroughThis PR extends the application with three independent feature enhancements: context menu entries now resolve and display friendly names with optional system-entry filtering; the bulk installer adds grouped view layout and integrated winget package search with descriptions; and both Privacy and Windows Features views gain admin elevation relaunch commands with corresponding UI strips. ChangesContext Menu Friendly-Name Resolution and System Entry Filtering
Bulk Installer Grouping and Winget Search
Privacy View Admin Elevation
Windows Features View Admin Elevation
Possibly Related PRs
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
SysManager/SysManager/Services/ContextMenuService.cs (1)
62-64:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse the registry subkey name as
RawNameand forIsSystemEntry.
RawNameis intended to be the original registry key name, but at Line 80 it is populated fromdisplayName(which can come from the default value). This also affectsIsSystemEntryat Line 86 and can hide/show the wrong entries.Suggested fix
- // Read display name: prefer (Default) value, fall back to key name - var displayName = entryKey.GetValue("")?.ToString(); - if (string.IsNullOrWhiteSpace(displayName)) - displayName = entryName; + // Keep raw key name for filtering/tooltips; use default value only for display seed + var rawName = entryName; + var defaultDisplayName = entryKey.GetValue("")?.ToString(); + var displaySeed = string.IsNullOrWhiteSpace(defaultDisplayName) + ? rawName + : defaultDisplayName; @@ - var friendlyName = GetFriendlyName(displayName, command); + var friendlyName = GetFriendlyName(displaySeed, command); @@ - RawName = displayName, + RawName = rawName, @@ - IsSystemEntry = IsSystemEntry(displayName) + IsSystemEntry = IsSystemEntry(rawName)Also applies to: 79-87
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@SysManager/SysManager/Services/ContextMenuService.cs` around lines 62 - 64, The code currently assigns displayName from entryKey.GetValue("") and then uses that value to populate RawName and to decide IsSystemEntry, which can be wrong; instead keep displayName as the friendly name (from entryKey.GetValue("") falling back to entryName), but set RawName to the registry subkey name (entryName) and compute IsSystemEntry based on entryName (the original key name) rather than displayName so system entries are detected and toggled correctly (update references where RawName and IsSystemEntry are set to use entryName instead of displayName).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@SysManager/SysManager/Services/ContextMenuService.cs`:
- Around line 388-395: The current parsing trims quotes then cuts at the first
space which can turn quoted paths like "C:\Program Files\app.exe" into
"C:\Program"; update the parsing of command -> path to honor quoted paths: if
command starts with a quote, extract the content between the opening and
matching closing quote (use that as path) before any space-splitting; otherwise
keep the existing percent/space trimming but only split on the first space for
unquoted commands and still verify File.Exists(path). Refer to the local
variables 'command' and 'path' and the File.Exists check when making this
change.
In `@SysManager/SysManager/ViewModels/BulkInstallerViewModel.cs`:
- Around line 205-206: The current code in BulkInstallerViewModel uses
proc.StandardOutput.ReadToEnd which blocks and makes the subsequent
proc.WaitForExit(30_000) ineffective; change this to non-blocking async I/O and
enforce a real timeout: use proc.StandardOutput.ReadToEndAsync() or
BeginOutputReadLine/OutputDataReceived with Task-based completion, then use a
Task.WhenAny with proc.WaitForExitAsync() (or a Task.Delay(30_000)) to detect
timeout, and if the timeout elapses call proc.Kill()/Dispose() and fail the
search task so the UI can leave the searching state; replace references to
StandardOutput.ReadToEnd and WaitForExit(30_000) with this async timeout+kill
flow and ensure proper cleanup and error logging.
- Around line 226-231: The code in BulkInstallerViewModel that determines column
offsets by inspecting the header string (variables header, idStart, versionStart
and the header.IndexOf("Id"/"Version") logic) is fragile and locale-dependent;
replace this table-header parsing with a locale-invariant, structured WinGet
interface (for example use the WinGet COM API or the official PowerShell
cmdlets) to request search results as structured objects and read the package
Id/Version fields directly instead of parsing the human-readable table; update
the search/execute path in the BulkInstallerViewModel to call the COM/Powershell
API and map the returned object's Id and Version properties rather than relying
on idStart/versionStart string offsets.
In `@SysManager/SysManager/ViewModels/PrivacyViewModel.cs`:
- Around line 92-96: RelaunchElevated currently silently does nothing when
AdminHelper.RelaunchAsAdmin() returns false; update RelaunchElevated to detect
the false return and surface a failure message (e.g., set a ViewModel status
property, call the app logger, or display a MessageBox) so users know the
elevation was cancelled/blocked; modify the method around the
AdminHelper.RelaunchAsAdmin() call to log/assign a clear status like "Elevation
cancelled or blocked (UAC declined)" when false, while keeping the existing
Shutdown() path when true.
---
Outside diff comments:
In `@SysManager/SysManager/Services/ContextMenuService.cs`:
- Around line 62-64: The code currently assigns displayName from
entryKey.GetValue("") and then uses that value to populate RawName and to decide
IsSystemEntry, which can be wrong; instead keep displayName as the friendly name
(from entryKey.GetValue("") falling back to entryName), but set RawName to the
registry subkey name (entryName) and compute IsSystemEntry based on entryName
(the original key name) rather than displayName so system entries are detected
and toggled correctly (update references where RawName and IsSystemEntry are set
to use entryName instead of displayName).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: fe8d6e10-4468-445f-9e5f-8ef1cf971766
📒 Files selected for processing (11)
SysManager/SysManager/Models/ContextMenuEntry.csSysManager/SysManager/Models/InstallableApp.csSysManager/SysManager/Services/ContextMenuService.csSysManager/SysManager/ViewModels/BulkInstallerViewModel.csSysManager/SysManager/ViewModels/ContextMenuViewModel.csSysManager/SysManager/ViewModels/PrivacyViewModel.csSysManager/SysManager/ViewModels/WindowsFeaturesViewModel.csSysManager/SysManager/Views/BulkInstallerView.xamlSysManager/SysManager/Views/ContextMenuView.xamlSysManager/SysManager/Views/PrivacyView.xamlSysManager/SysManager/Views/WindowsFeaturesView.xaml
📜 Review details
⏰ 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: Build & unit tests
- GitHub Check: Analyze (csharp)
🔇 Additional comments (7)
SysManager/SysManager/Views/PrivacyView.xaml (1)
14-19: LGTM!Also applies to: 51-65, 68-68, 119-119
SysManager/SysManager/ViewModels/WindowsFeaturesViewModel.cs (1)
43-48: LGTM!SysManager/SysManager/Views/WindowsFeaturesView.xaml (1)
17-17: LGTM!Also applies to: 57-71, 74-74, 79-79, 185-185
SysManager/SysManager/Models/ContextMenuEntry.cs (1)
34-38: LGTM!SysManager/SysManager/Services/ContextMenuService.cs (1)
8-8: LGTM!Also applies to: 246-378, 411-435
SysManager/SysManager/ViewModels/ContextMenuViewModel.cs (1)
28-28: LGTM!Also applies to: 47-47, 120-136
SysManager/SysManager/Views/ContextMenuView.xaml (1)
23-24: LGTM!Also applies to: 50-53, 87-108
Comment on lines
+388
to
+395
| var path = command.Trim('"', ' '); | ||
| var percentIdx = path.IndexOf('%'); | ||
| if (percentIdx > 0) path = path[..percentIdx].Trim(); | ||
|
|
||
| var spaceIdx = path.IndexOf(' '); | ||
| if (spaceIdx > 0 && !File.Exists(path)) | ||
| path = path[..spaceIdx].Trim('"'); | ||
|
|
There was a problem hiding this comment.
Quoted command parsing can truncate paths like C:\Program Files\....
At Line 393/394, splitting on first space after a broad trim can reduce quoted paths to C:\Program, which breaks executable inference for common commands.
Suggested fix
- // Try to extract executable name
- var path = command.Trim('"', ' ');
- var percentIdx = path.IndexOf('%');
- if (percentIdx > 0) path = path[..percentIdx].Trim();
-
- var spaceIdx = path.IndexOf(' ');
- if (spaceIdx > 0 && !File.Exists(path))
- path = path[..spaceIdx].Trim('"');
+ // Extract executable token safely (quoted or unquoted)
+ var m = Regex.Match(
+ command,
+ "^\\s*\"([^\"]+\\.exe)\"|^\\s*([^\\s]+\\.exe)",
+ RegexOptions.IgnoreCase);
+ if (!m.Success) return "";
+ var path = m.Groups[1].Success ? m.Groups[1].Value : m.Groups[2].Value;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@SysManager/SysManager/Services/ContextMenuService.cs` around lines 388 - 395,
The current parsing trims quotes then cuts at the first space which can turn
quoted paths like "C:\Program Files\app.exe" into "C:\Program"; update the
parsing of command -> path to honor quoted paths: if command starts with a
quote, extract the content between the opening and matching closing quote (use
that as path) before any space-splitting; otherwise keep the existing
percent/space trimming but only split on the first space for unquoted commands
and still verify File.Exists(path). Refer to the local variables 'command' and
'path' and the File.Exists check when making this change.
Comment on lines
+205
to
+206
| var output = proc.StandardOutput.ReadToEnd(); | ||
| proc.WaitForExit(30_000); |
There was a problem hiding this comment.
Make the winget timeout real.
Line 205 blocks until winget exits, so Line 206 never meaningfully enforces the 30s limit. If the process stalls, the search task never completes and the UI stays in the searching state indefinitely. Switch this path to async process I/O with an actual timeout/kill branch before awaiting the full output.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@SysManager/SysManager/ViewModels/BulkInstallerViewModel.cs` around lines 205
- 206, The current code in BulkInstallerViewModel uses
proc.StandardOutput.ReadToEnd which blocks and makes the subsequent
proc.WaitForExit(30_000) ineffective; change this to non-blocking async I/O and
enforce a real timeout: use proc.StandardOutput.ReadToEndAsync() or
BeginOutputReadLine/OutputDataReceived with Task-based completion, then use a
Task.WhenAny with proc.WaitForExitAsync() (or a Task.Delay(30_000)) to detect
timeout, and if the timeout elapses call proc.Kill()/Dispose() and fail the
search task so the UI can leave the searching state; replace references to
StandardOutput.ReadToEnd and WaitForExit(30_000) with this async timeout+kill
flow and ensure proper cleanup and error logging.
Comment on lines
+226
to
+231
| // Determine column positions from the header line | ||
| var header = lines[separatorIndex - 1]; | ||
| var idStart = header.IndexOf("Id", StringComparison.Ordinal); | ||
| var versionStart = header.IndexOf("Version", StringComparison.Ordinal); | ||
|
|
||
| if (idStart < 0 || versionStart < 0) return results; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In current winget documentation, does winget search expose a machine-readable or locale-invariant output format, and are the default table headers/localized display columns considered stable for parsing?
💡 Result:
In current WinGet documentation, winget search is presented as a human-oriented table output with localized UI text; the docs do not describe any machine-readable, locale-invariant output mode for the winget search command itself (e.g., no documented --json/--csv-style output flag for winget search). The official winget search command reference focuses on query/filter options (like --id, --name, --moniker, --tag, --command, --source, --count, --exact) rather than output formatting. [1] Because there is no documented machine-readable output mode, you should assume winget search’s default table output is not stable for parsing across locales/terminal environments: even WinGet’s table headers/columns are user-visible strings and WinGet’s CLI UI text is designed to be localized. A WinGet CLI issue discussion highlights parsing difficulties attributed to formatting/localization concerns, and suggests using COM APIs / PowerShell cmdlets for structured output rather than parsing native CLI text. [2] Additionally, WinGet CLI discussions/issues explicitly call out that native CLI output is not intended to be machine-consumed and that adding a machine-parseable format (CSV/JSON/XML) has been requested for search/show/list. [3] Bottom line: 1) Machine-readable, locale-invariant output for winget search (as a documented option) is not provided in the current winget search docs. [1] 2) The default table headers / localized display columns should not be considered stable for parsing, because the CLI output is table-oriented and is subject to localization/formatting changes, and maintainers/users recommend structured alternatives (PowerShell module / COM APIs) for automation rather than parsing the table output. [2][3]
Citations:
- 1: https://learn.microsoft.com/en-us/windows/package-manager/winget/search
- 2: How can I parse "winget list" output without trimmed IDs microsoft/winget-cli#4090
- 3: Provide Structured Data Format Options microsoft/winget-cli#1753
Avoid parsing winget search display table headers; use structured output.
In SysManager/SysManager/ViewModels/BulkInstallerViewModel.cs (lines 226-231), the code locates column offsets by searching the human-readable "Id"/"Version" header text (header.IndexOf("Id"), header.IndexOf("Version")). WinGet’s winget search documentation does not provide a documented machine-readable/locale-invariant output mode, and WinGet discussions/issues note the native table output is localized and not intended for automation—so this parsing can fail silently across locales/format changes. Switch to WinGet’s structured interfaces (e.g., COM API / PowerShell cmdlets) or another locale-invariant contract before relying on parsed IDs for search/install results.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@SysManager/SysManager/ViewModels/BulkInstallerViewModel.cs` around lines 226
- 231, The code in BulkInstallerViewModel that determines column offsets by
inspecting the header string (variables header, idStart, versionStart and the
header.IndexOf("Id"/"Version") logic) is fragile and locale-dependent; replace
this table-header parsing with a locale-invariant, structured WinGet interface
(for example use the WinGet COM API or the official PowerShell cmdlets) to
request search results as structured objects and read the package Id/Version
fields directly instead of parsing the human-readable table; update the
search/execute path in the BulkInstallerViewModel to call the COM/Powershell API
and map the returned object's Id and Version properties rather than relying on
idStart/versionStart string offsets.
Comment on lines
+92
to
+96
| private void RelaunchElevated() | ||
| { | ||
| if (AdminHelper.RelaunchAsAdmin()) | ||
| System.Windows.Application.Current?.Shutdown(); | ||
| } |
There was a problem hiding this comment.
Add failure feedback when elevation relaunch does not start.
If AdminHelper.RelaunchAsAdmin() returns false, the click is silent. Add a status/log message so users know the relaunch was blocked/cancelled (e.g., UAC declined).
Suggested patch
[RelayCommand]
private void RelaunchElevated()
{
- if (AdminHelper.RelaunchAsAdmin())
- System.Windows.Application.Current?.Shutdown();
+ if (AdminHelper.RelaunchAsAdmin())
+ {
+ System.Windows.Application.Current?.Shutdown();
+ return;
+ }
+
+ StatusMessage = "Could not relaunch as administrator.";
+ Log.Warning("Privacy: relaunch as administrator was not started");
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@SysManager/SysManager/ViewModels/PrivacyViewModel.cs` around lines 92 - 96,
RelaunchElevated currently silently does nothing when
AdminHelper.RelaunchAsAdmin() returns false; update RelaunchElevated to detect
the false return and surface a failure message (e.g., set a ViewModel status
property, call the app logger, or display a MessageBox) so users know the
elevation was cancelled/blocked; modify the method around the
AdminHelper.RelaunchAsAdmin() call to log/assign a clear status like "Elevation
cancelled or blocked (UAC declined)" when false, while keeping the existing
Shutdown() path when true.
laurentiu021
force-pushed
the
fix/design-polish-batch
branch
from
fb6ecf2 to
a6abd42
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
SysManager/SysManager.Tests/BulkInstallerViewModelTests.cs (1)
20-100: ⚡ Quick winAdd focused tests for new search/add behaviors.
Please add coverage for
AddToInstallListduplicate prevention andSearchWingetAsyncstate transitions (IsSearching, result replacement, and failure status path). Those are key new paths and currently unverified here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@SysManager/SysManager.Tests/BulkInstallerViewModelTests.cs` around lines 20 - 100, Add unit tests to cover duplicate prevention in AddToInstallList and SearchWingetAsync state transitions: add tests that call AddToInstallList twice with the same AppViewModel and assert the InstallList (or whichever collection is used by the VM) contains only one entry (no duplicates) and that IsSelected/IsInInstallList flags are set appropriately; and add tests for SearchWingetAsync that assert IsSearching is true while the async search runs, that a successful search replaces the previous FilteredApps (or search result collection) with the new results, and that a failing search sets the SearchFailed/IsSearchFailed flag and clears or leaves results as intended. Target the methods/properties AddToInstallList, SearchWingetAsync, IsSearching, FilteredApps (or SearchResults), InstallList (or the VM's install collection), and SearchFailed so the tests exercise duplicate prevention, in-progress state, success replacement, and failure paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@SysManager/SysManager/ViewModels/BulkInstallerViewModel.cs`:
- Around line 272-345: The BuildCuratedApps method returns 46 InstallableApp
entries but the PR objective/tests expect 47; update BuildCuratedApps to either
add the missing InstallableApp entry (create a new new() { Name = "...",
WingetId = "...", Category = "...", Description = "..." } in the returned list)
so the list contains 47 items, or adjust the PR objective/tests to expect 46;
locate the list inside the BuildCuratedApps function and modify it accordingly,
ensuring the InstallableApp objects remain correctly formed.
- Around line 163-165: The early return in BulkInstallerViewModel (the block
using SearchQuery, IsSearching, and SearchResults) skips clearing previous
results, so move or add SearchResults.Clear() (and set IsSearching = false if
appropriate) before the return when SearchQuery is null/whitespace or shorter
than 2; ensure the logic in the search-start guard always clears stale
SearchResults and leaves view state consistent.
In `@SysManager/SysManager/ViewModels/WindowsFeaturesViewModel.cs`:
- Around line 43-48: The RelaunchElevated method only shuts down the app on
success and provides no user feedback when AdminHelper.RelaunchAsAdmin() returns
false; update RelaunchElevated to detect the false return and display a clear
user-facing notification (e.g., MessageBox/notification via the VM's messaging
service) indicating that elevation was cancelled or blocked and any recommended
next steps; keep the existing shutdown behavior when RelaunchAsAdmin() returns
true and reference the RelaunchElevated method and AdminHelper.RelaunchAsAdmin()
when making the change.
---
Nitpick comments:
In `@SysManager/SysManager.Tests/BulkInstallerViewModelTests.cs`:
- Around line 20-100: Add unit tests to cover duplicate prevention in
AddToInstallList and SearchWingetAsync state transitions: add tests that call
AddToInstallList twice with the same AppViewModel and assert the InstallList (or
whichever collection is used by the VM) contains only one entry (no duplicates)
and that IsSelected/IsInInstallList flags are set appropriately; and add tests
for SearchWingetAsync that assert IsSearching is true while the async search
runs, that a successful search replaces the previous FilteredApps (or search
result collection) with the new results, and that a failing search sets the
SearchFailed/IsSearchFailed flag and clears or leaves results as intended.
Target the methods/properties AddToInstallList, SearchWingetAsync, IsSearching,
FilteredApps (or SearchResults), InstallList (or the VM's install collection),
and SearchFailed so the tests exercise duplicate prevention, in-progress state,
success replacement, and failure paths.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 97229025-c4a5-4cfa-a820-aab6c8f98adb
📒 Files selected for processing (12)
SysManager/SysManager.Tests/BulkInstallerViewModelTests.csSysManager/SysManager/Models/ContextMenuEntry.csSysManager/SysManager/Models/InstallableApp.csSysManager/SysManager/Services/ContextMenuService.csSysManager/SysManager/ViewModels/BulkInstallerViewModel.csSysManager/SysManager/ViewModels/ContextMenuViewModel.csSysManager/SysManager/ViewModels/PrivacyViewModel.csSysManager/SysManager/ViewModels/WindowsFeaturesViewModel.csSysManager/SysManager/Views/BulkInstallerView.xamlSysManager/SysManager/Views/ContextMenuView.xamlSysManager/SysManager/Views/PrivacyView.xamlSysManager/SysManager/Views/WindowsFeaturesView.xaml
✅ Files skipped from review due to trivial changes (1)
- SysManager/SysManager/Views/ContextMenuView.xaml
📜 Review details
⏰ 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: Build & unit tests
- GitHub Check: Analyze (csharp)
🔇 Additional comments (15)
SysManager/SysManager/Services/ContextMenuService.cs (6)
388-395: The quoted command parsing issue at these lines was already flagged in a previous review.
74-87: LGTM!
246-303: LGTM!
309-359: LGTM!
364-378: LGTM!
411-435: LGTM!SysManager/SysManager/Models/ContextMenuEntry.cs (1)
33-38: LGTM!SysManager/SysManager/ViewModels/ContextMenuViewModel.cs (1)
28-28: LGTM!Also applies to: 47-47, 120-122, 135-136
SysManager/SysManager/ViewModels/PrivacyViewModel.cs (1)
91-96: Silent failure path in relaunch command remains.Line 94 still has no user feedback when elevation relaunch returns
false; this was already flagged in prior review comments.SysManager/SysManager/Views/PrivacyView.xaml (1)
14-19: LGTM!Also applies to: 51-66, 68-69, 119-119
SysManager/SysManager/Views/WindowsFeaturesView.xaml (1)
17-20: LGTM!Also applies to: 57-72, 74-75, 79-80, 185-185
SysManager/SysManager/ViewModels/BulkInstallerViewModel.cs (2)
205-206: Previously reported timeout issue still present.The blocking output read still makes the timeout ineffective in practice. This was already flagged earlier; keeping this as a duplicate marker for tracking.
226-231: Previously reported locale-dependent parser is still present.Header-index parsing still depends on human-readable
winget searchtable text. This was already flagged earlier; marking as duplicate.SysManager/SysManager/Models/InstallableApp.cs (1)
20-20: LGTM!SysManager/SysManager/Views/BulkInstallerView.xaml (1)
14-19: LGTM!Also applies to: 57-121, 123-198, 201-201
Comment on lines
+163
to
+165
| if (string.IsNullOrWhiteSpace(SearchQuery) || SearchQuery.Length < 2) return; | ||
| IsSearching = true; | ||
| SearchResults.Clear(); |
There was a problem hiding this comment.
Clear stale search results when query is too short.
Line 163 returns before clearing previous results, so old matches can remain visible for an invalid/short query.
Suggested fix
private async Task SearchWingetAsync()
{
- if (string.IsNullOrWhiteSpace(SearchQuery) || SearchQuery.Length < 2) return;
+ if (string.IsNullOrWhiteSpace(SearchQuery) || SearchQuery.Length < 2)
+ {
+ SearchResults.Clear();
+ StatusMessage = "Enter at least 2 characters to search.";
+ return;
+ }
IsSearching = true;
SearchResults.Clear();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (string.IsNullOrWhiteSpace(SearchQuery) || SearchQuery.Length < 2) return; | |
| IsSearching = true; | |
| SearchResults.Clear(); | |
| if (string.IsNullOrWhiteSpace(SearchQuery) || SearchQuery.Length < 2) | |
| { | |
| SearchResults.Clear(); | |
| StatusMessage = "Enter at least 2 characters to search."; | |
| return; | |
| } | |
| IsSearching = true; | |
| SearchResults.Clear(); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@SysManager/SysManager/ViewModels/BulkInstallerViewModel.cs` around lines 163
- 165, The early return in BulkInstallerViewModel (the block using SearchQuery,
IsSearching, and SearchResults) skips clearing previous results, so move or add
SearchResults.Clear() (and set IsSearching = false if appropriate) before the
return when SearchQuery is null/whitespace or shorter than 2; ensure the logic
in the search-start guard always clears stale SearchResults and leaves view
state consistent.
Comment on lines
272
to
345
| private static List<InstallableApp> BuildCuratedApps() => | ||
| [ | ||
| // Browsers | ||
| new() { Name = "Google Chrome", WingetId = "Google.Chrome", Category = "Browsers" }, | ||
| new() { Name = "Firefox", WingetId = "Mozilla.Firefox", Category = "Browsers" }, | ||
| new() { Name = "Brave", WingetId = "Brave.Brave", Category = "Browsers" }, | ||
| new() { Name = "Vivaldi", WingetId = "Vivaldi.Vivaldi", Category = "Browsers" }, | ||
| new() { Name = "Google Chrome", WingetId = "Google.Chrome", Category = "Browsers", Description = "Fast, secure web browser by Google" }, | ||
| new() { Name = "Firefox", WingetId = "Mozilla.Firefox", Category = "Browsers", Description = "Privacy-focused open-source web browser" }, | ||
| new() { Name = "Brave", WingetId = "Brave.Brave", Category = "Browsers", Description = "Privacy-first browser with built-in ad blocker" }, | ||
| new() { Name = "Vivaldi", WingetId = "Vivaldi.Vivaldi", Category = "Browsers", Description = "Highly customizable browser for power users" }, | ||
|
|
||
| // Communication | ||
| new() { Name = "Discord", WingetId = "Discord.Discord", Category = "Communication" }, | ||
| new() { Name = "Slack", WingetId = "SlackTechnologies.Slack", Category = "Communication" }, | ||
| new() { Name = "Zoom", WingetId = "Zoom.Zoom", Category = "Communication" }, | ||
| new() { Name = "Telegram", WingetId = "Telegram.TelegramDesktop", Category = "Communication" }, | ||
| new() { Name = "Discord", WingetId = "Discord.Discord", Category = "Communication", Description = "Voice, video, and text chat for communities" }, | ||
| new() { Name = "Slack", WingetId = "SlackTechnologies.Slack", Category = "Communication", Description = "Team messaging and collaboration platform" }, | ||
| new() { Name = "Zoom", WingetId = "Zoom.Zoom", Category = "Communication", Description = "Video conferencing and online meetings" }, | ||
| new() { Name = "Telegram", WingetId = "Telegram.TelegramDesktop", Category = "Communication", Description = "Fast, secure cloud-based messaging" }, | ||
|
|
||
| // Media | ||
| new() { Name = "VLC", WingetId = "VideoLAN.VLC", Category = "Media" }, | ||
| new() { Name = "Spotify", WingetId = "Spotify.Spotify", Category = "Media" }, | ||
| new() { Name = "foobar2000", WingetId = "PeterPawlowski.foobar2000", Category = "Media" }, | ||
| new() { Name = "VLC", WingetId = "VideoLAN.VLC", Category = "Media", Description = "Free multimedia player supporting all formats" }, | ||
| new() { Name = "Spotify", WingetId = "Spotify.Spotify", Category = "Media", Description = "Music streaming with millions of songs" }, | ||
| new() { Name = "foobar2000", WingetId = "PeterPawlowski.foobar2000", Category = "Media", Description = "Lightweight advanced audio player" }, | ||
|
|
||
| // Development | ||
| new() { Name = "VS Code", WingetId = "Microsoft.VisualStudioCode", Category = "Development" }, | ||
| new() { Name = "Git", WingetId = "Git.Git", Category = "Development" }, | ||
| new() { Name = "Node.js", WingetId = "OpenJS.NodeJS.LTS", Category = "Development" }, | ||
| new() { Name = "Python", WingetId = "Python.Python.3.12", Category = "Development" }, | ||
| new() { Name = "VS Code", WingetId = "Microsoft.VisualStudioCode", Category = "Development", Description = "Lightweight code editor with extensions" }, | ||
| new() { Name = "Git", WingetId = "Git.Git", Category = "Development", Description = "Distributed version control system" }, | ||
| new() { Name = "Node.js", WingetId = "OpenJS.NodeJS.LTS", Category = "Development", Description = "JavaScript runtime for server-side apps" }, | ||
| new() { Name = "Python", WingetId = "Python.Python.3.12", Category = "Development", Description = "General-purpose programming language" }, | ||
|
|
||
| // Utilities | ||
| new() { Name = "7-Zip", WingetId = "7zip.7zip", Category = "Utilities" }, | ||
| new() { Name = "Notepad++", WingetId = "Notepad++.Notepad++", Category = "Utilities" }, | ||
| new() { Name = "Everything", WingetId = "voidtools.Everything", Category = "Utilities" }, | ||
| new() { Name = "PowerToys", WingetId = "Microsoft.PowerToys", Category = "Utilities" }, | ||
| new() { Name = "7-Zip", WingetId = "7zip.7zip", Category = "Utilities", Description = "Free file archiver with high compression" }, | ||
| new() { Name = "Notepad++", WingetId = "Notepad++.Notepad++", Category = "Utilities", Description = "Powerful text and source code editor" }, | ||
| new() { Name = "Everything", WingetId = "voidtools.Everything", Category = "Utilities", Description = "Instant file search for Windows" }, | ||
| new() { Name = "PowerToys", WingetId = "Microsoft.PowerToys", Category = "Utilities", Description = "Microsoft utilities for power users" }, | ||
|
|
||
| // Gaming | ||
| new() { Name = "Steam", WingetId = "Valve.Steam", Category = "Gaming" }, | ||
| new() { Name = "Epic Games", WingetId = "EpicGames.EpicGamesLauncher", Category = "Gaming" }, | ||
| new() { Name = "GOG Galaxy", WingetId = "GOG.Galaxy", Category = "Gaming" }, | ||
| new() { Name = "Steam", WingetId = "Valve.Steam", Category = "Gaming", Description = "Gaming platform and digital store" }, | ||
| new() { Name = "Epic Games", WingetId = "EpicGames.EpicGamesLauncher", Category = "Gaming", Description = "Game store and launcher" }, | ||
| new() { Name = "GOG Galaxy", WingetId = "GOG.Galaxy", Category = "Gaming", Description = "DRM-free gaming platform" }, | ||
|
|
||
| // Security | ||
| new() { Name = "Bitwarden", WingetId = "Bitwarden.Bitwarden", Category = "Security" }, | ||
| new() { Name = "Malwarebytes", WingetId = "Malwarebytes.Malwarebytes", Category = "Security" }, | ||
| new() { Name = "Bitwarden", WingetId = "Bitwarden.Bitwarden", Category = "Security", Description = "Open-source password manager" }, | ||
| new() { Name = "Malwarebytes", WingetId = "Malwarebytes.Malwarebytes", Category = "Security", Description = "Anti-malware protection and scanning" }, | ||
|
|
||
| // Office & Productivity | ||
| new() { Name = "LibreOffice", WingetId = "TheDocumentFoundation.LibreOffice", Category = "Office & Productivity", Description = "Free open-source office suite" }, | ||
| new() { Name = "Obsidian", WingetId = "Obsidian.Obsidian", Category = "Office & Productivity", Description = "Knowledge base with Markdown notes" }, | ||
| new() { Name = "Notion", WingetId = "Notion.Notion", Category = "Office & Productivity", Description = "All-in-one workspace for notes and docs" }, | ||
| new() { Name = "Adobe Acrobat Reader", WingetId = "Adobe.Acrobat.Reader.64-bit", Category = "Office & Productivity", Description = "View, print, and annotate PDFs" }, | ||
|
|
||
| // Creativity | ||
| new() { Name = "OBS Studio", WingetId = "OBSProject.OBSStudio", Category = "Creativity", Description = "Free streaming and screen recording" }, | ||
| new() { Name = "GIMP", WingetId = "GIMP.GIMP", Category = "Creativity", Description = "Free image editor (Photoshop alternative)" }, | ||
| new() { Name = "Audacity", WingetId = "Audacity.Audacity", Category = "Creativity", Description = "Free audio editor and recorder" }, | ||
| new() { Name = "Blender", WingetId = "BlenderFoundation.Blender", Category = "Creativity", Description = "Free 3D creation suite" }, | ||
|
|
||
| // Networking & VPN | ||
| new() { Name = "qBittorrent", WingetId = "qBittorrent.qBittorrent", Category = "Networking & VPN", Description = "Free open-source BitTorrent client" }, | ||
| new() { Name = "ProtonVPN", WingetId = "ProtonTechnologies.ProtonVPN", Category = "Networking & VPN", Description = "Free privacy-focused VPN" }, | ||
| new() { Name = "WireGuard", WingetId = "WireGuard.WireGuard", Category = "Networking & VPN", Description = "Fast modern VPN protocol" }, | ||
| new() { Name = "PuTTY", WingetId = "SimonTatham.PuTTY", Category = "Networking & VPN", Description = "SSH and Telnet client" }, | ||
|
|
||
| // Runtimes & Frameworks | ||
| new() { Name = ".NET Desktop Runtime 8", WingetId = "Microsoft.DotNet.DesktopRuntime.8", Category = "Runtimes & Frameworks", Description = "Required by many modern apps" }, | ||
| new() { Name = "Visual C++ Redistributable", WingetId = "Microsoft.VCRedist.2015+.x64", Category = "Runtimes & Frameworks", Description = "Required by games and apps" }, | ||
| new() { Name = "Java Runtime", WingetId = "Oracle.JavaRuntimeEnvironment", Category = "Runtimes & Frameworks", Description = "Required by Minecraft and enterprise apps" }, | ||
| new() { Name = "DirectX Runtime", WingetId = "Microsoft.DirectX", Category = "Runtimes & Frameworks", Description = "Required by most games" }, | ||
|
|
||
| // More Utilities | ||
| new() { Name = "WinRAR", WingetId = "RARLab.WinRAR", Category = "Utilities", Description = "Popular file archiver" }, | ||
| new() { Name = "ShareX", WingetId = "ShareX.ShareX", Category = "Utilities", Description = "Screenshot and screen recording tool" }, | ||
| new() { Name = "Greenshot", WingetId = "Greenshot.Greenshot", Category = "Utilities", Description = "Lightweight screenshot tool" }, | ||
| new() { Name = "TreeSize Free", WingetId = "JAMSoftware.TreeSize.Free", Category = "Utilities", Description = "Visualize disk space usage" }, | ||
|
|
||
| // More Communication | ||
| new() { Name = "WhatsApp", WingetId = "WhatsApp.WhatsApp", Category = "Communication", Description = "Messaging app for desktop" }, | ||
| new() { Name = "Microsoft Teams", WingetId = "Microsoft.Teams", Category = "Communication", Description = "Business communication platform" }, | ||
| ]; |
There was a problem hiding this comment.
Curated app count does not meet the stated PR objective.
This curated list currently contains 46 entries, while the PR objective states 47 curated apps. Please either add the missing app entry or update the objective/tests to match the intended shipped scope.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@SysManager/SysManager/ViewModels/BulkInstallerViewModel.cs` around lines 272
- 345, The BuildCuratedApps method returns 46 InstallableApp entries but the PR
objective/tests expect 47; update BuildCuratedApps to either add the missing
InstallableApp entry (create a new new() { Name = "...", WingetId = "...",
Category = "...", Description = "..." } in the returned list) so the list
contains 47 items, or adjust the PR objective/tests to expect 46; locate the
list inside the BuildCuratedApps function and modify it accordingly, ensuring
the InstallableApp objects remain correctly formed.
Comment on lines
+43
to
+48
| [RelayCommand] | ||
| private void RelaunchElevated() | ||
| { | ||
| if (AdminHelper.RelaunchAsAdmin()) | ||
| System.Windows.Application.Current?.Shutdown(); | ||
| } |
There was a problem hiding this comment.
Add feedback when elevation relaunch is cancelled/blocked.
Line 46 handles only success; if relaunch fails, users get no indication of what happened.
Suggested patch
[RelayCommand]
private void RelaunchElevated()
{
- if (AdminHelper.RelaunchAsAdmin())
- System.Windows.Application.Current?.Shutdown();
+ if (AdminHelper.RelaunchAsAdmin())
+ {
+ System.Windows.Application.Current?.Shutdown();
+ return;
+ }
+
+ StatusMessage = "Could not relaunch as administrator.";
+ Log.Warning("Windows Features: relaunch as administrator was not started");
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@SysManager/SysManager/ViewModels/WindowsFeaturesViewModel.cs` around lines 43
- 48, The RelaunchElevated method only shuts down the app on success and
provides no user feedback when AdminHelper.RelaunchAsAdmin() returns false;
update RelaunchElevated to detect the false return and display a clear
user-facing notification (e.g., MessageBox/notification via the VM's messaging
service) indicating that elevation was cancelled or blocked and any recommended
next steps; keep the existing shutdown behavior when RelaunchAsAdmin() returns
true and reference the RelaunchElevated method and AdminHelper.RelaunchAsAdmin()
when making the change.
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
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.
Summary
UI/UX polish for 4 pages based on user feedback.
Bulk Installer
Context Menu Manager
Privacy Toggles + Windows Features
Test plan
Summary by CodeRabbit
New Features
Tests