diff --git a/lib/esbonio/changes/718.fix.md b/lib/esbonio/changes/718.fix.md new file mode 100644 index 000000000..8a9e79c0c --- /dev/null +++ b/lib/esbonio/changes/718.fix.md @@ -0,0 +1 @@ +The server will no longer raise a `ValueError` when used in a situation where there is an empty workspace diff --git a/lib/esbonio/esbonio/server/features/sphinx_manager/config.py b/lib/esbonio/esbonio/server/features/sphinx_manager/config.py index a21cf5d87..ba92489f0 100644 --- a/lib/esbonio/esbonio/server/features/sphinx_manager/config.py +++ b/lib/esbonio/esbonio/server/features/sphinx_manager/config.py @@ -141,18 +141,23 @@ def _resolve_cwd( if self.cwd: return self.cwd - for folder_uri in workspace.folders.keys(): - folder_uri = Uri.parse(folder_uri) - if folder_uri and str(uri).startswith(str(folder_uri)): - break - else: - folder_uri = Uri.parse(workspace.root_uri) - - if folder_uri is None or (cwd := folder_uri.fs_path) is None: - logger.error("Unable to determine working directory from '%s'", folder_uri) - return None + candidates = [Uri.parse(f) for f in workspace.folders.keys()] + + if workspace.root_uri is not None: + if (root_uri := Uri.parse(workspace.root_uri)) not in candidates: + candidates.append(root_uri) + + for folder in candidates: + if str(uri).startswith(str(folder)): + if (cwd := folder.fs_path) is None: + logger.error( + "Unable to determine working directory from '%s'", folder + ) + return None - return cwd + return cwd + + return None def _resolve_python_path(self, logger: logging.Logger) -> List[pathlib.Path]: """Return the list of paths to put on the sphinx agent's ``PYTHONPATH`` @@ -161,9 +166,6 @@ def _resolve_python_path(self, logger: logging.Logger) -> List[pathlib.Path]: packages into the user's Python environment. This method will locate the installation path of the sphinx agent and return it. - Additionally if the ``enable_dev_tools`` flag is set, this will attempt to - locate the ``lsp_devtools`` package - Parameters ---------- logger diff --git a/lib/esbonio/tests/server/features/test_sphinx_config.py b/lib/esbonio/tests/server/features/test_sphinx_config.py new file mode 100644 index 000000000..701cbec3e --- /dev/null +++ b/lib/esbonio/tests/server/features/test_sphinx_config.py @@ -0,0 +1,177 @@ +import logging +import pathlib +from typing import Optional + +import pytest +from lsprotocol.types import WorkspaceFolder +from pygls.workspace import Workspace + +from esbonio.server import Uri +from esbonio.server.features.sphinx_manager import SphinxConfig + +logger = logging.getLogger(__name__) + + +# Default values +CWD = "/path/to/workspace" +PYTHON_CMD = ["/bin/python"] +BUILD_CMD = ["sphinx-build", "-M", "html", "src", "dest"] +PYPATH = [pathlib.Path("/path/to/site-packages/esbonio")] + + +def mk_uri(path: str) -> str: + return str(Uri.for_file(path)) + + +@pytest.mark.parametrize( + "uri, workspace, config, expected", + [ + ( # If everything is specified, resolve should be a no-op + "file:///path/to/workspace/file.rst", + Workspace(None), + SphinxConfig( + python_command=PYTHON_CMD, + build_command=BUILD_CMD, + cwd=CWD, + python_path=PYPATH, + ), + SphinxConfig( + python_command=PYTHON_CMD, + build_command=BUILD_CMD, + cwd=CWD, + python_path=PYPATH, + ), + ), + ( # If no cwd is given, and there is no available workspace root the config + # should be considered invalid + "file:///path/to/file.rst", + Workspace(None), + SphinxConfig( + python_command=PYTHON_CMD, + build_command=BUILD_CMD, + python_path=PYPATH, + ), + None, + ), + ( # If the workspace is empty, we should still be able to progress as long as + # the user provides a cwd + "file:///path/to/file.rst", + Workspace(None), + SphinxConfig( + python_command=PYTHON_CMD, + build_command=BUILD_CMD, + python_path=PYPATH, + cwd=CWD, + ), + SphinxConfig( + python_command=PYTHON_CMD, + build_command=BUILD_CMD, + python_path=PYPATH, + cwd=CWD, + ), + ), + ( # If only a ``root_uri`` is given use that. + "file:///path/to/workspace/file.rst", + Workspace(mk_uri(CWD)), + SphinxConfig( + python_command=PYTHON_CMD, + build_command=BUILD_CMD, + python_path=PYPATH, + ), + SphinxConfig( + python_command=PYTHON_CMD, + build_command=BUILD_CMD, + python_path=PYPATH, + cwd=CWD, + ), + ), + ( # Assuming that the requested uri resides within it. + "file:///path/to/other/workspace/file.rst", + Workspace(mk_uri(CWD)), + SphinxConfig( + python_command=PYTHON_CMD, + build_command=BUILD_CMD, + python_path=PYPATH, + ), + None, + ), + ( # Otherwise, prefer workspace_folders. + "file:///path/to/workspace/file.rst", + Workspace( + "file:///path/to", + workspace_folders=[WorkspaceFolder(mk_uri(CWD), "workspace")], + ), + SphinxConfig( + python_command=PYTHON_CMD, + build_command=BUILD_CMD, + python_path=PYPATH, + ), + SphinxConfig( + python_command=PYTHON_CMD, + build_command=BUILD_CMD, + python_path=PYPATH, + cwd=CWD, + ), + ), + ( # Handle multi-root scenarios. + "file:///path/to/workspace-b/file.rst", + Workspace( + "file:///path/to", + workspace_folders=[ + WorkspaceFolder("file:///path/to/workspace-a", "workspace-a"), + WorkspaceFolder("file:///path/to/workspace-b", "workspace-b"), + ], + ), + SphinxConfig( + python_command=PYTHON_CMD, + build_command=BUILD_CMD, + python_path=PYPATH, + ), + SphinxConfig( + python_command=PYTHON_CMD, + build_command=BUILD_CMD, + python_path=PYPATH, + cwd="/path/to/workspace-b", + ), + ), + ( # Again, make sure the requested uri resides within the workspace. + "file:///path/for/workspace-c/file.rst", + Workspace( + "file:///path/to", + workspace_folders=[ + WorkspaceFolder("file:///path/to/workspace-a", "workspace-a"), + WorkspaceFolder("file:///path/to/workspace-b", "workspace-b"), + ], + ), + SphinxConfig( + python_command=PYTHON_CMD, + build_command=BUILD_CMD, + python_path=PYPATH, + ), + None, + ), + ], +) +def test_resolve( + uri: str, + workspace: Workspace, + config: SphinxConfig, + expected: Optional[SphinxConfig], +): + """Ensure that we can resolve a user's configuration correctly. + + Parameters + ---------- + uri + The uri the config should be resolved relative to + + workspace + The workspace in which to resolve the configuration + + config + The base configuration to resolve + + expected + The expected outcome + """ + assert config.resolve(Uri.parse(uri), workspace, logger) == expected diff --git a/lib/esbonio/tests/sphinx-agent/test_sa_unit.py b/lib/esbonio/tests/sphinx-agent/test_sa_unit.py index b731c0237..c99a13ed1 100644 --- a/lib/esbonio/tests/sphinx-agent/test_sa_unit.py +++ b/lib/esbonio/tests/sphinx-agent/test_sa_unit.py @@ -1,7 +1,6 @@ import logging import os import pathlib -import sys from typing import Any from typing import Dict from typing import List @@ -10,13 +9,7 @@ from unittest import mock import pytest -from lsprotocol.types import WorkspaceFolder -from pygls.workspace import Workspace -from esbonio.server import Uri -from esbonio.server.features.sphinx_manager.config import ( - SphinxConfig as SphinxAgentConfig, -) from esbonio.sphinx_agent.config import SphinxConfig from esbonio.sphinx_agent.log import SphinxLogHandler @@ -438,80 +431,6 @@ def test_cli_arg_handling(args: List[str], expected: Dict[str, Any]): assert expected == config.to_application_args() -@pytest.mark.parametrize( - "config, uri, workspace, expected", - [ - # If everything is specified, resolve should be a no-op - ( - SphinxAgentConfig( - python_command=[sys.executable], - build_command=["sphinx-build", "-M", "html", "src", "dest"], - cwd="/path/to/workspace", - python_path=["/path/to/site-packages/esbonio/"], - ), - "file::///path/to/file.rst", - Workspace(None), - SphinxAgentConfig( - python_command=[sys.executable], - build_command=["sphinx-build", "-M", "html", "src", "dest"], - cwd="/path/to/workspace", - python_path=["/path/to/site-packages/esbonio/"], - ), - ), - # If no cwd given, we should try to pick one based on the given workspace - ( - SphinxAgentConfig( - python_command=[sys.executable], - build_command=["sphinx-build", "-M", "html", "src", "dest"], - python_path=["/path/to/site-packages/esbonio/"], - ), - "file::///path/to/file.rst", - Workspace("file:///path/to/workspace/root"), - SphinxAgentConfig( - python_command=[sys.executable], - build_command=["sphinx-build", "-M", "html", "src", "dest"], - cwd=os.path.join(".", "path", "to", "workspace", "root")[1:], - python_path=["/path/to/site-packages/esbonio/"], - ), - ), - ( - SphinxAgentConfig( - python_command=[sys.executable], - build_command=["sphinx-build", "-M", "html", "src", "dest"], - python_path=["/path/to/site-packages/esbonio/"], - ), - "file:///path/to/workspace-b/file.rst", - Workspace( - "file:///path/to/workspace/root", - workspace_folders=[ - WorkspaceFolder( - uri="file:///path/to/workspace-a", name="Workspace A" - ), - WorkspaceFolder( - uri="file:///path/to/workspace-b", name="Workspace B" - ), - ], - ), - SphinxAgentConfig( - python_command=[sys.executable], - build_command=["sphinx-build", "-M", "html", "src", "dest"], - cwd=os.path.join(".", "path", "to", "workspace-b")[1:], - python_path=["/path/to/site-packages/esbonio/"], - ), - ), - ], -) -def test_resolve_sphinx_config( - config: SphinxAgentConfig, - uri: str, - workspace: Workspace, - expected: SphinxAgentConfig, -): - """Ensure that we can resolve the client side config for the SphinxAgent - correctly.""" - assert expected == config.resolve(Uri.parse(uri), workspace, logger) - - ROOT = pathlib.Path(__file__).parent.parent / "sphinx-extensions" / "workspace" PY_PATH = ROOT / "code" / "diagnostics.py" CONF_PATH = ROOT / "sphinx-extensions" / "conf.py"