Merge pull request #1912 from mo8it/performance

Optimizations 1

Author: mo8it

Cargo.lock

@@ -14,12 +14,12 @@ console = "0.15.8"
 glob = "0.3.0"
 indicatif = "0.17.8"
 notify-debouncer-mini = "0.4.1"
-regex = "1.10.3"
 serde_json = "1.0.114"
 serde = { version = "1.0.197", features = ["derive"] }
 shlex = "1.3.0"
 toml_edit = { version = "0.22.9", default-features = false, features = ["parse", "serde"] }
 which = "6.0.1"
+winnow = "0.6.5"
 
 [[bin]]
 name = "rustlings"

Cargo.toml

@@ -1,19 +1,33 @@
-use regex::Regex;
 use serde::Deserialize;
-use std::env;
 use std::fmt::{self, Display, Formatter};
 use std::fs::{self, remove_file, File};
-use std::io::Read;
+use std::io::{self, BufRead, BufReader};
 use std::path::PathBuf;
-use std::process::{self, Command, Stdio};
+use std::process::{self, exit, Command, Stdio};
+use std::{array, env, mem};
+use winnow::ascii::{space0, Caseless};
+use winnow::combinator::opt;
+use winnow::Parser;
 
 const RUSTC_COLOR_ARGS: &[&str] = &["--color", "always"];
 const RUSTC_EDITION_ARGS: &[&str] = &["--edition", "2021"];
 const RUSTC_NO_DEBUG_ARGS: &[&str] = &["-C", "strip=debuginfo"];
-const I_AM_DONE_REGEX: &str = r"(?m)^\s*///?\s*I\s+AM\s+NOT\s+DONE";
 const CONTEXT: usize = 2;
 const CLIPPY_CARGO_TOML_PATH: &str = "./exercises/22_clippy/Cargo.toml";
 
