From c154dd17d05be2d97bcebc07ab7e8277b708d650 Mon Sep 17 00:00:00 2001 From: Alex Carney Date: Sun, 7 Jul 2024 10:15:14 +0100 Subject: [PATCH] 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 #711 --- lib/esbonio/changes/711.fix.md | 1 + lib/esbonio/esbonio/server/__init__.py | 2 + lib/esbonio/esbonio/server/_configuration.py | 136 ++++++++----- .../server/features/sphinx_manager/config.py | 5 +- .../server/features/test_sphinx_config.py | 3 +- .../tests/server/test_configuration.py | 188 ++++++++++++++++++ 6 files changed, 286 insertions(+), 49 deletions(-) create mode 100644 lib/esbonio/changes/711.fix.md diff --git a/lib/esbonio/changes/711.fix.md b/lib/esbonio/changes/711.fix.md new file mode 100644 index 00000000..59b19545 --- /dev/null +++ b/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 diff --git a/lib/esbonio/esbonio/server/__init__.py b/lib/esbonio/esbonio/server/__init__.py index 4b7f4b23..d57f3871 100644 --- a/lib/esbonio/esbonio/server/__init__.py +++ b/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", diff --git a/lib/esbonio/esbonio/server/_configuration.py b/lib/esbonio/esbonio/server/_configuration.py index fb6ed333..fad620a1 100644 --- a/lib/esbonio/esbonio/server/_configuration.py +++ b/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) diff --git a/lib/esbonio/esbonio/server/features/sphinx_manager/config.py b/lib/esbonio/esbonio/server/features/sphinx_manager/config.py index 855f53d3..53e0c48f 100644 --- a/lib/esbonio/esbonio/server/features/sphinx_manager/config.py +++ b/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()] diff --git a/lib/esbonio/tests/server/features/test_sphinx_config.py b/lib/esbonio/tests/server/features/test_sphinx_config.py index 918ac9a6..96e3bdd5 100644 --- a/lib/esbonio/tests/server/features/test_sphinx_config.py +++ b/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 diff --git a/lib/esbonio/tests/server/test_configuration.py b/lib/esbonio/tests/server/test_configuration.py index 2480a11c..d6fa90ad 100644 --- a/lib/esbonio/tests/server/test_configuration.py +++ b/lib/esbonio/tests/server/test_configuration.py @@ -26,6 +26,12 @@ class ExampleConfig: log_names: List[str] = attrs.field(factory=list) +@attrs.define +class ColorConfig: + color: str + scope: str = attrs.field(default="${scope}") + + @pytest.fixture def server(event_loop): """Return a server instance for testing.""" @@ -234,6 +240,188 @@ def server(event_loop): id="workspace-file-override[win]", marks=pytest.mark.skipif(not IS_WIN, reason="windows only"), ), + pytest.param( # Check that we can expand config variables correctly + {}, + { + "file:///path/to/workspace": dict( + esbonio=dict(colors=dict(color="red")) + ), + }, + {}, + "esbonio.colors", + ColorConfig, + "file:///path/to/workspace/docs/file.txt", + ColorConfig(color="red", scope="file:///path/to/workspace"), + id="scope-variable[workspace][unix]", + marks=pytest.mark.skipif(IS_WIN, reason="windows"), + ), + pytest.param( # Check that we can expand config variables correctly + {}, + { + "file:///c%3A/path/to/workspace": dict( + esbonio=dict(colors=dict(color="red")) + ), + }, + {}, + "esbonio.colors", + ColorConfig, + "file:///c:/path/to/workspace/docs/file.txt", + ColorConfig(color="red", scope="file:///c:/path/to/workspace"), + id="scope-variable[workspace][win]", + marks=pytest.mark.skipif(not IS_WIN, reason="windows only"), + ), + pytest.param( # Check that we can expand config variables correctly + {}, + { + "file:///path/to/workspace": dict( + esbonio=dict(colors=dict(color="red")) + ), + }, + { + "file:///path/to/workspace/docs": dict( + esbonio=dict(colors=dict(color="blue")) + ), + }, + "esbonio.colors", + ColorConfig, + "file:///path/to/workspace/docs/file.txt", + ColorConfig(color="red", scope="file:///path/to/workspace/docs"), + id="scope-variable[workspace+file][unix]", + marks=pytest.mark.skipif(IS_WIN, reason="windows"), + ), + pytest.param( # Check that we can expand config variables correctly + {}, + { + "file:///c%3A/path/to/workspace": dict( + esbonio=dict(colors=dict(color="red")) + ), + }, + { + "file:///c%3A/path/to/workspace/docs": dict( + esbonio=dict(colors=dict(color="blue")) + ), + }, + "esbonio.colors", + ColorConfig, + "file:///c:/path/to/workspace/docs/file.txt", + ColorConfig(color="red", scope="file:///c:/path/to/workspace/docs"), + id="scope-variable[workspace+file][win]", + marks=pytest.mark.skipif(not IS_WIN, reason="windows only"), + ), + pytest.param( # The user should still be able to override them + {}, + { + "file:///path/to/workspace": dict( + esbonio=dict(colors=dict(color="red")) + ), + }, + { + "file:///path/to/workspace/docs": dict( + esbonio=dict(colors=dict(color="blue", scope="file:///my/scope")) + ), + }, + "esbonio.colors", + ColorConfig, + "file:///path/to/workspace/docs/file.txt", + ColorConfig(color="red", scope="file:///my/scope"), + id="scope-variable-override[unix]", + marks=pytest.mark.skipif(IS_WIN, reason="windows"), + ), + pytest.param( # The user should still be able to override them + {}, + { + "file:///c%3A/path/to/workspace": dict( + esbonio=dict(colors=dict(color="red")) + ), + }, + { + "file:///c%3A/path/to/workspace/docs": dict( + esbonio=dict(colors=dict(color="blue", scope="file:///my/scope")) + ), + }, + "esbonio.colors", + ColorConfig, + "file:///c:/path/to/workspace/docs/file.txt", + ColorConfig(color="red", scope="file:///my/scope"), + id="scope-variable-override[win]", + marks=pytest.mark.skipif(not IS_WIN, reason="windows only"), + ), + pytest.param( + {}, + { + "file:///path/to/workspace": dict( + esbonio=dict(colors=dict(color="red")) + ), + }, + { + "file:///path/to/workspace/docs": dict( + esbonio=dict(colors=dict(color="blue", scope="${scopePath}")) + ), + }, + "esbonio.colors", + ColorConfig, + "file:///path/to/workspace/docs/file.txt", + ColorConfig(color="red", scope="/path/to/workspace/docs"), + id="scope-path-variable[unix]", + marks=pytest.mark.skipif(IS_WIN, reason="windows"), + ), + pytest.param( + {}, + { + "file:///c%3A/path/to/workspace": dict( + esbonio=dict(colors=dict(color="red")) + ), + }, + { + "file:///c%3A/path/to/workspace/docs": dict( + esbonio=dict(colors=dict(color="blue", scope="${scopePath}")) + ), + }, + "esbonio.colors", + ColorConfig, + "file:///c:/path/to/workspace/docs/file.txt", + ColorConfig(color="red", scope="/c:/path/to/workspace/docs"), + id="scope-path-variable[win]", + marks=pytest.mark.skipif(not IS_WIN, reason="windows only"), + ), + pytest.param( + {}, + { + "file:///path/to/workspace": dict( + esbonio=dict(colors=dict(color="red")) + ), + }, + { + "file:///path/to/workspace/docs": dict( + esbonio=dict(colors=dict(color="blue", scope="${scopeFsPath}")) + ), + }, + "esbonio.colors", + ColorConfig, + "file:///path/to/workspace/docs/file.txt", + ColorConfig(color="red", scope="/path/to/workspace/docs"), + id="scope-fspath-variable[unix]", + marks=pytest.mark.skipif(IS_WIN, reason="windows"), + ), + pytest.param( + {}, + { + "file:///c%3A/path/to/workspace": dict( + esbonio=dict(colors=dict(color="red")) + ), + }, + { + "file:///c%3A/path/to/workspace/docs": dict( + esbonio=dict(colors=dict(color="blue", scope="${scopeFsPath}")) + ), + }, + "esbonio.colors", + ColorConfig, + "file:///c:/path/to/workspace/docs/file.txt", + ColorConfig(color="red", scope="c:\\path\\to\\workspace\\docs"), + id="scope-fspath-variable[win]", + marks=pytest.mark.skipif(not IS_WIN, reason="windows only"), + ), ], ) def test_get_configuration(