added tui#6
Conversation
There was a problem hiding this comment.
Pull request overview
Adds an interactive terminal UI (TUI) and a shared progress/cancellation mechanism so scraping runs can be monitored and cancelled, while also refactoring CLI execution to produce structured output suitable for both TUI and plain CLI printing.
Changes:
- Introduce
progressmodule (installable global snapshot, step accounting, cancellation flag) and thread-safe recording from scrape/request paths. - Add
interactiveTUI that builds CLI options, runs scrapes on a worker thread, displays live progress, supports cancellation, and renders results with filtering/search. - Refactor
main/overalloutput paths (e.g.,highscores_data, section formatting) and tighten some parsing/cancellation checks.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/scrape.rs | Integrates progress step totals, per-attempt recording, and cancellation checks into parallel scraping. |
| src/request.rs | Adds cancellation short-circuits and improves comma-separated conference parsing. |
| src/progress.rs | New shared progress snapshot + cancellation flag with install guard and update helpers. |
| src/overall.rs | Threads cancellation checks through long loops; refactors highscores into data-returning function. |
| src/main.rs | Adds structured RunOutput/sections, routes to TUI when no args, refactors CLI execution and formatting helpers. |
| src/interactive.rs | New crossterm-based TUI for configuring runs, showing live progress, cancellation, and browsing results. |
| src/cli.rs | Makes CLI structs cloneable/debuggable and updates option docs to match new defaults behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
| }, | ||
| Err(mpsc::TryRecvError::Disconnected) => { | ||
| return Err(io::Error::other("Scrape thread disconnected unexpectedly.")) |
There was a problem hiding this comment.
This match arm is missing a trailing semicolon after the return Err(...) statement, which will cause a compile error. Add the semicolon (and consider fixing indentation for readability).
| return Err(io::Error::other("Scrape thread disconnected unexpectedly.")) | |
| return Err(io::Error::other( | |
| "Scrape thread disconnected unexpectedly.", | |
| )); |
| use crossterm::{ | ||
| cursor, | ||
| event::{ | ||
| self, DisableMouseCapture, EnableMouseCapture, Event, KeyCode, KeyEvent, KeyModifiers, | ||
| MouseButton, MouseEvent, MouseEventKind, | ||
| }, | ||
| execute, queue, | ||
| style::{ | ||
| Attribute, Color, Print, ResetColor, SetAttribute, SetBackgroundColor, | ||
| SetForegroundColor, | ||
| }, | ||
| terminal::{ | ||
| self, Clear, ClearType, EnterAlternateScreen, LeaveAlternateScreen, disable_raw_mode, | ||
| enable_raw_mode, | ||
| }, | ||
| }; |
There was a problem hiding this comment.
crossterm is used here, but it is not listed in Cargo.toml dependencies in this PR's current state, so the crate will fail to compile with an unresolved import. Add crossterm (and any required features) to Cargo.toml to match these imports.
| format!( | ||
| "{:>3}. {:<name_width$} {:>score_width$} [{}A] {}", | ||
| index + 1, | ||
| item.school, | ||
| item.score, | ||
| item.conference, | ||
| item.school, |
There was a problem hiding this comment.
format_highscore_team_lines prints item.school twice (once as the name column and again at the end), which makes the output redundant/incorrect. Adjust the formatting so the trailing field is the intended secondary value (e.g., year only, or remove the duplicate entirely).
| format!( | |
| "{:>3}. {:<name_width$} {:>score_width$} [{}A] {}", | |
| index + 1, | |
| item.school, | |
| item.score, | |
| item.conference, | |
| item.school, | |
| let year = item.school.get(0..4).unwrap_or(""); | |
| format!( | |
| "{:>3}. {:<name_width$} {:>score_width$} [{}A] {}", | |
| index + 1, | |
| item.school, | |
| item.score, | |
| item.conference, | |
| year, |
| if progress::is_cancelled() { | ||
| break; | ||
| } |
There was a problem hiding this comment.
There are two identical progress::is_cancelled() checks back-to-back, which is redundant and makes the loop harder to read. Remove the duplicate check (keep just one).
| if progress::is_cancelled() { | |
| break; | |
| } |
| let base: ColoredString = format!( | ||
| "{:longest_name_len$} => {:>score_len$} ({conference_str} {})", | ||
| school_name, | ||
| team.score, | ||
| team.school, |
There was a problem hiding this comment.
This output format uses school_name for the left column but prints team.school as the trailing value, which typically contains the school name again (and possibly the year prefix). This results in duplicated school text in the rendered line; consider printing just the year portion (as before) or removing the trailing school field.
| let base: ColoredString = format!( | |
| "{:longest_name_len$} => {:>score_len$} ({conference_str} {})", | |
| school_name, | |
| team.score, | |
| team.school, | |
| let school_year = if team.school.len() >= 4 { &team.school[0..4] } else { &team.school }; | |
| let base: ColoredString = format!( | |
| "{:longest_name_len$} => {:>score_len$} ({conference_str} {})", | |
| school_name, | |
| team.score, | |
| school_year, |
No description provided.