Plugins settings: Enabled/Disabled tabs (drop plugin uninstall) - #42
Open
csmashe wants to merge 2 commits into
Open
Plugins settings: Enabled/Disabled tabs (drop plugin uninstall)#42csmashe wants to merge 2 commits into
csmashe wants to merge 2 commits into
Conversation
Replace the single "Installed" plugin list with Enabled and Disabled
tabs, following the DictionarySection in-app tab pattern (SelectedTab +
SetTabCommand + IsXTabSelected flags styled via BoolBrushConverter; no
TabControl).
Refresh() now partitions the plugin rows by activation state into
EnabledGroups / DisabledGroups, with per-tab counts, tab labels, and
empty states. Dropdowns already list enabled plugins only, so no other
behavior changes.
Retire the now-unused Plugins.Installed string and add
Plugins.Tab{Enabled,Disabled} and Plugins.No{Enabled,Disabled}Plugins
with real translations across en/de/es/ru.
Tests: seed activated plugins via TestPluginManagerFactory and add
PluginsSectionViewModelTests covering the partition, counts, empty-state
flags, and SetTab.
This supersedes the earlier (never-committed) plugin uninstall approach,
which could not reclaim the package-managed /opt master and duplicated
what the existing Disable toggle already does.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe Plugins section now separates plugins into enabled and disabled tabs, with updated view model state, UI rendering, localization strings, and test coverage for partitioning and tab selection. ChangesPlugins Enabled/Disabled Tabs
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant View as PluginsSection.axaml
participant VM as PluginsSectionViewModel
participant Groups as EnabledGroups/DisabledGroups
User->>View: Click tab button
View->>VM: SetTabCommand(0 or 1)
VM->>VM: SelectedTab = parameter
VM->>VM: Notify IsEnabledTabSelected/IsDisabledTabSelected
VM->>VM: Refresh()
VM->>VM: Partition plugins by activation state
VM->>Groups: BuildGroups(enabled/disabled by CategoryKey)
VM->>View: Update EnabledGroups/DisabledGroups, counts, labels
View->>User: Render selected tab content
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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: 1
🧹 Nitpick comments (1)
src/TypeWhisper.Linux/ViewModels/Sections/PluginsSectionViewModel.cs (1)
185-189: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMinor duplication:
enabledCountrecomputed separately fromEnabledCount.
EnabledCount(Line 126) already derives fromEnabledGroups.Sum(...), butHeaderSummaryrecomputes an equivalent value viaplugins.Count(p => p.IsEnabled)at Line 207. Both should stay in sync sinceBuildGroupsruns first, but consider reusingEnabledCountfor clarity and to avoid maintaining two equivalent computations.♻️ Suggested simplification
- var enabledCount = plugins.Count(p => p.IsEnabled); - HeaderSummary = Loc.Instance.GetString("Plugins.HeaderSummary", plugins.Count, enabledCount); + HeaderSummary = Loc.Instance.GetString("Plugins.HeaderSummary", plugins.Count, EnabledCount);Also applies to: 207-208
🤖 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 `@src/TypeWhisper.Linux/ViewModels/Sections/PluginsSectionViewModel.cs` around lines 185 - 189, `HeaderSummary` in `PluginsSectionViewModel` is recomputing the enabled plugin count with `plugins.Count(p => p.IsEnabled)` even though `EnabledCount` already exposes the same value from `EnabledGroups`. Update the `HeaderSummary` logic to reuse `EnabledCount` instead of duplicating the count calculation, and keep the summary text generation in sync with the existing `EnabledGroups`/`DisabledGroups` state.
🤖 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 `@src/TypeWhisper.Linux/Resources/Localization/de.json`:
- Line 438: The German localization string for Plugins.SummaryFailed is
singular-only even though it is rendered with a count. Update the de.json entry
used by the Plugins.SummaryFailed resource to use a plural-neutral wording, or
split it into separate singular and plural variants if that pattern is already
used elsewhere in the localization set. Keep the change localized to the
resource key so all count-based renderings of the summary read correctly.
---
Nitpick comments:
In `@src/TypeWhisper.Linux/ViewModels/Sections/PluginsSectionViewModel.cs`:
- Around line 185-189: `HeaderSummary` in `PluginsSectionViewModel` is
recomputing the enabled plugin count with `plugins.Count(p => p.IsEnabled)` even
though `EnabledCount` already exposes the same value from `EnabledGroups`.
Update the `HeaderSummary` logic to reuse `EnabledCount` instead of duplicating
the count calculation, and keep the summary text generation in sync with the
existing `EnabledGroups`/`DisabledGroups` state.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8f65c4e5-4f44-448a-936a-1eb6ac3c3397
📒 Files selected for processing (9)
src/TypeWhisper.Linux/Resources/Localization/de.jsonsrc/TypeWhisper.Linux/Resources/Localization/en.jsonsrc/TypeWhisper.Linux/Resources/Localization/es.jsonsrc/TypeWhisper.Linux/Resources/Localization/ru.jsonsrc/TypeWhisper.Linux/ViewModels/Sections/PluginsSectionViewModel.cssrc/TypeWhisper.Linux/Views/Sections/PluginsSection.axamltests/TypeWhisper.Linux.Tests/PluginCollectionSettingsViewModelTests.cstests/TypeWhisper.Linux.Tests/PluginsSectionViewModelTests.cstests/TypeWhisper.Linux.Tests/TestPluginManagerFactory.cs
Plugins.SummaryFailed is rendered with a numeric count, but the German
string ("konnte ... geladen") was singular-only and the Spanish
("no se pudieron cargar") plural-only, so one of count == 1 / count > 1
always read ungrammatically. Switch both to count-neutral phrasings
("{0} nicht geladen" / "{0} sin cargar"). en and ru were already
count-neutral. (CodeRabbit PR #42.)
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Comment on lines
+237
to
+241
| foreach (var group in rows.GroupBy(p => p.CategoryKey)) | ||
| { | ||
| var categoryPlugins = group.ToList(); | ||
| target.Add(new PluginCategoryGroup(categoryPlugins[0].CategoryLabel, categoryPlugins)); | ||
| } |
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.
What & why
The Plugins settings section listed every loaded plugin under one "Installed" header. With ~30 bundled plugins, you scroll past disabled ones to manage the few you actually use. This splits the list into two tabs — Enabled and Disabled — so each view only shows what belongs there. Dropdowns already show enabled-only, so no other behavior changes.
This supersedes an earlier plugin-uninstall approach (the branch's original focus). Uninstall doesn't earn its keep on Linux: it can't reclaim the
/optmaster (package-managed), the per-plugin user copy is tiny, and the existing Disable toggle already removes a plugin from every dropdown and all active use — reversibly, keeping settings. So uninstall was dropped and none of that work is in this PR.Changes
PluginsSectionViewModel): follows the existing DictionarySection in-app tab pattern —SelectedTab+SetTabCommand+IsEnabledTabSelected/IsDisabledTabSelectedflags.Refresh()partitions rows by activation state intoEnabledGroups/DisabledGroups; adds per-tab counts, tab labels, and empty-state flags. Toggling a plugin moves its row between tabs automatically (existingPluginStateChanged→Refresh).PluginsSection.axaml): replaced the "Installed" header with twoBoolBrushConverter-styled tab buttons; the group/row template is extracted to a shared resource and rendered by twoItemsControls (visibility bound to the tab flags), each with a per-tab empty state.Plugins.Installed; addedPlugins.TabEnabled,Plugins.TabDisabled,Plugins.NoEnabledPlugins,Plugins.NoDisabledPluginswith real translations across en/de/es/ru.TestPluginManagerFactorycan now seed activated plugins; newPluginsSectionViewModelTestscovers the partition, counts, empty-state flags, andSetTab.Testing
dotnet build TypeWhisper.slnx— 0 warnings / 0 errorsdotnet test— Linux.Tests 620/620, Core.Tests 362/362 green🤖 Generated with Claude Code
Summary by CodeRabbit