Merge pull request #61 from oraios/fix/ignored-path-handling

Fix handling of ignored paths

CHANGELOG.md

@@ -3,11 +3,13 @@
 Changes prior to the next official version change will appear here.
 
 * Serena core:
-    * bugfix in find_symbol tool (a bug fixed in LS)
-    * merged the two overview tools (for dir and file) int a single one
-    * one-click setup for Cline enabled
-    * search for pattern tool can now (optionally) search in the entire project
-    * new tool for restarting the language server, in case of other sources of editing apart from Serena
+    * Bugfix in `FindSymbolTool` (a bug fixed in LS)
+    * Fix in `ListDirTool`: Do not ignore files with extensions not understood by the language server, only skip ignored directories
+      (error introduced in previous version)
+    * Merged the two overview tools (for directories and files) into a single one: `GetSymbolsOverviewTool`
+    * One-click setup for Cline enabled
+    * `SearchForPatternTool` can now (optionally) search in the entire project
+    * New tool `RestartLanguageServerTool` for restarting the language server (in case of other sources of editing apart from Serena)
     * Fix `CheckOnboardingPerformedTool`:
         * Tool description was incompatible with project change
         * Returned result was not as useful as it could be (now added list of memories)

src/multilspy/language_server.py

@@ -91,9 +91,9 @@ class LanguageServer:
     """
 
     # To be overridden and extended by subclasses
-    def should_always_ignore(self, dirname: str) -> bool:
+    def is_ignored_dirname(self, dirname: str) -> bool:
         """
-        A language-specific condition for directories that should be ignored always. For example, venv
+        A language-specific condition for directories that should always be ignored. For example, venv
         in Python and node_modules in JS/TS should be ignored always.
         """
         return dirname.startswith('.')
@@ -282,33 +282,38 @@ class LanguageServer:
         """
         return self._ignore_spec
 
-    def should_ignore_path(self, relative_path: str) -> bool:
+    def is_ignored_path(self, relative_path: str, ignore_unsupported_files: bool = True) -> bool:
         """
         Determine if a path should be ignored based on file type
         and ignore patterns.
 
         :param relative_path: Relative path to check
+        :param ignore_unsupported_files: whether files that are not supported source files should be ignored
 
         :return: True if the path should be ignored, False otherwise
         """
-        # Check file extension if it's a file
-        fn_matcher = self.language.get_source_fn_matcher()
         abs_path = os.path.join(self.repository_root_path, relative_path)
         if not os.path.exists(abs_path):
             raise FileNotFoundError(f"File {abs_path} not found, the ignore check cannot be performed")
 
-        if os.path.isfile(abs_path) and not fn_matcher.is_relevant_filename(abs_path):
-            return True
+        # Check file extension if it's a file
+        is_file = os.path.isfile(abs_path)
+        if is_file and ignore_unsupported_files:
+            fn_matcher = self.language.get_source_fn_matcher()
+            if not fn_matcher.is_relevant_filename(abs_path):
+                return True
 
         # Create normalized path for consistent handling
         rel_path = Path(relative_path)
 
         # Check each part of the path against always fulfilled ignore conditions
-        for part in rel_path.parts:
+        dir_parts = rel_path.parts
+        if is_file:
+            dir_parts = dir_parts[:-1]
+        for part in dir_parts:
             if not part:  # Skip empty parts (e.g., from leading '/')
                 continue
-            # Check standard ignores
-            if self.should_always_ignore(part):
+            if self.is_ignored_dirname(part):
                 return True
 
         # Use pathspec for gitignore-style pattern matching
@@ -619,7 +624,7 @@ class LanguageServer:
 
             abs_path = PathUtils.uri_to_path(item[LSPConstants.URI])
             rel_path = Path(abs_path).relative_to(self.repository_root_path)
-            if self.should_ignore_path(str(rel_path)):
+            if self.is_ignored_path(str(rel_path)):
                 self.logger.log(f"Ignoring reference in {rel_path} since it should be ignored", logging.DEBUG)
                 continue
 
