fix invisible TUI cursor/buttons on serial consoles with broken color pairs
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 <noreply@anthropic.com>
This commit is contained in:
@@ -541,12 +541,22 @@ def init_colors() -> None:
|
|||||||
# (terminfo has no "op"/"AX" capability); fall back to explicit black.
|
# (terminfo has no "op"/"AX" capability); fall back to explicit black.
|
||||||
bg = curses.COLOR_BLACK
|
bg = curses.COLOR_BLACK
|
||||||
disabled_fg = 8 if curses.COLORS > 8 else curses.COLOR_WHITE
|
disabled_fg = 8 if curses.COLORS > 8 else curses.COLOR_WHITE
|
||||||
curses.init_pair(COLOR_NORMAL, curses.COLOR_WHITE, bg)
|
# Some serial-console terminfo entries advertise start_color()/COLORS
|
||||||
curses.init_pair(COLOR_SELECTED, curses.COLOR_BLACK, curses.COLOR_CYAN)
|
# support but still can't actually set custom pairs (e.g. COLOR_PAIRS is
|
||||||
curses.init_pair(COLOR_DISABLED, disabled_fg, bg)
|
# too small). Set each pair independently so a rejected pair just falls
|
||||||
curses.init_pair(COLOR_TITLE, curses.COLOR_WHITE, bg)
|
# back to the terminal's default rendering instead of crashing the app.
|
||||||
curses.init_pair(COLOR_ERROR, curses.COLOR_RED, bg)
|
for pair_id, fg, pair_bg in (
|
||||||
curses.init_pair(COLOR_SUCCESS, curses.COLOR_GREEN, bg)
|
(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:
|
def draw_box(win, title: str = "") -> None:
|
||||||
@@ -752,9 +762,10 @@ def show_cert_form(
|
|||||||
("Continue", _BTN_CONTINUE, _btn_x_cont),
|
("Continue", _BTN_CONTINUE, _btn_x_cont),
|
||||||
("Cancel", _BTN_CANCEL, _btn_x_canc),
|
("Cancel", _BTN_CANCEL, _btn_x_canc),
|
||||||
):
|
):
|
||||||
btn_attr = (curses.color_pair(COLOR_SELECTED)
|
if current == btn_id:
|
||||||
if current == btn_id
|
btn_attr = curses.color_pair(COLOR_SELECTED) | curses.A_REVERSE
|
||||||
else curses.color_pair(COLOR_NORMAL))
|
else:
|
||||||
|
btn_attr = curses.color_pair(COLOR_NORMAL)
|
||||||
try:
|
try:
|
||||||
win.addstr(_BTN_Y, bx, f"[ {btn_label} ]", btn_attr | curses.A_BOLD)
|
win.addstr(_BTN_Y, bx, f"[ {btn_label} ]", btn_attr | curses.A_BOLD)
|
||||||
except curses.error:
|
except curses.error:
|
||||||
@@ -905,6 +916,11 @@ def show_main_screen(
|
|||||||
break
|
break
|
||||||
is_cur = (i == cursor)
|
is_cur = (i == cursor)
|
||||||
sel_attr = curses.color_pair(COLOR_SELECTED if is_cur else COLOR_NORMAL)
|
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):
|
if is_cert(i):
|
||||||
cert = certs[i]
|
cert = certs[i]
|
||||||
@@ -922,6 +938,8 @@ def show_main_screen(
|
|||||||
notice = " ← clear all checkboxes to use"
|
notice = " ← clear all checkboxes to use"
|
||||||
line = f" {item}{notice}"
|
line = f" {item}{notice}"
|
||||||
attr = curses.color_pair(COLOR_SELECTED if is_cur else COLOR_DISABLED)
|
attr = curses.color_pair(COLOR_SELECTED if is_cur else COLOR_DISABLED)
|
||||||
|
if is_cur:
|
||||||
|
attr |= curses.A_REVERSE
|
||||||
else:
|
else:
|
||||||
line = f" {item}"
|
line = f" {item}"
|
||||||
attr = sel_attr
|
attr = sel_attr
|
||||||
|
|||||||
@@ -107,6 +107,20 @@ def test_capital_Q_exits():
|
|||||||
assert result.action == Action.EXIT
|
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
|
# Enter on cert → RENEW_SELECTED
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
|
|||||||
@@ -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)
|
disabled = next(c for c in calls if c[0] == COLOR_DISABLED)
|
||||||
assert normal[2] == _curses.COLOR_BLACK
|
assert normal[2] == _curses.COLOR_BLACK
|
||||||
assert disabled[1:] == (_curses.COLOR_WHITE, _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
|
||||||
|
|||||||
Reference in New Issue
Block a user