Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement a fallback html_theme #916

Merged
merged 3 commits into from
Oct 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions lib/esbonio/changes/916.enhancement.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
When asking for a `html_theme` that is not available in the current environment, the server will now fallback to Sphinx's `alabaster` theme and report the error as a diagnostic
182 changes: 161 additions & 21 deletions lib/esbonio/esbonio/sphinx_agent/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import typing

from sphinx.application import Sphinx as _Sphinx
from sphinx.errors import ThemeError
from sphinx.util import console
from sphinx.util import logging as sphinx_logging_module
from sphinx.util.logging import NAMESPACE as SPHINX_LOG_NAMESPACE
Expand All @@ -17,6 +18,10 @@
if typing.TYPE_CHECKING:
from typing import IO
from typing import Any
from typing import Literal

from docutils.nodes import Element
from docutils.parsers.rst import Directive

RoleDefinition = tuple[str, Any, list[types.Role.TargetProvider]]

Expand All @@ -43,12 +48,41 @@ class Esbonio:
log: DiagnosticFilter

def __init__(self, dbpath: pathlib.Path, app: _Sphinx):
self.app: _Sphinx = app
self.db = Database(dbpath)
self.log = DiagnosticFilter(app)

self._roles: list[RoleDefinition] = []
"""Roles captured during Sphinx startup."""

self._config_ast: ast.Module | Literal[False] | None = None
"""The parsed AST of the user's conf.py file.
If ``False``, we already tried parsing the module and were unable to."""

self.diagnostics: dict[types.Uri, set[types.Diagnostic]] = {}
"""Recorded diagnostics."""

@property
def config_uri(self) -> types.Uri:
return types.Uri.for_file(pathlib.Path(self.app.confdir, "conf.py"))

@property
def config_ast(self) -> ast.Module | None:
"""Return the AST for the user's conf.py (if possible)"""

if self._config_ast is not None:
return self._config_ast or None # convert ``False`` to ``None``

try:
conf_py = pathlib.Path(self.app.confdir, "conf.py")
self._config_ast = ast.parse(source=conf_py.read_text())
return self._config_ast
except Exception as exc:
logger.debug("Unable to parse user's conf.py: %s", exc)
self._config_ast = False

return None

