From 3f871e550ae39057fab24cb3f1ded4a17093f23f Mon Sep 17 00:00:00 2001
From: Peter Holloway <peter.holloway@diamond.ac.uk>
Date: Fri, 12 Dec 2025 16:12:09 +0000
Subject: [PATCH] Add padding to levelname in terminal logs

It makes it easier to follow logs when the message part is aligned and
the level does not affect where the text starts.

The colouring method has been refactored at the same time to reduce the
duplication around adding styles.
---
 src/blueapi/log.py           | 29 ++++++++++-------------------
 tests/unit_tests/test_log.py | 31 ++++++++++++++++++++-----------
 2 files changed, 30 insertions(+), 30 deletions(-)

diff --git a/src/blueapi/log.py b/src/blueapi/log.py
index df1a17d40..b33431deb 100644
--- a/src/blueapi/log.py
+++ b/src/blueapi/log.py
@@ -139,34 +139,25 @@ class IBMColorBlindSafeColors(enum.Enum):
 class ColorFormatter(logging.Formatter):
     """Colors level_name of log using IBM color blind safe palette."""
 
-    def color_level_name(self, level_name: str, level_no: int) -> str:
+    def _level_colour(self, level_no: int) -> tuple[int, int, int] | None:
         match level_no:
             case logging.DEBUG:
-                return click.style(
-                    str(level_name), fg=IBMColorBlindSafeColors.ultramarine.value
-                )
+                return IBMColorBlindSafeColors.ultramarine.value
             case logging.INFO:
-                return click.style(
-                    str(level_name), fg=IBMColorBlindSafeColors.indigo.value
-                )
+                return IBMColorBlindSafeColors.indigo.value
             case logging.WARNING:
-                return click.style(
-                    str(level_name), fg=IBMColorBlindSafeColors.gold.value
-                )
+                return IBMColorBlindSafeColors.gold.value
             case logging.ERROR:
-                return click.style(
-                    str(level_name), fg=IBMColorBlindSafeColors.magenta.value
-                )
+                return IBMColorBlindSafeColors.magenta.value
             case logging.CRITICAL:
-                return click.style(
-                    str(level_name), fg=IBMColorBlindSafeColors.orange.value
-                )
-        return level_name
+                return IBMColorBlindSafeColors.orange.value
+            case _:
+                return None
 
     def formatMessage(self, record: logging.LogRecord) -> str:  # noqa: N802
         # Copy record to avoid modifying for other handlers etc.
         recordcopy = copy(record)
-        recordcopy.levelname = self.color_level_name(
-            recordcopy.levelname, recordcopy.levelno
+        recordcopy.levelname = click.style(
+            f"{recordcopy.levelname:>8}", fg=self._level_colour(recordcopy.levelno)
         )
         return super().formatMessage(recordcopy)
diff --git a/tests/unit_tests/test_log.py b/tests/unit_tests/test_log.py
index 83a1faabf..ba3325b15 100644
--- a/tests/unit_tests/test_log.py
+++ b/tests/unit_tests/test_log.py
@@ -142,43 +142,52 @@ def foo_record():
 
 
 class TestColorFormatter:
+    def _update_record(self, record: logging.LogRecord, level: int):
+        record.levelno = level
+        record.levelname = logging.getLevelName(level)
+
     def test_debug(self, color_formatter, foo_record):
-        foo_record.levelno = logging.DEBUG
+        self._update_record(foo_record, logging.DEBUG)
 
         assert (
             color_formatter.format(foo_record)
-            == "\x1b[38;2;100;143;255mINFO\x1b[0m foo"
+            == "\x1b[38;2;100;143;255m   DEBUG\x1b[0m foo"
         )
 
     def test_info(self, color_formatter, foo_record):
-        foo_record.levelno = logging.INFO
+        self._update_record(foo_record, logging.INFO)
 
         assert (
-            color_formatter.format(foo_record) == "\x1b[38;2;120;94;240mINFO\x1b[0m foo"
+            color_formatter.format(foo_record)
+            == "\x1b[38;2;120;94;240m    INFO\x1b[0m foo"
         )
 
     def test_warning(self, color_formatter, foo_record):
-        foo_record.levelno = logging.WARNING
+        self._update_record(foo_record, logging.WARNING)
 
         assert (
-            color_formatter.format(foo_record) == "\x1b[38;2;255;176;0mINFO\x1b[0m foo"
+            color_formatter.format(foo_record)
+            == "\x1b[38;2;255;176;0m WARNING\x1b[0m foo"
         )
 
     def test_error(self, color_formatter, foo_record):
-        foo_record.levelno = logging.ERROR
+        self._update_record(foo_record, logging.ERROR)
 
         assert (
-            color_formatter.format(foo_record) == "\x1b[38;2;220;38;127mINFO\x1b[0m foo"
+            color_formatter.format(foo_record)
+            == "\x1b[38;2;220;38;127m   ERROR\x1b[0m foo"
         )
 
     def test_critical(self, color_formatter, foo_record):
-        foo_record.levelno = logging.CRITICAL
+        self._update_record(foo_record, logging.CRITICAL)
 
         assert (
-            color_formatter.format(foo_record) == "\x1b[38;2;254;97;0mINFO\x1b[0m foo"
+            color_formatter.format(foo_record)
+            == "\x1b[38;2;254;97;0mCRITICAL\x1b[0m foo"
         )
 
     def test_other(self, color_formatter, foo_record):
         foo_record.levelno = -1
+        foo_record.levelname = "UNKNOWN"
 
-        assert color_formatter.format(foo_record) == "INFO foo"
+        assert color_formatter.format(foo_record) == " UNKNOWN\x1b[0m foo"