use erase() instead of clear() in per-keystroke TUI redraw loops
clear() forces a full physical screen repaint on the next refresh(); erase() just blanks the buffer content and lets curses diff against what's already on the terminal, sending only the changed cells. The main cert list and the cert-entry form both redraw on every keystroke, so clear() there meant retransmitting the whole screen on every arrow key / typed character — slow and flickery over a low-bandwidth link like a serial console. One-shot dialogs (show_confirm, _msg, _error) draw once and don't loop-redraw, so they keep clear() as-is — no benefit to changing them and it's one less thing to get wrong. Also handle curses.KEY_RESIZE explicitly in the main list loop: force one real clear() right after a detected resize, since erase()'s diff against the pre-resize screen model isn't reliable across a genuine dimension change. Screen-size changes were already picked up on the next redraw before this (draw() runs unconditionally every loop iteration), this just guarantees a clean repaint at the moment of the actual resize instead of trusting the diff engine through it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -749,7 +749,7 @@ def show_cert_form(
|
||||
|
||||
curses.curs_set(1)
|
||||
while True:
|
||||
win.clear()
|
||||
win.erase() # see show_main_screen's draw() for why not clear()
|
||||
draw_box(win, "Certificate Details")
|
||||
for name in _FORM_FIELDS:
|
||||
fy = _FORM_FIELD_Y[name]
|
||||
@@ -897,7 +897,11 @@ def show_main_screen(
|
||||
def menu_item(idx): return _MENU_ITEMS[idx - n_certs] if idx >= n_certs else None
|
||||
|
||||
def draw():
|
||||
stdscr.clear()
|
||||
# erase() (not clear()) so refresh() only transmits the cells that
|
||||
# actually changed instead of repainting the whole screen on every
|
||||
# keystroke — clear() forces a full repaint and is slow/flickery
|
||||
# over a low-bandwidth link like a serial console.
|
||||
stdscr.erase()
|
||||
sh, sw = stdscr.getmaxyx()
|
||||
|
||||
# Header bar — shows current filter mode
|
||||
@@ -967,6 +971,13 @@ def show_main_screen(
|
||||
draw()
|
||||
key = stdscr.getch()
|
||||
|
||||
if key == curses.KEY_RESIZE:
|
||||
# Force one true full repaint on the actual new dimensions —
|
||||
# erase()+refresh() diffs against the pre-resize screen model,
|
||||
# which isn't reliable across a genuine size change.
|
||||
stdscr.clear()
|
||||
continue
|
||||
|
||||
if key == 27: # Esc
|
||||
if esc_pending:
|
||||
return ScreenResult(action=Action.EXIT)
|
||||
|
||||
@@ -102,6 +102,18 @@ def test_show_cert_form_cancel():
|
||||
assert result.confirmed is False
|
||||
|
||||
|
||||
def test_show_cert_form_uses_erase_not_clear():
|
||||
# clear() forces a full-screen repaint on every refresh(); the form
|
||||
# redraws on every keystroke, so it must use erase() instead.
|
||||
stdscr = _make_stdscr()
|
||||
win = _make_win(rows=20, cols=70)
|
||||
win.getch.side_effect = [9, 27] # Tab, then Escape
|
||||
with patch("curses.newwin", return_value=win):
|
||||
show_cert_form(stdscr, cn="alice", email="alice@example.com")
|
||||
assert win.erase.called
|
||||
assert not win.clear.called
|
||||
|
||||
|
||||
def test_show_cert_form_confirm():
|
||||
"""Enter advances through the 3 fields to the Continue button; Enter on
|
||||
Continue confirms. That's 4 Enter keypresses total."""
|
||||
|
||||
@@ -103,6 +103,29 @@ def test_q_exits():
|
||||
assert result.selected_cns == []
|
||||
|
||||
|
||||
def test_draw_uses_erase_not_clear():
|
||||
# clear() forces a full-screen repaint on every refresh(); erase() lets
|
||||
# curses diff against the previous frame instead — important over a
|
||||
# low-bandwidth link like a serial console.
|
||||
stdscr = _make_stdscr()
|
||||
stdscr.getch.side_effect = [ord("q")]
|
||||
with patch("openvpncertupdate.init_colors"):
|
||||
show_main_screen(stdscr, _CERTS)
|
||||
assert stdscr.erase.called
|
||||
assert not stdscr.clear.called
|
||||
|
||||
|
||||
def test_key_resize_forces_one_full_clear():
|
||||
# A genuine terminal resize should force one true full repaint rather
|
||||
# than relying on erase()'s diff against pre-resize screen contents.
|
||||
stdscr = _make_stdscr()
|
||||
stdscr.getch.side_effect = [_curses.KEY_RESIZE, ord("q")]
|
||||
with patch("openvpncertupdate.init_colors"):
|
||||
show_main_screen(stdscr, _CERTS)
|
||||
assert stdscr.clear.call_count == 1
|
||||
assert stdscr.erase.call_count == 2 # once before the resize key, once after
|
||||
|
||||
|
||||
def test_capital_Q_exits():
|
||||
result = _run([ord("Q")])
|
||||
assert result.action == Action.EXIT
|
||||
|
||||
Reference in New Issue
Block a user