printer: trim before applying max column windowing

Previously, we were applying the -M/--max-columns flag *before* triming
prefix ASCII whitespace. But this doesn't make a whole lot of sense. We
should be trimming first, but the result of trimming is ultimately what
we'll be printing and that's what -M/--max-columns should be applied to.

Fixes #2458

3 files changed, 122 insertions, 15 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index b68c3220..bb46bbad 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -77,6 +77,8 @@ Bug fixes:
   Make `-p/--pretty` override flags like `--no-line-number`.
 * [BUG #2392](https://github.com/BurntSushi/ripgrep/issues/2392):
   Improve global git config parsing of the `excludesFile` field.
+* [BUG #2458](https://github.com/BurntSushi/ripgrep/pull/2458):
+  Make `--trim` run before `-M/--max-columns` takes effect.
 * [BUG #2479](https://github.com/BurntSushi/ripgrep/issues/2479):
   Add documentation about `.ignore`/`.rgignore` files in parent directories.
 * [BUG #2480](https://github.com/BurntSushi/ripgrep/issues/2480):
diff --git a/crates/printer/src/standard.rs b/crates/printer/src/standard.rs
index 2287b5d7..78489906 100644
--- a/crates/printer/src/standard.rs
+++ b/crates/printer/src/standard.rs
@@ -1100,13 +1100,14 @@ impl<'a, M: Matcher, W: WriteColor> StandardImpl<'a, M, W> {
         let mut count = 0;
         let mut stepper = LineStep::new(line_term, 0, bytes.len());
         while let Some((start, end)) = stepper.next(bytes) {
-            let line = Match::new(start, end);
+            let mut line = Match::new(start, end);
             self.write_prelude(
                 self.sunk.absolute_byte_offset() + line.start() as u64,
                 self.sunk.line_number().map(|n| n + count),
                 Some(matches[0].start() as u64 + 1),
             )?;
             count += 1;
+            self.trim_ascii_prefix(bytes, &mut line);
             if self.exceeds_max_columns(&bytes[line]) {
                 self.write_exceeded_line(bytes, line, matches, &mut midx)?;
             } else {
@@ -1189,12 +1190,12 @@ impl<'a, M: Matcher, W: WriteColor> StandardImpl<'a, M, W> {
                     Some(m.start().saturating_sub(line.start()) as u64 + 1),
                 )?;
                 count += 1;
+                self.trim_line_terminator(bytes, &mut line);
+                self.trim_ascii_prefix(bytes, &mut line);
                 if self.exceeds_max_columns(&bytes[line]) {
                     self.write_exceeded_line(bytes, line, &[m], &mut 0)?;
                     continue;
                 }
-                self.trim_line_terminator(bytes, &mut line);
-                self.trim_ascii_prefix(bytes, &mut line);
 
                 while !line.is_empty() {
                     if m.end() <= line.start() {
@@ -1246,6 +1247,14 @@ impl<'a, M: Matcher, W: WriteColor> StandardImpl<'a, M, W> {
 
     #[inline(always)]
     fn write_line(&self, line: &[u8]) -> io::Result<()> {
+        let line = if !self.config().trim_ascii {
+            line
+        } else {
+            let lineterm = self.searcher.line_terminator();
+            let full_range = Match::new(0, line.len());
+            let range = trim_ascii_prefix(lineterm, line, full_range);
+            &line[range]
+        };
         if self.exceeds_max_columns(line) {
             let range = Match::new(0, line.len());
             self.write_exceeded_line(
@@ -1255,7 +1264,8 @@ impl<'a, M: Matcher, W: WriteColor> StandardImpl<'a, M, W> {
                 &mut 0,
             )?;
         } else {
-            self.write_trim(line)?;
+            // self.write_trim(line)?;
+            self.write(line)?;
             if !self.has_line_terminator(line) {
                 self.write_line_term()?;
             }
@@ -1274,7 +1284,8 @@ impl<'a, M: Matcher, W: WriteColor> StandardImpl<'a, M, W> {
             return self.write_line(bytes);
         }
 
-        let line = Match::new(0, bytes.len());
+        let mut line = Match::new(0, bytes.len());
+        self.trim_ascii_prefix(bytes, &mut line);
         if self.exceeds_max_columns(bytes) {
             self.write_exceeded_line(bytes, line, matches, &mut 0)
         } else {
@@ -1298,7 +1309,6 @@ impl<'a, M: Matcher, W: WriteColor> StandardImpl<'a, M, W> {
         match_index: &mut usize,
     ) -> io::Result<()> {
         self.trim_line_terminator(bytes, &mut line);
-        self.trim_ascii_prefix(bytes, &mut line);
         if matches.is_empty() {
             self.write(&bytes[line])?;
             return Ok(());
@@ -1535,15 +1545,6 @@ impl<'a, M: Matcher, W: WriteColor> StandardImpl<'a, M, W> {
         Ok(())
     }
 
-    fn write_trim(&self, buf: &[u8]) -> io::Result<()> {
-        if !self.config().trim_ascii {
-            return self.write(buf);
-        }
-        let mut range = Match::new(0, buf.len());
-        self.trim_ascii_prefix(buf, &mut range);
-        self.write(&buf[range])
-    }
-
     fn write(&self, buf: &[u8]) -> io::Result<()> {
         self.wtr().borrow_mut().write_all(buf)
     }
diff --git a/tests/feature.rs b/tests/feature.rs
index 5321d110..ce5459c2 100644
--- a/tests/feature.rs
+++ b/tests/feature.rs
@@ -657,6 +657,110 @@ but Doctor Watson has to have it taken out for him and dusted,
     eqnice!(expected, cmd.stdout());
 });
 
+rgtest!(f917_trim_multi_standard, |dir: Dir, mut cmd: TestCommand| {
+    const HAYSTACK: &str = "     0123456789abcdefghijklmnopqrstuvwxyz";
+    dir.create("haystack", HAYSTACK);
+    cmd.args(&["--multiline", "--trim", "-r$0", "--no-filename", r"a\n?bc"]);
+
+    let expected = "0123456789abcdefghijklmnopqrstuvwxyz\n";
+    eqnice!(expected, cmd.stdout());
+});
+
+rgtest!(f917_trim_max_columns_normal, |dir: Dir, mut cmd: TestCommand| {
+    const HAYSTACK: &str = "     0123456789abcdefghijklmnopqrstuvwxyz";
+    dir.create("haystack", HAYSTACK);
+    cmd.args(&[
+        "--trim",
+        "--max-columns-preview",
+        "-M8",
+        "--no-filename",
+        "abc",
+    ]);
+
+    let expected = "01234567 [... omitted end of long line]\n";
+    eqnice!(expected, cmd.stdout());
+});
+
+rgtest!(f917_trim_max_columns_matches, |dir: Dir, mut cmd: TestCommand| {
+    const HAYSTACK: &str = "     0123456789abcdefghijklmnopqrstuvwxyz";
+    dir.create("haystack", HAYSTACK);
+    cmd.args(&[
+        "--trim",
+        "--max-columns-preview",
+        "-M8",
+        "--color=always",
+        "--colors=path:none",
+        "--no-filename",
+        "abc",
+    ]);
+
+    let expected = "01234567 [... 1 more match]\n";
+    eqnice!(expected, cmd.stdout());
+});
+
+rgtest!(
+    f917_trim_max_columns_multi_standard,
+    |dir: Dir, mut cmd: TestCommand| {
+        const HAYSTACK: &str = "     0123456789abcdefghijklmnopqrstuvwxyz";
+        dir.create("haystack", HAYSTACK);
+        cmd.args(&[
+            "--multiline",
+            "--trim",
+            "--max-columns-preview",
+            "-M8",
+            // Force the "slow" printing path without actually
+            // putting colors in the output.
+            "--color=always",
+            "--colors=path:none",
+            "--no-filename",
+            r"a\n?bc",
+        ]);
+
+        let expected = "01234567 [... 1 more match]\n";
+        eqnice!(expected, cmd.stdout());
+    }
+);
+
+rgtest!(
+    f917_trim_max_columns_multi_only_matching,
+    |dir: Dir, mut cmd: TestCommand| {
+        const HAYSTACK: &str = "     0123456789abcdefghijklmnopqrstuvwxyz";
+        dir.create("haystack", HAYSTACK);
+        cmd.args(&[
+            "--multiline",
+            "--trim",
+            "--max-columns-preview",
+            "-M8",
+            "--only-matching",
+            "--no-filename",
+            r".*a\n?bc.*",
+        ]);
+
+        let expected = "01234567 [... 0 more matches]\n";
+        eqnice!(expected, cmd.stdout());
+    }
+);
+
+rgtest!(
+    f917_trim_max_columns_multi_per_match,
+    |dir: Dir, mut cmd: TestCommand| {
+        const HAYSTACK: &str = "     0123456789abcdefghijklmnopqrstuvwxyz";
+        dir.create("haystack", HAYSTACK);
+        cmd.args(&[
+            "--multiline",
+            "--trim",
+            "--max-columns-preview",
+            "-M8",
+            "--vimgrep",
+            "--no-filename",
+            r".*a\n?bc.*",
+        ]);
+
+        let expected = "1:1:01234567 [... 0 more matches]\n";
+        eqnice!(expected, cmd.stdout());
+    }
+);
+
 // See: https://github.com/BurntSushi/ripgrep/issues/993
 rgtest!(f993_null_data, |dir: Dir, mut cmd: TestCommand| {
     dir.create("test", "foo\x00bar\x00\x00\x00baz\x00");