+// Checks if the line contains the "I AM NOT DONE" comment.
+fn contains_not_done_comment(input: &str) -> bool {
+    (
+        space0::<_, ()>,
+        "//",
+        opt('/'),
+        space0,
+        Caseless("I AM NOT DONE"),
+    )
+        .parse_next(&mut &*input)
+        .is_ok()
+}
+
 // Get a temporary file name that is hopefully unique
 #[inline]
 fn temp_file() -> String {
@@ -211,51 +225,101 @@ path = "{}.rs""#,
     }
 
     pub fn state(&self) -> State {
-        let mut source_file = File::open(&self.path).unwrap_or_else(|e| {
-            panic!(
-                "We were unable to open the exercise file {}! {e}",
-                self.path.display()
-            )
+        let source_file = File::open(&self.path).unwrap_or_else(|e| {
+            println!(
+                "Failed to open the exercise file {}: {e}",
+                self.path.display(),
+            );
+            exit(1);
         });
-
-        let source = {
-            let mut s = String::new();
-            source_file.read_to_string(&mut s).unwrap_or_else(|e| {
-                panic!(
-                    "We were unable to read the exercise file {}! {e}",
-                    self.path.display()
-                )
-            });
-            s
+        let mut source_reader = BufReader::new(source_file);
+
+        // Read the next line into `buf` without the newline at the end.
+        let mut read_line = |buf: &mut String| -> io::Result<_> {
+            let n = source_reader.read_line(buf)?;
+            if buf.ends_with('\n') {
+                buf.pop();
+                if buf.ends_with('\r') {
+                    buf.pop();
+                }
+            }
+            Ok(n)
         };
 
-        let re = Regex::new(I_AM_DONE_REGEX).unwrap();
+        let mut current_line_number: usize = 1;
+        // Keep the last `CONTEXT` lines while iterating over the file lines.
+        let mut prev_lines: [_; CONTEXT] = array::from_fn(|_| String::with_capacity(256));
+        let mut line = String::with_capacity(256);
 
-        if !re.is_match(&source) {
-            return State::Done;
-        }
+        loop {
+            let n = read_line(&mut line).unwrap_or_else(|e| {
+                println!(
+                    "Failed to read the exercise file {}: {e}",
+                    self.path.display(),
+                );
+                exit(1);
+            });
 
-        let matched_line_index = source
-            .lines()
-            .enumerate()
-            .find_map(|(i, line)| if re.is_match(line) { Some(i) } else { None })
-            .expect("This should not happen at all");
-
-        let min_line = ((matched_line_index as i32) - (CONTEXT as i32)).max(0) as usize;
-        let max_line = matched_line_index + CONTEXT;
-
-        let context = source
-            .lines()
-            .enumerate()
-            .filter(|&(i, _)| i >= min_line && i <= max_line)
-            .map(|(i, line)| ContextLine {
-                line: line.to_string(),
-                number: i + 1,
-                important: i == matched_line_index,
-            })
-            .collect();
+            // Reached the end of the file and didn't find the comment.
+            if n == 0 {
+                return State::Done;
+            }
+
+            if contains_not_done_comment(&line) {
+                let mut context = Vec::with_capacity(2 * CONTEXT + 1);
+                // Previous lines.
+                for (ind, prev_line) in prev_lines
+                    .into_iter()
+                    .take(current_line_number - 1)
+                    .enumerate()
+                    .rev()
+                {
+                    context.push(ContextLine {
+                        line: prev_line,
+                        number: current_line_number - 1 - ind,
+                        important: false,
+                    });
+                }
+
+                // Current line.
+                context.push(ContextLine {
+                    line,
+                    number: current_line_number,
+                    important: true,
+                });
+
+                // Next lines.
+                for ind in 0..CONTEXT {
+                    let mut next_line = String::with_capacity(256);
+                    let Ok(n) = read_line(&mut next_line) else {
+                        // If an error occurs, just ignore the next lines.
+                        break;
+                    };
+
+                    // Reached the end of the file.
+                    if n == 0 {
+                        break;
+                    }
+
+                    context.push(ContextLine {
+                        line: next_line,
+                        number: current_line_number + 1 + ind,
+                        important: false,
+                    });
+                }
+
+                return State::Pending(context);
+            }
 
-        State::Pending(context)
+            current_line_number += 1;
+            // Add the current line as a previous line and shift the older lines by one.
+            for prev_line in &mut prev_lines {
+                mem::swap(&mut line, prev_line);
+            }
+            // The current line now contains the oldest previous line.
+            // Recycle it for reading the next line.
+            line.clear();
+        }
     }
 
     // Check that the exercise looks to be solved using self.state()
@@ -381,4 +445,20 @@ mod test {
         let out = exercise.compile().unwrap().run().unwrap();
         assert!(out.stdout.contains("THIS TEST TOO SHALL PASS"));
     }
+
+    #[test]
+    fn test_not_done() {
+        assert!(contains_not_done_comment("// I AM NOT DONE"));
+        assert!(contains_not_done_comment("/// I AM NOT DONE"));
+        assert!(contains_not_done_comment("//  I AM NOT DONE"));
+        assert!(contains_not_done_comment("///  I AM NOT DONE"));
+        assert!(contains_not_done_comment("// I AM NOT DONE "));
+        assert!(contains_not_done_comment("// I AM NOT DONE!"));
+        assert!(contains_not_done_comment("// I am not done"));
+        assert!(contains_not_done_comment("// i am NOT done"));
+
+        assert!(!contains_not_done_comment("I AM NOT DONE"));
+        assert!(!contains_not_done_comment("// NOT DONE"));
+        assert!(!contains_not_done_comment("DONE"));
+    }
 }

src/exercise.rs

@@ -130,31 +130,43 @@ fn main() {
                 println!("{:<17}\t{:<46}\t{:<7}", "Name", "Path", "Status");
             }
             let mut exercises_done: u16 = 0;
-            let filters = filter.clone().unwrap_or_default().to_lowercase();
-            exercises.iter().for_each(|e| {
-                let fname = format!("{}", e.path.display());
+            let lowercase_filter = filter
+                .as_ref()
+                .map(|s| s.to_lowercase())
+                .unwrap_or_default();
+            let filters = lowercase_filter
+                .split(',')
+                .filter_map(|f| {
+                    let f = f.trim();
+                    if f.is_empty() {
+                        None
+                    } else {
+                        Some(f)
+                    }
+                })
+                .collect::<Vec<_>>();
+
+            for exercise in &exercises {
+                let fname = exercise.path.to_string_lossy();
                 let filter_cond = filters
-                    .split(',')
-                    .filter(|f| !f.trim().is_empty())
-                    .any(|f| e.name.contains(f) || fname.contains(f));
-                let status = if e.looks_done() {
+                    .iter()
+                    .any(|f| exercise.name.contains(f) || fname.contains(f));
+                let looks_done = exercise.looks_done();
+                let status = if looks_done {
                     exercises_done += 1;
                     "Done"
                 } else {
                     "Pending"
                 };
-                let solve_cond = {
-                    (e.looks_done() && solved)
-                        || (!e.looks_done() && unsolved)
-                        || (!solved && !unsolved)
-                };
+                let solve_cond =
+                    (looks_done && solved) || (!looks_done && unsolved) || (!solved && !unsolved);
                 if solve_cond && (filter_cond || filter.is_none()) {
                     let line = if paths {
                         format!("{fname}\n")
                     } else if names {
-                        format!("{}\n", e.name)
+                        format!("{}\n", exercise.name)
                     } else {
-                        format!("{:<17}\t{fname:<46}\t{status:<7}\n", e.name)
+                        format!("{:<17}\t{fname:<46}\t{status:<7}\n", exercise.name)
                     };
                     // Somehow using println! leads to the binary panicking
                     // when its output is piped.
@@ -170,7 +182,8 @@ fn main() {
                         });
                     }
                 }
-            });
+            }
+
             let percentage_progress = exercises_done as f32 / exercises.len() as f32 * 100.0;
             println!(
                 "Progress: You completed {} / {} exercises ({:.1} %).",

src/main.rs

@@ -223,7 +223,7 @@ fn prompt_for_completion(
         let formatted_line = if context_line.important {
             format!("{}", style(context_line.line).bold())
         } else {
-            context_line.line.to_string()
+            context_line.line
         };
 
         println!(