Simplify search_text interface and handling of patterns

scripts/demo_run_tools.py

@@ -28,8 +28,6 @@ if __name__ == "__main__":
     search_pattern_tool = agent.get_tool(SearchForPatternTool)
 
     result = agent.execute_task(
-        lambda: search_pattern_tool.apply(
-            ".*Pyright.*|.*Omnisharp.*", restrict_search_to_code_files=False, relative_path="src/solidlsp/ls.py"
-        )
+        lambda: search_pattern_tool.apply("\n[^\n]*?Pyright", restrict_search_to_code_files=False, relative_path="src/solidlsp/ls.py")
     )
     pprint(json.loads(result))

src/serena/text_utils.py

@@ -1,5 +1,6 @@
 import fnmatch
 import logging
+import os
 import re
 from collections.abc import Callable
 from dataclasses import dataclass, field
@@ -111,8 +112,29 @@ class MatchedConsecutiveLines:
         return cls(lines=text_lines, source_file_path=source_file_path)
 
 
+def glob_to_regex(glob_pat: str) -> str:
+    regex_parts: list[str] = []
+    i = 0
+    while i < len(glob_pat):
+        ch = glob_pat[i]
+        if ch == "*":
+            regex_parts.append(".*")
+        elif ch == "?":
+            regex_parts.append(".")
+        elif ch == "\\":
+            i += 1
+            if i < len(glob_pat):
+                regex_parts.append(re.escape(glob_pat[i]))
+            else:
+                regex_parts.append("\\")
+        else:
+            regex_parts.append(re.escape(ch))
+        i += 1
+    return "".join(regex_parts)
+
+
 def search_text(
-    pattern: str | re.Pattern[str],
+    pattern: str,
     content: str | None = None,
     source_file_path: str | None = None,
     allow_multiline_match: bool = False,
@@ -123,20 +145,18 @@ def search_text(
     """
     Search for a pattern in text content. Supports both regex and glob-like patterns.
 
-    Args:
-        pattern: Pattern to search for (regex or glob-like pattern)
-        content: The text content to search. May be None if source_file_path is provided.
-        source_file_path: Optional path to the source file. If content is None,
-            this has to be passed and the file will be read.
-        allow_multiline_match: Whether to search across multiple lines. Currently, the default
-            option (False) is very inefficient, so it is recommended to set this to True.
-        context_lines_before: Number of context lines to include before matches
-        context_lines_after: Number of context lines to include after matches
-        is_glob: If True, pattern is treated as a glob-like pattern (e.g., "*.py", "test_??.py")
-                 and will be converted to regex internally
+    :param pattern: Pattern to search for (regex or glob-like pattern)
+    :param content: The text content to search. May be None if source_file_path is provided.
+    :param source_file_path: Optional path to the source file. If content is None,
+        this has to be passed and the file will be read.
+    :param allow_multiline_match: Whether to search across multiple lines. Currently, the default
+        option (False) is very inefficient, so it is recommended to set this to True.
+    :param context_lines_before: Number of context lines to include before matches
+    :param context_lines_after: Number of context lines to include after matches
+    :param is_glob: If True, pattern is treated as a glob-like pattern (e.g., "*.py", "test_??.py")
+             and will be converted to regex internally
 
-    Returns:
-        List of TextSearchMatch objects
+    :return: List of `TextSearchMatch` objects
 
     :raises: ValueError if the pattern is not valid
 
@@ -149,52 +169,15 @@ def search_text(
         raise ValueError("Pass either content or source_file_path")
 
     matches = []
-
-    # Convert pattern to a compiled regex if it's a string
-    if is_glob and isinstance(pattern, str):
-        # Convert glob pattern with optional backslash escaping to regex
-        def glob_to_regex(glob_pat: str) -> str:
-            regex_parts: list[str] = []
-            i = 0
-            while i < len(glob_pat):
-                ch = glob_pat[i]
-                if ch == "*":
-                    regex_parts.append(".*")
-                elif ch == "?":
-                    regex_parts.append(".")
-                elif ch == "\\":
-                    i += 1
-                    if i < len(glob_pat):
-                        regex_parts.append(re.escape(glob_pat[i]))
-                    else:
-                        regex_parts.append("\\")
-                else:
-                    regex_parts.append(re.escape(ch))
-                i += 1
-            return "".join(regex_parts)
-
-        escaped_pattern = glob_to_regex(pattern)
-        # For glob patterns, don't anchor with ^ and $ to allow partial line matches
-        compiled_pattern = re.compile(escaped_pattern)
-    elif isinstance(pattern, str):
-        try:
-            compiled_pattern = re.compile(pattern)
-        except re.error as e:
-            raise ValueError(f"Invalid regex pattern: {e}") from e
-    else:
-        # Pattern is already a compiled regex
-        compiled_pattern = pattern
-
-    # Split the content into lines for processing
     lines = content.splitlines()
     total_lines = len(lines)
 
+    # Convert pattern to a compiled regex if it's a string
+    if is_glob:
+        pattern = glob_to_regex(pattern)
     if allow_multiline_match:
         # For multiline matches, we need to use the DOTALL flag to make '.' match newlines
-        if isinstance(pattern, str):
-            # If we've compiled the pattern ourselves, we need to recompile with DOTALL
-            pattern_str = compiled_pattern.pattern
-            compiled_pattern = re.compile(pattern_str, re.DOTALL)
+        compiled_pattern = re.compile(pattern, re.DOTALL)
         # Search across the entire content as a single string
         for match in compiled_pattern.finditer(content):
             start_pos = match.start()
@@ -225,7 +208,8 @@ def search_text(
     else:
         # TODO: extremely inefficient! Since we currently don't use this option in SerenaAgent or LanguageServer,
         #   it is not urgent to fix, but should be either improved or the option should be removed.
-        # Search line by line
+        # Search line by line, normal compile without DOTALL
+        compiled_pattern = re.compile(pattern)
         for i, line in enumerate(lines):
             line_num = i + 1
             if compiled_pattern.search(line):
@@ -304,8 +288,9 @@ def glob_match(pattern: str, path: str) -> bool:
 
 
 def search_files(
-    file_paths: list[str],
-    pattern: re.Pattern | str,
+    relative_file_paths: list[str],
+    pattern: str,
+    root_path: str = "",
     file_reader: Callable[[str], str] = default_file_reader,
     context_lines_before: int = 0,
     context_lines_after: int = 0,
@@ -315,8 +300,9 @@ def search_files(
     """
     Search for a pattern in a list of files.
 
-    :param file_paths: List of files in which to search
+    :param relative_file_paths: List of relative file paths in which to search
     :param pattern: Pattern to search for
+    :param root_path: Root path to resolve relative paths against (by default, current working directory).
     :param file_reader: Function to read a file, by default will just use os.open.
         All files that can't be read by it will be skipped.
     :param context_lines_before: Number of context lines to include before matches
@@ -327,9 +313,8 @@ def search_files(
     """
     # Pre-filter paths (done sequentially to avoid overhead)
     # Use proper glob matching instead of gitignore patterns
-
     filtered_paths = []
-    for path in file_paths:
+    for path in relative_file_paths:
         if paths_include_glob and not glob_match(paths_include_glob, path):
             log.debug(f"Skipping {path}: does not match include pattern {paths_include_glob}")
             continue
@@ -343,7 +328,8 @@ def search_files(
     def process_single_file(path: str) -> dict[str, Any]:
         """Process a single file - this function will be parallelized."""
         try:
-            file_content = file_reader(path)
+            abs_path = os.path.join(root_path, path)
+            file_content = file_reader(abs_path)
             search_results = search_text(
                 pattern,
                 content=file_content,

src/solidlsp/ls.py

@@ -1242,9 +1242,6 @@ class SolidLanguageServer(ABC):
         :param paths_exclude_glob: Glob pattern to filter which files to exclude from the search. Takes precedence over paths_include_glob.
         :return: List of matched consecutive lines with context
         """
-        if isinstance(pattern, str):
-            pattern = re.compile(pattern)
-
         relative_file_paths = self.request_parsed_files(relative_path=relative_path)
         return search_files(
             relative_file_paths,

test/serena/test_text_utils.py

@@ -214,14 +214,6 @@ class TestSearchText:
 
         assert len(matches) == 0
 
-    def test_search_text_invalid_regex(self):
-        """Test searching with an invalid regex pattern raises ValueError."""
-        content = "def example(): pass"
-
-        # Search with an invalid regex pattern (unmatched parenthesis)
-        with pytest.raises(ValueError):
-            search_text("example(", content=content)
-
 
 # Mock file reader that always returns matching content
 def mock_reader_always_match(file_path: str) -> str:
@@ -262,7 +254,7 @@ class TestSearchFiles:
         Test the include/exclude glob filtering logic in search_files using PathSpec patterns.
         """
         results = search_files(
-            file_paths=file_paths,
+            relative_file_paths=file_paths,
             pattern=pattern,
             file_reader=mock_reader_always_match,
             paths_include_glob=paths_include_glob,
@@ -341,7 +333,7 @@ class TestSearchFiles:
         Test glob patterns that were problematic with the previous gitignore-based implementation.
         """
         results = search_files(
-            file_paths=file_paths,
+            relative_file_paths=file_paths,
             pattern=pattern,
             file_reader=mock_reader_always_match,
             paths_include_glob=paths_include_glob,
@@ -371,7 +363,7 @@ class TestSearchFiles:
         file_paths = ["a.py", "b.txt"]
         pattern = "non_existent_pattern_in_mock_content"  # This won't match mock_reader_always_match content
         results = search_files(
-            file_paths=file_paths,
+            relative_file_paths=file_paths,
             pattern=pattern,
             file_reader=mock_reader_always_match,  # Content is "This line contains a match."
             paths_include_glob=None,  # Both files would pass filters
@@ -393,10 +385,10 @@ class TestSearchFiles:
             return "No values here."
 
         file_paths = ["a.py", "b.py", "c.txt"]
-        pattern = re.compile(r"value=(\d+)")  # Regex pattern to find numbers after 'value='
+        pattern = r"value=(\d+)"
 
         results = search_files(
-            file_paths=file_paths,
+            relative_file_paths=file_paths,
             pattern=pattern,
             file_reader=specific_mock_reader,
             paths_include_glob="*.py",  # Only include .py files
@@ -426,7 +418,7 @@ class TestSearchFiles:
         pattern = "MATCH HERE"
 
         results = search_files(
-            file_paths=file_paths,
+            relative_file_paths=file_paths,
             pattern=pattern,
             file_reader=context_mock_reader,
             paths_include_glob="*.txt",  # Only include .txt files