Skip to content

Perl: expose file_filter/ignore_dirs via ls_specific_settings (#1449)#1642

Open
weiconghe wants to merge 1 commit into
oraios:mainfrom
weiconghe:feat/perl-configurable-file-filter
Open

Perl: expose file_filter/ignore_dirs via ls_specific_settings (#1449)#1642
weiconghe wants to merge 1 commit into
oraios:mainfrom
weiconghe:feat/perl-configurable-file-filter

Conversation

@weiconghe

Copy link
Copy Markdown
Contributor

Fixes #1449

What & why

Perl::LanguageServer only indexes files whose extension is in its perl.fileFilter and skips directories in its perl.ignoreDirs. Both lists were hardcoded in PerlLanguageServer, so projects whose Perl source uses non-standard extensions — e.g. .cgi / .psgi web handlers, which can outnumber .pm/.pl in mature Perl backends (the reporter's example: 825 .cgi vs 340 .pm + 285 .pl) — had no way to make those files visible to find_symbol / find_referencing_symbols without forking Serena.

860c5841 added .t to the default fileFilter as a surface-level fix (one more extension). This PR addresses the root configurability gap that the maintainer acknowledged in #1449:

MischaPanch: "The proposed patch works, would you like to open a PR with it?"

The reporter's proposed approach is exactly what's implemented here. Opening it myself since the issue has been idle for ~2 months after that invitation, with no PR or follow-up from the reporter. All diagnosis credit for the configurability direction goes to @aaronbamblett.

Change

Surface the two lists through ls_specific_settings["perl"], mirroring the established pattern in groovy_language_server.py:

ls_specific_settings:
  perl:
    file_filter: [".pm", ".pl", ".t", ".cgi", ".psgi"]
    ignore_dirs: [".git", "blib", "local", "vendor", "cover_db"]
  • PerlLanguageServer.__init__ now resolves self._file_filter / self._ignore_dirs from solidlsp_settings.get_ls_specific_settings(Language.PERL), defaulting to the existing values, so behaviour is unchanged for projects that don't set the override.
  • Both LSP handshake sites (workspace/configuration handler and didChangeConfiguration notification) read the instance attributes instead of the old class constant / inline literals.
  • The old _FILE_FILTER class constant is replaced by module-level _DEFAULT_FILE_FILTER / _DEFAULT_IGNORE_DIRS, with the "keep in sync with Language.PERL.get_source_fn_matcher()" note preserved on the default.

This follows serena's CONTRIBUTING.md scope ("small bug fixes" / "extend along existing lines" via PR without prior discussion), and the trusted-project gating added recently to ls_specific_settings applies automatically.

Verification

Added test/solidlsp/perl/test_perl_config.py — 6 unit tests against the pure _resolve_filter_settings helper, deliberately not requiring a Perl runtime (existing Perl integration tests are skipped when Perl::LanguageServer isn't installed; these run everywhere):

Local checks on Windows / Python 3.13:

$ uv run --extra dev pytest test/solidlsp/perl/test_perl_config.py -v
6 passed
$ ruff check src/solidlsp/language_servers/perl_language_server.py test/solidlsp/perl/test_perl_config.py
All checks passed!
$ ruff format --check <same files>
2 files already formatted
$ ty check src/solidlsp        # _ty_core
All checks passed!
$ ty check test --exclude test/resources   # _ty_test
All checks passed!

CHANGELOG.md updated under "Language Servers".

@opcode81

opcode81 commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

The problem is that Serena's symbol index is exclusively determined by the FilenameMatcher returned by Language.PERL.get_source_fn_matcher(). If the matcher is out of sync with what is passed to the language server, this leaves you with inconsistent behaviour:

  • The language server will return symbols for a file with a different extension (e.g. a .cgi file) when explicitly asked to do so.
  • But find_symbol will not find symbols that don't use one of the extensions in the FilenameMatcher.

Therefore, the out-of-sync case is a problem, and changing the extensions in only one place won't give you the desired behaviour. We would need to modify the FilenameMatcher as well, and we could easily do so if we turn the instances into per-language singletons.

@weiconghe

Copy link
Copy Markdown
Contributor Author

Good catch — you're right that find_symbol relies on Language.PERL.get_source_fn_matcher(), so extending only the LS-side fileFilter would leave the matcher (and thus the symbol index) out of sync. Fixed in the updated push.

What I did:

  • Added FilenameMatcher.add_extensions(*exts) (idempotent, respects case_sensitive) — a small mutator intended for the now-@cached per-language singletons.
  • In PerlLanguageServer, after resolving file_filter from ls_specific_settings["perl"], I call Language.PERL.get_source_fn_matcher().add_extensions(*file_filter), so the configured extensions reach every consumer of the matcher: project.is_ignored_path (symbol index traversal), ls.is_ignored_path, and the language-composition detection.

The default file_filter ([".pm", ".pl", ".t"]) matches the existing Perl matcher, so behaviour is unchanged for projects that don't set the override; only added extensions (e.g. .cgi) propagate. The sync is a separate _sync_source_fn_matcher step so _resolve_filter_settings stays a pure, unit-testable function.

Tests added (test/solidlsp/perl/test_perl_config.py): the resolver cases, FilenameMatcher.add_extensions (new extension, idempotency across repeated calls, case-insensitive normalisation), and the matcher sync (custom file_filter extends the Perl matcher; default file_filter leaves it unchanged — both restore the singleton afterwards). 11 tests pass; ruff/ty clean.

@opcode81

opcode81 commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

There are further subtleties to consider: If a new project is activated following the activation of a project that reconfigured the file extensions, then the original extensions must apply; the reconfiguration must be undone.
Therefore,

  • the matcher needs a reset method (and it needs to store the original configuration for this work)
  • the reset method must be called upon language server initialization. The natural place for this would be in SolidLanguageServer.__init__ after self.language = self.get_language_enum_instance().

@weiconghe weiconghe force-pushed the feat/perl-configurable-file-filter branch from ce67e46 to de4b78e Compare July 3, 2026 16:04
@weiconghe

Copy link
Copy Markdown
Contributor Author

Good catch on the cross-project leak — that's a real defect in my add-only approach. Once project A had added .cgi, the singleton would have kept it for every subsequently activated project. Thanks for spotting it.

Implemented exactly as you suggested:

  • FilenameMatcher now snapshots its initial extensions at construction and exposes reset(), which restores that snapshot.
  • SolidLanguageServer.__init__ calls self.language.get_source_fn_matcher().reset() right after self.language is determined, so each activation starts from the language's defaults before the subclass re-applies its own file_filter (Perl's _sync_source_fn_matcher runs after super().__init__(), so the ordering is reset → re-apply).
  • Added test_reset_prevents_cross_project_leak in test_perl_config.py, which reproduces the scenario directly: project A syncs .cgi, project B activates (reset + default config), and asserts .cgi is gone while .pm survives.

15 tests pass; ruff / ty clean. Pushed as de4b78ed.

Comment thread src/solidlsp/ls.py Outdated
Comment thread CHANGELOG.md
@weiconghe weiconghe force-pushed the feat/perl-configurable-file-filter branch from de4b78e to 30bb6b8 Compare July 3, 2026 16:12
@weiconghe

Copy link
Copy Markdown
Contributor Author

Thanks — moved the reset below the self.language attribute docstring so it stays attached to the attribute it documents. Pushed as 30bb6b87.

…#1449)

Perl::LanguageServer only indexes files whose extension is in its
`perl.fileFilter` and skips directories in `perl.ignoreDirs`. Both were
hardcoded in PerlLanguageServer, so projects whose Perl source uses
non-standard extensions (e.g. .cgi / .psgi web handlers, which can
dominate a mature codebase) had no way to make those files visible to
symbol/reference queries without forking Serena (oraios#1449).

Surface both lists through `ls_specific_settings["perl"]` (keys
`file_filter` and `ignore_dirs`), defaulting to the existing values so
behaviour is unchanged for current projects. The resolution lives in a
pure static method `_resolve_filter_settings` so the config plumbing is
covered by unit tests that don't require a Perl runtime.

Keep Serena's symbol index in sync with the LS (oraios#1449, per review):
`find_symbol` and symbol indexing are driven by
`Language.PERL.get_source_fn_matcher()`, so adding an extension (e.g.
.cgi) to `file_filter` alone would leave the matcher untouched and the
extension's symbols invisible. Since that matcher is now a @cache'd
per-language singleton, add a `FilenameMatcher.add_extensions()` mutator
and call it from `PerlLanguageServer` so the configured extensions also
reach the symbol index, the ignore checks, and language composition.

Avoid cross-project leakage (per review): because the matcher is a
process-wide singleton, extensions added for one project would otherwise
persist into the next. `FilenameMatcher` now snapshots its initial
configuration and exposes `reset()`, and `SolidLanguageServer.__init__`
resets the matcher right after `self.language` is determined, so every
activation starts from the language's defaults before the subclass
re-applies its own `file_filter`.

This complements 860c584 (which added .t to the default list as a
surface-level fix) by addressing the root configurability gap that the
maintainer acknowledged in oraios#1449.
@weiconghe weiconghe force-pushed the feat/perl-configurable-file-filter branch from 30bb6b8 to 967472e Compare July 3, 2026 16:36
@weiconghe

Copy link
Copy Markdown
Contributor Author

There was no Perl section yet, so I added #### Perl in lexicographical position (between Pascal and PHP). It documents both file_filter and ignore_dirs under ls_specific_settings.perl — defaults, an example serena_config.yml snippet, and notes that configured extensions are also synced into Serena's source-file matcher (and reset on each activation so they don't leak across projects). Pushed as 967472e4.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Perl: file extensions hardcoded to .pm/.pl — exclude legitimate Perl source like .cgi

2 participants