@@ -893,7 +898,7 @@ class LanguageServer:
             if not os.path.exists(within_abs_path):
                 raise FileNotFoundError(f"File or directory not found: {within_abs_path}")
             if os.path.isfile(within_abs_path):
-                if self.should_ignore_path(within_relative_path):
+                if self.is_ignored_path(within_relative_path):
                     self.logger.log(f"You passed a file explicitly, but it is ignored. This is probably an error. File: {within_relative_path}", logging.ERROR)
                     return []
                 else:
@@ -905,7 +910,7 @@ class LanguageServer:
             abs_dir_path = self.repository_root_path if dir_path == "." else os.path.join(self.repository_root_path, dir_path)
             abs_dir_path = os.path.realpath(abs_dir_path)
 
-            if self.should_ignore_path(str(Path(abs_dir_path).relative_to(self.repository_root_path))):
+            if self.is_ignored_path(str(Path(abs_dir_path).relative_to(self.repository_root_path))):
                 self.logger.log(f"Skipping directory: {dir_path}\n(because it should be ignored)", logging.DEBUG)
                 return []
 
@@ -933,7 +938,7 @@ class LanguageServer:
                 item_path = os.path.join(abs_dir_path, item)
                 abs_item_path = os.path.join(self.repository_root_path, item_path)
                 rel_item_path = str(Path(abs_item_path).resolve().relative_to(self.repository_root_path))
-                if self.should_ignore_path(rel_item_path):
+                if self.is_ignored_path(rel_item_path):
                     self.logger.log(f"Skipping item: {rel_item_path}\n(because it should be ignored)", logging.DEBUG)
                     continue
 
@@ -2066,18 +2071,18 @@ class SyncLanguageServer:
         """
         self.language_server.load_cache()
 
-    def should_always_ignore(self, dirname: str) -> bool:
+    def is_ignored_dirname(self, dirname: str) -> bool:
         """
         A language-specific condition for directories that should be ignored always. For example, venv
         in Python and node_modules in JS/TS should be ignored always.
         """
-        return self.language_server.should_always_ignore(dirname)
+        return self.language_server.is_ignored_dirname(dirname)
 
-    def should_ignore_path(self, relative_path: str) -> bool:
+    def is_ignored_path(self, relative_path: str, ignore_unsupported_files: bool = True) -> bool:
         """
         Whether the given path should be ignored.
         """
-        return self.language_server.should_ignore_path(relative_path)
+        return self.language_server.is_ignored_path(relative_path, ignore_unsupported_files=ignore_unsupported_files)
 
     def get_ignore_spec(self) -> pathspec.PathSpec:
         """Returns the pathspec matcher for the paths that were configured to be ignored through

src/multilspy/language_servers/eclipse_jdtls/eclipse_jdtls.py

@@ -142,14 +142,14 @@ class EclipseJDTLS(LanguageServer):
         super().__init__(config, logger, repository_root_path, ProcessLaunchInfo(cmd, proc_env, proc_cwd), "java")
     
     @override
-    def should_always_ignore(self, dirname: str) -> bool:
+    def is_ignored_dirname(self, dirname: str) -> bool:
         # Ignore common Java build directories from different build tools:
         # - Maven: target
         # - Gradle: build, .gradle
         # - Eclipse: bin, .settings
         # - IntelliJ IDEA: out, .idea
         # - General: classes, dist, lib
