Fix code review issues: dead code, error handling, alignment, DRY defaults

src/graylog_tui/cli.py

@@ -7,7 +7,15 @@
 
 from graylog_tui import __version__
 from graylog_tui.client import GraylogAuthError, GraylogClient, GraylogError
-from graylog_tui.config import ConfigError, GraylogConfig, load_config
+from graylog_tui.config import (
+    DEFAULT_FIELDS,
+    DEFAULT_POLL_INTERVAL_MS,
+    DEFAULT_RANGE_SECONDS,
+    ConfigError,
+    ConfigFileNotFoundError,
+    GraylogConfig,
+    load_config,
+)
 from graylog_tui.modes.plain import run_plain
 
 app = typer.Typer(name="graylog-tui", add_completion=False, pretty_exceptions_enable=False)
@@ -74,9 +82,14 @@ def main(
     cfg: GraylogConfig | None = None
     try:
         cfg = load_config(config_file)
-    except ConfigError:
-        if not host:
-            pass  # will error below if host still missing
+    except ConfigFileNotFoundError as e:
+        if config_file is not None:
+            typer.echo(f"ERROR: {e}", err=True)
+            raise typer.Exit(1) from None
+        # Default config absent — CLI args may supply what's needed
+    except ConfigError as e:
+        typer.echo(f"ERROR: {e}", err=True)
+        raise typer.Exit(1) from None
 
     effective_host = host or (cfg.host if cfg else None)
     if not effective_host:
@@ -89,7 +102,7 @@ def main(
         typer.echo("ERROR: username and password must be set in config file", err=True)
         raise typer.Exit(1)
 
-    effective_poll_ms = poll_interval or (cfg.poll_interval_ms if cfg else 1000)
+    effective_poll_ms = poll_interval or (cfg.poll_interval_ms if cfg else DEFAULT_POLL_INTERVAL_MS)
     effective_insecure = insecure or (cfg.insecure if cfg else False)
 
     client = GraylogClient(
@@ -98,8 +111,8 @@ def main(
         password=effective_password,
         insecure=effective_insecure,
         query=cfg.query if cfg else "*",
-        fields=cfg.fields if cfg else "timestamp,message,source,orig_timestamp",
-        range_seconds=cfg.range_seconds if cfg else 300,
+        fields=cfg.fields if cfg else DEFAULT_FIELDS,
+        range_seconds=cfg.range_seconds if cfg else DEFAULT_RANGE_SECONDS,
     )
 
     effective_stream_id = _resolve_stream(

src/graylog_tui/client.py

@@ -5,6 +5,8 @@
 
 import httpx
 
+from graylog_tui.config import DEFAULT_FIELDS, DEFAULT_RANGE_SECONDS
+
 
 class GraylogError(Exception):
     pass
@@ -50,8 +52,8 @@ def __init__(
         password: str,
         insecure: bool = False,
         query: str = "*",
-        fields: str = "timestamp,message,source,orig_timestamp",
-        range_seconds: int = 300,
+        fields: str = DEFAULT_FIELDS,
+        range_seconds: int = DEFAULT_RANGE_SECONDS,
     ) -> None:
         self.stream_id: str | None = None
         self._query = query
@@ -92,7 +94,7 @@ def _get(self, path: str, params: dict[str, Any] | None = None) -> Any:
 
         try:
             return resp.json()
-        except Exception as e:
+        except ValueError as e:
             raise GraylogAPIError(f"Graylog returned invalid JSON: {e}") from e
 
     def fetch_streams(self) -> list[GraylogStream]:

src/graylog_tui/config.py

@@ -7,30 +7,38 @@
 
 DEFAULT_CONFIG_PATH = Path.home() / ".graylog_tui"
 
+DEFAULT_FIELDS = "timestamp,message,source,orig_timestamp"
+DEFAULT_RANGE_SECONDS = 300
+DEFAULT_POLL_INTERVAL_MS = 1000
+
 
 class ConfigError(Exception):
     pass
 
 
+class ConfigFileNotFoundError(ConfigError):
+    pass
+
+
 @dataclass
 class GraylogConfig:
     host: str | None
     username: str
     password: str
-    poll_interval_ms: int = 1000
+    poll_interval_ms: int = DEFAULT_POLL_INTERVAL_MS
     insecure: bool = False
     stream_title: str | None = None
     query: str = "*"
-    fields: str = "timestamp,message,source,orig_timestamp"
-    range_seconds: int = 300
+    fields: str = DEFAULT_FIELDS
+    range_seconds: int = DEFAULT_RANGE_SECONDS
 
 
 def load_config(path: Path | None = None) -> GraylogConfig:
     config_path = path or DEFAULT_CONFIG_PATH
     try:
         raw = config_path.read_text()
     except FileNotFoundError:
-        raise ConfigError(f"Config file not found: {config_path}") from None
+        raise ConfigFileNotFoundError(f"Config file not found: {config_path}") from None
     except OSError as e:
         raise ConfigError(f"Cannot read config file {config_path}: {e}") from e
 
@@ -51,10 +59,12 @@ def load_config(path: Path | None = None) -> GraylogConfig:
         host=str(raw_host).rstrip("/") if raw_host else None,
         username=str(data["username"]),
         password=str(data["password"]),
-        poll_interval_ms=int(data.get("poll-interval") or data.get("poll_interval_ms") or 1000),
+        poll_interval_ms=int(
+            data.get("poll-interval") or data.get("poll_interval_ms") or DEFAULT_POLL_INTERVAL_MS
+        ),
         insecure=bool(data.get("insecure", False)),
         stream_title=data.get("stream-title") or data.get("stream_title"),
         query=str(data.get("query", "*")),
-        fields=str(data.get("fields", "timestamp,message,source,orig_timestamp")),
-        range_seconds=int(data.get("range", 300)),
+        fields=str(data.get("fields", DEFAULT_FIELDS)),
+        range_seconds=int(data.get("range", DEFAULT_RANGE_SECONDS)),
     )

src/graylog_tui/formatter.py

@@ -1,16 +1,10 @@
 from __future__ import annotations
 
-import sys
-
 from rich.text import Text
 
 from graylog_tui.client import Message
 
 
-def detect_color() -> bool:
-    return sys.stdout.isatty()
-
-
 def plain_line(msg: Message, source_width: int = 0) -> str:
     source = msg.source.ljust(source_width) if source_width else msg.source
     return f"{msg.timestamp} - {source} - {msg.message}"

src/graylog_tui/tui/app.py

@@ -40,7 +40,7 @@ def compose(self) -> ComposeResult:
                 with Horizontal(id="charts-row"):
                     yield ThroughputWidget("Stream", color="red", id="stream-chart")
                     yield ThroughputWidget("Total", color="green", id="total-chart")
-                yield MessageLogWidget(id="msg-widget")
+                yield MessageLogWidget(align=self._align, id="msg-widget")
             yield StreamsWidget(id="streams-widget")
 
     def on_mount(self) -> None:
@@ -69,10 +69,11 @@ async def _poll(self) -> None:
 
         messages_result, total_tp_result, stream_tp_result = results
 
-        if isinstance(messages_result, GraylogAuthError):
-            self.notify("Authentication failed", severity="error", timeout=0)
-            self.exit()
-            return
+        for result in results:
+            if isinstance(result, GraylogAuthError):
+                self.notify("Authentication failed", severity="error", timeout=0)
+                self.exit()
+                return
 
         for result in results:
             if isinstance(result, GraylogError):

src/graylog_tui/tui/app_logs.py

@@ -30,7 +30,7 @@ def __init__(self, client: GraylogClient, poll_interval_ms: int, align: bool = F
         self._paused = False
 
     def compose(self) -> ComposeResult:
-        yield MessageLogWidget(id="msg-widget")
+        yield MessageLogWidget(align=self._align, id="msg-widget")
 
     def on_mount(self) -> None:
         widget = self.query_one(MessageLogWidget)

src/graylog_tui/tui/widgets.py

@@ -1,5 +1,7 @@
 from __future__ import annotations
 
+from collections import deque
+
 from textual.app import ComposeResult
 from textual.message import Message as TextualMessage
 from textual.widget import Widget
@@ -29,24 +31,22 @@ def __init__(self, title: str, color: str = "green", **kwargs: object) -> None:
         super().__init__(**kwargs)  # type: ignore[arg-type]
         self._title = title
         self._color = color
-        self._data: list[float] = [0.0] * 60
+        self._data: deque[float] = deque([0.0] * 60, maxlen=60)
 
     def compose(self) -> ComposeResult:
         yield Label(f"{self._title}: 0.0 msg/s", id="throughput-label")
-        yield Sparkline(data=self._data, summary_function=max)
+        yield Sparkline(data=list(self._data), summary_function=max)
 
     def push_value(self, value: float) -> None:
         self._data.append(value)
-        if len(self._data) > 60:
-            self._data = self._data[-60:]
         self.query_one("#throughput-label", Label).update(
             f"{self._title}: {value:.1f} msg/s"
         )
-        self.query_one(Sparkline).data = self._data
+        self.query_one(Sparkline).data = list(self._data)
 
     def clear(self) -> None:
-        self._data = [0.0] * 60
-        self.query_one(Sparkline).data = self._data
+        self._data = deque([0.0] * 60, maxlen=60)
+        self.query_one(Sparkline).data = list(self._data)
 
 
 class StreamsWidget(Widget):
@@ -81,9 +81,14 @@ def update_streams(self, streams: list[GraylogStream], active_id: str | None = N
         self._streams = streams
         lv = self.query_one("#stream-list", ListView)
         lv.clear()
-        for stream in streams:
+        active_index = None
+        for i, stream in enumerate(streams):
             item = ListItem(Label(stream.title[:22]), id=f"stream-{stream.id}")
             lv.append(item)
+            if stream.id == active_id:
+                active_index = i
+        if active_index is not None:
+            lv.index = active_index
 
     def on_list_view_selected(self, event: ListView.Selected) -> None:
         if event.item.id:
@@ -105,8 +110,9 @@ class MessageLogWidget(Widget):
     }
     """
 
-    def __init__(self, **kwargs: object) -> None:
+    def __init__(self, align: bool = True, **kwargs: object) -> None:
         super().__init__(**kwargs)  # type: ignore[arg-type]
+        self._align = align
         self._source_width = 0
 
     def compose(self) -> ComposeResult:
@@ -115,12 +121,13 @@ def compose(self) -> ComposeResult:
     def add_messages(self, messages: list[GraylogMessage]) -> None:
         if not messages:
             return
-        max_sw = max(len(m.source) for m in messages)
-        if max_sw > self._source_width:
-            self._source_width = max_sw
+        if self._align:
+            max_sw = max(len(m.source) for m in messages)
+            if max_sw > self._source_width:
+                self._source_width = max_sw
         log = self.query_one("#msg-log", RichLog)
         for msg in messages:
-            rendered = format_message(msg, self._source_width, color=True)
+            rendered = format_message(msg, self._source_width if self._align else 0, color=True)
             log.write(rendered)
 
     def clear(self) -> None:

tests/test_config.py

@@ -2,7 +2,7 @@
 
 import pytest
 
-from graylog_tui.config import ConfigError, load_config
+from graylog_tui.config import ConfigError, ConfigFileNotFoundError, load_config
 
 
 def write_config(tmp_path: Path, content: str) -> Path:
@@ -43,7 +43,7 @@ def test_load_config_optional_fields(tmp_path: Path) -> None:
 
 
 def test_load_config_file_not_found() -> None:
-    with pytest.raises(ConfigError, match="not found"):
+    with pytest.raises(ConfigFileNotFoundError, match="not found"):
         load_config(Path("/nonexistent/path/.graylog_tui"))