def add_role(
self,
name: str,
Expand Down Expand Up @@ -108,12 +142,22 @@ def __init__(self, *args, **kwargs):
# Override sphinx's usual logging setup function
sphinx_logging_module.setup = setup_logging # type: ignore

super().__init__(*args, **kwargs)
# `try_run_init` may call `__init__` more than once, this could lead to spamming
# the user with warning messages, so we will suppress these messages if the
# retry counter has been set.
self._esbonio_retry_count = 0
try_run_init(self, super().__init__, *args, **kwargs)

def add_role(self, name: str, role: Any, override: bool = False):
super().add_role(name, role, override)
super().add_role(name, role, override or self._esbonio_retry_count > 0)
self.esbonio.add_role(name, role)

def add_directive(self, name: str, cls: type[Directive], override: bool = False):
super().add_directive(name, cls, override or self._esbonio_retry_count > 0)

def add_node(self, node: type[Element], override: bool = False, **kwargs):
super().add_node(node, override or self._esbonio_retry_count > 0, **kwargs)

def setup_extension(self, extname: str):
"""Override Sphinx's implementation of `setup_extension`

Expand All @@ -135,22 +179,13 @@ def _report_missing_extension(self, extname: str, exc: Exception):
Parameters
----------
extname
The name of the extension that caused the excetion
The name of the extension that caused the exception

exc
The exception instance
"""

if not isinstance(cause := exc.__cause__, ImportError):
return

# Parse the user's config file
# TODO: Move this somewhere more central.
try:
conf_py = pathlib.Path(self.confdir, "conf.py")
config = ast.parse(source=conf_py.read_text())
except Exception:
logger.debug("Unable to parse user's conf.py")
if (config := self.esbonio.config_ast) is None:
return

# Now attempt to find the soure location of the extenison.
Expand All @@ -159,13 +194,112 @@ def _report_missing_extension(self, extname: str, exc: Exception):
return

diagnostic = types.Diagnostic(
range=range_, message=str(cause), severity=types.DiagnosticSeverity.Error
range=range_, message=f"{exc}", severity=types.DiagnosticSeverity.Error
)

# TODO: Move the set of diagnostics somewhere more central.
uri = types.Uri.for_file(conf_py)
uri = self.esbonio.config_uri
logger.debug("Adding diagnostic %s: %s", uri, diagnostic)
self.esbonio.log.diagnostics.setdefault(uri, set()).add(diagnostic)
self.esbonio.diagnostics.setdefault(uri, set()).add(diagnostic)


def try_run_init(app: Sphinx, init_fn, *args, **kwargs):
"""Try and run Sphinx's ``__init__`` function.

There are occasions where Sphinx will try and throw an error that is recoverable
e.g. a missing theme. In these situations we want to suppress the error, record a
diagnostic, and try again - which is what this function will do.

Some errors however, are not recoverable in which case we will allow the error to
proceed as normal.

Parameters
----------
app
The application instance we are trying to initialize

init_fn
The application's `__init__` method, as returned by ``super().__init__``

args
Positional arguments to ``__init__``

retries
Max number of retries, a fallback in case we end up creating infinite recursion

kwargs
Keyword arguments to ``__init__``
"""

if app._esbonio_retry_count >= 100:
raise RuntimeError("Unable to initialize Sphinx: max retries exceeded")

try:
init_fn(*args, **kwargs)
except ThemeError as exc:
# Fallback to the default theme.
kwargs.setdefault("confoverrides", {})["html_theme"] = "alabaster"
kwargs["confoverrides"]["html_theme_options"] = {}

app._esbonio_retry_count += 1
report_theme_error(app, exc)
try_run_init(app, init_fn, *args, **kwargs)
except Exception:
logger.exception("Unable to initialize Sphinx")
raise


def report_theme_error(app: Sphinx, exc: ThemeError):
"""Attempt to convert the given theme error into a useful diagnostic.

Parameters
----------
app
The Sphinx object being initialized

exc
The error instance
"""

if (config := app.esbonio.config_ast) is None:
return

if (range_ := find_html_theme_declaration(config)) is None:
return

diagnostic = types.Diagnostic(
range=range_,
message=f"{exc}",
severity=types.DiagnosticSeverity.Error,
)

uri = app.esbonio.config_uri
logger.debug("Adding diagnostic %s: %s", uri, diagnostic)
app.esbonio.diagnostics.setdefault(uri, set()).add(diagnostic)


def find_html_theme_declaration(mod: ast.Module) -> types.Range | None:
"""Attempt to find the location in the user's conf.py file where the ``html_theme``
was declared."""

for node in mod.body:
if not isinstance(node, ast.Assign):
continue

if len(targets := node.targets) != 1:
continue

if not isinstance(name := targets[0], ast.Name):
continue

if name.id == "html_theme":
break

else:
# Nothing found, abort
logger.debug("Unable to find 'html_theme' node")
return None

return ast_node_to_range(node)


def find_extension_declaration(mod: ast.Module, extname: str) -> types.Range | None:
Expand Down Expand Up @@ -210,15 +344,21 @@ def find_extension_declaration(mod: ast.Module, extname: str) -> types.Range | N
logger.debug("Unable to find node for extension %r", extname)
return None

return ast_node_to_range(element)


def ast_node_to_range(node: ast.stmt | ast.expr) -> types.Range:
"""Convert the given ast node to a range."""

# Finally, try and extract the source location.
start_line = element.lineno - 1
start_char = element.col_offset
start_line = node.lineno - 1
start_char = node.col_offset

if (end_line := (element.end_lineno or 0) - 1) < 0:
if (end_line := (node.end_lineno or 0) - 1) < 0:
end_line = start_line + 1
end_char: int | None = 0

elif (end_char := element.end_col_offset) is None:
elif (end_char := node.end_col_offset) is None:
end_line += 1
end_char = 0

Expand Down
4 changes: 2 additions & 2 deletions lib/esbonio/esbonio/sphinx_agent/handlers/diagnostics.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,14 @@ def init_db(app: Sphinx, config: Config):
def clear_diagnostics(app: Sphinx, docname: str, source):
"""Clear the diagnostics assocated with the given file."""
uri = Uri.for_file(app.env.doc2path(docname, base=True))
app.esbonio.log.diagnostics.pop(uri, None)
app.esbonio.diagnostics.pop(uri, None)


def sync_diagnostics(app: Sphinx, *args):
app.esbonio.db.clear_table(DIAGNOSTICS_TABLE)

results = []
diagnostics = app.esbonio.log.diagnostics
diagnostics = app.esbonio.diagnostics

for uri, items in diagnostics.items():
for item in items:
Expand Down
3 changes: 1 addition & 2 deletions lib/esbonio/esbonio/sphinx_agent/log.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ def __init__(self, app, *args, **kwargs):
super().__init__(*args, **kwargs)

self.app = app
self.diagnostics: dict[Uri, set[types.Diagnostic]] = {}

def filter(self, record: logging.LogRecord) -> bool:
conditions = [
Expand Down Expand Up @@ -76,7 +75,7 @@ def filter(self, record: logging.LogRecord) -> bool:
),
)

