refactor(tui): split 7.8k-line app.rs into cohesive app/ modules - #403
Conversation
Pure structural move with zero behavior change: app.rs (7,807 lines) becomes app/ with one module per responsibility — input (keys, mouse), state model (types), provider management (providers), command dispatch (commands), session lifecycle (turns, messages, run), and the test suite (tests). Public API and re-exports are unchanged; the only visibility adjustment is pub(super) on moved private methods, which restores exactly the visibility the single-module layout provided. Verified: workspace cargo check clean, full test suite green (~1,000 tests), and cargo clippy --workspace --all-targets -D warnings clean.
|
Reviewed this closely and it holds up — I diffed all 208 functions against One small correction to "Public API unchanged": On timing: seven open PRs currently touch |
…utocomplete - scan_dir now handles subdirectories with SKILL.md (directory-based skills) - parse_skill_file uses parent dir name when file stem is 'skill' - resolve_path expands ~ and ~/ paths, leaves ~user unexpanded, handles both / and backslash separators for Windows - discover_skills adds ~/.agents/skills/ as a global source - commands_from_discovered_skills is now wired into autocomplete via all_slash_command_names, so skills appear in the command palette - Discovered slash commands cached on App and merged at prompt update time, not re-discovered per keystroke - No CRLF/LF churn: all changes preserve original line endings Addresses review feedback on PR Kuberwastaken#401: 1. Rebased on post-Kuberwastaken#403 main, no reformatting 2. resolve_path handles ~user paths (left unexpanded) and Windows separators 3. scan_dir collects rather than returning early on SKILL.md 4. Discovery runs once at startup (not in App::new, not per keystroke) 5. all_slash_command_names cached, not rebuilt per keystroke
From SlugThugLabs — building from whatever's on hand, to standards worth aspiring to.
Summary
tui/src/app.rswas a 7,807-line file containing the fullAppstate struct, a 5,557-line impl block, the keyboard dispatch tree, and a 950-line test suite. This PR restructures it intoapp/with one module per responsibility. Pure structural move — zero behavior change. Every function body is byte-identical to the original; no logic was touched, no warnings fixed, no style "improved" while moving.Module layout
mod.rsAppstruct, constructor, re-exports, module wiringtypes.rskeys.rsmouse.rsproviders.rscommands.rsturns.rsmessages.rsprompt.rsviews.rsrun.rstests.rsuse super::*)Public API unchanged —
crate::app::{App, ...}re-exported frommod.rsexactly as before; zero ripples intorender.rs,messages/, orlib.rs.Visibility note (the one deliberate adjustment)
Private methods moved across module boundaries became
pub(super)— restoring exactly the visibility the single-module layout provided. Of the 79 items lifted: 48 have genuine cross-module callers; 31 are used only within their defining file and are candidates to return toprivateduring the phase-two interface work below.Placement decisions
try_copy_to_clipboardstays inmod.rs— it's crate-level public API, re-exported bylib.rs(pub use app::try_copy_to_clipboard).tick_rustle_pose,cycle_agent_mode,apply_theme, …) stay near the constructor for now; their callers live insideapp, and regrouping them belongs with the interface work.views.rsmirrors the original file's conflation; splitting it out is flagged for phase two.Known follow-up (phase two, separate PRs)
Honest caveats this PR does not hide:
keys.rsremains the largest file —handle_key_eventalone is ~2,300 lines and is the next target.Appstate; phase two replaces&mut self-everywhere with narrow interfaces and encapsulated state, and privatizes the ~31 lazy lifts.cargo fmtstays advisory per repo policy — import ordering here matches the crate's existing conventions (messages/, etc.); fmt drift is at parity with upstream (156 diffs pre-split → 171 across the same volume).Verification (matches CI gates)
cargo check --workspace --locked✅cargo test --workspace --locked✅ 1,799 tests, 0 failures (tui: 708)cargo clippy --workspace --all-targets -D warnings✅ cleanapp.rsdeleted, 11 new files, +7,893 / −7,807 (the +86 is module glue)No behavior change, no test changes, no dependency changes.