From 2dfffab549ca5cc734b644d8d85868979f7d3976 Mon Sep 17 00:00:00 2001 From: DualFroz Date: Thu, 3 Sep 2026 13:42:11 +0200 Subject: [PATCH] fix(grep): locate code by measuring the grep line prefix get_code_style_sections rebuilt the "path[line-number]" prefix from the parsed path and line number in order to find where the code starts in the raw line. That reconstruction can be shorter than the text which was actually printed: the parsed line number does not record the width of its own text, so an input "007" is rebuilt as "7", and a parsed path may contain a tab, which has already been expanded in the raw line. The slice then started inside the prefix, so the style sections described a different string than the parsed code, and superimpose panicked with "String mismatch encountered while superimposing style sections". Measure the prefix instead: the code is the tail of the line the parser was given, so the prefix is everything before it, and expanding its tabs gives its width in the tab-expanded raw line. For a well formed grep line the result is identical to the previous computation. --- src/handlers/grep.rs | 87 ++++++++++++++++++++++++++++++++++---------- 1 file changed, 68 insertions(+), 19 deletions(-) diff --git a/src/handlers/grep.rs b/src/handlers/grep.rs index 7ed0ba6..5609df0 100644 --- a/src/handlers/grep.rs +++ b/src/handlers/grep.rs @@ -206,13 +206,13 @@ impl StateMachine<'_> { // enough. But at this point it is guaranteed // that this handler is going to handle this // line, so mutating it is acceptable. + let prefix_len = code_prefix_len(&self.line, &grep_line.code, &self.config.tab_cfg); self.raw_line = tabs::expand(&self.raw_line, &self.config.tab_cfg); get_code_style_sections( &self.raw_line, self.config.grep_match_word_style, self.config.grep_match_line_style, - &grep_line.path, - grep_line.line_number, + prefix_len, ) .unwrap_or(StyleSectionSpecifier::Style( self.config.grep_match_line_style, @@ -347,13 +347,13 @@ impl StateMachine<'_> { // enough. But at the point it is guaranteed // that this handler is going to handle this // line, so mutating it is acceptable. + let prefix_len = code_prefix_len(&self.line, &grep_line.code, &self.config.tab_cfg); self.raw_line = tabs::expand(&self.raw_line, &self.config.tab_cfg); get_code_style_sections( &self.raw_line, self.config.grep_match_word_style, self.config.grep_match_line_style, - &grep_line.path, - grep_line.line_number, + prefix_len, ) .unwrap_or(StyleSectionSpecifier::Style( self.config.grep_match_line_style, @@ -393,21 +393,36 @@ fn make_style_sections<'a>( StyleSectionSpecifier::StyleSections(sections) } +// Return the length of the `path[line-number]` prefix which the grep +// tool emitted in front of the code, measured as it appears in the raw line +// after tab expansion. +// +// The prefix cannot be reconstructed from the parsed path and line number: the +// parsed line number does not record how many characters it occupied (e.g. a +// misparse can pick up "007" and report 7), and the parsed path may contain +// tabs, which are expanded in the raw line. `line` must be the ANSI-stripped, +// tab-unexpanded line which `code` was parsed from. +fn code_prefix_len(line: &str, code: &str, tab_cfg: &tabs::TabCfg) -> usize { + match line + .len() + .checked_sub(code.len()) + .and_then(|prefix_len| line.get(..prefix_len)) + { + Some(prefix) => tabs::expand(prefix, tab_cfg).len(), + // `code` is a suffix of `line`, so this is unreachable; return 0, which + // makes `get_code_style_sections` decline to style the line. + None => 0, + } +} + // Return style sections describing colors received from git. fn get_code_style_sections<'b>( raw_line: &'b str, match_style: Style, non_match_style: Style, - path: &str, - line_number: Option, + prefix_len: usize, ) -> Option> { - if let Some(prefix_end) = ansi::ansi_preserving_index( - raw_line, - match line_number { - Some(n) => format!("{path}:{n}:").len() - 1, - None => path.len(), - }, - ) { + if let Some(prefix_end) = ansi::ansi_preserving_index(raw_line, prefix_len.checked_sub(1)?) { let match_style_sections = ansi::parse_style_sections(&raw_line[(prefix_end + 1)..]) .iter() .map(|(ansi_term_style, s)| { @@ -747,6 +762,7 @@ mod tests { use crate::handlers::grep::{ parse_grep_line, parse_raw_grep_line, GrepLine, GrepType, LineType, }; + use crate::tests::integration_test_utils::DeltaTest; use crate::utils::process::tests::FakeParentArgs; #[test] @@ -1252,9 +1268,10 @@ mod tests { #[test] fn test_get_code_style_sections() { use crate::ansi::strip_ansi_codes; - use crate::handlers::grep::get_code_style_sections; + use crate::handlers::grep::{code_prefix_len, get_code_style_sections}; use crate::paint::StyleSectionSpecifier; use crate::style::Style; + use crate::utils::tabs::TabCfg; let fake_parent_grep_command = "git --doesnt-matter grep --nor-this nor_this -- nor_this"; let _args = FakeParentArgs::for_scope(fake_parent_grep_command); @@ -1264,6 +1281,7 @@ mod tests { is_emph: true, ..miss }; + let tab_cfg = TabCfg::new(0); let escape = "\x1B"; let working_example = format!("foo/bar/baz.yaml{escape}[36m:{escape}[m1090{escape}[36m:{escape}[m - {escape}[1;31mkind: Service{escape}[mAccount"); @@ -1271,7 +1289,12 @@ mod tests { let grep = parse_grep_line(&stripped).unwrap(); assert_eq!( - get_code_style_sections(&working_example, hit, miss, &grep.path, grep.line_number), + get_code_style_sections( + &working_example, + hit, + miss, + code_prefix_len(&stripped, &grep.code, &tab_cfg) + ), Some(StyleSectionSpecifier::StyleSections(vec![ (miss, " - "), (hit, "kind: Service"), @@ -1288,8 +1311,7 @@ mod tests { &broken_example, hit, miss, - &broken_grep.path, - broken_grep.line_number + code_prefix_len(&broken_stripped, &broken_grep.code, &tab_cfg) ), Some(StyleSectionSpecifier::StyleSections(vec![( hit, @@ -1306,8 +1328,7 @@ mod tests { &plus_example, hit, miss, - &plus_grep.path, - plus_grep.line_number + code_prefix_len(&plus_stripped, &plus_grep.code, &tab_cfg) ), Some(StyleSectionSpecifier::StyleSections(vec![ (miss, " + let (style, non_emph_style) = "), @@ -1316,4 +1337,32 @@ mod tests { ])) ); } + + #[test] + fn test_grep_line_number_not_matching_its_own_text() { + // The line number is parsed as 7, but its text in the grep line is + // "007", so the width of the prefix cannot be recovered from the + // parsed line number. + let fake_parent_grep_command = + "/usr/local/bin/git --doesnt-matter grep --nor-this nor_this -- nor_this"; + + DeltaTest::with_args(&[]) + .with_calling_process(fake_parent_grep_command) + .with_input("notes:007:hello world\n") + .expect_contains("hello world"); + } + + #[test] + fn test_grep_line_with_tab_in_parsed_path() { + // `git grep` output for a file named `gcc/ChangeLog-2004`: the path + // guessed by the heuristic parsers swallows the tab which indents the + // code, so the width of the prefix changes when tabs are expanded. + let fake_parent_grep_command = + "/usr/local/bin/git --doesnt-matter grep --nor-this nor_this -- nor_this"; + + DeltaTest::with_args(&[]) + .with_calling_process(fake_parent_grep_command) + .with_input("gcc/ChangeLog-2004-\tdoc/tm.texi, doc/tree-ssa.texi: Fix typos.\n") + .expect_contains("Fix typos."); + } }