refactor(gui): extract column persistence out of CMuleListCtrl - #787
Merged
Conversation
Splits the column-name/width/sort-order load-save logic out of CMuleListCtrl into a new widget-agnostic CListColumnStore, talking to the list widget through a small IColumnWidthProvider interface (column count, get/set width by index). Why: prep for the wxDataViewCtrl port of CSearchListCtrl (amule-org#675/amule-org#180 phase 2). got3nks asked that this extraction land as its own, independently reviewable commit, since the persistence logic (including the legacy pre-2.2.2 config migration path) must not be duplicated against wxDataViewColumn -- the new search list control will reuse CListColumnStore unchanged instead of reimplementing it. No new functionality: config keys ("/eMule/TableOrdering<name>", "/eMule/TableWidths<name>") and on-disk format are unchanged, so existing user settings keep working. CMuleListCtrl keeps its public API (SaveSettings/LoadSettings/ InsertColumn/SetTableName/GetColumnIndex/...) and now implements IColumnWidthProvider via explicit forwarding overrides -- needed because C++ won't otherwise link wxGenericListCtrl's GetColumnCount/GetColumnWidth/SetColumnWidth to the identically-named pure virtuals of an unrelated second base class. Verified: builds clean (macOS, full amule target); launched the built app against an isolated ~/aMuleTest config dir (own ports, never the real app) and confirmed column settings load and save back correctly, matching pre-refactor behaviour. Ran the local clang-tidy CI replica (Tier-1 whole-tree + Tier-2 changed-lines); Tier-1 reported no hits in the new/changed files, Tier-2 flagged narrowing conversions and a few modernize-* suggestions on the new code, all fixed.
|
Merged — thanks @LSalami. Faithful extraction: the sort-order reversal and the pre-2.2.2 migration path both came through intact, config keys are untouched so existing settings keep working, and the public API is unchanged (only private helpers moved). Good base for the |
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
Extracts the column-name/width/sort-order persistence logic out of
CMuleListCtrlinto a new, widget-agnosticCListColumnStore, which talks to the list widget through a smallIColumnWidthProviderinterface (column count, get/set width by index).This is prep work for #180 phase 2 (porting
CSearchListCtrltowxDataViewCtrlfor accessibility, tracked under #675). @got3nks asked in the #180 discussion that this extraction land as its own, independently reviewable commit before the actualwxDataViewCtrlport, since the persistence logic — including the legacy pre-2.2.2 config migration path — must not be duplicated againstwxDataViewColumn. The new search list control will reuseCListColumnStoreunchanged instead of reimplementing it.No behaviour change. Config keys (
/eMule/TableOrdering<name>,/eMule/TableWidths<name>) and the on-disk format are unchanged, so existing user settings keep working.CMuleListCtrlkeeps its full public API (SaveSettings/LoadSettings/InsertColumn/SetTableName/GetColumnIndex/...).Notes
CMuleListCtrlnow implementsIColumnWidthProvidervia explicit forwarding overrides (GetColumnCount/GetColumnWidth/SetColumnWidth) rather than relying onwxGenericListCtrl's identically-signatured methods to satisfy the interface implicitly — with two unrelated base classes, the compiler won't link one base's definition to the other's pure virtual on its own.CListColumnStore: applying a loaded sort order has widget-specific side effects (CMuleListCtrl::SetSorting()also triggers a re-sort and validates againstAltSortAllowed()), soLoadSettings()only decodes the stored order and hands it back to the caller to apply through its ownSetSorting().CMuleListCtrl::OnMenuSelected) moved into the store too (GetCachedWidth/SetCachedWidth), since it's part of the same per-column state.Test plan
amuletarget)clang-formatv18 (pinned) appliedclang-tidyTier-1 (whole-tree) and Tier-2 (changed-lines) both clean on the touched files