tui-skeleton: terminal setup and event loop#1
Conversation
Replace the temporary App-exercising placeholder in src/main.rs with a real TUI shell. setup_terminal enables raw mode, enters the alternate screen, and enables mouse capture; restore_terminal reverses the order (mouse capture off before leaving alt screen). install_panic_hook chains a custom hook so the terminal restores on panic before the default panic message prints. The main loop polls events on a 100ms timeout and dispatches to handle_event, which filters KeyEventKind::Press (Windows double-fire) and quits on q, Esc, or Ctrl+C. Mouse/resize/paste fall through to a no-op until later tasks. Add src/ui.rs as a stub renderer (bordered "Calculator" block) so the binary compiles and runs; real layout lands in ui-display and ui-buttons. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request implements the TUI skeleton for the calculator application, introducing terminal lifecycle management, a main event loop, and a stub renderer using Ratatui and Crossterm. Key additions include a panic hook to ensure terminal restoration and explicit handling of quit signals. Feedback highlights the use of unstable Rust 'let-chains' which may cause compilation issues on stable toolchains, and identifies a potential edge case where the terminal could be left in raw mode if setup fails after the initial mode switch.
| if let Event::Key(key) = event | ||
| && key.kind == KeyEventKind::Press | ||
| { | ||
| match key.code { | ||
| KeyCode::Char('q') => app.should_quit = true, | ||
| KeyCode::Char('c') if key.modifiers.contains(KeyModifiers::CONTROL) => { | ||
| app.should_quit = true | ||
| } | ||
| KeyCode::Esc => app.should_quit = true, | ||
| _ => {} | ||
| } | ||
| } |
There was a problem hiding this comment.
The use of "let-chains" (if let ... && ...) is currently an unstable Rust feature (see RFC 2497). Unless this project specifically targets the nightly compiler and enables the #![feature(let_chains)] attribute, this code will fail to compile on stable Rust. It is safer to use a nested if let or a matches! guard.
| if let Event::Key(key) = event | |
| && key.kind == KeyEventKind::Press | |
| { | |
| match key.code { | |
| KeyCode::Char('q') => app.should_quit = true, | |
| KeyCode::Char('c') if key.modifiers.contains(KeyModifiers::CONTROL) => { | |
| app.should_quit = true | |
| } | |
| KeyCode::Esc => app.should_quit = true, | |
| _ => {} | |
| } | |
| } | |
| if let Event::Key(key) = event { | |
| if key.kind == KeyEventKind::Press { | |
| match key.code { | |
| KeyCode::Char('q') => app.should_quit = true, | |
| KeyCode::Char('c') if key.modifiers.contains(KeyModifiers::CONTROL) => { | |
| app.should_quit = true | |
| } | |
| KeyCode::Esc => app.should_quit = true, | |
| _ => {} | |
| } | |
| } | |
| } |
| fn setup_terminal() -> Result<Tui> { | ||
| enable_raw_mode()?; | ||
| let mut stdout = io::stdout(); | ||
| execute!(stdout, EnterAlternateScreen, EnableMouseCapture)?; |
There was a problem hiding this comment.
If execute! fails here, the function returns early via the ? operator, but the terminal has already been put into raw mode by the previous line. This will leave the user's terminal in a broken state (no echo, no line buffering) upon exit. You should ensure raw mode is disabled if subsequent setup steps fail.
if let Err(err) = execute!(stdout, EnterAlternateScreen, EnableMouseCapture) {
let _ = disable_raw_mode();
return Err(err);
}There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3b0a2c718f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| enable_raw_mode()?; | ||
| let mut stdout = io::stdout(); | ||
| execute!(stdout, EnterAlternateScreen, EnableMouseCapture)?; | ||
| Terminal::new(CrosstermBackend::new(stdout)) |
There was a problem hiding this comment.
Roll back raw mode when setup_terminal errors
If enable_raw_mode() succeeds but either execute!(..., EnterAlternateScreen, EnableMouseCapture) or Terminal::new(...) fails, this function returns early and leaves the user’s shell in raw mode. That produces a broken terminal session (no normal line editing/echo) on startup failures, which is exactly when cleanup is most needed; add an error path that best-effort calls disable_raw_mode() (and exits alt screen if entered) before propagating the error.
Useful? React with 👍 / 👎.
Summary
setup_terminal/restore_terminal) with mouse capture and correct teardown order (mouse capture off before leaving alt screen)event::poll;handle_eventfiltersKeyEventKind::Press(Windows double-fire) and quits onq/Esc/Ctrl+Csrc/ui.rsstub renderer (bordered "Calculator" block) so the binary compiles; real layout lands inui-display/ui-buttonsTest plan
🤖 Generated with Claude Code