Skip to content

feat: add cursor blink animation to embedded terminal#380

Merged
Wirasm merged 2 commits into
mainfrom
kild/314-cursor-blink
Feb 11, 2026
Merged

feat: add cursor blink animation to embedded terminal#380
Wirasm merged 2 commits into
mainfrom
kild/314-cursor-blink

Conversation

@Wirasm

@Wirasm Wirasm commented Feb 11, 2026

Copy link
Copy Markdown
Owner

Summary

  • Add 530ms on/off cursor blink to the kild-ui embedded terminal
  • Blink resets on every keystroke so the cursor stays solid while typing
  • Unfocused cursors remain static (thin bar, no blinking)
  • Uses epoch-based timer invalidation to prevent stale timers from toggling state

Implementation

  • terminal_view.rs: Added cursor_visible, blink_epoch, _blink_task fields. Spawns blink timer in from_terminal(). reset_blink() called on every keystroke.
  • terminal_element.rs: Added cursor_visible parameter. Skips cursor preparation in prepaint when hidden.
  • git/health.rs: Fixed pre-existing clippy error (moved test-only imports into #[cfg(test)])

Test plan

  • cargo fmt --check && cargo clippy --all -- -D warnings passes
  • cargo test --all passes (1676 tests, 0 failures)
  • cargo build --all succeeds
  • Manual: cursor blinks at ~530ms when terminal is focused
  • Manual: cursor stays solid while typing, restarts blink after pause
  • Manual: unfocused cursor shows static thin bar (no blinking)

Closes #314

@Wirasm

Wirasm commented Feb 11, 2026

Copy link
Copy Markdown
Owner Author

PR Review Summary

Critical Issues (1 found)

Agent Issue Location
code-reviewer Initial blink timer can perform one stale toggle after reset_blink() — timer waits 530ms before checking epoch, so if a keystroke fires during the wait, the old timer still executes one toggle terminal_view.rs:75-92

Important Issues (3 found)

Agent Issue Location
code-reviewer Blink timer loop duplicated verbatim between from_terminal() and reset_blink() — extract to shared method terminal_view.rs:75-92, 115-131
comment-analyzer Comment says "always show the thin bar" but cursor_visible only controls whether cursor renders at all, not its style (thin bar is from has_focus in element) terminal_view.rs:231
silent-failure-hunter _ => break swallows error details — should match codebase pattern (main_view.rs:187-197) with explicit Ok(false) / Err(e) arms and tracing::debug! logging terminal_view.rs:79-91, 118-130

Strengths

  • Epoch-based timer invalidation is a clean pattern for cancelling stale timers
  • Keystroke reset keeps cursor solid while typing (good UX)
  • Unfocused cursor logic correctly delegates style to element layer
  • health.rs clippy fix properly scopes test-only imports

Verdict

NEEDS FIXES — 1 critical (timer race), 3 important (duplication, misleading comment, silent error swallowing)

Recommended Actions

  1. Fix timer race: check epoch before first wait, or accept the one-toggle latency is negligible (530ms max)
  2. Extract spawn_blink_timer(cx, epoch) -> Task<()> to deduplicate
  3. Fix comment to: // Focused cursors blink; unfocused cursors are always visible (rendered as thin bar in element).
  4. Split _ => break into Ok(false) + Err(e) with tracing::debug! for consistency with main_view.rs

Add a 530ms on/off cursor blink to the kild-ui terminal. The blink
timer lives in TerminalView using an epoch-based invalidation pattern:
each keystroke resets the blink so the cursor stays solid while typing.
Unfocused cursors remain static (no blinking).

Also fix pre-existing unused imports in git/health.rs (moved test-only
imports into #[cfg(test)] module).

Closes #314
- Extract duplicated blink timer loop into `spawn_blink_timer()` method
- Split catch-all `_ => break` into explicit `Ok(false)` (stale epoch)
  and `Err(e)` (view dropped) arms with debug-level tracing
- Fix misleading "thin bar" comment to accurately describe cursor_visible
  vs has_focus responsibility split
@Wirasm Wirasm force-pushed the kild/314-cursor-blink branch from 9daea28 to 3e5f60a Compare February 11, 2026 15:35
@Wirasm Wirasm merged commit 3575e51 into main Feb 11, 2026
5 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

kild-ui: cursor blink animation for embedded terminals

1 participant