diff --git a/src/solidlsp/language_servers/marksman.py b/src/solidlsp/language_servers/marksman.py index 14805f21..c5d039e3 100644 --- a/src/solidlsp/language_servers/marksman.py +++ b/src/solidlsp/language_servers/marksman.py @@ -6,11 +6,19 @@ Contains various configurations and settings specific to Markdown. import logging import os import pathlib +from collections.abc import Hashable from overrides import override -from solidlsp.ls import LanguageServerDependencyProvider, LanguageServerDependencyProviderSinglePath, SolidLanguageServer +from solidlsp.ls import ( + DocumentSymbols, + LanguageServerDependencyProvider, + LanguageServerDependencyProviderSinglePath, + LSPFileBuffer, + SolidLanguageServer, +) from solidlsp.ls_config import LanguageServerConfig +from solidlsp.ls_types import SymbolKind, UnifiedSymbolInformation from solidlsp.lsp_protocol_handler.lsp_types import InitializeParams from solidlsp.settings import SolidLSPSettings @@ -106,6 +114,35 @@ class Marksman(SolidLanguageServer): def is_ignored_dirname(self, dirname: str) -> bool: return super().is_ignored_dirname(dirname) or dirname in ["node_modules", ".obsidian", ".vitepress", ".vuepress"] + def _document_symbols_cache_fingerprint(self) -> Hashable | None: + request_document_symbols_override_version = 1 + return request_document_symbols_override_version + + @override + def request_document_symbols(self, relative_file_path: str, file_buffer: LSPFileBuffer | None = None) -> DocumentSymbols: + """Override to remap Marksman's heading symbol kinds from String to Namespace. + + Marksman LSP returns all markdown headings (h1-h6) with SymbolKind.String (15). + This is problematic because String (15) >= Variable (13), so headings are + classified as "low-level" and filtered out of symbol overviews. + Remapping to Namespace (3) fixes this and is semantically appropriate + (headings are named sections containing other content). + """ + document_symbols = super().request_document_symbols(relative_file_path, file_buffer=file_buffer) + + # NOTE: When changing this method, also update the cache fingerprint method above + + def remap_heading_kinds(symbol: UnifiedSymbolInformation) -> None: + if symbol["kind"] == SymbolKind.String: + symbol["kind"] = SymbolKind.Namespace + for child in symbol.get("children", []): + remap_heading_kinds(child) + + for sym in document_symbols.root_symbols: + remap_heading_kinds(sym) + + return document_symbols + @staticmethod def _get_initialize_params(repository_absolute_path: str) -> InitializeParams: """ diff --git a/test/solidlsp/markdown/test_markdown_basic.py b/test/solidlsp/markdown/test_markdown_basic.py index 81a988c0..e8f96c5a 100644 --- a/test/solidlsp/markdown/test_markdown_basic.py +++ b/test/solidlsp/markdown/test_markdown_basic.py @@ -7,8 +7,10 @@ like request_document_symbols using the markdown test repository. import pytest +from serena.symbol import LanguageServerSymbol from solidlsp import SolidLanguageServer from solidlsp.ls_config import Language +from solidlsp.ls_types import SymbolKind @pytest.mark.markdown @@ -24,17 +26,19 @@ class TestMarkdownLanguageServerBasics: @pytest.mark.parametrize("language_server", [Language.MARKDOWN], indirect=True) def test_markdown_request_document_symbols(self, language_server: SolidLanguageServer) -> None: """Test request_document_symbols for markdown files.""" - # Test getting symbols from README.md all_symbols, _root_symbols = language_server.request_document_symbols("README.md").get_all_symbols_and_roots() - # Extract heading symbols (LSP Symbol Kind 15 is String, but marksman uses kind 15 for headings) - # Note: Different markdown LSPs may use different symbol kinds for headings - # Marksman typically uses kind 15 (String) for markdown headings heading_names = [symbol["name"] for symbol in all_symbols] # Should detect headings from README.md assert "Test Repository" in heading_names or len(all_symbols) > 0, "Should find at least one heading" + # Verify that markdown headings are remapped from String to Namespace + for symbol in all_symbols: + assert ( + symbol["kind"] == SymbolKind.Namespace + ), f"Heading '{symbol['name']}' should have kind Namespace, got {SymbolKind(symbol['kind']).name}" + @pytest.mark.parametrize("language_server", [Language.MARKDOWN], indirect=True) def test_markdown_request_symbols_from_guide(self, language_server: SolidLanguageServer) -> None: """Test symbol detection in guide.md file.""" @@ -54,7 +58,6 @@ class TestMarkdownLanguageServerBasics: @pytest.mark.parametrize("language_server", [Language.MARKDOWN], indirect=True) def test_markdown_request_document_symbols_with_body(self, language_server: SolidLanguageServer) -> None: """Test request_document_symbols with body extraction.""" - # Test with include_body=True all_symbols, _root_symbols = language_server.request_document_symbols("README.md").get_all_symbols_and_roots() # Should have found some symbols @@ -63,3 +66,30 @@ class TestMarkdownLanguageServerBasics: # Note: Not all markdown LSPs provide body information for symbols # This test is more lenient and just verifies the API works assert all_symbols is not None, "Should return symbols even if body extraction is limited" + + @pytest.mark.parametrize("language_server", [Language.MARKDOWN], indirect=True) + def test_markdown_headings_not_low_level(self, language_server: SolidLanguageServer) -> None: + """Test that markdown headings are not classified as low-level symbols. + + Verifies the fix for the issue where Marksman's SymbolKind.String (15) + caused all headings to be filtered out of get_symbols_overview. + """ + all_symbols, _root_symbols = language_server.request_document_symbols("README.md").get_all_symbols_and_roots() + assert len(all_symbols) > 0, "Should find headings in README.md" + + for symbol in all_symbols: + ls_symbol = LanguageServerSymbol(symbol) + assert ( + not ls_symbol.is_low_level() + ), f"Heading '{symbol['name']}' should not be low-level (kind={SymbolKind(symbol['kind']).name})" + + @pytest.mark.parametrize("language_server", [Language.MARKDOWN], indirect=True) + def test_markdown_nested_headings_remapped(self, language_server: SolidLanguageServer) -> None: + """Test that nested headings (h1-h5) are all remapped from String to Namespace.""" + all_symbols, _root_symbols = language_server.request_document_symbols("api.md").get_all_symbols_and_roots() + + # api.md has deeply nested headings (h1 through h5) + assert len(all_symbols) > 5, "api.md should have many headings" + + for symbol in all_symbols: + assert symbol["kind"] == SymbolKind.Namespace, f"Nested heading '{symbol['name']}' should be remapped to Namespace"