mirror of
https://github.com/Second-Hand-Friends/kleinanzeigen-bot.git
synced 2026-07-09 12:41:05 +02:00
feat: colorize status output (#1178)
## ℹ️ Description - Link to the related issue(s): Issue # - Adds optional color to the `kleinanzeigen-bot status` table using the existing Colorama dependency. - Keeps default status behavior local/read-only and gates ANSI output via terminal/env policy. ## 📋 Changes Summary - Adds `utils.color.should_use_color()` with `NO_COLOR` > `FORCE_COLOR` > TTY detection. - Colors only the status column while preserving plain ASCII layout and summary output. - Wires status rendering to the color gate in `_handle_status()`. - Removes legacy `colorama.init()` from `update_checker.py`; package init already enables Windows ANSI support. - Adds unit tests for color gating and colored status rendering. - No new dependencies or configuration changes. ### ⚙️ Type of Change - [ ] 🐞 Bug fix (non-breaking change which fixes an issue) - [x] ✨ New feature (adds new functionality without breaking existing usage) - [ ] 💥 Breaking change (changes that might break existing user setups, scripts, or configurations) ## ✅ Checklist - [x] I have reviewed my changes to ensure they meet the project's standards. - [x] I have tested my changes and ensured that all tests pass (`pdm run test`: 1332 passed, 4 skipped). - [x] I have formatted the code (`pdm run format`). - [x] I have verified that linting passes (`pdm run lint`). - [x] I have updated documentation where necessary. By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Status output can now be shown with optional terminal colors for improved readability. * Color display automatically follows your terminal and environment settings. * **Bug Fixes** * Status tables keep proper alignment even when colored text is enabled. * Unknown statuses remain uncolored, avoiding inconsistent display. * **Tests** * Added coverage for color handling and status rendering to improve reliability. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
@@ -11,6 +11,8 @@ from datetime import datetime # noqa: TC003 — used in runtime type annotation
|
||||
from gettext import gettext as _
|
||||
from typing import TYPE_CHECKING, Any
|
||||
|
||||
import colorama
|
||||
|
||||
from . import ad_loading
|
||||
|
||||
if TYPE_CHECKING:
|
||||
@@ -41,6 +43,28 @@ def _translate_status(status:str) -> str:
|
||||
return status
|
||||
|
||||
|
||||
# Canonical status ordering (matches precedence in :func:`compute_ad_status`).
|
||||
_STATUS_ORDER:tuple[str, ...] = ("disabled", "draft", "changed", "due", "published-local")
|
||||
|
||||
# Status → ANSI colour prefix mapping.
|
||||
# Applied only when *color* is enabled in :func:`render_status_rows`.
|
||||
_STATUS_COLORS:dict[str, str] = {
|
||||
"published-local": colorama.Fore.GREEN,
|
||||
"changed": colorama.Fore.YELLOW,
|
||||
"due": colorama.Fore.RED,
|
||||
"draft": colorama.Fore.BLUE,
|
||||
"disabled": colorama.Style.DIM,
|
||||
}
|
||||
|
||||
|
||||
def _colorize_status(status:str, text:str) -> str:
|
||||
"""Wrap *text* in ANSI colour codes for the given *status*, if a colour is mapped."""
|
||||
prefix = _STATUS_COLORS.get(status)
|
||||
if prefix is None:
|
||||
return text
|
||||
return f"{prefix}{text}{colorama.Style.RESET_ALL}"
|
||||
|
||||
|
||||
def compute_ad_status(
|
||||
ad:Ad,
|
||||
ad_cfg_orig:dict[str, Any],
|
||||
@@ -88,8 +112,15 @@ def build_status_rows(
|
||||
return rows
|
||||
|
||||
|
||||
def render_status_rows(rows:list[StatusRow]) -> str:
|
||||
"""Format status rows into an ASCII table string."""
|
||||
def render_status_rows(rows:list[StatusRow], *, color:bool = False) -> str:
|
||||
"""Format status rows into an ASCII table string.
|
||||
|
||||
Args:
|
||||
rows: Rows to render.
|
||||
color: If ``True``, apply ANSI colour codes to the status column.
|
||||
Column widths are always computed from plain (uncoloured)
|
||||
labels so that coloured and uncoloured output align identically.
|
||||
"""
|
||||
if not rows:
|
||||
return ""
|
||||
|
||||
@@ -102,13 +133,7 @@ def render_status_rows(rows:list[StatusRow]) -> str:
|
||||
# Translated labels for column width calculation.
|
||||
# Uses ``_translate_status()`` which contains explicit ``_("...")`` calls
|
||||
# that the translation-coverage scanner can find.
|
||||
translated_statuses = [
|
||||
_translate_status("disabled"),
|
||||
_translate_status("draft"),
|
||||
_translate_status("changed"),
|
||||
_translate_status("due"),
|
||||
_translate_status("published-local"),
|
||||
]
|
||||
translated_statuses = [_translate_status(s) for s in _STATUS_ORDER]
|
||||
col_status = max(len(h_status), *[len(s) for s in translated_statuses], 0)
|
||||
|
||||
# Title width is data-driven, but clamp to at least header width
|
||||
@@ -137,26 +162,30 @@ def render_status_rows(rows:list[StatusRow]) -> str:
|
||||
|
||||
for r in rows:
|
||||
label = _translate_status(r.status)
|
||||
# Pad with plain label first, then wrap in colour if enabled.
|
||||
# This keeps stripped (ANSI-free) column widths identical.
|
||||
padded = label.ljust(col_status)
|
||||
display = _colorize_status(r.status, padded) if color else padded
|
||||
lines.append(
|
||||
"| "
|
||||
+ r.ad_id.ljust(col_id)
|
||||
+ " | "
|
||||
+ r.title.ljust(col_title)
|
||||
+ " | "
|
||||
+ label.ljust(col_status)
|
||||
+ display
|
||||
+ " |"
|
||||
)
|
||||
|
||||
lines.append(sep)
|
||||
|
||||
# Summary line
|
||||
# Summary line — always plain
|
||||
counts:dict[str, int] = {}
|
||||
for r in rows:
|
||||
counts[r.status] = counts.get(r.status, 0) + 1
|
||||
|
||||
summary_parts:list[str] = [
|
||||
f"{_translate_status(label)}: {counts[label]}"
|
||||
for label in ("disabled", "draft", "changed", "due", "published-local")
|
||||
for label in _STATUS_ORDER
|
||||
if label in counts
|
||||
]
|
||||
|
||||
|
||||
@@ -20,6 +20,7 @@ from .login_flow import LoginDetectionResult
|
||||
from .model.ad_model import Ad, AdUpdateStrategy
|
||||
from .model.config_model import Config # noqa: TC001 — used at runtime, config injection
|
||||
from .published_ads import PublishedAd
|
||||
from .utils import color as _color
|
||||
from .utils import diagnostics as _diagnostics
|
||||
from .utils import loggers as _loggers
|
||||
from .utils import misc as _misc
|
||||
@@ -265,7 +266,8 @@ class KleinanzeigenBot(WebScrapingMixin): # noqa: PLR0904
|
||||
now = _misc.now()
|
||||
ads_for_status = [(abspath, ad_cfg, raw) for abspath, _relpath, ad_cfg, raw in loaded]
|
||||
rows = ad_status.build_status_rows(ads_for_status, now = now)
|
||||
output = ad_status.render_status_rows(rows)
|
||||
use_color = _color.should_use_color()
|
||||
output = ad_status.render_status_rows(rows, color = use_color)
|
||||
print(output)
|
||||
|
||||
async def _handle_publish(self) -> None:
|
||||
|
||||
@@ -8,7 +8,6 @@ import logging
|
||||
from datetime import datetime
|
||||
from typing import TYPE_CHECKING, Any
|
||||
|
||||
import colorama
|
||||
import requests
|
||||
|
||||
if TYPE_CHECKING:
|
||||
@@ -25,8 +24,6 @@ from kleinanzeigen_bot.model.update_check_state import UpdateCheckState
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
colorama.init()
|
||||
|
||||
|
||||
class UpdateChecker:
|
||||
"""Checks for updates to the bot."""
|
||||
|
||||
62
src/kleinanzeigen_bot/utils/color.py
Normal file
62
src/kleinanzeigen_bot/utils/color.py
Normal file
@@ -0,0 +1,62 @@
|
||||
# SPDX-FileCopyrightText: © Jens Bergmann and contributors
|
||||
# SPDX-License-Identifier: AGPL-3.0-or-later
|
||||
# SPDX-ArtifactOfProjectHomePage: https://github.com/Second-Hand-Friends/kleinanzeigen-bot/
|
||||
"""Terminal color gating for CLI output.
|
||||
|
||||
Provides a single pure function, :func:`should_use_color`, that
|
||||
encapsulates the project's color policy:
|
||||
|
||||
* ``NO_COLOR`` environment variable present and non-empty → ``False``
|
||||
* ``FORCE_COLOR`` environment variable present and non-empty → ``True``
|
||||
* If both are set, ``NO_COLOR`` wins.
|
||||
* Otherwise color is enabled only when *stream* is a TTY
|
||||
(``isatty()`` returns ``True``).
|
||||
|
||||
Empty-string environment variable values are treated as unset.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import sys
|
||||
from typing import IO, Mapping
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Public API
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def should_use_color(
|
||||
stream:IO[str] | None = None,
|
||||
env:Mapping[str, str] | None = None,
|
||||
) -> bool:
|
||||
"""Determine whether coloured terminal output should be used.
|
||||
|
||||
Args:
|
||||
stream: The output stream to check for TTY status.
|
||||
``None`` (default) reads ``sys.stdout`` at call time.
|
||||
Inject a fake stream for tests.
|
||||
env: Environment-variable mapping. ``None`` (default) reads
|
||||
``os.environ`` at call time. Inject for tests.
|
||||
|
||||
Returns:
|
||||
``True`` if color output is enabled per the policy above.
|
||||
"""
|
||||
_stream:IO[str] = sys.stdout if stream is None else stream
|
||||
if env is None:
|
||||
env = os.environ
|
||||
|
||||
no_color = env.get("NO_COLOR", "")
|
||||
force_color = env.get("FORCE_COLOR", "")
|
||||
|
||||
# NO_COLOR wins when both are present and non-empty.
|
||||
if no_color:
|
||||
return False
|
||||
if force_color:
|
||||
return True
|
||||
|
||||
# Fall back to TTY detection.
|
||||
try:
|
||||
return bool(_stream.isatty())
|
||||
except (OSError, AttributeError):
|
||||
return False
|
||||
@@ -5,6 +5,7 @@
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import re
|
||||
from datetime import datetime, timedelta, timezone
|
||||
from typing import Any
|
||||
from unittest.mock import patch
|
||||
@@ -26,6 +27,8 @@ from kleinanzeigen_bot.utils import xdg_paths
|
||||
|
||||
pytestmark = pytest.mark.unit
|
||||
|
||||
_ANSI_RE = re.compile(r"\x1b\[[0-9;]*m")
|
||||
|
||||
_TZ = timezone.utc
|
||||
|
||||
|
||||
@@ -244,6 +247,70 @@ class TestRenderStatusRows:
|
||||
# Summary ends with total count
|
||||
assert "total" in output.casefold() or "gesamt" in output.casefold()
|
||||
|
||||
# ------------------------------------------------------------------ #
|
||||
# Colour rendering
|
||||
# ------------------------------------------------------------------ #
|
||||
|
||||
def test_render_uncoloured_unchanged(self) -> None:
|
||||
"""Uncoloured output matches the default (color=False)."""
|
||||
rows = [
|
||||
StatusRow(title = "Item", ad_id = "1", status = "draft"),
|
||||
StatusRow(title = "Thing", ad_id = "2", status = "published-local"),
|
||||
]
|
||||
assert render_status_rows(rows) == render_status_rows(rows, color = False)
|
||||
|
||||
def test_render_coloured_contains_ansi(self) -> None:
|
||||
"""Coloured output includes ANSI escape sequences."""
|
||||
rows = [StatusRow(title = "Test", ad_id = "42", status = "changed")]
|
||||
output = render_status_rows(rows, color = True)
|
||||
assert "\x1b[" in output
|
||||
|
||||
def test_render_coloured_stripped_equals_uncoloured(self) -> None:
|
||||
"""Stripping ANSI from coloured output produces identical text to uncoloured."""
|
||||
rows = [
|
||||
StatusRow(title = "Sofa", ad_id = "-", status = "draft"),
|
||||
StatusRow(title = "Chair", ad_id = "99", status = "published-local"),
|
||||
StatusRow(title = "Table", ad_id = "55", status = "changed"),
|
||||
StatusRow(title = "Lamp", ad_id = "33", status = "due"),
|
||||
StatusRow(title = "Rug", ad_id = "11", status = "disabled"),
|
||||
]
|
||||
|
||||
plain = render_status_rows(rows, color = False)
|
||||
coloured = render_status_rows(rows, color = True)
|
||||
stripped = _ANSI_RE.sub("", coloured)
|
||||
|
||||
assert stripped == plain, (
|
||||
"Stripped coloured output must exactly match uncoloured output"
|
||||
)
|
||||
|
||||
def test_render_coloured_only_status_column(self) -> None:
|
||||
"""Only the status column contains ANSI codes; headers and separators are plain."""
|
||||
rows = [StatusRow(title = "Desk", ad_id = "7", status = "due")]
|
||||
output = render_status_rows(rows, color = True)
|
||||
|
||||
# Header row should not contain ANSI
|
||||
header_line = output.splitlines()[1]
|
||||
assert "\x1b[" not in header_line, "Header must not be coloured"
|
||||
|
||||
# Separator lines should not contain ANSI
|
||||
sep_lines = [line for line in output.splitlines() if line.startswith("+")]
|
||||
for sep in sep_lines:
|
||||
assert "\x1b[" not in sep, "Separators must not be coloured"
|
||||
|
||||
# The status column cell (last pipe segment) should contain ANSI
|
||||
data_line = output.splitlines()[3]
|
||||
cells = [c.strip() for c in data_line.split("|") if c.strip()]
|
||||
assert len(cells) >= 3, "Expected at least 3 cells (id, title, status)"
|
||||
status_cell = cells[-1]
|
||||
assert "\x1b[" in status_cell, "Status cell should be coloured"
|
||||
|
||||
def test_render_colour_only_mapped_statuses(self) -> None:
|
||||
"""Only known status values get colour; unmapped statuses are unchanged."""
|
||||
rows = [StatusRow(title = "?iss", ad_id = "0", status = "unknown")]
|
||||
plain = render_status_rows(rows, color = False)
|
||||
coloured = render_status_rows(rows, color = True)
|
||||
assert plain == coloured, "Unmapped status should not be coloured"
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# Guardrail: status does not call load_ads or open browser
|
||||
@@ -293,6 +360,7 @@ async def test_status_guardrails(
|
||||
patch.object(bot, "close_browser_session"),
|
||||
patch("kleinanzeigen_bot.ad_loading.load_ad_configs", return_value = []),
|
||||
patch("kleinanzeigen_bot.ad_status.render_status_rows", return_value = ""),
|
||||
patch("kleinanzeigen_bot.utils.color.should_use_color", return_value = False),
|
||||
):
|
||||
await bot.run(["app", "status"])
|
||||
|
||||
|
||||
137
tests/unit/test_color.py
Normal file
137
tests/unit/test_color.py
Normal file
@@ -0,0 +1,137 @@
|
||||
# SPDX-FileCopyrightText: © Jens Bergmann and contributors
|
||||
# SPDX-License-Identifier: AGPL-3.0-or-later
|
||||
# SPDX-ArtifactOfProjectHomePage: https://github.com/Second-Hand-Friends/kleinanzeigen-bot/
|
||||
"""Tests for the :mod:`kleinanzeigen_bot.utils.color` gating helper."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import sys
|
||||
from typing import Any
|
||||
|
||||
import pytest
|
||||
|
||||
from kleinanzeigen_bot.utils.color import should_use_color
|
||||
|
||||
pytestmark = pytest.mark.unit
|
||||
|
||||
|
||||
class TestShouldUseColor:
|
||||
"""``should_use_color()`` — pure env/TTY gating policy."""
|
||||
|
||||
# ------------------------------------------------------------------ #
|
||||
# TTY detection
|
||||
# ------------------------------------------------------------------ #
|
||||
|
||||
def test_tty_true_enables_color(self) -> None:
|
||||
"""TTY stream with no env overrides → colour enabled."""
|
||||
assert should_use_color(stream = _tty(True))
|
||||
|
||||
def test_tty_false_disables_color(self) -> None:
|
||||
"""Non-TTY stream with no env overrides → colour disabled."""
|
||||
assert not should_use_color(stream = _tty(False))
|
||||
|
||||
def test_tty_oserror_disables_color(self) -> None:
|
||||
"""Stream.isatty() raising OSError → colour disabled."""
|
||||
|
||||
class _Boom:
|
||||
def isatty(self) -> bool: # noqa: PLR6301
|
||||
msg = "boom"
|
||||
raise OSError(msg)
|
||||
|
||||
assert not should_use_color(stream = _Boom()) # type: ignore[arg-type]
|
||||
|
||||
def test_tty_attributeerror_disables_color(self) -> None:
|
||||
"""Stream without isatty() → colour disabled."""
|
||||
assert not should_use_color(stream = _NoIsATTY()) # type: ignore[arg-type]
|
||||
|
||||
# ------------------------------------------------------------------ #
|
||||
# NO_COLOR
|
||||
# ------------------------------------------------------------------ #
|
||||
|
||||
def test_no_color_disables_on_tty(self) -> None:
|
||||
"""NO_COLOR present and non-empty disables colour even on TTY."""
|
||||
assert not should_use_color(stream = _tty(True), env = {"NO_COLOR": "1"})
|
||||
|
||||
def test_no_color_empty_ignored(self) -> None:
|
||||
"""Empty-string NO_COLOR is treated as unset — TTY still enables."""
|
||||
assert should_use_color(stream = _tty(True), env = {"NO_COLOR": ""})
|
||||
|
||||
# ------------------------------------------------------------------ #
|
||||
# FORCE_COLOR
|
||||
# ------------------------------------------------------------------ #
|
||||
|
||||
def test_force_color_enables_on_non_tty(self) -> None:
|
||||
"""FORCE_COLOR present and non-empty enables colour even on non-TTY."""
|
||||
assert should_use_color(stream = _tty(False), env = {"FORCE_COLOR": "1"})
|
||||
|
||||
def test_force_color_empty_ignored(self) -> None:
|
||||
"""Empty-string FORCE_COLOR is treated as unset — non-TTY stays disabled."""
|
||||
assert not should_use_color(stream = _tty(False), env = {"FORCE_COLOR": ""})
|
||||
|
||||
# ------------------------------------------------------------------ #
|
||||
# Both env vars
|
||||
# ------------------------------------------------------------------ #
|
||||
|
||||
def test_both_no_color_wins(self) -> None:
|
||||
"""If both NO_COLOR and FORCE_COLOR are non-empty, NO_COLOR wins."""
|
||||
assert not should_use_color(
|
||||
stream = _tty(False),
|
||||
env = {"NO_COLOR": "1", "FORCE_COLOR": "1"},
|
||||
)
|
||||
|
||||
def test_both_no_color_wins_even_on_tty(self) -> None:
|
||||
"""NO_COLOR beats FORCE_COLOR even on TTY."""
|
||||
assert not should_use_color(
|
||||
stream = _tty(True),
|
||||
env = {"NO_COLOR": "1", "FORCE_COLOR": "1"},
|
||||
)
|
||||
|
||||
# ------------------------------------------------------------------ #
|
||||
# Defaults (no env)
|
||||
# ------------------------------------------------------------------ #
|
||||
|
||||
def test_default_env_equals_no_env(self, monkeypatch:pytest.MonkeyPatch) -> None:
|
||||
"""Calling with env=None behaves the same as empty env dict.
|
||||
|
||||
Clears ambient NO_COLOR/FORCE_COLOR so the os.environ path
|
||||
produces the same result as an explicit empty dict.
|
||||
"""
|
||||
monkeypatch.delenv("NO_COLOR", raising = False)
|
||||
monkeypatch.delenv("FORCE_COLOR", raising = False)
|
||||
assert not should_use_color(stream = _tty(False), env = {})
|
||||
assert not should_use_color(stream = _tty(False))
|
||||
|
||||
def test_lazy_default_uses_current_stdout(self, monkeypatch:pytest.MonkeyPatch) -> None:
|
||||
"""``stream=None`` reads the current ``sys.stdout`` for TTY detection."""
|
||||
monkeypatch.delenv("NO_COLOR", raising = False)
|
||||
monkeypatch.delenv("FORCE_COLOR", raising = False)
|
||||
|
||||
marker:list[bool] = []
|
||||
|
||||
class _FakeStdout:
|
||||
def isatty(self) -> bool:
|
||||
marker.append(True)
|
||||
return False
|
||||
|
||||
monkeypatch.setattr(sys, "stdout", _FakeStdout())
|
||||
assert not should_use_color(env = {})
|
||||
assert marker, "sys.stdout.isatty() should be consulted when stream=None"
|
||||
|
||||
|
||||
# ------------------------------------------------------------------ #
|
||||
# Helpers
|
||||
# ------------------------------------------------------------------ #
|
||||
|
||||
|
||||
class _NoIsATTY:
|
||||
"""Fake stream without an ``isatty`` method."""
|
||||
|
||||
|
||||
def _tty(isatty_val:bool) -> Any:
|
||||
"""Build a fake stream whose ``isatty()`` returns *isatty_val*."""
|
||||
|
||||
class _FakeTTY:
|
||||
def isatty(self) -> bool:
|
||||
return isatty_val
|
||||
|
||||
return _FakeTTY()
|
||||
Reference in New Issue
Block a user