-        return super().should_always_ignore(dirname) or dirname in [
+        return super().is_ignored_dirname(dirname) or dirname in [
             "target",      # Maven
             "build",       # Gradle
             "bin",         # Eclipse

src/multilspy/language_servers/gopls/gopls.py

@@ -22,12 +22,12 @@ class Gopls(LanguageServer):
     """
     
     @override
-    def should_always_ignore(self, dirname: str) -> bool:
+    def is_ignored_dirname(self, dirname: str) -> bool:
         # For Go projects, we should ignore:
         # - vendor: third-party dependencies vendored into the project
         # - node_modules: if the project has JavaScript components
         # - dist/build: common output directories
-        return super().should_always_ignore(dirname) or dirname in ["vendor", "node_modules", "dist", "build"]
+        return super().is_ignored_dirname(dirname) or dirname in ["vendor", "node_modules", "dist", "build"]
 
     @staticmethod
     def _get_go_version():

src/multilspy/language_servers/jedi_language_server/jedi_server.py

@@ -36,8 +36,8 @@ class JediServer(LanguageServer):
         )
         
     @override
-    def should_always_ignore(self, dirname: str) -> bool:
-        return super().should_always_ignore(dirname) or dirname in ["venv", "__pycache__"]
+    def is_ignored_dirname(self, dirname: str) -> bool:
+        return super().is_ignored_dirname(dirname) or dirname in ["venv", "__pycache__"]
 
     def _get_initialize_params(self, repository_absolute_path: str) -> InitializeParams:
         """

src/multilspy/language_servers/omnisharp/omnisharp.py

@@ -111,8 +111,8 @@ class OmniSharp(LanguageServer):
         self.references_available = asyncio.Event()
         
     @override
-    def should_always_ignore(self, dirname: str) -> bool:
-        return super().should_always_ignore(dirname) or dirname in ["bin", "obj"]
+    def is_ignored_dirname(self, dirname: str) -> bool:
+        return super().is_ignored_dirname(dirname) or dirname in ["bin", "obj"]
 
     def _get_initialize_params(self, repository_absolute_path: str) -> InitializeParams:
         """

src/multilspy/language_servers/pyright_language_server/pyright_server.py

@@ -39,8 +39,8 @@ class PyrightServer(LanguageServer):
         )
         
     @override
-    def should_always_ignore(self, dirname: str) -> bool:
-        return super().should_always_ignore(dirname) or dirname in ["venv", "__pycache__"]
+    def is_ignored_dirname(self, dirname: str) -> bool:
+        return super().is_ignored_dirname(dirname) or dirname in ["venv", "__pycache__"]
 
     def _get_initialize_params(self, repository_absolute_path: str) -> InitializeParams:
         """

src/multilspy/language_servers/rust_analyzer/rust_analyzer.py

@@ -42,8 +42,8 @@ class RustAnalyzer(LanguageServer):
         self.server_ready = asyncio.Event()
     
     @override
-    def should_always_ignore(self, dirname: str) -> bool:
-        return super().should_always_ignore(dirname) or dirname in ["target"]
+    def is_ignored_dirname(self, dirname: str) -> bool:
+        return super().is_ignored_dirname(dirname) or dirname in ["target"]
 
     def setup_runtime_dependencies(self, logger: MultilspyLogger, config: MultilspyConfig) -> str:
         """

src/multilspy/language_servers/solargraph/solargraph.py

@@ -44,8 +44,8 @@ class Solargraph(LanguageServer):
         self.server_ready = asyncio.Event()
         
     @override
-    def should_always_ignore(self, dirname: str) -> bool:
-        return super().should_always_ignore(dirname) or dirname in ["vendor"]
+    def is_ignored_dirname(self, dirname: str) -> bool:
+        return super().is_ignored_dirname(dirname) or dirname in ["vendor"]
 
     def setup_runtime_dependencies(self, logger: MultilspyLogger, config: MultilspyConfig, repository_root_path: str) -> str:
         """

src/multilspy/language_servers/typescript_language_server/typescript_language_server.py

@@ -57,8 +57,8 @@ class TypeScriptLanguageServer(LanguageServer):
         self.server_ready = asyncio.Event()
 
     @override
-    def should_always_ignore(self, dirname: str) -> bool:
-        return super().should_always_ignore(dirname) or dirname in [
+    def is_ignored_dirname(self, dirname: str) -> bool:
+        return super().is_ignored_dirname(dirname) or dirname in [
             "node_modules",
             "dist",
             "build",

src/serena/agent.py

@@ -620,16 +620,19 @@ class ListDirTool(Tool):
             required for the task.
         :return: a JSON object with the names of directories and files within the given directory
         """
+
+        def is_ignored_path(abs_path: str):
+            rel_path = os.path.relpath(abs_path, self.project_root)
+            return self.language_server.is_ignored_path(rel_path, ignore_unsupported_files=False)
+
         dirs, files = scan_directory(
             os.path.join(self.project_root, relative_path),
             relative_to=self.project_root,
             recursive=recursive,
+            is_ignored_dir=is_ignored_path,
+            is_ignored_file=is_ignored_path,
         )
 
-        # Don't use the scan_directory ignoring mechanism, instead rely on the language server,
-        # which has all information about ignored paths
-        dirs = [d for d in dirs if not self.language_server.should_ignore_path(d)]
-        files = [f for f in files if not self.language_server.should_ignore_path(f)]
         result = json.dumps({"dirs": dirs, "files": files})
         return self._limit_length(result, max_answer_chars)
 

src/serena/util/file_system.py

@@ -1,20 +1,20 @@
 import os
-from collections.abc import Sequence
+from collections.abc import Callable
 
 
 def scan_directory(
     path: str,
     recursive: bool = False,
     relative_to: str | None = None,
-    ignored_dirs: Sequence[str] = (),
-    ignored_files: Sequence[str] = (),
+    is_ignored_dir: Callable[[str], bool] = lambda x: False,
+    is_ignored_file: Callable[[str], bool] = lambda x: False,
 ) -> tuple[list[str], list[str]]:
     """
     :param path: the path to scan
     :param recursive: whether to recursively scan subdirectories
     :param relative_to: the path to which the results should be relative to; if None, provide absolute paths
-    :param ignored_dirs: a list of directory names or relative paths to ignore
-    :param ignored_files: a list of file names or relative paths to ignore
+    :param is_ignored_dir: a function with which to determine whether the given directory (abs. path) shall be ignored
+    :param is_ignored_file: a function with which to determine whether the given file (abs. path) shall be ignored
     :return: the list of directories and files
     """
     files = []
@@ -23,23 +23,6 @@ def scan_directory(
     abs_path = os.path.abspath(path)
     rel_base = os.path.abspath(relative_to) if relative_to else None
 
-    # Helper function to check if an item should be ignored
-    def is_ignored(entry_path: str, ignored_items: Sequence[str]) -> bool:
-        entry_name = os.path.basename(entry_path)
-
-        # Check if name is directly in ignored list
-        if entry_name in ignored_items:
-            return True
-
-        # Check if relative path matches any ignored path
-        if rel_base:
-            rel_path = os.path.relpath(entry_path, rel_base)
-            for item in ignored_items:
-                if rel_path == item or rel_path.startswith(f"{item}/"):
-                    return True
-
-        return False
-
     with os.scandir(abs_path) as entries:
         for entry in entries:
             entry_path = entry.path
@@ -50,17 +33,16 @@ def scan_directory(
                 result_path = entry_path
 
             if entry.is_file():
-                if not is_ignored(entry_path, ignored_files):
+                if not is_ignored_file(entry_path):
                     files.append(result_path)
             elif entry.is_dir():
-                if not is_ignored(entry_path, ignored_dirs):
+                if not is_ignored_dir(entry_path):
                     directories.append(result_path)
                     if recursive:
                         sub_dirs, sub_files = scan_directory(
-                            entry_path, recursive=True, relative_to=relative_to, ignored_dirs=ignored_dirs, ignored_files=ignored_files
+                            entry_path, recursive=True, relative_to=relative_to, is_ignored_dir=is_ignored_dir
                         )
                         files.extend(sub_files)
                         directories.extend(sub_dirs)
 
-    # Note: I've swapped the return order to match your function signature
     return directories, files