more efficient line iterator

signal-gateway-code-tool/src/cached.rs

@@ -10,16 +10,130 @@ use tracing::info;
 #[derive(Debug, Clone)]
 pub struct CachedFile {
     /// The file contents as a string (lossy UTF-8 conversion).
-    pub content: String,
+    content: Box<str>,
+    /// Indices of newline characters in content.
+    newline_indices: Vec<u32>,
     /// Whether this file looks like binary data.
-    pub is_binary: bool,
+    is_binary: bool,
 }
 
 impl CachedFile {
     /// Create a new CachedFile from content, detecting if it looks binary.
     fn new(content: String) -> Self {
+        assert!(
+            content.len() <= u32::MAX as usize,
+            "File content exceeds 4GB limit"
+        );
+
         let is_binary = looks_binary(&content);
-        Self { content, is_binary }
+        let newline_indices: Vec<u32> = content
+            .bytes()
+            .enumerate()
+            .filter(|(_, b)| *b == b'\n')
+            .map(|(i, _)| i as u32)
+            .collect();
+
+        Self {
+            content: content.into_boxed_str(),
+            newline_indices,
+            is_binary,
+        }
+    }
+
+    /// Get the file content as a string slice.
+    pub fn as_str(&self) -> &str {
+        &self.content
+    }
+
+    /// Check if file looks like binary data.
+    pub fn is_binary(&self) -> bool {
+        self.is_binary
+    }
+
+    /// Get the number of lines in the file.
+    pub fn line_count(&self) -> u32 {
+        if self.content.is_empty() {
+            0
+        } else {
+            // Number of lines = number of newlines + 1 (unless file ends with newline)
+            let count = self.newline_indices.len() as u32;
+            if self.content.ends_with('\n') {
+                count
+            } else {
+                count + 1
+            }
+        }
+    }
+
+    /// Iterate over a range of lines (1-indexed, inclusive).
+    ///
+    /// - `start`: Starting line number (1-indexed). Defaults to 1.
+    /// - `end`: Ending line number (inclusive). Defaults to last line.
+    pub fn line_range(&self, start: Option<u32>, end: Option<u32>) -> impl Iterator<Item = &str> {
+        let total_lines = self.line_count();
+        let start_line = start.unwrap_or(1).saturating_sub(1); // Convert to 0-indexed
+        let end_line = end.unwrap_or(total_lines).min(total_lines).saturating_sub(1); // Convert to 0-indexed
+
+        LineRangeIter {
+            content: &self.content,
+            newline_indices: &self.newline_indices,
+            current_line: start_line,
+            end_line,
+        }
+    }
+
+    /// Find which line (1-indexed) a character index falls on.
+    ///
+    /// Returns the number of newline characters before `idx`, plus 1.
+    pub fn idx_to_line(&self, idx: usize) -> u32 {
+        let idx = idx as u32;
+        // Binary search to find how many newlines are before idx
+        match self.newline_indices.binary_search(&idx) {
+            Ok(pos) => pos as u32 + 1,  // idx is exactly on a newline
+            Err(pos) => pos as u32 + 1, // pos is the number of newlines before idx
+        }
+    }
+
+}
+
+/// Iterator over a range of lines.
+struct LineRangeIter<'a> {
+    content: &'a str,
+    newline_indices: &'a [u32],
+    current_line: u32, // 0-indexed
+    end_line: u32,     // 0-indexed, inclusive
+}
+
+impl<'a> Iterator for LineRangeIter<'a> {
+    type Item = &'a str;
+
+    fn next(&mut self) -> Option<Self::Item> {
+        if self.current_line > self.end_line {
+            return None;
+        }
+
+        let line = self.current_line as usize;
+
+        // Get start offset: after previous line's newline, or 0 for first line
+        let start = if line == 0 {
+            0
+        } else {
+            self.newline_indices.get(line - 1).map(|&i| i as usize + 1)?
+        };
+
+        // Get end offset: at this line's newline, or end of content for last line
+        let end = self
+            .newline_indices
+            .get(line)
+            .map(|&i| i as usize)
+            .unwrap_or(self.content.len());
+
+        if start > self.content.len() {
+            return None;
+        }
+
+        self.current_line += 1;
+        Some(&self.content[start..end])
     }
 }
 
