fix: smooth GUI during pipeline downloads and harden wizard connection test
GUI log batching (_poll_log now drains queue into a single CTkTextbox.insert
call per 80 ms tick instead of N calls, each with see("end") scroll).
_QueueWriter strips ANSI/CSI escape codes and bare \r before enqueuing so
tqdm progress output is legible in the log textbox. OSC sequences terminated
by both BEL (\x07) and ST (\x1b\) are handled.
Wizard "Test Connection" moved off the main thread: requests.get runs in a
daemon thread; result posted back via after(0, ...). Widget refs captured
before thread launch to prevent stale updates if user navigates away. Bare
except narrowed to TclError (destroyed-widget guard only).
Code quality: import os moved to module level in app.py; _read_raw_config()
helper extracted to deduplicate dual raw config.json reads; return type
annotations added to _get_view_class, _get_dashboard, and cfg property.
Tests: 11 new unit tests for _QueueWriter (RED -> GREEN on OSC-ST fix).
This commit is contained in:
@@ -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"`).
|
||||
|
||||
@@ -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.
|
||||
|
||||
14
gui/_io.py
14
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:
|
||||
|
||||
49
gui/app.py
49
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):
|
||||
|
||||
@@ -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()
|
||||
# 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(self._url.get(),
|
||||
auth=(self._user.get(), self._pw.get()),
|
||||
timeout=8)
|
||||
r = requests.get(url, auth=auth, timeout=8)
|
||||
if r.ok:
|
||||
self._conn_lbl.configure(text=t("wizard.connected"),
|
||||
text_color=COLOR_OK)
|
||||
result = (t("wizard.connected"), COLOR_OK)
|
||||
else:
|
||||
self._conn_lbl.configure(text=t("wizard.http_error", code=r.status_code),
|
||||
text_color=COLOR_ERROR)
|
||||
result = (t("wizard.http_error", code=r.status_code), COLOR_ERROR)
|
||||
except Exception as e:
|
||||
self._conn_lbl.configure(text=t("wizard.conn_error", e=e),
|
||||
text_color=COLOR_ERROR)
|
||||
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 ────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
111
test_suite.py
111
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
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user