diff --git a/docs/updates.md b/docs/updates.md index d26b66d..832f3da 100644 --- a/docs/updates.md +++ b/docs/updates.md @@ -52,7 +52,7 @@ if info and info.available: if client.download_update(on_progress): # Apply and restart - client.apply_update_and_restart() + client.apply_update_on_exit(restart=True) ``` ## Configuration diff --git a/pdm.lock b/pdm.lock index 7ce71de..d86b255 100644 --- a/pdm.lock +++ b/pdm.lock @@ -5,7 +5,7 @@ groups = ["default", "build", "lint", "test"] strategy = ["inherit_metadata"] lock_version = "4.5.0" -content_hash = "sha256:e85a2b6900e703e4c93e257c1c94c30f48735d346f031475f8380b42b70c7e95" +content_hash = "sha256:819ce64c96cf4526bcc1f30c8cf820cd22a9196f809ddc3af94f8e5e6c772bae" [[metadata.targets]] requires_python = ">=3.14,<3.15" @@ -336,7 +336,7 @@ files = [ [[package]] name = "porringer" -version = "0.2.1.dev54" +version = "0.2.1.dev56" requires_python = ">=3.14" summary = "" groups = ["default"] @@ -349,8 +349,8 @@ dependencies = [ "userpath>=1.9.2", ] files = [ - {file = "porringer-0.2.1.dev54-py3-none-any.whl", hash = "sha256:679363277906be375fd5d24f1cb1207cebd6d9f62958d3f5c733753a544c795f"}, - {file = "porringer-0.2.1.dev54.tar.gz", hash = "sha256:e2d51685e95440aa9c81cb826168d28b316c088c8ba79a67c5611a4d3b902bde"}, + {file = "porringer-0.2.1.dev56-py3-none-any.whl", hash = "sha256:e855e5582e542f6050b6f976233893dac980241c4b999f3e480cf649b8295b40"}, + {file = "porringer-0.2.1.dev56.tar.gz", hash = "sha256:aca45a1a33a4acb0833a642ca7751aeeca6986aab7405e74544deaf327f669b5"}, ] [[package]] @@ -631,29 +631,29 @@ files = [ [[package]] name = "ruff" -version = "0.15.3" +version = "0.15.4" requires_python = ">=3.7" summary = "An extremely fast Python linter and code formatter, written in Rust." groups = ["lint"] files = [ - {file = "ruff-0.15.3-py3-none-linux_armv6l.whl", hash = "sha256:f7df0fd6f889a8d8de2ddb48a9eb55150954400f2157ea15b21a2f49ecaaf988"}, - {file = "ruff-0.15.3-py3-none-macosx_10_12_x86_64.whl", hash = "sha256:0198b5445197d443c3bbf2cc358f4bd477fb3951e3c7f2babc13e9bb490614a8"}, - {file = "ruff-0.15.3-py3-none-macosx_11_0_arm64.whl", hash = "sha256:adf95b5be57b25fbbbc07cd68d37414bee8729e807ad0217219558027186967e"}, - {file = "ruff-0.15.3-py3-none-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:8b56dbd9cd86489ccbad96bb58fa4c958342b5510fdeb60ea13d9d3566bd845c"}, - {file = "ruff-0.15.3-py3-none-manylinux_2_17_armv7l.manylinux2014_armv7l.whl", hash = "sha256:c6f263ce511871955d8c5401b62c7e863988ea4d0527aa0a3b1b7ecff4d4abc4"}, - {file = "ruff-0.15.3-py3-none-manylinux_2_17_i686.manylinux2014_i686.whl", hash = "sha256:e90fa1bed82ffede5768232b9bd23212c547ab7cd74c752007ecade1d895ee1a"}, - {file = "ruff-0.15.3-py3-none-manylinux_2_17_ppc64le.manylinux2014_ppc64le.whl", hash = "sha256:2e9d53760b7061ddbe5ea9e25381332c607fc14c40bde78f8a25392a93a68d74"}, - {file = "ruff-0.15.3-py3-none-manylinux_2_17_s390x.manylinux2014_s390x.whl", hash = "sha256:ec90e3b78c56c4acca4264d371dd48e29215ecb673cc2fa3c4b799b72050e491"}, - {file = "ruff-0.15.3-py3-none-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:f7ce448fd395f822e34c8f6f7dfcd84b6726340082950858f92c4daa6baf8915"}, - {file = "ruff-0.15.3-py3-none-manylinux_2_31_riscv64.whl", hash = "sha256:14f7d763962d385f75b9b3b57fcc5661c56c20d8b1ddc9f5c881b5fa0ba499fa"}, - {file = "ruff-0.15.3-py3-none-musllinux_1_2_aarch64.whl", hash = "sha256:b57084e3a3d65418d376c7023711c37cce023cd2fb038a76ba15ee21f3c2c2ee"}, - {file = "ruff-0.15.3-py3-none-musllinux_1_2_armv7l.whl", hash = "sha256:d567523ff7dcf3112b0f71231d18c3506dd06943359476ee64dea0f9c8f63976"}, - {file = "ruff-0.15.3-py3-none-musllinux_1_2_i686.whl", hash = "sha256:4223088d255bf31a50b6640445b39f668164d64c23e5fa403edfb1e0b11122e5"}, - {file = "ruff-0.15.3-py3-none-musllinux_1_2_x86_64.whl", hash = "sha256:32399ddae088970b2db6efd8d3f49981375cb828075359b6c088ed1fe63d64e1"}, - {file = "ruff-0.15.3-py3-none-win32.whl", hash = "sha256:1f1eb95ff614351e3a89a862b6d94e6c42c170e61916e1f20facd6c38477f5f3"}, - {file = "ruff-0.15.3-py3-none-win_amd64.whl", hash = "sha256:2b22dffe5f5e1e537097aa5208684f069e495f980379c4491b1cfb198a444d0c"}, - {file = "ruff-0.15.3-py3-none-win_arm64.whl", hash = "sha256:82443c14d694d4cbd9e598ede27ef5d6f08389ccad91c933be775ea2f4e66f76"}, - {file = "ruff-0.15.3.tar.gz", hash = "sha256:78757853320d8ddb9da24e614ef69a37bcbcfd477e5a6435681188d4bce4eaa1"}, + {file = "ruff-0.15.4-py3-none-linux_armv6l.whl", hash = "sha256:a1810931c41606c686bae8b5b9a8072adac2f611bb433c0ba476acba17a332e0"}, + {file = "ruff-0.15.4-py3-none-macosx_10_12_x86_64.whl", hash = "sha256:5a1632c66672b8b4d3e1d1782859e98d6e0b4e70829530666644286600a33992"}, + {file = "ruff-0.15.4-py3-none-macosx_11_0_arm64.whl", hash = "sha256:a4386ba2cd6c0f4ff75252845906acc7c7c8e1ac567b7bc3d373686ac8c222ba"}, + {file = "ruff-0.15.4-py3-none-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:b2496488bdfd3732747558b6f95ae427ff066d1fcd054daf75f5a50674411e75"}, + {file = "ruff-0.15.4-py3-none-manylinux_2_17_armv7l.manylinux2014_armv7l.whl", hash = "sha256:3f1c4893841ff2d54cbda1b2860fa3260173df5ddd7b95d370186f8a5e66a4ac"}, + {file = "ruff-0.15.4-py3-none-manylinux_2_17_i686.manylinux2014_i686.whl", hash = "sha256:820b8766bd65503b6c30aaa6331e8ef3a6e564f7999c844e9a547c40179e440a"}, + {file = "ruff-0.15.4-py3-none-manylinux_2_17_ppc64le.manylinux2014_ppc64le.whl", hash = "sha256:c9fb74bab47139c1751f900f857fa503987253c3ef89129b24ed375e72873e85"}, + {file = "ruff-0.15.4-py3-none-manylinux_2_17_s390x.manylinux2014_s390x.whl", hash = "sha256:f80c98765949c518142b3a50a5db89343aa90f2c2bf7799de9986498ae6176db"}, + {file = "ruff-0.15.4-py3-none-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:451a2e224151729b3b6c9ffb36aed9091b2996fe4bdbd11f47e27d8f2e8888ec"}, + {file = "ruff-0.15.4-py3-none-manylinux_2_31_riscv64.whl", hash = "sha256:a8f157f2e583c513c4f5f896163a93198297371f34c04220daf40d133fdd4f7f"}, + {file = "ruff-0.15.4-py3-none-musllinux_1_2_aarch64.whl", hash = "sha256:917cc68503357021f541e69b35361c99387cdbbf99bd0ea4aa6f28ca99ff5338"}, + {file = "ruff-0.15.4-py3-none-musllinux_1_2_armv7l.whl", hash = "sha256:e9737c8161da79fd7cfec19f1e35620375bd8b2a50c3e77fa3d2c16f574105cc"}, + {file = "ruff-0.15.4-py3-none-musllinux_1_2_i686.whl", hash = "sha256:291258c917539e18f6ba40482fe31d6f5ac023994ee11d7bdafd716f2aab8a68"}, + {file = "ruff-0.15.4-py3-none-musllinux_1_2_x86_64.whl", hash = "sha256:3f83c45911da6f2cd5936c436cf86b9f09f09165f033a99dcf7477e34041cbc3"}, + {file = "ruff-0.15.4-py3-none-win32.whl", hash = "sha256:65594a2d557d4ee9f02834fcdf0a28daa8b3b9f6cb2cb93846025a36db47ef22"}, + {file = "ruff-0.15.4-py3-none-win_amd64.whl", hash = "sha256:04196ad44f0df220c2ece5b0e959c2f37c777375ec744397d21d15b50a75264f"}, + {file = "ruff-0.15.4-py3-none-win_arm64.whl", hash = "sha256:60d5177e8cfc70e51b9c5fad936c634872a74209f934c1e79107d11787ad5453"}, + {file = "ruff-0.15.4.tar.gz", hash = "sha256:3412195319e42d634470cc97aa9803d07e9d5c9223b99bcb1518f0c725f26ae1"}, ] [[package]] diff --git a/pyproject.toml b/pyproject.toml index 0ba9c24..1f2b145 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -14,7 +14,7 @@ requires-python = ">=3.14, <3.15" dependencies = [ "pyside6>=6.10.2", "packaging>=26.0", - "porringer>=0.2.1.dev54", + "porringer>=0.2.1.dev56", "qasync>=0.28.0", "velopack>=0.0.1444.dev49733", "typer>=0.24.1", @@ -25,18 +25,9 @@ homepage = "https://github.com/synodic/synodic-client" repository = "https://github.com/synodic/synodic-client" [dependency-groups] -build = [ - "pyinstaller>=6.19.0", -] -lint = [ - "ruff>=0.15.3", - "pyrefly>=0.54.0", -] -test = [ - "pytest>=9.0.2", - "pytest-cov>=7.0.0", - "pytest-mock>=3.15.1", -] +build = ["pyinstaller>=6.19.0"] +lint = ["ruff>=0.15.4", "pyrefly>=0.54.0"] +test = ["pytest>=9.0.2", "pytest-cov>=7.0.0", "pytest-mock>=3.15.1"] [project.scripts] synodic-c = "synodic_client.cli:app" diff --git a/synodic_client/application/screen/install.py b/synodic_client/application/screen/install.py index 6bca5f2..c69909b 100644 --- a/synodic_client/application/screen/install.py +++ b/synodic_client/application/screen/install.py @@ -4,8 +4,9 @@ previews and executing porringer setup actions, along with the standalone :class:`InstallPreviewWindow` used for URI-based manifest installs. -Execution runs on a background ``QThread`` with real-time output routed -to a unified :class:`~synodic_client.application.screen.log_panel.ExecutionLogPanel`. +Preview and install operations run as ``asyncio`` coroutines on the +qasync main-thread event loop, with callbacks delivering real-time +output to the :class:`~synodic_client.application.screen.log_panel.ExecutionLogPanel`. """ from __future__ import annotations @@ -15,6 +16,7 @@ import logging import shutil import tempfile +from collections.abc import Callable from dataclasses import dataclass, field from pathlib import Path from typing import Any @@ -23,8 +25,8 @@ from porringer.api import API from porringer.schema import ( - CancellationToken, DownloadParameters, + ProgressEvent, ProgressEventKind, SetupAction, SetupActionResult, @@ -34,7 +36,7 @@ SubActionProgress, SyncStrategy, ) -from PySide6.QtCore import Qt, QThread, QTimer, Signal +from PySide6.QtCore import Qt, QTimer, Signal from PySide6.QtWidgets import ( QFileDialog, QFrame, @@ -161,6 +163,8 @@ def __init__(self) -> None: self.plugin_installed: dict[str, bool] = {} self.prerelease_overrides: set[str] = set() self.action_states: list[ActionState] = [] + self._action_state_map: dict[tuple[object, ...], ActionState] = {} + self._action_state_map_len: int = 0 self.upgradable_keys: set[tuple[object, ...]] = set() self.checked_count: int = 0 self.completed_count: int = 0 @@ -168,6 +172,13 @@ def __init__(self) -> None: # -- Computed helpers -------------------------------------------------- + def _ensure_action_state_map(self) -> dict[tuple[object, ...], ActionState]: + """Return the action-key → state lookup, rebuilding if stale.""" + if len(self.action_states) != self._action_state_map_len: + self._action_state_map = {action_key(s.action): s for s in self.action_states} + self._action_state_map_len = len(self.action_states) + return self._action_state_map + @property def actionable_count(self) -> int: """Number of needed + upgradable actions.""" @@ -183,12 +194,8 @@ def install_enabled(self) -> bool: return self.actionable_count > 0 or any(s.action.kind is None for s in self.action_states) def action_state_for(self, act: SetupAction) -> ActionState | None: - """Look up :class:`ActionState` by content key.""" - key = action_key(act) - for s in self.action_states: - if action_key(s.action) == key: - return s - return None + """Look up :class:`ActionState` by content key (O(1) amortized).""" + return self._ensure_action_state_map().get(action_key(act)) def has_same_manifest(self, key: str) -> bool: """Return ``True`` if *key* matches the current manifest key.""" @@ -204,88 +211,81 @@ class InstallConfig: prerelease_packages: set[str] | None = field(default=None) -class InstallWorker(QThread): - """Background worker that executes setup actions via porringer. - - Uses the ``execute_stream`` async generator to consume progress events - and emits per-action progress signals for GUI updates. - """ - - finished = Signal(object) # SetupResults - progress = Signal(object, object) # (SetupAction, SetupActionResult) - action_started = Signal(object) # SetupAction - sub_progress = Signal(object, object) # (SetupAction, SubActionProgress) - error = Signal(str) - - def __init__( - self, - porringer: API, - manifest_path: Path, - cancellation_token: CancellationToken, - config: InstallConfig | None = None, - ) -> None: - """Initialize the worker. +@dataclass(frozen=True, slots=True) +class InstallCallbacks: + """Callbacks for :func:`run_install` progress reporting.""" - Args: - porringer: The porringer API instance. - manifest_path: Path to the manifest file to execute. - cancellation_token: Token for cooperative cancellation. - config: Optional execution parameters (directory, strategy, - prerelease overrides). - """ - super().__init__() - self._porringer = porringer - self._manifest_path = manifest_path - self._cancellation_token = cancellation_token - self._config = config or InstallConfig() + on_action_started: Callable[[SetupAction], None] | None = None + """Called when an action begins execution.""" - def run(self) -> None: - """Execute the setup actions on this thread's event loop.""" - try: - results = asyncio.run(self._execute()) - self.finished.emit(results) - except asyncio.CancelledError: - self.finished.emit(SetupResults(actions=[])) - except Exception as exc: - logger.exception('Install execution failed') - self.error.emit(str(exc)) - - async def _execute(self) -> SetupResults: - """Stream execution events and collect results.""" - params = SetupParameters( - paths=[self._manifest_path], - project_directory=self._config.project_directory, - strategy=self._config.strategy, - prerelease_packages=self._config.prerelease_packages, - ) - actions: list[SetupAction] = [] - collected: list[SetupActionResult] = [] - manifest_result: SetupResults | None = None + on_sub_progress: Callable[[SetupAction, SubActionProgress], None] | None = None + """Called for sub-action progress events.""" - async for event in self._porringer.sync.execute_stream(params): - if self._cancellation_token.is_cancelled: - raise asyncio.CancelledError + on_progress: Callable[[SetupAction, SetupActionResult], None] | None = None + """Called when a single action completes.""" - if event.kind == ProgressEventKind.MANIFEST_LOADED and event.manifest: - manifest_result = event.manifest - actions = list(event.manifest.actions) - if event.kind == ProgressEventKind.ACTION_STARTED and event.action: - self.action_started.emit(event.action) +async def run_install( + porringer: API, + manifest_path: Path, + config: InstallConfig | None = None, + callbacks: InstallCallbacks | None = None, +) -> SetupResults: + """Execute setup actions via porringer and stream progress. - if event.kind == ProgressEventKind.SUB_ACTION_PROGRESS and event.action and event.sub_action: - self.sub_progress.emit(event.action, event.sub_action) + Runs on the caller's event loop (typically the qasync main-thread + loop). Callbacks are invoked between ``await`` points so the GUI + stays responsive without cross-thread signalling. - if event.kind == ProgressEventKind.ACTION_COMPLETED and event.result: - collected.append(event.result) - self.progress.emit(event.action, event.result) + Args: + porringer: The porringer API instance. + manifest_path: Path to the manifest file to execute. + config: Optional execution parameters (directory, strategy, + prerelease overrides). + callbacks: Optional progress callbacks. - return SetupResults( - actions=actions, - results=collected, - manifest_path=manifest_result.manifest_path if manifest_result else None, - metadata=manifest_result.metadata if manifest_result else None, - ) + Returns: + Aggregated :class:`SetupResults`. + """ + cfg = config or InstallConfig() + cb = callbacks or InstallCallbacks() + params = SetupParameters( + paths=[manifest_path], + project_directory=cfg.project_directory, + strategy=cfg.strategy, + prerelease_packages=cfg.prerelease_packages, + ) + actions: list[SetupAction] = [] + collected: list[SetupActionResult] = [] + manifest_result: SetupResults | None = None + + async for event in porringer.sync.execute_stream(params): + if event.kind == ProgressEventKind.MANIFEST_LOADED and event.manifest: + manifest_result = event.manifest + actions = list(event.manifest.actions) + + if event.kind == ProgressEventKind.ACTION_STARTED and event.action and cb.on_action_started is not None: + cb.on_action_started(event.action) + + if ( + event.kind == ProgressEventKind.SUB_ACTION_PROGRESS + and event.action + and event.sub_action + and cb.on_sub_progress is not None + ): + cb.on_sub_progress(event.action, event.sub_action) + + if event.kind == ProgressEventKind.ACTION_COMPLETED and event.result: + collected.append(event.result) + if cb.on_progress is not None: + cb.on_progress(event.action, event.result) + + return SetupResults( + actions=actions, + results=collected, + manifest_path=manifest_result.manifest_path if manifest_result else None, + metadata=manifest_result.metadata if manifest_result else None, + ) # --------------------------------------------------------------------------- @@ -342,8 +342,7 @@ def __init__( self._config = config self._model = PreviewModel() - self._runner: QThread | None = None - self._cancellation_token: CancellationToken | None = None + self._task: asyncio.Task[None] | None = None # Debounce timer for per-row pre-release checkbox changes self._prerelease_debounce = QTimer(self) @@ -554,22 +553,14 @@ def load( overrides = self._model.prerelease_overrides or None - preview_worker = PreviewWorker( - self._porringer, - path_or_url, - project_directory=self._model.project_directory, - detect_updates=detect_updates, - prerelease_packages=overrides, + self._task = asyncio.create_task( + self._run_preview_task( + path_or_url, + project_directory=self._model.project_directory, + detect_updates=detect_updates, + prerelease_packages=overrides, + ), ) - preview_worker.manifest_parsed.connect(self._on_manifest_parsed) - preview_worker.plugins_queried.connect(self._on_plugins_queried) - preview_worker.preview_ready.connect(self._on_preview_resolved) - preview_worker.action_checked.connect(self._on_action_checked) - preview_worker.finished.connect(self._on_preview_finished) - preview_worker.error.connect(self._on_preview_error) - - self._runner = preview_worker - self._runner.start() def reset(self) -> None: """Clear all state and UI for a fresh preview. @@ -684,12 +675,67 @@ def _flush_prerelease_overrides(self) -> None: # --- Internal worker management --- def _stop_preview(self) -> None: - """Wait for any running worker to finish before starting a new one.""" + """Cancel any running preview or install task.""" self._prerelease_debounce.stop() - if self._runner is not None and self._runner.isRunning(): - self._runner.quit() - self._runner.wait() - self._runner = None + if self._task is not None and not self._task.done(): + self._task.cancel() + self._task = None + + async def _run_preview_task( + self, + path_or_url: str, + *, + project_directory: Path | None = None, + detect_updates: bool = True, + prerelease_packages: set[str] | None = None, + ) -> None: + """Run the preview coroutine and route completion/errors.""" + try: + await run_preview( + self._porringer, + path_or_url, + config=PreviewConfig( + project_directory=project_directory, + detect_updates=detect_updates, + prerelease_packages=prerelease_packages, + ), + callbacks=PreviewCallbacks( + on_manifest_parsed=self._on_manifest_parsed, + on_plugins_queried=self._on_plugins_queried, + on_preview_ready=self._on_preview_resolved, + on_action_checked=self._on_action_checked, + ), + ) + self._on_preview_finished() + except asyncio.CancelledError: + pass + except Exception as exc: + logger.exception('Preview failed') + self._on_preview_error(str(exc)) + + async def _run_install_task( + self, + manifest_path: Path, + config: InstallConfig, + ) -> None: + """Run the install coroutine and route completion/errors.""" + try: + results = await run_install( + self._porringer, + manifest_path, + config, + InstallCallbacks( + on_action_started=self._on_action_started, + on_sub_progress=self._on_sub_progress, + on_progress=self._on_action_progress, + ), + ) + self._on_install_finished(results) + except asyncio.CancelledError: + self._on_install_finished(SetupResults(actions=[])) + except Exception as exc: + logger.exception('Install execution failed') + self._on_install_error(str(exc)) # --- Preview callbacks (wired by load()) --- @@ -917,8 +963,6 @@ def _on_install(self) -> None: self._close_btn.setEnabled(False) m.completed_count = 0 - self._cancellation_token = CancellationToken() - self._status_label.setText('Installing\u2026') # Show the unified execution log panel @@ -929,25 +973,16 @@ def _on_install(self) -> None: # porringer actually upgrades the already-installed packages. strategy = SyncStrategy.LATEST if m.upgradable_keys else SyncStrategy.MINIMAL - # Worker thread - worker = InstallWorker( - self._porringer, - m.manifest_path, - self._cancellation_token, - InstallConfig( - project_directory=m.project_directory, - strategy=strategy, - prerelease_packages=m.prerelease_overrides or None, + self._task = asyncio.create_task( + self._run_install_task( + m.manifest_path, + InstallConfig( + project_directory=m.project_directory, + strategy=strategy, + prerelease_packages=m.prerelease_overrides or None, + ), ), ) - worker.action_started.connect(self._on_action_started) - worker.sub_progress.connect(self._on_sub_progress) - worker.progress.connect(self._on_action_progress) - worker.finished.connect(self._on_install_finished) - worker.error.connect(self._on_install_error) - - self._runner = worker - self._runner.start() def _on_action_started(self, action: SetupAction) -> None: """Handle an action starting execution — update card badge and add a log section.""" @@ -991,8 +1026,8 @@ def _on_action_progress(self, action: SetupAction, result: SetupActionResult) -> def _on_cancel(self) -> None: """Handle cancel request.""" - if self._cancellation_token: - self._cancellation_token.cancel() + if self._task is not None and not self._task.done(): + self._task.cancel() def _on_install_finished(self, results: SetupResults) -> None: """Handle install completion.""" @@ -1167,148 +1202,172 @@ def _on_metadata_ready(self, preview: object) -> None: self.setWindowTitle(f'Install Preview \u2014 {preview.metadata.name}') -class PreviewWorker(QThread): - """Background worker that downloads a manifest and performs a dry-run. +@dataclass(frozen=True, slots=True) +class PreviewCallbacks: + """Callbacks for :func:`run_preview` progress reporting.""" - Combines two stages into a single background pipeline: + on_manifest_parsed: Callable[[SetupResults, str, str], None] | None = None + """``(SetupResults, manifest_path, temp_dir)`` — after JSON load.""" - 1. Download the manifest (if remote). - 2. Run ``execute_stream`` with ``dry_run=True`` to stream events. + on_plugins_queried: Callable[[dict[str, bool]], None] | None = None + """``(dict[str, bool])`` — plugin → installed mapping.""" - The worker emits signals in phases: + on_preview_ready: Callable[[SetupResults, str, str], None] | None = None + """``(SetupResults, manifest_path, temp_dir)`` — CLI commands resolved.""" - * ``manifest_parsed`` — emitted as soon as the JSON is loaded, - before plugin discovery. GUI clients can populate cards - immediately from this. - * ``plugins_queried`` — emitted after porringer discovers plugins, - carrying plugin name → installed mapping. - * ``preview_ready`` — emitted once CLI commands are resolved. - * ``action_checked`` — emitted per-action as dry-run results - stream in (may arrive out of order due to parallel checks). - """ + on_action_checked: Callable[[int, SetupActionResult], None] | None = None + """``(row_index, SetupActionResult)`` — per-action dry-run result.""" - manifest_parsed = Signal(object, str, str) # (SetupResults, manifest_path, temp_dir_path) — fast preview - preview_ready = Signal(object, str, str) # (SetupResults, manifest_path, temp_dir_path) — fully resolved - action_checked = Signal(int, object) # (row_index, SetupActionResult) - plugins_queried = Signal(object) # dict[str, bool] — plugin name → installed - finished = Signal() - error = Signal(str) - def __init__( - self, - porringer: API, - url: str, - *, - project_directory: Path | None = None, - detect_updates: bool = True, - prerelease_packages: set[str] | None = None, - ) -> None: - """Initialize the preview worker. +@dataclass(frozen=True, slots=True) +class PreviewConfig: + """Optional execution parameters for :func:`run_preview`.""" - Args: - porringer: The porringer API instance. - url: Manifest URL or local path. - project_directory: Working directory for project sync actions. - detect_updates: Query package indices for newer versions. - prerelease_packages: Package names whose ``include_prereleases`` - flag should be forced to ``True``, overriding the manifest - default. - """ - super().__init__() - self._porringer = porringer - self._url = url - self._project_directory = project_directory - self._detect_updates = detect_updates - self._prerelease_packages = prerelease_packages - - def run(self) -> None: - """Download the manifest and perform a dry-run to check status.""" - logger.info('PreviewWorker starting for: %s', self._url) - temp_dir = None - try: - local_path = resolve_local_path(self._url) + project_directory: Path | None = None + detect_updates: bool = True + prerelease_packages: set[str] | None = None - if local_path is not None: - if not local_path.exists(): - self.error.emit(f'Manifest not found:\n{local_path}') - return - manifest_path = local_path - else: - temp_dir = tempfile.mkdtemp(prefix='synodic_install_') - dest = Path(temp_dir) / 'porringer.json' - params = DownloadParameters(url=self._url, destination=dest, timeout=3) - result = API.download(params) +@dataclass(slots=True) +class _DispatchState: + """Mutable accumulator for :func:`_dispatch_preview_event`.""" - if not result.success: - _safe_rmtree(temp_dir) - self.error.emit(f'Failed to download manifest:\n{result.message}') - return + action_index: dict[int, int] = field(default_factory=dict) + got_parsed: bool = False - manifest_path = dest - # Dry-run: parses manifest, resolves actions, and checks status - asyncio.run(self._dry_run(manifest_path, temp_dir or '')) +async def _resolve_manifest_path(url: str) -> tuple[Path, str | None]: + """Resolve *url* to a local manifest path, downloading if remote. - self.finished.emit() + Returns: + ``(manifest_path, temp_dir)`` — *temp_dir* is ``None`` for local + manifests and a temporary directory string for downloads. - except Exception as exc: - if temp_dir: - _safe_rmtree(temp_dir) - logger.exception('Preview failed') - self.error.emit(str(exc)) + Raises: + FileNotFoundError: If a local path does not exist. + RuntimeError: If the download fails. + """ + local_path = resolve_local_path(url) - async def _dry_run(self, manifest_path: Path, temp_dir: str) -> None: - """Stream dry-run events, emitting signals as each phase completes. + if local_path is not None: + if not local_path.exists(): + msg = f'Manifest not found:\n{local_path}' + raise FileNotFoundError(msg) + return local_path, None - The new event pipeline is: + temp_dir = tempfile.mkdtemp(prefix='synodic_install_') + dest = Path(temp_dir) / 'porringer.json' - 1. ``MANIFEST_PARSED`` → emit ``manifest_parsed`` (fast, before discovery) - 2. ``PLUGINS_DISCOVERED`` → emit ``plugins_queried`` - 3. ``MANIFEST_LOADED`` → emit ``preview_ready`` (CLI commands populated) - 4. ``ACTION_COMPLETED`` → emit ``action_checked`` (per-action status) + params = DownloadParameters(url=url, destination=dest, timeout=3) + loop = asyncio.get_running_loop() + result = await loop.run_in_executor(None, API.download, params) - Falls back to the previous behaviour when the porringer version - does not emit the newer event kinds (``MANIFEST_PARSED`` / - ``PLUGINS_DISCOVERED``). - """ - params = SetupParameters( + if not result.success: + _safe_rmtree(temp_dir) + msg = f'Failed to download manifest:\n{result.message}' + raise RuntimeError(msg) + + return dest, temp_dir + + +def _dispatch_preview_event( + event: ProgressEvent, + manifest_path: str, + temp_dir_str: str, + state: _DispatchState, + cb: PreviewCallbacks, +) -> None: + """Route a single preview stream event to the appropriate callback. + + Mutates *state* in-place with updated ``action_index`` / ``got_parsed``. + """ + if event.kind == ProgressEventKind.MANIFEST_PARSED and event.manifest: + state.action_index = {id(a): i for i, a in enumerate(event.manifest.actions)} + if cb.on_manifest_parsed is not None: + cb.on_manifest_parsed(event.manifest, manifest_path, temp_dir_str) + state.got_parsed = True + return + + if event.kind == ProgressEventKind.PLUGINS_DISCOVERED and cb.on_plugins_queried is not None: + if event.plugin_availability is not None: + cb.on_plugins_queried(event.plugin_availability) + elif event.plugin_names is not None: + cb.on_plugins_queried({name: True for name in event.plugin_names}) + return + + if event.kind == ProgressEventKind.MANIFEST_LOADED and event.manifest: + if not state.got_parsed: + state.action_index = {id(a): i for i, a in enumerate(event.manifest.actions)} + if cb.on_preview_ready is not None: + cb.on_preview_ready(event.manifest, manifest_path, temp_dir_str) + return + + if event.kind == ProgressEventKind.ACTION_COMPLETED and event.result and event.action: + row = state.action_index.get(id(event.action)) + if row is not None and cb.on_action_checked is not None: + cb.on_action_checked(row, event.result) + + +async def run_preview( + porringer: API, + url: str, + *, + config: PreviewConfig | None = None, + callbacks: PreviewCallbacks | None = None, +) -> None: + """Download a manifest and perform a dry-run preview. + + Runs on the caller's event loop (typically the qasync main-thread + loop). Callbacks fire between ``await`` points so the GUI remains + responsive without cross-thread signalling. + + Combines two stages: + + 1. Download the manifest (if remote) — runs in a thread-pool executor. + 2. Run ``execute_stream`` with ``dry_run=True`` to stream events. + + Args: + porringer: The porringer API instance. + url: Manifest URL or local path. + config: Optional preview configuration. + callbacks: Optional preview callbacks. + """ + logger.info('run_preview starting for: %s', url) + temp_dir: str | None = None + cb = callbacks or PreviewCallbacks() + cfg = config or PreviewConfig() + try: + manifest_path, temp_dir = await _resolve_manifest_path(url) + + # Dry-run: parses manifest, resolves actions, and checks status + setup_params = SetupParameters( paths=[manifest_path], dry_run=True, - project_directory=self._project_directory, - detect_updates=self._detect_updates, - prerelease_packages=self._prerelease_packages, + project_directory=cfg.project_directory, + detect_updates=cfg.detect_updates, + prerelease_packages=cfg.prerelease_packages, ) - action_index: dict[int, int] = {} - got_parsed = False - - async for event in self._porringer.sync.execute_stream(params): - if event.kind == ProgressEventKind.MANIFEST_PARSED and event.manifest: - # Fast path: cards can be shown immediately - action_index = {id(a): i for i, a in enumerate(event.manifest.actions)} - self.manifest_parsed.emit(event.manifest, str(manifest_path), temp_dir) - got_parsed = True - - elif event.kind == ProgressEventKind.PLUGINS_DISCOVERED: - # Use the plugin availability from porringer's own discovery - # rather than making a separate plugin.list() call. - if event.plugin_availability is not None: - self.plugins_queried.emit(event.plugin_availability) - elif event.plugin_names is not None: - # Fallback: only names available, assume all installed - self.plugins_queried.emit({name: True for name in event.plugin_names}) - - elif event.kind == ProgressEventKind.MANIFEST_LOADED and event.manifest: - # Fully-resolved preview with CLI commands - if not got_parsed: - # Fallback: older porringer without MANIFEST_PARSED - action_index = {id(a): i for i, a in enumerate(event.manifest.actions)} - self.preview_ready.emit(event.manifest, str(manifest_path), temp_dir) - - elif event.kind == ProgressEventKind.ACTION_COMPLETED and event.result and event.action: - row = action_index.get(id(event.action)) - if row is not None: - self.action_checked.emit(row, event.result) + state = _DispatchState() + temp_dir_str = temp_dir or '' + manifest_path_str = str(manifest_path) + + async for event in porringer.sync.execute_stream(setup_params): + _dispatch_preview_event( + event, + manifest_path_str, + temp_dir_str, + state, + cb, + ) + + except asyncio.CancelledError: + if temp_dir: + _safe_rmtree(temp_dir) + raise + except Exception: + if temp_dir: + _safe_rmtree(temp_dir) + raise def resolve_local_path(manifest_ref: str) -> Path | None: diff --git a/synodic_client/application/screen/screen.py b/synodic_client/application/screen/screen.py index 8510248..c6734fa 100644 --- a/synodic_client/application/screen/screen.py +++ b/synodic_client/application/screen/screen.py @@ -10,11 +10,9 @@ from porringer.schema import DirectoryValidationResult, ManifestDirectory, PluginInfo from porringer.schema.plugin import PluginKind from PySide6.QtCore import Qt, Signal -from PySide6.QtGui import QResizeEvent, QStandardItem +from PySide6.QtGui import QResizeEvent from PySide6.QtWidgets import ( - QComboBox, QFileDialog, - QGridLayout, QHBoxLayout, QHeaderView, QLabel, @@ -22,6 +20,7 @@ QPushButton, QScrollArea, QSizePolicy, + QStackedWidget, QTableWidget, QTableWidgetItem, QTabWidget, @@ -32,11 +31,11 @@ from synodic_client.application.icon import app_icon from synodic_client.application.screen import plugin_kind_group_label from synodic_client.application.screen.card import CHEVRON_DOWN, CHEVRON_RIGHT, ClickableHeader -from synodic_client.application.screen.install import SetupPreviewWidget +from synodic_client.application.screen.install import PreviewPhase, SetupPreviewWidget +from synodic_client.application.screen.sidebar import ManifestSidebar from synodic_client.application.screen.spinner import SpinnerWidget from synodic_client.application.screen.update_banner import UpdateBanner from synodic_client.application.theme import ( - CARD_SPACING, COMPACT_MARGINS, LOG_CHEVRON_STYLE, LOG_SECTION_TITLE_STYLE, @@ -358,7 +357,7 @@ def refresh(self) -> None: """Schedule an asynchronous rebuild of the plugin sections.""" if self._refresh_in_progress: return - asyncio.ensure_future(self._async_refresh()) + asyncio.create_task(self._async_refresh()) async def _async_refresh(self) -> None: """Rebuild the plugin sections from porringer data, grouped by kind.""" @@ -497,10 +496,10 @@ def _on_auto_update_toggled(self, plugin_name: str, enabled: bool) -> None: class ProjectsView(QWidget): """Widget for managing project directories and previewing their manifests. - Combines a cached-directory selector (editable ``QComboBox`` with - Browse) and a :class:`SetupPreviewWidget` for dry-run preview and - install execution. Preview loading is fully delegated to the - embedded widget via :meth:`SetupPreviewWidget.load`. + Displays a vertical sidebar of cached project directories on the + left with a stacked widget on the right showing one + :class:`SetupPreviewWidget` per manifest. All manifests are loaded + in parallel on first refresh; switching between them is instant. """ def __init__(self, porringer: API, config: ResolvedConfig, parent: QWidget | None = None) -> None: @@ -515,47 +514,40 @@ def __init__(self, porringer: API, config: ResolvedConfig, parent: QWidget | Non self._porringer = porringer self._config = config self._refresh_in_progress = False + self._pending_select: Path | None = None + self._widgets: dict[Path, SetupPreviewWidget] = {} self._init_ui() def _init_ui(self) -> None: - """Initialize the UI components with a grid layout. - - The loading spinner is a floating overlay parented to ``self`` - but **not** part of the grid, so showing/hiding it never - changes the geometry of the rows beneath. - """ - grid = QGridLayout(self) - grid.setContentsMargins(*COMPACT_MARGINS) - grid.setVerticalSpacing(CARD_SPACING) - - # Row 0 — Project directory selector - self._combo = QComboBox() - self._combo.setEditable(True) - self._combo.setToolTip('Select a cached project directory or enter a new path') - self._combo.setSizeAdjustPolicy(QComboBox.SizeAdjustPolicy.AdjustToMinimumContentsLengthWithIcon) - self._combo.setMinimumContentsLength(40) - self._combo.currentIndexChanged.connect(self._on_selection_changed) - grid.addWidget(self._combo, 0, 0) - grid.setColumnStretch(0, 1) - - self._browse_btn = QPushButton('Browse…') - self._browse_btn.clicked.connect(self._on_browse) - grid.addWidget(self._browse_btn, 0, 1) - - self._remove_btn = QPushButton('Remove') - self._remove_btn.setToolTip('Remove the selected directory from the cache') - self._remove_btn.clicked.connect(self._on_remove) - self._remove_btn.setEnabled(False) - grid.addWidget(self._remove_btn, 0, 2) - - # Row 1 — Shared preview widget (takes majority of space) - self._preview = SetupPreviewWidget(self._porringer, self, show_close=False, config=self._config) - self._preview.install_finished.connect(self._on_install_finished) - grid.addWidget(self._preview, 1, 0, 1, 3) - grid.setRowStretch(1, 1) - - # Floating overlay spinner — not in the grid layout. - # Positioned in resizeEvent to cover the full widget area. + """Build the sidebar + stacked widget layout.""" + outer = QHBoxLayout(self) + outer.setContentsMargins(0, 0, 0, 0) + outer.setSpacing(0) + + # Left — sidebar + self._sidebar = ManifestSidebar() + self._sidebar.add_requested.connect(self._on_add) + self._sidebar.remove_requested.connect(self._on_remove) + self._sidebar.selection_changed.connect(self._on_selection_changed) + outer.addWidget(self._sidebar) + + # Right — stacked previews + empty placeholder + right = QVBoxLayout() + right.setContentsMargins(*COMPACT_MARGINS) + right.setSpacing(0) + + self._stack = QStackedWidget() + right.addWidget(self._stack, stretch=1) + + # Empty placeholder shown when there are no manifests + self._empty_placeholder = QLabel('No projects. Click + Add Project to get started.') + self._empty_placeholder.setAlignment(Qt.AlignmentFlag.AlignCenter) + self._empty_placeholder.setStyleSheet('color: grey; font-size: 13px;') + self._stack.addWidget(self._empty_placeholder) + + outer.addLayout(right, stretch=1) + + # Floating overlay spinner — positioned in resizeEvent self._loading_spinner = SpinnerWidget('Loading projects\u2026', parent=self) self._loading_spinner.setSizePolicy(QSizePolicy.Policy.Expanding, QSizePolicy.Policy.Expanding) self._loading_spinner.raise_() @@ -572,162 +564,137 @@ def refresh(self) -> None: """Schedule an asynchronous refresh of the cached directories.""" if self._refresh_in_progress: return - asyncio.ensure_future(self._async_refresh()) + asyncio.create_task(self._async_refresh()) async def _async_refresh(self) -> None: - """Refresh the cached directories combo box from porringer cache.""" + """Refresh the sidebar and stacked widgets from the porringer cache.""" self._refresh_in_progress = True self._loading_spinner.start() - self._combo.setEnabled(False) - self._browse_btn.setEnabled(False) - self._remove_btn.setEnabled(False) + self._sidebar.set_enabled(False) try: + previous = self._pending_select or self._sidebar.selected_path + self._pending_select = None + loop = asyncio.get_running_loop() results: list[DirectoryValidationResult] = await loop.run_in_executor( None, lambda: self._porringer.cache.validate_directories(check_manifest=True), ) - self._combo.blockSignals(True) - current_text = self._combo.currentText() - self._combo.clear() - + directories: list[tuple[Path, str, bool]] = [] + current_paths: set[Path] = set() for result in results: - directory = result.directory - display = str(directory.path) - tooltip = directory.name or '' - - idx = self._combo.count() - self._combo.addItem(display) - self._combo.setItemData(idx, tooltip, Qt.ItemDataRole.ToolTipRole) - self._combo.setItemData(idx, str(directory.path), Qt.ItemDataRole.UserRole) - - if not result.exists: - # Grey out entries whose path no longer exists on disk - self._grey_out_item(idx, tooltip, 'Path not found') - elif result.has_manifest is False: - # Dim entries where the path exists but no manifest is found - self._grey_out_item(idx, tooltip, 'No manifest found') - - # Restore previous selection if it still exists - idx = self._combo.findText(current_text) - if idx >= 0: - self._combo.setCurrentIndex(idx) - elif self._combo.count() > 0: - self._combo.setCurrentIndex(0) - - self._combo.blockSignals(False) - self._update_remove_btn() - - # Trigger preview for the current selection - if self._combo.currentText(): - path_text = self._combo.currentText().strip() - selected = Path(path_text) - self._preview.load( - path_text, - project_directory=selected if selected.is_dir() else selected.parent, - detect_updates=self._config.detect_updates, - ) + d = result.directory + valid = result.exists and result.has_manifest is not False + path = Path(d.path) + directories.append((path, d.name or '', valid)) + current_paths.add(path) + + # Remove widgets for directories no longer in cache + for path in list(self._widgets): + if path not in current_paths: + widget = self._widgets.pop(path) + self._stack.removeWidget(widget) + widget.reset() + widget.deleteLater() + + # Create new widgets for new directories + for path, _name, valid in directories: + if path not in self._widgets and valid: + widget = SetupPreviewWidget( + self._porringer, + self, + show_close=False, + config=self._config, + ) + widget.install_finished.connect(self._on_install_finished) + widget.phase_changed.connect( + lambda phase, p=path: self._on_widget_phase_changed(p, phase), + ) + self._widgets[path] = widget + self._stack.addWidget(widget) + + # Rebuild sidebar + self._sidebar.set_directories(directories) + self._sidebar.select(previous) + + # Load all stacked widgets in parallel + for path, _name, valid in directories: + widget = self._widgets.get(path) + if widget is not None and valid: + widget.load( + str(path), + project_directory=path if path.is_dir() else path.parent, + detect_updates=self._config.detect_updates, + ) + except Exception: logger.exception('Failed to refresh projects') finally: self._loading_spinner.stop() - self._loading_spinner.lower() # put behind content after loading - self._combo.setEnabled(True) - self._browse_btn.setEnabled(True) - self._update_remove_btn() + self._loading_spinner.lower() + self._sidebar.set_enabled(True) self._refresh_in_progress = False - def _grey_out_item(self, idx: int, tooltip: str, reason: str) -> None: - """Grey out a combo box item and append a reason to its tooltip.""" - model = self._combo.model() - item = model.item(idx) if hasattr(model, 'item') else None - if isinstance(item, QStandardItem): - item.setForeground(self.palette().placeholderText()) - item.setToolTip(f'{tooltip} \u2014 {reason}' if tooltip else reason) - # --- Event handlers --- - def _on_selection_changed(self, _index: int) -> None: - """Handle combo box selection changes — delegate to the preview widget.""" - self._update_remove_btn() - path_text = self._combo.currentText().strip() - if path_text: - selected = Path(path_text) - self._preview.load( - str(selected), - project_directory=selected if selected.is_dir() else selected.parent, - detect_updates=self._config.detect_updates, - ) + def _on_selection_changed(self, path: Path) -> None: + """Handle sidebar selection — switch the stacked widget.""" + widget = self._widgets.get(path) + if widget is not None: + self._stack.setCurrentWidget(widget) + else: + self._stack.setCurrentWidget(self._empty_placeholder) + + def _on_widget_phase_changed(self, path: Path, phase: PreviewPhase) -> None: + """Update the sidebar item's phase indicator.""" + item = self._sidebar.get_item(path) + if item is not None: + item.set_phase(phase) - def _on_browse(self) -> None: - """Open a file picker filtered to recognised manifest filenames.""" + def _on_add(self) -> None: + """Open a file picker and immediately cache the chosen directory.""" filenames = self._porringer.sync.manifest_filenames() filter_str = 'Manifests (' + ' '.join(filenames) + ');;All Files (*)' chosen, _ = QFileDialog.getOpenFileName( self, 'Select Manifest File', - self._combo.currentText() or '', + '', filter_str, ) - if chosen: - self._combo.setEditText(chosen) - selected = Path(chosen) - self._preview.load( - chosen, - project_directory=selected if selected.is_dir() else selected.parent, - detect_updates=self._config.detect_updates, - ) - - def _on_remove(self) -> None: - """Remove the currently selected directory from the cache.""" - idx = self._combo.currentIndex() - if idx < 0: + if not chosen: return - path_str = self._combo.itemData(idx, Qt.ItemDataRole.UserRole) - if path_str: - self._porringer.cache.remove_directory(Path(path_str)) + selected = Path(chosen) + directory = selected if selected.is_dir() else selected.parent + + try: + self._porringer.cache.add_directory(directory) + logger.info('Cached new project directory: %s', directory) + except ValueError: + logger.debug('Directory already cached: %s', directory) + self._pending_select = directory self.refresh() - def _on_install_finished(self, _results: object) -> None: - """Register a new path in the cache after successful install. - - The directory is added to the porringer cache and the combo box - is updated *without* reloading the preview, so the execution - log remains visible. Combo signals are blocked during the - update to prevent ``_on_selection_changed`` from firing a - redundant :meth:`SetupPreviewWidget.load` (which would be - skipped anyway due to the DONE-phase guard, but blocking - is cleaner). - """ - current_text = self._combo.currentText().strip() - if not current_text: - return + def _on_remove(self, path: Path) -> None: + """Remove a directory from the porringer cache.""" + self._porringer.cache.remove_directory(path) + logger.info('Removed project directory from cache: %s', path) - idx = self._combo.findText(current_text) - item_data = self._combo.itemData(idx, Qt.ItemDataRole.UserRole) if idx >= 0 else None - if item_data is None: - try: - self._porringer.cache.add_directory(Path(current_text)) - logger.info('Registered new project directory: %s', current_text) - self._combo.blockSignals(True) - new_idx = self._combo.count() - self._combo.addItem(current_text) - self._combo.setItemData(new_idx, current_text, Qt.ItemDataRole.UserRole) - self._combo.setCurrentIndex(new_idx) - self._combo.blockSignals(False) - self._update_remove_btn() - except ValueError: - logger.debug('Directory already cached or invalid: %s', current_text) - - def _update_remove_btn(self) -> None: - """Enable the Remove button only for cached (non-freeform) entries.""" - idx = self._combo.currentIndex() - has_data = idx >= 0 and self._combo.itemData(idx, Qt.ItemDataRole.UserRole) is not None - self._remove_btn.setEnabled(has_data) + # Tear down the widget immediately + widget = self._widgets.pop(path, None) + if widget is not None: + self._stack.removeWidget(widget) + widget.reset() + widget.deleteLater() + + self.refresh() + + def _on_install_finished(self, _results: object) -> None: + """Refresh after a successful install.""" + self.refresh() class MainWindow(QMainWindow): diff --git a/synodic_client/application/screen/sidebar.py b/synodic_client/application/screen/sidebar.py new file mode 100644 index 0000000..91fd012 --- /dev/null +++ b/synodic_client/application/screen/sidebar.py @@ -0,0 +1,330 @@ +"""Manifest sidebar — a vertical panel for selecting cached project directories. + +:class:`ManifestItem` renders a single directory as a row with an inline +close button visible on hover. :class:`ManifestSidebar` manages a +scrollable column of items plus an *Add* button, emitting signals for +selection, removal, and addition. +""" + +from __future__ import annotations + +import logging +from pathlib import Path + +from PySide6.QtCore import Qt, Signal +from PySide6.QtWidgets import ( + QFrame, + QHBoxLayout, + QLabel, + QPushButton, + QScrollArea, + QSizePolicy, + QVBoxLayout, + QWidget, +) + +from synodic_client.application.screen.install import PreviewPhase +from synodic_client.application.theme import ( + SIDEBAR_ADD_STYLE, + SIDEBAR_CLOSE_STYLE, + SIDEBAR_HEADER_STYLE, + SIDEBAR_ITEM_DIMMED_STYLE, + SIDEBAR_ITEM_HEIGHT, + SIDEBAR_ITEM_SELECTED_STYLE, + SIDEBAR_ITEM_STYLE, + SIDEBAR_LABEL_DIMMED_STYLE, + SIDEBAR_LABEL_STYLE, + SIDEBAR_PHASE_DONE_STYLE, + SIDEBAR_PHASE_ERROR_STYLE, + SIDEBAR_PHASE_INSTALLING_STYLE, + SIDEBAR_PHASE_LOADING_STYLE, + SIDEBAR_PHASE_READY_STYLE, + SIDEBAR_SPACING, + SIDEBAR_STYLE, + SIDEBAR_WIDTH, +) + +logger = logging.getLogger(__name__) + +_PHASE_LABELS: dict[PreviewPhase, tuple[str, str]] = { + PreviewPhase.LOADING: ('Loading…', SIDEBAR_PHASE_LOADING_STYLE), + PreviewPhase.PREVIEWING: ('Checking…', SIDEBAR_PHASE_LOADING_STYLE), + PreviewPhase.READY: ('Ready', SIDEBAR_PHASE_READY_STYLE), + PreviewPhase.INSTALLING: ('Installing…', SIDEBAR_PHASE_INSTALLING_STYLE), + PreviewPhase.DONE: ('Done', SIDEBAR_PHASE_DONE_STYLE), + PreviewPhase.ERROR: ('Error', SIDEBAR_PHASE_ERROR_STYLE), +} + + +class ManifestItem(QFrame): + """A sidebar row representing a single cached project directory. + + Emits :attr:`clicked` when the item body is pressed and + :attr:`remove_requested` when the close button is pressed. + """ + + clicked = Signal(Path) + """Emitted with the directory path when the item is clicked.""" + + remove_requested = Signal(Path) + """Emitted with the directory path when the × button is clicked.""" + + def __init__( + self, + path: Path, + name: str = '', + *, + valid: bool = True, + parent: QWidget | None = None, + ) -> None: + """Initialise the item. + + Args: + path: Absolute path to the project directory. + name: Optional human-readable name (falls back to last path component). + valid: When ``False`` the item renders dimmed. + parent: Optional parent widget. + """ + super().__init__(parent) + self.setObjectName('sidebarItem') + self._path = path + self._name = name + self._valid = valid + self._selected = False + + self.setFixedHeight(SIDEBAR_ITEM_HEIGHT) + self.setCursor(Qt.CursorShape.PointingHandCursor) + self._apply_style() + + layout = QHBoxLayout(self) + layout.setContentsMargins(0, 0, 0, 0) + layout.setSpacing(4) + + display = name or path.name or str(path) + self._label = QLabel(display) + self._label.setStyleSheet(SIDEBAR_LABEL_STYLE if valid else SIDEBAR_LABEL_DIMMED_STYLE) + self._label.setToolTip(str(path)) + layout.addWidget(self._label, stretch=1) + + # Phase indicator (updated externally) + self._phase_label = QLabel() + self._phase_label.hide() + layout.addWidget(self._phase_label) + + self._close_btn = QPushButton('\u00d7') # × + self._close_btn.setFixedSize(18, 18) + self._close_btn.setStyleSheet(SIDEBAR_CLOSE_STYLE) + self._close_btn.setToolTip('Remove from cache') + self._close_btn.setCursor(Qt.CursorShape.PointingHandCursor) + self._close_btn.clicked.connect(self._on_close) + layout.addWidget(self._close_btn) + + self.setToolTip(str(path)) + + # --- Properties ------------------------------------------------------- + + @property + def path(self) -> Path: + """Return the project directory path.""" + return self._path + + @property + def selected(self) -> bool: + """Return whether this item is currently selected.""" + return self._selected + + @selected.setter + def selected(self, value: bool) -> None: + """Set the selected state and update styling.""" + self._selected = value + self._apply_style() + + # --- Public API ------------------------------------------------------- + + def set_phase(self, phase: PreviewPhase) -> None: + """Update the phase indicator label.""" + info = _PHASE_LABELS.get(phase) + if info is not None: + text, style = info + self._phase_label.setText(text) + self._phase_label.setStyleSheet(style) + self._phase_label.show() + else: + self._phase_label.hide() + + # --- Styling ----------------------------------------------------------- + + def _apply_style(self) -> None: + """Apply the appropriate stylesheet based on state.""" + if self._selected: + self.setStyleSheet(SIDEBAR_ITEM_SELECTED_STYLE) + elif not self._valid: + self.setStyleSheet(SIDEBAR_ITEM_DIMMED_STYLE) + else: + self.setStyleSheet(SIDEBAR_ITEM_STYLE) + + # --- Events ------------------------------------------------------------ + + def mousePressEvent(self, _event: object) -> None: + """Emit :attr:`clicked` on mouse press.""" + self.clicked.emit(self._path) + + def _on_close(self) -> None: + """Emit :attr:`remove_requested` when the × button is clicked.""" + self.remove_requested.emit(self._path) + + +class ManifestSidebar(QWidget): + """Vertical sidebar for managing cached project directories. + + Displays a scrollable column of :class:`ManifestItem` widgets with + a trailing *Add* button. Emits signals for user interactions. + """ + + selection_changed = Signal(Path) + """Emitted with the directory path when an item is clicked.""" + + remove_requested = Signal(Path) + """Emitted with the directory path when an item's close button is clicked.""" + + add_requested = Signal() + """Emitted when the Add (+) button is clicked.""" + + def __init__(self, parent: QWidget | None = None) -> None: + """Initialise the sidebar.""" + super().__init__(parent) + self._items: list[ManifestItem] = [] + self._selected_path: Path | None = None + + self.setFixedWidth(SIDEBAR_WIDTH) + + frame = QFrame(self) + frame.setObjectName('sidebar') + frame.setStyleSheet(SIDEBAR_STYLE) + + outer = QVBoxLayout(self) + outer.setContentsMargins(0, 0, 0, 0) + outer.setSpacing(0) + outer.addWidget(frame) + + frame_layout = QVBoxLayout(frame) + frame_layout.setContentsMargins(8, 8, 8, 8) + frame_layout.setSpacing(SIDEBAR_SPACING) + + # Header + header = QLabel('PROJECTS') + header.setStyleSheet(SIDEBAR_HEADER_STYLE) + frame_layout.addWidget(header) + + # Scroll area for the items + self._scroll = QScrollArea() + self._scroll.setWidgetResizable(True) + self._scroll.setHorizontalScrollBarPolicy(Qt.ScrollBarPolicy.ScrollBarAlwaysOff) + self._scroll.setVerticalScrollBarPolicy(Qt.ScrollBarPolicy.ScrollBarAsNeeded) + self._scroll.setFrameShape(QFrame.Shape.NoFrame) + self._scroll.setSizePolicy(QSizePolicy.Policy.Expanding, QSizePolicy.Policy.Expanding) + + self._container = QWidget() + self._column = QVBoxLayout(self._container) + self._column.setContentsMargins(0, 0, 0, 0) + self._column.setSpacing(SIDEBAR_SPACING) + self._column.setAlignment(Qt.AlignmentFlag.AlignTop) + self._column.addStretch() + + self._scroll.setWidget(self._container) + frame_layout.addWidget(self._scroll, stretch=1) + + # Add (+) button at the bottom + self._add_btn = QPushButton('+ Add Project') + self._add_btn.setStyleSheet(SIDEBAR_ADD_STYLE) + self._add_btn.setToolTip('Add a manifest') + self._add_btn.setCursor(Qt.CursorShape.PointingHandCursor) + self._add_btn.clicked.connect(self.add_requested.emit) + frame_layout.addWidget(self._add_btn) + + # --- Public API -------------------------------------------------------- + + @property + def selected_path(self) -> Path | None: + """Return the currently selected directory path.""" + return self._selected_path + + def set_directories( + self, + directories: list[tuple[Path, str, bool]], + ) -> None: + """Rebuild all items from a list of ``(path, name, valid)`` tuples. + + Any previous selection is **not** restored — callers should call + :meth:`select` afterwards if desired. + """ + # Remove existing items + for item in self._items: + self._column.removeWidget(item) + item.deleteLater() + self._items.clear() + self._selected_path = None + + # Insert new items before the trailing stretch + for insert_idx, (path, name, valid) in enumerate(directories): + item = ManifestItem(path, name, valid=valid, parent=self._container) + item.clicked.connect(self._on_item_clicked) + item.remove_requested.connect(self._on_item_remove) + self._column.insertWidget(insert_idx, item) + self._items.append(item) + + def select(self, path: Path | None) -> None: + """Programmatically select an item by path. + + If *path* is ``None`` or not found, the first item (if any) is + selected instead. + """ + # Build a fast lookup: exact path → item, resolved path → item + exact: dict[Path, ManifestItem] = {i.path: i for i in self._items} + + target: Path | None = None + if path is not None: + if path in exact: + target = path + else: + resolved = path.resolve() + for item in self._items: + if item.path.resolve() == resolved: + target = item.path + break + + if target is None and self._items: + target = self._items[0].path + + self._selected_path = target + for item in self._items: + item.selected = item.path == target + + if target is not None: + self.selection_changed.emit(target) + + def set_enabled(self, enabled: bool) -> None: + """Enable or disable the add button and all items.""" + self._add_btn.setEnabled(enabled) + for item in self._items: + item.setEnabled(enabled) + + def get_item(self, path: Path) -> ManifestItem | None: + """Return the :class:`ManifestItem` for *path*, or ``None``.""" + for item in self._items: + if item.path == path: + return item + return None + + # --- Internal slots ---------------------------------------------------- + + def _on_item_clicked(self, path: Path) -> None: + """Handle an item click — update selection and emit signal.""" + self._selected_path = path + for item in self._items: + item.selected = item.path == path + self.selection_changed.emit(path) + + def _on_item_remove(self, path: Path) -> None: + """Forward the remove request signal.""" + self.remove_requested.emit(path) diff --git a/synodic_client/application/screen/tray.py b/synodic_client/application/screen/tray.py index 84a41e0..6a04e5b 100644 --- a/synodic_client/application/screen/tray.py +++ b/synodic_client/application/screen/tray.py @@ -5,7 +5,7 @@ from collections.abc import Callable from porringer.api import API -from PySide6.QtCore import QThread, QTimer +from PySide6.QtCore import QTimer from PySide6.QtGui import QAction from PySide6.QtWidgets import ( QApplication, @@ -16,7 +16,7 @@ from synodic_client.application.icon import app_icon from synodic_client.application.screen.screen import MainWindow from synodic_client.application.screen.settings import SettingsWindow -from synodic_client.application.workers import ToolUpdateWorker, UpdateCheckWorker, UpdateDownloadWorker +from synodic_client.application.workers import check_for_update, download_update, run_tool_updates from synodic_client.client import Client from synodic_client.resolution import ( ResolvedConfig, @@ -52,8 +52,8 @@ def __init__( self._client = client self._window = window self._config = config - self._runner: QThread | None = None - self._tool_runner: QThread | None = None + self._update_task: asyncio.Task[None] | None = None + self._tool_task: asyncio.Task[None] | None = None self.tray_icon = app_icon() @@ -245,12 +245,16 @@ def _do_check_updates(self, *, silent: bool) -> None: self.update_action.setText('Checking for Updates...') self._settings_window.set_checking() - worker = UpdateCheckWorker(self._client) - worker.finished.connect(lambda result: self._on_update_check_finished(result, silent=silent)) - worker.error.connect(lambda error: self._on_update_check_error(error, silent=silent)) + self._update_task = asyncio.create_task(self._async_check_updates(silent=silent)) - self._runner = worker - self._runner.start() + async def _async_check_updates(self, *, silent: bool) -> None: + """Run the update check coroutine and route results.""" + try: + result = await check_for_update(self._client) + self._on_update_check_finished(result, silent=silent) + except Exception as exc: + logger.exception('Update check failed') + self._on_update_check_error(str(exc), silent=silent) def _on_update_check_finished(self, result: UpdateInfo | None, *, silent: bool = False) -> None: """Handle update check completion.""" @@ -310,26 +314,23 @@ def _on_tool_update(self) -> None: return logger.info('Starting periodic tool update check') - asyncio.ensure_future(self._do_tool_update(porringer)) + self._tool_task = asyncio.create_task(self._do_tool_update(porringer)) async def _do_tool_update(self, porringer: API) -> None: - """Resolve enabled plugins off-thread, then start the update worker.""" + """Resolve enabled plugins off-thread, then run the tool update.""" loop = asyncio.get_running_loop() config = self._resolve_config() - def fetch_plugins() -> list: - return porringer.plugin.list() - - all_plugins = await loop.run_in_executor(None, fetch_plugins) + all_plugins = await loop.run_in_executor(None, porringer.plugin.list) all_names = [p.name for p in all_plugins if p.installed] enabled = resolve_enabled_plugins(config, all_names) - worker = ToolUpdateWorker(porringer, plugins=enabled) - worker.finished.connect(self._on_tool_update_finished) - worker.error.connect(self._on_tool_update_error) - - self._tool_runner = worker - self._tool_runner.start() + try: + count = await run_tool_updates(porringer, plugins=enabled) + self._on_tool_update_finished(count) + except Exception as exc: + logger.exception('Tool update failed') + self._on_tool_update_error(str(exc)) def _on_single_plugin_update(self, plugin_name: str) -> None: """Upgrade a single plugin across all cached projects.""" @@ -339,13 +340,18 @@ def _on_single_plugin_update(self, plugin_name: str) -> None: return logger.info('Starting update for plugin: %s', plugin_name) + self._tool_task = asyncio.create_task( + self._async_single_plugin_update(porringer, plugin_name), + ) - worker = ToolUpdateWorker(porringer, plugins=[plugin_name]) - worker.finished.connect(self._on_tool_update_finished) - worker.error.connect(self._on_tool_update_error) - - self._tool_runner = worker - self._tool_runner.start() + async def _async_single_plugin_update(self, porringer: API, plugin_name: str) -> None: + """Run a single-plugin tool update and route results.""" + try: + count = await run_tool_updates(porringer, plugins=[plugin_name]) + self._on_tool_update_finished(count) + except Exception as exc: + logger.exception('Tool update failed') + self._on_tool_update_error(str(exc)) def _on_tool_update_finished(self, count: int) -> None: """Handle tool update completion.""" @@ -369,13 +375,19 @@ def _start_download(self, version: str) -> None: Args: version: The version string being downloaded (for banner display). """ - worker = UpdateDownloadWorker(self._client) - worker.finished.connect(lambda success: self._on_download_finished(success, version)) - worker.progress.connect(self._banner.show_downloading_progress) - worker.error.connect(self._on_download_error) + self._update_task = asyncio.create_task(self._async_download(version)) - self._runner = worker - self._runner.start() + async def _async_download(self, version: str) -> None: + """Run the download coroutine and route results.""" + try: + success = await download_update( + self._client, + on_progress=self._banner.show_downloading_progress, + ) + self._on_download_finished(success, version) + except Exception as exc: + logger.exception('Update download failed') + self._on_download_error(str(exc)) def _on_download_finished(self, success: bool, version: str) -> None: """Handle download completion — transition banner to ready state.""" diff --git a/synodic_client/application/theme.py b/synodic_client/application/theme.py index 893164a..27be8d6 100644 --- a/synodic_client/application/theme.py +++ b/synodic_client/application/theme.py @@ -8,7 +8,7 @@ # Window sizes (width, height) # --------------------------------------------------------------------------- INSTALL_PREVIEW_MIN_SIZE = (650, 400) -MAIN_WINDOW_MIN_SIZE = (600, 400) +MAIN_WINDOW_MIN_SIZE = (900, 600) # --------------------------------------------------------------------------- # Layout margins (left, top, right, bottom) @@ -359,3 +359,94 @@ '}' ) """Thin inline progress bar for the downloading state.""" + +# --------------------------------------------------------------------------- +# Manifest sidebar +# --------------------------------------------------------------------------- +SIDEBAR_WIDTH = 220 +"""Fixed width for the manifest sidebar panel.""" + +SIDEBAR_ITEM_HEIGHT = 32 +"""Fixed height for each manifest item row.""" + +SIDEBAR_SPACING = 2 +"""Vertical spacing between manifest items.""" + +SIDEBAR_STYLE = 'QFrame#sidebar { background: #252526; border-right: 1px solid palette(mid);}' +"""Container style for the sidebar panel.""" + +SIDEBAR_ITEM_STYLE = ( + 'QFrame#sidebarItem {' + ' background: transparent;' + ' border-radius: 4px;' + ' padding: 2px 8px;' + '}' + 'QFrame#sidebarItem:hover {' + ' background: #2a2d2e;' + '}' +) +"""Default style for a manifest sidebar item.""" + +SIDEBAR_ITEM_SELECTED_STYLE = 'QFrame#sidebarItem { background: #094771; border-radius: 4px; padding: 2px 8px;}' +"""Selected manifest sidebar item style — blue highlight.""" + +SIDEBAR_ITEM_DIMMED_STYLE = ( + 'QFrame#sidebarItem {' + ' background: transparent;' + ' border-radius: 4px;' + ' padding: 2px 8px;' + ' border: 1px dashed palette(mid);' + '}' +) +"""Dimmed sidebar item for directories whose path or manifest is missing.""" + +SIDEBAR_LABEL_STYLE = 'font-size: 12px; color: #cccccc;' +"""Sidebar item text label style.""" + +SIDEBAR_LABEL_DIMMED_STYLE = 'font-size: 12px; color: grey;' +"""Sidebar item text label for dimmed/invalid entries.""" + +SIDEBAR_CLOSE_STYLE = ( + 'QPushButton {' + ' border: none;' + ' font-size: 12px;' + ' color: transparent;' + ' padding: 0px 2px;' + ' min-width: 18px;' + ' max-width: 18px;' + '}' + 'QFrame#sidebarItem:hover QPushButton { color: #808080; }' + 'QPushButton:hover { color: #d4d4d4 !important; }' +) +"""Close (×) button — hidden until parent row is hovered.""" + +SIDEBAR_ADD_STYLE = ( + 'QPushButton {' + ' background: transparent;' + ' border: 1px dashed palette(mid);' + ' border-radius: 4px;' + ' font-size: 16px;' + ' color: #808080;' + ' padding: 4px;' + '}' + 'QPushButton:hover { color: #d4d4d4; border-color: #3794ff; }' +) +"""Add (+) button styled at the bottom of the sidebar.""" + +SIDEBAR_HEADER_STYLE = 'font-size: 11px; font-weight: bold; color: #808080; text-transform: uppercase;' +"""Style for the sidebar section heading.""" + +SIDEBAR_PHASE_LOADING_STYLE = 'font-size: 10px; color: #3794ff;' +"""Sidebar phase indicator — loading.""" + +SIDEBAR_PHASE_READY_STYLE = 'font-size: 10px; color: #89d185;' +"""Sidebar phase indicator — ready.""" + +SIDEBAR_PHASE_ERROR_STYLE = 'font-size: 10px; color: #f48771;' +"""Sidebar phase indicator — error.""" + +SIDEBAR_PHASE_INSTALLING_STYLE = 'font-size: 10px; color: #d7ba7d;' +"""Sidebar phase indicator — installing.""" + +SIDEBAR_PHASE_DONE_STYLE = 'font-size: 10px; color: #89d185;' +"""Sidebar phase indicator — done.""" diff --git a/synodic_client/application/workers.py b/synodic_client/application/workers.py index c0085ad..025d1be 100644 --- a/synodic_client/application/workers.py +++ b/synodic_client/application/workers.py @@ -1,112 +1,100 @@ -"""Background worker threads for the Synodic Client application. +"""Async background workers for the Synodic Client application. -Each worker wraps an off-main-thread operation and communicates results -back via Qt signals so that callers remain responsive. +Each coroutine runs on the caller's event loop (typically the qasync +main-thread loop) and communicates results via return values or +callbacks. Blocking calls are wrapped in ``loop.run_in_executor`` +to avoid stalling the GUI. """ import asyncio import logging +from collections.abc import Callable from pathlib import Path from porringer.api import API from porringer.schema import SetupParameters, SyncStrategy -from PySide6.QtCore import QThread, Signal from synodic_client.client import Client logger = logging.getLogger(__name__) -class UpdateCheckWorker(QThread): - """Worker for checking updates in a background thread.""" - - finished = Signal(object) # UpdateInfo - error = Signal(str) - - def __init__(self, client: Client) -> None: - """Initialize the worker.""" - super().__init__() - self._client = client - - def run(self) -> None: - """Run the update check.""" - try: - result = self._client.check_for_update() - self.finished.emit(result) - except Exception as e: - logger.exception('Update check failed') - self.error.emit(str(e)) - - -class UpdateDownloadWorker(QThread): - """Worker for downloading updates in a background thread.""" - - finished = Signal(bool) # success status - progress = Signal(int) # percentage (0-100) - error = Signal(str) - - def __init__(self, client: Client) -> None: - """Initialize the worker.""" - super().__init__() - self._client = client - - def run(self) -> None: - """Run the update download.""" - try: - - def progress_callback(percentage: int) -> None: - self.progress.emit(percentage) - - success = self._client.download_update(progress_callback) - self.finished.emit(success) - except Exception as e: - logger.exception('Update download failed') - self.error.emit(str(e)) - - -class ToolUpdateWorker(QThread): - """Worker for re-syncing manifest-declared tools in a background thread.""" - - finished = Signal(int) # number of manifests processed - error = Signal(str) - - def __init__(self, porringer: API, plugins: list[str] | None = None) -> None: - """Initialize the worker. - - Args: - porringer: The porringer API instance. - plugins: Optional include-list of plugin names. When set, only - actions handled by these plugins are executed. ``None`` - means all plugins. - """ - super().__init__() - self._porringer = porringer - self._plugins = plugins - - def run(self) -> None: - """Re-sync all cached project manifests.""" - try: - directories = self._porringer.cache.list_directories() - count = 0 - for directory in directories: - path = Path(directory.path) - if not self._porringer.sync.has_manifest(path): - logger.debug('Skipping path without manifest: %s', path) - continue - params = SetupParameters( - paths=[path], - project_directory=path if path.is_dir() else None, - strategy=SyncStrategy.LATEST, - plugins=self._plugins, - ) - asyncio.run(self._sync(params)) - count += 1 - self.finished.emit(count) - except Exception as e: - logger.exception('Tool update failed') - self.error.emit(str(e)) - - async def _sync(self, params: SetupParameters) -> None: - """Execute a sync stream for the given parameters.""" - async for _event in self._porringer.sync.execute_stream(params): +async def check_for_update(client: Client) -> object: + """Check for application updates off the main thread. + + Args: + client: The Synodic Client service. + + Returns: + An ``UpdateInfo`` result. + """ + loop = asyncio.get_running_loop() + return await loop.run_in_executor(None, client.check_for_update) + + +async def download_update( + client: Client, + on_progress: Callable[[int], None] | None = None, +) -> bool: + """Download an application update off the main thread. + + Args: + client: The Synodic Client service. + on_progress: Optional callback for download progress (0-100). + Invoked on the event loop thread via + ``call_soon_threadsafe``. + + Returns: + ``True`` if the download succeeded. + """ + loop = asyncio.get_running_loop() + + def _run() -> bool: + def progress_callback(percentage: int) -> None: + if on_progress is not None: + loop.call_soon_threadsafe(on_progress, percentage) + + return client.download_update(progress_callback) + + return await loop.run_in_executor(None, _run) + + +async def run_tool_updates( + porringer: API, + plugins: list[str] | None = None, +) -> int: + """Re-sync all cached project manifests. + + Args: + porringer: The porringer API instance. + plugins: Optional include-list of plugin names. When set, only + actions handled by these plugins are executed. ``None`` + means all plugins. + + Returns: + Number of manifests processed. + """ + loop = asyncio.get_running_loop() + directories = await loop.run_in_executor(None, porringer.cache.list_directories) + + # Check all directories for manifests in parallel + paths = [Path(d.path) for d in directories] + has_results = await asyncio.gather( + *(loop.run_in_executor(None, porringer.sync.has_manifest, p) for p in paths), + ) + + count = 0 + for path, has in zip(paths, has_results, strict=True): + if not has: + logger.debug('Skipping path without manifest: %s', path) + continue + params = SetupParameters( + paths=[path], + project_directory=path if path.is_dir() else None, + strategy=SyncStrategy.LATEST, + plugins=plugins, + ) + async for _event in porringer.sync.execute_stream(params): pass # consume events to completion + count += 1 + return count diff --git a/synodic_client/client.py b/synodic_client/client.py index aec853f..c25b68e 100644 --- a/synodic_client/client.py +++ b/synodic_client/client.py @@ -105,17 +105,6 @@ def download_update(self, progress_callback: Callable[[int], None] | None = None return self._updater.download_update(progress_callback) - def apply_update_and_restart(self) -> None: - """Apply a downloaded update and restart the application. - - This method will not return - it exits and relaunches the app. - """ - if self._updater is None: - logger.warning('Updater not initialized') - return - - self._updater.apply_update_and_restart() - def apply_update_on_exit(self, restart: bool = True) -> None: """Schedule the update to apply when the application exits. diff --git a/synodic_client/updater.py b/synodic_client/updater.py index 5e73fc0..f91b118 100644 --- a/synodic_client/updater.py +++ b/synodic_client/updater.py @@ -302,14 +302,17 @@ def download_update(self, progress_callback: Callable[[int], None] | None = None self._update_info.error = str(e) return False - def apply_update_and_restart(self, restart_args: list[str] | None = None) -> None: - """Apply the downloaded update and restart the application. + def apply_update_on_exit(self, restart: bool = True, restart_args: list[str] | None = None) -> None: + """Apply the downloaded update, optionally restarting the application. - This method will not return - it exits the current process - and launches the updated version. + When *restart* is ``True`` the Velopack runtime applies the update + **and** relaunches the new version (the call does not return). + When ``False`` the update is staged and applied after the process + exits without relaunching. Args: - restart_args: Optional arguments to pass to the restarted application + restart: Whether to restart the application after applying. + restart_args: Optional arguments to pass to the restarted application. """ if not self.is_installed: raise NotImplementedError('Updates are only supported for Velopack installs') @@ -320,23 +323,28 @@ def apply_update_and_restart(self, restart_args: list[str] | None = None) -> Non if self._update_info._velopack_info is None: raise RuntimeError('No Velopack update info available') - self._state = UpdateState.APPLYING - try: manager = self._get_velopack_manager() if manager is None: raise RuntimeError('Velopack manager not available') - logger.info('Applying update and restarting...') - if restart_args: - manager.apply_updates_and_restart_with_args( - self._update_info._velopack_info, - restart_args, - ) + logger.info('Applying update (restart=%s)', restart) + + if restart: + self._state = UpdateState.APPLYING + if restart_args: + manager.apply_updates_and_restart_with_args( + self._update_info._velopack_info, + restart_args, + ) + else: + manager.apply_updates_and_restart(self._update_info._velopack_info) + # apply_updates_and_restart terminates the process; + # fall through only as a safety net. + sys.exit(0) else: - manager.apply_updates_and_restart(self._update_info._velopack_info) - # This should not return, but just in case - sys.exit(0) + manager.apply_updates_and_exit(self._update_info._velopack_info) + self._state = UpdateState.APPLIED except Exception as e: logger.exception('Failed to apply update') @@ -344,43 +352,6 @@ def apply_update_and_restart(self, restart_args: list[str] | None = None) -> Non self._update_info.error = str(e) raise - def apply_update_on_exit(self, restart: bool = True, restart_args: list[str] | None = None) -> None: - """Schedule the update to be applied when the application exits. - - Unlike apply_update_and_restart, this method returns immediately - and the update is applied after the application exits gracefully. - - Args: - restart: Whether to restart the application after applying - restart_args: Optional arguments to pass to the restarted application - """ - if not self.is_installed: - raise NotImplementedError('Updates are only supported for Velopack installs') - - if self._state != UpdateState.DOWNLOADED or not self._update_info: - raise RuntimeError('No downloaded update to apply') - - if self._update_info._velopack_info is None: - raise RuntimeError('No Velopack update info available') - - try: - manager = self._get_velopack_manager() - if manager is None: - raise RuntimeError('Velopack manager not available') - - logger.info('Scheduling update to apply on exit (restart=%s)', restart) - # Velopack apply_updates_and_exit applies on exit - # Note: The restart parameter is not supported by Velopack's exit method - # The app will need to be manually restarted or use apply_updates_and_restart - manager.apply_updates_and_exit(self._update_info._velopack_info) - self._state = UpdateState.APPLIED - - except Exception as e: - logger.exception('Failed to schedule update') - self._state = UpdateState.FAILED - self._update_info.error = str(e) - raise - _NOT_INSTALLED_SENTINEL = 'not properly installed' """Substring the Velopack SDK includes in its ``RuntimeError`` when the application was not installed via Velopack.""" diff --git a/tests/unit/qt/test_install_preview.py b/tests/unit/qt/test_install_preview.py index 335250a..65d016b 100644 --- a/tests/unit/qt/test_install_preview.py +++ b/tests/unit/qt/test_install_preview.py @@ -2,13 +2,13 @@ from __future__ import annotations +import asyncio from pathlib import Path from typing import Any from unittest.mock import MagicMock import pytest from porringer.schema import ( - CancellationToken, DownloadResult, ProgressEvent, ProgressEventKind, @@ -26,10 +26,12 @@ ) from synodic_client.application.screen.install import ( InstallConfig, - InstallWorker, - PreviewWorker, + PreviewCallbacks, + PreviewConfig, normalize_manifest_key, resolve_local_path, + run_install, + run_preview, ) _DOWNLOAD_PATCH = 'synodic_client.application.screen.install.API.download' @@ -137,11 +139,11 @@ def test_description_fallback(self) -> None: class TestInstallWorker: - """Tests for InstallWorker signal emission.""" + """Tests for run_install coroutine.""" @staticmethod def test_worker_emits_finished_on_success() -> None: - """Verify worker emits finished signal with results.""" + """Verify coroutine returns results on success.""" porringer = MagicMock() manifest_path = Path('/tmp/test/porringer.json') @@ -162,19 +164,13 @@ async def mock_stream(*args, **kwargs): porringer.sync.execute_stream = mock_stream - token = CancellationToken() - worker = InstallWorker(porringer, manifest_path, token) + results = asyncio.run(run_install(porringer, manifest_path)) - received: list[SetupResults] = [] - worker.finished.connect(received.append) - worker.run() - - assert len(received) == 1 - assert received[0].actions == manifest.actions + assert results.actions == manifest.actions @staticmethod def test_worker_emits_error_on_failure() -> None: - """Verify worker emits error signal on exception.""" + """Verify coroutine raises on exception.""" porringer = MagicMock() manifest_path = Path('/tmp/test/porringer.json') @@ -186,15 +182,8 @@ async def mock_stream(*args, **kwargs): porringer.sync.execute_stream = mock_stream - token = CancellationToken() - worker = InstallWorker(porringer, manifest_path, token) - - errors: list[str] = [] - worker.error.connect(errors.append) - worker.run() - - assert len(errors) == 1 - assert 'boom' in errors[0] + with pytest.raises(RuntimeError, match='boom'): + asyncio.run(run_install(porringer, manifest_path)) @staticmethod def test_worker_passes_prerelease_packages() -> None: @@ -213,14 +202,13 @@ async def mock_stream(params: Any) -> Any: porringer.sync.execute_stream = mock_stream - token = CancellationToken() - worker = InstallWorker( - porringer, - manifest_path, - token, - InstallConfig(prerelease_packages={'cppython'}), + asyncio.run( + run_install( + porringer, + manifest_path, + InstallConfig(prerelease_packages={'cppython'}), + ), ) - worker.run() assert len(captured_params) == 1 assert captured_params[0].prerelease_packages == {'cppython'} @@ -242,9 +230,7 @@ async def mock_stream(params: Any) -> Any: porringer.sync.execute_stream = mock_stream - token = CancellationToken() - worker = InstallWorker(porringer, manifest_path, token) - worker.run() + asyncio.run(run_install(porringer, manifest_path)) assert len(captured_params) == 1 assert captured_params[0].prerelease_packages is None @@ -283,11 +269,11 @@ def test_existing_relative_path(tmp_path: Path) -> None: class TestPreviewWorkerLocal: - """Tests for PreviewWorker with local manifest files.""" + """Tests for run_preview with local manifest files.""" @staticmethod def test_local_manifest_skips_download(tmp_path: Path) -> None: - """Verify PreviewWorker skips download for local files.""" + """Verify run_preview skips download for local files.""" manifest = tmp_path / 'porringer.json' manifest.write_text('{}') @@ -300,11 +286,17 @@ async def mock_stream(*args: Any, **kwargs: Any) -> Any: porringer.sync.execute_stream = mock_stream - worker = PreviewWorker(porringer, str(manifest)) - results: list[tuple[object, str, str]] = [] - worker.preview_ready.connect(lambda r, p, t: results.append((r, p, t))) - worker.run() + + asyncio.run( + run_preview( + porringer, + str(manifest), + callbacks=PreviewCallbacks( + on_preview_ready=lambda r, p, t: results.append((r, p, t)), + ), + ), + ) assert len(results) == 1 assert results[0][0] is expected @@ -313,25 +305,20 @@ async def mock_stream(*args: Any, **kwargs: Any) -> Any: @staticmethod def test_local_manifest_not_found(tmp_path: Path) -> None: - """Verify PreviewWorker emits error for missing local file.""" + """Verify run_preview raises error for missing local file.""" path = str(tmp_path / 'nonexistent' / 'porringer.json') porringer = MagicMock() - worker = PreviewWorker(porringer, path) - - errors: list[str] = [] - worker.error.connect(errors.append) - worker.run() - assert len(errors) == 1 - assert 'not found' in errors[0].lower() + with pytest.raises(FileNotFoundError, match='not found'): + asyncio.run(run_preview(porringer, path)) class TestPreviewWorker: - """Tests for PreviewWorker download and preview flow.""" + """Tests for run_preview download and preview flow.""" @staticmethod def test_emits_error_on_download_failure(monkeypatch: pytest.MonkeyPatch) -> None: - """Verify PreviewWorker emits error when download fails.""" + """Verify run_preview raises when download fails.""" porringer = MagicMock() monkeypatch.setattr( _DOWNLOAD_PATCH, @@ -344,18 +331,12 @@ def test_emits_error_on_download_failure(monkeypatch: pytest.MonkeyPatch) -> Non ), ) - worker = PreviewWorker(porringer, 'https://example.com/bad.json') - - errors: list[str] = [] - worker.error.connect(errors.append) - worker.run() - - assert len(errors) == 1 - assert 'Network error' in errors[0] + with pytest.raises(RuntimeError, match='Network error'): + asyncio.run(run_preview(porringer, 'https://example.com/bad.json')) @staticmethod def test_emits_preview_ready_on_success(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: - """Verify PreviewWorker emits preview_ready with SetupResults.""" + """Verify run_preview invokes on_preview_ready with SetupResults.""" porringer = MagicMock() dest = tmp_path / 'porringer.json' @@ -380,22 +361,28 @@ async def mock_stream(*args: Any, **kwargs: Any) -> Any: porringer.sync.execute_stream = mock_stream - worker = PreviewWorker(porringer, 'https://example.com/good.json') - results: list[tuple[object, str, str]] = [] - worker.preview_ready.connect(lambda r, p, t: results.append((r, p, t))) - worker.run() + + asyncio.run( + run_preview( + porringer, + 'https://example.com/good.json', + callbacks=PreviewCallbacks( + on_preview_ready=lambda r, p, t: results.append((r, p, t)), + ), + ), + ) assert len(results) == 1 assert results[0][0] is expected class TestPreviewWorkerSignals: - """Tests for PreviewWorker signal emission and dry-run status check.""" + """Tests for run_preview callback invocation and dry-run status check.""" @staticmethod def test_emits_preview_ready_and_finished(tmp_path: Path) -> None: - """Verify worker emits preview_ready then finished for a local manifest.""" + """Verify coroutine invokes on_preview_ready for a local manifest.""" manifest = tmp_path / 'porringer.json' manifest.write_text('{}') @@ -415,25 +402,33 @@ async def mock_stream(*args: Any, **kwargs: Any) -> Any: porringer.sync.execute_stream = mock_stream - worker = PreviewWorker(porringer, str(manifest)) - ready_calls: list[tuple[object, str, str]] = [] checked: list[tuple[int, SetupActionResult]] = [] - finished_count: list[int] = [] - worker.preview_ready.connect(lambda p, m, t: ready_calls.append((p, m, t))) - worker.action_checked.connect(lambda row, r: checked.append((row, r))) - worker.finished.connect(lambda: finished_count.append(1)) - worker.run() + finished = False + + async def _run() -> None: + nonlocal finished + await run_preview( + porringer, + str(manifest), + callbacks=PreviewCallbacks( + on_preview_ready=lambda p, m, t: ready_calls.append((p, m, t)), + on_action_checked=lambda row, r: checked.append((row, r)), + ), + ) + finished = True + + asyncio.run(_run()) assert len(ready_calls) == 1 assert ready_calls[0][0] is preview assert len(checked) == 1 assert checked[0] == (0, result) - assert len(finished_count) == 1 + assert finished is True @staticmethod def test_emits_finished_for_empty_actions(tmp_path: Path) -> None: - """Verify worker emits finished signal even with no actions.""" + """Verify coroutine completes normally with no actions.""" manifest = tmp_path / 'porringer.json' manifest.write_text('{}') @@ -446,17 +441,19 @@ async def mock_stream(*args: Any, **kwargs: Any) -> Any: porringer.sync.execute_stream = mock_stream - worker = PreviewWorker(porringer, str(manifest)) + finished = False - finished_count: list[int] = [] - worker.finished.connect(lambda: finished_count.append(1)) - worker.run() + async def _run() -> None: + nonlocal finished + await run_preview(porringer, str(manifest)) + finished = True - assert len(finished_count) == 1 + asyncio.run(_run()) + assert finished is True @staticmethod def test_action_checked_maps_correct_rows(tmp_path: Path) -> None: - """Verify action_checked emits correct row indices via identity matching.""" + """Verify on_action_checked receives correct row indices via identity matching.""" manifest = tmp_path / 'porringer.json' manifest.write_text('{}') @@ -484,11 +481,17 @@ async def mock_stream(*args: Any, **kwargs: Any) -> Any: porringer.sync.execute_stream = mock_stream - worker = PreviewWorker(porringer, str(manifest)) - checked: list[tuple[int, SetupActionResult]] = [] - worker.action_checked.connect(lambda row, r: checked.append((row, r))) - worker.run() + + asyncio.run( + run_preview( + porringer, + str(manifest), + callbacks=PreviewCallbacks( + on_action_checked=lambda row, r: checked.append((row, r)), + ), + ), + ) assert len(checked) == _EXPECTED_CHECKED_COUNT # action_b is at index 1 in preview, action_a at index 0 @@ -497,7 +500,7 @@ async def mock_stream(*args: Any, **kwargs: Any) -> Any: @staticmethod def test_emits_error_when_dry_run_fails(tmp_path: Path) -> None: - """Verify worker emits error when dry-run raises.""" + """Verify coroutine raises when dry-run fails.""" manifest = tmp_path / 'porringer.json' manifest.write_text('{}') @@ -511,21 +514,12 @@ async def mock_stream(*args: Any, **kwargs: Any) -> Any: porringer.sync.execute_stream = mock_stream - worker = PreviewWorker(porringer, str(manifest)) - - errors: list[str] = [] - finished_count: list[int] = [] - worker.error.connect(errors.append) - worker.finished.connect(lambda: finished_count.append(1)) - worker.run() - - assert len(errors) == 1 - assert 'dry-run boom' in errors[0] - assert len(finished_count) == 0 + with pytest.raises(RuntimeError, match='dry-run boom'): + asyncio.run(run_preview(porringer, str(manifest))) @staticmethod def test_emits_plugins_queried(tmp_path: Path) -> None: - """Verify plugins_queried is emitted with plugin presence mapping.""" + """Verify on_plugins_queried is invoked with plugin presence mapping.""" manifest = tmp_path / 'porringer.json' manifest.write_text('{}') @@ -544,18 +538,24 @@ async def mock_stream(*args: Any, **kwargs: Any) -> Any: porringer.sync.execute_stream = mock_stream - worker = PreviewWorker(porringer, str(manifest)) - plugin_data: list[dict[str, bool]] = [] - worker.plugins_queried.connect(plugin_data.append) - worker.run() + + asyncio.run( + run_preview( + porringer, + str(manifest), + callbacks=PreviewCallbacks( + on_plugins_queried=plugin_data.append, + ), + ), + ) assert len(plugin_data) == 1 assert plugin_data[0] == {'pip': True, 'uv': False} @staticmethod def test_plugins_queried_emitted_before_preview_ready(tmp_path: Path) -> None: - """Verify plugins_queried fires before preview_ready.""" + """Verify on_plugins_queried fires before on_preview_ready.""" manifest = tmp_path / 'porringer.json' manifest.write_text('{}') @@ -574,18 +574,24 @@ async def mock_stream(*args: Any, **kwargs: Any) -> Any: porringer.sync.execute_stream = mock_stream - worker = PreviewWorker(porringer, str(manifest)) - order: list[str] = [] - worker.plugins_queried.connect(lambda _: order.append('plugins')) - worker.preview_ready.connect(lambda *_: order.append('preview')) - worker.run() + + asyncio.run( + run_preview( + porringer, + str(manifest), + callbacks=PreviewCallbacks( + on_plugins_queried=lambda _: order.append('plugins'), + on_preview_ready=lambda *_: order.append('preview'), + ), + ), + ) assert order == ['plugins', 'preview'] @staticmethod def test_emits_manifest_parsed(tmp_path: Path) -> None: - """Verify manifest_parsed is emitted from MANIFEST_PARSED events.""" + """Verify on_manifest_parsed is invoked from MANIFEST_PARSED events.""" manifest = tmp_path / 'porringer.json' manifest.write_text('{}') @@ -604,17 +610,23 @@ async def mock_stream(*args: Any, **kwargs: Any) -> Any: porringer.sync.execute_stream = mock_stream - worker = PreviewWorker(porringer, str(manifest)) - parsed_data: list[Any] = [] - worker.manifest_parsed.connect(lambda *a: parsed_data.append(a)) - worker.run() + + asyncio.run( + run_preview( + porringer, + str(manifest), + callbacks=PreviewCallbacks( + on_manifest_parsed=lambda *a: parsed_data.append(a), + ), + ), + ) assert len(parsed_data) == 1 @staticmethod def test_two_phase_signal_order(tmp_path: Path) -> None: - """Verify the full two-phase signal order: parsed → plugins → ready.""" + """Verify the full two-phase callback order: parsed → plugins → ready.""" manifest = tmp_path / 'porringer.json' manifest.write_text('{}') @@ -638,19 +650,25 @@ async def mock_stream(*args: Any, **kwargs: Any) -> Any: porringer.sync.execute_stream = mock_stream - worker = PreviewWorker(porringer, str(manifest)) - order: list[str] = [] - worker.manifest_parsed.connect(lambda *_: order.append('parsed')) - worker.plugins_queried.connect(lambda _: order.append('plugins')) - worker.preview_ready.connect(lambda *_: order.append('ready')) - worker.run() + + asyncio.run( + run_preview( + porringer, + str(manifest), + callbacks=PreviewCallbacks( + on_manifest_parsed=lambda *_: order.append('parsed'), + on_plugins_queried=lambda _: order.append('plugins'), + on_preview_ready=lambda *_: order.append('ready'), + ), + ), + ) assert order == ['parsed', 'plugins', 'ready'] class TestPreviewWorkerUpdateDetection: - """Tests for PreviewWorker passing update-detection flags to porringer.""" + """Tests for run_preview passing update-detection flags to porringer.""" @staticmethod def test_passes_detect_updates_and_prerelease_packages(tmp_path: Path) -> None: @@ -670,13 +688,16 @@ async def mock_stream(params: Any) -> Any: porringer.sync.execute_stream = mock_stream - worker = PreviewWorker( - porringer, - str(manifest), - detect_updates=True, - prerelease_packages={'some-pkg'}, + asyncio.run( + run_preview( + porringer, + str(manifest), + config=PreviewConfig( + detect_updates=True, + prerelease_packages={'some-pkg'}, + ), + ), ) - worker.run() assert len(captured_params) == 1 assert captured_params[0].detect_updates is True @@ -700,8 +721,7 @@ async def mock_stream(params: Any) -> Any: porringer.sync.execute_stream = mock_stream - worker = PreviewWorker(porringer, str(manifest)) - worker.run() + asyncio.run(run_preview(porringer, str(manifest))) assert len(captured_params) == 1 assert captured_params[0].detect_updates is True @@ -751,12 +771,13 @@ async def mock_stream(params: Any) -> Any: porringer.sync.execute_stream = mock_stream - worker = PreviewWorker( - porringer, - str(manifest), - project_directory=manifest.parent, + asyncio.run( + run_preview( + porringer, + str(manifest), + config=PreviewConfig(project_directory=manifest.parent), + ), ) - worker.run() assert len(captured_params) == 1 assert captured_params[0].project_directory == tmp_path @@ -779,12 +800,13 @@ async def mock_stream(params: Any) -> Any: porringer.sync.execute_stream = mock_stream - worker = PreviewWorker( - porringer, - str(tmp_path), - project_directory=tmp_path, + asyncio.run( + run_preview( + porringer, + str(tmp_path), + config=PreviewConfig(project_directory=tmp_path), + ), ) - worker.run() assert len(captured_params) == 1 assert captured_params[0].project_directory == tmp_path @@ -805,7 +827,7 @@ def _make_scm_action(description: str = 'Clone repo') -> MagicMock: return action def test_scm_already_installed_emits_correct_result(self, tmp_path: Path) -> None: - """An SCM action with ALREADY_INSTALLED is surfaced via action_checked.""" + """An SCM action with ALREADY_INSTALLED is surfaced via on_action_checked.""" manifest = tmp_path / 'porringer.json' manifest.write_text('{}') @@ -832,11 +854,18 @@ async def mock_stream(*args: Any, **kwargs: Any) -> Any: porringer.sync.execute_stream = mock_stream - worker = PreviewWorker(porringer, str(manifest), project_directory=tmp_path) - checked: list[tuple[int, SetupActionResult]] = [] - worker.action_checked.connect(lambda row, r: checked.append((row, r))) - worker.run() + + asyncio.run( + run_preview( + porringer, + str(manifest), + config=PreviewConfig(project_directory=tmp_path), + callbacks=PreviewCallbacks( + on_action_checked=lambda row, r: checked.append((row, r)), + ), + ), + ) assert len(checked) == 1 assert checked[0] == (0, result) @@ -871,11 +900,18 @@ async def mock_stream(*args: Any, **kwargs: Any) -> Any: porringer.sync.execute_stream = mock_stream - worker = PreviewWorker(porringer, str(manifest), project_directory=tmp_path) - checked: list[tuple[int, SetupActionResult]] = [] - worker.action_checked.connect(lambda row, r: checked.append((row, r))) - worker.run() + + asyncio.run( + run_preview( + porringer, + str(manifest), + config=PreviewConfig(project_directory=tmp_path), + callbacks=PreviewCallbacks( + on_action_checked=lambda row, r: checked.append((row, r)), + ), + ), + ) assert len(checked) == 1 assert checked[0][1].skipped is False diff --git a/tests/unit/qt/test_log_panel.py b/tests/unit/qt/test_log_panel.py index b2e124f..61f2415 100644 --- a/tests/unit/qt/test_log_panel.py +++ b/tests/unit/qt/test_log_panel.py @@ -1,12 +1,12 @@ -"""Tests for the execution log panel widgets and new InstallWorker signals.""" +"""Tests for the execution log panel widgets and run_install callback signals.""" from __future__ import annotations +import asyncio from pathlib import Path from unittest.mock import MagicMock from porringer.schema import ( - CancellationToken, ProgressEvent, ProgressEventKind, SetupAction, @@ -17,7 +17,7 @@ ) from porringer.schema.plugin import PluginKind -from synodic_client.application.screen.install import InstallWorker +from synodic_client.application.screen.install import InstallCallbacks, run_install from synodic_client.application.screen.log_panel import ( CHEVRON_DOWN, CHEVRON_RIGHT, @@ -447,16 +447,16 @@ def test_multiple_actions_tracked_independently() -> None: # --------------------------------------------------------------------------- -# InstallWorker signal tests for new action_started / sub_progress signals +# run_install callback tests for action_started / sub_progress signals # --------------------------------------------------------------------------- -class TestInstallWorkerNewSignals: - """Tests for InstallWorker action_started and sub_progress signals.""" +class TestRunInstallCallbackSignals: + """Tests for run_install action_started and sub_progress callbacks.""" @staticmethod def test_emits_action_started() -> None: - """Verify worker emits action_started when ACTION_STARTED event arrives.""" + """Verify run_install invokes on_action_started when ACTION_STARTED event arrives.""" porringer = MagicMock() manifest_path = Path('/tmp/test/porringer.json') @@ -479,19 +479,22 @@ async def mock_stream(*args, **kwargs): porringer.sync.execute_stream = mock_stream - token = CancellationToken() - worker = InstallWorker(porringer, manifest_path, token) - started_actions: list[object] = [] - worker.action_started.connect(started_actions.append) - worker.run() + + asyncio.run( + run_install( + porringer, + manifest_path, + callbacks=InstallCallbacks(on_action_started=started_actions.append), + ), + ) assert len(started_actions) == 1 assert started_actions[0] is action @staticmethod def test_emits_sub_progress() -> None: - """Verify worker emits sub_progress when SUB_ACTION_PROGRESS event arrives.""" + """Verify run_install invokes on_sub_progress when SUB_ACTION_PROGRESS event arrives.""" porringer = MagicMock() manifest_path = Path('/tmp/test/porringer.json') @@ -524,12 +527,17 @@ async def mock_stream(*args, **kwargs): porringer.sync.execute_stream = mock_stream - token = CancellationToken() - worker = InstallWorker(porringer, manifest_path, token) - received: list[tuple[object, object]] = [] - worker.sub_progress.connect(lambda a, s: received.append((a, s))) - worker.run() + + asyncio.run( + run_install( + porringer, + manifest_path, + callbacks=InstallCallbacks( + on_sub_progress=lambda a, s: received.append((a, s)), + ), + ), + ) assert len(received) == 1 assert received[0][0] is action @@ -564,12 +572,17 @@ async def mock_stream(*args, **kwargs): porringer.sync.execute_stream = mock_stream - token = CancellationToken() - worker = InstallWorker(porringer, manifest_path, token) - received: list[tuple[object, object]] = [] - worker.sub_progress.connect(lambda a, s: received.append((a, s))) - worker.run() + + asyncio.run( + run_install( + porringer, + manifest_path, + callbacks=InstallCallbacks( + on_sub_progress=lambda a, s: received.append((a, s)), + ), + ), + ) assert len(received) == 0 @@ -598,11 +611,14 @@ async def mock_stream(*args, **kwargs): porringer.sync.execute_stream = mock_stream - token = CancellationToken() - worker = InstallWorker(porringer, manifest_path, token) - started_actions: list[object] = [] - worker.action_started.connect(started_actions.append) - worker.run() + + asyncio.run( + run_install( + porringer, + manifest_path, + callbacks=InstallCallbacks(on_action_started=started_actions.append), + ), + ) assert len(started_actions) == 0 diff --git a/tests/unit/qt/test_sidebar.py b/tests/unit/qt/test_sidebar.py new file mode 100644 index 0000000..7d8eae6 --- /dev/null +++ b/tests/unit/qt/test_sidebar.py @@ -0,0 +1,308 @@ +"""Tests for the ManifestSidebar and ManifestItem widgets.""" + +from __future__ import annotations + +from pathlib import Path + +from synodic_client.application.screen.install import PreviewPhase +from synodic_client.application.screen.sidebar import ManifestItem, ManifestSidebar +from synodic_client.application.theme import SIDEBAR_WIDTH + +_EXPECTED_DIRECTORY_COUNT = 2 + + +# --------------------------------------------------------------------------- +# ManifestItem +# --------------------------------------------------------------------------- + + +class TestManifestItemInit: + """Basic construction and property access.""" + + @staticmethod + def test_path_property(tmp_path: Path) -> None: + """Verify the path property returns the construction path.""" + item = ManifestItem(tmp_path, 'proj') + assert item.path == tmp_path + + @staticmethod + def test_default_not_selected(tmp_path: Path) -> None: + """Verify a new item is not selected by default.""" + item = ManifestItem(tmp_path, 'proj') + assert item.selected is False + + @staticmethod + def test_display_name_uses_explicit_name(tmp_path: Path) -> None: + """Verify label uses the explicit display name when provided.""" + item = ManifestItem(tmp_path, 'My Project') + assert item._label.text() == 'My Project' + + @staticmethod + def test_display_name_falls_back_to_path_name(tmp_path: Path) -> None: + """Verify label falls back to the path stem when no name is given.""" + item = ManifestItem(tmp_path) + assert item._label.text() == tmp_path.name + + @staticmethod + def test_tooltip_shows_full_path(tmp_path: Path) -> None: + """Verify the tooltip shows the full path string.""" + item = ManifestItem(tmp_path, 'proj') + assert item.toolTip() == str(tmp_path) + + +class TestManifestItemSelection: + """Selection state changes.""" + + @staticmethod + def test_set_selected_true(tmp_path: Path) -> None: + """Verify setting selected to True updates the state.""" + item = ManifestItem(tmp_path, 'proj') + item.selected = True + assert item.selected is True + + @staticmethod + def test_set_selected_false(tmp_path: Path) -> None: + """Verify toggling selected back to False works.""" + item = ManifestItem(tmp_path, 'proj') + item.selected = True + item.selected = False + assert item.selected is False + + +class TestManifestItemPhase: + """Phase indicator updates.""" + + @staticmethod + def test_phase_loading(tmp_path: Path) -> None: + """Verify LOADING phase shows the loading indicator.""" + item = ManifestItem(tmp_path, 'proj') + item.set_phase(PreviewPhase.LOADING) + assert not item._phase_label.isHidden() + assert item._phase_label.text() == 'Loading…' + + @staticmethod + def test_phase_ready(tmp_path: Path) -> None: + """Verify READY phase label text.""" + item = ManifestItem(tmp_path, 'proj') + item.set_phase(PreviewPhase.READY) + assert item._phase_label.text() == 'Ready' + + @staticmethod + def test_phase_error(tmp_path: Path) -> None: + """Verify ERROR phase label text.""" + item = ManifestItem(tmp_path, 'proj') + item.set_phase(PreviewPhase.ERROR) + assert item._phase_label.text() == 'Error' + + @staticmethod + def test_phase_installing(tmp_path: Path) -> None: + """Verify INSTALLING phase label text.""" + item = ManifestItem(tmp_path, 'proj') + item.set_phase(PreviewPhase.INSTALLING) + assert item._phase_label.text() == 'Installing…' + + @staticmethod + def test_phase_done(tmp_path: Path) -> None: + """Verify DONE phase label text.""" + item = ManifestItem(tmp_path, 'proj') + item.set_phase(PreviewPhase.DONE) + assert item._phase_label.text() == 'Done' + + +class TestManifestItemSignals: + """Signal emission.""" + + @staticmethod + def test_clicked_on_mouse_press(tmp_path: Path) -> None: + """Verify clicked signal fires on mousePressEvent.""" + item = ManifestItem(tmp_path, 'proj') + received: list[Path] = [] + item.clicked.connect(received.append) + item.mousePressEvent(None) + assert received == [tmp_path] + + @staticmethod + def test_remove_requested_on_close(tmp_path: Path) -> None: + """Verify remove_requested signal fires on close button click.""" + item = ManifestItem(tmp_path, 'proj') + received: list[Path] = [] + item.remove_requested.connect(received.append) + item._close_btn.click() + assert received == [tmp_path] + + +# --------------------------------------------------------------------------- +# ManifestSidebar +# --------------------------------------------------------------------------- + + +class TestManifestSidebarInit: + """Basic construction.""" + + @staticmethod + def test_default_no_items() -> None: + """Verify a new sidebar has no selection.""" + sidebar = ManifestSidebar() + assert sidebar.selected_path is None + + @staticmethod + def test_fixed_width() -> None: + """Verify the sidebar minimum width matches the theme constant.""" + sidebar = ManifestSidebar() + assert sidebar.minimumWidth() == SIDEBAR_WIDTH + + +class TestManifestSidebarSetDirectories: + """Populating the sidebar with items.""" + + @staticmethod + def test_creates_items(tmp_path: Path) -> None: + """Verify set_directories creates the expected number of items.""" + sidebar = ManifestSidebar() + d1 = tmp_path / 'a' + d2 = tmp_path / 'b' + sidebar.set_directories([(d1, 'A', True), (d2, 'B', True)]) + assert len(sidebar._items) == _EXPECTED_DIRECTORY_COUNT + + @staticmethod + def test_replaces_previous_items(tmp_path: Path) -> None: + """Verify set_directories replaces prior items completely.""" + sidebar = ManifestSidebar() + d1 = tmp_path / 'a' + d2 = tmp_path / 'b' + d3 = tmp_path / 'c' + sidebar.set_directories([(d1, 'A', True), (d2, 'B', True)]) + sidebar.set_directories([(d3, 'C', True)]) + assert len(sidebar._items) == 1 + assert sidebar._items[0].path == d3 + + @staticmethod + def test_clears_selection(tmp_path: Path) -> None: + """Verify set_directories resets the selection to None.""" + sidebar = ManifestSidebar() + d1 = tmp_path / 'a' + sidebar.set_directories([(d1, 'A', True)]) + sidebar.select(d1) + sidebar.set_directories([(d1, 'A', True)]) + assert sidebar.selected_path is None + + +class TestManifestSidebarSelect: + """Selection behaviour.""" + + @staticmethod + def test_select_by_path(tmp_path: Path) -> None: + """Verify selecting a path updates selected_path.""" + sidebar = ManifestSidebar() + d1 = tmp_path / 'a' + d2 = tmp_path / 'b' + sidebar.set_directories([(d1, 'A', True), (d2, 'B', True)]) + sidebar.select(d2) + assert sidebar.selected_path == d2 + + @staticmethod + def test_select_none_falls_back_to_first(tmp_path: Path) -> None: + """Verify selecting None falls back to the first item.""" + sidebar = ManifestSidebar() + d1 = tmp_path / 'a' + d2 = tmp_path / 'b' + sidebar.set_directories([(d1, 'A', True), (d2, 'B', True)]) + sidebar.select(None) + assert sidebar.selected_path == d1 + + @staticmethod + def test_select_missing_path_falls_back_to_first(tmp_path: Path) -> None: + """Verify selecting a nonexistent path falls back to the first item.""" + sidebar = ManifestSidebar() + d1 = tmp_path / 'a' + sidebar.set_directories([(d1, 'A', True)]) + sidebar.select(tmp_path / 'nonexistent') + assert sidebar.selected_path == d1 + + @staticmethod + def test_select_emits_signal(tmp_path: Path) -> None: + """Verify select emits selection_changed with the selected path.""" + sidebar = ManifestSidebar() + d1 = tmp_path / 'a' + sidebar.set_directories([(d1, 'A', True)]) + received: list[Path] = [] + sidebar.selection_changed.connect(received.append) + sidebar.select(d1) + assert received == [d1] + + +class TestManifestSidebarGetItem: + """Finding items by path.""" + + @staticmethod + def test_found(tmp_path: Path) -> None: + """Verify get_item returns the item matching the given path.""" + sidebar = ManifestSidebar() + d1 = tmp_path / 'a' + sidebar.set_directories([(d1, 'A', True)]) + item = sidebar.get_item(d1) + assert item is not None + assert item.path == d1 + + @staticmethod + def test_not_found(tmp_path: Path) -> None: + """Verify get_item returns None for an unknown path.""" + sidebar = ManifestSidebar() + sidebar.set_directories([]) + assert sidebar.get_item(tmp_path / 'x') is None + + +class TestManifestSidebarSignals: + """Signal forwarding from child items and the Add button.""" + + @staticmethod + def test_add_requested() -> None: + """Verify add_requested fires when the add button is clicked.""" + sidebar = ManifestSidebar() + received: list[bool] = [] + sidebar.add_requested.connect(lambda: received.append(True)) + sidebar._add_btn.click() + assert received == [True] + + @staticmethod + def test_remove_requested_forwarded(tmp_path: Path) -> None: + """Verify remove_requested is forwarded from child items.""" + sidebar = ManifestSidebar() + d1 = tmp_path / 'a' + sidebar.set_directories([(d1, 'A', True)]) + received: list[Path] = [] + sidebar.remove_requested.connect(received.append) + # Simulate the item's close button click + sidebar._items[0]._close_btn.click() + assert received == [d1] + + @staticmethod + def test_selection_changed_on_item_click(tmp_path: Path) -> None: + """Verify selection_changed fires when an item is clicked.""" + sidebar = ManifestSidebar() + d1 = tmp_path / 'a' + d2 = tmp_path / 'b' + sidebar.set_directories([(d1, 'A', True), (d2, 'B', True)]) + received: list[Path] = [] + sidebar.selection_changed.connect(received.append) + sidebar._items[1].mousePressEvent(None) + assert received == [d2] + + +class TestManifestSidebarSetEnabled: + """Enabling and disabling the sidebar.""" + + @staticmethod + def test_disable_add_button() -> None: + """Verify disabling the sidebar disables the add button.""" + sidebar = ManifestSidebar() + sidebar.set_enabled(False) + assert not sidebar._add_btn.isEnabled() + + @staticmethod + def test_reenable() -> None: + """Verify re-enabling the sidebar re-enables the add button.""" + sidebar = ManifestSidebar() + sidebar.set_enabled(False) + sidebar.set_enabled(True) + assert sidebar._add_btn.isEnabled() diff --git a/tests/unit/test_client_updater.py b/tests/unit/test_client_updater.py index abe3600..606ed78 100644 --- a/tests/unit/test_client_updater.py +++ b/tests/unit/test_client_updater.py @@ -92,13 +92,6 @@ def test_download_update_with_progress(client_with_updater: Client) -> None: assert result is True mock_download.assert_called_once_with(progress_cb) - @staticmethod - def test_apply_update_and_restart_without_init() -> None: - """Verify apply_update_and_restart does nothing when updater not initialized.""" - client = Client() - # Should not raise - client.apply_update_and_restart() - @staticmethod def test_apply_update_on_exit_without_init() -> None: """Verify apply_update_on_exit does nothing when updater not initialized.""" diff --git a/tests/unit/test_updater.py b/tests/unit/test_updater.py index e8d4ea7..0917c1f 100644 --- a/tests/unit/test_updater.py +++ b/tests/unit/test_updater.py @@ -320,24 +320,6 @@ def test_download_error(updater: Updater) -> None: class TestUpdaterApplyUpdate: """Tests for apply_update methods.""" - @staticmethod - def test_apply_and_restart_not_installed(updater: Updater) -> None: - """Verify apply_update_and_restart raises when not installed.""" - with ( - patch.object(Updater, 'is_installed', new_callable=PropertyMock, return_value=False), - pytest.raises(NotImplementedError, match='Velopack installs'), - ): - updater.apply_update_and_restart() - - @staticmethod - def test_apply_and_restart_no_downloaded_update(updater: Updater) -> None: - """Verify apply_update_and_restart raises when no downloaded update.""" - with ( - patch.object(Updater, 'is_installed', new_callable=PropertyMock, return_value=True), - pytest.raises(RuntimeError, match='No downloaded update'), - ): - updater.apply_update_and_restart() - @staticmethod def test_apply_on_exit_not_installed(updater: Updater) -> None: """Verify apply_update_on_exit raises when not installed.""" @@ -357,8 +339,8 @@ def test_apply_on_exit_no_downloaded_update(updater: Updater) -> None: updater.apply_update_on_exit() @staticmethod - def test_apply_on_exit_success(updater: Updater) -> None: - """Verify apply_update_on_exit schedules update.""" + def test_apply_on_exit_with_restart(updater: Updater) -> None: + """Verify apply_update_on_exit(restart=True) uses apply_updates_and_restart.""" mock_velopack_info = MagicMock(spec=velopack.UpdateInfo) updater._update_info = UpdateInfo( available=True, @@ -373,15 +355,43 @@ def test_apply_on_exit_success(updater: Updater) -> None: with ( patch.object(Updater, 'is_installed', new_callable=PropertyMock, return_value=True), patch.object(updater, '_get_velopack_manager', return_value=mock_manager), + pytest.raises(SystemExit, match='0'), ): updater.apply_update_on_exit(restart=True) - assert updater.state == UpdateState.APPLIED - mock_manager.apply_updates_and_exit.assert_called_once_with(mock_velopack_info) + assert updater.state == UpdateState.APPLYING + mock_manager.apply_updates_and_restart.assert_called_once_with(mock_velopack_info) + + @staticmethod + def test_apply_on_exit_with_restart_args(updater: Updater) -> None: + """Verify restart_args are forwarded to apply_updates_and_restart_with_args.""" + mock_velopack_info = MagicMock(spec=velopack.UpdateInfo) + updater._update_info = UpdateInfo( + available=True, + current_version=Version('1.0.0'), + latest_version=Version('2.0.0'), + _velopack_info=mock_velopack_info, + ) + updater._state = UpdateState.DOWNLOADED + + mock_manager = MagicMock(spec=velopack.UpdateManager) + + with ( + patch.object(Updater, 'is_installed', new_callable=PropertyMock, return_value=True), + patch.object(updater, '_get_velopack_manager', return_value=mock_manager), + pytest.raises(SystemExit, match='0'), + ): + updater.apply_update_on_exit(restart=True, restart_args=['--minimized']) + + assert updater.state == UpdateState.APPLYING + mock_manager.apply_updates_and_restart_with_args.assert_called_once_with( + mock_velopack_info, + ['--minimized'], + ) @staticmethod def test_apply_on_exit_no_restart(updater: Updater) -> None: - """Verify apply_update_on_exit can disable restart (note: not supported by Velopack).""" + """Verify apply_update_on_exit(restart=False) uses apply_updates_and_exit.""" mock_velopack_info = MagicMock(spec=velopack.UpdateInfo) updater._update_info = UpdateInfo( available=True, @@ -399,7 +409,7 @@ def test_apply_on_exit_no_restart(updater: Updater) -> None: ): updater.apply_update_on_exit(restart=False) - # Note: Velopack's apply_updates_and_exit doesn't support restart parameter + assert updater.state == UpdateState.APPLIED mock_manager.apply_updates_and_exit.assert_called_once_with(mock_velopack_info)