printer: fix --vimgrep for multi-line mode

BurntSushi · BurntSushi · commit 94e4b8e30130 · 2021-05-15T08:27:59.000-04:00
It turned out that --vimgrep wasn't quite getting the column of each match correctly. Instead of printing column numbers relative to the current line, it was printing column numbers as byte offsets relative to where the match began. To fix this, we simply subtract the offset of the line number from the beginning of the match. If the beginning of the match came before the start of the current line, then there's really nothing sensible we can do other than to use a column number of 1, which we now document. Interestingly, existing tests were checking that the previous behavior was intended. My only defense is that I somehow tricked myself into thinking it was a byte offset instead of a column number. Kudos to @bfrg for calling this out in #1866: #1866 (comment)
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -21,6 +21,8 @@ Bug fixes:
   Document cygwin path translation behavior in the FAQ.
 * [BUG #1741](https://github.com/BurntSushi/ripgrep/issues/1741):
   Fix stdin detection when using PowerShell in UNIX environments.
+* [BUG #1866](https://github.com/BurntSushi/ripgrep/issues/1866#issuecomment-841635553):
+  Fix bug when computing column numbers in `--vimgrep` mode.
 
 
 12.1.1 (2020-05-29)
diff --git a/crates/printer/src/standard.rs b/crates/printer/src/standard.rs
@@ -222,7 +222,9 @@ impl StandardBuilder {
     /// When multi-line mode is enabled, each match and its accompanying lines
     /// are printed. As with single line matches, if a line contains multiple
     /// matches (even if only partially), then that line is printed once for
-    /// each match it participates in.
+    /// each match it participates in. In multi-line mode, column numbers only
+    /// indicate the start of a match. If a line only continues a match, then
+    /// the column number printed is always `1`.
     pub fn per_match(&mut self, yes: bool) -> &mut StandardBuilder {
         self.config.per_match = yes;
         self
@@ -1090,7 +1092,7 @@ impl<'a, M: Matcher, W: WriteColor> StandardImpl<'a, M, W> {
                 self.write_prelude(
                     self.sunk.absolute_byte_offset() + line.start() as u64,
                     self.sunk.line_number().map(|n| n + count),
-                    Some(m.start() as u64 + 1),
+                    Some(m.start().saturating_sub(line.start()) as u64 + 1),
                 )?;
                 count += 1;
                 if self.exceeds_max_columns(&bytes[line]) {
@@ -2990,9 +2992,9 @@ Holmeses, success in the province of detective work must always
         let got = printer_contents(&mut printer);
         let expected = "\
 1:16:For the Doctor Watsons of this world, as opposed to the Sherlock
-2:16:Holmeses, success in the province of detective work must always
+2:1:Holmeses, success in the province of detective work must always
 5:12:but Doctor Watson has to have it taken out for him and dusted,
-6:12:and exhibited clearly, with a label attached.
+6:1:and exhibited clearly, with a label attached.
 ";
         assert_eq_printed!(expected, got);
     }
@@ -3019,9 +3021,9 @@ Holmeses, success in the province of detective work must always
         let got = printer_contents(&mut printer);
         let expected = "\
 1:16:For the Doctor Watsons of this world, as opposed to the Sherlock
-2:16:Holmeses, success in the province of detective work must always
-2:123:Holmeses, success in the province of detective work must always
-3:123:be, to a very large extent, the result of luck. Sherlock Holmes
+2:1:Holmeses, success in the province of detective work must always
+2:58:Holmeses, success in the province of detective work must always
+3:1:be, to a very large extent, the result of luck. Sherlock Holmes
 ";
         assert_eq_printed!(expected, got);
     }
diff --git a/tests/multiline.rs b/tests/multiline.rs
@@ -77,7 +77,7 @@ rgtest!(vimgrep, |dir: Dir, mut cmd: TestCommand| {
     let expected = "\
 sherlock:1:16:For the Doctor Watsons of this world, as opposed to the Sherlock
 sherlock:1:57:For the Doctor Watsons of this world, as opposed to the Sherlock
-sherlock:2:57:Holmeses, success in the province of detective work must always
+sherlock:2:1:Holmeses, success in the province of detective work must always
 sherlock:3:49:be, to a very large extent, the result of luck. Sherlock Holmes
 sherlock:5:12:but Doctor Watson has to have it taken out for him and dusted,
 ";
diff --git a/tests/regression.rs b/tests/regression.rs
@@ -864,3 +864,21 @@ use B;
     ]);
     eqnice!("2\n", cmd.stdout());
 });
+
+rgtest!(r1866, |dir: Dir, mut cmd: TestCommand| {
+    dir.create("test", "foobar\nfoobar\nfoo quux");
+    cmd.args(&[
+        "--multiline",
+        "--vimgrep",
+        r"foobar\nfoobar\nfoo|quux",
+        "test",
+    ]);
+
+    let expected = "\
+test:1:1:foobar
+test:2:1:foobar
+test:3:1:foo quux
+test:3:5:foo quux
+";
+    eqnice!(expected, cmd.stdout());
+});
