From 17abe31650763d5068837a3b59ce424459b7d226 Mon Sep 17 00:00:00 2001 From: Vlad Doloman Date: Wed, 8 Jul 2026 16:07:50 +0300 Subject: [PATCH] fix invisible TUI cursor/buttons on serial consoles with broken color pairs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit init_colors() now survives a terminal rejecting every init_pair() call (some serial-console terminfo entries claim start_color()/COLORS support but can't actually set custom pairs), but that exposed a follow-on bug: the list cursor and the cert-form Continue/Cancel buttons indicated selection purely via the COLOR_SELECTED color pair, so once that pair silently no-ops the current row/button becomes indistinguishable from an unselected one. Add curses.A_REVERSE alongside the color pair for the selected state — a plain video attribute that doesn't depend on custom-pair support, so the cursor stays visible either way. Co-Authored-By: Claude Sonnet 5 --- openvpncertupdate.py | 36 +++++++++++++++++++++++++++--------- tests/test_main_screen.py | 14 ++++++++++++++ tests/test_widgets.py | 12 ++++++++++++ 3 files changed, 53 insertions(+), 9 deletions(-) diff --git a/openvpncertupdate.py b/openvpncertupdate.py index 316c48c..248cba8 100644 --- a/openvpncertupdate.py +++ b/openvpncertupdate.py @@ -541,12 +541,22 @@ def init_colors() -> None: # (terminfo has no "op"/"AX" capability); fall back to explicit black. bg = curses.COLOR_BLACK disabled_fg = 8 if curses.COLORS > 8 else curses.COLOR_WHITE - curses.init_pair(COLOR_NORMAL, curses.COLOR_WHITE, bg) - curses.init_pair(COLOR_SELECTED, curses.COLOR_BLACK, curses.COLOR_CYAN) - curses.init_pair(COLOR_DISABLED, disabled_fg, bg) - curses.init_pair(COLOR_TITLE, curses.COLOR_WHITE, bg) - curses.init_pair(COLOR_ERROR, curses.COLOR_RED, bg) - curses.init_pair(COLOR_SUCCESS, curses.COLOR_GREEN, bg) + # Some serial-console terminfo entries advertise start_color()/COLORS + # support but still can't actually set custom pairs (e.g. COLOR_PAIRS is + # too small). Set each pair independently so a rejected pair just falls + # back to the terminal's default rendering instead of crashing the app. + for pair_id, fg, pair_bg in ( + (COLOR_NORMAL, curses.COLOR_WHITE, bg), + (COLOR_SELECTED, curses.COLOR_BLACK, curses.COLOR_CYAN), + (COLOR_DISABLED, disabled_fg, bg), + (COLOR_TITLE, curses.COLOR_WHITE, bg), + (COLOR_ERROR, curses.COLOR_RED, bg), + (COLOR_SUCCESS, curses.COLOR_GREEN, bg), + ): + try: + curses.init_pair(pair_id, fg, pair_bg) + except curses.error: + pass def draw_box(win, title: str = "") -> None: @@ -752,9 +762,10 @@ def show_cert_form( ("Continue", _BTN_CONTINUE, _btn_x_cont), ("Cancel", _BTN_CANCEL, _btn_x_canc), ): - btn_attr = (curses.color_pair(COLOR_SELECTED) - if current == btn_id - else curses.color_pair(COLOR_NORMAL)) + if current == btn_id: + btn_attr = curses.color_pair(COLOR_SELECTED) | curses.A_REVERSE + else: + btn_attr = curses.color_pair(COLOR_NORMAL) try: win.addstr(_BTN_Y, bx, f"[ {btn_label} ]", btn_attr | curses.A_BOLD) except curses.error: @@ -905,6 +916,11 @@ def show_main_screen( break is_cur = (i == cursor) sel_attr = curses.color_pair(COLOR_SELECTED if is_cur else COLOR_NORMAL) + if is_cur: + # A_REVERSE keeps the cursor visible even when the terminal + # (e.g. a serial console) silently rejects the COLOR_SELECTED + # pair and this color_pair() call is a no-op. + sel_attr |= curses.A_REVERSE if is_cert(i): cert = certs[i] @@ -922,6 +938,8 @@ def show_main_screen( notice = " ← clear all checkboxes to use" line = f" {item}{notice}" attr = curses.color_pair(COLOR_SELECTED if is_cur else COLOR_DISABLED) + if is_cur: + attr |= curses.A_REVERSE else: line = f" {item}" attr = sel_attr diff --git a/tests/test_main_screen.py b/tests/test_main_screen.py index b972bbc..0530463 100644 --- a/tests/test_main_screen.py +++ b/tests/test_main_screen.py @@ -107,6 +107,20 @@ def test_capital_Q_exits(): assert result.action == Action.EXIT +def test_cursor_row_has_reverse_attribute(): + # A_REVERSE must mark the cursor row so it's still visible even when a + # terminal (e.g. a serial console) silently rejects the COLOR_SELECTED + # pair and color_pair() becomes a no-op. + stdscr = _make_stdscr() + stdscr.getch.side_effect = [ord("q")] + with patch("openvpncertupdate.init_colors"): + show_main_screen(stdscr, _CERTS) # cursor defaults to row 0 ("bob") + bob_calls = [c for c in stdscr.addstr.call_args_list if "bob" in str(c.args[2])] + alice_calls = [c for c in stdscr.addstr.call_args_list if "alice" in str(c.args[2])] + assert bob_calls and bob_calls[0].args[3] & _curses.A_REVERSE + assert alice_calls and not (alice_calls[0].args[3] & _curses.A_REVERSE) + + # --------------------------------------------------------------------------- # Enter on cert → RENEW_SELECTED # --------------------------------------------------------------------------- diff --git a/tests/test_widgets.py b/tests/test_widgets.py index bdeff86..b0d4b7d 100644 --- a/tests/test_widgets.py +++ b/tests/test_widgets.py @@ -82,3 +82,15 @@ def test_init_colors_falls_back_on_serial_console(): disabled = next(c for c in calls if c[0] == COLOR_DISABLED) assert normal[2] == _curses.COLOR_BLACK assert disabled[1:] == (_curses.COLOR_WHITE, _curses.COLOR_BLACK) + +def test_init_colors_survives_init_pair_rejecting_every_pair(): + # Some serial-console terminfo entries claim start_color()/COLORS + # support but reject every init_pair() call (e.g. COLOR_PAIRS too + # small). init_colors() must not raise -- the app should still start, + # just without custom colors. + with patch.object(_curses, "start_color"), \ + patch.object(_curses, "use_default_colors"), \ + patch.object(_curses, "init_pair", + side_effect=_curses.error("init_pair() returned ERR")), \ + patch.object(_curses, "COLORS", 256, create=True): + init_colors() # must not raise