From 71eca2f194b9b72aa4f0d17041bc0c8972c5d68a Mon Sep 17 00:00:00 2001
From: Craig Williams <craig@avsdev.uk>
Date: Wed, 21 Oct 2026 15:39:36 +0100
Subject: [PATCH] Terminal logging is now coloured only on an interactive
 terminal

The terminal logger always coloured its text, so output piped or
redirected to a file, and the systemd journal, got escape codes. It
now colours only an interactive terminal.

The service's logs can therefore go to the journal: logging.file is
now optional for it, and the unit's comments and the README say so.
---
 README.md                        |  6 ++++--
 deploy/job-search.service        |  6 +++---
 internal/logging/logging.go      | 12 ++++++++----
 internal/logging/logging_test.go | 30 ++++++++++++++++++++++++++++++
 4 files changed, 45 insertions(+), 9 deletions(-)

diff --git a/README.md b/README.md
index 067b5e3..9848734 100644
--- a/README.md
+++ b/README.md
@@ -44,8 +44,10 @@ The `sqlite_fts5` tag is needed for the database's title search (see
 (the server and every enabled source's fetcher) as a `job-search` system
 user, from `/usr/local/bin/job-search` with `/etc/job-search/config.yaml`.
 Its comments give the install steps. It keeps its data in
-`/var/lib/job-search` and its log in `/var/log/job-search`, the only places
-it can write, so set `database.file` and `logging.file` there.
+`/var/lib/job-search` and any log file in `/var/log/job-search`, the only
+places it can write, so set `database.file` there. Logs go to the journal
+(`journalctl -u job-search`) unless `logging.file` is set to a file in
+`/var/log/job-search`, which then needs rotating (below).
 
 - The database isn't migrated on start: run `job-search migrate up`
   (`--create` the first time) as the `job-search` user before the first
diff --git a/deploy/job-search.service b/deploy/job-search.service
index 7e3b154..2d1ab88 100644
--- a/deploy/job-search.service
+++ b/deploy/job-search.service
@@ -12,13 +12,13 @@
 #   sudo systemctl daemon-reload
 #   sudo systemctl enable --now job-search
 #
-# /etc/job-search/config.yaml should set at least:
+# /etc/job-search/config.yaml should set the database:
 #   database:
 #       file: /var/lib/job-search/jobsearch.db
+# Logs go to the journal (journalctl -u job-search), unless logging.file is
+# set to log to a file instead, rotated with job-search.logrotate:
 #   logging:
 #       file: /var/log/job-search/job-search.log
-# Without logging.file, logs go to the journal (as coloured text). Rotate the
-# log file with job-search.logrotate:
 #   sudo install -m 644 job-search.logrotate /etc/logrotate.d/job-search
 #
 # After each upgrade, migrate the database before restarting:
diff --git a/internal/logging/logging.go b/internal/logging/logging.go
index 56c753f..7222f65 100644
--- a/internal/logging/logging.go
+++ b/internal/logging/logging.go
@@ -79,16 +79,20 @@ func Shutdown() error {
 	return err
 }
 
-// LogToTerminal sends all logging to stdout as coloured text, replacing the
-// default slog logger. When stdout is an interactive terminal, loggers
-// derived from it report so to WritesToTerminal.
+// LogToTerminal sends all logging to stdout as text, replacing the default
+// slog logger. The text is coloured only on an interactive terminal, so a
+// pipe, a file or the systemd journal gets no escape codes. On an
+// interactive terminal, loggers derived from it report so to
+// WritesToTerminal.
 func LogToTerminal() {
+	interactive := isTerminal(terminal)
 	opts := &tint.Options{
 		Level:      &logLevel,
 		TimeFormat: "*",
+		NoColor:    !interactive,
 	}
 	handler := tint.NewTextHandler(terminal, opts)
-	if isTerminal(terminal) {
+	if interactive {
 		handler = terminalHandler{
 			Handler: handler,
 		}
diff --git a/internal/logging/logging_test.go b/internal/logging/logging_test.go
index 0a2974f..56441de 100644
--- a/internal/logging/logging_test.go
+++ b/internal/logging/logging_test.go
@@ -171,6 +171,36 @@ func TestLogToTerminal(t *testing.T) {
 	}
 }
 
+// The text is coloured on an interactive terminal only.
+func TestLogToTerminalColour(t *testing.T) {
+	for _, interactive := range []bool{true, false} {
+		saveGlobals(t)
+		out, err := os.CreateTemp(t.TempDir(), "stdout")
+		if err != nil {
+			t.Fatal(err)
+		}
+		t.Cleanup(func() {
+			out.Close()
+		})
+		terminal = out
+		isTerminal = func(*os.File) bool {
+			return interactive
+		}
+
+		LogToTerminal()
+		slog.Warn("hello", "key", "value")
+
+		written, err := os.ReadFile(out.Name())
+		if err != nil {
+			t.Fatal(err)
+		}
+		coloured := strings.Contains(string(written), "\x1b[")
+		if coloured != interactive || !strings.Contains(string(written), "hello") {
+			t.Errorf("interactive %v: wrote %q", interactive, written)
+		}
+	}
+}
+
 // On an interactive terminal the handler, and loggers derived from it, report
 // that they write to the terminal; when stdout is a file or pipe they don't.
 func TestWritesToTerminal(t *testing.T) {