From 599dd82f96b9d091aae78300aa6c3fbc81f9eb69 Mon Sep 17 00:00:00 2001 From: Gabriel Schneider Date: Sun, 9 Aug 2026 06:54:27 -0300 Subject: review pass: fix the eaten Tab, drop the duplicated code, cover the gaps --- src/pardes.zig | 307 +++++++++++++++++++-------------------------------------- 1 file changed, 100 insertions(+), 207 deletions(-) (limited to 'src/pardes.zig') diff --git a/src/pardes.zig b/src/pardes.zig index 0dba0abf..592dd059 100644 --- a/src/pardes.zig +++ b/src/pardes.zig @@ -1919,75 +1919,6 @@ test "an untouched tagline ends where its layout column's widest one does" { ); } -test "tagPrefixLen agrees with tagPrefix for every pane kind" { - const gpa = std.testing.allocator; - const p = try Pardes.init(gpa, .{ .cols = 100, .rows = 30 }); - defer p.deinit(); - while (p.nextEffect()) |_| {} - - const same = struct { - fn f(pp: *Pardes, pane: *Pane) !void { - const s = try pp.tagPrefix(pane); - try std.testing.expectEqual(s.len, Pardes.tagPrefixLen(pane)); - } - }.f; - - // terminal: whatever boot gave it, then empty, then a changed cwd - const term = p.panes[0].?; - try same(p, term); - p.setCwd(0, ""); - try same(p, term); - p.setCwd(0, "/some/where/deep/enough/to/matter"); - try same(p, term); - p.setCwd(0, "/caf\u{e9}/\u{5b50}"); // multibyte: BYTES, both sides - try same(p, term); - - // an output buffer (+Help) — a file pane whose path the table names - const help_id = p.freeSlot().?; - _ = try p.newDocPane(help_id); - output_pane.openHelp(p, help_id, ""); - try same(p, p.panes[help_id].?); - - // an image - const img_id = p.freeSlot().?; - const img = try p.newDocPane(img_id); - img.image = .{ .path = try gpa.dupe(u8, "/tmp/pardes-parity/pic.ppm") }; - try same(p, img); - - // an ordinary file pane (this one replaces pane 0, so it goes last) - const file = try p.hxOpenFileContent("hello\n"); - try same(p, file); -} - -test "tagPrefixLen agrees with tagPrefix for a pdf pane" { - if (!pdf_enabled or platform == .web) return; - const gpa = std.testing.allocator; - var tmp = std.testing.tmpDir(.{}); - defer tmp.cleanup(); - const fixture = try pdf_impl.makeOutlineTestPdf(gpa); - defer gpa.free(fixture); - try tmp.dir.writeFile(std.testing.io, .{ .sub_path = "outline.pdf", .data = fixture }); - var path_buf: [256]u8 = undefined; - const path = try std.fmt.bufPrint(&path_buf, ".zig-cache/tmp/{s}/outline.pdf", .{tmp.sub_path}); - - const p = try Pardes.init(gpa, .{ .file = path, .cols = 100, .rows = 30 }); - defer p.deinit(); - while (p.nextEffect()) |_| {} - const pdf_pane = p.panes[0].?; - try std.testing.expect(pdf_pane.pdf != null); - const s0 = try p.tagPrefix(pdf_pane); - try std.testing.expectEqual(s0.len, Pardes.tagPrefixLen(pdf_pane)); - // ...and on a later page / another fit / another tint, where the digits - // and the @tagName words change width - pdf_pane.pdf.?.page = 9; - pdf_pane.pdf.?.page_count = 100; - const s1 = try p.tagPrefix(pdf_pane); - try std.testing.expectEqual(s1.len, Pardes.tagPrefixLen(pdf_pane)); - pdf_pane.pdf.?.page = 99; - const s2 = try p.tagPrefix(pdf_pane); - try std.testing.expectEqual(s2.len, Pardes.tagPrefixLen(pdf_pane)); -} - test "legacy default tag tails upgrade while custom tails remain owned" { const gpa = std.testing.allocator; const p = try Pardes.init(gpa, .{}); @@ -3971,9 +3902,10 @@ test "a corner drag's two axes clamp independently" { const ay: u16 = TOPBAR_H; const ah: u16 = 15; const bh: u16 = 14; - // the walls, spelled out: the handle is the upper pane's LAST row, so the - // upper pane bottoms out with its tag row alone at ay, and the lower pane - // does the same one row above the pair's end + // the walls, spelled out for tags-on-top (the tag_bottom = false below), + // where the handle is the upper pane's LAST row: the upper pane bottoms out + // with its tag row alone at ay, and the lower pane does the same one row + // above the pair's end const row_lo: u16 = ay + BOX_H - 1; const row_hi: u16 = ay + ah + bh - (BOX_H + 1); @@ -4662,9 +4594,6 @@ pub const Pardes = struct { // ---- tag + selection text (chord sources) ---- - // shared by tagPrefix and tagPrefixLen below, which have to agree - const pdf_tag_fmt = "pdf {d}/{d} {s} PdfFit {s} PdfTint PdfSections {s}"; - /// the live tag prefix: the pane's cwd/path, and nothing else (an image /// still names its kind — the renderer toggles it used to spell out are /// builtins now, under SPC t). The mode used to lead this as a word; it is @@ -4679,7 +4608,7 @@ pub const Pardes = struct { const arena = p.scratch.allocator(); if (comptime pdf_enabled) if (pane.pdf) |pv| return std.fmt.allocPrint( arena, - pdf_tag_fmt, + "pdf {d}/{d} {s} PdfFit {s} PdfTint PdfSections {s}", .{ pv.page + 1, pv.page_count, @tagName(pv.fit), @tagName(pv.tint), pv.path }, ); if (pane.image) |iv| return std.fmt.allocPrint(arena, config.tag_image ++ " {s}", .{iv.path}); @@ -4687,20 +4616,6 @@ pub const Pardes = struct { return arena.dupe(u8, pane.cwdSlice()); } - /// the same prefix's LENGTH, without the allocation. tagGap measures every - /// pane in a layout column, once per pane per frame, and has no arena — - /// formatting a path just to ask how wide it is would put the whole column - /// on the render hot path. Mirror any change to tagPrefix here. - fn tagPrefixLen(pane: *const Pane) usize { - if (comptime pdf_enabled) if (pane.pdf) |pv| return std.fmt.count( - pdf_tag_fmt, - .{ pv.page + 1, pv.page_count, @tagName(pv.fit), @tagName(pv.tint), pv.path }, - ); - if (pane.image) |iv| return config.tag_image.len + 1 + iv.path.len; - if (pane.file) |f| return f.path.len; - return pane.cwdSlice().len; - } - /// the editable tail: the user's edited buffer once touched, else defaults /// (a buffer with nothing to Save gets the plain tail — the table decides) fn curTail(pane: *Pane) []const u8 { @@ -4766,7 +4681,14 @@ pub const Pardes = struct { /// every keystroke. It still takes no gap of its own (above) — but it /// has to keep voting, or clicking the widest tagline in a column would /// snap every other one left, out from under the next click. - fn tagGap(p: *const Pardes, pane: *const Pane, used: usize) usize { + /// + /// ponytail: every voter's prefix is FORMATTED to be measured, so a frame + /// costs up to MAX_PANES² path dupes — 256 bump allocations into the + /// scratch arena that renderPane resets anyway, at a realistic two to four + /// panes. The alternative was a second tagPrefix that only counted, and + /// keeping two spellings of one string in step by comment is the more + /// expensive kind of cost. + fn tagGap(p: *Pardes, pane: *const Pane, used: usize) usize { if (pane.tag_init) return 0; const id = p.paneIdOf(pane) orelse return 0; const r = p.rects[id]; @@ -4785,7 +4707,7 @@ pub const Pardes = struct { pane_tail; const laid = if (q.tag_init) q.tag_tail.items else words; const lead = laid.len - std.mem.trimStart(u8, laid, " ").len; - const q_end = tagPrefixLen(q) + lead + std.mem.trimStart(u8, words, " ").len; + const q_end = (p.tagPrefix(q) catch continue).len + lead + std.mem.trimStart(u8, words, " ").len; end = @max(end, @min(q_end, tw)); }; return end -| used; @@ -7326,48 +7248,12 @@ pub const Pardes = struct { .find => .{ .cmd = .Find }, .grep => .{ .cmd = .Grep }, }; - // The SAME search asked again REFILLS the list it already opened — - // right-clicking a word in four places is one +Search walked four - // times, not four +Searches over identical rows. A different pattern - // still gets its own buffer, and that IS the old rule: two searches are - // two lists, both stay open at their sizes, and the new one stacks - // directly below this pane (placeDoc). Focus stays here either way. - // ...and it is ANY open list this search already filled, not only the - // one n/N are armed on: search `foo`, then `bar`, then `foo` again and - // the third search re-arms foo's own buffer rather than opening its - // identical twin below it. Same directory only — the rows are written - // relative to it, so another dir's list is a different list. Never the - // searching pane itself (a `/` inside a +Search writes its own rows). - for (p.panes, 0..) |slot, i| { - if (i == id) continue; - const rp = slot orelse continue; - const rf = if (rp.file) |*f| f else continue; - const o = rf.output orelse continue; - if (!std.meta.eql(o.from, from) or !std.mem.eql(u8, o.arg(), pat)) continue; - if (!std.mem.eql(u8, std.fs.path.dirname(rf.path) orelse "", dir)) continue; - // a refill that changes NOTHING keeps its place: a right click on - // an already-armed word is an `n`, and throwing the list back to - // the top only to scroll down to the stepped row is a jump with no - // information in it. - const same = std.mem.eql(u8, rf.content, content); - file_pane.setContent(p, rf, content); - if (!same) rf.scroll = 0; - pane.search_pane = i; - pane.search_row = anchor; - return; - } - const free = p.freeSlot() orelse { - p.gpa.free(content); - return; - }; - const np = output_pane.open(p, free, dir, from, pat, content) catch { - p.gpa.free(content); - return; - }; - p.placeDoc(id, free, np); - p.active = id; - pane.search_pane = free; - pane.search_row = anchor; + // A different pattern still gets its own buffer: two searches are two + // lists, both stay open at their sizes, and the new one stacks directly + // below this pane. Everything about landing the rows — which open + // buffer counts as this same search, keeping a refill's place, opening + // fresh when there is none — is output_pane.fillResults. + output_pane.fillResults(p, id, dir, from, pat, content, anchor); } /// n/N: step to the next/previous row of this pane's results buffer and @@ -7509,61 +7395,10 @@ pub const Pardes = struct { const dir = if (pane.file) |f| (std.fs.path.dirname(f.path) orelse "/") else pane.cwdSlice(); const content = p.gpa.dupe(u8, rows) catch return; - // The same KIND asked again REFILLS the buffer it already opened, the - // rule runSearch has always had. It was missing here and that was - // survivable while every language query was a deliberate press: `gr` - // twice left two identical lists and you closed one. Tab after a dot - // is an ordinary typing keystroke, which turns the same bug fatal — - // measured, twenty Tabs stacked fifteen byte-identical `+Search` panes - // under the file, crushed it to one visible line, and from the - // sixteenth on freeSlot returned null and the key was eaten for the - // rest of the session with nothing said. One code path, so every kind - // that lands in a results buffer gets the fix. - // - // Unlike runSearch this does NOT key on the ARGUMENT. A search is - // identified by its pattern; a language query is asked about a - // different symbol every time with the same (usually empty) arg, so - // the arg cannot tell two lists apart and the KIND is the natural - // unit: a second `gr` replaces the first list rather than growing a - // stack of them. - for (p.panes, 0..) |slot, i| { - if (i == w.pane) continue; - const rp = slot orelse continue; - const rf = if (rp.file) |*f| f else continue; - const o = rf.output orelse continue; - if (!std.meta.eql(o.from, from)) continue; - if (!std.mem.eql(u8, std.fs.path.dirname(rf.path) orelse "", dir)) continue; - output_pane.setArg(&rf.output.?, w.arg.slice()); - // a refill that changes nothing keeps its place (runSearch's rule, - // and the same reason): re-asking about a symbol you are already - // stepping must not throw the list back to the top - const same = std.mem.eql(u8, rf.content, content); - file_pane.setContent(p, rf, content); - if (!same) rf.scroll = 0; - p.active = w.pane; - if (output_pane.traits(from).steps) { - pane.search_pane = i; - pane.search_row = null; - } - return; - } - const free = p.freeSlot() orelse { - p.gpa.free(content); - return; - }; - const np = output_pane.open(p, free, dir, from, w.arg.slice(), content) catch { - p.gpa.free(content); - return; - }; - p.placeDoc(w.pane, free, np); - p.active = w.pane; - // prose is not a list of locations: n/N over a hover blurb would step - // to nowhere, so only stepping buffers arm the stepper — and WHICH - // query filled it is now the buffer's own record, not a field here. - if (output_pane.traits(from).steps) { - pane.search_pane = free; - pane.search_row = null; - } + // Landing the rows is runSearch's path exactly, keyed on the KIND + // rather than the argument (fillResults reads that off the origin). + // Why the refill is not optional here: docs/lsp.md. + output_pane.fillResults(p, w.pane, dir, from, w.arg.slice(), content, null); } /// n/N on a terminal pane: a MOTION over lookable tokens. Select the @@ -8024,7 +7859,14 @@ pub const Pardes = struct { if (!p.multi_on and c.col > 0 and c.col <= ln.len and ln[c.col - 1] == '.') dot: { const f = pane.file orelse break :dot; if (f.output != null or !lsp.speaks(f.path)) break :dot; - return p.lspRequest(p.active, .completion, ""); + // speaks() is the fast path only — lspRequest has four + // bails of its own (unsupported kind, dead pane, output + // buffer, multiOnce) and each one would eat the Tab. The + // seq bump is the one honest "the question went out", so + // ask and fall through to the indent if it did not. + const seq = p.lsp_seq; + p.lspRequest(p.active, .completion, ""); + if (p.lsp_seq != seq) return; } p.insertTab(pane); }, @@ -8033,11 +7875,8 @@ pub const Pardes = struct { } /// helix insert_tab with a Spaces indent style: spaces to the next tab - /// stop (smart-tab machinery skipped). It is a function rather than the - /// five lines it used to be inside the Tab prong because Tab after a `.` - /// asks the language backend FIRST and indents only if the answer comes - /// back empty — which happens on another turn of the loop entirely, so - /// lspResponse needs to be able to press the same key. + /// stop (smart-tab machinery skipped). A function because lspResponse + /// presses the same key, a turn of the loop later. fn insertTab(p: *Pardes, pane: *Pane) void { const eb = p.editText(pane, pane.cur_row, pane.cur_row, pane.cur_col) orelse return; const c: modal.Cursor = .{ @@ -8045,8 +7884,7 @@ pub const Pardes = struct { .col = @intCast(@max(0, pane.cur_col)), }; const pad = modal.INDENT_W - (c.col % modal.INDENT_W); - const spaces = " "; - const new = modal.insertAt(p.gpa, eb.text, c, spaces[0..pad]) catch return; + const new = modal.insertAt(p.gpa, eb.text, c, " "[0..pad]) catch return; p.setEditText(pane, new); pane.cur_col += @intCast(pad); pane.cur_pinned = true; @@ -9039,6 +8877,69 @@ pub const Pardes = struct { }); } + pub const ChromeTarget = struct { col: u16, row: u16 }; + + /// Is this cell layout CHROME, and if so which cell should the press be + /// delivered at? For the touch shells: a finger on a tag row or a resize + /// handle latches a left-mouse drag, everything else is body text and gets + /// one-finger scrolling and tap-as-look. A gesture is classified once, at + /// finger-down, and never turns into a scroll afterwards. + /// + /// It lives here because it is a MIRROR of handleMouse's own hit test + /// below, in both the geometry and the ORDER: the move box beats a + /// horizontal handle on a tag-only pane, the rest of the tag row beats the + /// fat-finger tolerance around a separator, and Tagbottom moves both the + /// tag row and the h-handle together (a pane's tag on its LAST row makes + /// its first an ordinary body row and puts the seam on the lower pane's + /// first). It was a line-for-line clone in web.zig and gui.zig, kept in + /// step by a comment in each saying it was a clone of the other; the two + /// conditionals Tagbottom added went into both copies four times. + /// + /// The one-cell tolerance is the only thing here that is not handleMouse's + /// rule: a mouse is exact, a finger is not. + pub fn chromeTarget(p: *const Pardes, col: u16, row: u16) ?ChromeTarget { + if (row < TOPBAR_H) return .{ .col = col, .row = row }; + for (p.panes, 0..) |slot, id| { + if (slot == null) continue; + const rect = p.rects[id]; + const tag = if (p.tag_bottom) rect.y + rect.h -| BOX_H else rect.y; + if (row == tag and col >= rect.x and col < rect.x + rect.w and col < rect.x + config.GUTTER) + return .{ .col = col, .row = tag }; + } + for (0..p.ncol -| 1) |column| { + const handle = p.col_x[column] + p.col_w[column] -| 1; + if (col == handle) return .{ .col = handle, .row = row }; + } + for (0..p.ncol) |column| { + if (col < p.col_x[column] or col >= p.col_x[column] + p.col_w[column]) continue; + for (0..p.col_n[column] -| 1) |index| { + const rect = p.rects[p.col_terms[column][index]]; + const handle = if (p.tag_bottom) rect.y +| rect.h else rect.y + rect.h -| 1; + if (row == handle) return .{ .col = col, .row = handle }; + } + } + for (p.panes, 0..) |slot, id| { + if (slot == null) continue; + const rect = p.rects[id]; + const tag = if (p.tag_bottom) rect.y + rect.h -| BOX_H else rect.y; + if (row == tag and col >= rect.x and col < rect.x + rect.w) + return .{ .col = col, .row = tag }; + } + for (0..p.ncol -| 1) |column| { + const handle = p.col_x[column] + p.col_w[column] -| 1; + if (@max(col, handle) - @min(col, handle) == 1) return .{ .col = handle, .row = row }; + } + for (0..p.ncol) |column| { + if (col < p.col_x[column] or col >= p.col_x[column] + p.col_w[column]) continue; + for (0..p.col_n[column] -| 1) |index| { + const rect = p.rects[p.col_terms[column][index]]; + const handle = if (p.tag_bottom) rect.y +| rect.h else rect.y + rect.h -| 1; + if (@max(row, handle) - @min(row, handle) == 1) return .{ .col = col, .row = handle }; + } + } + return null; + } + fn handleMouse(p: *Pardes, m: Mouse) void { const mcol = @min(m.col, p.screen_w -| 1); const mrow = @min(m.row, p.screen_h -| 1); @@ -9135,12 +9036,6 @@ pub const Pardes = struct { // pair that is a BODY row — the upper pane's last, or with // Tagbottom, where that one is the upper pane's tag, the // lower pane's first). - // Either way the handle EATS one body row of one of the two - // panes — a press there resizes instead of placing a cursor - // — and it always has. Tagbottom does not change how many - // rows that costs, only WHICH pane pays: the lower pane's - // first row rather than the upper pane's last. A no-drag - // click on it is a no-op in both orientations. // The v test still wins outright, but it now also asks // whether this same cell is one of ITS OWN column's h // handles — that cell is the corner where the two lines @@ -9359,8 +9254,6 @@ pub const Pardes = struct { // .tag drag) stays on the tag, which is one line anyway. // The cost is that a body sweep can no longer be extended // onto the bottom tagline to pick up the tag text. - // With the tag on TOP, Sel order IS screen order and the - // gesture is contiguous and correct, so nothing is clamped. if (p.tag_bottom) { const body_h = r.h -| BOX_H; pane.sel[b].r1 = if (pane.sel[b].r0 < BOX_H or body_h == 0) @@ -12191,7 +12084,7 @@ pub const Pardes = struct { try p.renderPane(arena, pane, p.rects[id], id == p.active); } - // ---- the transient message row: a pane's LAST row, left-aligned ---- + // ---- the transient message row: the end away from the tag ---- // // An OVERLAY, not geometry: no rect moves, no pane shrinks, and a pane // with neither a message nor an armed prompt is not touched at all. @@ -12422,7 +12315,7 @@ pub const Pardes = struct { // body's first. The Tagbottom builtin swaps which end each is at and // NOTHING else in here reads r.y — that is the whole of the feature on // the render side. r.h == 0 returned above, so the bottom row exists. - const tag_y = if (p.tag_bottom) r.y + r.h - 1 else r.y; + const tag_y = if (p.tag_bottom) r.y + r.h -| BOX_H else r.y; const body_y = if (p.tag_bottom) r.y else r.y + BOX_H; // the pane's own background, for everything that has to read as "no // chrome here": the body text, and the blank right half of the -- cgit v1.3