Add alt-screen and key bindings to live mode#28
Merged
Conversation
There was a problem hiding this comment.
Code Review
This pull request implements a robust live mode for the weather application using the crossterm library, featuring an alternate screen buffer, an interactive event loop, and a customizable footer with update timestamps. The core application logic was refactored to separate weather data fetching and rendering, allowing for continuous updates and better error handling with provider fallbacks. Review feedback highlights opportunities to optimize performance by passing weather data by reference instead of by value and suggests improving the event loop to handle terminal resize events to prevent display corruption.
- Live mode now uses the terminal's alternate screen buffer via crossterm and restores the original screen on exit (clean exit, Ctrl+C, or panic) through a TerminalGuard RAII type. - Add interactive keys: q / Esc / Ctrl+C to quit, r to force refresh. - Add footer with key hints and last-update timestamp; suppressible via live_mode_footer = false in config or --no-footer on the CLI. - Initial fetch runs before entering the alt screen so the user's existing terminal contents stay visible during the first API call. - Refactor provider fallback into App::fetch_with_fallback method so the live path can return Result and let the guard restore the terminal before display_error's process::exit fires.
f933050 to
7976997
Compare
- Handle Event::Resize in the live mode event loop instead of dropping it via a let-chain — raw mode resize was corrupting the display until the next refresh tick - Take &Weather in formatter and render fns so the cached weather can be re-rendered without re-fetching or cloning
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Live mode now uses the terminal's alternate screen buffer with non-blocking key handling, so the user's existing terminal contents are preserved and they can quit or refresh interactively.
crosstermon first successful fetch, restored on exit (clean exit,Ctrl+C, error path, panic) by aTerminalGuardRAII type.q/Esc/Ctrl+Cquit immediately;rforces an immediate refresh.[q] quit [r] refresh • last update: HH:MM:SS). Configurable vialive_mode_footer = true|falsein TOML and--no-footeron the CLI.App::fetch_with_fallbackis now a method, used by both one-shot and live paths. The live path can now returnResultsoTerminalGuardruns before any error exit (previouslydisplay_errorcalledprocess::exit, skipping destructors).Test plan
just checkpasses — 104 tests, clippy-D pedanticcleanlive_mode_footerconfig field (defaults, explicitfalse, CLI override, CLI absent preserves config value)format_footer(color on/off) andwrite_payload(CRLF translation, 5 edge cases)WeatherFormatter::render_to_string(full text + one-line modes)cargo run -- -c "<city>" --live -i 10— verify alt-screen restore, key bindings, footer--no-footerNotes
crossterm = "0.28"(cross-platform alt-screen + raw mode + key events; no separatectrlccrate needed sinceCtrl+Carrives as a regularKeyEventin raw mode).rustormy -c <city>) is byte-for-byte identical to before — only the live path changed.\n → \r\ntranslation is required because raw mode disables the kernel'sONLCRline-discipline; verified to behave the same on Linux, macOS, and Windows console.