Skip to content

refactor(tui): split god-package into per-mode handlers#32

Open
axelgar wants to merge 1 commit into
mainfrom
base-branch-list-improvements
Open

refactor(tui): split god-package into per-mode handlers#32
axelgar wants to merge 1 commit into
mainfrom
base-branch-list-improvements

Conversation

@axelgar

@axelgar axelgar commented May 19, 2026

Copy link
Copy Markdown
Owner

Summary

  • Splits pkg/tui/ (one 677-line Update(), one 514-line View(), 50+ flat fields on Model, 10 interleaved modes) into a Mode-dispatched architecture with per-mode handler files (mode_<mode>.go) and typed per-mode state sub-structs.
  • view.go shrinks from 514 to 27 lines; update.go from 677 to 314 (the rest is non-key message handlers, which deliberately stay in the dispatcher). Biggest single file is now mode_list.go at ~446 lines.
  • Fixes three latent bugs the dispatcher made visible: incomplete errMsg reset, prGenerating not gating keys, and a PR-content async race after esc.

What changed

Stage Result
1a — Mode enum + dispatcher Update/View route via switch m.mode
3 — per-mode files mode_list.go, mode_create.go, mode_create_remote.go, mode_delete.go, mode_diff.go, mode_pr_creating.go, mode_pr_generating.go, mode_agentselect.go, mode_errorlog.go
2 — typed sub-structs listState, createState, deleteState, diffState, prState, agentSelectState, errorLogState. Active-mode booleans removed; m.mode is the single source of truth
4 — test split helpers_test.go for shared test helpers; new mode_pr_generating_test.go with regression tests for bugs A/B/C

Latent bugs fixed

  • Bug AerrMsg previously cleared only creating, filtering, prCreating while leaving deleting, diffViewing, agentSelecting, showErrLog, prGenerating, etc. hanging. New m.resetToList() zeroes every per-mode sub-struct.
  • Bug BprGenerating had no key guard, so pressing n/i/r during PR content generation stacked a create dialog over the loading screen. ModePRGenerating now has its own handler that swallows non-esc keys.
  • Bug C — a late prContentGeneratedMsg arriving after the user pressed esc silently flipped into ModePRCreating. esc now sets pr.cancelled; the message handler drops the late content and resets the flag.

Test plan

  • go test ./... — all 237 tests pass across 10 packages (96 existing TUI + 8 new regression tests for bugs A/B/C)
  • go vet ./... clean
  • Manually run ./opentree and exercise each mode end-to-end:
    • List view: navigate, multi-select, sort, filter (/), error log (E)
    • Create: n (plain), i (issue), r (remote)
    • Delete: x (single + batch)
    • Diff: d
    • PR creation: p (verify generating screen swallows keys; esc cancels even if content arrives later)
    • Agent select: A

🤖 Generated with Claude Code

The TUI was a single Model with 50+ flat fields, one 677-line Update(),
one 514-line View(), and 11 modes interleaved via boolean flag checks.
Refactoring was risky and no mode had isolated tests.

Stage 1a: introduce a Mode enum and dispatcher. Update() and View() now
route through `switch m.mode` instead of overlapping if-chains.

Stage 3: extract per-mode files (mode_<mode>.go) for list, create,
create-from-remote, delete, diff, pr-creating, pr-generating,
agent-select, and error-log. view.go shrinks to 27 lines; update.go
keeps only non-key message handlers and shared helpers.

Stage 2: replace flat fields with typed sub-structs (listState,
createState, deleteState, diffState, prState, agentSelectState,
errorLogState). Active-mode booleans (creating, deleting, …) are gone;
m.mode is the single source of truth. Adds resetToList() for exhaustive
recovery to the list view.

Stage 4: extract test helpers to helpers_test.go. Add regression tests
covering three latent bugs that the dispatcher made visible:
 - Bug A: errMsg used to clear only some mode flags. resetToList() now
   exhaustively zeroes every per-mode sub-struct.
 - Bug B: prGenerating did not gate keys, so pressing 'n' during PR
   generation opened the create dialog on top of the loading screen.
   ModePRGenerating now has its own handler that swallows non-esc keys.
 - Bug C: a late prContentGeneratedMsg arriving after esc would
   silently flip into ModePRCreating. esc now sets pr.cancelled and the
   message handler drops the late content.

All 237 tests across 10 packages pass. go vet clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

1 participant