feat: add context menu manager with non-destructive toggle#484
Conversation
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughAdds a Context Menu Manager: model, service, view-model, DI wiring, WPF UI, registry scanning (HKCR), enable/disable via LegacyDisable with registry backups, source extraction from commands, filtering, and sidebar integration. ChangesContext Menu Manager Feature
Sequence DiagramsequenceDiagram
participant User as User
participant View as ContextMenuView
participant VM as ContextMenuViewModel
participant Service as ContextMenuService
participant Registry as Windows Registry
participant Backup as Registry Backup
User->>View: Open / Refresh Context Menu Manager
View->>VM: RefreshCommand / ToggleEntryCommand
VM->>Service: ScanEntries()
Service->>Registry: Read HKCR shell locations (files/folders/desktop/background)
Registry-->>Service: Entry subkeys with command & LegacyDisable status
Service-->>VM: List<ContextMenuEntry>
VM->>VM: ApplyFilter() & UpdateCounts()
VM-->>View: Updated Entries collection
User->>View: Toggle entry checkbox
View->>VM: ToggleEntryCommand(entry)
VM->>Backup: BackupRegistry(entry.RegistryPath)
Backup->>Backup: Export .reg with timestamp
alt entry.IsEnabled (now true)
VM->>Service: EnableEntry(entry)
Service->>Registry: Delete LegacyDisable value
else
VM->>Service: DisableEntry(entry)
Service->>Registry: Set LegacyDisable = ""
end
VM->>VM: UpdateCounts()
VM-->>View: Update status / revert checkbox on failure
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/ContextMenuViewModel.cs`:
- Around line 55-67: ScanAsync is performing UI-bound mutations (_allEntries,
StatusMessage, IsBusy, ApplyFilter) inside Task.Run on a background thread;
change it to do only the blocking work (call _service.ScanEntries()) on the
background thread and marshal all UI updates back to the UI thread (e.g. via
Application.Current.Dispatcher/Dispatcher.Invoke or the captured
SynchronizationContext) to clear/_allEntries.Add, set StatusMessage and IsBusy,
and call ApplyFilter; apply the same pattern to the similar Task.Run block
around lines 77–78 so all collection/property changes happen on the UI thread
while heavy scanning remains on the worker thread.
- Around line 80-107: ToggleEntry currently runs synchronous registry/backup
work on the UI thread; make it asynchronous to avoid freezes by changing the
method to an async Task ToggleEntry(object? parameter) and offloading the
blocking work to a background thread (either by calling new async service
methods like EnableEntryAsync/DisableEntryAsync or by awaiting Task.Run(() =>
_service.EnableEntry(entry)) / Task.Run(() => _service.DisableEntry(entry))).
Preserve the current desiredState value, await the enable/disable call, then on
completion (continuation on UI context) call UpdateCounts(), set StatusMessage
and Log on success, and revert entry.IsEnabled and set the failure StatusMessage
on failure; ensure any newly added service async methods keep exception/return
semantics and update callers accordingly.
In `@SysManager/SysManager/Views/ContextMenuView.xaml`:
- Around line 30-50: Add two toolbar controls to expose the missing actions:
wire a "Restore Defaults" button bound to a RestoreDefaultsCommand and a "Group
by Type" control (either a ToggleButton or a small ComboBox) bound to a
GroupByTypeCommand or GroupingMode property; place them alongside the existing
ComboBox (LocationFilters/SelectedLocation), TextBox (FilterText) and
RefreshCommand Button so users can trigger restore-all/defaults and switch
grouping by entry type from the same StackPanel. Ensure the new control names
match your viewmodel members (RestoreDefaultsCommand, GroupByTypeCommand or
GroupingMode) so the bindings resolve.
🪄 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: 961ddd53-b4b8-4c0a-859b-db5fcb1ef21f
📒 Files selected for processing (8)
CHANGELOG.mdSysManager/SysManager/Models/ContextMenuEntry.csSysManager/SysManager/ServiceRegistration.csSysManager/SysManager/Services/ContextMenuService.csSysManager/SysManager/ViewModels/ContextMenuViewModel.csSysManager/SysManager/ViewModels/MainWindowViewModel.csSysManager/SysManager/Views/ContextMenuView.xamlSysManager/SysManager/Views/ContextMenuView.xaml.cs
📜 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 (13)
SysManager/SysManager/Models/ContextMenuEntry.cs (1)
1-33: LGTM!SysManager/SysManager/Services/ContextMenuService.cs (6)
20-29: LGTM!
35-112: LGTM!
119-153: LGTM!
159-193: LGTM!
199-238: LGTM!
243-276: LGTM!SysManager/SysManager/ViewModels/ContextMenuViewModel.cs (1)
113-140: LGTM!SysManager/SysManager/ServiceRegistration.cs (1)
57-57: LGTM!Also applies to: 90-90
SysManager/SysManager/ViewModels/MainWindowViewModel.cs (1)
48-48: LGTM!Also applies to: 81-81, 147-147, 196-196, 237-237, 423-423, 571-571
SysManager/SysManager/Views/ContextMenuView.xaml (1)
1-25: LGTM!Also applies to: 60-137
SysManager/SysManager/Views/ContextMenuView.xaml.cs (1)
1-13: LGTM!CHANGELOG.md (1)
10-12: LGTM!
| private Task ScanAsync() => Task.Run(() => | ||
| { | ||
| IsBusy = true; | ||
| StatusMessage = "Scanning context menu entries..."; | ||
| try | ||
| { | ||
| var items = _service.ScanEntries(); | ||
| _allEntries.Clear(); | ||
| foreach (var item in items.OrderBy(e => e.Name, StringComparer.OrdinalIgnoreCase)) | ||
| _allEntries.Add(item); | ||
|
|
||
| ApplyFilter(); | ||
| StatusMessage = $"Found {_allEntries.Count} context menu entries."; |
There was a problem hiding this comment.
Move UI-bound mutations off the worker thread.
ScanAsync currently runs the whole workflow in Task.Run, then updates bindable properties/collections from a background thread. In WPF, this can fail with cross-thread collection/property notifications and can race with filter-triggered reads of _allEntries.
⚙️ Suggested fix
[RelayCommand]
-private Task ScanAsync() => Task.Run(() =>
-{
- IsBusy = true;
- StatusMessage = "Scanning context menu entries...";
- try
- {
- var items = _service.ScanEntries();
- _allEntries.Clear();
- foreach (var item in items.OrderBy(e => e.Name, StringComparer.OrdinalIgnoreCase))
- _allEntries.Add(item);
-
- ApplyFilter();
- StatusMessage = $"Found {_allEntries.Count} context menu entries.";
- }
- catch (InvalidOperationException ex)
- {
- StatusMessage = $"Scan failed: {ex.Message}";
- }
- catch (UnauthorizedAccessException ex)
- {
- StatusMessage = $"Scan failed: {ex.Message}";
- }
- finally { IsBusy = false; }
-});
+private async Task ScanAsync()
+{
+ IsBusy = true;
+ StatusMessage = "Scanning context menu entries...";
+ try
+ {
+ var items = await Task.Run(_service.ScanEntries);
+ _allEntries.Clear();
+ foreach (var item in items.OrderBy(e => e.Name, StringComparer.OrdinalIgnoreCase))
+ _allEntries.Add(item);
+
+ ApplyFilter();
+ StatusMessage = $"Found {_allEntries.Count} context menu entries.";
+ }
+ catch (InvalidOperationException ex)
+ {
+ StatusMessage = $"Scan failed: {ex.Message}";
+ }
+ catch (UnauthorizedAccessException ex)
+ {
+ StatusMessage = $"Scan failed: {ex.Message}";
+ }
+ finally
+ {
+ IsBusy = false;
+ }
+}Also applies to: 77-78
🤖 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/ContextMenuViewModel.cs` around lines 55 -
67, ScanAsync is performing UI-bound mutations (_allEntries, StatusMessage,
IsBusy, ApplyFilter) inside Task.Run on a background thread; change it to do
only the blocking work (call _service.ScanEntries()) on the background thread
and marshal all UI updates back to the UI thread (e.g. via
Application.Current.Dispatcher/Dispatcher.Invoke or the captured
SynchronizationContext) to clear/_allEntries.Add, set StatusMessage and IsBusy,
and call ApplyFilter; apply the same pattern to the similar Task.Run block
around lines 77–78 so all collection/property changes happen on the UI thread
while heavy scanning remains on the worker thread.
| [RelayCommand] | ||
| private void ToggleEntry(object? parameter) | ||
| { | ||
| if (parameter is not ContextMenuEntry entry) return; | ||
|
|
||
| // The CheckBox two-way binding has already flipped IsEnabled before | ||
| // this command runs. We use the current (already-flipped) value as | ||
| // the desired new state. | ||
| var desiredState = entry.IsEnabled; | ||
| bool success; | ||
|
|
||
| if (desiredState) | ||
| success = _service.EnableEntry(entry); | ||
| else | ||
| success = _service.DisableEntry(entry); | ||
|
|
||
| if (success) | ||
| { | ||
| UpdateCounts(); | ||
| StatusMessage = $"{entry.Name} {(desiredState ? "enabled" : "disabled")}."; | ||
| Log.Information("Context menu entry toggled: {Name} -> {State}", entry.Name, desiredState ? "enabled" : "disabled"); | ||
| } | ||
| else | ||
| { | ||
| // Revert the CheckBox state since the operation failed | ||
| entry.IsEnabled = !desiredState; | ||
| StatusMessage = $"Could not toggle {entry.Name} — requires administrator privileges."; | ||
| } |
There was a problem hiding this comment.
Avoid blocking the UI thread during toggle operations.
ToggleEntry performs registry write/backup synchronously. If backup or registry access is slow, checkbox interaction will freeze.
⚙️ Suggested fix
[RelayCommand]
-private void ToggleEntry(object? parameter)
+private async Task ToggleEntry(object? parameter)
{
if (parameter is not ContextMenuEntry entry) return;
@@
- bool success;
-
- if (desiredState)
- success = _service.EnableEntry(entry);
- else
- success = _service.DisableEntry(entry);
+ IsBusy = true;
+ bool success;
+ try
+ {
+ success = await Task.Run(() =>
+ desiredState ? _service.EnableEntry(entry) : _service.DisableEntry(entry));
+ }
+ finally
+ {
+ IsBusy = false;
+ }
@@
- else
+ else
{
// Revert the CheckBox state since the operation failed
entry.IsEnabled = !desiredState;
StatusMessage = $"Could not toggle {entry.Name} — requires administrator privileges.";
}
}🤖 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/ContextMenuViewModel.cs` around lines 80 -
107, ToggleEntry currently runs synchronous registry/backup work on the UI
thread; make it asynchronous to avoid freezes by changing the method to an async
Task ToggleEntry(object? parameter) and offloading the blocking work to a
background thread (either by calling new async service methods like
EnableEntryAsync/DisableEntryAsync or by awaiting Task.Run(() =>
_service.EnableEntry(entry)) / Task.Run(() => _service.DisableEntry(entry))).
Preserve the current desiredState value, await the enable/disable call, then on
completion (continuation on UI context) call UpdateCounts(), set StatusMessage
and Log on success, and revert entry.IsEnabled and set the failure StatusMessage
on failure; ensure any newly added service async methods keep exception/return
semantics and update callers accordingly.
| <StackPanel Orientation="Horizontal" DockPanel.Dock="Left"> | ||
| <ComboBox ItemsSource="{Binding LocationFilters}" | ||
| SelectedItem="{Binding SelectedLocation}" | ||
| Width="160" Margin="0,0,8,0" | ||
| VerticalAlignment="Center"/> | ||
| <TextBox Text="{Binding FilterText, UpdateSourceTrigger=PropertyChanged}" | ||
| Width="200" Margin="0,0,8,0" | ||
| VerticalAlignment="Center"> | ||
| <TextBox.Style> | ||
| <Style TargetType="TextBox" BasedOn="{StaticResource {x:Type TextBox}}"> | ||
| <Style.Triggers> | ||
| <Trigger Property="Text" Value=""> | ||
| <Setter Property="Tag" Value="Search entries..."/> | ||
| </Trigger> | ||
| </Style.Triggers> | ||
| </Style> | ||
| </TextBox.Style> | ||
| </TextBox> | ||
| <Button Content="Refresh" Command="{Binding RefreshCommand}" | ||
| Style="{StaticResource SecondaryButton}" Margin="0,0,8,0"/> | ||
| </StackPanel> |
There was a problem hiding this comment.
Missing required “restore defaults” and type-grouping entry points in the toolbar.
The linked issue contract includes both a restore-all/defaults action and grouping by entry type, but this toolbar only exposes location/search/refresh. Please add UI hooks for those required operations.
Proposed UI additions
<StackPanel Orientation="Horizontal" DockPanel.Dock="Left">
<ComboBox ItemsSource="{Binding LocationFilters}"
SelectedItem="{Binding SelectedLocation}"
Width="160" Margin="0,0,8,0"
VerticalAlignment="Center"/>
+ <ComboBox ItemsSource="{Binding TypeFilters}"
+ SelectedItem="{Binding SelectedType}"
+ Width="180" Margin="0,0,8,0"
+ VerticalAlignment="Center"/>
<TextBox Text="{Binding FilterText, UpdateSourceTrigger=PropertyChanged}"
Width="200" Margin="0,0,8,0"
VerticalAlignment="Center">
@@
<Button Content="Refresh" Command="{Binding RefreshCommand}"
Style="{StaticResource SecondaryButton}" Margin="0,0,8,0"/>
+ <Button Content="Restore defaults"
+ Command="{Binding RestoreDefaultsCommand}"
+ Style="{StaticResource SecondaryButton}"/>
</StackPanel>🤖 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/Views/ContextMenuView.xaml` around lines 30 - 50, Add
two toolbar controls to expose the missing actions: wire a "Restore Defaults"
button bound to a RestoreDefaultsCommand and a "Group by Type" control (either a
ToggleButton or a small ComboBox) bound to a GroupByTypeCommand or GroupingMode
property; place them alongside the existing ComboBox
(LocationFilters/SelectedLocation), TextBox (FilterText) and RefreshCommand
Button so users can trigger restore-all/defaults and switch grouping by entry
type from the same StackPanel. Ensure the new control names match your viewmodel
members (RestoreDefaultsCommand, GroupByTypeCommand or GroupingMode) so the
bindings resolve.
0be804d to
245028f
Compare
…ting (#498) ## Problem Disk Analyzer reported total used space exceeding actual disk capacity when scanning drive C:. Junctions and symbolic links (e.g. `C:\Documents and Settings` → `C:\Users`) caused the same files to be counted multiple times. ## Fix Added `FileAttributes.ReparsePoint` check before traversing subdirectories, both at top-level and during recursive scan. Junctions, symlinks, and mount points are now skipped. ## Files changed - `Services/DiskAnalyzerService.cs` — reparse point detection in `Analyze()` and `MeasureFolder()` - `CHANGELOG.md` Closes #484 Co-authored-by: laurentiu021 <laurentiu021@users.noreply.github.com>
Co-authored-by: laurentiu021 <laurentiu021@users.noreply.github.com>
Summary
New System feature: Context Menu Manager — scan and manage Windows Explorer right-click menu entries.
Features:
LegacyDisableregistry value (standard Windows approach)reg exportbefore modificationsCloses #8
Test plan
Summary by CodeRabbit