PR #114 swaps the dialog text input from a simple ArrayList-backed implementation to vaxis's gap-buffer TextInput. The swap changes three public API contracts. This document analyzes each change, explains why it happened, and asks whether there's a better way.
Before: text() returned []const u8 — a direct slice of the internal ArrayList buffer. Zero allocation, couldn't fail.
After: text() returns ![]const u8 — allocates a new buffer and copies both halves of the gap buffer into it. Callers must try and defer allocator.free(...).
Why: The gap buffer stores text in two non-contiguous halves (before-cursor and after-cursor). There's no single contiguous slice to return without copying. The vaxis TextInput exposes firstHalf() and secondHalf() but no "give me everything as one slice" method.
Current implementation:
pub fn text(self: *const TextInput) ![]const u8 {
const first = self.vaxis_input.buf.firstHalf();
const second = self.vaxis_input.buf.secondHalf();
const buf = try self.allocator.alloc(u8, first.len + second.len);
@memcpy(buf[0..first.len], first);
@memcpy(buf[first.len..], second);
return buf;
}Callers affected: Lua bindings in ui.zig (getText function), and any future code that reads the input value. Currently there are few call sites so the blast radius is small.
Question for reviewer: Is there a way to get a contiguous view from the gap buffer without allocating? For instance, does vaxis have (or could it have) a method that collapses the gap temporarily? If text is read infrequently (only on submit/confirm), the allocation is fine. But if it's called per-frame for display purposes, the allocation pattern could matter.
Before: Two insertion methods: insert(char: u8) for single bytes and insertSlice([]const u8) for strings.
After: Only insertSlice([]const u8) remains. The Lua binding routes single-character insertion through insertSlice by passing a 1-byte slice.
Why: The vaxis TextInput API doesn't have a single-byte insert — it works with slices (which can contain full graphemes). Rather than wrapping a single byte into a slice at the Zig API level, the wrapping happens at the Lua binding level where the input arrives as a string anyway.
Impact: Minimal. The old insert(char: u8) was only called from Lua bindings. No external callers need updating.
Before: render took *const TextInput — the widget was immutable during rendering.
After: render takes *TextInput — it mutates self.scroll_offset during rendering to keep the cursor visible in the viewport.
Why: Scroll offset adjustment is tightly coupled to the render pass (it depends on the window width, which is only known at render time). Computing it separately would duplicate the display-column calculation. Storing scroll state on the widget is the natural place for it.
Impact: Any code holding a const *TextInput can no longer call render(). In practice this is fine — the widget is always mutable in the rendering path. But it's a signature change worth noting.
Question for reviewer: Some widget libraries separate "layout" (mutable, computes positions) from "paint" (immutable, just draws). Would it be cleaner to split scroll computation into a separate layout(width) method that mutates, keeping render as *const? This is a style question — the current approach works correctly.
| Change | Severity | Migration effort |
|---|---|---|
text() allocates |
Medium | Small — few call sites, mechanical try/defer |
insert() removed |
Low | None — binding already updated |
render() mutates |
Low | None — widget is always mutable in render path |
None of these are architectural problems — they're natural consequences of the gap-buffer swap. The question is whether any can be smoothed out at the vaxis level (especially text() allocation).