refactor(archive): peel diff and rename into views/archive/ helpers (Phase 4 pt 2)#2525
Conversation
|
@m3nu No rush on this , it's stacked on #2519, so that one's the real dependency (its commits show in this diff until it merges; the new work here is just archive_diff.py,archive_rename.py, and the archive_tab.py ). Flagging it now mainly for a shape check: it follows the ArchiveExtract/ArchiveMount collaborator pattern from #2519, and I kept the context menu in ArchiveTab since it's just glue over the other helpers. I'm holding delete + check/compact/prune for a follow- up so the pattern can be settled here first. |
Follows the stateless ArchiveExtract/ArchiveMount collaborator pattern: helpers hold only self.tab; transient state (_t, is_editing, renamed_archive_original_name) stays on the tab. Pure structural move, no behavior change.
c5ab42c to
270fa18
Compare
|
Rebased onto master now that #2519 has merged, so the diff is just the part-2 work: Follows the |
|
question on context menu extraction (#2361) : I temporarily kept |
m3nu
left a comment
There was a problem hiding this comment.
Reviewed in depth. Verified behavior preservation mechanically: reversing the self. → self.tab. transform on the two helper files and diffing against master gives byte-identical bodies for all 7 moved methods. All 5 rewired call sites accounted for, no stale references or circular imports, and the existing rename test still drives the real editor, so the archive_model.dataChanged → on_name_edited wiring — the riskiest part of this move — is exercised end-to-end.
On your open question: leaving archiveitem_contextmenu on ArchiveTab is right. It's dispatch glue over the helpers; extracting it would add indirection without shrinking anything.
One series-wide note, not specific to this PR (it started with the extract/mount helpers in #2519): self.tab.tr(...) resolves under the ArchiveTab context at runtime, but pylupdate extracts context from the enclosing class, so the next make translations-from-source would re-file these strings under the helper class names and orphan the existing translations. Let's fix this once across views/archive/ (e.g. translate('ArchiveTab', ...)) — I'll note it on #2361.
Merging this first; #2526 rebases on top, and #2523 last (it touches the same import block in archive_tab.py).
Description
Phase 4 part 2 of the Archive tab refactor. Part 1 (#2519) converted the table to
QAbstractTableModel+QTableViewand peeled the first two operations (mount/unmount,extract) intoviews/archive/collaborator classes. This PR continues that exact pattern, peeling diff and rename off the orchestrator into focused helpers:views/archive/archive_diff.py→ArchiveDiff(diff_action,list_diff_result,show_diff_result)views/archive/archive_rename.py→ArchiveRename(cell_double_clicked,on_name_edited,_revert_name,rename_result)ArchiveTabstays the container (populate, selection, context-menu wiring, refresh, status, prune settings). Pure structural move — no behavior change.Helpers follow the existing
ArchiveExtract/ArchiveMountconvention from #2519. The context menu stays inArchiveTabsince it just dispatches to the other helpers.deleteandcheck/compact/pruneare held back for a follow-up PR so the helper shape can be reviewed here first. Happy to redirect the decomposition — this is the cheap point to do it.Related Issue
Part of #2361 (Phase 4 — split large tabs into parts). Builds on #2519.
Motivation and Context
archive_tab.pywas a large orchestrator mixing the table view with diff, rename, delete and repo-maintenance logic. #2361 calls for converting the table to a model (done in #2519) and then peeling the remaining operations into focused helpers, keepingArchiveTabas the container. This PR does that for diff and rename, making each operation independently readable and shrinking the orchestrator, with no change to runtime behavior.How Has This Been Tested?
uv run pytest tests/unit/test_archives.py tests/unit/test_diff.py— 42 passed.ruff check+ruff format --checkclean. Test call-sites updated to the new entry points (tab.archive_diff.diff_action,tab.archive_rename.{cell_double_clicked,rename_result});_resultwindowis still set on the tab so the diff tests are otherwise unchanged. CI runs the full Borg matrix via nox.Screenshots (if appropriate):
N/A — no UI changes.
Types of changes
Checklist:
I provide my contribution under the terms of the license of this repository and I affirm the Developer Certificate of Origin.