Skip to content

Fix/ctrl bracket 291#292

Merged
lassejlv merged 2 commits intomainfrom
fix/ctrl-bracket-291
Apr 1, 2026
Merged

Fix/ctrl bracket 291#292
lassejlv merged 2 commits intomainfrom
fix/ctrl-bracket-291

Conversation

@lassejlv
Copy link
Copy Markdown
Member

@lassejlv lassejlv commented Apr 1, 2026

This pull request introduces several optimizations and bug fixes to the terminal UI codebase, focusing on improving rendering efficiency, event handling, and keyboard input compatibility. The main changes include caching font objects to avoid redundant allocations, batching event draining to prevent UI stalls, fixing legacy control character handling, and optimizing row merging and cell initialization in rendering logic.

Rendering and Caching Optimizations:

  • Added cached_font_normal and cached_font_bold fields to TerminalGridPaintCache, rebuilding them only when the style changes to avoid unnecessary font object allocations. Fonts are now reused during painting, improving performance. [1] [2]
  • Optimized the merging of pane render rows by using Arc::make_mut for efficient, copy-on-write updates, reducing unnecessary cloning of row data.
  • Improved initialization of render cell rows by pre-filling with a default cell and then updating coordinates, simplifying and speeding up cell creation. [1] [2]

Event Handling Improvements:

  • Introduced a batch limit (EVENT_DRAIN_BATCH_LIMIT) for draining terminal events (2048 per frame) to prevent massive output from blocking the render thread. The event draining functions now return whether more events remain, allowing the UI to schedule additional redraws if needed. [1] [2] [3] [4] [5]

Keyboard Input Compatibility:

  • Fixed legacy mode handling for CTRL combined with @, [, `This pull request introduces several optimizations and bug fixes to the terminal UI codebase, focusing on improving rendering efficiency, event handling, and keyboard input compatibility. The main changes include caching font objects to avoid redundant allocations, batching event draining to prevent UI stalls, fixing legacy control character handling, and optimizing row merging and cell initialization in rendering logic.

Rendering and Caching Optimizations:

  • Added cached_font_normal and cached_font_bold fields to TerminalGridPaintCache, rebuilding them only when the style changes to avoid unnecessary font object allocations. Fonts are now reused during painting, improving performance. [1] [2]
  • Optimized the merging of pane render rows by using Arc::make_mut for efficient, copy-on-write updates, reducing unnecessary cloning of row data.
  • Improved initialization of render cell rows by pre-filling with a default cell and then updating coordinates, simplifying and speeding up cell creation. [1] [2]

Event Handling Improvements:

  • Introduced a batch limit (EVENT_DRAIN_BATCH_LIMIT) for draining terminal events (2048 per frame) to prevent massive output from blocking the render thread. The event draining functions now return whether more events remain, allowing the UI to schedule additional redraws if needed. [1] [2] [3] [4] [5]

Keyboard Input Compatibility:

, ], ^, and _ by mapping these to their correct ASCII control codes, improving compatibility with standard terminal control sequences. Added a test for CTRL+RightBracket. [1] [2]

Other Minor Fixes:

  • Refactored row cache resizing and clearing logic for better memory usage and clarity.
  • Updated internal tests to match new function signatures and behaviors.

These changes collectively enhance the terminal's performance, correctness, and compatibility, especially under heavy workloads and with legacy keyboard input.

Summary by CodeRabbit

Release Notes

  • New Features

    • Extended keyboard control character support for special ASCII symbols (@, [, , ], ^, _) with Ctrl modifier.
  • Performance

    • Optimized font caching to reduce redundant font object creation during rendering.
    • Improved event batching and rendering responsiveness.
    • Enhanced terminal view render cache efficiency.

lassejlv and others added 2 commits April 1, 2026 02:47
Extend CTRL key handling to support symbol keys (@, [, \, ], ^, _) using
the standard ASCII bitmask (key & 0x1F). Previously only alphabetic keys
were handled, so CTRL+] (0x1D) was silently dropped.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Cache Font objects in GridPaintCache, rebuild only on style change
- Use Arc::make_mut in merge_pane_render_rows to avoid cloning row Vecs
  when the Arc has a single owner
- Use vec![default; cols] in fallback path instead of per-cell push loop
- Batch event drain with 2048-event limit to keep UI responsive during
  massive output (e.g. cat huge_file)
- Reuse dirty_col_ranges allocation across frames via resize+fill

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@lassejlv lassejlv merged commit d614142 into main Apr 1, 2026
1 of 2 checks passed
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 1, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: bcda4a13-bb3b-4b65-9d13-bb8ae3a38a8b

📥 Commits

Reviewing files that changed from the base of the PR and between 1edfa7f and 276e2fc.

📒 Files selected for processing (5)
  • crates/terminal_ui/src/grid.rs
  • crates/terminal_ui/src/keyboard.rs
  • crates/terminal_ui/src/runtime.rs
  • src/terminal_view/mod.rs
  • src/terminal_view/render.rs

📝 Walkthrough

Walkthrough

This PR introduces performance optimizations across multiple layers: caching Font objects in the terminal grid to avoid per-call construction, extending control-key character mapping for keyboard input, implementing event draining with batch limits and a "has more" flag, and optimizing render row merging and cell initialization logic.

Changes

Cohort / File(s) Summary
Grid Font Caching
crates/terminal_ui/src/grid.rs
Adds cached cached_font_normal and cached_font_bold fields to TerminalGridPaintCache and refactors paint_with_row_cache to build Font instances only when styles change or cache entries are missing, replacing unconditional Font construction on every paint call. Also optimizes ensure_row_capacity to reuse existing allocations.
Keyboard Control Character Mapping
crates/terminal_ui/src/keyboard.rs
Extends basic_keystroke_to_input control-key conversion to map ASCII symbols (@, [, \, ], ^, _) in addition to existing alphabetic characters, using bitwise operation (byte & 0x1F). Includes regression test for the control+bracket case.
Event Batching and Draining
crates/terminal_ui/src/runtime.rs, src/terminal_view/mod.rs
Changes Terminal::drain_events to return (Vec<TerminalEvent>, bool) tuple with a has_more flag indicating pending events beyond a batch limit. Introduces EVENT_DRAIN_BATCH_LIMIT constant and updates callers to propagate the flag and force redraw when more events remain.
Render Optimization
src/terminal_view/render.rs
Refactors merge_pane_render_rows to use Arc::make_mut for in-place updates instead of rebuilding row vectors, and factors default cell initialization into a single template value in rebuild_pane_render_cache_fallback to reduce per-cell allocation overhead.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

🐰 Fonts now dance in the cache so bright,
Control keys leap with special might,
Events batch as they flow on through,
Renders merge with Arc's clever glue—
Performance hops, our work rings true!

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/ctrl-bracket-291

Comment @coderabbitai help to get the list of available commands and usage tips.

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.

1 participant