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

Introduce configuration variables #847

Merged
merged 5 commits into from
Jul 7, 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 .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
.env
.ipynb_checkpoints
.tox
.vscode/extensions/esbonio
.vscode-test

*.pyc
Expand Down
4 changes: 2 additions & 2 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,15 @@ repos:
- id: trailing-whitespace

- repo: https://github.com/astral-sh/ruff-pre-commit
rev: v0.4.10
rev: v0.5.0
hooks:
- id: ruff
args: [--fix]

- id: ruff-format

- repo: https://github.com/pre-commit/mirrors-mypy
rev: 'v1.10.0'
rev: 'v1.10.1'
hooks:
- id: mypy
name: mypy (scripts)
Expand Down
3 changes: 3 additions & 0 deletions .vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@
"--folder-uri=${workspaceRoot}/docs",
"--folder-uri=${workspaceRoot}/lib/esbonio/tests/workspaces/demo"
],
"env": {
"TZ": "UTC"
},
"outFiles": [
"${workspaceRoot}/code/dist/node/**/*.js"
],
Expand Down
2 changes: 1 addition & 1 deletion .vscode/settings.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"[python]": {
"editor.defaultFormatter": "ms-python.black-formatter",
"editor.defaultFormatter": "charliermarsh.ruff",
"editor.codeActionsOnSave": {
"source.organizeImports": "explicit"
},
Expand Down
6 changes: 5 additions & 1 deletion code/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ include ../.devcontainer/tools.mk

ESBONIO ?= --pre esbonio

.PHONY: dist dev-deps release-deps release
.PHONY: dist dev-deps release-deps release install

watch: dev-deps $(NPM)
-test -d dist && rm -r dist
Expand All @@ -12,6 +12,10 @@ compile: dev-deps $(NPM)
-test -d dist && rm -r dist
$(NPM) run compile

install: compile
-test -d ../.vscode/extensions || mkdir -p ../.vscode/extensions
test -L ../.vscode/extensions/esbonio || ln -s $(PWD) ../.vscode/extensions/esbonio

dist: release-deps $(NPM)
-test -d dist && rm -r dist
$(NPM) run package
Expand Down
2 changes: 1 addition & 1 deletion code/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
"package": "vsce package --pre-release --baseImagesUrl https://github.com/swyddfa/esbonio/raw/release/code/",
"vscode:prepublish": "npm run compile"
},
"main": "dist/node/extension",
"main": "dist/node/extension.js",
"extensionDependencies": [
"ms-python.python",
"chrisjsewell.myst-tml-syntax"
Expand Down
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
2 changes: 1 addition & 1 deletion lib/esbonio/hatch.toml
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,4 @@ value = ["tests/sphinx-agent", "tests/e2e"]

[envs.hatch-static-analysis]
config-path = "ruff_defaults.toml"
dependencies = ["ruff==0.4.10"]
dependencies = ["ruff==0.5.0"]
Loading