lsp: Implement a ${defaultBuildDir} config variable

Unless esbonio finds a `sphinx-build` command to use from the
user's config it will attempt to guess something reasonable. During
this process it generates a default build directory to use, in a
subfolder of `platformdirs.user_cache_dir()` so that it does not
interfere with the user's files.

Up until now, the moment a user sets their own `sphinx-build` command
this behavior is lost, which can lead to issues on some systems
(see #865).

This commit introduces a `${defaultBuildDir}` placeholder value that
the user can use to provide their own build flags, while maintaining
the default choice of build dir provided by esbonio.

lib/esbonio/esbonio/server/features/sphinx_manager/client_subprocess.py

@@ -11,6 +11,7 @@
 import typing
 from uuid import uuid4
 
+import platformdirs
 from pygls.client import JsonRPCClient
 from pygls.protocol import JsonRPCProtocol
 
@@ -212,6 +213,9 @@ async def start(self) -> SphinxClient:
             params = types.CreateApplicationParams(
                 command=self.config.build_command,
                 config_overrides=self.config.config_overrides,
+                context={
+                    "cacheDir": platformdirs.user_cache_dir("esbonio", "swyddfa"),
+                },
             )
             self.sphinx_info = await self.protocol.send_request_async(
                 "sphinx/createApp", params

lib/esbonio/esbonio/server/features/sphinx_manager/config.py

@@ -1,14 +1,12 @@
 from __future__ import annotations
 
-import hashlib
 import importlib.util
 import logging
 import pathlib
 from typing import Any
 from typing import Optional
 
 import attrs
-import platformdirs
 from pygls.workspace import Workspace
 
 from esbonio.server import Uri
@@ -227,9 +225,12 @@ def _resolve_build_command(self, uri: Uri, logger: logging.Logger) -> list[str]:
             conf_py = current / "conf.py"
             logger.debug("Trying path: %s", current)
             if conf_py.exists():
-                cache = platformdirs.user_cache_dir("esbonio", "swyddfa")
-                project = hashlib.md5(str(current).encode()).hexdigest()  # noqa: S324
-                build_dir = str(pathlib.Path(cache, project))
-                return ["sphinx-build", "-M", "dirhtml", str(current), str(build_dir)]
+                return [
+                    "sphinx-build",
+                    "-M",
+                    "dirhtml",
+                    str(current),
+                    "${defaultBuildDir}",
+                ]
 
         return []

lib/esbonio/esbonio/sphinx_agent/config.py

@@ -1,8 +1,10 @@
 from __future__ import annotations
 
 import dataclasses
+import hashlib
 import inspect
 import pathlib
+import re
 import sys
 from typing import Any
 from typing import Literal
@@ -13,6 +15,8 @@
 from sphinx.application import Sphinx
 from sphinx.cmd.build import main as sphinx_build
 
+VARIABLE = re.compile(r"\$\{(\w+)\}")
+
 
 @dataclasses.dataclass
 class SphinxConfig:
@@ -128,7 +132,7 @@ def fromcli(cls, args: list[str]):
             warning_is_error=sphinx_args.get("warningiserror", False),
         )
 
-    def to_application_args(self) -> dict[str, Any]:
+    def to_application_args(self, context: dict[str, Any]) -> dict[str, Any]:
         """Convert this into the equivalent Sphinx application arguments."""
 
         # On OSes like Fedora Silverblue, `/home` is symlinked to `/var/home`.  This
@@ -139,6 +143,25 @@ def to_application_args(self) -> dict[str, Any]:
         # Resolving these paths here, should ensure that the agent always
         # reports the true location of any given directory.
         conf_dir = pathlib.Path(self.conf_dir).resolve()
+        self.conf_dir = str(conf_dir)
+
+        # Resolve any config variables.
+        #
+        # This is a bit hacky, but let's go with it for now. The only config variable
+        # we currently support is 'defaultBuildDir' which is derived from the value
+        # of `conf_dir`. So we resolve the full path of `conf_dir` above, then resolve
+        # the configuration variables here, before finally calling resolve() on the
+        # remaining paths below.
+        for name, value in dataclasses.asdict(self).items():
+            if not isinstance(value, str):
+                continue
+
+            if (match := VARIABLE.match(value)) is None:
+                continue
+
+            replacement = self.resolve_config_variable(match.group(1), context)
+            setattr(self, name, VARIABLE.sub(replacement, value))
+
         build_dir = pathlib.Path(self.build_dir).resolve()
         doctree_dir = pathlib.Path(self.doctree_dir).resolve()
         src_dir = pathlib.Path(self.src_dir).resolve()
@@ -159,3 +182,18 @@ def to_application_args(self) -> dict[str, Any]:
             "warning": sys.stderr,
             "warningiserror": self.warning_is_error,
         }
