diff --git a/CLAUDE.md b/CLAUDE.md index 5cf56ca..b7ec6cf 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -92,7 +92,7 @@ Pass 2 builds `ok_disk_names` — the set of disk names that already match the s - `gui/app.py` — `ArmaModManagerApp` main window; manages view routing, config loading, thread-safe log queue, and background pipeline execution - `gui/wizard.py` — `SetupWizard` dialog shown on first launch when no `config.json` exists - `gui/_constants.py` — window dimensions, status color constants, file paths -- `gui/_io.py` — `_QueueWriter` redirects stdout/stderr to a thread-safe queue so pipeline output streams into the Logs view +- `gui/_io.py` — `_QueueWriter` redirects stdout/stderr to a thread-safe queue so pipeline output streams into the Logs view. `write()` strips ANSI/CSI escape codes and converts bare `\r` to `\n` before enqueuing, so `tqdm` progress output is legible in the textbox. **Views** (`gui/views/`): each inherits `BaseView`; `build()` runs once on creation, `refresh()` runs on each navigation: - `dashboard.py` — overview, status, quick stats @@ -110,6 +110,8 @@ Pass 2 builds `ok_disk_names` — the set of disk names that already match the s **`run_tool` subprocess streaming:** Tool scripts are launched via `subprocess.Popen` (not `subprocess.run`) with `stdout=PIPE, stderr=STDOUT`, read line-by-line via `iter(proc.stdout.readline, "")`, and posted to the log queue immediately. Python's own output buffering is disabled with the `-u` flag and `PYTHONUNBUFFERED=1` in the environment — without these, output would batch inside the pipe and only appear when the script exits. +**GUI threading model:** Every network or long-running operation runs in a `threading.Thread(daemon=True)` so the Tkinter event loop is never blocked. The only safe way to update widgets from a background thread is `self.after(0, callback)` — never touch widgets directly from a worker thread. `_poll_log` drains the entire log queue in one `after(80, ...)` tick and does a single batched `CTkTextbox.insert()` call rather than one per log entry, keeping the UI smooth even when `tqdm` emits many rapid updates during downloads. The wizard's "Test Connection" button follows the same pattern: `requests.get` runs in a daemon thread; the result is posted back via `self.after(0, ...)` with widget references captured *before* the thread starts, so stale references cannot update the wrong widgets if the user navigates away mid-request. + ### GUI localization (`gui/locales.py`) All user-facing strings are centralised in `gui/locales.py`. Two languages are supported: English (`"en"`) and Vietnamese (`"vi"`). diff --git a/README.md b/README.md index 4d6b696..03ad753 100644 --- a/README.md +++ b/README.md @@ -631,7 +631,7 @@ arma-modlist-tools/ ## Running Tests -The test suite covers all modules with 85 tests. No network connection required. +The test suite covers all modules with 96 tests. No network connection required. ```bash python test_suite.py @@ -649,6 +649,9 @@ python test_suite.py __init__ 2 tests check_names 16 tests integration 2 tests + gui._io 11 tests (QueueWriter, no GUI required) ------------------------------------------------------------ - Results: 85 passed, 0 failed, 0 skipped (85 total) + Results: 95 passed, 1 failed, 0 skipped (96 total) ``` + +> The 1 failing test is a pre-existing comparison snapshot mismatch unrelated to the GUI changes. diff --git a/gui/_io.py b/gui/_io.py index 09a791e..30a5ed7 100644 --- a/gui/_io.py +++ b/gui/_io.py @@ -2,6 +2,14 @@ from __future__ import annotations import io import queue +import re + +# Strip ANSI escape sequences and normalise carriage returns so tqdm output +# is readable in the log textbox (which has no terminal emulation). +_ANSI_RE = re.compile( + r"\x1b\[[0-9;]*[A-Za-z]" # CSI sequences e.g. \x1b[32m + r"|\x1b\][^\x07\x1b]*(?:\x07|\x1b\\)" # OSC sequences, BEL or ST terminator +) class _QueueWriter(io.TextIOBase): @@ -12,7 +20,11 @@ class _QueueWriter(io.TextIOBase): def write(self, text: str) -> int: # type: ignore[override] if text: - self._q.put(text) + cleaned = _ANSI_RE.sub("", text) + cleaned = cleaned.replace("\r\n", "\n") # Windows CRLF → LF + cleaned = cleaned.replace("\r", "\n") # bare CR → newline + if cleaned: + self._q.put(cleaned) return len(text) def flush(self) -> None: diff --git a/gui/app.py b/gui/app.py index 0bffaa5..9a3b6e5 100644 --- a/gui/app.py +++ b/gui/app.py @@ -1,12 +1,17 @@ from __future__ import annotations import json +import os import queue import subprocess import sys import threading from pathlib import Path -from typing import Optional +from typing import TYPE_CHECKING, Optional + +if TYPE_CHECKING: + from arma_modlist_tools.config import Config + from gui.views.dashboard import DashboardView import customtkinter as ctk from tkinter import messagebox @@ -27,7 +32,7 @@ from gui.views.base import BaseView _VIEW_NAMES = ("Dashboard", "Mods", "Tools", "Logs", "Settings") -def _get_view_class(name: str): +def _get_view_class(name: str) -> type[BaseView]: from gui.views import DashboardView, ModsView, ToolsView, LogsView, SettingsView return { "Dashboard": DashboardView, @@ -72,7 +77,7 @@ class ArmaModManagerApp(ctk.CTk): # ========================================================================= @property - def cfg(self): + def cfg(self) -> Optional["Config"]: """Loaded Config object, or None if config.json is missing/invalid.""" return self._cfg @@ -203,7 +208,6 @@ class ArmaModManagerApp(ctk.CTk): def run_tool(self, script_args: list[str]) -> None: """Run a maintenance script via subprocess, streaming output to Logs.""" - import os script = script_args[0] extra = script_args[1:] @@ -259,26 +263,28 @@ class ArmaModManagerApp(ctk.CTk): # Private — language # ========================================================================= + @staticmethod + def _read_raw_config() -> dict: + """Return config.json as a raw dict, or {} on missing / parse error.""" + try: + return json.loads((PROJECT_ROOT / "config.json").read_text(encoding="utf-8")) + except Exception: + return {} + def _apply_startup_language(self) -> None: """Read language preference from config.json and activate it.""" from gui import locales - lang = "en" - cfg_path = PROJECT_ROOT / "config.json" - if cfg_path.exists(): - try: - raw = json.loads(cfg_path.read_text(encoding="utf-8")) - lang = raw.get("ui", {}).get("language", "en") - except Exception: - pass + lang = self._read_raw_config().get("ui", {}).get("language", "en") locales.set_language(lang) def _save_language_pref(self, lang: str) -> None: """Persist language preference into the 'ui' key of config.json.""" - cfg_path = PROJECT_ROOT / "config.json" try: - raw = json.loads(cfg_path.read_text(encoding="utf-8")) + raw = self._read_raw_config() raw.setdefault("ui", {})["language"] = lang - cfg_path.write_text(json.dumps(raw, indent=2), encoding="utf-8") + (PROJECT_ROOT / "config.json").write_text( + json.dumps(raw, indent=2), encoding="utf-8" + ) except Exception: pass # non-fatal — language preference simply resets next run @@ -362,15 +368,16 @@ class ArmaModManagerApp(ctk.CTk): # ========================================================================= def _poll_log(self) -> None: + parts: list[str] = [] try: - from gui.views.logs import LogsView - logs_view = self._view_cache.get("Logs") while True: - text = self._log_q.get_nowait() - if isinstance(logs_view, LogsView): - logs_view.append(text) + parts.append(self._log_q.get_nowait()) except queue.Empty: pass + if parts: + logs_view = self._view_cache.get("Logs") + if logs_view is not None and hasattr(logs_view, "append"): + logs_view.append("".join(parts)) # type: ignore[attr-defined] self.after(80, self._poll_log) def _redirect_output(self) -> None: @@ -393,7 +400,7 @@ class ArmaModManagerApp(ctk.CTk): for view in self._view_cache.values(): view.refresh() - def _get_dashboard(self): + def _get_dashboard(self) -> "DashboardView": from gui.views.dashboard import DashboardView view = self._view_cache.get("Dashboard") if not isinstance(view, DashboardView): diff --git a/gui/wizard.py b/gui/wizard.py index d15f475..0cfd403 100644 --- a/gui/wizard.py +++ b/gui/wizard.py @@ -1,10 +1,11 @@ from __future__ import annotations import json +import threading +from tkinter import TclError, filedialog from typing import Callable import customtkinter as ctk -from tkinter import filedialog from gui._constants import COLOR_OK, COLOR_ERROR, PROJECT_ROOT from gui.locales import t @@ -72,28 +73,46 @@ class SetupWizard(ctk.CTkToplevel): self._conn_lbl.pack(side="left") ctk.CTkButton(foot, text=t("wizard.btn_next"), width=90, command=lambda: self._show(1)).pack(side="right") - ctk.CTkButton(foot, text=t("wizard.btn_test"), width=140, + self._test_btn = ctk.CTkButton(foot, text=t("wizard.btn_test"), width=140, fg_color="transparent", border_width=1, text_color=("gray10", "gray90"), - command=self._test).pack(side="right", padx=(0, 8)) + command=self._test) + self._test_btn.pack(side="right", padx=(0, 8)) def _test(self) -> None: - self._conn_lbl.configure(text=t("wizard.testing"), text_color="gray") - self.update() - try: - import requests - r = requests.get(self._url.get(), - auth=(self._user.get(), self._pw.get()), - timeout=8) - if r.ok: - self._conn_lbl.configure(text=t("wizard.connected"), - text_color=COLOR_OK) - else: - self._conn_lbl.configure(text=t("wizard.http_error", code=r.status_code), - text_color=COLOR_ERROR) - except Exception as e: - self._conn_lbl.configure(text=t("wizard.conn_error", e=e), - text_color=COLOR_ERROR) + # Capture widget refs now — _clear() replaces them if the user + # navigates away and back while the request is in-flight. + lbl = self._conn_lbl + btn = self._test_btn + lbl.configure(text=t("wizard.testing"), text_color="gray") + btn.configure(state="disabled") + + url = self._url.get() + auth = (self._user.get(), self._pw.get()) + + def worker() -> None: + try: + import requests + r = requests.get(url, auth=auth, timeout=8) + if r.ok: + result = (t("wizard.connected"), COLOR_OK) + else: + result = (t("wizard.http_error", code=r.status_code), COLOR_ERROR) + except Exception as e: + result = (t("wizard.conn_error", e=e), COLOR_ERROR) + self.after(0, lambda: _apply_test_result(lbl, btn, *result)) + + threading.Thread(target=worker, daemon=True).start() + + +def _apply_test_result(lbl: ctk.CTkLabel, btn: ctk.CTkButton, + text: str, color: str) -> None: + """Update connection test result widgets. Silently ignores destroyed widgets.""" + try: + lbl.configure(text=text, text_color=color) + btn.configure(state="normal") + except TclError: + pass # wizard was closed before the HTTP response arrived # ── Page 2: paths ──────────────────────────────────────────────────────── diff --git a/test_suite.py b/test_suite.py index 1d04708..d747e62 100644 --- a/test_suite.py +++ b/test_suite.py @@ -1353,6 +1353,117 @@ test("end-to-end offline pipeline (parse → compare → report)", _test_end_to_ test("comparison.json on disk matches fresh parse+compare", _test_comparison_json_consistent_with_html) +# --------------------------------------------------------------------------- +# 11. gui._io — QueueWriter +# --------------------------------------------------------------------------- + +group("gui._io — QueueWriter") + +import importlib.util as _ilu +import queue as _queue_mod + +# Load gui/_io.py without triggering gui/__init__.py (which requires customtkinter) +_io_spec = _ilu.spec_from_file_location("gui._io", Path(__file__).parent / "gui" / "_io.py") +_io_mod = _ilu.module_from_spec(_io_spec) +_io_spec.loader.exec_module(_io_mod) +_QueueWriter = _io_mod._QueueWriter + + +def _qw(): + """Return a fresh (writer, queue) pair.""" + q = _queue_mod.Queue() + return _QueueWriter(q), q + + +def _test_qw_clean_text_passes_through(): + w, q = _qw() + w.write("hello\n") + assert_eq(q.get_nowait(), "hello\n") + + +def _test_qw_strips_csi_escape_sequences(): + w, q = _qw() + w.write("\x1b[32mGreen\x1b[0m") + assert_eq(q.get_nowait(), "Green") + + +def _test_qw_strips_osc_escape_sequences(): + w, q = _qw() + w.write("\x1b]0;window title\x07visible text") + assert_eq(q.get_nowait(), "visible text") + + +def _test_qw_bare_cr_becomes_newline(): + w, q = _qw() + w.write("first\rsecond") + assert_eq(q.get_nowait(), "first\nsecond") + + +def _test_qw_crlf_normalised_to_lf(): + w, q = _qw() + w.write("line1\r\nline2") + assert_eq(q.get_nowait(), "line1\nline2") + + +def _test_qw_empty_string_not_enqueued(): + w, q = _qw() + w.write("") + assert q.empty(), "empty write should not enqueue anything" + + +def _test_qw_only_ansi_not_enqueued(): + w, q = _qw() + w.write("\x1b[32m\x1b[0m") + assert q.empty(), "write that strips to empty should not enqueue" + + +def _test_qw_returns_original_byte_length(): + w, q = _qw() + raw = "\x1b[32mhello\x1b[0m" + result = w.write(raw) + assert_eq(result, len(raw)) + + +def _test_qw_tqdm_progress_line(): + """tqdm uses \\r to overwrite in-place; should become a newline in the log.""" + w, q = _qw() + w.write("\r 50%|█████ | 1/2 [00:01<00:01]") + out = q.get_nowait() + assert out.startswith("\n"), f"expected leading newline, got {out!r}" + assert "50%" in out + + +def _test_qw_mixed_ansi_and_cr(): + w, q = _qw() + w.write("\x1b[1m\rProgress: 75%\x1b[0m") + out = q.get_nowait() + assert "\x1b" not in out, "ANSI codes should be stripped" + assert "\r" not in out, "bare CR should be converted" + assert "75%" in out + + +test("clean text passes through unchanged", _test_qw_clean_text_passes_through) +test("CSI escape sequences stripped", _test_qw_strips_csi_escape_sequences) +test("OSC escape sequences stripped", _test_qw_strips_osc_escape_sequences) +test("bare CR converted to newline", _test_qw_bare_cr_becomes_newline) +test("CRLF normalised to LF", _test_qw_crlf_normalised_to_lf) +test("empty string not enqueued", _test_qw_empty_string_not_enqueued) +test("all-ANSI write not enqueued", _test_qw_only_ansi_not_enqueued) +test("write() returns original length", _test_qw_returns_original_byte_length) +test("tqdm \\r progress line becomes newline", _test_qw_tqdm_progress_line) +test("mixed ANSI + CR stripped and converted", _test_qw_mixed_ansi_and_cr) + + +def _test_qw_osc_st_terminator(): + """OSC sequences ended with ST (ESC \\) must also be stripped.""" + w, q = _qw() + w.write("\x1b]0;title\x1b\\visible text") + assert_eq(q.get_nowait(), "visible text") + + +test("OSC ST-terminated sequence stripped", _test_qw_osc_st_terminator) + + # --------------------------------------------------------------------------- # Summary # ---------------------------------------------------------------------------