# HG changeset patch
# User Mitchell Kember <mkember@janestreet.com>
# Date 1740500964 18000
#      Tue Feb 25 11:29:24 2025 -0500
# Node ID 08500079488831c37eec8c08ffb8930c55c02a37
# Parent  d81714a1c88d5143d3999afaa6e4eab4f32f590e
# EXP-Topic rust-whitespace
rust-utils: remove CleanWhitespace::None

Instead we use Option<CleanWhitespace>. This is in preparation for a follow-up
change that reimplements clean_whitespace, where the None case felt awkward.

diff --git a/rust/hg-core/src/operations/annotate.rs b/rust/hg-core/src/operations/annotate.rs
--- a/rust/hg-core/src/operations/annotate.rs
+++ b/rust/hg-core/src/operations/annotate.rs
@@ -26,7 +26,7 @@
 pub struct AnnotateOptions {
     pub treat_binary_as_text: bool,
     pub follow_copies: bool,
-    pub whitespace: CleanWhitespace,
+    pub whitespace: Option<CleanWhitespace>,
 }
 
 /// The final result of annotating a file.
@@ -72,11 +72,14 @@
     /// Cleans `data` based on `whitespace` and then splits into lines.
     fn split(
         data: Vec<u8>,
-        whitespace: CleanWhitespace,
+        whitespace: Option<CleanWhitespace>,
     ) -> Result<Self, HgError> {
-        let data = match clean_whitespace(&data, whitespace) {
-            Cow::Borrowed(_) => data,
-            Cow::Owned(data) => data,
+        let data = match whitespace {
+            None => data,
+            Some(ws) => match clean_whitespace(&data, ws) {
+                Cow::Borrowed(_) => data,
+                Cow::Owned(data) => data,
+            },
         };
         Self::try_new(data, |data| bdiff::split_lines(data))
     }
@@ -323,11 +326,8 @@
     // Step 3: Read files and split lines. Do the base file with and without
     // whitespace cleaning. Do the rest of the files in parallel with rayon.
     let base_file_original_lines = match options.whitespace {
-        CleanWhitespace::None => None,
-        _ => Some(OwnedLines::split(
-            base_file_data.clone(),
-            CleanWhitespace::None,
-        )?),
+        None => None,
+        _ => Some(OwnedLines::split(base_file_data.clone(), None)?),
     };
     graph[base_id].file = AnnotatedFileState::Read(OwnedLines::split(
         base_file_data,
diff --git a/rust/hg-core/src/utils/strings.rs b/rust/hg-core/src/utils/strings.rs
--- a/rust/hg-core/src/utils/strings.rs
+++ b/rust/hg-core/src/utils/strings.rs
@@ -304,8 +304,6 @@
 /// Options for [`clean_whitespace`].
 #[derive(Copy, Clone)]
 pub enum CleanWhitespace {
-    /// Do nothing.
-    None,
     /// Remove whitespace at ends of lines.
     AtEol,
     /// Collapse consecutive whitespace characters into a single space.
@@ -326,7 +324,6 @@
             Regex::new(r"[ \t\r]+").expect("valid regex");
     }
     let replacement: &[u8] = match how {
-        CleanWhitespace::None => return Cow::Borrowed(text),
         CleanWhitespace::AtEol => return AT_EOL.replace_all(text, b""),
         CleanWhitespace::Collapse => b" ",
         CleanWhitespace::All => b"",
diff --git a/rust/rhg/src/commands/annotate.rs b/rust/rhg/src/commands/annotate.rs
--- a/rust/rhg/src/commands/annotate.rs
+++ b/rust/rhg/src/commands/annotate.rs
@@ -161,15 +161,15 @@
         treat_binary_as_text: args.get_flag("text"),
         follow_copies: !args.get_flag("no-follow"),
         whitespace: if args.get_flag("ignore-all-space") {
-            CleanWhitespace::All
+            Some(CleanWhitespace::All)
         } else if args.get_flag("ignore-space-change") {
-            CleanWhitespace::Collapse
+            Some(CleanWhitespace::Collapse)
         } else if args.get_flag("ignore-space-at-eol") {
-            CleanWhitespace::AtEol
+            Some(CleanWhitespace::AtEol)
         } else {
             // We ignore the --ignore-blank-lines flag (present for consistency
             // with other commands) since it has no effect on annotate.
-            CleanWhitespace::None
+            None
         },
     };
 
# HG changeset patch
# User Mitchell Kember <mkember@janestreet.com>
# Date 1740500698 18000
#      Tue Feb 25 11:24:58 2025 -0500
# Node ID 6c894c2129a814576f9a54b5260e730e75034750
# Parent  08500079488831c37eec8c08ffb8930c55c02a37
# EXP-Topic rust-whitespace
rust-utils: make clean_whitespace mutate input

This makes clean_whitespace take a &mut Vec<u8> instead of returning Cow<[u8]>.
Where before it returned Cow::Borrowed, now it is a no-op.

This is in preparation for a follow-up change that reimplements clean_whitespace
without regexes.

diff --git a/rust/hg-core/src/operations/annotate.rs b/rust/hg-core/src/operations/annotate.rs
--- a/rust/hg-core/src/operations/annotate.rs
+++ b/rust/hg-core/src/operations/annotate.rs
@@ -1,5 +1,3 @@
-use std::borrow::Cow;
-
 use crate::{
     bdiff::{self, Lines},
     errors::HgError,
@@ -71,16 +69,12 @@
 impl OwnedLines {
     /// Cleans `data` based on `whitespace` and then splits into lines.
     fn split(
-        data: Vec<u8>,
+        mut data: Vec<u8>,
         whitespace: Option<CleanWhitespace>,
     ) -> Result<Self, HgError> {
-        let data = match whitespace {
-            None => data,
-            Some(ws) => match clean_whitespace(&data, ws) {
-                Cow::Borrowed(_) => data,
-                Cow::Owned(data) => data,
-            },
-        };
+        if let Some(ws) = whitespace {
+            clean_whitespace(&mut data, ws);
+        }
         Self::try_new(data, |data| bdiff::split_lines(data))
     }
 
diff --git a/rust/hg-core/src/utils/strings.rs b/rust/hg-core/src/utils/strings.rs
--- a/rust/hg-core/src/utils/strings.rs
+++ b/rust/hg-core/src/utils/strings.rs
@@ -313,8 +313,7 @@
 }
 
 /// Normalizes whitespace in text so that it won't apppear in diffs.
-/// Returns `Cow::Borrowed(text)` if the result is unchanged.
-pub fn clean_whitespace(text: &[u8], how: CleanWhitespace) -> Cow<[u8]> {
+pub fn clean_whitespace(text: &mut Vec<u8>, how: CleanWhitespace) {
     lazy_static! {
         // To match wsclean in mdiff.py, this includes "\f".
         static ref AT_EOL: Regex =
@@ -324,24 +323,23 @@
             Regex::new(r"[ \t\r]+").expect("valid regex");
     }
     let replacement: &[u8] = match how {
-        CleanWhitespace::AtEol => return AT_EOL.replace_all(text, b""),
+        CleanWhitespace::AtEol => {
+            replace_all_mut(&AT_EOL, text, b"");
+            return;
+        }
         CleanWhitespace::Collapse => b" ",
         CleanWhitespace::All => b"",
     };
-    let text = MULTIPLE.replace_all(text, replacement);
-    replace_all_cow(&AT_EOL, text, b"")
+    replace_all_mut(&MULTIPLE, text, replacement);
+    replace_all_mut(&AT_EOL, text, b"");
 }
 
-/// Helper to call [`Regex::replace_all`] with `Cow` as input and output.
-fn replace_all_cow<'a>(
-    regex: &Regex,
-    haystack: Cow<'a, [u8]>,
-    replacement: &[u8],
-) -> Cow<'a, [u8]> {
-    match haystack {
-        Cow::Borrowed(haystack) => regex.replace_all(haystack, replacement),
-        Cow::Owned(haystack) => {
-            Cow::Owned(regex.replace_all(&haystack, replacement).into_owned())
+/// Helper to make [`Regex::replace_all`] mutate a vector.
+fn replace_all_mut(regex: &Regex, haystack: &mut Vec<u8>, replacement: &[u8]) {
+    match regex.replace_all(haystack, replacement) {
+        Cow::Borrowed(_) => {}
+        Cow::Owned(result) => {
+            *haystack = result;
         }
     }
 }
# HG changeset patch
# User Mitchell Kember <mkember@janestreet.com>
# Date 1741017527 18000
#      Mon Mar 03 10:58:47 2025 -0500
# Node ID 422c8b0a17ab172938a878959b02e5f3219eaef6
# Parent  6c894c2129a814576f9a54b5260e730e75034750
# EXP-Topic rust-whitespace
rust-utils: add tests for clean_whitespace

This is in preparation for a follow-up change that reimplements the function.

diff --git a/rust/hg-core/src/utils/strings.rs b/rust/hg-core/src/utils/strings.rs
--- a/rust/hg-core/src/utils/strings.rs
+++ b/rust/hg-core/src/utils/strings.rs
@@ -373,4 +373,57 @@
         assert_eq!(short_user(b"First Last <user@example.com>"), b"user");
         assert_eq!(short_user(b"First Last <user.name@example.com>"), b"user");
     }
+
+    fn clean_ws(text: &[u8], how: CleanWhitespace) -> Vec<u8> {
+        let mut vec = text.to_vec();
+        clean_whitespace(&mut vec, how);
+        vec
+    }
+
+    #[test]
+    fn test_clean_whitespace_at_eol() {
+        // To match wsclean in mdiff.py, CleanWhitespace::AtEol only removes
+        // the final line's trailing whitespace if it ends in \n.
+        use CleanWhitespace::AtEol;
+        assert_eq!(clean_ws(b"", AtEol), b"");
+        assert_eq!(clean_ws(b" ", AtEol), b" ");
+        assert_eq!(clean_ws(b"  ", AtEol), b"  ");
+        assert_eq!(clean_ws(b"A", AtEol), b"A");
+        assert_eq!(clean_ws(b"\n\n\n", AtEol), b"\n\n\n");
+        assert_eq!(clean_ws(b" \n", AtEol), b"\n");
+        assert_eq!(clean_ws(b"A \n", AtEol), b"A\n");
+        assert_eq!(clean_ws(b"A B  C\t\r\n", AtEol), b"A B  C\n");
+        assert_eq!(clean_ws(b"A \tB  C\r\nD  ", AtEol), b"A \tB  C\nD  ");
+        assert_eq!(clean_ws(b"A\x0CB\x0C\n", AtEol), b"A\x0CB\n");
+    }
+
+    #[test]
+    fn test_clean_whitespace_collapse() {
+        use CleanWhitespace::Collapse;
+        assert_eq!(clean_ws(b"", Collapse), b"");
+        assert_eq!(clean_ws(b" ", Collapse), b" ");
+        assert_eq!(clean_ws(b"  ", Collapse), b" ");
+        assert_eq!(clean_ws(b"A", Collapse), b"A");
+        assert_eq!(clean_ws(b"\n\n\n", Collapse), b"\n\n\n");
+        assert_eq!(clean_ws(b" \n", Collapse), b"\n");
+        assert_eq!(clean_ws(b"A \n", Collapse), b"A\n");
+        assert_eq!(clean_ws(b"A B  C\t\r\n", Collapse), b"A B C\n");
+        assert_eq!(clean_ws(b"A \tB  C\r\nD  ", Collapse), b"A B C\nD ");
+        assert_eq!(clean_ws(b"A\x0CB\x0C\n", Collapse), b"A\x0CB\x0C\n");
+    }
+
+    #[test]
+    fn test_clean_whitespace_all() {
+        use CleanWhitespace::All;
+        assert_eq!(clean_ws(b"", All), b"");
+        assert_eq!(clean_ws(b" ", All), b"");
+        assert_eq!(clean_ws(b"  ", All), b"");
+        assert_eq!(clean_ws(b"A", All), b"A");
+        assert_eq!(clean_ws(b"\n\n\n", All), b"\n\n\n");
+        assert_eq!(clean_ws(b" \n", All), b"\n");
+        assert_eq!(clean_ws(b"A \n", All), b"A\n");
+        assert_eq!(clean_ws(b"A B  C\t\r\n", All), b"ABC\n");
+        assert_eq!(clean_ws(b"A \tB  C\r\nD  ", All), b"ABC\nD");
+        assert_eq!(clean_ws(b"A\x0CB\x0C\n", All), b"A\x0CB\x0C\n");
+    }
 }
# HG changeset patch
# User Mitchell Kember <mkember@janestreet.com>
# Date 1741017530 18000
#      Mon Mar 03 10:58:50 2025 -0500
# Node ID cc30bead2d6153a7477785aa1495e5f6f155d62e
# Parent  422c8b0a17ab172938a878959b02e5f3219eaef6
# EXP-Topic rust-whitespace
rust-utils: reimplement clean_whitespace without regex

This is similar to fixws in mercurial/cext/bdiff.c, except in addition to
--ignore-all-space (-w) and --ignore-space-change (-b), it also handles
--ignore-ws-at-eol (-Z). Unlike the regex, it operates in-place.

For files with large histories such as mercurial/commands.py, it brings the
penalty of -w and -b from 25% down to around 10%. For -Z, and for files with
small histories, it makes little difference.

Here are some poulpe benchmarks on the mercurial repo for the stack of four
changes in this merge request (!1299):

### benchmark.name                = hg.command.annotate
  # data-env-vars.name            = mercurial-devel
  # bin-env-vars.hg.flavor        = rhg

 ## benchmark.variants.files      = 100-random-files
 ## benchmark.variants.flags      = default
0471bf04ddf1: 0.647526  ~~~~~
366d3433bed2: 0.633503   (-2.17%, -0.01)
 ## benchmark.variants.files      = 100-random-files
 ## benchmark.variants.flags      = -Z
0471bf04ddf1: 0.649229  ~~~~~
366d3433bed2: 0.642296   (-1.07%, -0.01)
 ## benchmark.variants.files      = 100-random-files
 ## benchmark.variants.flags      = -b
0471bf04ddf1: 0.675751  ~~~~~
366d3433bed2: 0.645437   (-4.49%, -0.03)
 ## benchmark.variants.files      = 100-random-files
 ## benchmark.variants.flags      = -w
0471bf04ddf1: 0.678141  ~~~~~
366d3433bed2: 0.645375   (-4.83%, -0.03)
 ## benchmark.variants.files      = file-with-long-history.a
 ## benchmark.variants.flags      = default
0471bf04ddf1: 1.700064  ~~~~~
366d3433bed2: 1.695099
 ## benchmark.variants.files      = file-with-long-history.a
 ## benchmark.variants.flags      = -Z
0471bf04ddf1: 1.682014  ~~~~~
366d3433bed2: 1.697669
 ## benchmark.variants.files      = file-with-long-history.a
 ## benchmark.variants.flags      = -b
0471bf04ddf1: 2.140711  ~~~~~
366d3433bed2: 1.857253  (-13.24%, -0.28)
 ## benchmark.variants.files      = file-with-long-history.a
 ## benchmark.variants.flags      = -w
0471bf04ddf1: 2.127745  ~~~~~
366d3433bed2: 1.814402  (-14.73%, -0.31)
 ## benchmark.variants.files      = file-with-long-history.b
 ## benchmark.variants.flags      = default
0471bf04ddf1: 0.263050  ~~~~~
366d3433bed2: 0.263603
 ## benchmark.variants.files      = file-with-long-history.b
 ## benchmark.variants.flags      = -Z
0471bf04ddf1: 0.261817  ~~~~~
366d3433bed2: 0.263896
 ## benchmark.variants.files      = file-with-long-history.b
 ## benchmark.variants.flags      = -b
0471bf04ddf1: 0.286191  ~~~~~
366d3433bed2: 0.276698   (-3.32%, -0.01)
 ## benchmark.variants.files      = file-with-long-history.b
 ## benchmark.variants.flags      = -w
0471bf04ddf1: 0.282997  ~~~~~
366d3433bed2: 0.275538   (-2.64%, -0.01)
 ## benchmark.variants.files      = file-with-long-history.c
 ## benchmark.variants.flags      = default
0471bf04ddf1: 0.318708  ~~~~~
366d3433bed2: 0.318006
 ## benchmark.variants.files      = file-with-long-history.c
 ## benchmark.variants.flags      = -Z
0471bf04ddf1: 0.323910  ~~~~~
366d3433bed2: 0.318743   (-1.60%, -0.01)
 ## benchmark.variants.files      = file-with-long-history.c
 ## benchmark.variants.flags      = -b
0471bf04ddf1: 0.390691  ~~~~~
366d3433bed2: 0.372053   (-4.77%, -0.02)
 ## benchmark.variants.files      = file-with-long-history.c
 ## benchmark.variants.flags      = -w
0471bf04ddf1: 0.407243  ~~~~~
366d3433bed2: 0.403874
 ## benchmark.variants.files      = file-with-long-history.d
 ## benchmark.variants.flags      = default
0471bf04ddf1: 0.295730  ~~~~~
366d3433bed2: 0.269190   (-8.97%, -0.03)
 ## benchmark.variants.files      = file-with-long-history.d
 ## benchmark.variants.flags      = -Z
0471bf04ddf1: 0.276715  ~~~~~
366d3433bed2: 0.271512   (-1.88%, -0.01)
 ## benchmark.variants.files      = file-with-long-history.d
 ## benchmark.variants.flags      = -b
0471bf04ddf1: 0.283624  ~~~~~
366d3433bed2: 0.278003   (-1.98%, -0.01)
 ## benchmark.variants.files      = file-with-long-history.d
 ## benchmark.variants.flags      = -w
0471bf04ddf1: 0.286407  ~~~~~
366d3433bed2: 0.276957   (-3.30%, -0.01)
 ## benchmark.variants.files      = file-with-long-history.e
 ## benchmark.variants.flags      = default
0471bf04ddf1: 0.349047  ~~~~~
366d3433bed2: 0.331948   (-4.90%, -0.02)
 ## benchmark.variants.files      = file-with-long-history.e
 ## benchmark.variants.flags      = -Z
0471bf04ddf1: 0.359457  ~~~~~
366d3433bed2: 0.334011   (-7.08%, -0.03)
 ## benchmark.variants.files      = file-with-long-history.e
 ## benchmark.variants.flags      = -b
0471bf04ddf1: 0.373671  ~~~~~
366d3433bed2: 0.347020   (-7.13%, -0.03)
 ## benchmark.variants.files      = file-with-long-history.e
 ## benchmark.variants.flags      = -w
0471bf04ddf1: 0.374186  ~~~~~
366d3433bed2: 0.347367   (-7.17%, -0.03)

diff --git a/rust/hg-core/src/utils/strings.rs b/rust/hg-core/src/utils/strings.rs
--- a/rust/hg-core/src/utils/strings.rs
+++ b/rust/hg-core/src/utils/strings.rs
@@ -1,9 +1,7 @@
 //! Contains string-related utilities.
 
 use crate::utils::hg_path::HgPath;
-use lazy_static::lazy_static;
-use regex::bytes::Regex;
-use std::{borrow::Cow, cell::Cell, fmt, io::Write as _, ops::Deref as _};
+use std::{cell::Cell, fmt, io::Write as _, ops::Deref as _};
 
 /// Useful until rust/issues/56345 is stable
 ///
@@ -314,34 +312,61 @@
 
 /// Normalizes whitespace in text so that it won't apppear in diffs.
 pub fn clean_whitespace(text: &mut Vec<u8>, how: CleanWhitespace) {
-    lazy_static! {
-        // To match wsclean in mdiff.py, this includes "\f".
-        static ref AT_EOL: Regex =
-            Regex::new(r"(?m)[ \t\r\f]+$").expect("valid regex");
-        // To match fixws in cext/bdiff.c, this does *not* include "\f".
-        static ref MULTIPLE: Regex =
-            Regex::new(r"[ \t\r]+").expect("valid regex");
-    }
-    let replacement: &[u8] = match how {
+    // We copy text[i] to text[w], where w advances more slowly than i.
+    let mut w = 0;
+    match how {
+        // To match wsclean in mdiff.py, this removes "\f" (0xC).
         CleanWhitespace::AtEol => {
-            replace_all_mut(&AT_EOL, text, b"");
-            return;
+            let mut newline_dest = 0;
+            for i in 0..text.len() {
+                let char = text[i];
+                match char {
+                    b' ' | b'\t' | b'\r' | 0xC => {}
+                    _ => {
+                        if char == b'\n' {
+                            w = newline_dest;
+                        }
+                        newline_dest = w + 1;
+                    }
+                }
+                text[w] = char;
+                w += 1;
+            }
         }
-        CleanWhitespace::Collapse => b" ",
-        CleanWhitespace::All => b"",
-    };
-    replace_all_mut(&MULTIPLE, text, replacement);
-    replace_all_mut(&AT_EOL, text, b"");
-}
-
-/// Helper to make [`Regex::replace_all`] mutate a vector.
-fn replace_all_mut(regex: &Regex, haystack: &mut Vec<u8>, replacement: &[u8]) {
-    match regex.replace_all(haystack, replacement) {
-        Cow::Borrowed(_) => {}
-        Cow::Owned(result) => {
-            *haystack = result;
+        // To match fixws in cext/bdiff.c, CleanWhitespace::Collapse and
+        // CleanWhitespace::All do *not* remove "\f".
+        CleanWhitespace::Collapse => {
+            for i in 0..text.len() {
+                match text[i] {
+                    b' ' | b'\t' | b'\r' => {
+                        if w == 0 || text[w - 1] != b' ' {
+                            text[w] = b' ';
+                            w += 1;
+                        }
+                    }
+                    b'\n' if w > 0 && text[w - 1] == b' ' => {
+                        text[w - 1] = b'\n';
+                    }
+                    char => {
+                        text[w] = char;
+                        w += 1;
+                    }
+                }
+            }
+        }
+        CleanWhitespace::All => {
+            for i in 0..text.len() {
+                match text[i] {
+                    b' ' | b'\t' | b'\r' => {}
+                    char => {
+                        text[w] = char;
+                        w += 1;
+                    }
+                }
+            }
         }
     }
+    text.truncate(w);
 }
 
 #[cfg(test)]