Proposal for optimization (requires refactoring)

[MischaPanch]

Currently all lines are taken into account in the from_lines method, and then are used for every check in a loop afterwards. This is inefficient - lines that are unused for matching (like comments and empty lines) should rather be filtered out once during initialization instead of looping over them in each check.

This could be done in a clean way by adding a method accepts_line to the Pattern (or rather a PatternFactory, see below) class, implement it in the implementations, and use it for filtering at construction of PathSpec.

Unfortunately, with the current functional-ish design this is not possible. from_lines accepts Callable[[AnyStr], Pattern] as pattern_factory instead of a PatternFactory class, and we can't assume anything about this callable (like an additional method for checking line appropriateness).

One could solve this by actually introducing a PatternFactory class and use that one instead of Callable for annotations, but it would break backwards compatibility. I think generally relying on Callables instead of factory classes has multiple downsides in the codebase - the existing and allowed pattern factories are not easily discoverable (as they would be through type hierarchy and name conventions). And, of course, there are the extendibility problems like the inability to add a filtering method I mentioned,

This is just a proposal, I understand if you want to keep the mix of the object-oriented and functional approach in your codebase.

[MischaPanch]

Here an implementation plan that would keep backwards compatibility (created with Serena, which analyzed the relevant parts of the codebase for it).

If you agree with it, you can probably reach good results if you plug it into any AI assistant of your choice (including Serena itself)

Refactoring Plan: Optimize PathSpec.from_lines Line Processing

Summary

This plan details the steps to refactor pathspec.PathSpec.from_lines to improve efficiency by filtering out irrelevant lines (like comments and blank lines) before attempting pattern compilation. This addresses the current inefficiency where the pattern_factory callable is invoked for every non-empty line, even if the line is ultimately ignored by the pattern implementation. The plan introduces a PatternFactory abstract base class to encapsulate line validation and compilation logic, while carefully maintaining backward compatibility with the existing Union[str, Callable] signature for the pattern_factory argument.

Goal

• Optimize PathSpec.from_lines by avoiding pattern compilation attempts and object creation for irrelevant input lines.
• Introduce a clearer abstraction (PatternFactory) for pattern creation logic.
• Maintain backward compatibility for existing users of PathSpec.from_lines.

Detailed Steps

1. Define PatternFactory Abstract Base Class (ABC)

   • Location: pathspec/pattern.py
   • Implementation:
     • Create class PatternFactory(abc.ABC):
     • Define @abc.abstractmethod def compile(self, line: AnyStr) -> Optional[Pattern]:
       • This method is responsible for taking a raw input line.
       • It should return a compiled Pattern instance if the line represents a valid pattern.
       • It must return None if the line is irrelevant (e.g., comment, blank) or invalid according to the factory's rules.
     • (Optional but Recommended) Define @abc.abstractmethod def accepts_line(self, line: AnyStr) -> bool:
       • This method provides an explicit check for line relevance before attempting compilation via compile.
       • It should return True if the line might be a valid pattern (e.g., not obviously a comment or blank), False otherwise. This allows from_lines to skip calling compile entirely for clearly irrelevant lines.