+
+    def resolve_config_variable(self, name: str, context: dict[str, str]):
+        """Resolve the value for the given configuration variable."""
+
+        if name.lower() == "defaultbuilddir":
+            if (cache_dir := context.get("cacheDir")) is None:
+                raise RuntimeError(
+                    f"Unable to resolve config variable {name!r}, "
+                    "missing context value: 'cacheDir'"
+                )
+
+            project = hashlib.md5(self.conf_dir.encode()).hexdigest()  # noqa: S324
+            return str(pathlib.Path(cache_dir, project))
+
+        raise ValueError(f"Unknown configuration variable {name!r}")

lib/esbonio/esbonio/sphinx_agent/handlers/__init__.py

@@ -106,12 +106,14 @@ class definition from the ``types`` module.
 
     def create_sphinx_app(self, request: types.CreateApplicationRequest):
         """Create a new sphinx application instance."""
-        sphinx_config = SphinxConfig.fromcli(request.params.command)
+        params = request.params
+
+        sphinx_config = SphinxConfig.fromcli(params.command)
         if sphinx_config is None:
             raise ValueError("Invalid build command")
 
-        sphinx_config.config_overrides.update(request.params.config_overrides)
-        sphinx_args = sphinx_config.to_application_args()
+        sphinx_config.config_overrides.update(params.config_overrides)
+        sphinx_args = sphinx_config.to_application_args(params.context)
         self.app = Sphinx(**sphinx_args)
 
         # Connect event handlers.

lib/esbonio/esbonio/sphinx_agent/types/__init__.py

@@ -144,6 +144,10 @@
 ]
 
 
+# -- RPC Types
+#
+# These represent the structure of the messages sent between the Sphinx agent and the
+# parent language server.
 @dataclasses.dataclass
 class CreateApplicationParams:
     """Parameters of a ``sphinx/createApp`` request."""
@@ -154,6 +158,9 @@ class CreateApplicationParams:
     config_overrides: dict[str, Any]
     """Overrides to apply to the application's configuration."""
 
+    context: dict[str, str] = dataclasses.field(default_factory=dict)
+    """The context in which to resolve config variables."""
+
 
 @dataclasses.dataclass
 class CreateApplicationRequest:

lib/esbonio/tests/sphinx-agent/test_sa_unit.py

@@ -3,6 +3,7 @@
 import logging
 import os
 import pathlib
+import re
 import sys
 import typing
 from unittest import mock
@@ -431,7 +432,7 @@ def test_cli_arg_handling(args: list[str], expected: dict[str, Any]):
     config = SphinxConfig.fromcli(args)
     assert config is not None
 
-    actual = config.to_application_args()
+    actual = config.to_application_args({})
 
     # pytest overrides stderr on windows, so if we were to put `sys.stderr` in the
     # `expected` dict this test would fail as `sys.stderr` inside a test function has a
@@ -444,6 +445,64 @@ def test_cli_arg_handling(args: list[str], expected: dict[str, Any]):
     assert expected == actual
 
 
+@pytest.mark.parametrize(
+    "args, expected, context",
+    [
+        (
+            ["-M", "html", "src", "${defaultBuildDir}"],
+            application_args(
+                srcdir="src",
+                confdir="src",
+                outdir=os.sep + os.path.join("path", "to", "cache", r"\w+", "html"),
+                doctreedir=(
+                    os.sep + os.path.join("path", "to", "cache", r"\w+", "doctrees")
+                ),
+                buildername="html",
+            ),
+            {
+                "cacheDir": os.sep + os.sep.join(["path", "to", "cache"]),
+            },
+        ),
+        (
+            ["-b", "html", "src", "${defaultBuildDir}"],
+            application_args(
+                srcdir="src",
+                confdir="src",
+                outdir=os.sep + os.sep.join(["path", "to", "cache", r"\w+"]),
+                doctreedir=os.sep
+                + os.path.join("path", "to", "cache", r"\w+", ".doctrees"),
+                buildername="html",
+            ),
+            {
+                "cacheDir": os.sep + os.sep.join(["path", "to", "cache"]),
+            },
+        ),
+    ],
+)
+def test_cli_default_build_dir(
+    args: list[str], expected: dict[str, Any], context: dict[str, str]
+):
+    """Ensure that we can handle ``${defaultBuildDir}`` variable correctly."""
+    config = SphinxConfig.fromcli(args)
+    assert config is not None
+
+    actual = config.to_application_args(context)
+
+    # pytest overrides stderr on windows, so if we were to put `sys.stderr` in the
+    # `expected` dict this test would fail as `sys.stderr` inside a test function has a
+    # different value.
+    #
+    # So, let's test for it here instead
+    assert actual.pop("status") == sys.stderr
+    assert actual.pop("warning") == sys.stderr
+
+    # Also test the outdir and doctree match the expected pattern
+    assert re.match(expected.pop("outdir"), actual.pop("outdir"))
+    assert re.match(expected.pop("doctreedir"), actual.pop("doctreedir"))
+
+    assert expected == actual
+
+
 ROOT = pathlib.Path(__file__).parent.parent / "sphinx-extensions" / "workspace"
 PY_PATH = ROOT / "code" / "diagnostics.py"
 CONF_PATH = ROOT / "sphinx-extensions" / "conf.py"