Merge pull request #1959 from rust-lang/output

Improve output

9 files changed, 206 insertions, 103 deletions

diff --git a/src/app_state.rs b/src/app_state.rs
index 33d3de2..476b5a9 100644
--- a/src/app_state.rs
+++ b/src/app_state.rs
@@ -11,7 +11,12 @@ use std::{
     process::{Command, Stdio},
 };
 
-use crate::{embedded::EMBEDDED_FILES, exercise::Exercise, info_file::ExerciseInfo, DEBUG_PROFILE};
+use crate::{
+    embedded::EMBEDDED_FILES,
+    exercise::{Exercise, OUTPUT_CAPACITY},
+    info_file::ExerciseInfo,
+    DEBUG_PROFILE,
+};
 
 const STATE_FILE_NAME: &str = ".rustlings-state.txt";
 const BAD_INDEX_ERR: &str = "The current exercise index is higher than the number of exercises";
@@ -107,7 +112,8 @@ impl AppState {
                     dir,
                     name,
                     path,
-                    mode: exercise_info.mode,
+                    test: exercise_info.test,
+                    strict_clippy: exercise_info.strict_clippy,
                     hint,
                     done: false,
                 }
@@ -302,12 +308,14 @@ impl AppState {
         let Some(ind) = self.next_pending_exercise_ind() else {
             writer.write_all(RERUNNING_ALL_EXERCISES_MSG)?;
 
+            let mut output = Vec::with_capacity(OUTPUT_CAPACITY);
             for (exercise_ind, exercise) in self.exercises().iter().enumerate() {
-                writer.write_fmt(format_args!("Running {exercise} ... "))?;
+                write!(writer, "Running {exercise} ... ")?;
                 writer.flush()?;
 
-                if !exercise.run()?.status.success() {
-                    writer.write_fmt(format_args!("{}\n\n", "FAILED".red()))?;
+                let success = exercise.run(&mut output)?;
+                if !success {
+                    writeln!(writer, "{}\n", "FAILED".red())?;
 
                     self.current_exercise_ind = exercise_ind;
 
@@ -321,7 +329,7 @@ impl AppState {
                     return Ok(ExercisesProgress::Pending);
                 }
 
-                writer.write_fmt(format_args!("{}\n", "ok".green()))?;
+                writeln!(writer, "{}", "ok".green())?;
             }
 
             writer.execute(Clear(ClearType::All))?;
diff --git a/src/dev/new.rs b/src/dev/new.rs
index 82aba42..8f87010 100644
--- a/src/dev/new.rs
+++ b/src/dev/new.rs
@@ -99,10 +99,15 @@ name = "???"
 # Otherwise, the path is `exercises/NAME.rs`
 # dir = "???"
 
-# The mode to run the exercise in.
-# The mode "test" (preferred) runs the exercise's tests.
-# The mode "run" only checks if the exercise compiles and runs it.
-mode = "test"
+# Rustlings expects the exercise to contain tests and run them.
+# You can optionally disable testing by setting `test` to `false` (the default is `true`).
+# In that case, the exercise will be considered done when it just successfully compiles.
+# test = true
+
+# Rustlings will always run Clippy on exercises.
+# You can optionally set `strict_clippy` to `true` (the default is `false`) to only consider
+# the exercise as done when there are no warnings left.
+# strict_clippy = false
 
 # A multi-line hint to be shown to users on request.
 hint = """???"""
diff --git a/src/exercise.rs b/src/exercise.rs
index 45ac208..21ae582 100644
--- a/src/exercise.rs
+++ b/src/exercise.rs
@@ -2,11 +2,55 @@ use anyhow::{Context, Result};
 use crossterm::style::{style, StyledContent, Stylize};
 use std::{
     fmt::{self, Display, Formatter},
-    path::Path,
-    process::{Command, Output},
+    io::{Read, Write},
+    process::{Command, Stdio},
 };
 
-use crate::{info_file::Mode, terminal_link::TerminalFileLink, DEBUG_PROFILE};
+use crate::{in_official_repo, terminal_link::TerminalFileLink, DEBUG_PROFILE};
+
+pub const OUTPUT_CAPACITY: usize = 1 << 14;
+
+fn run_command(
+    mut cmd: Command,
+    cmd_description: &str,
+    output: &mut Vec<u8>,
+    stderr: bool,
+) -> Result<bool> {
+    let (mut reader, writer) = os_pipe::pipe().with_context(|| {
+        format!("Failed to create a pipe to run the command `{cmd_description}``")
+    })?;
+
+    let (stdout, stderr) = if stderr {
+        (
+            Stdio::from(writer.try_clone().with_context(|| {
+                format!("Failed to clone the pipe writer for the command `{cmd_description}`")
+            })?),
+            Stdio::from(writer),
+        )
+    } else {
+        (Stdio::from(writer), Stdio::null())
+    };
+
+    let mut handle = cmd
+        .stdout(stdout)
+        .stderr(stderr)
+        .spawn()
+        .with_context(|| format!("Failed to run the command `{cmd_description}`"))?;
+
+    // Prevent pipe deadlock.
+    drop(cmd);
+
+    reader
+        .read_to_end(output)
+        .with_context(|| format!("Failed to read the output of the command `{cmd_description}`"))?;
+
+    output.push(b'\n');
+
+    handle
+        .wait()
+        .with_context(|| format!("Failed to wait on the command `{cmd_description}` to exit"))
+        .map(|status| status.success())
+}
 
 pub struct Exercise {
     pub dir: Option<&'static str>,
@@ -14,21 +58,51 @@ pub struct Exercise {
     pub name: &'static str,
     // Exercise's path
     pub path: &'static str,
-    // The mode of the exercise
-    pub mode: Mode,
+    pub test: bool,
+    pub strict_clippy: bool,
     // The hint text associated with the exercise
     pub hint: String,
     pub done: bool,
 }
 
 impl Exercise {
-    fn cargo_cmd(&self, command: &str, args: &[&str]) -> Result<Output> {
+    fn run_bin(&self, output: &mut Vec<u8>) -> Result<bool> {
+        writeln!(output, "{}", "Output".underlined())?;
+
+        let bin_path = format!("target/debug/{}", self.name);
+        let success = run_command(Command::new(&bin_path), &bin_path, output, true)?;
+
+        if !success {
+            writeln!(
+                output,
+                "{}",
+                "The exercise didn't run successfully (nonzero exit code)"
+                    .bold()
+                    .red()
+            )?;
+        }
+
+        Ok(success)
+    }
+
+    fn cargo_cmd(
+        &self,
+        command: &str,
+        args: &[&str],
+        cmd_description: &str,
+        output: &mut Vec<u8>,
+        dev: bool,
+        stderr: bool,
+    ) -> Result<bool> {
         let mut cmd = Command::new("cargo");
         cmd.arg(command);
 
         // A hack to make `cargo run` work when developing Rustlings.
-        if DEBUG_PROFILE && Path::new("tests").exists() {
-            cmd.arg("--manifest-path").arg("dev/Cargo.toml");
+        if dev {
+            cmd.arg("--manifest-path")
+                .arg("dev/Cargo.toml")
+                .arg("--target-dir")
+                .arg("target");
         }
 
         cmd.arg("--color")
@@ -36,30 +110,60 @@ impl Exercise {
             .arg("-q")
             .arg("--bin")
             .arg(self.name)
-            .args(args)
-            .output()
-            .context("Failed to run Cargo")
+            .args(args);
+
+        run_command(cmd, cmd_description, output, stderr)
     }
 
-    pub fn run(&self) -> Result<Output> {
-        match self.mode {
-            Mode::Run => self.cargo_cmd("run", &[]),
-            Mode::Test => self.cargo_cmd(
-                "test",
-                &[
-                    "--",
-                    "--color",
-                    "always",
-                    "--nocapture",
-                    "--format",
-                    "pretty",
-                ],
-            ),
-            Mode::Clippy => self.cargo_cmd(
-                "clippy",
-                &["--", "-D", "warnings", "-D", "clippy::float_cmp"],
-            ),
+    pub fn run(&self, output: &mut Vec<u8>) -> Result<bool> {
+        output.clear();
+
+        // Developing the official Rustlings.
+        let dev = DEBUG_PROFILE && in_official_repo();
+
+        let build_success = self.cargo_cmd("build", &[], "cargo build …", output, dev, true)?;
+        if !build_success {
+            return Ok(false);
         }
+
+        // Discard the output of `cargo build` because it will be shown again by the Cargo command.
+        output.clear();
+
+        let clippy_args: &[&str] = if self.strict_clippy {
+            &["--", "-D", "warnings"]
+        } else {
+            &[]
+        };
+        let clippy_success =
+            self.cargo_cmd("clippy", clippy_args, "cargo clippy …", output, dev, true)?;
+        if !clippy_success {
+            return Ok(false);
+        }
+
+        if !self.test {
+            return self.run_bin(output);
+        }
+
+        let test_success = self.cargo_cmd(
+            "test",
+            &[
+                "--",
+                "--color",
+                "always",
+                "--nocapture",
+                "--format",
+                "pretty",
+            ],
+            "cargo test …",
+            output,
+            dev,
+            // Hide warnings because they are shown by Clippy.
+            false,
+        )?;
+
+        let run_success = self.run_bin(output)?;
+
+        Ok(test_success && run_success)
     }
 
     pub fn terminal_link(&self) -> StyledContent<TerminalFileLink<'_>> {
diff --git a/src/info_file.rs b/src/info_file.rs
index 6938cd0..dbe4f08 100644
--- a/src/info_file.rs
+++ b/src/info_file.rs
@@ -2,18 +2,6 @@ use anyhow::{bail, Context, Error, Result};
 use serde::Deserialize;
 use std::{fs, io::ErrorKind};
 
-// The mode of the exercise.
-#[derive(Deserialize, Copy, Clone)]
-#[serde(rename_all = "lowercase")]
-pub enum Mode {
-    // The exercise should be compiled as a binary
-    Run,
-    // The exercise should be compiled as a test harness
-    Test,
-    // The exercise should be linted with clippy
-    Clippy,
-}
-
 // Deserialized from the `info.toml` file.
 #[derive(Deserialize)]
 pub struct ExerciseInfo {
@@ -21,11 +9,17 @@ pub struct ExerciseInfo {
     pub name: String,
     // The exercise's directory inside the `exercises` directory
     pub dir: Option<String>,
-    // The mode of the exercise
-    pub mode: Mode,
+    #[serde(default = "default_true")]
+    pub test: bool,
+    #[serde(default)]
+    pub strict_clippy: bool,
     // The hint text associated with the exercise
     pub hint: String,
 }
+#[inline]
+const fn default_true() -> bool {
+    true
+}
 
 impl ExerciseInfo {
     pub fn path(&self) -> String {
diff --git a/src/list/state.rs b/src/list/state.rs
index 0253bb9..19a77fe 100644
--- a/src/list/state.rs
+++ b/src/list/state.rs
@@ -231,8 +231,7 @@ impl<'a> UiState<'a> {
             .context("Invalid selection index")?;
 
         let exercise_path = self.app_state.reset_exercise_by_ind(ind)?;
-        self.message
-            .write_fmt(format_args!("The exercise {exercise_path} has been reset"))?;
+        write!(self.message, "The exercise {exercise_path} has been reset")?;
 
         Ok(self.with_updated_rows())
     }
diff --git a/src/main.rs b/src/main.rs
index 790fff6..a928504 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -75,10 +75,14 @@ enum Subcommands {
     Dev(DevCommands),
 }
 
+fn in_official_repo() -> bool {
+    Path::new("dev/rustlings-repo.txt").exists()
+}
+
 fn main() -> Result<()> {
     let args = Args::parse();
 
-    if !DEBUG_PROFILE && Path::new("dev/rustlings-repo.txt").exists() {
+    if !DEBUG_PROFILE && in_official_repo() {
         bail!("{OLD_METHOD_ERR}");
     }
 
diff --git a/src/run.rs b/src/run.rs
index a2b6972..cbc9ad7 100644
--- a/src/run.rs
+++ b/src/run.rs
@@ -4,20 +4,19 @@ use std::io::{self, Write};
 
 use crate::{
     app_state::{AppState, ExercisesProgress},
+    exercise::OUTPUT_CAPACITY,
     terminal_link::TerminalFileLink,
 };
 
 pub fn run(app_state: &mut AppState) -> Result<()> {
     let exercise = app_state.current_exercise();
-    let output = exercise.run()?;
+    let mut output = Vec::with_capacity(OUTPUT_CAPACITY);
+    let success = exercise.run(&mut output)?;
 
     let mut stdout = io::stdout().lock();
-    stdout.write_all(&output.stdout)?;
-    stdout.write_all(b"\n")?;
-    stdout.write_all(&output.stderr)?;
-    stdout.flush()?;
+    stdout.write_all(&output)?;
 
-    if !output.status.success() {
+    if !success {
         app_state.set_pending(app_state.current_exercise_ind())?;
 
         bail!(
@@ -26,11 +25,12 @@ pub fn run(app_state: &mut AppState) -> Result<()> {
         );
     }
 
-    stdout.write_fmt(format_args!(
-        "{}{}\n",
+    writeln!(
+        stdout,
+        "{}{}",
         "✓ Successfully ran ".green(),
         exercise.path.green(),
-    ))?;
+    )?;
 
     if let Some(solution_path) = app_state.current_solution_path()? {
         println!(
diff --git a/src/watch.rs b/src/watch.rs
index 5ebe526..1e22f59 100644
--- a/src/watch.rs
+++ b/src/watch.rs
@@ -51,7 +51,7 @@ pub fn watch(
     // Otherwise, the file watcher exits.
     let _debouncer_guard = if let Some(exercise_paths) = notify_exercise_paths {
         let mut debouncer = new_debouncer(
-            Duration::from_secs(1),
+            Duration::from_millis(500),
             DebounceEventHandler {
                 tx: tx.clone(),
                 exercise_paths,
diff --git a/src/watch/state.rs b/src/watch/state.rs
index 5f4abf3..40c01bf 100644
--- a/src/watch/state.rs
+++ b/src/watch/state.rs
@@ -8,6 +8,7 @@ use std::io::{self, StdoutLock, Write};
 
 use crate::{
     app_state::{AppState, ExercisesProgress},
+    exercise::OUTPUT_CAPACITY,
     progress_bar::progress_bar,
     terminal_link::TerminalFileLink,
 };
@@ -21,8 +22,7 @@ enum DoneStatus {
 pub struct WatchState<'a> {
     writer: StdoutLock<'a>,
     app_state: &'a mut AppState,
-    stdout: Option<Vec<u8>>,
-    stderr: Option<Vec<u8>>,
+    output: Vec<u8>,
     show_hint: bool,
     done_status: DoneStatus,
     manual_run: bool,
@@ -35,8 +35,7 @@ impl<'a> WatchState<'a> {
         Self {
             writer,
             app_state,
-            stdout: None,
-            stderr: None,
+            output: Vec::with_capacity(OUTPUT_CAPACITY),
             show_hint: false,
             done_status: DoneStatus::Pending,
             manual_run,
@@ -51,11 +50,8 @@ impl<'a> WatchState<'a> {
     pub fn run_current_exercise(&mut self) -> Result<()> {
         self.show_hint = false;
 
-        let output = self.app_state.current_exercise().run()?;
-        self.stdout = Some(output.stdout);
-
-        if output.status.success() {
-            self.stderr = None;
+        let success = self.app_state.current_exercise().run(&mut self.output)?;
+        if success {
             self.done_status =
                 if let Some(solution_path) = self.app_state.current_solution_path()? {
                     DoneStatus::DoneWithSolution(solution_path)
@@ -66,7 +62,6 @@ impl<'a> WatchState<'a> {
             self.app_state
                 .set_pending(self.app_state.current_exercise_ind())?;
 
-            self.stderr = Some(output.stderr);
             self.done_status = DoneStatus::Pending;
         }
 
@@ -93,19 +88,18 @@ impl<'a> WatchState<'a> {
         self.writer.write_all(b"\n")?;
 
         if self.manual_run {
-            self.writer.write_fmt(format_args!("{}un/", 'r'.bold()))?;
+            write!(self.writer, "{}un/", 'r'.bold())?;
         }
 
         if !matches!(self.done_status, DoneStatus::Pending) {
-            self.writer.write_fmt(format_args!("{}ext/", 'n'.bold()))?;
+            write!(self.writer, "{}ext/", 'n'.bold())?;
         }
 
         if !self.show_hint {
-            self.writer.write_fmt(format_args!("{}int/", 'h'.bold()))?;
+            write!(self.writer, "{}int/", 'h'.bold())?;
         }
 
-        self.writer
-            .write_fmt(format_args!("{}ist/{}uit? ", 'l'.bold(), 'q'.bold()))?;
+        write!(self.writer, "{}ist/{}uit? ", 'l'.bold(), 'q'.bold())?;
 
         self.writer.flush()
     }
@@ -116,41 +110,35 @@ impl<'a> WatchState<'a> {
 
         self.writer.execute(Clear(ClearType::All))?;
 
-        if let Some(stdout) = &self.stdout {
-            self.writer.write_all(stdout)?;
-            self.writer.write_all(b"\n")?;
-        }
-
-        if let Some(stderr) = &self.stderr {
-            self.writer.write_all(stderr)?;
-            self.writer.write_all(b"\n")?;
-        }
-
+        self.writer.write_all(&self.output)?;
         self.writer.write_all(b"\n")?;
 
         if self.show_hint {
-            self.writer.write_fmt(format_args!(
-                "{}\n{}\n\n",
+            writeln!(
+                self.writer,
+                "{}\n{}\n",
                 "Hint".bold().cyan().underlined(),
                 self.app_state.current_exercise().hint,
-            ))?;
+            )?;
         }
 
         if !matches!(self.done_status, DoneStatus::Pending) {
-            self.writer.write_fmt(format_args!(
-                "{}\n\n",
+            writeln!(
+                self.writer,
+                "{}\n",
                 "Exercise done ✓
 When you are done experimenting, enter `n` or `next` to go to the next exercise 🦀"
                     .bold()
                     .green(),
-            ))?;
+            )?;
         }
 
         if let DoneStatus::DoneWithSolution(solution_path) = &self.done_status {
-            self.writer.write_fmt(format_args!(
-                "A solution file can be found at {}\n\n",
+            writeln!(
+                self.writer,
+                "A solution file can be found at {}\n",
                 style(TerminalFileLink(solution_path)).underlined().green()
-            ))?;
+            )?;
         }
 
         let line_width = size()?.0;
@@ -159,10 +147,11 @@ When you are done experimenting, enter `n` or `next` to go to the next exercise
             self.app_state.exercises().len() as u16,
             line_width,
         )?;
-        self.writer.write_fmt(format_args!(
-            "{progress_bar}Current exercise: {}\n",
+        writeln!(
+            self.writer,
+            "{progress_bar}Current exercise: {}",
             self.app_state.current_exercise().terminal_link(),
-        ))?;
+        )?;
 
         self.show_prompt()?;