2. Implement GitWildMatchPatternFactory

   • Location: pathspec/patterns/gitwildmatch.py
   • Implementation:
     • Create class GitWildMatchPatternFactory(PatternFactory):
     • Implement accepts_line(self, line: AnyStr) -> bool:
       • Strip leading/trailing whitespace from line.
       • Return False if the stripped line is empty or starts with #.
       • Return True otherwise.
     • Implement compile(self, line: AnyStr) -> Optional[GitWildMatchPattern]:
       • Reuse or refactor the logic currently in GitWildMatchPattern.pattern_to_regex.
       • This method should handle stripping, comment checking (#), blank line checking, negation (!), and the actual pattern-to-regex conversion.
       • If the line results in a valid regex (i.e., pattern_to_regex would return a non-None regex), instantiate and return GitWildMatchPattern(regex, include).
       • If the line is a comment, blank, or results in an invalid pattern (e.g., pattern_to_regex returns (None, None) or raises an error), return None.

3. Update Pattern Registration Mechanism

   • Location: pathspec/util.py
   • Implementation:
     • Modify the internal storage for registered patterns (e.g., _registered_patterns) to store Type[PatternFactory] instead of the old Callable.
     • Update register_pattern(name: str, factory: Type[PatternFactory]): Adapt the function signature and implementation to accept and store PatternFactory classes. Ensure appropriate type checking.
     • Update lookup_pattern(name: str) -> Type[PatternFactory]: Adapt the function to retrieve and return the registered PatternFactory class associated with the name. Update return type hints.
     • Update the existing registration call in pathspec/patterns/gitwildmatch.py from util.register_pattern('gitwildmatch', GitWildMatchPattern) to util.register_pattern('gitwildmatch', GitWildMatchPatternFactory).

4. Refactor PathSpec.from_lines

   • Location: pathspec/pathspec.py
   • Implementation:
     • Factory Instantiation:
       • Check the type of the pattern_factory argument.
       • If isinstance(pattern_factory, str):
         • Call factory_cls = util.lookup_pattern(pattern_factory).
         • Instantiate factory_instance = factory_cls().
       • If callable(pattern_factory):
         • Define an internal adapter class:

           import abc
           from typing import AnyStr, Callable, Optional, Type
           # Assuming Pattern is defined elsewhere
           # from .pattern import Pattern 
           
           class PatternFactory(abc.ABC): # Placeholder if not already defined
               @abc.abstractmethod
               def accepts_line(self, line: AnyStr) -> bool: pass
               @abc.abstractmethod
               def compile(self, line: AnyStr) -> Optional['Pattern']: pass
           
           class _CallablePatternFactory(PatternFactory):
               def __init__(self, user_callable: Callable[[AnyStr], Optional['Pattern']]):
                   self._callable = user_callable
           
               def accepts_line(self, line: AnyStr) -> bool:
                   # Simplest approach: always try to compile if not obviously blank
                   return bool(line.strip()) 
           
               def compile(self, line: AnyStr) -> Optional['Pattern']:
                   # Filter truly empty lines first
                   if not line.strip():
                        return None
                   try:
                       # Assume callable returns None or raises Exception for invalid/ignored lines
                       pattern = self._callable(line)
                       return pattern
                   except Exception: # Broad exception, consider refining if specific errors are expected
                       return None # Treat errors during callable execution as invalid lines

         • Instantiate factory_instance = _CallablePatternFactory(pattern_factory).
       • Else: Raise TypeError.
     • Line Processing Loop: Replace the existing list comprehension with one using the factory_instance:

       # Preferred version using accepts_line:
       patterns = []
       for line in lines:
           # Basic non-empty check + factory pre-check
           if line and factory_instance.accepts_line(line):
               # Attempt compilation
               compiled_pattern = factory_instance.compile(line)
               if compiled_pattern is not None:
                   patterns.append(compiled_pattern)
       
       # Alternative version relying only on compile returning None:
       # patterns = []
       # for line in lines:
       #     if line: # Basic non-empty check
       #         compiled_pattern = factory_instance.compile(line)
       #         if compiled_pattern is not None:
       #             patterns.append(compiled_pattern)

     • Return cls(patterns).

5. Update GitIgnoreSpec.from_lines

   • Location: pathspec/gitignore.py
   • Implementation:
     • Review the existing GitIgnoreSpec.from_lines. It likely calls super().from_lines.
     • Ensure it continues to function correctly. If it explicitly passes 'gitwildmatch' or relies on a default, the refactoring in PathSpec.from_lines should handle it automatically via the updated registration and lookup. No changes might be needed here, but verification is crucial.

6. Testing

   • Execute: Run the full test suite using make test-all.
   • Verify: Ensure all existing tests pass. Pay close attention to tests involving comments, blank lines, or potentially edge-case patterns in input files/lists.
   • Add: Create new tests specifically verifying that PatternFactory.compile (or the underlying pattern_to_regex) is not called for lines that accepts_line identifies as irrelevant (e.g., using mocking or by checking for side effects if applicable). Test the _CallablePatternFactory adapter if possible.

Backward Compatibility Strategy

• The PathSpec.from_lines signature (cls, pattern_factory: Union[str, Callable[[AnyStr], Pattern]], lines: Iterable[AnyStr]) -> Self remains unchanged externally.
• String inputs ('gitwildmatch') are handled by looking up and using the new PatternFactory classes. This covers the common use case found in the codebase.
• Callable inputs are handled by wrapping them in the _CallablePatternFactory adapter, preserving the existing behavior as closely as possible.