Skip to content

Commit

Permalink
lsp: Introduce configuration variables
Browse files Browse the repository at this point in the history
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
  • Loading branch information
alcarney committed Jul 7, 2024
1 parent 4c91a01 commit c154dd1
Show file tree
Hide file tree
Showing 6 changed files with 286 additions and 49 deletions.
1 change: 1 addition & 0 deletions lib/esbonio/changes/711.fix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
`esbonio.sphinx.buildCommand` settings provided in a `pyproject.toml` file are now resolved relative to the file's location
2 changes: 2 additions & 0 deletions lib/esbonio/esbonio/server/__init__.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -14,6 +15,7 @@
__all__ = (
"__version__",
"ConfigChangeEvent",
"ConfigurationContext",
"CompletionConfig",
"CompletionContext",
"CompletionTrigger",
Expand Down
136 changes: 91 additions & 45 deletions lib/esbonio/esbonio/server/_configuration.py
Original file line number Diff line number Diff line change
@@ -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

Expand Down Expand Up @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -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,
)
Expand All @@ -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(
Expand All @@ -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.
Expand All @@ -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.
Expand All @@ -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(
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down
5 changes: 2 additions & 3 deletions lib/esbonio/esbonio/server/features/sphinx_manager/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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()]
Expand Down
3 changes: 2 additions & 1 deletion lib/esbonio/tests/server/features/test_sphinx_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading

0 comments on commit c154dd1

Please sign in to comment.