lsp: Introduce configuration variables

Configuration objects can now accept variables of the form `${variable}`
and have them expanded by the configuration system at runtime.

This commit adds support for the following variables

- `${scope}`: The full uri of the scope at which the configuration
  object was requested

- `${scopePath}`: The path component of the scope uri

- `${scopeFsPath}`: The path component of the scope uri as a
  filesystem path

Currently these configuration variables will only be expanded if they
appear in a field that is a simple string.

Additionaly, this commit sets the default value for `SphinxConfig.cwd`
to be `${scopeFsPath}`. This should mean that any
`esbonio.sphinx.buildCommand` values in a `pyproject.toml` file will
be evaluated relative to the file's location. Closes swyddfa#711

lib/esbonio/changes/711.fix.md

@@ -0,0 +1 @@
+`esbonio.sphinx.buildCommand` settings provided in a `pyproject.toml` file are now resolved relative to the file's location

lib/esbonio/esbonio/server/__init__.py

@@ -1,6 +1,7 @@
 from esbonio.sphinx_agent.types import Uri
 
 from ._configuration import ConfigChangeEvent
+from ._configuration import ConfigurationContext
 from .events import EventSource
 from .feature import CompletionConfig
 from .feature import CompletionContext
@@ -14,6 +15,7 @@
 __all__ = (
     "__version__",
     "ConfigChangeEvent",
+    "ConfigurationContext",
     "CompletionConfig",
     "CompletionContext",
     "CompletionTrigger",

lib/esbonio/esbonio/server/_configuration.py

@@ -1,12 +1,10 @@
 from __future__ import annotations
 
-import asyncio
 import inspect
 import json
 import pathlib
-import traceback
+import re
 import typing
-from functools import partial
 from typing import Generic
 from typing import TypeVar
 
@@ -55,11 +53,8 @@ class Subscription(Generic[T]):
     callback: ConfigurationCallback
     """The subscription's callback."""
 
-    workspace_scope: str
-    """The corresponding workspace scope for the subscription."""
-
-    file_scope: str
-    """The corresponding file scope for the subscription."""
+    context: ConfigurationContext
+    """The context for this subscription."""
 
 
 @attrs.define
@@ -76,6 +71,67 @@ class ConfigChangeEvent(Generic[T]):
     """The previous configuration value, (if any)."""
 
 
+VARIABLE = re.compile(r"\$\{(\w+)\}")
+
+
+@attrs.define(frozen=True)
+class ConfigurationContext:
+    """The context in which configuration variables are evaluated."""
+
+    file_scope: str
+    """The uri of the file scope of this context."""
+
+    workspace_scope: str
+    """The uri of the workspace scope of this context."""
+
+    @property
+    def scope(self) -> str:
+        """The effective scope of the config context."""
+        return max([self.file_scope, self.workspace_scope], key=len)
+
+    @property
+    def scope_path(self) -> Optional[str]:
+        """The scope uri as a path."""
+        uri = Uri.parse(self.scope)
+        return uri.path
+
+    @property
+    def scope_fs_path(self) -> Optional[str]:
+        """The scope uri as an fs path."""
+        uri = Uri.parse(self.scope)
+        return uri.fs_path
+
+    def expand(self, config: attrs.AttrsInstance) -> attrs.AttrsInstance:
+        """Expand any configuration variables in the given config value."""
+        for name in attrs.fields_dict(type(config)):
+            value = getattr(config, name)
+
+            # For now, we only support variables that are a string.
+            if not isinstance(value, str):
+                continue
+
+            if (match := VARIABLE.match(value)) is None:
+                continue
+
+            setattr(config, name, self.expand_value(match.group(1)))
+
+        return config
+
+    def expand_value(self, variable: str) -> Any:
+        """Return the value for the given variable name."""
+
+        if variable == "scope":
+            return self.scope
+
+        if variable == "scopePath":
+            return self.scope_path
+
+        if variable == "scopeFsPath":
+            return self.scope_fs_path
+
+        raise ValueError(f"Undefined variable: {variable!r}")
+
+
 class Configuration:
     """Manages the configuration values for the server.
 
@@ -114,9 +170,6 @@ def __init__(self, server: EsbonioLanguageServer):
         self._subscriptions: Dict[Subscription, Any] = {}
         """Subscriptions and their last known value"""
 
-        self._tasks: Set[asyncio.Task] = set()
-        """Holds tasks that are currently executing an async config handler."""
-
     @property
     def initialization_options(self):
         return self._initialization_options
@@ -172,9 +225,11 @@ def subscribe(
         """
         file_scope = self._uri_to_file_scope(scope)
         workspace_scope = self._uri_to_workspace_scope(scope)
-        subscription = Subscription(
-            section, spec, callback, workspace_scope, file_scope
+
+        context = ConfigurationContext(
+            file_scope=file_scope, workspace_scope=workspace_scope
         )
+        subscription = Subscription(section, spec, callback, context)
 
         if subscription in self._subscriptions:
             self.logger.debug("Ignoring duplicate subscription: %s", subscription)
@@ -192,8 +247,7 @@ def _notify_subscriptions(self, *args):
             value = self._get_config(
                 subscription.section,
                 subscription.spec,
-                subscription.workspace_scope,
-                subscription.file_scope,
+                subscription.context,
             )
 
             # No need to notify if nothing has changed
@@ -204,9 +258,7 @@ def _notify_subscriptions(self, *args):
 
             self._subscriptions[subscription] = value
             change_event = ConfigChangeEvent(
-                scope=max(
-                    [subscription.file_scope, subscription.workspace_scope], key=len
-                ),
+                scope=subscription.context.scope,
                 value=value,
                 previous=previous_value,
             )
@@ -215,8 +267,7 @@ def _notify_subscriptions(self, *args):
             try:
                 ret = subscription.callback(change_event)
                 if inspect.iscoroutine(ret):
-                    task = asyncio.create_task(ret)
-                    task.add_done_callback(partial(self._finish_task, subscription))
+                    self.server.run_task(ret)
 
             except Exception:
                 self.logger.error(
@@ -225,17 +276,6 @@ def _notify_subscriptions(self, *args):
                     exc_info=True,
                 )
 
-    def _finish_task(self, subscription: Subscription, task: asyncio.Task[None]):
-        """Cleanup a finished task."""
-        self._tasks.discard(task)
-
-        if (exc := task.exception()) is not None:
-            self.logger.error(
-                "Error in async configuration handler '%s'\n%s",
-                subscription.callback,
-                "".join(traceback.format_exception(type(exc), exc, exc.__traceback__)),
-            )
-
     def get(self, section: str, spec: Type[T], scope: Optional[Uri] = None) -> T:
         """Get the requested configuration section.
 
@@ -255,10 +295,12 @@ def get(self, section: str, spec: Type[T], scope: Optional[Uri] = None) -> T:
         T
            The requested configuration section parsed as an instance of ``T``.
         """
-        file_scope = self._uri_to_file_scope(scope)
-        workspace_scope = self._uri_to_workspace_scope(scope)
+        context = ConfigurationContext(
+            file_scope=self._uri_to_file_scope(scope),
+            workspace_scope=self._uri_to_workspace_scope(scope),
+        )
 
-        return self._get_config(section, spec, workspace_scope, file_scope)
+        return self._get_config(section, spec, context)
 
     def scope_for(self, uri: Uri) -> str:
         """Return the configuration scope that corresponds to the given uri.
@@ -273,24 +315,27 @@ def scope_for(self, uri: Uri) -> str:
         str
            The scope corresponding with the given uri
         """
+        context = ConfigurationContext(
+            file_scope=self._uri_to_file_scope(uri),
+            workspace_scope=self._uri_to_workspace_scope(uri),
+        )
 
-        file_scope = self._uri_to_file_scope(uri)
-        workspace_scope = self._uri_to_workspace_scope(uri)
-
-        return max([file_scope, workspace_scope], key=len)
+        return context.scope
 
     def _get_config(
-        self, section: str, spec: Type[T], workspace_scope: str, file_scope: str
+        self,
+        section: str,
+        spec: Type[T],
+        context: ConfigurationContext,
     ) -> T:
         """Get the requested configuration section."""
 
-        self.logger.debug("File scope: '%s'", file_scope)
-        self.logger.debug("Workspace scope: '%s'", workspace_scope)
+        self.logger.debug("%s", context)
 
         # To keep things simple, this method assumes that all available config is already
         # cached locally. Populating the cache is handled elsewhere.
-        file_config = self._file_config.get(file_scope, {})
-        workspace_config = self._workspace_config.get(workspace_scope, {})
+        file_config = self._file_config.get(context.file_scope, {})
+        workspace_config = self._workspace_config.get(context.workspace_scope, {})
 
         # Combine and resolve all the config sources - order matters!
         config = _merge_configs(
@@ -303,10 +348,11 @@ def _get_config(
         for name in section.split("."):
             config_section = config_section.get(name, {})
 
-        self.logger.debug("Resolved config: %s", json.dumps(config_section, indent=2))
+        self.logger.debug("%s: %s", section, json.dumps(config_section, indent=2))
 
         try:
             value = self.converter.structure(config_section, spec)
+            value = context.expand(value)
             self.logger.debug("%s", value)
 
             return value
@@ -342,7 +388,7 @@ def _discover_config_files(self) -> List[pathlib.Path]:
             if (folder_path := Uri.parse(uri).fs_path) is None:
                 continue
 
-            self.logger.debug("Scanning workspace folder: '%s'", folder_path)
+            self.logger.debug("Looking for pyproject.toml files in: '%s'", folder_path)
             for p in pathlib.Path(folder_path).glob("**/pyproject.toml"):
                 self.logger.debug("Found '%s'", p)
                 paths.append(p)

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

@@ -54,7 +54,7 @@ class SphinxConfig:
     env_passthrough: List[str] = attrs.field(factory=list)
     """List of environment variables to pass through to the Sphinx subprocess"""
 
-    cwd: str = attrs.field(default="")
+    cwd: str = attrs.field(default="${scopeFsPath}")
     """The working directory to use."""
 
     python_path: List[pathlib.Path] = attrs.field(factory=list)
@@ -133,8 +133,7 @@ def _resolve_cwd(
            The working directory to launch the sphinx agent in.
            If ``None``, the working directory could not be determined.
         """
-
-        if self.cwd:
+        if self.cwd and self.cwd != "${scopeFsPath}":
             return self.cwd
 
         candidates = [Uri.parse(f) for f in workspace.folders.keys()]

lib/esbonio/tests/server/features/test_sphinx_config.py

@@ -175,4 +175,5 @@ def test_resolve(
     expected
        The expected outcome
     """
-    assert config.resolve(Uri.parse(uri), workspace, logger) == expected
+    actual = config.resolve(Uri.parse(uri), workspace, logger)
+    assert actual == expected