From 8f2b545c81e8cb9839f64c861becd151b8084b10 Mon Sep 17 00:00:00 2001 From: Emil Lerch Date: Thu, 3 Sep 2026 15:47:06 -0700 Subject: [PATCH] human review: tui/draw.zig and test_render --- .kiro/specs/calculator/requirements.md | 3 + .kiro/specs/calculator/tasks.md | 62 ++++++- src/tui.zig | 177 +++++++++++++++--- src/tui/convert.zig | 6 +- src/tui/draw.zig | 237 +++++++++++++++++++++---- src/tui/financial.zig | 12 +- src/tui/float_view.zig | 4 +- src/tui/help.zig | 4 +- src/tui/programmer.zig | 173 ++++++++++++------ src/tui/test_render.zig | 141 +++++++++++++-- 10 files changed, 685 insertions(+), 134 deletions(-) diff --git a/.kiro/specs/calculator/requirements.md b/.kiro/specs/calculator/requirements.md index fca458e..c5eac9e 100644 --- a/.kiro/specs/calculator/requirements.md +++ b/.kiro/specs/calculator/requirements.md @@ -130,6 +130,7 @@ A calculator application with three frontends (CLI, TUI, Android) sharing a comm - **FR-7.2**: Standard mode: expression input line, result display, scrollable history. - **FR-7.3**: Programmer mode: bit grid (navigable with arrow keys, toggled with Space), simultaneous base displays, expression input. NOTE: Enter does not toggle a bit. It is consumed by the input line, and the value-zone status hint that advertised `Enter:set` is a known defect. Space is the only toggle. - **FR-7.3.1**: When the terminal is too short for the bit grid plus all six base rows, the view says so and how many rows it needs, rather than drawing rows over the separator and the input line. +- **FR-7.3.2**: When the terminal is too narrow for the whole binary row, it continues onto further rows rather than being cut off. 64 bits grouped in fours is 79 characters and starts at column 11, so the row needs a 90-column terminal; on the default 80 it was silently clipped, and what went missing was the right-hand end, which is the low bits - the value read as wrong rather than as incomplete. The split halves at a time (64 bits becomes two rows of 32), matching the grouping the bit grid above already uses, and a wider terminal gets the single row back. The label sits on the first row; the rest are the same field, with the same click targets and the same cursor. - **FR-7.4**: Programmer mode struct sub-view: field list editor, live-updating memory map visualization. - **FR-7.5**: Financial mode: form-style input for parameters, result display with formula breakdown, and a scrollable amortization schedule. Four calculations behind one selector: CAGR, compound interest, TVM, and amortization. TVM and compound interest solve for whichever field is left blank; an unanswerable form reports what it is waiting for rather than doing nothing. Results are live, so financial mode records nothing in history. - **FR-7.5.1**: Form fields group digits as they are typed: entering `200` then `0` displays `2,000`. The stored text stays ungrouped so it still parses, and a typed comma is accepted and discarded rather than corrupting the value. @@ -141,6 +142,8 @@ A calculator application with three frontends (CLI, TUI, Android) sharing a comm - **FR-7.7**: Every action must be reachable from the keyboard alone; the TUI is fully usable without a mouse. - **FR-7.8**: REMOVED. Quick-switch keys (`d`=dec, `h`=hex, `o`=oct, `b`=bin) to highlight the primary base cannot coexist with editable value fields: in the value zone those letters are hex and binary digits. The requirement predates editable fields, and the fields are the more useful feature. Base selection stays on the arrow keys and the mouse. - **FR-7.9**: Support terminal resize gracefully. +- **FR-7.9.1**: Nothing in the TUI's own chrome is cut off mid-word at 60 columns or more. Status lines are lists of key hints, and a line too long for the terminal drops whole hints from the end rather than losing the last one's tail: each mode orders its hints so the keys that leave the current zone (`?:help`, `Tab:mode`, the zone toggle) come first and are the last to go. Status lines also leave the final column clear, which is what lets a rendered frame be checked for clipping at all: any row but the separator reaching the right edge is then a fault, not a maybe. Below 60 columns the notices themselves cannot fit, and shortening them further would leave them saying nothing. +- **FR-7.9.2**: Input is ASCII. A key event carrying non-ASCII text is dropped rather than stored: every column in the TUI is one byte, so an accented character would occupy a cell per UTF-8 byte, drawn as blanks, and then fail to tokenize. Refusing it keeps what is on screen equal to what is in the buffer. A wider fix is a different piece of work: the whole layout counts columns in bytes. - **FR-7.10**: Vi-style and Emacs-style keybinding options for expression input. NOT IMPLEMENTED: the prompt is a plain text field. Deferred, not dropped. #### FR-7.11: Mouse Support diff --git a/.kiro/specs/calculator/tasks.md b/.kiro/specs/calculator/tasks.md index aa349cd..1181d7f 100644 --- a/.kiro/specs/calculator/tasks.md +++ b/.kiro/specs/calculator/tasks.md @@ -914,7 +914,67 @@ plans a `--raw` flag, but today it is exercised only by tests. 100% line coverage, engine 99.44%. CLI output byte-identical across all five rows, both byte orders, ASCII packing and the multi-base standard-mode view. -### Task 5.26: The CLI's output leaves main.zig +### Task 5.27: The check that could fail, and the two faults it found + +`test_render.zig` opens by explaining that an earlier `wellFormed(rows, width)` was a +tautology: every row is allocated at exactly `width`, so the assertion could not fail. +It then lists three things that are detectable, one of which was right-edge clipping, +via `rowsReachingRightEdge` and `noTextClipped`. Neither had a caller outside its own +test. Wiring them into the render tests explained why. + +**The binary row was cut off on a default terminal.** 64 bits grouped in fours is 79 +characters starting at column 11, so the row needs 90 columns. At 80 it lost its +right-hand end, which is the low bits: the value read as wrong rather than as +incomplete, with no indication either way. 128-bit needed 170 columns and had been +unreadable since it was added. There was a height guard for this view and no width +equivalent. + +The row now continues onto further rows, halving until it fits: 64 bits at 80 columns +is two rows of 32, the same grouping the bit grid above uses, and a 90-column terminal +gets the single row back. The label is on the first row; the others are the same field +with the same click targets and the same cursor, which took a `DigitRun { before, +total }` through `registerDigitRegions` and `drawFieldWithCursor` so a digit's bit +position stays a property of the value rather than of the row it landed on. The height +guard counts the binary rows it is about to draw, so a short terminal still reports +what it needs. `drawFieldWithCursor` also lost a `total_bits` parameter it never read. + +**Every status line lost its last hint mid-word.** Programmer mode's is 101 +characters; at 80 columns it ended on "Ctrl", and convert's ended on "Tab:m" at 60. +`draw.hintRow` writes as many whole hints as fit and drops the rest, and each mode now +orders its hints so the keys that leave the current zone survive. It also leaves the +final column clear, which is what makes the clipping check exact: any row but the +separator reaching the right edge is now a fault rather than a maybe. The +`too short for the {d}-bit view` notice was shortened for the same reason - a notice +that is itself clipped is no better than the layout it reports on. + +**The rest of the entry.** `draw.zig` now writes cells through vaxis's own +`Surface.writeCell`, which does the bounds check the three helpers each hand-rolled, +and `fillRow` and `writeStr` go through `writeChar`, so nothing in the TUI turns a row +and column into an index any more. The 128 one-byte pointers behind `charGrapheme` +became one 128-byte array, which also drops a `@setEvalBranchQuota`. The file had no +tests of its own: clipping at the right edge, dropping writes outside the surface, and +substituting a space for a byte above 127 are the assumptions every view is built on, +and all three are now asserted directly. + +That substitution turned out to be reachable rather than defensive: the input line +draws `TextField` bytes one at a time, so a pasted accented character became a cell +per UTF-8 byte, drawn blank, and then failed to tokenize. `handleKey` now drops key +events carrying non-ASCII text (FR-7.9.2), which is what the programmer value zone +already did for itself. + +`flatten` became private, `noTextClipped` takes the number of full-width rows to +expect rather than hardcoding one, and `fault`/`expectSound` replace the pair of +assertions each render test used to make, reporting the offending rows instead of a +bare false. The stale comment above the render tests, which still claimed they +asserted the frame stayed rectangular, is gone. + +- Verify: 938 tests pass, 16 of them new (the last being a click on the second row of + a wrapped binary field, which is the one place the new digit bookkeeping could be + wrong without any frame looking wrong: bit 0 is the last digit of the last row). draw.zig and programmer.zig are at 100% line + coverage, up from 96.2% and 100%; test_render.zig is 92.3%, the uncovered lines + being the failure reports that only run when a test fails. TUI total 98.56%. Proving + the check can fail: disabling the binary split makes two render tests fail with the + clipped row printed, including one that had been passing while the row was clipped. `main.zig` held both ends of the program: which arguments mean what, and how a finished figure reads on a terminal. One file decided `--bits` spellings and money diff --git a/src/tui.zig b/src/tui.zig index eb6d38c..a00a307 100644 --- a/src/tui.zig +++ b/src/tui.zig @@ -579,6 +579,18 @@ pub const App = struct { return; } + // Non-ASCII text is refused rather than stored. Every column in this TUI is a + // byte (`draw.writeChar` writes one cell per byte) and the engine tokenizes + // bytes, so a typed or pasted `e` with an acute accent would occupy two cells + // drawn as spaces and then fail to parse. Dropping it keeps what is on screen + // equal to what is in the buffer. No binding uses these keys: the programmer + // value zone already gated its own input to printable ASCII. + if (key.text) |text| { + for (text) |byte| { + if (byte > 127) return; + } + } + if (self.show_help) { // Scrolling keys navigate the overlay; anything else dismisses it, so // the "press a key to get out" behavior survives while the sections @@ -1287,7 +1299,7 @@ pub const App = struct { self.drawInput(surface, height -| 2); self.addRegion(height -| 2, 0, width, .focus_input); draw.fillRow(surface, height -| 1, ' ', .{ .fg = C.muted, .bg = C.bg }); - draw.writeStr(surface, height -| 1, 1, "?:help | Tab:mode | Enter:eval | Ctrl-L:clear | Ctrl-C:quit", .{ .fg = C.muted, .bg = C.bg }); + draw.hintRow(surface, height -| 1, 1, "?:help | Tab:mode | Enter:eval | Ctrl-L:clear | Ctrl-C:quit", .{ .fg = C.muted, .bg = C.bg }); } pub fn drawInput(self: *App, surface: *vxfw.Surface, row: u16) void { @@ -1930,11 +1942,16 @@ test "financial mode: an expression typed into a field survives field changes" { // -- Rendered frames for every mode -- // -// Drawing code was previously untested: the view modules had zero instrumented +// Drawing code was previously untested: the view files had zero instrumented // coverage, so a layout regression would only show up by eye. These draw real -// frames through the actual widget draw path and read the cells back. They also -// assert the frame stays rectangular, since the drawing helpers clip silently -// rather than erroring when a column is miscomputed. +// frames through the actual widget draw path and read the cells back. +// +// Two things are checked on every frame: that the separator, prompt and status rows +// are where they belong (`furniture`), and that no row other than the separator +// reaches the right edge (`noTextClipped`). The second matters because the drawing +// helpers clip silently, so a row that outgrew its terminal looks like a shorter row. +// An earlier `wellFormed(rows, width)` claimed to catch that and could not: every row +// is allocated at exactly `width`, so it was a tautology. fn renderApp(arena: std.mem.Allocator, app: *App, width: u16, height: u16) ![][]u8 { return test_render.frame(arena, app, width, height); @@ -1951,7 +1968,7 @@ test "render: the tab bar shows every mode and marks the active one" { for ([_]Mode{ .standard, .programmer, .financial, .convert }) |mode| { app.setMode(mode); const rows = try renderApp(arena, &app, 100, 24); - try testing.expect(test_render.furniture(rows).intact()); + try test_render.expectSound(rows, 1); try testing.expect(test_render.contains(rows, "Tally")); try testing.expect(test_render.contains(rows, "Standard")); try testing.expect(test_render.contains(rows, "Programmer")); @@ -1980,7 +1997,7 @@ test "render: standard mode shows history, results and details" { try press(&app, &ctx, .{ .codepoint = vaxis.Key.enter }); const rows = try renderApp(arena, &app, 80, 24); - try testing.expect(test_render.furniture(rows).intact()); + try test_render.expectSound(rows, 1); try testing.expect(test_render.contains(rows, "2 + 3 * 4")); try testing.expect(test_render.contains(rows, "= 14")); try testing.expect(test_render.contains(rows, "= 256")); @@ -2026,7 +2043,7 @@ test "render: programmer mode draws the bit grid and all base rows" { app.prog_value = 0xDEADBEEF; const rows = try renderApp(arena, &app, 100, 30); - try testing.expect(test_render.furniture(rows).intact()); + try test_render.expectSound(rows, 1); try testing.expect(test_render.contains(rows, "Bits: 64")); try testing.expect(test_render.contains(rows, "Signed: yes")); try testing.expect(test_render.contains(rows, "DEC(s):")); @@ -2073,7 +2090,7 @@ test "render: programmer mode is well formed at every width and setting" { for ([_]engine.Integer.Signedness{ .signed, .unsigned }) |signedness| { app.prog_config.signedness = signedness; const rows = try renderApp(arena, &app, 100, 40); - try testing.expect(test_render.furniture(rows).intact()); + try test_render.expectSound(rows, 1); var buf: [24]u8 = undefined; const expected = try std.fmt.bufPrint(&buf, "Bits: {d}", .{width.bits()}); try testing.expect(test_render.contains(rows, expected)); @@ -2096,7 +2113,7 @@ test "render: the float view decodes a known bit pattern" { app.prog_value = @as(u32, @bitCast(@as(f32, 1.0))); const rows = try renderApp(arena, &app, 100, 30); - try testing.expect(test_render.furniture(rows).intact()); + try test_render.expectSound(rows, 1); try testing.expect(test_render.contains(rows, "sign")); try testing.expect(test_render.contains(rows, "exponent")); try testing.expect(test_render.contains(rows, "significand")); @@ -2136,7 +2153,7 @@ test "render: the float view decodes every classification by name" { app.prog_config.width = if (case.format == .f32) .bits32 else .bits64; app.prog_value = case.bits; const rows = try renderApp(arena, &app, 100, 34); - try testing.expect(test_render.furniture(rows).intact()); + try test_render.expectSound(rows, 1); if (!test_render.contains(rows, case.class)) { std.debug.print("float view did not report \"{s}\" for bits 0x{X}\n", .{ case.class, case.bits }); return error.TestUnexpectedResult; @@ -2154,7 +2171,7 @@ test "render: convert mode draws chips, both unit columns and the exact result" app.setMode(.convert); const rows = try renderApp(arena, &app, 100, 34); - try testing.expect(test_render.furniture(rows).intact()); + try test_render.expectSound(rows, 1); try testing.expect(test_render.contains(rows, "Category:")); try testing.expect(test_render.contains(rows, "Length")); try testing.expect(test_render.contains(rows, "From")); @@ -2196,7 +2213,7 @@ test "render: convert mode is well formed for every category" { var ctx = testCtx(); try app.applyAction(&ctx, .{ .conv_category = category }); const rows = try renderApp(arena, &app, 100, 34); - try testing.expect(test_render.furniture(rows).intact()); + try test_render.expectSound(rows, 1); try testing.expect(test_render.contains(rows, category.label())); } } @@ -2288,9 +2305,11 @@ test "render: the help overlay lists every mode's bindings across its pages" { while (page < 12) : (page += 1) { const rows = try renderApp(arena, &app, 80, 24); // The help overlay has no prompt, so its structural invariant is its own: - // the title on row 1 and a footer on the last row. + // the title on row 1, a footer on the last row, and nothing reaching the + // right edge, since it draws no full-width separator either. try testing.expectEqual(@as(?usize, 1), test_render.rowOf(rows, "Tally - Help")); try testing.expect(test_render.rowOf(rows, "scroll") == rows.len - 1); + try testing.expect(test_render.noTextClipped(rows, 0)); for (wanted, 0..) |needle, i| { if (test_render.contains(rows, needle)) found[i] = true; } @@ -2350,10 +2369,23 @@ test "render: every mode keeps its input line at every terminal size" { // region lands on the separator or the prompt, which is exactly what happened // in programmer mode at 100x16 and financial mode at 20x5. The old assertion // here could not fail, so both went unnoticed. + // + // Clipping is only asserted from 60 columns up. Below that the chrome itself + // cannot fit: the "too short" notice is a sentence, and shortening it to 20 + // columns would leave it saying nothing. 60 is where every mode's furniture and + // value rows fit with the final column to spare. + const clipping_free_from: u16 = 60; for ([_]Mode{ .standard, .programmer, .financial, .convert }) |mode| { app.setMode(mode); for ([_][2]u16{ .{ 20, 6 }, .{ 40, 10 }, .{ 60, 15 }, .{ 100, 16 }, .{ 200, 60 } }) |size| { const rows = try renderApp(arena, &app, size[0], size[1]); + if (size[0] >= clipping_free_from) { + test_render.expectSound(rows, 1) catch |err| { + std.debug.print("{s} at {d}x{d}\n", .{ @tagName(mode), size[0], size[1] }); + return err; + }; + continue; + } const bottom = test_render.furniture(rows); if (!bottom.intact()) { std.debug.print( @@ -2366,6 +2398,71 @@ test "render: every mode keeps its input line at every terminal size" { } } +test "render: every bit of the binary row survives an 80-column terminal" { + var arena_state = std.heap.ArenaAllocator.init(testing.allocator); + defer arena_state.deinit(); + const arena = arena_state.allocator(); + + var app = testApp(); + defer app.deinit(); + app.setMode(.programmer); + // Every bit set, so a missing bit is a missing '1' rather than an invisible zero. + app.prog_value = std.math.maxInt(u128); + + // 64 bits grouped in fours is 79 characters starting at column 11, so the row + // needed a 90-column terminal and was cut short on the default 80 - losing the + // right-hand end, which is the low bits. It now wraps onto as many rows as the + // width needs. + for ([_]engine.BitWidth{ .bits8, .bits16, .bits32, .bits64, .bits128 }) |bits| { + app.prog_config.width = bits; + const rows = try renderApp(arena, &app, 80, 30); + try test_render.expectSound(rows, 1); + + const label_row = test_render.rowOf(rows, "BIN:") orelse return error.NoBinaryRow; + var shown: usize = 0; + var row = label_row; + while (row < rows.len) : (row += 1) { + const ones = countBinaryDigits(rows[row]); + // The row after the last one of the binary field is blank, so the count + // stops there rather than walking into the history. + if (ones == 0) break; + shown += ones; + } + if (shown != bits.bits()) { + std.debug.print("{d}-bit: {d} bits drawn at 80 columns\n", .{ bits.bits(), shown }); + return error.BinaryRowClipped; + } + } +} + +/// Binary digits in the value area of a row, which is everything past the labels. +fn countBinaryDigits(row: []const u8) usize { + const value_col = 11; + if (row.len <= value_col) return 0; + var count: usize = 0; + for (row[value_col..]) |ch| { + if (ch == '0' or ch == '1') count += 1; + } + return count; +} + +test "non-ASCII input is refused rather than stored as bytes that draw as spaces" { + var app = testApp(); + defer app.deinit(); + var ctx = testCtx(); + defer ctx.cmds.deinit(testing.allocator); + + // A pasted or composed accented character arrives as its UTF-8 bytes. Storing it + // would put two cells of nothing in the input line and then fail to tokenize. + try press(&app, &ctx, .{ .codepoint = '2', .text = "2" }); + try press(&app, &ctx, .{ .codepoint = 0xE9, .text = "\xC3\xA9" }); + try press(&app, &ctx, .{ .codepoint = '+', .text = "+" }); + try press(&app, &ctx, .{ .codepoint = '2', .text = "2" }); + + try testing.expectEqualStrings("2+2", app.input.buf.firstHalf()); + try testing.expectEqualStrings("", app.input.buf.secondHalf()); +} + test "render: 128-bit programmer mode keeps its input line on a short terminal" { var arena_state = std.heap.ArenaAllocator.init(testing.allocator); defer arena_state.deinit(); @@ -2379,17 +2476,17 @@ test "render: 128-bit programmer mode keeps its input line on a short terminal" // Four grid rows plus six base rows do not fit in 16 rows. The view used to // draw them anyway, putting the BIN row on the prompt. const rows = try renderApp(arena, &app, 100, 16); - try testing.expect(test_render.furniture(rows).intact()); - try testing.expect(test_render.contains(rows, "terminal too short")); + try test_render.expectSound(rows, 1); + try testing.expect(test_render.contains(rows, "too short for the")); // And it says what it needs, rather than showing a half-drawn layout. try testing.expect(test_render.contains(rows, "128-bit")); try testing.expect(!test_render.contains(rows, "BIN:")); // With room, the full view is back. const tall = try renderApp(arena, &app, 100, 30); - try testing.expect(test_render.furniture(tall).intact()); + try test_render.expectSound(tall, 1); try testing.expect(test_render.contains(tall, "BIN:")); - try testing.expect(!test_render.contains(tall, "terminal too short")); + try testing.expect(!test_render.contains(tall, "too short for the")); } test "help overlay: arrows scroll, other keys dismiss" { @@ -2548,6 +2645,46 @@ test "clicking a tab switches mode through the region table" { } } +test "clicking a bit on a wrapped binary row flips the bit that digit stands for" { + var arena_state = std.heap.ArenaAllocator.init(testing.allocator); + defer arena_state.deinit(); + + var app = testApp(); + defer app.deinit(); + var ctx = testCtx(); + defer ctx.cmds.deinit(testing.allocator); + app.setMode(.programmer); + app.prog_config.width = .bits64; + app.prog_value = 0; + + // 80 columns splits the row in two, so the digits on the second row are the ones + // whose bit position no longer follows from their column alone. Bit 0 is the last + // digit of the last row and bit 63 the first digit of the first. + const rows = try renderApp(arena_state.allocator(), &app, 80, 30); + const first_bin = test_render.rowOf(rows, "BIN:") orelse return error.NoBinaryRow; + const second_bin = first_bin + 1; + try testing.expect(test_render.contains(rows[second_bin .. second_bin + 1], "0000")); + + // Last digit of the second row: 39 characters of grouped bits from column 11. + const value_col: u16 = 11; + const lsb_col = value_col + 38; + const lsb = app.regions.at(@intCast(second_bin), lsb_col) orelse return error.NoRegion; + try testing.expectEqual(@as(u7, 0), lsb.toggle_bit); + + const msb = app.regions.at(@intCast(first_bin), value_col) orelse return error.NoRegion; + try testing.expectEqual(@as(u7, 63), msb.toggle_bit); + + // And the click does what the region says. + try app.handleMouse(&ctx, .{ + .col = lsb_col, + .row = @intCast(second_bin), + .button = .left, + .mods = .{}, + .type = .press, + }); + try testing.expectEqual(@as(u128, 1), app.prog_value); +} + test "clicking empty space does nothing" { var arena_state = std.heap.ArenaAllocator.init(testing.allocator); defer arena_state.deinit(); @@ -3054,7 +3191,7 @@ test "render: programmer mode draws the cursor in whichever field is focused" { app.prog_config.width = width; app.bit_cursor = @intCast(@min(5, width.bits() - 1)); const rows = try renderApp(arena, &app, 110, 40); - try testing.expect(test_render.furniture(rows).intact()); + try test_render.expectSound(rows, 1); // The bit grid has one row per 32 bits, and the base rows follow it, so // this pins the layout rather than just the presence of a string. const grid_rows = (@as(usize, width.bits()) + 31) / 32; @@ -3079,7 +3216,7 @@ test "render: convert mode draws the focused column and category" { for ([_]ConvZone{ .category, .from, .to }) |zone| { app.conv_zone = zone; const rows = try renderApp(arena, &app, 100, 34); - try testing.expect(test_render.furniture(rows).intact()); + try test_render.expectSound(rows, 1); try testing.expect(test_render.contains(rows, "Category:")); } } diff --git a/src/tui/convert.zig b/src/tui/convert.zig index a006c5c..32b01c3 100644 --- a/src/tui/convert.zig +++ b/src/tui/convert.zig @@ -145,10 +145,10 @@ pub fn drawConvertMode(app: *tui.App, surface: *vxfw.Surface, width: u16, height draw.fillRow(surface, height -| 1, ' ', .{ .fg = C.muted, .bg = C.bg }); const status = if (zone_active) - "Arrows:select | Ctrl-S:swap | `:input | Tab:mode | ?:help" + "Tab:mode | ?:help | `:input | Arrows:select | Ctrl-S:swap" else - "Type a value + Enter | `:select units | Ctrl-S:swap | Tab:mode | ?:help"; - draw.writeStr(surface, height -| 1, 1, status, .{ .fg = C.muted, .bg = C.bg }); + "?:help | Tab:mode | Type a value + Enter | `:select units | Ctrl-S:swap"; + draw.hintRow(surface, height -| 1, 1, status, .{ .fg = C.muted, .bg = C.bg }); } /// Draw one unit name in a column, highlighting it when selected, and register diff --git a/src/tui/draw.zig b/src/tui/draw.zig index 7f5fb9f..e4b795b 100644 --- a/src/tui/draw.zig +++ b/src/tui/draw.zig @@ -1,5 +1,16 @@ //! Shared drawing helpers for the TUI. +//! +//! One cell at a time, one byte per cell. Everything above this file computes +//! columns as byte counts, which holds because the TUI is ASCII: `charGrapheme` +//! substitutes a space for anything wider, and `tui.App.handleKey` refuses +//! non-ASCII input so a grapheme never has to span two cells. +//! +//! Nothing here reports failure. A view draws with the geometry it was handed, and +//! a cell outside the surface is dropped: during a resize the alternative is every +//! caller re-checking bounds the surface already knows. What that costs is silence +//! when a row really is too long, which is why `test_render.noTextClipped` exists. +const std = @import("std"); const vaxis = @import("vaxis"); const vxfw = vaxis.vxfw; @@ -23,49 +34,201 @@ pub const C = struct { pub const sel: vaxis.Cell.Color = .{ .rgb = .{ 0x49, 0x48, 0x3E } }; }; -/// Static single-byte grapheme strings with static lifetime. -const ascii_graphemes: [128]*const [1]u8 = blk: { - @setEvalBranchQuota(200); - var table: [128]*const [1]u8 = undefined; - for (0..128) |i| { - table[i] = &[1]u8{@as(u8, @intCast(i))}; - } +/// The printable ASCII range, one byte each, so `charGrapheme` can hand out a +/// grapheme with static lifetime instead of pointing at a caller's buffer. +const ascii_bytes: [128]u8 = blk: { + var table: [128]u8 = undefined; + for (&table, 0..) |*byte, i| byte.* = @intCast(i); break :blk table; }; +/// A single byte as a grapheme. Bytes above 127 become a space: they are one third +/// of a UTF-8 sequence this file cannot place in one cell, and a space keeps the +/// column arithmetic honest rather than drawing a partial glyph. pub fn charGrapheme(byte: u8) []const u8 { - if (byte < 128) return ascii_graphemes[byte]; - return " "; -} - -pub fn fillRow(surface: *vxfw.Surface, row: u16, char: u8, style: vaxis.Style) void { - const w = surface.size.width; - const grapheme: []const u8 = charGrapheme(char); - for (0..w) |col| { - const idx = @as(usize, row) * @as(usize, w) + col; - if (idx < surface.buffer.len) { - surface.buffer[idx] = .{ .char = .{ .grapheme = grapheme, .width = 1 }, .style = style }; - } - } -} - -pub fn writeStr(surface: *vxfw.Surface, row: u16, col: u16, text: []const u8, style: vaxis.Style) void { - const w = surface.size.width; - for (0..text.len) |i| { - const c = col + @as(u16, @intCast(i)); - if (c >= w) break; - const idx = @as(usize, row) * @as(usize, w) + @as(usize, c); - if (idx < surface.buffer.len) { - surface.buffer[idx] = .{ .char = .{ .grapheme = charGrapheme(text[i]), .width = 1 }, .style = style }; - } - } + if (byte >= ascii_bytes.len) return " "; + return ascii_bytes[byte .. byte + 1]; } +/// Write one cell. Out-of-range rows and columns are dropped by `writeCell`, which +/// is the only place in the TUI that turns a row and column into an index. pub fn writeChar(surface: *vxfw.Surface, row: u16, col: u16, byte: u8, style: vaxis.Style) void { - const w = surface.size.width; - if (col >= w) return; - const idx = @as(usize, row) * @as(usize, w) + @as(usize, col); - if (idx < surface.buffer.len) { - surface.buffer[idx] = .{ .char = .{ .grapheme = charGrapheme(byte), .width = 1 }, .style = style }; + surface.writeCell(col, row, .{ + .char = .{ .grapheme = charGrapheme(byte), .width = 1 }, + .style = style, + }); +} + +/// Fill a whole row with one byte. +pub fn fillRow(surface: *vxfw.Surface, row: u16, byte: u8, style: vaxis.Style) void { + var col: u16 = 0; + while (col < surface.size.width) : (col += 1) { + writeChar(surface, row, col, byte, style); } } + +/// Write text left to right from `col`, stopping at the right edge. A string that +/// runs past the edge loses its tail silently, which is what `hintRow` below and +/// the width checks in the views exist to avoid. +pub fn writeStr(surface: *vxfw.Surface, row: u16, col: u16, text: []const u8, style: vaxis.Style) void { + var c = col; + for (text) |byte| { + if (c >= surface.size.width) break; + writeChar(surface, row, c, byte, style); + c += 1; + } +} + +/// What separates one key hint from the next on a status line. +const hint_separator = " | "; + +/// Write a status line of key hints, dropping whole hints from the end when they do +/// not all fit. +/// +/// Status lines are the longest fixed strings the TUI draws - programmer mode's is +/// 101 characters - and `writeStr` would cut the last one mid-word: an 80-column +/// terminal used to end on "Ctrl" and a 60-column one on "Tab:m". Losing a whole +/// hint reads as a shorter list; losing half of one reads as a bug. Hints are +/// written in order, so each mode leads with the ones that matter most. +/// +/// The final column is left clear, so no status line reaches the right edge and +/// `test_render.noTextClipped` can treat a row that does as a layout fault. +pub fn hintRow(surface: *vxfw.Surface, row: u16, col: u16, text: []const u8, style: vaxis.Style) void { + const width = surface.size.width; + const room = if (width > col + 1) width - col - 1 else 0; + writeStr(surface, row, col, fittingHints(text, room), style); +} + +/// The longest prefix of `text` that ends on a hint boundary and fits `room`. +/// +/// When not even the first hint fits it is written clipped. A blank status row is +/// worse: `test_render.furniture` reads one as a broken layout, and it is, since the +/// row is part of what tells the user the program is still listening. +fn fittingHints(text: []const u8, room: usize) []const u8 { + if (text.len <= room) return text; + + var end: usize = 0; + var from: usize = 0; + while (std.mem.indexOfPos(u8, text, from, hint_separator)) |sep| { + if (sep > room) break; + end = sep; + from = sep + hint_separator.len; + } + if (end == 0) return text[0..@min(text.len, room)]; + return text[0..end]; +} + +// -- Tests -- +// +// These draw into a surface and read the cells back, because every one of the +// bounds decisions above is invisible from the outside: a clipped row and a +// correctly fitted row look identical to a caller, and the whole TUI is laid out on +// the assumption that a byte is a column. + +const testing = std.testing; + +fn testSurface(allocator: std.mem.Allocator, width: u16, height: u16) !vxfw.Surface { + const size: vxfw.Size = .{ .width = width, .height = height }; + return .{ + .size = size, + // SAFETY: the helpers here touch `size` and `buffer` only. Nothing in a test + // dispatches back into the widget that would own this surface. + .widget = undefined, + .buffer = try vxfw.Surface.createBuffer(allocator, size), + .children = &.{}, + }; +} + +/// One row read back as text. Untouched cells are a space, which is what +/// `Surface.createBuffer` leaves behind. +fn rowText(arena: std.mem.Allocator, surface: vxfw.Surface, row: u16) ![]u8 { + const width = surface.size.width; + const text = try arena.alloc(u8, width); + for (0..width) |col| { + const grapheme = surface.buffer[@as(usize, row) * width + col].char.grapheme; + text[col] = if (grapheme.len == 1) grapheme[0] else ' '; + } + return text; +} + +test "writeStr clips at the right edge instead of wrapping or erroring" { + var arena = std.heap.ArenaAllocator.init(testing.allocator); + defer arena.deinit(); + var surface = try testSurface(arena.allocator(), 10, 3); + + writeStr(&surface, 1, 6, "abcdefgh", .{}); + try testing.expectEqualStrings(" abcd", try rowText(arena.allocator(), surface, 1)); + // The tail went nowhere: it did not wrap onto the next row. + try testing.expectEqualStrings(" ", try rowText(arena.allocator(), surface, 2)); +} + +test "writes outside the surface are dropped" { + var arena = std.heap.ArenaAllocator.init(testing.allocator); + defer arena.deinit(); + var surface = try testSurface(arena.allocator(), 8, 2); + + // Past the last column, past the last row, and a row so far out that a + // hand-rolled `row * width + col` would land back inside the buffer. + writeChar(&surface, 0, 8, 'X', .{}); + writeStr(&surface, 5, 0, "gone", .{}); + writeChar(&surface, 2, 0, 'Y', .{}); + fillRow(&surface, 9, '#', .{}); + + for (0..2) |row| { + try testing.expectEqualStrings(" ", try rowText(arena.allocator(), surface, @intCast(row))); + } +} + +test "fillRow covers exactly the width" { + var arena = std.heap.ArenaAllocator.init(testing.allocator); + defer arena.deinit(); + var surface = try testSurface(arena.allocator(), 6, 2); + + fillRow(&surface, 0, '-', .{}); + try testing.expectEqualStrings("------", try rowText(arena.allocator(), surface, 0)); + try testing.expectEqualStrings(" ", try rowText(arena.allocator(), surface, 1)); +} + +test "a byte above ASCII draws as a space rather than a partial glyph" { + var arena = std.heap.ArenaAllocator.init(testing.allocator); + defer arena.deinit(); + var surface = try testSurface(arena.allocator(), 8, 1); + + // "e" followed by the two bytes of U+00E9, which is what a paste would leave in + // the input buffer. Each byte still takes one column, so the columns after it + // stay where the caller put them. + writeStr(&surface, 0, 0, "e\xC3\xA9!", .{}); + try testing.expectEqualStrings("e ! ", try rowText(arena.allocator(), surface, 0)); + try testing.expectEqualStrings(" ", charGrapheme(0x80)); + try testing.expectEqualStrings("A", charGrapheme('A')); +} + +test "hintRow drops whole hints and never reaches the right edge" { + var arena = std.heap.ArenaAllocator.init(testing.allocator); + defer arena.deinit(); + const alloc = arena.allocator(); + const hints = "?:help | Tab:mode | Enter:eval | Ctrl-C:quit"; + + // Room for everything. + var wide = try testSurface(alloc, 60, 1); + hintRow(&wide, 0, 1, hints, .{}); + try testing.expect(std.mem.indexOf(u8, try rowText(alloc, wide, 0), "Ctrl-C:quit") != null); + + // Room for three of the four. The fourth goes whole, and the last column is + // still clear. + var narrow = try testSurface(alloc, 34, 1); + hintRow(&narrow, 0, 1, hints, .{}); + const narrow_row = try rowText(alloc, narrow, 0); + try testing.expectEqualStrings(" ?:help | Tab:mode | Enter:eval ", narrow_row); + try testing.expectEqual(@as(u8, ' '), narrow_row[narrow_row.len - 1]); + + // Room for one, and then for none: the first hint is written cut short rather + // than leaving the row blank. + var tiny = try testSurface(alloc, 12, 1); + hintRow(&tiny, 0, 1, hints, .{}); + try testing.expectEqualStrings(" ?:help ", try rowText(alloc, tiny, 0)); + + var absurd = try testSurface(alloc, 5, 1); + hintRow(&absurd, 0, 1, hints, .{}); + try testing.expectEqualStrings(" ?:h ", try rowText(alloc, absurd, 0)); +} diff --git a/src/tui/financial.zig b/src/tui/financial.zig index 03b5b0f..d88682d 100644 --- a/src/tui/financial.zig +++ b/src/tui/financial.zig @@ -608,10 +608,10 @@ pub fn drawFinancialMode(app: *tui.App, surface: *vxfw.Surface, width: u16, heig draw.fillRow(surface, height -| 1, ' ', .{ .fg = C.muted, .bg = C.bg }); const status = if (zone_active) - "Up/Dn:field | L/R:calc | Enter:eval | Space:END/BGN | Ctrl-U:clear | `:input" + "`:input | Up/Dn:field | L/R:calc | Enter:eval | Space:END/BGN | Ctrl-U:clear" else - "Expr + Enter fills the field | `:fields | PgUp/PgDn:scroll | Tab:mode | ?:help"; - draw.writeStr(surface, height -| 1, 1, status, .{ .fg = C.muted, .bg = C.bg }); + "?:help | Tab:mode | Expr + Enter fills the field | `:fields | PgUp/PgDn:scroll"; + draw.hintRow(surface, height -| 1, 1, status, .{ .fg = C.muted, .bg = C.bg }); } /// Draw one field's value, or its blank placeholder. @@ -1577,7 +1577,7 @@ test "render: a short terminal keeps the input line and drops content" { const rows = try renderFrame(testing.allocator, arena, state, 80, 10, false); // Checking the furniture rather than "a > appears somewhere": at 20x5 a chip // was being drawn onto the prompt row, and a bare `contains(">")` passed. - try testing.expect(test_render.furniture(rows).intact()); + try test_render.expectSound(rows, 1); // The fields that did fit are real ones, not half-drawn rows over the prompt. try testing.expect(frameContains(rows, "N (periods)")); @@ -1609,7 +1609,7 @@ test "render: every form draws its own fields, empty or filled" { var empty: State = .{}; empty.setForm(form); const empty_rows = try renderFrame(testing.allocator, arena, empty, 80, 24, true); - try testing.expect(test_render.furniture(empty_rows).intact()); + try test_render.expectSound(empty_rows, 1); for (form.fields()) |spec| { if (!frameContains(empty_rows, spec.label)) { std.debug.print("{s}: field \"{s}\" was not drawn\n", .{ form.label(), spec.label }); @@ -1624,7 +1624,7 @@ test "render: every form draws its own fields, empty or filled" { filled.setForm(form); for (0..filled.fieldCount()) |i| filled.fieldAt(i).set("12"); const filled_rows = try renderFrame(testing.allocator, arena, filled, 80, 24, true); - try testing.expect(test_render.furniture(filled_rows).intact()); + try test_render.expectSound(filled_rows, 1); // Whatever the form computes, the entered value is on screen. try testing.expect(frameContains(filled_rows, "12")); } diff --git a/src/tui/float_view.zig b/src/tui/float_view.zig index 4c40e1f..acf0a2c 100644 --- a/src/tui/float_view.zig +++ b/src/tui/float_view.zig @@ -166,8 +166,8 @@ pub fn drawFloatView(app: *tui.App, surface: *vxfw.Surface, width: u16, height: app.drawInput(surface, height -| 2); app.addRegion(height -| 2, 0, surface.size.width, .focus_input); draw.fillRow(surface, height -| 1, ' ', .{ .fg = C.muted, .bg = C.bg }); - const status = "Type a float | Arrows:nav bits | Space:toggle | Ctrl-W:f32/f64 | Ctrl-F:exit | ?:help"; - draw.writeStr(surface, height -| 1, 1, status, .{ .fg = C.muted, .bg = C.bg }); + const status = "?:help | Ctrl-F:exit | Type a float | Arrows:nav bits | Space:toggle | Ctrl-W:f32/f64"; + draw.hintRow(surface, height -| 1, 1, status, .{ .fg = C.muted, .bg = C.bg }); } /// Draw the color-coded bit grid. Returns the number of rows drawn. diff --git a/src/tui/help.zig b/src/tui/help.zig index 7582ac0..3ae6ab4 100644 --- a/src/tui/help.zig +++ b/src/tui/help.zig @@ -160,9 +160,9 @@ pub fn drawHelp(surface: *vxfw.Surface, width: u16, height: u16, scroll: usize) const status = std.fmt.bufPrint(&buf, "lines {d}-{d} of {d} | Up/Down or wheel: scroll | any other key: return", .{ first + 1, last, lines.len, }) catch "Up/Down: scroll | any other key: return"; - draw.writeStr(surface, height -| 1, 1, status, .{ .fg = C.muted }); + draw.hintRow(surface, height -| 1, 1, status, .{ .fg = C.muted }); } else { - draw.writeStr(surface, height -| 1, 1, "Press any key to return", .{ .fg = C.muted }); + draw.hintRow(surface, height -| 1, 1, "Press any key to return", .{ .fg = C.muted }); } _ = width; } diff --git a/src/tui/programmer.zig b/src/tui/programmer.zig index fd070dd..36c8381 100644 --- a/src/tui/programmer.zig +++ b/src/tui/programmer.zig @@ -13,6 +13,52 @@ const C = draw.C; /// from the widest supported value, so this is unreachable in practice. const too_wide: []const u8 = "(too wide)"; +/// Column every base row draws its digits in, past the widest label ("ASCII:"). +const value_col: u16 = 11; + +/// Characters a run of `bits` binary digits takes, grouped in fours. +fn binaryWidth(bits: u16) u16 { + return bits + (bits / 4) - 1; +} + +/// How many bits of the binary row fit on one line, halved until they do. +/// +/// 64 bits grouped in fours is 79 characters starting at column 11, so the row needs +/// a 90-column terminal. On the default 80 it was cut off by `draw.writeStr`, and +/// what went missing was the right-hand end: the LOW bits, the ones being read. +/// Splitting keeps every bit on screen, and halving keeps the split on a byte +/// boundary, so 64 bits at 80 columns shows as two rows of 32 - the same grouping +/// the bit grid above already uses. A wider terminal gets the whole row back. +fn binBitsPerRow(width: u16, bits: u8) u8 { + var chunk: u16 = bits; + // `<` not `<=`: the final column stays clear, so a full-width row is always the + // separator and never a value that just fitted. + while (chunk > 8 and value_col + binaryWidth(chunk) >= width) chunk /= 2; + return @intCast(chunk); +} + +/// Digits in a grouped display string, which is digits and spaces only. +fn digitsIn(text: []const u8) u16 { + var count: u16 = 0; + for (text) |ch| { + if (ch != ' ') count += 1; + } + return count; +} + +/// Where a run of digits sits within a value. +/// +/// A field on one row passes only `total`. The binary row splits across rows when the +/// terminal is too narrow for all of it, and each row after the first says how many +/// digits came before it, so a digit's bit position stays a property of the value +/// rather than of the row it landed on. +const DigitRun = struct { + /// Digits drawn on the rows above this one. + before: u16 = 0, + /// Digits in the whole value. + total: u16, +}; + pub fn drawProgrammerMode(app: *tui.App, surface: *vxfw.Surface, width: u16, height: u16) void { const bw = app.prog_config.width; const val = app.prog_value & bw.mask(); @@ -50,19 +96,28 @@ pub fn drawProgrammerMode(app: *tui.App, surface: *vxfw.Surface, width: u16, hei const grid_rows: u16 = (@as(u16, bw.bits()) + 31) / 32; const base_start: u16 = grid_start + grid_rows + 1; - // The value view needs the grid plus six base rows. Without this check the + // The binary row takes more than one line when the terminal is too narrow for + // all of it, so its height is known before the fit check below rather than + // after the rows are drawn. + const bin_bits_per_row = binBitsPerRow(width, bw.bits()); + const bin_rows: u16 = @as(u16, bw.bits()) / bin_bits_per_row; + const value_rows: u16 = 5 + bin_rows; + + // The value view needs the grid plus the base rows. Without this check the // rows were drawn unconditionally and landed on the separator and the input // line: at 100x16 in 128-bit mode the BIN row overwrote the prompt, and at // 20x6 every base row was clipped away with no indication. Saying so is better // than drawing a layout that lies. const content_end = height -| 3; - if (base_start + 6 > content_end) { + if (base_start + value_rows > content_end) { var need_buf: [96]u8 = undefined; + // Kept short enough to fit a 60-column terminal at column 2: a notice that + // is itself clipped is no better than the layout it is reporting on. const need = std.fmt.bufPrint( &need_buf, - "terminal too short for the {d}-bit view: needs {d} rows, has {d}", - .{ bw.bits(), base_start + 6 + 3, height }, - ) catch "terminal too short for the bit view"; + "too short for the {d}-bit view: needs {d} rows, has {d}", + .{ bw.bits(), base_start + value_rows + 3, height }, + ) catch "too short for the bit view"; draw.writeStr(surface, 4, 2, need, .{ .fg = C.pink }); drawFurniture(app, surface, width, height, focused); return; @@ -80,7 +135,7 @@ pub fn drawProgrammerMode(app: *tui.App, surface: *vxfw.Surface, width: u16, hei else .{ .fg = C.cyan }; draw.writeStr(surface, base_start, 2, "DEC(s):", sdec_style); - draw.writeStr(surface, base_start, 11, sdec, if (focused == .dec_signed) .{ .fg = C.fg, .bold = true } else .{ .fg = C.fg }); + draw.writeStr(surface, base_start, value_col, sdec, if (focused == .dec_signed) .{ .fg = C.fg, .bold = true } else .{ .fg = C.fg }); app.addRegion(base_start, 0, width, .{ .prog_field = .{ .field = .dec_signed, .bit = null } }); // DEC(u) @@ -91,7 +146,7 @@ pub fn drawProgrammerMode(app: *tui.App, surface: *vxfw.Surface, width: u16, hei else .{ .fg = C.cyan }; draw.writeStr(surface, base_start + 1, 2, "DEC(u):", udec_style); - draw.writeStr(surface, base_start + 1, 11, udec, if (focused == .dec_unsigned) .{ .fg = C.fg, .bold = true } else .{ .fg = C.fg }); + draw.writeStr(surface, base_start + 1, value_col, udec, if (focused == .dec_unsigned) .{ .fg = C.fg, .bold = true } else .{ .fg = C.fg }); app.addRegion(base_start + 1, 0, width, .{ .prog_field = .{ .field = .dec_unsigned, .bit = null } }); // HEX @@ -102,21 +157,22 @@ pub fn drawProgrammerMode(app: *tui.App, surface: *vxfw.Surface, width: u16, hei else .{ .fg = C.cyan }; draw.writeStr(surface, base_start + 2, 2, "HEX:", hex_style); + const hex_run: DigitRun = .{ .total = digitsIn(hex) }; if (focused == .hex) { - drawFieldWithCursor(surface, base_start + 2, 11, hex, app.bit_cursor, 4, bw.bits(), C.green); + drawFieldWithCursor(surface, base_start + 2, value_col, hex, app.bit_cursor, 4, C.green, hex_run); } else { - draw.writeStr(surface, base_start + 2, 11, hex, .{ .fg = C.green }); + draw.writeStr(surface, base_start + 2, value_col, hex, .{ .fg = C.green }); } // Row-wide fallback focuses the field; per-digit regions (added next) place // the cursor on the exact nibble that was clicked. app.addRegion(base_start + 2, 0, width, .{ .prog_field = .{ .field = .hex, .bit = null } }); - registerDigitRegions(app, base_start + 2, 11, hex, 4, .hex, false); + registerDigitRegions(app, base_start + 2, value_col, hex, 4, .hex, false, hex_run); // ASCII (derived, read-only): one glyph per byte, aligned under HEX. var ascii_buf: [128]u8 = undefined; const ascii = int.fmt(.ascii, .{ .endian = app.prog_config.display_endian }).render(&ascii_buf) catch too_wide; draw.writeStr(surface, base_start + 3, 2, "ASCII:", .{ .fg = C.cyan }); - draw.writeStr(surface, base_start + 3, 11, ascii, .{ .fg = C.orange }); + draw.writeStr(surface, base_start + 3, value_col, ascii, .{ .fg = C.orange }); // OCT var oct_buf: [256]u8 = undefined; @@ -126,15 +182,18 @@ pub fn drawProgrammerMode(app: *tui.App, surface: *vxfw.Surface, width: u16, hei else .{ .fg = C.cyan }; draw.writeStr(surface, base_start + 4, 2, "OCT:", oct_style); + const oct_run: DigitRun = .{ .total = digitsIn(oct) }; if (focused == .oct) { - drawFieldWithCursor(surface, base_start + 4, 11, oct, app.bit_cursor, 3, bw.bits(), C.purple); + drawFieldWithCursor(surface, base_start + 4, value_col, oct, app.bit_cursor, 3, C.purple, oct_run); } else { - draw.writeStr(surface, base_start + 4, 11, oct, .{ .fg = C.purple }); + draw.writeStr(surface, base_start + 4, value_col, oct, .{ .fg = C.purple }); } app.addRegion(base_start + 4, 0, width, .{ .prog_field = .{ .field = .oct, .bit = null } }); - registerDigitRegions(app, base_start + 4, 11, oct, 3, .oct, false); + registerDigitRegions(app, base_start + 4, value_col, oct, 3, .oct, false, oct_run); - // BIN + // BIN, on as many rows as the terminal is wide enough for. The label sits on the + // first; the rest are the same field continued, so they carry the same clickable + // regions and the same cursor. var bin_buf: [512]u8 = undefined; const bin = int.fmt(.binary, .{}).render(&bin_buf) catch too_wide; const bin_style: vaxis.Style = if (focused == .bin) @@ -142,17 +201,29 @@ pub fn drawProgrammerMode(app: *tui.App, surface: *vxfw.Surface, width: u16, hei else .{ .fg = C.cyan }; draw.writeStr(surface, base_start + 5, 2, "BIN:", bin_style); - if (focused == .bin) { - drawFieldWithCursor(surface, base_start + 5, 11, bin, app.bit_cursor, 1, bw.bits(), C.yellow); - } else { - draw.writeStr(surface, base_start + 5, 11, bin, .{ .fg = C.yellow }); + const bin_chunk_len = binaryWidth(bin_bits_per_row); + for (0..bin_rows) |i| { + const row = base_start + 5 + @as(u16, @intCast(i)); + // Groups are four bits wide and every chunk is a multiple of four, so a chunk + // boundary is always a group boundary: the slice never splits a group. + const start = i * (bin_chunk_len + 1); + const chunk = if (start < bin.len) bin[start..@min(bin.len, start + bin_chunk_len)] else ""; + const run: DigitRun = .{ + .before = @as(u16, @intCast(i)) * bin_bits_per_row, + .total = bw.bits(), + }; + if (focused == .bin) { + drawFieldWithCursor(surface, row, value_col, chunk, app.bit_cursor, 1, C.yellow, run); + } else { + draw.writeStr(surface, row, value_col, chunk, .{ .fg = C.yellow }); + } + app.addRegion(row, 0, width, .{ .prog_field = .{ .field = .bin, .bit = null } }); + // A binary digit IS a single bit, so clicking one flips it directly. + registerDigitRegions(app, row, value_col, chunk, 1, .bin, true, run); } - app.addRegion(base_start + 5, 0, width, .{ .prog_field = .{ .field = .bin, .bit = null } }); - // A binary digit IS a single bit, so clicking one flips it directly. - registerDigitRegions(app, base_start + 5, 11, bin, 1, .bin, true); // History - const hist_start = base_start + 7; + const hist_start = base_start + value_rows + 1; const hist_end = height -| 4; if (hist_start < hist_end) { tui.drawHistory(app.history.items, surface, hist_start, hist_end); @@ -176,11 +247,14 @@ fn drawFurniture( app.drawInput(surface, height -| 2); app.addRegion(height -| 2, 0, width, .focus_input); draw.fillRow(surface, height -| 1, ' ', .{ .fg = C.muted, .bg = C.bg }); + // Ordered so the keys that get you out of here survive a narrow terminal: + // `draw.hintRow` drops whole hints from the end. Programmer mode's list is the + // longest in the TUI at 101 characters and used to end on "Ctrl" at 80 columns. const status = if (focused == .bits) - "Arrows:nav | Space:toggle | Up/Down:field | Ctrl-W:width | Ctrl-E:endian | Ctrl-F:float | Tab:mode" + "Tab:mode | Arrows:nav | Space:toggle | Up/Down:field | Ctrl-W:width | Ctrl-E:endian | Ctrl-F:float" else - "Up/Down:field | Enter:set | Ctrl-W:width | Ctrl-E:endian | Ctrl-F:float | Tab:mode | ?:help"; - draw.writeStr(surface, height -| 1, 1, status, .{ .fg = C.muted, .bg = C.bg }); + "?:help | Tab:mode | Up/Down:field | Enter:set | Ctrl-W:width | Ctrl-E:endian | Ctrl-F:float"; + draw.hintRow(surface, height -| 1, 1, status, .{ .fg = C.muted, .bg = C.bg }); } /// Register clickable regions over the "Bits: / Signed: / Endian:" config line @@ -219,19 +293,16 @@ fn registerDigitRegions( bits_per_digit: u8, field: tui.App.ProgField, toggles: bool, + run: DigitRun, ) void { - var displayed_digits: u16 = 0; - for (text) |ch| { - if (ch != ' ') displayed_digits += 1; - } - if (displayed_digits == 0) return; + if (run.total == 0) return; var text_col: u16 = col; var digit_idx: u16 = 0; for (text) |ch| { if (ch != ' ') { // Digits are drawn MSB-first; convert to a bit offset from the LSB. - const from_lsb: u16 = displayed_digits - 1 - digit_idx; + const from_lsb: u16 = run.total - 1 - (run.before + digit_idx); const bit_pos: u32 = @as(u32, from_lsb) * bits_per_digit; if (bit_pos < 128) { const bit: u7 = @intCast(bit_pos); @@ -247,36 +318,38 @@ fn registerDigitRegions( } } -/// Draw a field's display string with a cursor highlighting the digit at bit_cursor position. -/// `bits_per_digit` is 4 for hex, 3 for oct, 1 for bin. +/// Draw a run of a field's digits with the cursor highlighting the one that holds +/// `bit_cursor`. `bits_per_digit` is 4 for hex, 3 for oct, 1 for bin. /// /// `text` is a grouped display string: digits and spaces, never a `0x`/`0o`/`0b` /// prefix. This used to skip a prefix and draw it unhighlighted, which was dead -/// code in both this function and `registerDigitRegions`. -fn drawFieldWithCursor(surface: *vxfw.Surface, row: u16, col: u16, text: []const u8, bit_cursor: u7, bits_per_digit: u8, total_bits: u8, color: vaxis.Cell.Color) void { - _ = total_bits; - - var displayed_digits: u16 = 0; - for (text) |ch| { - if (ch != ' ') displayed_digits += 1; - } - - // The cursor is at bit_cursor, which represents a digit position from LSB - const cursor_digit_from_lsb: u16 = @as(u16, bit_cursor) / bits_per_digit; - // Convert to position from MSB (0 = leftmost displayed digit) - const cursor_digit_from_msb: u16 = if (cursor_digit_from_lsb < displayed_digits) - displayed_digits - 1 - cursor_digit_from_lsb +/// code in both this function and `registerDigitRegions`. It also took a +/// `total_bits` it never read; `run.total` is the count it actually needs. +fn drawFieldWithCursor( + surface: *vxfw.Surface, + row: u16, + col: u16, + text: []const u8, + bit_cursor: u7, + bits_per_digit: u8, + color: vaxis.Cell.Color, + run: DigitRun, +) void { + // The cursor names a bit. The digit holding it is counted from the LSB, and the + // digits are drawn from the MSB. + const cursor_from_lsb: u16 = @as(u16, bit_cursor) / bits_per_digit; + const cursor_digit: u16 = if (cursor_from_lsb < run.total) + run.total - 1 - cursor_from_lsb else 0; - // Draw digits with cursor highlight var text_col: u16 = col; var digit_idx: u16 = 0; for (text) |ch| { if (ch == ' ') { draw.writeChar(surface, row, text_col, ' ', .{ .fg = color }); } else { - const style: vaxis.Style = if (digit_idx == cursor_digit_from_msb) + const style: vaxis.Style = if (run.before + digit_idx == cursor_digit) .{ .fg = C.bg, .bg = color, .bold = true } else .{ .fg = color }; diff --git a/src/tui/test_render.zig b/src/tui/test_render.zig index 069b363..4b3347c 100644 --- a/src/tui/test_render.zig +++ b/src/tui/test_render.zig @@ -19,7 +19,11 @@ //! on `height-2`, and a status line on `height-1`. Content that overflows its //! region lands on one of those rows, so checking them detects the overflow. //! - **Right-edge clipping.** `draw.writeStr` clips instead of erroring, so text -//! that reaches the final column has almost certainly lost its tail. +//! that reaches the final column has lost its tail. `noTextClipped` was here for +//! a long time with no caller outside its own test, and wiring it into the render +//! tests found two live faults: the 64-bit binary row needed 90 columns and was +//! cut short on an 80-column terminal, losing its low bits, and every mode's +//! status line lost its last hint mid-word at narrow widths. //! - **Cell content**, via `contains` and `rowOf`. //! //! Style is NOT observable: `flatten` keeps graphemes and discards attributes, so @@ -28,6 +32,7 @@ //! and say so. const std = @import("std"); +const log = std.log.scoped(.test_render); const vaxis = @import("vaxis"); const vxfw = vaxis.vxfw; const tui = @import("../tui.zig"); @@ -49,7 +54,7 @@ pub fn frame(arena: std.mem.Allocator, app: *tui.App, width: u16, height: u16) ! /// Read a surface back as text. Multi-byte graphemes become a single space: the /// TUI is ASCII by design, so anything wider is a bug this does not need to model. -pub fn flatten( +fn flatten( arena: std.mem.Allocator, surface: vxfw.Surface, width: u16, @@ -132,10 +137,11 @@ pub fn furniture(rows: [][]u8) Furniture { } /// Rows whose final column holds something other than a space, i.e. rows whose -/// text ran to the edge and was probably clipped. +/// text ran to the edge and lost its tail. /// -/// The separator rows legitimately fill the width, so callers pass how many of -/// those to expect rather than getting a bare boolean. +/// `draw.writeStr` clips silently, so this is the only way a too-long row shows up +/// in a test. Separator rows fill the width on purpose and are counted too, which is +/// why `noTextClipped` takes the number to expect rather than guessing. pub fn rowsReachingRightEdge(rows: [][]u8) usize { var count: usize = 0; for (rows) |row| { @@ -145,11 +151,68 @@ pub fn rowsReachingRightEdge(rows: [][]u8) usize { return count; } -/// True when no row other than the separator reaches the right edge. -pub fn noTextClipped(rows: [][]u8) bool { - // One row may reach the edge: the separator, which is drawn full width on - // purpose. Anything more means a line of text hit the boundary. - return rowsReachingRightEdge(rows) <= 1; +/// True when exactly `full_width_rows` rows reach the right edge, which is how many +/// the layout draws full width on purpose: one for an input mode (the separator +/// above the prompt), none for the help overlay. +/// +/// Exact rather than a ceiling, so a separator that stopped being drawn fails here +/// too. Status lines are kept clear of the final column by `draw.hintRow`, and the +/// value rows by the width checks in the views, so anything above the expected count +/// is a row that outgrew its terminal. +pub fn noTextClipped(rows: [][]u8, full_width_rows: usize) bool { + return rowsReachingRightEdge(rows) == full_width_rows; +} + +/// What a frame got wrong. +pub const Fault = union(enum) { + /// Content landed on the separator, the prompt or the status row. + furniture: Furniture, + /// More or fewer rows reached the right edge than the layout draws full width. + edges: struct { found: usize, expected: usize }, +}; + +/// The two faults a frame can have that are visible from here, or null when it has +/// neither: furniture in place, and only the intended rows reaching the right edge. +/// +/// Separate from `expectSound` so the decision can be tested without a failing test +/// to trigger it, and so the reporting has one home. +pub fn fault(rows: [][]u8, full_width_rows: usize) ?Fault { + const bottom = furniture(rows); + if (!bottom.intact()) return .{ .furniture = bottom }; + const found = rowsReachingRightEdge(rows); + if (found != full_width_rows) { + return .{ .edges = .{ .found = found, .expected = full_width_rows } }; + } + return null; +} + +/// The baseline every rendered frame has to meet, with the rows at fault reported. +/// +/// A bare `expect(false)` on a frame says nothing about which row grew, and these are +/// the two checks every render test wants, so they are one call rather than two that +/// drift apart. +/// +/// Reported through `std.log` rather than `std.debug.print` because this is a +/// function rather than a test body, and the lint that separates the two is right: +/// only a test should write straight to stderr. +pub fn expectSound(rows: [][]u8, full_width_rows: usize) !void { + switch (fault(rows, full_width_rows) orelse return) { + .furniture => |bottom| { + log.err("furniture overdrawn: separator={} prompt={} status={}", .{ + bottom.separator, bottom.prompt, bottom.status, + }); + const first = rows.len -| 3; + for (rows[first..], first..) |row, i| log.err(" row {d}: [{s}]", .{ i, row }); + return error.FurnitureOverdrawn; + }, + .edges => |counts| { + log.err("{d} rows reach the right edge, expected {d}:", .{ counts.found, counts.expected }); + for (rows, 0..) |row, i| { + if (row.len > 0 and row[row.len - 1] != ' ') log.err(" row {d}: [{s}]", .{ i, row }); + } + return error.RowReachedRightEdge; + }, + } } // -- Tests for the harness itself -- @@ -269,12 +332,64 @@ test "rowsReachingRightEdge counts only rows whose text hits the boundary" { "----------", }, 10); try testing.expectEqual(@as(usize, 2), rowsReachingRightEdge(rows)); - // Two rows at the edge is one more than the separator, so text was clipped. - try testing.expect(!noTextClipped(rows)); + // Two rows at the edge where one separator was expected: text was clipped. + try testing.expect(!noTextClipped(rows, 1)); + // And two where two are expected is fine. + try testing.expect(noTextClipped(rows, 2)); const clean = try syntheticFrame(arena_state.allocator(), &.{ "short", "----------", }, 10); - try testing.expect(noTextClipped(clean)); + try testing.expect(noTextClipped(clean, 1)); + // A separator that stopped being drawn fails the same check, which a ceiling + // would have let through. + try testing.expect(!noTextClipped(clean, 0)); +} + +test "fault names what a frame got wrong, and stays quiet when nothing is" { + var arena_state = std.heap.ArenaAllocator.init(testing.allocator); + defer arena_state.deinit(); + const arena = arena_state.allocator(); + + const sound = try syntheticFrame(arena, &.{ + " content", + "----------", + " > 2 + 2", + " ?:help", + }, 10); + try testing.expect(fault(sound, 1) == null); + + // A chip drawn over the prompt: reported as furniture rather than as an edge, + // since that is the more specific fault and the one worth showing first. + const overdrawn = try syntheticFrame(arena, &.{ + " content", + "----------", + " CAGR", + " ?:help", + }, 10); + switch (fault(overdrawn, 1).?) { + .furniture => |bottom| try testing.expect(!bottom.prompt), + .edges => return error.WrongFault, + } + + // A value row that ran to the last column, which is one more full-width row than + // the separator. + const clipped = try syntheticFrame(arena, &.{ + "0123456789", + "----------", + " > ", + " ?:help", + }, 10); + switch (fault(clipped, 1).?) { + .edges => |counts| { + try testing.expectEqual(@as(usize, 2), counts.found); + try testing.expectEqual(@as(usize, 1), counts.expected); + }, + .furniture => return error.WrongFault, + } + + // And a frame the caller says draws nothing full width, where the separator + // itself is the surprise. + try testing.expect(fault(sound, 0) != null); }