From 26fd97a209d936755aa653ee0110d17d27e47306 Mon Sep 17 00:00:00 2001 From: Nahor Date: Wed, 2 Oct 2024 11:45:55 -0700 Subject: [PATCH 01/15] Update all exercises during the final check The previous code run the check on all exercises but only updates one exercise (the first that failed) even if multiple failed. The user won't be able to see all the failed exercises when viewing the list, and will have to run check_all after each fixed exercise. This change will update all the exercises so the user can see all that failed, fix them all, and only then need run check_all again. --- src/app_state.rs | 143 +++++++++++++++++++++++++++++++---------------- 1 file changed, 96 insertions(+), 47 deletions(-) diff --git a/src/app_state.rs b/src/app_state.rs index c879955f..de5a3828 100644 --- a/src/app_state.rs +++ b/src/app_state.rs @@ -35,10 +35,12 @@ pub enum StateFileStatus { NotRead, } -enum AllExercisesCheck { - Pending(usize), - AllDone, - CheckedUntil(usize), +#[derive(Clone, Copy, PartialEq)] +enum AllExercisesResult { + Pending, + Success, + Failed, + Error, } pub struct AppState { @@ -270,18 +272,32 @@ impl AppState { self.write() } - pub fn set_pending(&mut self, exercise_ind: usize) -> Result<()> { + // Set the status of an exercise without saving. Returns `true` if the + // status actually changed (and thus needs saving later) + pub fn set_status(&mut self, exercise_ind: usize, done: bool) -> Result { let exercise = self .exercises .get_mut(exercise_ind) .context(BAD_INDEX_ERR)?; - if exercise.done { - exercise.done = false; - self.n_done -= 1; + if exercise.done == done { + Ok(false) + } else { + exercise.done = done; + if done { + self.n_done += 1; + } else { + self.n_done -= 1; + } + Ok(true) + } + } + + // Set the status of an exercise to "pending" and save + pub fn set_pending(&mut self, exercise_ind: usize) -> Result<()> { + if self.set_status(exercise_ind, false)? { self.write()?; } - Ok(()) } @@ -380,11 +396,11 @@ impl AppState { } // Return the exercise index of the first pending exercise found. - fn check_all_exercises(&self, stdout: &mut StdoutLock) -> Result> { + fn check_all_exercises(&mut self, stdout: &mut StdoutLock) -> Result> { stdout.write_all(FINAL_CHECK_MSG)?; let n_exercises = self.exercises.len(); - let status = thread::scope(|s| { + let (mut checked_count, mut results) = thread::scope(|s| { let handles = self .exercises .iter() @@ -394,48 +410,83 @@ impl AppState { }) .collect::>(); + let mut results = vec![AllExercisesResult::Pending; n_exercises]; + let mut checked_count = 0; for (exercise_ind, spawn_res) in handles.into_iter().enumerate() { - write!(stdout, "\rProgress: {exercise_ind}/{n_exercises}")?; + write!(stdout, "\rProgress: {checked_count}/{n_exercises}")?; stdout.flush()?; - let Ok(handle) = spawn_res else { - return Ok(AllExercisesCheck::CheckedUntil(exercise_ind)); - }; - - let Ok(success) = handle.join().unwrap() else { - return Ok(AllExercisesCheck::CheckedUntil(exercise_ind)); - }; - - if !success { - return Ok(AllExercisesCheck::Pending(exercise_ind)); - } + results[exercise_ind] = spawn_res + .context("Spawn error") + .and_then(|handle| handle.join().unwrap()) + .map_or_else( + |_| AllExercisesResult::Error, + |success| { + checked_count += 1; + if success { + AllExercisesResult::Success + } else { + AllExercisesResult::Failed + } + }, + ); } - Ok::<_, io::Error>(AllExercisesCheck::AllDone) + Ok::<_, io::Error>((checked_count, results)) })?; - let mut exercise_ind = match status { - AllExercisesCheck::Pending(exercise_ind) => return Ok(Some(exercise_ind)), - AllExercisesCheck::AllDone => return Ok(None), - AllExercisesCheck::CheckedUntil(ind) => ind, - }; + // If we got an error while checking all exercises in parallel, + // it could be because we exceeded the limit of open file descriptors. + // Therefore, re-try those one at a time (i.e. sequentially). + results + .iter_mut() + .enumerate() + .filter(|(_, result)| { + **result == AllExercisesResult::Pending || **result == AllExercisesResult::Error + }) + .try_for_each(|(exercise_ind, result)| { + let exercise = self.exercises.get(exercise_ind).context(BAD_INDEX_ERR)?; + *result = match exercise + .run_exercise(None, &self.cmd_runner) + .context("Sequential retry") + { + Ok(true) => AllExercisesResult::Success, + Ok(false) => AllExercisesResult::Failed, + Err(err) => bail!(err), + }; + checked_count += 1; + write!(stdout, "\rProgress: {checked_count}/{n_exercises}")?; + stdout.flush()?; + Ok(()) + })?; - // We got an error while checking all exercises in parallel. - // This could be because we exceeded the limit of open file descriptors. - // Therefore, try to continue the check sequentially. - for exercise in &self.exercises[exercise_ind..] { - write!(stdout, "\rProgress: {exercise_ind}/{n_exercises}")?; - stdout.flush()?; + // Update the state of each exercise and return the first that failed + let first_fail = results + .iter() + .enumerate() + .filter_map(|(exercise_ind, result)| { + match result { + AllExercisesResult::Success => self + .set_status(exercise_ind, true) + .map_or_else(|err| Some(Err(err)), |_| None), + AllExercisesResult::Failed => self + .set_status(exercise_ind, false) + .map_or_else(|err| Some(Err(err)), |_| Some(Ok(exercise_ind))), + // The sequential check done earlier will have converted all + // exercises to Success/Failed, or bailed, so those are unreachable + AllExercisesResult::Pending | AllExercisesResult::Error => unreachable!(), + } + }) + .try_fold(None::, |current_min, index| { + match (current_min, index) { + (_, Err(err)) => Err(err), + (None, Ok(index)) => Ok(Some(index)), + (Some(current_min), Ok(index)) => Ok(Some(current_min.min(index))), + } + })?; + self.write()?; - let success = exercise.run_exercise(None, &self.cmd_runner)?; - if !success { - return Ok(Some(exercise_ind)); - } - - exercise_ind += 1; - } - - Ok(None) + Ok(first_fail) } /// Mark the current exercise as done and move on to the next pending exercise if one exists. @@ -467,9 +518,7 @@ impl AppState { self.current_exercise_ind = pending_exercise_ind; self.exercises[pending_exercise_ind].done = false; - // All exercises were marked as done. - self.n_done -= 1; - self.write()?; + return Ok(ExercisesProgress::NewPending); } From c52867eb8bf69f67e702b87dd2bf12125aa7ab12 Mon Sep 17 00:00:00 2001 From: Nahor Date: Wed, 2 Oct 2024 13:40:32 -0700 Subject: [PATCH 02/15] Add command to check all the exercises This allows for skipping repeating "next" when multiple exercises are done at once, or when earlier exercises have been updated/changed (and thus must be redone) while still working of the whole set (i.e. the final check_all is not yet available to flag those undone exercises) --- src/app_state.rs | 24 ++++++++++++++++++++---- src/watch.rs | 7 +++++++ src/watch/state.rs | 22 ++++++++++++++++++++++ src/watch/terminal_event.rs | 2 ++ 4 files changed, 51 insertions(+), 4 deletions(-) diff --git a/src/app_state.rs b/src/app_state.rs index de5a3828..99772b7b 100644 --- a/src/app_state.rs +++ b/src/app_state.rs @@ -396,8 +396,16 @@ impl AppState { } // Return the exercise index of the first pending exercise found. - fn check_all_exercises(&mut self, stdout: &mut StdoutLock) -> Result> { - stdout.write_all(FINAL_CHECK_MSG)?; + pub fn check_all_exercises( + &mut self, + stdout: &mut StdoutLock, + final_check: bool, + ) -> Result> { + if !final_check { + stdout.write_all(INTERMEDIATE_CHECK_MSG)?; + } else { + stdout.write_all(FINAL_CHECK_MSG)?; + } let n_exercises = self.exercises.len(); let (mut checked_count, mut results) = thread::scope(|s| { @@ -513,7 +521,7 @@ impl AppState { stdout.write_all(b"\n")?; } - if let Some(pending_exercise_ind) = self.check_all_exercises(stdout)? { + if let Some(pending_exercise_ind) = self.check_all_exercises(stdout, true)? { stdout.write_all(b"\n\n")?; self.current_exercise_ind = pending_exercise_ind; @@ -525,6 +533,12 @@ impl AppState { // Write that the last exercise is done. self.write()?; + self.render_final_message(stdout)?; + + Ok(ExercisesProgress::AllDone) + } + + pub fn render_final_message(&self, stdout: &mut StdoutLock) -> Result<()> { clear_terminal(stdout)?; stdout.write_all(FENISH_LINE.as_bytes())?; @@ -534,12 +548,14 @@ impl AppState { stdout.write_all(b"\n")?; } - Ok(ExercisesProgress::AllDone) + Ok(()) } } const BAD_INDEX_ERR: &str = "The current exercise index is higher than the number of exercises"; const STATE_FILE_HEADER: &[u8] = b"DON'T EDIT THIS FILE!\n\n"; +const INTERMEDIATE_CHECK_MSG: &[u8] = b"Checking all exercises +"; const FINAL_CHECK_MSG: &[u8] = b"All exercises seem to be done. Recompiling and running all exercises to make sure that all of them are actually done. "; diff --git a/src/watch.rs b/src/watch.rs index 35533b02..b9846757 100644 --- a/src/watch.rs +++ b/src/watch.rs @@ -103,6 +103,13 @@ fn run_watch( WatchEvent::Input(InputEvent::Run) => watch_state.run_current_exercise(&mut stdout)?, WatchEvent::Input(InputEvent::Hint) => watch_state.show_hint(&mut stdout)?, WatchEvent::Input(InputEvent::List) => return Ok(WatchExit::List), + WatchEvent::Input(InputEvent::CheckAll) => match watch_state + .check_all_exercises(&mut stdout)? + { + ExercisesProgress::AllDone => break, + ExercisesProgress::NewPending => watch_state.run_current_exercise(&mut stdout)?, + ExercisesProgress::CurrentPending => (), + }, WatchEvent::Input(InputEvent::Reset) => watch_state.reset_exercise(&mut stdout)?, WatchEvent::Input(InputEvent::Quit) => { stdout.write_all(QUIT_MSG)?; diff --git a/src/watch/state.rs b/src/watch/state.rs index 19910f03..67a63579 100644 --- a/src/watch/state.rs +++ b/src/watch/state.rs @@ -195,6 +195,11 @@ impl<'a> WatchState<'a> { stdout.queue(ResetColor)?; stdout.write_all(b":list / ")?; + stdout.queue(SetAttribute(Attribute::Bold))?; + stdout.write_all(b"c")?; + stdout.queue(ResetColor)?; + stdout.write_all(b":check all / ")?; + stdout.queue(SetAttribute(Attribute::Bold))?; stdout.write_all(b"x")?; stdout.queue(ResetColor)?; @@ -274,6 +279,23 @@ impl<'a> WatchState<'a> { Ok(()) } + pub fn check_all_exercises(&mut self, stdout: &mut StdoutLock) -> Result { + stdout.write_all(b"\n")?; + + if let Some(first_fail) = self.app_state.check_all_exercises(stdout, false)? { + // Only change exercise if the current one is done... + if self.app_state.current_exercise().done { + self.app_state.set_current_exercise_ind(first_fail)?; + } + // ...but always pretend it's a "new" anyway because that refreshes + // the display + Ok(ExercisesProgress::NewPending) + } else { + self.app_state.render_final_message(stdout)?; + Ok(ExercisesProgress::AllDone) + } + } + pub fn update_term_width(&mut self, width: u16, stdout: &mut StdoutLock) -> io::Result<()> { if self.term_width != width { self.term_width = width; diff --git a/src/watch/terminal_event.rs b/src/watch/terminal_event.rs index 1ed681dc..48411db0 100644 --- a/src/watch/terminal_event.rs +++ b/src/watch/terminal_event.rs @@ -11,6 +11,7 @@ pub enum InputEvent { Run, Hint, List, + CheckAll, Reset, Quit, } @@ -37,6 +38,7 @@ pub fn terminal_event_handler( KeyCode::Char('r') if manual_run => InputEvent::Run, KeyCode::Char('h') => InputEvent::Hint, KeyCode::Char('l') => break WatchEvent::Input(InputEvent::List), + KeyCode::Char('c') => InputEvent::CheckAll, KeyCode::Char('x') => { if sender.send(WatchEvent::Input(InputEvent::Reset)).is_err() { return; From 5c17abd1bf52d1222f6574d38a56a29ca0e7696f Mon Sep 17 00:00:00 2001 From: Nahor Date: Wed, 2 Oct 2024 14:10:26 -0700 Subject: [PATCH 03/15] Use a channel to update the check_all progress The previous code was checking the threads in the order they were created. So the progress update would be blocked on an earlier thread even if later thread were already done. Add to that that multiple instances of `cargo build` cannot run in parallel, they will be serialized instead. So if the exercises needs to be recompiled, depending on the order those `cargo build` are run, the first update can be a long time coming. So instead of relying on the thread terminating, use a channel to get notified when an exercise check is done, regardless of the order they finish in. --- src/app_state.rs | 55 ++++++++++++++++++++++++++++-------------------- 1 file changed, 32 insertions(+), 23 deletions(-) diff --git a/src/app_state.rs b/src/app_state.rs index 99772b7b..28226d64 100644 --- a/src/app_state.rs +++ b/src/app_state.rs @@ -5,6 +5,7 @@ use std::{ io::{self, Read, Seek, StdoutLock, Write}, path::{Path, MAIN_SEPARATOR_STR}, process::{Command, Stdio}, + sync::mpsc, thread, }; @@ -409,35 +410,43 @@ impl AppState { let n_exercises = self.exercises.len(); let (mut checked_count, mut results) = thread::scope(|s| { - let handles = self - .exercises + let (tx, rx) = mpsc::channel(); + + self.exercises .iter() - .map(|exercise| { - thread::Builder::new() - .spawn_scoped(s, || exercise.run_exercise(None, &self.cmd_runner)) - }) - .collect::>(); + .enumerate() + .for_each(|(index, exercise)| { + let tx = tx.clone(); + let cmd_runner = &self.cmd_runner; + let _ = thread::Builder::new().spawn_scoped(s, move || { + tx.send((index, exercise.run_exercise(None, cmd_runner))) + }); + }); + + // Drop this `tx`, since the `rx` loop will not stop while there is + // at least one tx alive (i.e. we want the loop to block only while + // there are `tx` clones, i.e. threads) + drop(tx); let mut results = vec![AllExercisesResult::Pending; n_exercises]; let mut checked_count = 0; - for (exercise_ind, spawn_res) in handles.into_iter().enumerate() { + write!(stdout, "\rProgress: {checked_count}/{n_exercises}")?; + stdout.flush()?; + while let Ok((exercise_ind, result)) = rx.recv() { + results[exercise_ind] = result.map_or_else( + |_| AllExercisesResult::Error, + |success| { + checked_count += 1; + if success { + AllExercisesResult::Success + } else { + AllExercisesResult::Failed + } + }, + ); + write!(stdout, "\rProgress: {checked_count}/{n_exercises}")?; stdout.flush()?; - - results[exercise_ind] = spawn_res - .context("Spawn error") - .and_then(|handle| handle.join().unwrap()) - .map_or_else( - |_| AllExercisesResult::Error, - |success| { - checked_count += 1; - if success { - AllExercisesResult::Success - } else { - AllExercisesResult::Failed - } - }, - ); } Ok::<_, io::Error>((checked_count, results)) From e2f7734f37394097f330c4073bf7784500afdc9d Mon Sep 17 00:00:00 2001 From: Nahor Date: Wed, 2 Oct 2024 14:42:50 -0700 Subject: [PATCH 04/15] Limit the amount of parallelism in check_all Don't create more threads than there are CPU cores. --- src/app_state.rs | 35 +++++++++++++++++++++++++---------- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/src/app_state.rs b/src/app_state.rs index 28226d64..ec791883 100644 --- a/src/app_state.rs +++ b/src/app_state.rs @@ -5,7 +5,7 @@ use std::{ io::{self, Read, Seek, StdoutLock, Write}, path::{Path, MAIN_SEPARATOR_STR}, process::{Command, Stdio}, - sync::mpsc, + sync::{atomic::AtomicUsize, mpsc, Arc}, thread, }; @@ -20,6 +20,7 @@ use crate::{ }; const STATE_FILE_NAME: &str = ".rustlings-state.txt"; +const DEFAULT_CHECK_PARALLELISM: usize = 8; #[must_use] pub enum ExercisesProgress { @@ -411,17 +412,31 @@ impl AppState { let (mut checked_count, mut results) = thread::scope(|s| { let (tx, rx) = mpsc::channel(); + let exercise_ind = Arc::new(AtomicUsize::default()); - self.exercises - .iter() - .enumerate() - .for_each(|(index, exercise)| { - let tx = tx.clone(); - let cmd_runner = &self.cmd_runner; - let _ = thread::Builder::new().spawn_scoped(s, move || { - tx.send((index, exercise.run_exercise(None, cmd_runner))) - }); + let num_core = thread::available_parallelism() + .map_or(DEFAULT_CHECK_PARALLELISM, |count| count.get()); + (0..num_core).for_each(|_| { + let tx = tx.clone(); + let exercise_ind = exercise_ind.clone(); + let this = &self; + let _ = thread::Builder::new().spawn_scoped(s, move || { + loop { + let exercise_ind = + exercise_ind.fetch_add(1, std::sync::atomic::Ordering::AcqRel); + let Some(exercise) = this.exercises.get(exercise_ind) else { + // No more exercises + break; + }; + if tx + .send((exercise_ind, exercise.run_exercise(None, &this.cmd_runner))) + .is_err() + { + break; + } + } }); + }); // Drop this `tx`, since the `rx` loop will not stop while there is // at least one tx alive (i.e. we want the loop to block only while From aa83fd6bc46167477447ee9b95d21e551e163411 Mon Sep 17 00:00:00 2001 From: Nahor Date: Wed, 2 Oct 2024 15:28:42 -0700 Subject: [PATCH 05/15] Show a progress bar when running check_all Replace the "Progress: xxx/yyy" with a progress bar when checking all the exercises --- src/app_state.rs | 97 ++++++++++++++++++++++++++++++++++++------------ src/term.rs | 75 ++++++++++++++++++++++++++++++++----- 2 files changed, 140 insertions(+), 32 deletions(-) diff --git a/src/app_state.rs b/src/app_state.rs index ec791883..f4cc1804 100644 --- a/src/app_state.rs +++ b/src/app_state.rs @@ -1,4 +1,9 @@ use anyhow::{bail, Context, Result}; +use crossterm::{ + queue, + style::{Print, ResetColor, SetForegroundColor}, + terminal, +}; use std::{ env, fs::{File, OpenOptions}, @@ -16,7 +21,7 @@ use crate::{ embedded::EMBEDDED_FILES, exercise::{Exercise, RunnableExercise}, info_file::ExerciseInfo, - term, + term::{self, progress_bar_with_success}, }; const STATE_FILE_NAME: &str = ".rustlings-state.txt"; @@ -428,10 +433,16 @@ impl AppState { // No more exercises break; }; - if tx - .send((exercise_ind, exercise.run_exercise(None, &this.cmd_runner))) - .is_err() - { + + // Notify the progress bar that this exercise is pending + if tx.send((exercise_ind, None)).is_err() { + break; + }; + + let result = exercise.run_exercise(None, &this.cmd_runner); + + // Notify the progress bar that this exercise is done + if tx.send((exercise_ind, Some(result))).is_err() { break; } } @@ -443,28 +454,68 @@ impl AppState { // there are `tx` clones, i.e. threads) drop(tx); - let mut results = vec![AllExercisesResult::Pending; n_exercises]; - let mut checked_count = 0; - write!(stdout, "\rProgress: {checked_count}/{n_exercises}")?; - stdout.flush()?; - while let Ok((exercise_ind, result)) = rx.recv() { - results[exercise_ind] = result.map_or_else( - |_| AllExercisesResult::Error, - |success| { - checked_count += 1; - if success { - AllExercisesResult::Success - } else { - AllExercisesResult::Failed - } - }, - ); + // Print the legend + queue!( + stdout, + Print("Color legend: "), + SetForegroundColor(term::PROGRESS_FAILED_COLOR), + Print("Failure"), + ResetColor, + Print(" - "), + SetForegroundColor(term::PROGRESS_SUCCESS_COLOR), + Print("Success"), + ResetColor, + Print(" - "), + SetForegroundColor(term::PROGRESS_PENDING_COLOR), + Print("Checking"), + ResetColor, + Print("\n"), + ) + .unwrap(); + // We expect at least a few "pending" notifications shortly, so don't + // bother printing the initial state of the progress bar and flushing + // stdout - write!(stdout, "\rProgress: {checked_count}/{n_exercises}")?; + let line_width = terminal::size().unwrap().0; + let mut results = vec![AllExercisesResult::Pending; n_exercises]; + let mut pending = 0; + let mut success = 0; + let mut failed = 0; + + while let Ok((exercise_ind, result)) = rx.recv() { + match result { + None => { + pending += 1; + } + Some(Err(_)) => { + results[exercise_ind] = AllExercisesResult::Error; + } + Some(Ok(true)) => { + results[exercise_ind] = AllExercisesResult::Success; + pending -= 1; + success += 1; + } + Some(Ok(false)) => { + results[exercise_ind] = AllExercisesResult::Failed; + pending -= 1; + failed += 1; + } + } + + write!(stdout, "\r").unwrap(); + progress_bar_with_success( + stdout, + pending, + failed, + success, + n_exercises as u16, + line_width, + ) + .unwrap(); stdout.flush()?; } - Ok::<_, io::Error>((checked_count, results)) + Ok::<_, io::Error>((success, results)) })?; // If we got an error while checking all exercises in parallel, diff --git a/src/term.rs b/src/term.rs index 5b557ecf..67ace4b8 100644 --- a/src/term.rs +++ b/src/term.rs @@ -9,6 +9,10 @@ use std::{ io::{self, BufRead, StdoutLock, Write}, }; +pub const PROGRESS_FAILED_COLOR: Color = Color::Red; +pub const PROGRESS_SUCCESS_COLOR: Color = Color::Green; +pub const PROGRESS_PENDING_COLOR: Color = Color::Blue; + pub struct MaxLenWriter<'a, 'b> { pub stdout: &'a mut StdoutLock<'b>, len: usize, @@ -85,15 +89,26 @@ impl<'a> CountedWrite<'a> for StdoutLock<'a> { } } -/// Terminal progress bar to be used when not using Ratataui. +/// Simple terminal progress bar pub fn progress_bar<'a>( writer: &mut impl CountedWrite<'a>, progress: u16, total: u16, line_width: u16, +) -> io::Result<()> { + progress_bar_with_success(writer, 0, 0, progress, total, line_width) +} +/// Terminal progress bar with three states (pending + failed + success) +pub fn progress_bar_with_success<'a>( + writer: &mut impl CountedWrite<'a>, + pending: u16, + failed: u16, + success: u16, + total: u16, + line_width: u16, ) -> io::Result<()> { debug_assert!(total < 1000); - debug_assert!(progress <= total); + debug_assert!((pending + failed + success) <= total); const PREFIX: &[u8] = b"Progress: ["; const PREFIX_WIDTH: u16 = PREFIX.len() as u16; @@ -104,25 +119,67 @@ pub fn progress_bar<'a>( if line_width < MIN_LINE_WIDTH { writer.write_ascii(b"Progress: ")?; // Integers are in ASCII. - return writer.write_ascii(format!("{progress}/{total}").as_bytes()); + return writer.write_ascii(format!("{}/{total}", failed + success).as_bytes()); } let stdout = writer.stdout(); stdout.write_all(PREFIX)?; let width = line_width - WRAPPER_WIDTH; - let filled = (width * progress) / total; + let mut failed_end = (width * failed) / total; + let mut success_end = (width * (failed + success)) / total; + let mut pending_end = (width * (failed + success + pending)) / total; - stdout.queue(SetForegroundColor(Color::Green))?; - for _ in 0..filled { + // In case the range boundaries overlap, "pending" has priority over both + // "failed" and "success" (don't show the bar as "complete" when we are + // still checking some things). + // "Failed" has priority over "success" (don't show 100% success if we + // have some failures, at the risk of showing 100% failures even with + // a few successes). + // + // "Failed" already has priority over "success" because it's displayed + // first. But "pending" is last so we need to fix "success"/"failed". + if pending > 0 { + pending_end = pending_end.max(1); + if pending_end == success_end { + success_end -= 1; + } + if pending_end == failed_end { + failed_end -= 1; + } + + // This will replace the last character of the "pending" range with + // the arrow char ('>'). This ensures that even if the progress bar + // is filled (everything either done or pending), we'll still see + // the '>' as long as we are not fully done. + pending_end -= 1; + } + + if failed > 0 { + stdout.queue(SetForegroundColor(PROGRESS_FAILED_COLOR))?; + for _ in 0..failed_end { + stdout.write_all(b"#")?; + } + } + + stdout.queue(SetForegroundColor(PROGRESS_SUCCESS_COLOR))?; + for _ in failed_end..success_end { stdout.write_all(b"#")?; } - if filled < width { + if pending > 0 { + stdout.queue(SetForegroundColor(PROGRESS_PENDING_COLOR))?; + + for _ in success_end..pending_end { + stdout.write_all(b"#")?; + } + } + + if pending_end < width { stdout.write_all(b">")?; } - let width_minus_filled = width - filled; + let width_minus_filled = width - pending_end; if width_minus_filled > 1 { let red_part_width = width_minus_filled - 1; stdout.queue(SetForegroundColor(Color::Red))?; @@ -133,7 +190,7 @@ pub fn progress_bar<'a>( stdout.queue(SetForegroundColor(Color::Reset))?; - write!(stdout, "] {progress:>3}/{total}") + write!(stdout, "] {:>3}/{}", failed + success, total) } pub fn clear_terminal(stdout: &mut StdoutLock) -> io::Result<()> { From d3f819f86f0fd7e67e9b995034947a65961cab34 Mon Sep 17 00:00:00 2001 From: Nahor Date: Fri, 4 Oct 2024 14:36:36 -0700 Subject: [PATCH 06/15] Add command line command to check all exercises --- src/main.rs | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/src/main.rs b/src/main.rs index fe4b3dcd..d257b408 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,6 +1,10 @@ use anyhow::{bail, Context, Result}; use app_state::StateFileStatus; use clap::{Parser, Subcommand}; +use crossterm::{ + style::{Color, Print, ResetColor, SetForegroundColor}, + QueueableCommand, +}; use std::{ io::{self, IsTerminal, Write}, path::Path, @@ -47,6 +51,8 @@ enum Subcommands { /// The name of the exercise name: Option, }, + /// Run all the exercises, marking them as done or pending accordingly. + RunAll, /// Reset a single exercise Reset { /// The name of the exercise @@ -138,6 +144,36 @@ fn main() -> Result<()> { } run::run(&mut app_state)?; } + Some(Subcommands::RunAll) => { + let mut stdout = io::stdout().lock(); + if let Some(first_fail) = app_state.check_all_exercises(&mut stdout, false)? { + let pending = app_state + .exercises() + .iter() + .filter(|exercise| !exercise.done) + .count(); + if app_state.current_exercise().done { + app_state.set_current_exercise_ind(first_fail)?; + } + stdout + .queue(Print("\n"))? + .queue(SetForegroundColor(Color::Red))? + .queue(Print(format!("{pending}")))? + .queue(ResetColor)?; + if pending == 1 { + stdout.queue(Print(" exercise has some errors: "))?; + } else { + stdout.queue(Print(" exercises have errors, including "))?; + } + app_state + .current_exercise() + .terminal_file_link(&mut stdout)?; + stdout.write_all(b".\n")?; + exit(1); + } else { + app_state.render_final_message(&mut stdout)?; + } + } Some(Subcommands::Reset { name }) => { app_state.set_current_exercise_by_name(&name)?; let exercise_path = app_state.reset_current_exercise()?; From 685e069c58ef02dae65381974722315ee8c84e8b Mon Sep 17 00:00:00 2001 From: mo8it Date: Thu, 10 Oct 2024 19:43:35 +0200 Subject: [PATCH 07/15] First PR review changes --- src/app_state.rs | 318 +++++++++++++++++++++------------------------ src/main.rs | 3 +- src/term.rs | 19 +-- src/watch.rs | 2 +- src/watch/state.rs | 18 +-- 5 files changed, 171 insertions(+), 189 deletions(-) diff --git a/src/app_state.rs b/src/app_state.rs index f4cc1804..7540181c 100644 --- a/src/app_state.rs +++ b/src/app_state.rs @@ -1,16 +1,18 @@ -use anyhow::{bail, Context, Result}; +use anyhow::{bail, Context, Error, Result}; use crossterm::{ - queue, - style::{Print, ResetColor, SetForegroundColor}, - terminal, + style::{ResetColor, SetForegroundColor}, + terminal, QueueableCommand, }; use std::{ env, fs::{File, OpenOptions}, - io::{self, Read, Seek, StdoutLock, Write}, + io::{Read, Seek, StdoutLock, Write}, path::{Path, MAIN_SEPARATOR_STR}, process::{Command, Stdio}, - sync::{atomic::AtomicUsize, mpsc, Arc}, + sync::{ + atomic::{AtomicUsize, Ordering::Relaxed}, + mpsc, + }, thread, }; @@ -42,11 +44,17 @@ pub enum StateFileStatus { NotRead, } -#[derive(Clone, Copy, PartialEq)] -enum AllExercisesResult { +enum ExerciseCheckProgress { + Checking, + Done, + Pending, + Error, +} + +#[derive(Clone, Copy)] +enum ExerciseCheckResult { + Done, Pending, - Success, - Failed, Error, } @@ -280,7 +288,7 @@ impl AppState { } // Set the status of an exercise without saving. Returns `true` if the - // status actually changed (and thus needs saving later) + // status actually changed (and thus needs saving later). pub fn set_status(&mut self, exercise_ind: usize, done: bool) -> Result { let exercise = self .exercises @@ -288,23 +296,25 @@ impl AppState { .context(BAD_INDEX_ERR)?; if exercise.done == done { - Ok(false) - } else { - exercise.done = done; - if done { - self.n_done += 1; - } else { - self.n_done -= 1; - } - Ok(true) + return Ok(false); } + + exercise.done = done; + if done { + self.n_done += 1; + } else { + self.n_done -= 1; + } + + Ok(true) } - // Set the status of an exercise to "pending" and save + // Set the status of an exercise to "pending" and save. pub fn set_pending(&mut self, exercise_ind: usize) -> Result<()> { if self.set_status(exercise_ind, false)? { self.write()?; } + Ok(()) } @@ -403,173 +413,154 @@ impl AppState { } // Return the exercise index of the first pending exercise found. - pub fn check_all_exercises( - &mut self, - stdout: &mut StdoutLock, - final_check: bool, - ) -> Result> { - if !final_check { - stdout.write_all(INTERMEDIATE_CHECK_MSG)?; - } else { - stdout.write_all(FINAL_CHECK_MSG)?; - } - let n_exercises = self.exercises.len(); + pub fn check_all_exercises(&mut self, stdout: &mut StdoutLock) -> Result> { + stdout.write_all("Checking all exercises…\n".as_bytes())?; + let n_exercises = self.exercises.len() as u16; + let next_exercise_ind = AtomicUsize::new(0); + let term_width = terminal::size() + .context("Failed to get the terminal size")? + .0; - let (mut checked_count, mut results) = thread::scope(|s| { - let (tx, rx) = mpsc::channel(); - let exercise_ind = Arc::new(AtomicUsize::default()); + let mut results = vec![ExerciseCheckResult::Error; self.exercises.len()]; + let mut done = 0; + let mut pending = 0; - let num_core = thread::available_parallelism() + thread::scope(|s| { + let mut checking = 0; + let (exercise_result_sender, exercise_result_receiver) = mpsc::channel(); + let n_threads = thread::available_parallelism() .map_or(DEFAULT_CHECK_PARALLELISM, |count| count.get()); - (0..num_core).for_each(|_| { - let tx = tx.clone(); - let exercise_ind = exercise_ind.clone(); - let this = &self; - let _ = thread::Builder::new().spawn_scoped(s, move || { - loop { - let exercise_ind = - exercise_ind.fetch_add(1, std::sync::atomic::Ordering::AcqRel); - let Some(exercise) = this.exercises.get(exercise_ind) else { - // No more exercises + + for _ in 0..n_threads { + let exercise_result_sender = exercise_result_sender.clone(); + let next_exercise_ind = &next_exercise_ind; + let slf = &self; + thread::Builder::new() + .spawn_scoped(s, move || loop { + let exercise_ind = next_exercise_ind.fetch_add(1, Relaxed); + let Some(exercise) = slf.exercises.get(exercise_ind) else { + // No more exercises. break; }; - // Notify the progress bar that this exercise is pending - if tx.send((exercise_ind, None)).is_err() { + // Notify the progress bar that this exercise is pending. + if exercise_result_sender + .send((exercise_ind, ExerciseCheckProgress::Checking)) + .is_err() + { break; }; - let result = exercise.run_exercise(None, &this.cmd_runner); + let success = exercise.run_exercise(None, &slf.cmd_runner); + let result = match success { + Ok(true) => ExerciseCheckProgress::Done, + Ok(false) => ExerciseCheckProgress::Pending, + Err(_) => ExerciseCheckProgress::Error, + }; - // Notify the progress bar that this exercise is done - if tx.send((exercise_ind, Some(result))).is_err() { + // Notify the progress bar that this exercise is done. + if exercise_result_sender.send((exercise_ind, result)).is_err() { break; } - } - }); - }); + }) + .context("Failed to spawn a thread to check all exercises")?; + } - // Drop this `tx`, since the `rx` loop will not stop while there is - // at least one tx alive (i.e. we want the loop to block only while - // there are `tx` clones, i.e. threads) - drop(tx); + // Drop this sender to detect when the last thread is done. + drop(exercise_result_sender); - // Print the legend - queue!( - stdout, - Print("Color legend: "), - SetForegroundColor(term::PROGRESS_FAILED_COLOR), - Print("Failure"), - ResetColor, - Print(" - "), - SetForegroundColor(term::PROGRESS_SUCCESS_COLOR), - Print("Success"), - ResetColor, - Print(" - "), - SetForegroundColor(term::PROGRESS_PENDING_COLOR), - Print("Checking"), - ResetColor, - Print("\n"), - ) - .unwrap(); - // We expect at least a few "pending" notifications shortly, so don't - // bother printing the initial state of the progress bar and flushing - // stdout + // Print the legend. + stdout.write_all(b"Color legend: ")?; + stdout.queue(SetForegroundColor(term::PROGRESS_FAILED_COLOR))?; + stdout.write_all(b"Pending")?; + stdout.queue(ResetColor)?; + stdout.write_all(b" - ")?; + stdout.queue(SetForegroundColor(term::PROGRESS_SUCCESS_COLOR))?; + stdout.write_all(b"Done")?; + stdout.queue(ResetColor)?; + stdout.write_all(b" - ")?; + stdout.queue(SetForegroundColor(term::PROGRESS_PENDING_COLOR))?; + stdout.write_all(b"Checking")?; + stdout.queue(ResetColor)?; + stdout.write_all(b"\n")?; - let line_width = terminal::size().unwrap().0; - let mut results = vec![AllExercisesResult::Pending; n_exercises]; - let mut pending = 0; - let mut success = 0; - let mut failed = 0; - - while let Ok((exercise_ind, result)) = rx.recv() { + while let Ok((exercise_ind, result)) = exercise_result_receiver.recv() { match result { - None => { + ExerciseCheckProgress::Checking => checking += 1, + ExerciseCheckProgress::Done => { + results[exercise_ind] = ExerciseCheckResult::Done; + checking -= 1; + done += 1; + } + ExerciseCheckProgress::Pending => { + results[exercise_ind] = ExerciseCheckResult::Pending; + checking -= 1; pending += 1; } - Some(Err(_)) => { - results[exercise_ind] = AllExercisesResult::Error; - } - Some(Ok(true)) => { - results[exercise_ind] = AllExercisesResult::Success; - pending -= 1; - success += 1; - } - Some(Ok(false)) => { - results[exercise_ind] = AllExercisesResult::Failed; - pending -= 1; - failed += 1; - } + ExerciseCheckProgress::Error => checking -= 1, } - write!(stdout, "\r").unwrap(); + stdout.write_all(b"\r")?; progress_bar_with_success( stdout, + checking, pending, - failed, - success, - n_exercises as u16, - line_width, - ) - .unwrap(); + done, + n_exercises, + term_width, + )?; stdout.flush()?; } - Ok::<_, io::Error>((success, results)) + Ok::<_, Error>(()) })?; - // If we got an error while checking all exercises in parallel, - // it could be because we exceeded the limit of open file descriptors. - // Therefore, re-try those one at a time (i.e. sequentially). - results - .iter_mut() - .enumerate() - .filter(|(_, result)| { - **result == AllExercisesResult::Pending || **result == AllExercisesResult::Error - }) - .try_for_each(|(exercise_ind, result)| { - let exercise = self.exercises.get(exercise_ind).context(BAD_INDEX_ERR)?; - *result = match exercise - .run_exercise(None, &self.cmd_runner) - .context("Sequential retry") - { - Ok(true) => AllExercisesResult::Success, - Ok(false) => AllExercisesResult::Failed, - Err(err) => bail!(err), - }; - checked_count += 1; - write!(stdout, "\rProgress: {checked_count}/{n_exercises}")?; - stdout.flush()?; - Ok(()) - })?; + let mut first_pending_exercise_ind = None; + for (exercise_ind, result) in results.into_iter().enumerate() { + match result { + ExerciseCheckResult::Done => { + self.set_status(exercise_ind, true)?; + } + ExerciseCheckResult::Pending => { + self.set_status(exercise_ind, false)?; + if first_pending_exercise_ind.is_none() { + first_pending_exercise_ind = Some(exercise_ind); + } + } + ExerciseCheckResult::Error => { + // If we got an error while checking all exercises in parallel, + // it could be because we exceeded the limit of open file descriptors. + // Therefore, try running exercises with errors sequentially. + let exercise = &self.exercises[exercise_ind]; + let success = exercise.run_exercise(None, &self.cmd_runner)?; + if success { + done += 1; + } else { + pending += 1; + if first_pending_exercise_ind.is_none() { + first_pending_exercise_ind = Some(exercise_ind); + } + } + self.set_status(exercise_ind, success)?; - // Update the state of each exercise and return the first that failed - let first_fail = results - .iter() - .enumerate() - .filter_map(|(exercise_ind, result)| { - match result { - AllExercisesResult::Success => self - .set_status(exercise_ind, true) - .map_or_else(|err| Some(Err(err)), |_| None), - AllExercisesResult::Failed => self - .set_status(exercise_ind, false) - .map_or_else(|err| Some(Err(err)), |_| Some(Ok(exercise_ind))), - // The sequential check done earlier will have converted all - // exercises to Success/Failed, or bailed, so those are unreachable - AllExercisesResult::Pending | AllExercisesResult::Error => unreachable!(), + stdout.write_all(b"\r")?; + progress_bar_with_success( + stdout, + u16::from(pending + done < n_exercises), + pending, + done, + n_exercises, + term_width, + )?; + stdout.flush()?; } - }) - .try_fold(None::, |current_min, index| { - match (current_min, index) { - (_, Err(err)) => Err(err), - (None, Ok(index)) => Ok(Some(index)), - (Some(current_min), Ok(index)) => Ok(Some(current_min.min(index))), - } - })?; + } + } + self.write()?; + stdout.write_all(b"\n\n")?; - Ok(first_fail) + Ok(first_pending_exercise_ind) } /// Mark the current exercise as done and move on to the next pending exercise if one exists. @@ -596,18 +587,12 @@ impl AppState { stdout.write_all(b"\n")?; } - if let Some(pending_exercise_ind) = self.check_all_exercises(stdout, true)? { - stdout.write_all(b"\n\n")?; - - self.current_exercise_ind = pending_exercise_ind; - self.exercises[pending_exercise_ind].done = false; + if let Some(first_pending_exercise_ind) = self.check_all_exercises(stdout)? { + self.set_current_exercise_ind(first_pending_exercise_ind)?; return Ok(ExercisesProgress::NewPending); } - // Write that the last exercise is done. - self.write()?; - self.render_final_message(stdout)?; Ok(ExercisesProgress::AllDone) @@ -629,11 +614,6 @@ impl AppState { const BAD_INDEX_ERR: &str = "The current exercise index is higher than the number of exercises"; const STATE_FILE_HEADER: &[u8] = b"DON'T EDIT THIS FILE!\n\n"; -const INTERMEDIATE_CHECK_MSG: &[u8] = b"Checking all exercises -"; -const FINAL_CHECK_MSG: &[u8] = b"All exercises seem to be done. -Recompiling and running all exercises to make sure that all of them are actually done. -"; const FENISH_LINE: &str = "+----------------------------------------------------+ | You made it to the Fe-nish line! | +-------------------------- ------------------------+ diff --git a/src/main.rs b/src/main.rs index d257b408..64b72bde 100644 --- a/src/main.rs +++ b/src/main.rs @@ -146,7 +146,7 @@ fn main() -> Result<()> { } Some(Subcommands::RunAll) => { let mut stdout = io::stdout().lock(); - if let Some(first_fail) = app_state.check_all_exercises(&mut stdout, false)? { + if let Some(first_fail) = app_state.check_all_exercises(&mut stdout)? { let pending = app_state .exercises() .iter() @@ -156,7 +156,6 @@ fn main() -> Result<()> { app_state.set_current_exercise_ind(first_fail)?; } stdout - .queue(Print("\n"))? .queue(SetForegroundColor(Color::Red))? .queue(Print(format!("{pending}")))? .queue(ResetColor)?; diff --git a/src/term.rs b/src/term.rs index 67ace4b8..31a951db 100644 --- a/src/term.rs +++ b/src/term.rs @@ -89,34 +89,35 @@ impl<'a> CountedWrite<'a> for StdoutLock<'a> { } } -/// Simple terminal progress bar +/// Simple terminal progress bar. pub fn progress_bar<'a>( writer: &mut impl CountedWrite<'a>, progress: u16, total: u16, - line_width: u16, + term_width: u16, ) -> io::Result<()> { - progress_bar_with_success(writer, 0, 0, progress, total, line_width) + progress_bar_with_success(writer, 0, 0, progress, total, term_width) } -/// Terminal progress bar with three states (pending + failed + success) + +/// Terminal progress bar with three states (pending + failed + success). pub fn progress_bar_with_success<'a>( writer: &mut impl CountedWrite<'a>, pending: u16, failed: u16, success: u16, total: u16, - line_width: u16, + term_width: u16, ) -> io::Result<()> { debug_assert!(total < 1000); - debug_assert!((pending + failed + success) <= total); + debug_assert!(pending + failed + success <= total); const PREFIX: &[u8] = b"Progress: ["; const PREFIX_WIDTH: u16 = PREFIX.len() as u16; const POSTFIX_WIDTH: u16 = "] xxx/xxx".len() as u16; const WRAPPER_WIDTH: u16 = PREFIX_WIDTH + POSTFIX_WIDTH; - const MIN_LINE_WIDTH: u16 = WRAPPER_WIDTH + 4; + const MIN_TERM_WIDTH: u16 = WRAPPER_WIDTH + 4; - if line_width < MIN_LINE_WIDTH { + if term_width < MIN_TERM_WIDTH { writer.write_ascii(b"Progress: ")?; // Integers are in ASCII. return writer.write_ascii(format!("{}/{total}", failed + success).as_bytes()); @@ -125,7 +126,7 @@ pub fn progress_bar_with_success<'a>( let stdout = writer.stdout(); stdout.write_all(PREFIX)?; - let width = line_width - WRAPPER_WIDTH; + let width = term_width - WRAPPER_WIDTH; let mut failed_end = (width * failed) / total; let mut success_end = (width * (failed + success)) / total; let mut pending_end = (width * (failed + success + pending)) / total; diff --git a/src/watch.rs b/src/watch.rs index b9846757..6259c9df 100644 --- a/src/watch.rs +++ b/src/watch.rs @@ -108,7 +108,7 @@ fn run_watch( { ExercisesProgress::AllDone => break, ExercisesProgress::NewPending => watch_state.run_current_exercise(&mut stdout)?, - ExercisesProgress::CurrentPending => (), + ExercisesProgress::CurrentPending => watch_state.render(&mut stdout)?, }, WatchEvent::Input(InputEvent::Reset) => watch_state.reset_exercise(&mut stdout)?, WatchEvent::Input(InputEvent::Quit) => { diff --git a/src/watch/state.rs b/src/watch/state.rs index 67a63579..8b58e311 100644 --- a/src/watch/state.rs +++ b/src/watch/state.rs @@ -157,8 +157,9 @@ impl<'a> WatchState<'a> { /// Move on to the next exercise if the current one is done. pub fn next_exercise(&mut self, stdout: &mut StdoutLock) -> Result { - if self.done_status == DoneStatus::Pending { - return Ok(ExercisesProgress::CurrentPending); + match self.done_status { + DoneStatus::DoneWithSolution(_) | DoneStatus::DoneWithoutSolution => (), + DoneStatus::Pending => return Ok(ExercisesProgress::CurrentPending), } self.app_state.done_current_exercise::(stdout) @@ -282,14 +283,15 @@ impl<'a> WatchState<'a> { pub fn check_all_exercises(&mut self, stdout: &mut StdoutLock) -> Result { stdout.write_all(b"\n")?; - if let Some(first_fail) = self.app_state.check_all_exercises(stdout, false)? { - // Only change exercise if the current one is done... + if let Some(first_pending_exercise_ind) = self.app_state.check_all_exercises(stdout)? { + // Only change exercise if the current one is done. if self.app_state.current_exercise().done { - self.app_state.set_current_exercise_ind(first_fail)?; + self.app_state + .set_current_exercise_ind(first_pending_exercise_ind)?; + Ok(ExercisesProgress::NewPending) + } else { + Ok(ExercisesProgress::CurrentPending) } - // ...but always pretend it's a "new" anyway because that refreshes - // the display - Ok(ExercisesProgress::NewPending) } else { self.app_state.render_final_message(stdout)?; Ok(ExercisesProgress::AllDone) From 326169a7fabacda9a21377b110371f91b32e8fd3 Mon Sep 17 00:00:00 2001 From: mo8it Date: Sun, 13 Oct 2024 22:02:41 +0200 Subject: [PATCH 08/15] Improve check-all command --- clippy.toml | 2 ++ src/app_state.rs | 5 +++++ src/main.rs | 54 ++++++++++++++++++++++++------------------------ src/run.rs | 8 +++---- 4 files changed, 38 insertions(+), 31 deletions(-) diff --git a/clippy.toml b/clippy.toml index 4a5dd06a..11ec6cc3 100644 --- a/clippy.toml +++ b/clippy.toml @@ -13,4 +13,6 @@ disallowed-methods = [ # Use `thread::Builder::spawn` instead and handle the error. "std::thread::spawn", "std::thread::Scope::spawn", + # Return `ExitCode` instead. + "std::process::exit", ] diff --git a/src/app_state.rs b/src/app_state.rs index 7540181c..c3998422 100644 --- a/src/app_state.rs +++ b/src/app_state.rs @@ -211,6 +211,11 @@ impl AppState { self.n_done } + #[inline] + pub fn n_pending(&self) -> u16 { + self.exercises.len() as u16 - self.n_done + } + #[inline] pub fn current_exercise(&self) -> &Exercise { &self.exercises[self.current_exercise_ind] diff --git a/src/main.rs b/src/main.rs index 64b72bde..f40bb89a 100644 --- a/src/main.rs +++ b/src/main.rs @@ -8,7 +8,7 @@ use crossterm::{ use std::{ io::{self, IsTerminal, Write}, path::Path, - process::exit, + process::ExitCode, }; use term::{clear_terminal, press_enter_prompt}; @@ -51,8 +51,8 @@ enum Subcommands { /// The name of the exercise name: Option, }, - /// Run all the exercises, marking them as done or pending accordingly. - RunAll, + /// Check all the exercises, marking them as done or pending accordingly. + CheckAll, /// Reset a single exercise Reset { /// The name of the exercise @@ -68,22 +68,26 @@ enum Subcommands { Dev(DevCommands), } -fn main() -> Result<()> { +fn main() -> Result { let args = Args::parse(); if cfg!(not(debug_assertions)) && Path::new("dev/rustlings-repo.txt").exists() { bail!("{OLD_METHOD_ERR}"); } - match args.command { - Some(Subcommands::Init) => return init::init().context("Initialization failed"), - Some(Subcommands::Dev(dev_command)) => return dev_command.run(), - _ => (), + 'priority_cmd: { + match args.command { + Some(Subcommands::Init) => init::init().context("Initialization failed")?, + Some(Subcommands::Dev(dev_command)) => dev_command.run()?, + _ => break 'priority_cmd, + } + + return Ok(ExitCode::SUCCESS); } if !Path::new("exercises").is_dir() { println!("{PRE_INIT_MSG}"); - exit(1); + return Ok(ExitCode::FAILURE); } let info_file = InfoFile::parse()?; @@ -142,33 +146,29 @@ fn main() -> Result<()> { if let Some(name) = name { app_state.set_current_exercise_by_name(&name)?; } - run::run(&mut app_state)?; + return run::run(&mut app_state); } - Some(Subcommands::RunAll) => { + Some(Subcommands::CheckAll) => { let mut stdout = io::stdout().lock(); - if let Some(first_fail) = app_state.check_all_exercises(&mut stdout)? { - let pending = app_state - .exercises() - .iter() - .filter(|exercise| !exercise.done) - .count(); + if let Some(first_pending_exercise_ind) = app_state.check_all_exercises(&mut stdout)? { if app_state.current_exercise().done { - app_state.set_current_exercise_ind(first_fail)?; + app_state.set_current_exercise_ind(first_pending_exercise_ind)?; } - stdout - .queue(SetForegroundColor(Color::Red))? - .queue(Print(format!("{pending}")))? - .queue(ResetColor)?; + + let pending = app_state.n_pending(); if pending == 1 { - stdout.queue(Print(" exercise has some errors: "))?; + stdout.queue(Print("One exercise pending: "))?; } else { - stdout.queue(Print(" exercises have errors, including "))?; + stdout.queue(SetForegroundColor(Color::Red))?; + write!(stdout, "{pending}")?; + stdout.queue(ResetColor)?; + stdout.queue(Print(" exercises are pending. The first: "))?; } app_state .current_exercise() .terminal_file_link(&mut stdout)?; - stdout.write_all(b".\n")?; - exit(1); + stdout.write_all(b"\n")?; + return Ok(ExitCode::FAILURE); } else { app_state.render_final_message(&mut stdout)?; } @@ -188,7 +188,7 @@ fn main() -> Result<()> { Some(Subcommands::Init | Subcommands::Dev(_)) => (), } - Ok(()) + Ok(ExitCode::SUCCESS) } const OLD_METHOD_ERR: &str = diff --git a/src/run.rs b/src/run.rs index 3fddcf22..f259f52c 100644 --- a/src/run.rs +++ b/src/run.rs @@ -5,7 +5,7 @@ use crossterm::{ }; use std::{ io::{self, Write}, - process::exit, + process::ExitCode, }; use crate::{ @@ -13,7 +13,7 @@ use crate::{ exercise::{solution_link_line, RunnableExercise, OUTPUT_CAPACITY}, }; -pub fn run(app_state: &mut AppState) -> Result<()> { +pub fn run(app_state: &mut AppState) -> Result { let exercise = app_state.current_exercise(); let mut output = Vec::with_capacity(OUTPUT_CAPACITY); let success = exercise.run_exercise(Some(&mut output), app_state.cmd_runner())?; @@ -29,7 +29,7 @@ pub fn run(app_state: &mut AppState) -> Result<()> { .current_exercise() .terminal_file_link(&mut stdout)?; stdout.write_all(b" with errors\n")?; - exit(1); + return Ok(ExitCode::FAILURE); } stdout.queue(SetForegroundColor(Color::Green))?; @@ -55,5 +55,5 @@ pub fn run(app_state: &mut AppState) -> Result<()> { ExercisesProgress::AllDone => (), } - Ok(()) + Ok(ExitCode::SUCCESS) } From 396ee4d618bc5e1cd5c84495f571f9d3f79774c8 Mon Sep 17 00:00:00 2001 From: mo8it Date: Sun, 13 Oct 2024 23:28:17 +0200 Subject: [PATCH 09/15] Show progress with exercise numbers --- src/app_state.rs | 126 +++++++++++++++++---------------------------- src/main.rs | 15 +++--- src/term.rs | 130 ++++++++++++++++++++++------------------------- 3 files changed, 113 insertions(+), 158 deletions(-) diff --git a/src/app_state.rs b/src/app_state.rs index c3998422..db9d1f10 100644 --- a/src/app_state.rs +++ b/src/app_state.rs @@ -1,8 +1,5 @@ use anyhow::{bail, Context, Error, Result}; -use crossterm::{ - style::{ResetColor, SetForegroundColor}, - terminal, QueueableCommand, -}; +use crossterm::{cursor, terminal, QueueableCommand}; use std::{ env, fs::{File, OpenOptions}, @@ -23,7 +20,7 @@ use crate::{ embedded::EMBEDDED_FILES, exercise::{Exercise, RunnableExercise}, info_file::ExerciseInfo, - term::{self, progress_bar_with_success}, + term::{self, show_exercises_check_progress}, }; const STATE_FILE_NAME: &str = ".rustlings-state.txt"; @@ -44,18 +41,12 @@ pub enum StateFileStatus { NotRead, } -enum ExerciseCheckProgress { +#[derive(Clone, Copy)] +pub enum ExerciseCheckProgress { + None, Checking, Done, Pending, - Error, -} - -#[derive(Clone, Copy)] -enum ExerciseCheckResult { - Done, - Pending, - Error, } pub struct AppState { @@ -417,27 +408,25 @@ impl AppState { } } - // Return the exercise index of the first pending exercise found. - pub fn check_all_exercises(&mut self, stdout: &mut StdoutLock) -> Result> { + fn check_all_exercises_impl(&mut self, stdout: &mut StdoutLock) -> Result> { stdout.write_all("Checking all exercises…\n".as_bytes())?; - let n_exercises = self.exercises.len() as u16; let next_exercise_ind = AtomicUsize::new(0); let term_width = terminal::size() .context("Failed to get the terminal size")? .0; + clear_terminal(stdout)?; - let mut results = vec![ExerciseCheckResult::Error; self.exercises.len()]; + let mut progresses = vec![ExerciseCheckProgress::None; self.exercises.len()]; let mut done = 0; let mut pending = 0; thread::scope(|s| { - let mut checking = 0; - let (exercise_result_sender, exercise_result_receiver) = mpsc::channel(); + let (exercise_progress_sender, exercise_progress_receiver) = mpsc::channel(); let n_threads = thread::available_parallelism() .map_or(DEFAULT_CHECK_PARALLELISM, |count| count.get()); for _ in 0..n_threads { - let exercise_result_sender = exercise_result_sender.clone(); + let exercise_progress_sender = exercise_progress_sender.clone(); let next_exercise_ind = &next_exercise_ind; let slf = &self; thread::Builder::new() @@ -449,7 +438,7 @@ impl AppState { }; // Notify the progress bar that this exercise is pending. - if exercise_result_sender + if exercise_progress_sender .send((exercise_ind, ExerciseCheckProgress::Checking)) .is_err() { @@ -457,14 +446,17 @@ impl AppState { }; let success = exercise.run_exercise(None, &slf.cmd_runner); - let result = match success { + let progress = match success { Ok(true) => ExerciseCheckProgress::Done, Ok(false) => ExerciseCheckProgress::Pending, - Err(_) => ExerciseCheckProgress::Error, + Err(_) => ExerciseCheckProgress::None, }; // Notify the progress bar that this exercise is done. - if exercise_result_sender.send((exercise_ind, result)).is_err() { + if exercise_progress_sender + .send((exercise_ind, progress)) + .is_err() + { break; } }) @@ -472,102 +464,76 @@ impl AppState { } // Drop this sender to detect when the last thread is done. - drop(exercise_result_sender); + drop(exercise_progress_sender); - // Print the legend. - stdout.write_all(b"Color legend: ")?; - stdout.queue(SetForegroundColor(term::PROGRESS_FAILED_COLOR))?; - stdout.write_all(b"Pending")?; - stdout.queue(ResetColor)?; - stdout.write_all(b" - ")?; - stdout.queue(SetForegroundColor(term::PROGRESS_SUCCESS_COLOR))?; - stdout.write_all(b"Done")?; - stdout.queue(ResetColor)?; - stdout.write_all(b" - ")?; - stdout.queue(SetForegroundColor(term::PROGRESS_PENDING_COLOR))?; - stdout.write_all(b"Checking")?; - stdout.queue(ResetColor)?; - stdout.write_all(b"\n")?; + while let Ok((exercise_ind, progress)) = exercise_progress_receiver.recv() { + progresses[exercise_ind] = progress; - while let Ok((exercise_ind, result)) = exercise_result_receiver.recv() { - match result { - ExerciseCheckProgress::Checking => checking += 1, - ExerciseCheckProgress::Done => { - results[exercise_ind] = ExerciseCheckResult::Done; - checking -= 1; - done += 1; - } - ExerciseCheckProgress::Pending => { - results[exercise_ind] = ExerciseCheckResult::Pending; - checking -= 1; - pending += 1; - } - ExerciseCheckProgress::Error => checking -= 1, + match progress { + ExerciseCheckProgress::None | ExerciseCheckProgress::Checking => (), + ExerciseCheckProgress::Done => done += 1, + ExerciseCheckProgress::Pending => pending += 1, } - stdout.write_all(b"\r")?; - progress_bar_with_success( - stdout, - checking, - pending, - done, - n_exercises, - term_width, - )?; - stdout.flush()?; + show_exercises_check_progress(stdout, &progresses, term_width)?; } Ok::<_, Error>(()) })?; let mut first_pending_exercise_ind = None; - for (exercise_ind, result) in results.into_iter().enumerate() { - match result { - ExerciseCheckResult::Done => { + for exercise_ind in 0..progresses.len() { + match progresses[exercise_ind] { + ExerciseCheckProgress::Done => { self.set_status(exercise_ind, true)?; } - ExerciseCheckResult::Pending => { + ExerciseCheckProgress::Pending => { self.set_status(exercise_ind, false)?; if first_pending_exercise_ind.is_none() { first_pending_exercise_ind = Some(exercise_ind); } } - ExerciseCheckResult::Error => { + ExerciseCheckProgress::None | ExerciseCheckProgress::Checking => { // If we got an error while checking all exercises in parallel, // it could be because we exceeded the limit of open file descriptors. // Therefore, try running exercises with errors sequentially. + progresses[exercise_ind] = ExerciseCheckProgress::Checking; + show_exercises_check_progress(stdout, &progresses, term_width)?; + let exercise = &self.exercises[exercise_ind]; let success = exercise.run_exercise(None, &self.cmd_runner)?; if success { done += 1; + progresses[exercise_ind] = ExerciseCheckProgress::Done; } else { pending += 1; if first_pending_exercise_ind.is_none() { first_pending_exercise_ind = Some(exercise_ind); } + progresses[exercise_ind] = ExerciseCheckProgress::Pending; } self.set_status(exercise_ind, success)?; - stdout.write_all(b"\r")?; - progress_bar_with_success( - stdout, - u16::from(pending + done < n_exercises), - pending, - done, - n_exercises, - term_width, - )?; - stdout.flush()?; + show_exercises_check_progress(stdout, &progresses, term_width)?; } } } self.write()?; - stdout.write_all(b"\n\n")?; + stdout.write_all(b"\n")?; Ok(first_pending_exercise_ind) } + // Return the exercise index of the first pending exercise found. + pub fn check_all_exercises(&mut self, stdout: &mut StdoutLock) -> Result> { + stdout.queue(cursor::Hide)?; + let res = self.check_all_exercises_impl(stdout); + stdout.queue(cursor::Show)?; + + res + } + /// Mark the current exercise as done and move on to the next pending exercise if one exists. /// If all exercises are marked as done, run all of them to make sure that they are actually /// done. If an exercise which is marked as done fails, mark it as pending and continue on it. diff --git a/src/main.rs b/src/main.rs index f40bb89a..075e7265 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,10 +1,6 @@ use anyhow::{bail, Context, Result}; use app_state::StateFileStatus; use clap::{Parser, Subcommand}; -use crossterm::{ - style::{Color, Print, ResetColor, SetForegroundColor}, - QueueableCommand, -}; use std::{ io::{self, IsTerminal, Write}, path::Path, @@ -157,12 +153,13 @@ fn main() -> Result { let pending = app_state.n_pending(); if pending == 1 { - stdout.queue(Print("One exercise pending: "))?; + stdout.write_all(b"One exercise pending: ")?; } else { - stdout.queue(SetForegroundColor(Color::Red))?; - write!(stdout, "{pending}")?; - stdout.queue(ResetColor)?; - stdout.queue(Print(" exercises are pending. The first: "))?; + write!( + stdout, + "{pending}/{} exercises are pending. The first: ", + app_state.exercises().len(), + )?; } app_state .current_exercise() diff --git a/src/term.rs b/src/term.rs index 31a951db..0294017b 100644 --- a/src/term.rs +++ b/src/term.rs @@ -1,6 +1,6 @@ use crossterm::{ cursor::MoveTo, - style::{Attribute, Color, SetAttribute, SetForegroundColor}, + style::{Attribute, Color, ResetColor, SetAttribute, SetForegroundColor}, terminal::{Clear, ClearType}, Command, QueueableCommand, }; @@ -9,9 +9,7 @@ use std::{ io::{self, BufRead, StdoutLock, Write}, }; -pub const PROGRESS_FAILED_COLOR: Color = Color::Red; -pub const PROGRESS_SUCCESS_COLOR: Color = Color::Green; -pub const PROGRESS_PENDING_COLOR: Color = Color::Blue; +use crate::app_state::ExerciseCheckProgress; pub struct MaxLenWriter<'a, 'b> { pub stdout: &'a mut StdoutLock<'b>, @@ -89,98 +87,43 @@ impl<'a> CountedWrite<'a> for StdoutLock<'a> { } } -/// Simple terminal progress bar. pub fn progress_bar<'a>( writer: &mut impl CountedWrite<'a>, progress: u16, total: u16, term_width: u16, -) -> io::Result<()> { - progress_bar_with_success(writer, 0, 0, progress, total, term_width) -} - -/// Terminal progress bar with three states (pending + failed + success). -pub fn progress_bar_with_success<'a>( - writer: &mut impl CountedWrite<'a>, - pending: u16, - failed: u16, - success: u16, - total: u16, - term_width: u16, ) -> io::Result<()> { debug_assert!(total < 1000); - debug_assert!(pending + failed + success <= total); + debug_assert!(progress <= total); const PREFIX: &[u8] = b"Progress: ["; const PREFIX_WIDTH: u16 = PREFIX.len() as u16; const POSTFIX_WIDTH: u16 = "] xxx/xxx".len() as u16; const WRAPPER_WIDTH: u16 = PREFIX_WIDTH + POSTFIX_WIDTH; - const MIN_TERM_WIDTH: u16 = WRAPPER_WIDTH + 4; + const MIN_LINE_WIDTH: u16 = WRAPPER_WIDTH + 4; - if term_width < MIN_TERM_WIDTH { + if term_width < MIN_LINE_WIDTH { writer.write_ascii(b"Progress: ")?; // Integers are in ASCII. - return writer.write_ascii(format!("{}/{total}", failed + success).as_bytes()); + return writer.write_ascii(format!("{progress}/{total}").as_bytes()); } let stdout = writer.stdout(); stdout.write_all(PREFIX)?; let width = term_width - WRAPPER_WIDTH; - let mut failed_end = (width * failed) / total; - let mut success_end = (width * (failed + success)) / total; - let mut pending_end = (width * (failed + success + pending)) / total; + let filled = (width * progress) / total; - // In case the range boundaries overlap, "pending" has priority over both - // "failed" and "success" (don't show the bar as "complete" when we are - // still checking some things). - // "Failed" has priority over "success" (don't show 100% success if we - // have some failures, at the risk of showing 100% failures even with - // a few successes). - // - // "Failed" already has priority over "success" because it's displayed - // first. But "pending" is last so we need to fix "success"/"failed". - if pending > 0 { - pending_end = pending_end.max(1); - if pending_end == success_end { - success_end -= 1; - } - if pending_end == failed_end { - failed_end -= 1; - } - - // This will replace the last character of the "pending" range with - // the arrow char ('>'). This ensures that even if the progress bar - // is filled (everything either done or pending), we'll still see - // the '>' as long as we are not fully done. - pending_end -= 1; - } - - if failed > 0 { - stdout.queue(SetForegroundColor(PROGRESS_FAILED_COLOR))?; - for _ in 0..failed_end { - stdout.write_all(b"#")?; - } - } - - stdout.queue(SetForegroundColor(PROGRESS_SUCCESS_COLOR))?; - for _ in failed_end..success_end { + stdout.queue(SetForegroundColor(Color::Green))?; + for _ in 0..filled { stdout.write_all(b"#")?; } - if pending > 0 { - stdout.queue(SetForegroundColor(PROGRESS_PENDING_COLOR))?; - - for _ in success_end..pending_end { - stdout.write_all(b"#")?; - } - } - - if pending_end < width { + if filled < width { stdout.write_all(b">")?; } - let width_minus_filled = width - pending_end; + let width_minus_filled = width - filled; if width_minus_filled > 1 { let red_part_width = width_minus_filled - 1; stdout.queue(SetForegroundColor(Color::Red))?; @@ -191,7 +134,56 @@ pub fn progress_bar_with_success<'a>( stdout.queue(SetForegroundColor(Color::Reset))?; - write!(stdout, "] {:>3}/{}", failed + success, total) + write!(stdout, "] {progress:>3}/{total}") +} + +pub fn show_exercises_check_progress( + stdout: &mut StdoutLock, + progresses: &[ExerciseCheckProgress], + term_width: u16, +) -> io::Result<()> { + stdout.queue(MoveTo(0, 0))?; + + // Legend + stdout.write_all(b"Color of exercise number: ")?; + stdout.queue(SetForegroundColor(Color::Blue))?; + stdout.write_all(b"Checking")?; + stdout.queue(ResetColor)?; + stdout.write_all(b" - ")?; + stdout.queue(SetForegroundColor(Color::Green))?; + stdout.write_all(b"Done")?; + stdout.queue(ResetColor)?; + stdout.write_all(b" - ")?; + stdout.queue(SetForegroundColor(Color::Red))?; + stdout.write_all(b"Pending")?; + stdout.queue(ResetColor)?; + stdout.write_all(b"\n")?; + + // Exercise numbers with up to 3 digits. + let n_cols = usize::from(term_width + 1) / 4; + + let mut exercise_num = 1; + for exercise_progress in progresses { + let color = match exercise_progress { + ExerciseCheckProgress::None => Color::Reset, + ExerciseCheckProgress::Checking => Color::Blue, + ExerciseCheckProgress::Done => Color::Green, + ExerciseCheckProgress::Pending => Color::Red, + }; + + stdout.queue(SetForegroundColor(color))?; + write!(stdout, "{exercise_num:<3}")?; + + if exercise_num % n_cols == 0 { + stdout.write_all(b"\n")?; + } else { + stdout.write_all(b" ")?; + } + + exercise_num += 1; + } + + stdout.queue(ResetColor)?.flush() } pub fn clear_terminal(stdout: &mut StdoutLock) -> io::Result<()> { From 8cac21511cbcc148ea7a4c8c6d196c9c0bf17255 Mon Sep 17 00:00:00 2001 From: mo8it Date: Mon, 14 Oct 2024 00:42:49 +0200 Subject: [PATCH 10/15] Small improvements to showing progress --- src/app_state.rs | 1 - src/main.rs | 4 +++- src/term.rs | 40 ++++++++++++++++++++++++---------------- 3 files changed, 27 insertions(+), 18 deletions(-) diff --git a/src/app_state.rs b/src/app_state.rs index db9d1f10..41231ef9 100644 --- a/src/app_state.rs +++ b/src/app_state.rs @@ -520,7 +520,6 @@ impl AppState { } self.write()?; - stdout.write_all(b"\n")?; Ok(first_pending_exercise_ind) } diff --git a/src/main.rs b/src/main.rs index 075e7265..5616d264 100644 --- a/src/main.rs +++ b/src/main.rs @@ -151,13 +151,15 @@ fn main() -> Result { app_state.set_current_exercise_ind(first_pending_exercise_ind)?; } + stdout.write_all(b"\n\n")?; + let pending = app_state.n_pending(); if pending == 1 { stdout.write_all(b"One exercise pending: ")?; } else { write!( stdout, - "{pending}/{} exercises are pending. The first: ", + "{pending}/{} exercises pending. The first: ", app_state.exercises().len(), )?; } diff --git a/src/term.rs b/src/term.rs index 0294017b..8a2f8c59 100644 --- a/src/term.rs +++ b/src/term.rs @@ -164,26 +164,34 @@ pub fn show_exercises_check_progress( let mut exercise_num = 1; for exercise_progress in progresses { - let color = match exercise_progress { - ExerciseCheckProgress::None => Color::Reset, - ExerciseCheckProgress::Checking => Color::Blue, - ExerciseCheckProgress::Done => Color::Green, - ExerciseCheckProgress::Pending => Color::Red, - }; - - stdout.queue(SetForegroundColor(color))?; - write!(stdout, "{exercise_num:<3}")?; - - if exercise_num % n_cols == 0 { - stdout.write_all(b"\n")?; - } else { - stdout.write_all(b" ")?; + match exercise_progress { + ExerciseCheckProgress::None => (), + ExerciseCheckProgress::Checking => { + stdout.queue(SetForegroundColor(Color::Blue))?; + } + ExerciseCheckProgress::Done => { + stdout.queue(SetForegroundColor(Color::Green))?; + } + ExerciseCheckProgress::Pending => { + stdout.queue(SetForegroundColor(Color::Red))?; + } } - exercise_num += 1; + write!(stdout, "{exercise_num:<3}")?; + stdout.queue(ResetColor)?; + + if exercise_num != progresses.len() { + if exercise_num % n_cols == 0 { + stdout.write_all(b"\n")?; + } else { + stdout.write_all(b" ")?; + } + + exercise_num += 1; + } } - stdout.queue(ResetColor)?.flush() + stdout.flush() } pub fn clear_terminal(stdout: &mut StdoutLock) -> io::Result<()> { From 9705c161b4d9b7fc8b071978f57b35a1b0c69819 Mon Sep 17 00:00:00 2001 From: mo8it Date: Mon, 14 Oct 2024 00:45:41 +0200 Subject: [PATCH 11/15] Remove the tracking of done and pending --- src/app_state.rs | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/src/app_state.rs b/src/app_state.rs index 41231ef9..d2dd87be 100644 --- a/src/app_state.rs +++ b/src/app_state.rs @@ -417,8 +417,6 @@ impl AppState { clear_terminal(stdout)?; let mut progresses = vec![ExerciseCheckProgress::None; self.exercises.len()]; - let mut done = 0; - let mut pending = 0; thread::scope(|s| { let (exercise_progress_sender, exercise_progress_receiver) = mpsc::channel(); @@ -468,13 +466,6 @@ impl AppState { while let Ok((exercise_ind, progress)) = exercise_progress_receiver.recv() { progresses[exercise_ind] = progress; - - match progress { - ExerciseCheckProgress::None | ExerciseCheckProgress::Checking => (), - ExerciseCheckProgress::Done => done += 1, - ExerciseCheckProgress::Pending => pending += 1, - } - show_exercises_check_progress(stdout, &progresses, term_width)?; } @@ -503,10 +494,8 @@ impl AppState { let exercise = &self.exercises[exercise_ind]; let success = exercise.run_exercise(None, &self.cmd_runner)?; if success { - done += 1; progresses[exercise_ind] = ExerciseCheckProgress::Done; } else { - pending += 1; if first_pending_exercise_ind.is_none() { first_pending_exercise_ind = Some(exercise_ind); } From fc5fc0920f3590d1b1e8c8186309ac1c5ec6fba5 Mon Sep 17 00:00:00 2001 From: mo8it Date: Mon, 14 Oct 2024 00:48:12 +0200 Subject: [PATCH 12/15] Remove outdated comments --- src/app_state.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/app_state.rs b/src/app_state.rs index d2dd87be..76a4c454 100644 --- a/src/app_state.rs +++ b/src/app_state.rs @@ -435,7 +435,6 @@ impl AppState { break; }; - // Notify the progress bar that this exercise is pending. if exercise_progress_sender .send((exercise_ind, ExerciseCheckProgress::Checking)) .is_err() @@ -450,7 +449,6 @@ impl AppState { Err(_) => ExerciseCheckProgress::None, }; - // Notify the progress bar that this exercise is done. if exercise_progress_sender .send((exercise_ind, progress)) .is_err() From ea73af9ba37bc1f6155a910c72f2ded8a0b64805 Mon Sep 17 00:00:00 2001 From: mo8it Date: Mon, 14 Oct 2024 01:06:11 +0200 Subject: [PATCH 13/15] Separate initialization with a struct --- src/app_state.rs | 13 +++-- src/term.rs | 125 ++++++++++++++++++++++++++--------------------- 2 files changed, 74 insertions(+), 64 deletions(-) diff --git a/src/app_state.rs b/src/app_state.rs index 76a4c454..57ffea85 100644 --- a/src/app_state.rs +++ b/src/app_state.rs @@ -20,7 +20,7 @@ use crate::{ embedded::EMBEDDED_FILES, exercise::{Exercise, RunnableExercise}, info_file::ExerciseInfo, - term::{self, show_exercises_check_progress}, + term::{self, ExercisesCheckProgressVisualizer}, }; const STATE_FILE_NAME: &str = ".rustlings-state.txt"; @@ -409,13 +409,12 @@ impl AppState { } fn check_all_exercises_impl(&mut self, stdout: &mut StdoutLock) -> Result> { - stdout.write_all("Checking all exercises…\n".as_bytes())?; - let next_exercise_ind = AtomicUsize::new(0); let term_width = terminal::size() .context("Failed to get the terminal size")? .0; - clear_terminal(stdout)?; + let mut progress_visualizer = ExercisesCheckProgressVisualizer::build(stdout, term_width)?; + let next_exercise_ind = AtomicUsize::new(0); let mut progresses = vec![ExerciseCheckProgress::None; self.exercises.len()]; thread::scope(|s| { @@ -464,7 +463,7 @@ impl AppState { while let Ok((exercise_ind, progress)) = exercise_progress_receiver.recv() { progresses[exercise_ind] = progress; - show_exercises_check_progress(stdout, &progresses, term_width)?; + progress_visualizer.update(&progresses)?; } Ok::<_, Error>(()) @@ -487,7 +486,7 @@ impl AppState { // it could be because we exceeded the limit of open file descriptors. // Therefore, try running exercises with errors sequentially. progresses[exercise_ind] = ExerciseCheckProgress::Checking; - show_exercises_check_progress(stdout, &progresses, term_width)?; + progress_visualizer.update(&progresses)?; let exercise = &self.exercises[exercise_ind]; let success = exercise.run_exercise(None, &self.cmd_runner)?; @@ -501,7 +500,7 @@ impl AppState { } self.set_status(exercise_ind, success)?; - show_exercises_check_progress(stdout, &progresses, term_width)?; + progress_visualizer.update(&progresses)?; } } } diff --git a/src/term.rs b/src/term.rs index 8a2f8c59..13d5657b 100644 --- a/src/term.rs +++ b/src/term.rs @@ -87,6 +87,74 @@ impl<'a> CountedWrite<'a> for StdoutLock<'a> { } } +pub struct ExercisesCheckProgressVisualizer<'a, 'b> { + stdout: &'a mut StdoutLock<'b>, + n_cols: usize, +} + +impl<'a, 'b> ExercisesCheckProgressVisualizer<'a, 'b> { + pub fn build(stdout: &'a mut StdoutLock<'b>, term_width: u16) -> io::Result { + clear_terminal(stdout)?; + stdout.write_all("Checking all exercises…\n".as_bytes())?; + + // Legend + stdout.write_all(b"Color of exercise number: ")?; + stdout.queue(SetForegroundColor(Color::Blue))?; + stdout.write_all(b"Checking")?; + stdout.queue(ResetColor)?; + stdout.write_all(b" - ")?; + stdout.queue(SetForegroundColor(Color::Green))?; + stdout.write_all(b"Done")?; + stdout.queue(ResetColor)?; + stdout.write_all(b" - ")?; + stdout.queue(SetForegroundColor(Color::Red))?; + stdout.write_all(b"Pending")?; + stdout.queue(ResetColor)?; + stdout.write_all(b"\n")?; + + // Exercise numbers with up to 3 digits. + // +1 because the last column doesn't end with a whitespace. + let n_cols = usize::from(term_width + 1) / 4; + + Ok(Self { stdout, n_cols }) + } + + pub fn update(&mut self, progresses: &[ExerciseCheckProgress]) -> io::Result<()> { + self.stdout.queue(MoveTo(0, 2))?; + + let mut exercise_num = 1; + for exercise_progress in progresses { + match exercise_progress { + ExerciseCheckProgress::None => (), + ExerciseCheckProgress::Checking => { + self.stdout.queue(SetForegroundColor(Color::Blue))?; + } + ExerciseCheckProgress::Done => { + self.stdout.queue(SetForegroundColor(Color::Green))?; + } + ExerciseCheckProgress::Pending => { + self.stdout.queue(SetForegroundColor(Color::Red))?; + } + } + + write!(self.stdout, "{exercise_num:<3}")?; + self.stdout.queue(ResetColor)?; + + if exercise_num != progresses.len() { + if exercise_num % self.n_cols == 0 { + self.stdout.write_all(b"\n")?; + } else { + self.stdout.write_all(b" ")?; + } + + exercise_num += 1; + } + } + + self.stdout.flush() + } +} + pub fn progress_bar<'a>( writer: &mut impl CountedWrite<'a>, progress: u16, @@ -137,63 +205,6 @@ pub fn progress_bar<'a>( write!(stdout, "] {progress:>3}/{total}") } -pub fn show_exercises_check_progress( - stdout: &mut StdoutLock, - progresses: &[ExerciseCheckProgress], - term_width: u16, -) -> io::Result<()> { - stdout.queue(MoveTo(0, 0))?; - - // Legend - stdout.write_all(b"Color of exercise number: ")?; - stdout.queue(SetForegroundColor(Color::Blue))?; - stdout.write_all(b"Checking")?; - stdout.queue(ResetColor)?; - stdout.write_all(b" - ")?; - stdout.queue(SetForegroundColor(Color::Green))?; - stdout.write_all(b"Done")?; - stdout.queue(ResetColor)?; - stdout.write_all(b" - ")?; - stdout.queue(SetForegroundColor(Color::Red))?; - stdout.write_all(b"Pending")?; - stdout.queue(ResetColor)?; - stdout.write_all(b"\n")?; - - // Exercise numbers with up to 3 digits. - let n_cols = usize::from(term_width + 1) / 4; - - let mut exercise_num = 1; - for exercise_progress in progresses { - match exercise_progress { - ExerciseCheckProgress::None => (), - ExerciseCheckProgress::Checking => { - stdout.queue(SetForegroundColor(Color::Blue))?; - } - ExerciseCheckProgress::Done => { - stdout.queue(SetForegroundColor(Color::Green))?; - } - ExerciseCheckProgress::Pending => { - stdout.queue(SetForegroundColor(Color::Red))?; - } - } - - write!(stdout, "{exercise_num:<3}")?; - stdout.queue(ResetColor)?; - - if exercise_num != progresses.len() { - if exercise_num % n_cols == 0 { - stdout.write_all(b"\n")?; - } else { - stdout.write_all(b" ")?; - } - - exercise_num += 1; - } - } - - stdout.flush() -} - pub fn clear_terminal(stdout: &mut StdoutLock) -> io::Result<()> { stdout .queue(MoveTo(0, 0))? From bdc6dad8de2d3b47e33098fd55956ba03b131b27 Mon Sep 17 00:00:00 2001 From: mo8it Date: Mon, 14 Oct 2024 01:28:12 +0200 Subject: [PATCH 14/15] Update names --- src/app_state.rs | 29 ++++++++++++++--------------- src/term.rs | 33 +++++++++++++++++++-------------- 2 files changed, 33 insertions(+), 29 deletions(-) diff --git a/src/app_state.rs b/src/app_state.rs index 57ffea85..5f84d35d 100644 --- a/src/app_state.rs +++ b/src/app_state.rs @@ -20,7 +20,7 @@ use crate::{ embedded::EMBEDDED_FILES, exercise::{Exercise, RunnableExercise}, info_file::ExerciseInfo, - term::{self, ExercisesCheckProgressVisualizer}, + term::{self, CheckProgressVisualizer}, }; const STATE_FILE_NAME: &str = ".rustlings-state.txt"; @@ -42,7 +42,7 @@ pub enum StateFileStatus { } #[derive(Clone, Copy)] -pub enum ExerciseCheckProgress { +pub enum CheckProgress { None, Checking, Done, @@ -412,10 +412,10 @@ impl AppState { let term_width = terminal::size() .context("Failed to get the terminal size")? .0; - let mut progress_visualizer = ExercisesCheckProgressVisualizer::build(stdout, term_width)?; + let mut progress_visualizer = CheckProgressVisualizer::build(stdout, term_width)?; let next_exercise_ind = AtomicUsize::new(0); - let mut progresses = vec![ExerciseCheckProgress::None; self.exercises.len()]; + let mut progresses = vec![CheckProgress::None; self.exercises.len()]; thread::scope(|s| { let (exercise_progress_sender, exercise_progress_receiver) = mpsc::channel(); @@ -435,7 +435,7 @@ impl AppState { }; if exercise_progress_sender - .send((exercise_ind, ExerciseCheckProgress::Checking)) + .send((exercise_ind, CheckProgress::Checking)) .is_err() { break; @@ -443,9 +443,9 @@ impl AppState { let success = exercise.run_exercise(None, &slf.cmd_runner); let progress = match success { - Ok(true) => ExerciseCheckProgress::Done, - Ok(false) => ExerciseCheckProgress::Pending, - Err(_) => ExerciseCheckProgress::None, + Ok(true) => CheckProgress::Done, + Ok(false) => CheckProgress::Pending, + Err(_) => CheckProgress::None, }; if exercise_progress_sender @@ -472,34 +472,33 @@ impl AppState { let mut first_pending_exercise_ind = None; for exercise_ind in 0..progresses.len() { match progresses[exercise_ind] { - ExerciseCheckProgress::Done => { + CheckProgress::Done => { self.set_status(exercise_ind, true)?; } - ExerciseCheckProgress::Pending => { + CheckProgress::Pending => { self.set_status(exercise_ind, false)?; if first_pending_exercise_ind.is_none() { first_pending_exercise_ind = Some(exercise_ind); } } - ExerciseCheckProgress::None | ExerciseCheckProgress::Checking => { + CheckProgress::None | CheckProgress::Checking => { // If we got an error while checking all exercises in parallel, // it could be because we exceeded the limit of open file descriptors. // Therefore, try running exercises with errors sequentially. - progresses[exercise_ind] = ExerciseCheckProgress::Checking; + progresses[exercise_ind] = CheckProgress::Checking; progress_visualizer.update(&progresses)?; let exercise = &self.exercises[exercise_ind]; let success = exercise.run_exercise(None, &self.cmd_runner)?; if success { - progresses[exercise_ind] = ExerciseCheckProgress::Done; + progresses[exercise_ind] = CheckProgress::Done; } else { + progresses[exercise_ind] = CheckProgress::Pending; if first_pending_exercise_ind.is_none() { first_pending_exercise_ind = Some(exercise_ind); } - progresses[exercise_ind] = ExerciseCheckProgress::Pending; } self.set_status(exercise_ind, success)?; - progress_visualizer.update(&progresses)?; } } diff --git a/src/term.rs b/src/term.rs index 13d5657b..86909f08 100644 --- a/src/term.rs +++ b/src/term.rs @@ -9,7 +9,7 @@ use std::{ io::{self, BufRead, StdoutLock, Write}, }; -use crate::app_state::ExerciseCheckProgress; +use crate::app_state::CheckProgress; pub struct MaxLenWriter<'a, 'b> { pub stdout: &'a mut StdoutLock<'b>, @@ -87,27 +87,31 @@ impl<'a> CountedWrite<'a> for StdoutLock<'a> { } } -pub struct ExercisesCheckProgressVisualizer<'a, 'b> { +pub struct CheckProgressVisualizer<'a, 'b> { stdout: &'a mut StdoutLock<'b>, n_cols: usize, } -impl<'a, 'b> ExercisesCheckProgressVisualizer<'a, 'b> { +impl<'a, 'b> CheckProgressVisualizer<'a, 'b> { + const CHECKING_COLOR: Color = Color::Blue; + const DONE_COLOR: Color = Color::Green; + const PENDING_COLOR: Color = Color::Red; + pub fn build(stdout: &'a mut StdoutLock<'b>, term_width: u16) -> io::Result { clear_terminal(stdout)?; stdout.write_all("Checking all exercises…\n".as_bytes())?; // Legend stdout.write_all(b"Color of exercise number: ")?; - stdout.queue(SetForegroundColor(Color::Blue))?; + stdout.queue(SetForegroundColor(Self::CHECKING_COLOR))?; stdout.write_all(b"Checking")?; stdout.queue(ResetColor)?; stdout.write_all(b" - ")?; - stdout.queue(SetForegroundColor(Color::Green))?; + stdout.queue(SetForegroundColor(Self::DONE_COLOR))?; stdout.write_all(b"Done")?; stdout.queue(ResetColor)?; stdout.write_all(b" - ")?; - stdout.queue(SetForegroundColor(Color::Red))?; + stdout.queue(SetForegroundColor(Self::PENDING_COLOR))?; stdout.write_all(b"Pending")?; stdout.queue(ResetColor)?; stdout.write_all(b"\n")?; @@ -119,21 +123,22 @@ impl<'a, 'b> ExercisesCheckProgressVisualizer<'a, 'b> { Ok(Self { stdout, n_cols }) } - pub fn update(&mut self, progresses: &[ExerciseCheckProgress]) -> io::Result<()> { + pub fn update(&mut self, progresses: &[CheckProgress]) -> io::Result<()> { self.stdout.queue(MoveTo(0, 2))?; let mut exercise_num = 1; for exercise_progress in progresses { match exercise_progress { - ExerciseCheckProgress::None => (), - ExerciseCheckProgress::Checking => { - self.stdout.queue(SetForegroundColor(Color::Blue))?; + CheckProgress::None => (), + CheckProgress::Checking => { + self.stdout + .queue(SetForegroundColor(Self::CHECKING_COLOR))?; } - ExerciseCheckProgress::Done => { - self.stdout.queue(SetForegroundColor(Color::Green))?; + CheckProgress::Done => { + self.stdout.queue(SetForegroundColor(Self::DONE_COLOR))?; } - ExerciseCheckProgress::Pending => { - self.stdout.queue(SetForegroundColor(Color::Red))?; + CheckProgress::Pending => { + self.stdout.queue(SetForegroundColor(Self::PENDING_COLOR))?; } } From 932bc25d8824e18debc91e5f25f022e8d066bcf8 Mon Sep 17 00:00:00 2001 From: mo8it Date: Mon, 14 Oct 2024 01:28:34 +0200 Subject: [PATCH 15/15] Remove unneeded line --- src/main.rs | 2 +- src/run.rs | 1 + src/watch/state.rs | 2 -- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/main.rs b/src/main.rs index 5616d264..c8bcd2e5 100644 --- a/src/main.rs +++ b/src/main.rs @@ -152,7 +152,6 @@ fn main() -> Result { } stdout.write_all(b"\n\n")?; - let pending = app_state.n_pending(); if pending == 1 { stdout.write_all(b"One exercise pending: ")?; @@ -167,6 +166,7 @@ fn main() -> Result { .current_exercise() .terminal_file_link(&mut stdout)?; stdout.write_all(b"\n")?; + return Ok(ExitCode::FAILURE); } else { app_state.render_final_message(&mut stdout)?; diff --git a/src/run.rs b/src/run.rs index f259f52c..ac8b26ad 100644 --- a/src/run.rs +++ b/src/run.rs @@ -29,6 +29,7 @@ pub fn run(app_state: &mut AppState) -> Result { .current_exercise() .terminal_file_link(&mut stdout)?; stdout.write_all(b" with errors\n")?; + return Ok(ExitCode::FAILURE); } diff --git a/src/watch/state.rs b/src/watch/state.rs index 8b58e311..0ac758ce 100644 --- a/src/watch/state.rs +++ b/src/watch/state.rs @@ -281,8 +281,6 @@ impl<'a> WatchState<'a> { } pub fn check_all_exercises(&mut self, stdout: &mut StdoutLock) -> Result { - stdout.write_all(b"\n")?; - if let Some(first_pending_exercise_ind) = self.app_state.check_all_exercises(stdout)? { // Only change exercise if the current one is done. if self.app_state.current_exercise().done {