@@ -132,3 +246,102 @@ fn looks_binary(content: &str) -> bool {
         .count();
     non_printable > sample.len() / 10
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    fn make_file(content: &str) -> CachedFile {
+        CachedFile::new(content.to_string())
+    }
+
+    #[test]
+    fn test_line_count_empty() {
+        let file = make_file("");
+        assert_eq!(file.line_count(), 0);
+    }
+
+    #[test]
+    fn test_line_count_single_line_no_newline() {
+        let file = make_file("hello");
+        assert_eq!(file.line_count(), 1);
+    }
+
+    #[test]
+    fn test_line_count_single_line_with_newline() {
+        let file = make_file("hello\n");
+        assert_eq!(file.line_count(), 1);
+    }
+
+    #[test]
+    fn test_line_count_multiple_lines() {
+        let file = make_file("line1\nline2\nline3");
+        assert_eq!(file.line_count(), 3);
+    }
+
+    #[test]
+    fn test_line_count_multiple_lines_trailing_newline() {
+        let file = make_file("line1\nline2\nline3\n");
+        assert_eq!(file.line_count(), 3);
+    }
+
+    #[test]
+    fn test_line_range_all_lines() {
+        let file = make_file("line1\nline2\nline3");
+        let lines: Vec<_> = file.line_range(None, None).collect();
+        assert_eq!(lines, vec!["line1", "line2", "line3"]);
+    }
+
+    #[test]
+    fn test_line_range_subset() {
+        let file = make_file("line1\nline2\nline3\nline4\nline5");
+        let lines: Vec<_> = file.line_range(Some(2), Some(4)).collect();
+        assert_eq!(lines, vec!["line2", "line3", "line4"]);
+    }
+
+    #[test]
+    fn test_line_range_single_line() {
+        let file = make_file("line1\nline2\nline3");
+        let lines: Vec<_> = file.line_range(Some(2), Some(2)).collect();
+        assert_eq!(lines, vec!["line2"]);
+    }
+
+    #[test]
+    fn test_line_range_past_end() {
+        let file = make_file("line1\nline2");
+        let lines: Vec<_> = file.line_range(Some(1), Some(10)).collect();
+        assert_eq!(lines, vec!["line1", "line2"]);
+    }
+
+    #[test]
+    fn test_idx_to_line_first_line() {
+        let file = make_file("hello\nworld\ntest");
+        // "hello" is at indices 0-4, newline at 5
+        assert_eq!(file.idx_to_line(0), 1);
+        assert_eq!(file.idx_to_line(4), 1);
+        assert_eq!(file.idx_to_line(5), 1); // newline itself is on line 1
+    }
+
+    #[test]
+    fn test_idx_to_line_second_line() {
+        let file = make_file("hello\nworld\ntest");
+        // "world" is at indices 6-10, newline at 11
+        assert_eq!(file.idx_to_line(6), 2);
+        assert_eq!(file.idx_to_line(10), 2);
+        assert_eq!(file.idx_to_line(11), 2); // newline itself is on line 2
+    }
+
+    #[test]
+    fn test_idx_to_line_third_line() {
+        let file = make_file("hello\nworld\ntest");
+        // "test" is at indices 12-15
+        assert_eq!(file.idx_to_line(12), 3);
+        assert_eq!(file.idx_to_line(15), 3);
+    }
+
+    #[test]
+    fn test_as_str() {
+        let file = make_file("hello\nworld");
+        assert_eq!(file.as_str(), "hello\nworld");
+    }
+}

signal-gateway-code-tool/src/lib.rs

