From 5d43ad0356de3da964450537efea540af33a82c8 Mon Sep 17 00:00:00 2001 From: Alex Carney Date: Sat, 30 Dec 2023 15:35:02 +0000 Subject: [PATCH] sphinx-agent: Add symbol support for MyST directives It turns out the information available to rst and myst directives is, annoyingly, subtly different. So rather than trying to parse the directive from the document's raw text (something not immediately accessible to MyST directives), we rely on the information available on the directive itself. This commit also updates our `disable_roles_and_directives` dispatcher to lookup the real implementation of the directive so that we can determine which options are valid. This isn't perfect by any means - symbol ranges are even less accurate than before - invalid options currently prevent a directive from being recognized as a symbol. But this is probably good enough for a beta :) --- .../esbonio/sphinx_agent/handlers/symbols.py | 109 ++++--- lib/esbonio/tests/e2e/conftest.py | 6 +- lib/esbonio/tests/e2e/test_e2e_symbols.py | 265 +++++++----------- 3 files changed, 176 insertions(+), 204 deletions(-) diff --git a/lib/esbonio/esbonio/sphinx_agent/handlers/symbols.py b/lib/esbonio/esbonio/sphinx_agent/handlers/symbols.py index 630c22798..a6c3c6b63 100644 --- a/lib/esbonio/esbonio/sphinx_agent/handlers/symbols.py +++ b/lib/esbonio/esbonio/sphinx_agent/handlers/symbols.py @@ -8,6 +8,7 @@ from docutils.io import NullOutput from docutils.io import StringInput from docutils.parsers.rst import Directive +from docutils.parsers.rst import directives from docutils.readers.standalone import Reader from docutils.utils import Reporter from sphinx.config import Config @@ -120,45 +121,36 @@ def dummy_role(name, rawtext, text, lineno, inliner, options={}, content=[]): return [node], [] -class DummyDirective(Directive): - has_content = True +def run_dummy_directive(self): + """This is a dummy implementation""" + node = a_directive() + node.line = self.lineno - def run(self): - node = a_directive() - node.line = self.lineno - parent = self.state.parent - lines = self.block_text + node.attributes["name"] = self.name + node.attributes["options"] = self.options - # substitution definitions require special handling - if isinstance(parent, nodes.substitution_definition): - lines = parent.rawsource - - text = lines.split("\n")[0] - match = types.DIRECTIVE.match(text) - if match: - node.attributes.update(match.groupdict()) - node.attributes["text"] = match.group(0) - else: - self.state.reporter.warning(f"Unable to parse directive: '{text}'") - node.attributes["text"] = text - - if self.content: - # This is essentially what `nested_parse_with_titles` does in Sphinx. - # But by passing the content_offset to state.nested_parse we ensure any line - # numbers remain relative to the start of the current file. - current_titles = self.state.memo.title_styles - current_sections = self.state.memo.section_level - self.state.memo.title_styles = [] - self.state.memo.section_level = 0 - try: - self.state.nested_parse( - self.content, self.content_offset, node, match_titles=1 - ) - finally: - self.state.memo.title_styles = current_titles - self.state.memo.section_level = current_sections - - return [node] + if len(self.arguments) > 0: + node.attributes["argument"] = self.arguments[0] + else: + node.attributes["argument"] = None + + if self.content: + # This is essentially what `nested_parse_with_titles` does in Sphinx. + # But by passing the content_offset to state.nested_parse we ensure any line + # numbers remain relative to the start of the current file. + current_titles = self.state.memo.title_styles + current_sections = self.state.memo.section_level + self.state.memo.title_styles = [] + self.state.memo.section_level = 0 + try: + self.state.nested_parse( + self.content, self.content_offset, node, match_titles=1 + ) + finally: + self.state.memo.title_styles = current_titles + self.state.memo.section_level = current_sections + + return [node] class disable_roles_and_directives(CustomReSTDispatcher): @@ -174,7 +166,41 @@ class disable_roles_and_directives(CustomReSTDispatcher): """ def directive(self, directive_name, language_module, document): - return DummyDirective, [] + # We still have access to the original dispatch mechanism + # This allows us to adapt our dummy directive to match the "shape" of the real + # directive implementation! + impl, _ = self.directive_func(directive_name, language_module, document) + if impl is None: + # Fallback to some sensible defaults. + has_content = True + option_spec = None + required_arguments = 0 + optional_arguments = 1 + final_argument_whitespace = True + else: + # Mimic the "shape" of the real directive + if impl.option_spec is None: + option_spec = None + else: + option_spec = {o: directives.unchanged for o in impl.option_spec} + + # It probably doesn't make sense to copy these values, as often the user's + # usage of a directive will be incorrect. + required_arguments = 0 # impl.required_arguments + has_content = True # impl.has_content + + optional_arguments = 1 + final_argument_whitespace = True + + attrs = { + "has_content": has_content, + "option_spec": option_spec, + "required_arguments": required_arguments, + "optional_arguments": optional_arguments, + "final_argument_whitespace": final_argument_whitespace, + "run": run_dummy_directive, + } + return type("DummyDirective", (Directive,), attrs), [] def role(self, role_name, language_module, lineno, reporter): return dummy_role, [] @@ -245,8 +271,7 @@ def depart_section(self, node: nodes.Node) -> None: def visit_a_directive(self, node: a_directive): argument = node.attributes.get("argument", None) - directive = node.attributes.get("directive", None) - text = node["text"] # type: ignore + directive = node.attributes.get("name", "<>") name = None detail = "" @@ -258,12 +283,12 @@ def visit_a_directive(self, node: a_directive): detail = directive if name is None: - name = text + name = directive line = (node.line or 1) - 1 range_ = types.Range( start=types.Position(line=line, character=0), - end=types.Position(line=line, character=len(text) - 1), + end=types.Position(line=line, character=len(name) - 1), ) self.push_symbol(name, ClassSymbol, range_, detail=detail) diff --git a/lib/esbonio/tests/e2e/conftest.py b/lib/esbonio/tests/e2e/conftest.py index 5d5bc637a..b6681ac0d 100644 --- a/lib/esbonio/tests/e2e/conftest.py +++ b/lib/esbonio/tests/e2e/conftest.py @@ -19,8 +19,8 @@ async def client(lsp_client: LanguageClient, uri_for, tmp_path_factory): """The "main" client to use for our tests.""" build_dir = tmp_path_factory.mktemp("build") - workspace_uri = uri_for("sphinx-default", "workspace") - test_uri = workspace_uri / "definitions.rst" + workspace_uri = uri_for("workspaces", "demo") + test_uri = workspace_uri / "index.rst" await lsp_client.initialize_session( types.InitializeParams( @@ -50,7 +50,7 @@ async def client(lsp_client: LanguageClient, uri_for, tmp_path_factory): }, }, workspace_folders=[ - types.WorkspaceFolder(uri=str(workspace_uri), name="sphinx-default"), + types.WorkspaceFolder(uri=str(workspace_uri), name="demo"), ], ) ) diff --git a/lib/esbonio/tests/e2e/test_e2e_symbols.py b/lib/esbonio/tests/e2e/test_e2e_symbols.py index 052b7dc1a..8a110f3e8 100644 --- a/lib/esbonio/tests/e2e/test_e2e_symbols.py +++ b/lib/esbonio/tests/e2e/test_e2e_symbols.py @@ -34,107 +34,61 @@ def document_symbol( [ (["conf.py"], None), ( - ["theorems", "pythagoras.rst"], + ["rst", "symbols.rst"], [ document_symbol( - name="Pythagoras' Theorem", - kind=types.SymbolKind.String, - range="3:0-3:18", + "Symbols", + types.SymbolKind.String, + "1:0-1:6", children=[ document_symbol( - name="../math.rst", - detail=".. include::", - kind=types.SymbolKind.Class, - range="8:0-8:23", + "What is a symbol?", + types.SymbolKind.Class, + "3:0-3:16", + detail="admonition", ), document_symbol( - name="/math.rst", - detail=".. include::", - kind=types.SymbolKind.Class, - range="10:0-10:21", + "Document Symbols", types.SymbolKind.String, "14:0-14:15" ), document_symbol( - name="Implementation", - kind=types.SymbolKind.String, - range="15:0-15:13", + "Workspace Symbols", types.SymbolKind.String, "23:0-23:16" + ), + ], + ) + ], + ), + ( + ["myst", "symbols.md"], + [ + document_symbol( + "Symbols", + types.SymbolKind.String, + "0:0-0:6", + children=[ + document_symbol( + "What is a symbol?", + types.SymbolKind.Class, + "2:0-2:16", + detail="admonition", + ), + document_symbol( + "Document Symbols", + types.SymbolKind.String, + "12:0-12:15", children=[ document_symbol( - name="pythagoras", - detail=".. module::", - kind=types.SymbolKind.Class, - range="20:0-20:21", - ), - document_symbol( - name="pythagoras", - detail=".. currentmodule::", - kind=types.SymbolKind.Class, - range="22:0-22:28", - ), - document_symbol( - name="PI", - detail=".. data::", - kind=types.SymbolKind.Class, - range="24:0-24:11", - ), - document_symbol( - name="UNKNOWN", - detail=".. data::", - kind=types.SymbolKind.Class, - range="28:0-28:16", - ), - document_symbol( - name="Triangle(a: float, b: float, c: float)", - detail=".. class::", - kind=types.SymbolKind.Class, - range="32:0-32:48", - children=[ - document_symbol( - name="a", - detail=".. attribute::", - kind=types.SymbolKind.Class, - range="36:0-36:15", - ), - document_symbol( - name="b", - detail=".. attribute::", - kind=types.SymbolKind.Class, - range="40:0-40:15", - ), - document_symbol( - name="c", - detail=".. attribute::", - kind=types.SymbolKind.Class, - range="44:0-44:15", - ), - document_symbol( - name="is_right_angled() -> bool", - detail=".. method::", - kind=types.SymbolKind.Class, - range="48:0-48:36", - children=[], - ), - ], - ), - document_symbol( - name="calc_hypotenuse(a: float, b: float) -> float", - detail=".. function::", - kind=types.SymbolKind.Class, - range="53:0-53:57", - ), - document_symbol( - name="calc_side(c: float, b: float) -> float", - detail="function::", - kind=types.SymbolKind.Class, - range="62:0-62:51", - ), - document_symbol( - name="right hand side", - detail=".. |rhs| replace::", - kind=types.SymbolKind.Class, - range="71:0-71:33", + "note", + types.SymbolKind.Class, + "19:0-19:3", + detail="note", ), ], ), + document_symbol( + "Workspace Symbols", + types.SymbolKind.String, + "26:0-26:16", + ), ], ) ], @@ -150,7 +104,7 @@ async def test_document_symbols( ): """Ensure that we handle ``textDocument/documentSymbols`` requests correctly.""" - test_uri = uri_for("sphinx-default", "workspace", *filepath) + test_uri = uri_for("workspaces", "demo", *filepath) actual = await client.text_document_document_symbol_async( types.DocumentSymbolParams( text_document=types.TextDocumentIdentifier(uri=str(test_uri)) @@ -182,118 +136,111 @@ async def test_document_symbols( # "container_name" # ) ( - "/definitions.rst", - "21:0-21:43", - "/theorems/pythagoras.rst .. literalinclude::", - types.SymbolKind.Class, - "Definition Tests", - ), - ( - "/theorems/pythagoras.rst", - "3:0-3:18", - "Pythagoras' Theorem", + "/rst/symbols.rst", + "1:0-1:6", + "Symbols", types.SymbolKind.String, "", ), ( - "/theorems/pythagoras.rst", - "20:0-20:21", - "pythagoras .. module::", - types.SymbolKind.Class, - "Implementation", - ), - ( - "/theorems/pythagoras.rst", - "22:0-22:28", - "pythagoras .. currentmodule::", + "/rst/symbols.rst", + "3:0-3:16", + "What is a symbol? admonition", types.SymbolKind.Class, - "Implementation", + "Symbols", ), ( - "/code/cpp.rst", - "5:0-5:33", - "bool isExample() .. cpp:function::", - types.SymbolKind.Class, - "ExampleClass", + "/myst/symbols.md", + "0:0-0:6", + "Symbols", + types.SymbolKind.String, + "", ), ( - "/theorems/pythagoras.rst", - "53:0-53:57", - "calc_hypotenuse(a: float, b: float) -> float .. function::", + "/myst/symbols.md", + "2:0-2:16", + "What is a symbol? admonition", types.SymbolKind.Class, - "Implementation", + "Symbols", ), ( - "/theorems/pythagoras.rst", - "62:0-62:51", - "calc_side(c: float, b: float) -> float .. function::", + "/myst/symbols.md", + "19:0-19:3", + "note", types.SymbolKind.Class, - "Implementation", + "Document Symbols", ), ] ), ), # We should be able to query by symbol name ( - "pythagoras", + "Symbols", set( [ ( - "/definitions.rst", - "21:0-21:43", - "/theorems/pythagoras.rst .. literalinclude::", - types.SymbolKind.Class, - "Definition Tests", + "/rst/symbols.rst", + "1:0-1:6", + "Symbols", + types.SymbolKind.String, + "", ), ( - "/theorems/pythagoras.rst", - "3:0-3:18", - "Pythagoras' Theorem", + "/rst/symbols.rst", + "14:0-14:15", + "Document Symbols", + types.SymbolKind.String, + "Symbols", + ), + ( + "/rst/symbols.rst", + "23:0-23:16", + "Workspace Symbols", + types.SymbolKind.String, + "Symbols", + ), + ( + "/myst/symbols.md", + "0:0-0:6", + "Symbols", types.SymbolKind.String, "", ), ( - "/theorems/pythagoras.rst", - "20:0-20:21", - "pythagoras .. module::", - types.SymbolKind.Class, - "Implementation", + "/myst/symbols.md", + "12:0-12:15", + "Document Symbols", + types.SymbolKind.String, + "Symbols", ), ( - "/theorems/pythagoras.rst", - "22:0-22:28", - "pythagoras .. currentmodule::", - types.SymbolKind.Class, - "Implementation", + "/myst/symbols.md", + "26:0-26:16", + "Workspace Symbols", + types.SymbolKind.String, + "Symbols", ), ] ), ), # We should also be able to query by (document) symbol `detail` e.g. a directive name ( - "function::", + "admonition", set( [ ( - "/code/cpp.rst", - "5:0-5:33", - "bool isExample() .. cpp:function::", - types.SymbolKind.Class, - "ExampleClass", - ), - ( - "/theorems/pythagoras.rst", - "53:0-53:57", - "calc_hypotenuse(a: float, b: float) -> float .. function::", + "/myst/symbols.md", + "2:0-2:16", + "What is a symbol? admonition", types.SymbolKind.Class, - "Implementation", + "Symbols", ), ( - "/theorems/pythagoras.rst", - "62:0-62:51", - "calc_side(c: float, b: float) -> float .. function::", + "/rst/symbols.rst", + "3:0-3:16", + "What is a symbol? admonition", types.SymbolKind.Class, - "Implementation", + "Symbols", ), ] ), @@ -311,7 +258,7 @@ async def test_workspace_symbols( ): """Ensure that we handle ``workspace/symbol`` requests correctly.""" - workspace_uri = str(uri_for("sphinx-default", "workspace")) + workspace_uri = str(uri_for("workspaces", "demo")) result = await client.workspace_symbol_async( types.WorkspaceSymbolParams(query=query) )