self.diagnostics.setdefault(uri, set()).add(diagnostic)
self.app.esbonio.diagnostics.setdefault(uri, set()).add(diagnostic)
return True


Expand Down
1 change: 0 additions & 1 deletion lib/esbonio/hatch.toml
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ sphinx = ["8"]

[envs.hatch-test.overrides]
matrix.sphinx.dependencies = [
"sphinx-design",
"myst-parser",
{ value = "sphinx>=6,<7", if = ["6"] },
{ value = "sphinx>=7,<8", if = ["7"] },
Expand Down
4 changes: 0 additions & 4 deletions lib/esbonio/tests/e2e/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,6 @@ async def client(lsp_client: LanguageClient, uri_for, tmp_path_factory):
workspace_uri.fs_path,
str(build_dir),
],
"configOverrides": {
"html_theme": "alabaster",
"html_theme_options": {},
},
"pythonCommand": [sys.executable],
},
},
Expand Down
13 changes: 10 additions & 3 deletions lib/esbonio/tests/e2e/test_e2e_diagnostics.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,19 @@ async def test_workspace_diagnostic(client: LanguageClient, uri_for):

workspace_uri = uri_for("workspaces", "demo")
expected = {
str(workspace_uri / "rst" / "diagnostics.rst"): {message},
str(workspace_uri / "myst" / "diagnostics.md"): {message},
str(workspace_uri / "conf.py"): (
"no theme named 'furo' found",
"Could not import extension sphinx_design (exception: "
"No module named 'sphinx_design')",
),
str(workspace_uri / "index.rst"): ('Unknown directive type "grid"'),
str(workspace_uri / "rst" / "diagnostics.rst"): (message,),
str(workspace_uri / "myst" / "diagnostics.md"): (message,),
}
assert len(report.items) == len(expected)
for item in report.items:
assert expected[item.uri] == {d.message for d in item.items}
for diagnostic in item.items:
assert diagnostic.message.startswith(expected[item.uri])

assert len(client.diagnostics) == 0, "Server should not publish diagnostics"

Expand Down
4 changes: 0 additions & 4 deletions lib/esbonio/tests/sphinx-agent/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,6 @@ async def client(request, uri_for, build_dir):
demo_workspace.fs_path,
str(build_dir),
],
config_overrides={
"html_theme": "alabaster",
"html_theme_options": {},
},
)
resolved = config.resolve(test_uri, workspace, logger)
assert resolved is not None
Expand Down
Loading