@@ -323,23 +323,20 @@ impl CodeTool {
             .get(path)
             .ok_or_else(|| format!("File not found: {}", path))?;
 
-        let lines: Vec<&str> = file.content.lines().collect();
+        let total_lines = file.line_count();
+        let start = line_start.unwrap_or(1) as u32;
+        let end = line_end.map(|e| e as u32);
 
-        // Handle line range (1-indexed)
+        if start > total_lines {
-        let start = line_start.unwrap_or(1).saturating_sub(1);
-        let end = line_end.unwrap_or(lines.len()).min(lines.len());
-
-        if start >= lines.len() {
             return Ok(format!(
                 "Line {} is past end of file ({} lines)",
-                start + 1,
+                start, total_lines
-                lines.len()
             ));
         }
 
         let mut output = String::new();
-        for (i, line) in lines[start..end].iter().enumerate() {
+        for (i, line) in file.line_range(Some(start), end).enumerate() {
-            writeln!(&mut output, "{:>6}\t{}", start + i + 1, line)
+            writeln!(&mut output, "{:>6}\t{}", start as usize + i, line)
                 .map_err(|e| format!("Format error: {e}"))?;
         }
 
@@ -378,15 +375,18 @@ impl CodeTool {
             }
 
             // Skip binary-looking files
-            if file.is_binary {
+            if file.is_binary() {
                 continue;
             }
 
-            let lines: Vec<&str> = file.content.lines().collect();
+            // Find all matches and map byte positions to line numbers
-            let mut file_matches = Vec::new();
+            let content = file.as_str();
+            let mut file_matches: Vec<u32> = Vec::new();
 
-            for (line_num, line) in lines.iter().enumerate() {
+            for m in regex.find_iter(content) {
-                if regex.is_match(line) {
+                let line_num = file.idx_to_line(m.start());
+                // Deduplicate: only add if this line isn't already recorded
+                if file_matches.last() != Some(&line_num) {
                     file_matches.push(line_num);
                     match_count += 1;
                     if match_count >= MAX_MATCHES {
@@ -397,30 +397,25 @@ impl CodeTool {
 
             if !file_matches.is_empty() {
                 file_count += 1;
+                let total_lines = file.line_count();
 
                 if context == 0 {
                     // No context, just print matches
                     for &line_num in &file_matches {
-                        writeln!(
+                        let line = file.line_range(Some(line_num), Some(line_num)).next().unwrap_or("");
-                            &mut output,
+                        writeln!(&mut output, "{}:{}: {}", path, line_num, line)
-                            "{}:{}: {}",
+                            .map_err(|e| format!("Format error: {e}"))?;
-                            path,
-                            line_num + 1,
-                            lines[line_num]
-                        )
-                        .map_err(|e| format!("Format error: {e}"))?;
                     }
                 } else {
                     // Print with context
                     writeln!(&mut output, "=== {} ===", path)
                         .map_err(|e| format!("Format error: {e}"))?;
 
-                    let context = context as usize;
                     let mut printed = std::collections::BTreeSet::new();
 
                     for &match_line in &file_matches {
-                        let start = match_line.saturating_sub(context);
+                        let start = match_line.saturating_sub(context).max(1);
-                        let end = (match_line + context + 1).min(lines.len());
+                        let end = (match_line + context).min(total_lines);
 
                         // Add separator if there's a gap
                         if let Some(&last) = printed.iter().next_back()
@@ -430,11 +425,11 @@ impl CodeTool {
                                 .map_err(|e| format!("Format error: {e}"))?;
                         }
 
-                        for (i, line) in lines[start..end].iter().enumerate() {
+                        for (i, line) in file.line_range(Some(start), Some(end)).enumerate() {
-                            let line_idx = start + i;
+                            let line_num = start + i as u32;
-                            if printed.insert(line_idx) {
+                            if printed.insert(line_num) {
-                                let marker = if line_idx == match_line { ">" } else { " " };
+                                let marker = if line_num == match_line { ">" } else { " " };
-                                writeln!(&mut output, "{}{:>5}\t{}", marker, line_idx + 1, line)
+                                writeln!(&mut output, "{}{:>5}\t{}", marker, line_num, line)
                                     .map_err(|e| format!("Format error: {e}"))?;
                             }
                         }