Skip to content

feat(gui): port CFileDetailListCtrl to CMuleVirtualDataViewCtrl (#180, #801) - #839

Merged
got3nks merged 4 commits into
amule-org:masterfrom
LSalami:filedetail-dataview-port
Aug 7, 2026
Merged

feat(gui): port CFileDetailListCtrl to CMuleVirtualDataViewCtrl (#180, #801)#839
got3nks merged 4 commits into
amule-org:masterfrom
LSalami:filedetail-dataview-port

Conversation

@LSalami

@LSalami LSalami commented Aug 7, 2026

Copy link
Copy Markdown

Context

Continues #180/#801's wxListCtrl -> wxDataViewCtrl migration on the shared base from #811. Search (#796), Servers (#807/#811) and Friends (#830) are done; this ports CFileDetailListCtrl (the small source-name list inside the File Details dialog).

Picked next because it has no CBarShader rendering — unlike Downloads/SharedFiles/Sources-Peers, which additionally need a wxDataViewCustomRenderer that doesn't exist anywhere in the tree yet, so those stay a separate, dedicated piece of work.

What changed

Its caller, CFileDetailDialog::FillSourcenameList(), drove the old list with raw position-indexed CRUD (FindItem by name, SetItemPtrData, SetItem by column index, DeleteItem) rather than the pointer-identity style CServerListCtrl/CFriendListCtrl already use, so porting the control cleanly meant refactoring the caller too:

  • CFileDetailDialog gained a std::map<wxString, SourcenameItem *> m_sourcenames member, replacing "search the list widget by name" with a map lookup. FillSourcenameList()'s reset/update/prune shape is otherwise unchanged, just re-keyed to the map and driven through the control's new AddSource/RefreshSource/RemoveSource API.
  • OnBnClickedTakeOver()/OnListClickedTakeOver() resolve the selected row through GetSelectedItemData() (inherited from CMuleVirtualDataViewCtrl) instead of GetNextItem+GetItemText.
  • The dialog's EVT_LIST_ITEM_ACTIVATED binding on IDC_LISTCTRLFILENAMES becomes EVT_DATAVIEW_ITEM_ACTIVATED; wxDataViewEvent is a wxNotifyEvent/wxCommandEvent descendant so it still propagates from the child control to the dialog-level handler the same way.
  • Fixed a latent leak: nothing in ~CFileDetailDialog() freed the SourcenameItem objects still referenced by open rows — only the "not a partfile" and "pruned to zero" paths in FillSourcenameList() ever deleted them. Now freed in the destructor via m_sourcenames.

CMuleDataViewCtrl always ORs in wxDV_MULTIPLE (no single-selection mode exists on the shared base), but this list's "take over filename" actions assume exactly one selection — same as before the port, since the old list never actually requested wxLC_SINGLE_SEL either (checked muuli_wdr.cpp); it enforced single-selection itself via OnSelect() deselecting every other row. CFileDetailListCtrl::OnSelectionChanged() does the equivalent: collapses to the just-clicked row whenever more than one ends up selected.

Also deleted a vestigial nested SourcenameItem struct in FileDetailListCtrl.h that duplicated (by accident of matching memory layout) the real one in PartFile.h, and dropped a per-row background-colour set that reproduced the default and carried its own "do we still need this?" comment.

Verification

  • Builds clean.
  • clang-format v18 (pinned Docker image) applied.
  • clang-tidy Tier-1 (whole-tree) and Tier-2 (changed-lines, .clang-tidy-new-code) both clean via a local CI replica.
  • Grepped for other CFileDetailListCtrl/IDC_LISTCTRLFILENAMES references to confirm muuli_wdr.cpp's construction call needed no changes.
  • Not yet verified interactively: source-name list populate/re-tally over the 5s refresh timer, "Take over filename" via button and via double-click, single-selection enforcement, sort by clicking either header, and a VoiceOver spot-check — happy to have this checked before or after merge.

…e-org#180, amule-org#801)

Continues the wxListCtrl -> wxDataViewCtrl migration (amule-org#180/amule-org#801) onto
the shared base from amule-org#811, after Search (amule-org#796), Servers (amule-org#807/amule-org#811)
and Friends (amule-org#830). CFileDetailListCtrl was next because it has no
CBarShader rendering (unlike Downloads/SharedFiles/Sources-Peers,
which additionally need a still-nonexistent wxDataViewCustomRenderer).

Its caller, CFileDetailDialog::FillSourcenameList(), drove the old
list with raw position-indexed CRUD (FindItem by name, SetItemPtrData,
SetItem by column index, DeleteItem) rather than the pointer-identity
style CServerListCtrl/CFriendListCtrl already use, so porting the
control cleanly meant refactoring the caller too:

- CFileDetailDialog gained a std::map<wxString, SourcenameItem *>
  m_sourcenames member, replacing "search the list widget by name"
  with a map lookup. FillSourcenameList()'s reset/update/prune shape
  is otherwise unchanged, just re-keyed to the map and driven through
  the control's new AddSource/RefreshSource/RemoveSource API.
- OnBnClickedTakeOver()/OnListClickedTakeOver() resolve the selected
  row through GetSelectedItemData() (inherited from
  CMuleVirtualDataViewCtrl) instead of GetNextItem+GetItemText.
- The dialog's EVT_LIST_ITEM_ACTIVATED binding on IDC_LISTCTRLFILENAMES
  becomes EVT_DATAVIEW_ITEM_ACTIVATED; wxDataViewEvent is a
  wxNotifyEvent/wxCommandEvent descendant so it still propagates from
  the child control to the dialog-level handler the same way.
- Fixed a latent leak: nothing in ~CFileDetailDialog() freed the
  SourcenameItem objects still referenced by open rows -- only the
  "not a partfile" and "pruned to zero" paths in FillSourcenameList()
  ever deleted them. Now freed in the destructor via m_sourcenames.

CMuleDataViewCtrl always ORs in wxDV_MULTIPLE (no single-selection
mode exists on the shared base), but this list's "take over filename"
actions assume exactly one selection, same as the old list did (it
never actually requested wxLC_SINGLE_SEL either -- checked
muuli_wdr.cpp -- it enforced single-selection itself via OnSelect()
deselecting every other row). CFileDetailListCtrl::OnSelectionChanged()
does the equivalent: collapses to the just-clicked row whenever more
than one ends up selected.

Also deleted a vestigial nested SourcenameItem struct in
FileDetailListCtrl.h that duplicated (by accident of matching layout)
the real one in PartFile.h, and dropped a per-row background-colour
set that reproduced the default and carried its own "do we still need
this?" comment.

Verified: builds clean; clang-format v18 (pinned Docker image) applied;
clang-tidy Tier-1 (whole-tree) and Tier-2 (changed-lines,
.clang-tidy-new-code) both clean via the ~/aMuleTest/ci-local replica.
Grepped for other CFileDetailListCtrl/IDC_LISTCTRLFILENAMES references
to confirm muuli_wdr.cpp's construction call needed no changes.
Interactive verification (source list populate/re-tally, take-over via
button and double-click, single-selection enforcement, sort, VoiceOver)
left for manual testing per project convention.
@got3nks

got3nks commented Aug 7, 2026

Copy link
Copy Markdown

Reviewed and pushed a follow-up commit to this branch - three things, all small:

Sorting. FillSourcenameList() zeroes every count and rewrites them in place, so while it runs the list isn't ordered by the column it's sorted on - and AddSource() places a new row with a binary search that needs that ordering to hold. Insertions were therefore landing in arbitrary positions. Now re-sorted after an insertion, exactly as the pre-port code did - and only then: a count that merely changed is what the live-sort preference governs, so sorting unconditionally would have quietly overridden that setting for this list.

Free ordering. The destructor and the not-a-partfile path freed the SourcenameItems while the control still held pointers to them. Not reachable today - both call sites are stack temporaries, so destruction is synchronous - but it rests on wx's teardown order rather than anything guaranteed. Both now clear the rows first, matching what the prune loop already did.

One note on the description: the removed nested SourcenameItem wasn't a layout-compatible duplicate - it declared long count where PartFile.h's has int count, so the old SortProc read a 4-byte field as 8 bytes on every comparison. This port fixes a real type-confusion bug, which is worth more credit than the body gives it.

Everything else checked out: CompareItemData applies the direction modifier where the base expects it, the leak fix is real, prune ordering is correct, and no other holder of these pointers exists in the tree. Visual pass on macOS is clean. Merging once CI is green.

…ing them

Review follow-up to the port.

FillSourcenameList() zeroes every count and then rewrites them in place, so
while it runs the list is not ordered by the column it is sorted on -- and
AddSource() places a new row with a binary search, which needs that ordering
to hold. An insertion therefore leaves rows in arbitrary positions, and the
repair has to happen here, exactly as the pre-port code did.

Only on insertion, though. A count that merely changed is what the live-sort
preference governs: RefreshSource() re-sorts when it is on, and when it is
off the row is meant to stay put rather than move under the user. Sorting
unconditionally would quietly override that setting for this list.

The destructor and the not-a-partfile path also freed the SourcenameItem
objects while the list control still held pointers to them. Nothing can
paint or sort in either window today -- both dialog call sites are stack
temporaries, so destruction is synchronous -- but that rests on wx's teardown
order rather than on anything guaranteed, and the base states the rule
plainly: it has to be told before the caller frees the item data. Both now
clear the rows first, as the prune loop already did.
@got3nks
got3nks force-pushed the filedetail-dataview-port branch from 39d1e2a to 9f8a99f Compare August 7, 2026 10:13
got3nks added 2 commits August 7, 2026 12:18
IsLiveSortColumn() answered true for every column, so a refresh tick
scheduled a re-sort even when the list was sorted by File Name -- a value
that never changes, since the name is the key each row was created under.

Answer per column instead, the shape CServerListCtrl already uses, so only a
sources-sorted list re-sorts on its own. The header comment already
described it this way; only the implementation did not.
FileDetailDialog.cpp carried the "Any parts of this program derived from
the xMule, lMule or eMule project" paragraph twice. It is the only file in
src/ that does, and the file is already being touched here.
@got3nks

got3nks commented Aug 7, 2026

Copy link
Copy Markdown

Two more commits on this branch.

Per-column live sort. IsLiveSortColumn() answered true unconditionally, so every refresh tick scheduled a re-sort even when the list was sorted by File Name - a value that never changes, since the name is the key each row is created under. It now answers per column, the same shape CServerListCtrl uses (Ping/Users/Files only). The header comment already described it that way; only the implementation didn't.

Header cleanup. FileDetailDialog.cpp carried the "Any parts of this program derived..." paragraph twice - the only file in src/ that does, and we're already touching it.

Combined with the earlier commit, sorting now behaves like this: an insertion re-sorts (it has to - zeroing the counts breaks the ordering AddSource()'s binary search relies on), a changing count re-sorts only when sorted by Sources and live sort is enabled, and sorting by File Name never re-sorts on its own.

@got3nks
got3nks merged commit 35e15f1 into amule-org:master Aug 7, 2026
14 checks passed
@LSalami
LSalami deleted the filedetail-dataview-port branch August 7, 2026 11:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants