aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authoruser <user@clank>2026-08-13 18:46:58 +0200
committeruser <user@clank>2026-08-13 18:46:58 +0200
commit2f0750285b1b174a552113c4aed9288f44985ae3 (patch)
treee8033ba0b3d3ac04e4e49fb4fa53f7049ac21499
parentdocs: handover (diff)
downloadsl0ppi-2f0750285b1b174a552113c4aed9288f44985ae3.tar.gz
sl0ppi-2f0750285b1b174a552113c4aed9288f44985ae3.tar.xz
sl0ppi-2f0750285b1b174a552113c4aed9288f44985ae3.zip
sl0p: make the split button visible, and clickable where it is drawn
The `+` shipped in 0.1.7 and 0.1.8 without ever appearing on a live session, while rendering in 265 snapshots. Four faults, each enough on its own: - title_line_without_middle computed the right side only inside the left side's and_then, so a pane with no title (hide_default_pane_names, the live default) got none at all; - title_line_with_middle discarded the right side whenever the left was absent -- that is every frame in a session with a second client, which the live box always had, and "MY FOCUS AND:" on every frame was saying so; - the title was budgeted before the button, so a title longer than the frame (any shell showing a path) consumed its columns. The button is anchored and hit-tested by pure geometry, so it cannot be the part that loses; - the click hit-tested position_and_size() while the frame is drawn inset by the pane gap, putting the clickable columns two right of the glyph and a row above it at the top of the screen. Hence "the click works" and "nothing is drawn" both being true, and that click test proving nothing. Adds Pane::frame_rect(), the rect the frame is really drawn in, which the status bar buttons had already had to work around by hand. Also fixes render_my_focus building its short form from separators that Vec::append had already drained: 4 cells wide while reporting 6, a title line two cells short of its pane. Found by asserting the width in the new test. Verification, since unit tests have now been insufficient here four times: split_button_is_drawn_where_it_is_hit_tested covers the combinations the live config produces, and packaging/checks/split-button-live.py (make check-live) reads the glyph off a real session, attaches a second client to force the middle builder, and clicks the columns it found the glyph in. Both confirmed to fail on the parent commit.
-rw-r--r--Makefile4
-rw-r--r--docs/HANDOVER.md101
-rw-r--r--docs/roadmap.md31
-rwxr-xr-xpackaging/checks/split-button-live.py242
-rw-r--r--sl0ppij/zellij-server/src/panes/plugin_pane.rs8
-rw-r--r--sl0ppij/zellij-server/src/panes/terminal_pane.rs8
-rw-r--r--sl0ppij/zellij-server/src/tab/mod.rs11
-rw-r--r--sl0ppij/zellij-server/src/tab/mouse_handler.rs23
-rw-r--r--sl0ppij/zellij-server/src/tab/unit/snapshots/zellij_server__tab__tab_integration_tests__ctrl_scroll_up_merging_stacks_preserves_all_panes.snap5
-rw-r--r--sl0ppij/zellij-server/src/ui/pane_boundaries_frame.rs275
10 files changed, 615 insertions, 93 deletions
diff --git a/Makefile b/Makefile
index e5b58a7..65b9909 100644
--- a/Makefile
+++ b/Makefile
@@ -98,6 +98,10 @@ check: ## Cheap compile gate
test: cli-test ## All tests: the CLI, plus the crates we actually modify
cd $(FORK) && cargo test -p zellij-server --lib && cargo test -p zellij-utils --lib
+.PHONY: check-live
+check-live: dev ## Drive a real session and check the mouse affordances are drawn where they are clicked
+ packaging/checks/split-button-live.py $(FORK)/target/fastdev/$(BIN_NAME)
+
##@ Package
.PHONY: vendor
diff --git a/docs/HANDOVER.md b/docs/HANDOVER.md
index 467c29b..6f7f89b 100644
--- a/docs/HANDOVER.md
+++ b/docs/HANDOVER.md
@@ -2,54 +2,64 @@
Written at the end of a long session. Start here.
-## Pick this up first: the `+` split button does not render
+## Done: the `+` split button renders (and clicks where it is drawn)
**Feature:** a `+` at the right end of every pane's title line; clicking it
-splits that pane. Shipped in 0.1.7 (hover-gated) and 0.1.8 (always drawn).
+splits that pane. Shipped invisible in 0.1.7 and 0.1.8; fixed after both.
-**Status: the click works, the button is invisible on a live session.**
+It was never one bug. The title line has three builders, and the button was
+falling out of two of them, while the click was hit-testing a rect the frame is
+not drawn in:
-- clicking those 3 columns splits the pane — verified by injecting real SGR
- mouse events into an attached client's pty (`1 -> 2 panes`; clicking the title
- or mid-frame correctly does nothing)
-- the button renders in **265 unit snapshots**
-- it has never once been seen on a real session
+1. **No title, no right side.** `title_line_without_middle` only computed the
+ right side *inside* the left side's `and_then`, so a pane with an empty title
+ got none. Live sets `hide_default_pane_names true`, so unnamed panes have
+ exactly that.
+2. **A "MY FOCUS" middle dropped the right side entirely.**
+ `title_line_with_middle` fell through to `middle_only_title_line` whenever
+ the left side was absent. Any second client in the session puts a middle
+ there — including the phantom client of open item 1 below, which is why the
+ live box *always* took this path (`MY FOCUS AND:` on every frame was the
+ clue, and it was in the bug report all along).
+3. **A long title ate the button.** The title was budgeted first and the right
+ side got the remainder; shell titles are long paths. The anchored button is
+ now reserved before the title, since its hit-test is pure geometry and cannot
+ ask the renderer what fitted.
+4. **The click hit-tested `position_and_size()`**, the pane's allocation, while
+ the frame is drawn inset by the pane gap (`gap 1` → 2 columns, 1 row). The
+ clickable columns sat out in the blank gap, two right of the drawn `+`. This
+ is why "clicking works" and "nothing is drawn" were both true, and why that
+ click test proved nothing. There is now `Pane::frame_rect()` — the rect the
+ frame is actually drawn in — and both the button and (already) the status bar
+ buttons derive from it.
-Two wrong diagnoses so far, both mine:
+The centre-aligned path, the strong lead in the previous handover, was fine.
-1. "It's hover-gated and hover is broken" → removed the gate in 0.1.8, still
- invisible. (`mouse_is_hovering_over_pane` does appear to be false live, but
- that was not the only problem.)
-2. "The click test proves the feature" → it does not. The hit-test is purely
- geometric and never consults the renderer, so it kept working while nothing
- was drawn.
+**What was missing was the reproduction, not the analysis.** Two things now
+exist so this cannot come back:
-**The strong lead, untested:** every snapshot uses zellij's DEFAULT frame
-config. The live config does not:
+- `split_button_is_drawn_where_it_is_hit_tested` — asserts the button occupies
+ exactly `split_button_columns()` across alignments × rounded × titled/untitled
+ × one/two clients × 4 widths. Positional, not `contains("+")`: the short
+ multi-client indication is itself a `+`.
+- `packaging/checks/split-button-live.py` (`make check-live`) — spawns a real
+ session at a pty, reads the glyph off the screen, attaches a second client to
+ force the middle builder, then clicks the columns it *found the glyph in* and
+ confirms the split. Verified to fail on the pre-fix commit for both reasons
+ (button absent with 2 clients; click in the gap does nothing).
-```kdl
-pane_frames { rounded_corners true; title_alignment "center"; gap 1; padding 0 0 0 0 }
-```
-
-`render_title_right_side` is called from **three** different title builders
-(`zellij-server/src/ui/pane_boundaries_frame.rs`, lines ~889, ~915, ~983) with
-different `max_length` budgets. The snapshots only ever exercised the
-left-aligned path; `title_alignment "center"` takes another one.
-
-**Ten-second check before writing any code:** set `title_alignment "left"` in
-`~/.config/sl0ppij/config.kdl` on the test box, re-attach, look for the `+`. If
-it appears, that is the bug confirmed.
-
-**Then:** add a frame unit test using the live config's alignment/rounded/gap
-settings — the reproduction that was missing all along — and fix whichever
-builder drops the button.
+One more bug fell out of the width assertion: `render_my_focus` built its full
+and short forms from the same separator vectors, and `Vec::append` drains — so
+the short "ME" form rendered 4 cells wide while reporting 6, two cells short of
+the pane. Fixed.
Code involved:
| what | where |
|---|---|
-| render + geometry | `ui/pane_boundaries_frame.rs`: `render_split_button`, `split_button_columns`, `SPLIT_BUTTON_*` |
+| render + geometry | `ui/pane_boundaries_frame.rs`: `render_split_button`, `split_button_columns`, `anchored_right_side_len`, `SPLIT_BUTTON_*` |
| click | `tab/mouse_handler.rs`: `intercept_split_button_click` |
+| where the frame really is | `tab/mod.rs`: `Pane::frame_rect` (+ the two impls) |
Not drawn on floating panes on purpose: the PIN control owns that corner and is
hit-tested by its drawn position, so anything to its right moves it out from
@@ -75,8 +85,7 @@ URL afterwards.
## Open, in the order I would do them
-1. **The `+` rendering** (above).
-2. **Phantom client on headless `sl0ppi up`.** `attach --create-background`
+1. **Phantom client on headless `sl0ppi up`.** `attach --create-background`
leaves one client in the session forever, so every pane frame shows
`MY FOCUS AND:`. Worked around by creating the session *by attaching* when
there is a tty; the headless path still leaks. Ruled out: the CLI-action
@@ -86,14 +95,15 @@ URL afterwards.
`ScreenInstruction::RemoveClient` on a debug build to find where it is lost.
Cheap alternative: give the headless path its own pty (`script -qec`) and
kill the client afterwards — killed clients demonstrably do not leak.
-3. **bun is the AVX2 build.** Caused `Illegal instruction` on a plain KVM guest.
+2. **bun is the AVX2 build.** Caused `Illegal instruction` on a plain KVM guest.
One line in `packaging/fetch-vendor.sh`: `bun-linux-x64-baseline.zip`.
-4. **`pir` needs python3 + fzf**, neither shipped. The fix is folding it into
+3. **`pir` needs python3 + fzf**, neither shipped. The fix is folding it into
`sl0ppi agent --resume` (already designed in `docs/cli-design.md`).
-5. **Tab overflow.** `sl0ppi open <name>` wants to be a fuzzy picker once a root
+4. **Tab overflow.** `sl0ppi open <name>` wants to be a fuzzy picker once a root
has a dozen projects.
-6. Button polish: shift/right-click for the other split direction; reuse
- `status_bar_button_style` so it looks pressable.
+5. Button polish: shift/right-click for the other split direction; reuse
+ `status_bar_button_style` so it looks pressable. Extend `make check-live`
+ rather than trusting a snapshot.
Fixed this session, for context: client ids were recycled (silently ate ~25% of
`zellij action` output), suspended panes spammed resize errors, responsive
@@ -123,6 +133,15 @@ bar. See `docs/roadmap.md` for each with its measurements.
template into each tab instead of emitting `default_tab_template`.
- **The tab test harness does no rendering**, so `panes_to_hide` is always empty
and stack-related bugs cannot be reproduced in it. Use a scripted pty.
+- **A hit-test that never consults the renderer is not evidence of anything.**
+ The split button "worked" for two releases while being invisible. If an
+ affordance is drawn, the check has to *find the glyph* and click where it
+ found it — `packaging/checks/split-button-live.py` is the pattern.
+- **`os.read` on a settled pty blocks forever.** `os.set_blocking(fd, False)`
+ and catch `BlockingIOError`, or a live check hangs instead of failing.
+- **The pane's geometry is not where its frame is drawn.** With a gap, the frame
+ is inset from the allocation: use `Pane::frame_rect()` for anything the user
+ can see, `position_and_size()` only for the layout solver.
- **Clean up test sessions.** A background-created session I left behind is what
produced a "MY FOCUS" bug report from the user.
diff --git a/docs/roadmap.md b/docs/roadmap.md
index bc8aa6f..98c8949 100644
--- a/docs/roadmap.md
+++ b/docs/roadmap.md
@@ -215,6 +215,37 @@ Two panes sharing one rect means a stack; distinct rects mean it really
expanded. Check both directions (create narrow then widen, create wide then
narrow) and add panes afterwards.
+## Fixed: the `+` split button was drawn nowhere and clicked in the gap
+
+Shipped in 0.1.7 and 0.1.8 without ever being visible on a live session, while
+rendering in 265 unit snapshots. Four separate faults, each individually enough
+to hide it:
+
+- `title_line_without_middle` computed the right side only if the pane had a
+ title -- and `hide_default_pane_names true` means unnamed panes have none;
+- `title_line_with_middle` dropped the right side whenever the left was absent,
+ which is every frame in a session with a second client (`MY FOCUS AND:`);
+- the title was budgeted before the button, so a title longer than the frame
+ (i.e. any shell showing a path) consumed its columns;
+- the click hit-tested the pane's allocation while the frame is drawn inset by
+ the pane gap, putting the clickable columns two to the right of the glyph.
+
+The first three are fixed in `ui/pane_boundaries_frame.rs` by reserving the
+anchored button before anything else and never discarding the right side; the
+fourth by `Pane::frame_rect()`, the rect the frame is actually drawn in.
+
+**How to re-run the check** -- unit tests were necessary and insufficient here
+for the fourth time, so this one reads the screen:
+
+```bash
+make check-live # packaging/checks/split-button-live.py
+```
+
+It starts a session at a pty with the live frame config, finds the `+` on the
+frame, attaches a second client (the middle builder), and clicks the columns the
+glyph was found in, expecting `2 -> 3` panes. Confirmed to fail on the commit
+before the fix, for both the render and the click.
+
## Standing risks
**`sl0ppij/target` is a symlink to `~/lol/zellij/target`.** Introduced when the
diff --git a/packaging/checks/split-button-live.py b/packaging/checks/split-button-live.py
new file mode 100755
index 0000000..51b8e25
--- /dev/null
+++ b/packaging/checks/split-button-live.py
@@ -0,0 +1,242 @@
+#!/usr/bin/env python3
+"""Live check: is the `+` split button DRAWN, and does clicking the drawn glyph
+split the pane?
+
+The unit tests can only prove the title line composes correctly. This drives a
+real session with the live config's frame settings -- centred titles, rounded
+corners, a pane gap, no default pane names -- because every one of those takes a
+different path through the title builders, and the gap moves the drawn frame
+away from the pane's allocation.
+
+It then attaches a SECOND client, because that is what the live box always had
+(the phantom client a headless `sl0ppi up` leaks) and it switches the frame onto
+a third title builder again -- the one with a "MY FOCUS" middle, which is where
+the button was being dropped for two releases.
+
+Usage: split-button-live.py [path-to-sl0ppij]
+Exits non-zero with a description of what it saw.
+"""
+
+import fcntl
+import os
+import pty
+import re
+import struct
+import subprocess
+import sys
+import tempfile
+import termios
+import time
+
+BIN = os.path.abspath(
+ sys.argv[1]
+ if len(sys.argv) > 1
+ else os.path.join(os.path.dirname(__file__), "../../sl0ppij/target/fastdev/sl0ppij")
+)
+ROWS, COLS = 30, 100
+SESSION = f"sl0ptest-splitbutton-{os.getpid()}"
+
+CONFIG = """
+ui {
+ pane_frames {
+ rounded_corners true
+ title_alignment "center"
+ gap 1
+ padding 0 0 0 0
+ gap_aspect 2
+ hide_default_pane_names true
+ }
+}
+pane_frames true
+pane_frame_style "full"
+default_mode "locked"
+show_startup_tips false
+"""
+
+ANSI = re.compile(r"\x1b\[[0-9;?]*[a-zA-Z]|\x1b[]P][^\x07\x1b]*(\x07|\x1b\\)?|\x1b[()][0-9A-B]")
+
+
+def screen_rows(raw):
+ """Rebuild the visible grid from the pty stream: absolute cursor moves plus
+ printable runs. Enough for reading a frame; not a terminal emulator."""
+ grid = {}
+ row = col = 0
+ i = 0
+ while i < len(raw):
+ ch = raw[i]
+ if ch == "\x1b":
+ m = re.match(r"\x1b\[([0-9;]*)([a-zA-Z])", raw[i:])
+ if m:
+ args, verb = m.group(1), m.group(2)
+ nums = [int(n) for n in args.split(";") if n != ""]
+ if verb == "H":
+ row = (nums[0] - 1) if nums else 0
+ col = (nums[1] - 1) if len(nums) > 1 else 0
+ elif verb == "J":
+ grid.clear()
+ i += m.end()
+ continue
+ m = re.match(r"\x1b][^\x07\x1b]*(\x07|\x1b\\)", raw[i:])
+ if m:
+ i += m.end()
+ continue
+ i += 1
+ continue
+ if ch == "\r":
+ col = 0
+ elif ch == "\n":
+ row += 1
+ col = 0
+ elif ch >= " ":
+ grid[(row, col)] = ch
+ col += 1
+ i += 1
+ out = []
+ for r in range(ROWS):
+ line = "".join(grid.get((r, c), " ") for c in range(COLS))
+ out.append(line.rstrip())
+ return out
+
+
+def spawn_client(env, args, rows=ROWS, cols=COLS):
+ pid, fd = pty.fork()
+ if pid == 0:
+ os.execvpe(BIN, [BIN] + args, env)
+ fcntl.ioctl(fd, termios.TIOCSWINSZ, struct.pack("HHHH", rows, cols, 0, 0))
+ # non-blocking: a settled client produces nothing, and a blocking read on
+ # its pty would hang this check instead of failing it
+ os.set_blocking(fd, False)
+ return pid, fd
+
+
+def read_screen(fd, buffered, seconds=2.5, until=None):
+ deadline = time.time() + seconds
+ while time.time() < deadline:
+ time.sleep(0.4)
+ try:
+ buffered += os.read(fd, 1 << 20).decode("utf8", "replace")
+ except BlockingIOError:
+ pass
+ except OSError:
+ break
+ if until and until(screen_rows(buffered)):
+ break
+ return buffered, screen_rows(buffered)
+
+
+def find_button(rows):
+ """(row, 0-based column of the `+`) on a pane's top frame row."""
+ for y, line in enumerate(rows):
+ if ("╭" in line or "┌" in line) and " + " in line:
+ return y, line.index(" + ") + 1
+ return None
+
+
+def main():
+ tmp = tempfile.mkdtemp(prefix="sl0p-splitbutton-")
+ config_file = os.path.join(tmp, "config.kdl")
+ with open(config_file, "w") as f:
+ f.write(CONFIG)
+ env = dict(os.environ)
+ env.update(
+ {
+ "SL0PPIJ_SOCKET_DIR": os.path.join(tmp, "sock"), # isolate from live sessions
+ "SL0PPIJ_CONFIG_FILE": config_file,
+ "SL0PPIJ_CONFIG_DIR": tmp,
+ "SL0PPIJ_CACHE_DIR": os.path.join(tmp, "cache"),
+ "TERM": "xterm-256color",
+ "SHELL": "/bin/sh",
+ "PS1": "$ ",
+ }
+ )
+
+ pid, fd = spawn_client(env, ["-s", SESSION])
+ second_pid = second_fd = None
+
+ raw = ""
+ failures = []
+ try:
+ raw, rows = read_screen(fd, raw, seconds=20, until=find_button)
+
+ # 1. one client: is it drawn at all, on the pane's top frame row?
+ hit = find_button(rows)
+ if hit is None:
+ failures.append("one client: the split button was not drawn on any frame row")
+ print("\n".join(f"{i:02d}|{r}" for i, r in enumerate(rows)))
+ return failures
+ y, x = hit
+ print(f"one client: drawn at row {y}, column {x}")
+ print(f" {rows[y]}")
+
+ # 2. two clients: the frame grows a "MY FOCUS" middle, which is a
+ # different title builder -- and the one the live box always used,
+ # because a headless `sl0ppi up` leaves a phantom client behind.
+ second_pid, second_fd = spawn_client(env, ["attach", SESSION])
+ _, second_rows = read_screen(second_fd, "", seconds=10, until=find_button)
+ raw, rows = read_screen(fd, raw, seconds=5)
+ focus_middle = any("FOCUS" in r or "| ME |" in r for r in rows + second_rows)
+ if not focus_middle:
+ failures.append("two clients: no focus indication -- the second client did not attach")
+ hit2 = find_button(rows)
+ print(f"two clients: drawn at {hit2}")
+ if hit2 is None:
+ failures.append("two clients: the split button was dropped from the title line")
+ print("\n".join(f"{i:02d}|{r}" for i, r in enumerate(rows)))
+ elif hit2 != hit:
+ failures.append(f"two clients: the button moved, {hit} -> {hit2}")
+ if hit2:
+ print(f" {rows[hit2[0]]}")
+
+ def pane_count():
+ proc = subprocess.run(
+ [BIN, "-s", SESSION, "action", "list-panes", "--json"],
+ env=env,
+ capture_output=True,
+ text=True,
+ )
+ if os.environ.get("DEBUG"):
+ print(f"list-panes: {proc.stdout[:2000]!r} {proc.stderr[:500]!r}")
+ return proc.stdout.count('"pane_x"')
+
+ before = pane_count()
+
+ # 2. clicking two columns right of the glyph -- where the OLD hit-test
+ # looked, out in the pane gap -- must do nothing
+ os.write(fd, f"\x1b[<0;{x + 3};{y + 1}M".encode())
+ os.write(fd, f"\x1b[<0;{x + 3};{y + 1}m".encode())
+ time.sleep(1.5)
+ if pane_count() != before:
+ failures.append("a click in the gap beside the button split the pane")
+
+ # 3. clicking the drawn glyph splits
+ os.write(fd, f"\x1b[<0;{x + 1};{y + 1}M".encode())
+ os.write(fd, f"\x1b[<0;{x + 1};{y + 1}m".encode())
+ time.sleep(2)
+ after = pane_count()
+ print(f"panes: {before} -> {after}")
+ if os.environ.get("DEBUG"):
+ raw, rows = read_screen(fd, raw, seconds=1)
+ print("\n".join(f"{i:02d}|{r}" for i, r in enumerate(rows)))
+ if after != before + 1:
+ failures.append(f"clicking the drawn button did not split ({before} -> {after})")
+ return failures
+ finally:
+ subprocess.run(
+ [BIN, "-s", SESSION, "kill-session"], env=env, capture_output=True, text=True
+ )
+ for p, f in ((pid, fd), (second_pid, second_fd)):
+ if f is None:
+ continue
+ try:
+ os.close(f)
+ except OSError:
+ pass
+ os.waitpid(p, os.WNOHANG)
+
+
+if __name__ == "__main__":
+ problems = main()
+ for p in problems:
+ print(f"FAIL: {p}")
+ print("ok: the split button is drawn where it is clicked" if not problems else "")
+ sys.exit(1 if problems else 0)
diff --git a/sl0ppij/zellij-server/src/panes/plugin_pane.rs b/sl0ppij/zellij-server/src/panes/plugin_pane.rs
index 3c1816d..9e96323 100644
--- a/sl0ppij/zellij-server/src/panes/plugin_pane.rs
+++ b/sl0ppij/zellij-server/src/panes/plugin_pane.rs
@@ -366,6 +366,14 @@ impl Pane for PluginPane {
fn current_geom(&self) -> PaneGeom {
self.geom_override.unwrap_or(self.geom)
}
+ fn frame_rect(&self) -> Viewport {
+ // must match render_frame(), which draws at exactly this rect
+ Viewport::from(self.current_geom()).inset_by(if self.draws_frame {
+ self.gap_offset
+ } else {
+ Offset::default()
+ })
+ }
fn geom_override(&self) -> Option<PaneGeom> {
self.geom_override
}
diff --git a/sl0ppij/zellij-server/src/panes/terminal_pane.rs b/sl0ppij/zellij-server/src/panes/terminal_pane.rs
index 473b11a..5b2e68a 100644
--- a/sl0ppij/zellij-server/src/panes/terminal_pane.rs
+++ b/sl0ppij/zellij-server/src/panes/terminal_pane.rs
@@ -473,6 +473,14 @@ impl Pane for TerminalPane {
fn current_geom(&self) -> PaneGeom {
self.geom_override.unwrap_or(self.geom)
}
+ fn frame_rect(&self) -> Viewport {
+ // must match render_frame(), which draws at exactly this rect
+ Viewport::from(self.current_geom()).inset_by(if self.draws_frame {
+ self.gap_offset
+ } else {
+ Offset::default()
+ })
+ }
fn geom_override(&self) -> Option<PaneGeom> {
self.geom_override
}
diff --git a/sl0ppij/zellij-server/src/tab/mod.rs b/sl0ppij/zellij-server/src/tab/mod.rs
index 9ed4ce2..8ce4af6 100644
--- a/sl0ppij/zellij-server/src/tab/mod.rs
+++ b/sl0ppij/zellij-server/src/tab/mod.rs
@@ -450,6 +450,17 @@ pub trait Pane {
}
fn position_and_size(&self) -> PaneGeom;
fn current_geom(&self) -> PaneGeom;
+ /// The rect this pane's FRAME is drawn in -- where its borders, title line
+ /// and the controls on that line actually land on screen.
+ ///
+ /// NOT the pane's allocation: this fork insets the drawn frame by the
+ /// configured pane gap, so with `gap 1` the top-left corner is a row and
+ /// two columns in from `position_and_size()`. Anything hit-testing a drawn
+ /// affordance has to use this, or it claims columns the user can see are
+ /// empty (and misses the ones they can see are not).
+ fn frame_rect(&self) -> Viewport {
+ Viewport::from(self.current_geom())
+ }
fn geom_override(&self) -> Option<PaneGeom>;
fn should_render(&self) -> bool;
fn set_should_render(&mut self, should_render: bool);
diff --git a/sl0ppij/zellij-server/src/tab/mouse_handler.rs b/sl0ppij/zellij-server/src/tab/mouse_handler.rs
index 6961fc7..6bb2a05 100644
--- a/sl0ppij/zellij-server/src/tab/mouse_handler.rs
+++ b/sl0ppij/zellij-server/src/tab/mouse_handler.rs
@@ -476,10 +476,17 @@ impl MouseHandler {
/// sl0p fork: clicking the `+` on a pane's title line splits that pane.
///
/// Spawning a pane was the one common operation with no mouse path at all --
- /// switching, renaming, reordering and resizing all have one. The button is
- /// only drawn while the pointer is over the pane (see `render_split_button`),
- /// and both sides derive its columns from `split_button_columns`, so what is
- /// clickable is exactly what is visible.
+ /// switching, renaming, reordering and resizing all have one. Both sides
+ /// derive its columns from `split_button_columns` over the pane's DRAWN
+ /// frame rect, so what is clickable is exactly what is visible.
+ ///
+ /// `frame_rect()`, not `position_and_size()`: with a pane gap configured the
+ /// frame is drawn inset from the allocation, and hit-testing the allocation
+ /// put the clickable columns out in the blank gap -- two columns right of
+ /// the drawn `+`, and a row above it for panes at the top of the screen.
+ /// The click "worked" there only because this test never consults the
+ /// renderer, which is precisely why it could not be trusted as evidence the
+ /// button was drawn.
///
/// Intercepted before the OSC-5577 status bar buttons: those are drawn by
/// the program inside the pane on the row BELOW the content, so the two
@@ -508,13 +515,13 @@ impl MouseHandler {
if !pane.selectable() {
return Ok(None);
}
- let geom = pane.position_and_size();
+ let frame = pane.frame_rect();
let pane_id = pane.pid();
- // the title line is the pane's top row
- if event.position.line() != geom.y as isize {
+ // the title line is the frame's top row
+ if event.position.line() != frame.y as isize {
return Ok(None);
}
- let Some(columns) = split_button_columns(geom.x, geom.cols.as_usize()) else {
+ let Some(columns) = split_button_columns(frame.x, frame.cols) else {
return Ok(None);
};
if !columns.contains(&event.position.column()) {
diff --git a/sl0ppij/zellij-server/src/tab/unit/snapshots/zellij_server__tab__tab_integration_tests__ctrl_scroll_up_merging_stacks_preserves_all_panes.snap b/sl0ppij/zellij-server/src/tab/unit/snapshots/zellij_server__tab__tab_integration_tests__ctrl_scroll_up_merging_stacks_preserves_all_panes.snap
index 173a0a8..4878cb1 100644
--- a/sl0ppij/zellij-server/src/tab/unit/snapshots/zellij_server__tab__tab_integration_tests__ctrl_scroll_up_merging_stacks_preserves_all_panes.snap
+++ b/sl0ppij/zellij-server/src/tab/unit/snapshots/zellij_server__tab__tab_integration_tests__ctrl_scroll_up_merging_stacks_preserves_all_panes.snap
@@ -1,13 +1,12 @@
---
source: zellij-server/src/tab/./unit/tab_integration_tests.rs
-assertion_line: 13079
expression: snapshot
---
00 (C): > │ 1: Pane #1 │
01 (C): │ 2: Pane #3 │
02 (C): │ 3: Pane #4 │
03 (C):
-04 (C): ┌────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┐
+04 (C): ┌───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── + ┐
05 (C): │ │
06 (C): │ │
07 (C): │ │
@@ -29,7 +28,7 @@ expression: snapshot
23 (C): │ 2: Pane #5 │
24 (C): > │ 3: Pane #6 │
25 (C):
-26 (C): ┌────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────┐
+26 (C): ┌───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── + ┐
27 (C): │ │
28 (C): │ │
29 (C): │ │
diff --git a/sl0ppij/zellij-server/src/ui/pane_boundaries_frame.rs b/sl0ppij/zellij-server/src/ui/pane_boundaries_frame.rs
index adfaf25..7fffceb 100644
--- a/sl0ppij/zellij-server/src/ui/pane_boundaries_frame.rs
+++ b/sl0ppij/zellij-server/src/ui/pane_boundaries_frame.rs
@@ -12,12 +12,16 @@ use crate::ClientId;
use zellij_utils::data::{client_id_to_colors, PaletteColor, PaneTitleAlignment, Style};
use zellij_utils::input::layout::{layout_status_bar, StatusBarButton};
-/// sl0p fork: the mouse "split this pane" button drawn at the right end of a
-/// hovered pane's title line, and the geometry both the renderer and the mouse
-/// handler derive it from.
+/// sl0p fork: the mouse "split this pane" button drawn at the right end of
+/// every tiled pane's title line, and the geometry both the renderer and the
+/// mouse handler derive it from.
///
/// Kept here, next to the code that draws it, so the two cannot drift: a button
/// you can see but not click (or click but not see) is worse than no button.
+/// Both halves of that drifted anyway, for two whole releases -- see
+/// `render_split_button` and `intercept_split_button_click`. The check that
+/// would have caught it is `packaging/checks/split-button-live.py`, which reads
+/// the glyph off a real session and clicks the columns it was found in.
pub const SPLIT_BUTTON_TEXT: &str = " + ";
pub const SPLIT_BUTTON_WIDTH: usize = 3;
/// Narrower than this and the title line has nothing to spare; the button is
@@ -365,16 +369,24 @@ impl PaneFrame {
///
/// Drawn on every frame, not only while hovered. It WAS hover-gated, which
/// looked tidier and did not work: `mouse_is_hovering_over_pane` never
- /// became true in a live session, so the button stayed invisible while its
- /// hit-test -- geometric, and never consulting hover -- kept working.
- /// Clickable but invisible is the worst of both, so visibility no longer
- /// depends on state that is not reaching this renderer.
+ /// became true in a live session. Removing the gate did not make the button
+ /// appear either -- the actual reason it was invisible for two releases was
+ /// upstairs, in the title line builders:
///
- /// Why that flag is false live is still open: the plumbing from
- /// `mouse_hover_pane_id` through `FrameParams` reads correctly, and both
- /// `advanced_mouse_actions` and `mouse_hover_effects` are set in the config
- /// that reproduces it. Re-gate on hover only once that is understood AND
- /// demonstrated on a real session, not just in a snapshot test.
+ /// - the right side was only computed at all if the pane had a TITLE, and
+ /// `hide_default_pane_names` (live) leaves unnamed panes with none;
+ /// - the builder that runs when a pane has a "MY FOCUS" middle -- i.e.
+ /// any session with a second client, including the phantom one a
+ /// headless `sl0ppi up` leaks -- dropped the right side entirely;
+ /// - a title longer than the frame ate the button's columns, and shell
+ /// titles are long paths.
+ ///
+ /// None of it reproduced in the 265 snapshots, which all had a titled pane,
+ /// one client and a left-aligned title. `split_button_is_drawn_where_it_is_hit_tested`
+ /// covers the combinations the live config actually produces.
+ ///
+ /// Re-gate on hover only once that flag is understood AND demonstrated on a
+ /// real session, not just in a snapshot test.
fn render_split_button(&self, max_length: usize) -> Option<(Vec<TerminalCharacter>, usize)> {
if !self.is_selectable {
return None;
@@ -478,23 +490,28 @@ impl PaneFrame {
}
}
fn render_my_focus(&self, max_length: usize) -> Option<(Vec<TerminalCharacter>, usize)> {
- let mut left_separator = foreground_color(boundary_type::VERTICAL_LEFT, self.color);
- let mut right_separator = foreground_color(boundary_type::VERTICAL_RIGHT, self.color);
+ // cloned per use: `Vec::append` DRAINS its argument, so building the
+ // full indication first left the short one with no separators at all --
+ // 4 characters wide while reporting 6, which is a title line two cells
+ // short of the pane and a corrupted grid row. Only bit on very narrow
+ // panes, which is where the short form is the one that gets used.
+ let left_separator = || foreground_color(boundary_type::VERTICAL_LEFT, self.color);
+ let right_separator = || foreground_color(boundary_type::VERTICAL_RIGHT, self.color);
let full_indication_text = "MY FOCUS";
let mut full_indication = vec![];
- full_indication.append(&mut left_separator);
+ full_indication.append(&mut left_separator());
full_indication.push(EMPTY_TERMINAL_CHARACTER);
full_indication.append(&mut foreground_color(full_indication_text, self.color));
full_indication.push(EMPTY_TERMINAL_CHARACTER);
- full_indication.append(&mut right_separator);
+ full_indication.append(&mut right_separator());
let full_indication_len = full_indication_text.width() + 4; // 2 for separators 2 for padding
let short_indication_text = "ME";
let mut short_indication = vec![];
- short_indication.append(&mut left_separator);
+ short_indication.append(&mut left_separator());
short_indication.push(EMPTY_TERMINAL_CHARACTER);
short_indication.append(&mut foreground_color(short_indication_text, self.color));
short_indication.push(EMPTY_TERMINAL_CHARACTER);
- short_indication.append(&mut right_separator);
+ short_indication.append(&mut right_separator());
let short_indication_len = short_indication_text.width() + 4; // 2 for separators 2 for padding
if full_indication_len <= max_length {
Some((full_indication, full_indication_len))
@@ -718,15 +735,18 @@ impl PaneFrame {
self.get_corner(boundary_type::TOP_RIGHT),
self.color,
));
- } else if col == left_side_start_position {
+ // the `> 0` guards let a caller pass an EMPTY segment (an unnamed
+ // pane has no left side) without the loop stalling on a segment
+ // that advances `col` by nothing.
+ } else if col == left_side_start_position && *left_side_len > 0 {
title_line.append(&mut left_side);
col += left_side_len;
continue;
- } else if col == middle_start_position {
+ } else if col == middle_start_position && *middle_len > 0 {
title_line.append(&mut middle);
col += middle_len;
continue;
- } else if col == right_side_start_position {
+ } else if col == right_side_start_position && *right_side_len > 0 {
title_line.append(&mut right_side);
col += right_side_len;
continue;
@@ -886,7 +906,12 @@ impl PaneFrame {
let total_title_length = self.geom.cols.saturating_sub(2); // 2 for the left and right corners
let length_of_each_side = total_title_length.saturating_sub(*middle_len + 2) / 2;
let left_side = self.render_title_left_side(length_of_each_side);
- let right_side = self.render_title_right_side(length_of_each_side);
+ // sl0p fork: at least the anchored button, even when half the leftover
+ // line is narrower than it. `render_title` has already kept the middle
+ // short enough that this cannot reach the middle: the middle is centred,
+ // so its budget was reduced by the button's width at BOTH ends.
+ let right_side =
+ self.render_title_right_side(length_of_each_side.max(self.anchored_right_side_len()));
match (left_side, right_side) {
(Some((left_side, left_side_len)), Some((right_side, right_side_len))) => self
@@ -901,7 +926,20 @@ impl PaneFrame {
(Some((left_side, left_side_len)), None) => {
self.left_and_middle_title_line(left_side, &left_side_len, middle, middle_len)
},
- _ => self.middle_only_title_line(middle, middle_len),
+ // sl0p fork: an untitled pane still has a right side, and it used
+ // to be dropped here along with the (absent) title -- which is how
+ // the split button could render in 265 snapshots and never once on
+ // a live session, where `hide_default_pane_names` leaves unnamed
+ // panes with no title at all.
+ (None, Some((right_side, right_side_len))) => self.three_part_title_line(
+ vec![],
+ &0,
+ middle,
+ middle_len,
+ right_side,
+ &right_side_len,
+ ),
+ (None, None) => self.middle_only_title_line(middle, middle_len),
}
}
/// Compose a title line with the pane title placed at `alignment` rather than
@@ -971,17 +1009,32 @@ impl PaneFrame {
title_line
}
fn title_line_without_middle(&self) -> Vec<TerminalCharacter> {
- // Left keeps the original code path untouched, so the default config
- // renders byte-for-byte as before.
+ // Left keeps the original code path, so the default config renders as
+ // before -- except that the anchored button is reserved up front.
if self.style.pane_title_alignment != PaneTitleAlignment::Left {
return self.aligned_title_line(self.style.pane_title_alignment);
}
+ // sl0p fork: the title is budgeted AFTER the anchored button, which
+ // upstream's order could not do -- and shell titles are long paths, so
+ // a title that filled the line used to leave the button nothing. Only
+ // the button is reserved: the indications to its left keep the
+ // historical "whatever the title leaves" budget.
let total_title_length = self.geom.cols.saturating_sub(2); // 2 for the left and right corners
- let left_side = self.render_title_left_side(total_title_length);
- let right_side = left_side.as_ref().and_then(|(_left_side, left_side_len)| {
- let space_left = total_title_length.saturating_sub(*left_side_len + 1); // 1 for a middle separator
- self.render_title_right_side(space_left)
- });
+ let anchored = self.anchored_right_side_len();
+ let separator = if anchored > 0 { 1 } else { 0 };
+ let left_side =
+ self.render_title_left_side(total_title_length.saturating_sub(anchored + separator));
+ let right_side = match left_side.as_ref() {
+ Some((_left_side, left_side_len)) => {
+ let space_left = total_title_length.saturating_sub(*left_side_len + 1); // 1 for a middle separator
+ self.render_title_right_side(space_left)
+ },
+ // sl0p fork: with no title the right side gets the whole line.
+ // Upstream only ever computed it INSIDE the left side's `and_then`,
+ // so an unnamed pane (`hide_default_pane_names true`, the live
+ // default) silently had no scroll indication and no split button.
+ None => self.render_title_right_side(total_title_length),
+ };
match (left_side, right_side) {
(Some((left_side, left_side_len)), Some((right_side, right_side_len))) => {
self.two_part_title_line(left_side, &left_side_len, right_side, &right_side_len)
@@ -989,16 +1042,40 @@ impl PaneFrame {
(Some((left_side, left_side_len)), None) => {
self.left_only_title_line(left_side, &left_side_len)
},
- _ => self.empty_title_line(),
+ // an empty left side lays out exactly like a two-part line whose
+ // left part is zero-width: padding, then the right side flush
+ // against the corner.
+ (None, Some((right_side, right_side_len))) => {
+ self.two_part_title_line(vec![], &0, right_side, &right_side_len)
+ },
+ (None, None) => self.empty_title_line(),
}
}
+ /// Width of the part of the title line that is ANCHORED to the top-right
+ /// corner and may therefore never be squeezed out by layout: the split
+ /// button, whose hit-test is a pure function of the pane's geometry
+ /// (`split_button_columns`) and cannot ask the renderer what it drew.
+ ///
+ /// Everything else on the right side (scroll, pin) is positional only in
+ /// relation to it, so it keeps its historical leftover budget.
+ fn anchored_right_side_len(&self) -> usize {
+ self.render_split_button(self.geom.cols.saturating_sub(2))
+ .map(|(_, len)| len)
+ .unwrap_or(0)
+ }
fn render_title(&self) -> Result<Vec<TerminalCharacter>> {
+ // The middle is CENTRED, so room for the anchored button has to come
+ // off BOTH ends -- otherwise a middle that is merely centred-and-wide
+ // reaches into the button's columns, and the layout loop, which only
+ // recognises a segment's exact start column, walks straight past the
+ // button and emits a title line narrower than the pane.
let total_title_length = self.geom.cols.saturating_sub(2); // 2 for the left and right corners
+ let middle_budget = total_title_length.saturating_sub(2 * self.anchored_right_side_len());
- self.render_title_middle(total_title_length)
- .map(|(middle, middle_length)| self.title_line_with_middle(middle, &middle_length))
- .or_else(|| Some(self.title_line_without_middle()))
- .with_context(|| format!("failed to render title '{}'", self.title))
+ Ok(match self.render_title_middle(middle_budget) {
+ Some((middle, middle_length)) => self.title_line_with_middle(middle, &middle_length),
+ None => self.title_line_without_middle(),
+ })
}
fn render_stack_list_entry(&self, entry: &StackListEntry) -> Vec<TerminalCharacter> {
let usable_cols = self.geom.cols;
@@ -1119,12 +1196,9 @@ impl PaneFrame {
}
fn render_one_line_title(&self) -> Result<Vec<TerminalCharacter>> {
if self.should_draw_pane_frames {
- let total_title_length = self.geom.cols.saturating_sub(2);
- return self
- .render_title_middle(total_title_length)
- .map(|(middle, middle_length)| self.title_line_with_middle(middle, &middle_length))
- .or_else(|| Some(self.title_line_without_middle()))
- .with_context(|| format!("failed to render title '{}'", self.title));
+ // a framed pane one row tall is all title line, laid out exactly as
+ // the top row of a taller one
+ return self.render_title();
}
let width = self.geom.cols;
@@ -2097,4 +2171,123 @@ mod tests {
assert!(!text.contains("MouseScroll"));
assert!(text.contains("<drag borders>"));
}
+
+ /// Every combination the LIVE config puts a frame in, which the earlier
+ /// snapshots never did: they all used zellij's defaults (left-aligned,
+ /// square corners, a pane title, one client).
+ ///
+ /// `title`: `hide_default_pane_names true` renders unnamed panes with an
+ /// EMPTY title. `other_focused_clients` / `other_cursors_exist_in_session`:
+ /// any second client in the session -- including the phantom one a headless
+ /// `sl0ppi up` leaks -- puts a "MY FOCUS" middle in the title line.
+ fn live_frames(cols: usize) -> Vec<(String, PaneFrame)> {
+ let mut frames = vec![];
+ for alignment in [
+ PaneTitleAlignment::Left,
+ PaneTitleAlignment::Center,
+ PaneTitleAlignment::Right,
+ ] {
+ for rounded_corners in [false, true] {
+ for title in ["", "mytitle"] {
+ for (other_clients, other_cursors) in
+ [(vec![], false), (vec![], true), (vec![2], true)]
+ {
+ let mut frame = pane_frame_with(false, false, cols);
+ frame.title = title.to_string();
+ frame.style.pane_title_alignment = alignment;
+ frame.style.rounded_corners = rounded_corners;
+ frame.other_focused_clients = other_clients.clone();
+ frame.other_cursors_exist_in_session = other_cursors;
+ frames.push((
+ format!(
+ "cols {} {:?} rounded {} title {:?} other_clients {:?} other_cursors {}",
+ cols, alignment, rounded_corners, title, other_clients, other_cursors
+ ),
+ frame,
+ ));
+ }
+ }
+ }
+ }
+ frames
+ }
+
+ /// The columns of a rendered title line that the mouse handler will treat
+ /// as the split button.
+ fn drawn_split_button(frame: &PaneFrame) -> String {
+ let rendered: Vec<char> = characters_to_string(&frame.render_title().unwrap())
+ .chars()
+ .collect();
+ assert_eq!(
+ rendered.len(),
+ frame.geom.cols,
+ "title line is not the width of the pane"
+ );
+ let columns = split_button_columns(frame.geom.x, frame.geom.cols)
+ .expect("no split button geometry for this pane");
+ rendered[columns.start - frame.geom.x..columns.end - frame.geom.x]
+ .iter()
+ .collect()
+ }
+
+ #[test]
+ fn split_button_is_drawn_where_it_is_hit_tested() {
+ // REGRESSION: the button rendered in every snapshot and was never once
+ // visible on a live session. The hit-test is geometric and never
+ // consults the renderer, so "clicking it works" proved nothing: the
+ // title builder that runs when a pane has no title AND a "MY FOCUS"
+ // middle dropped the whole right side of the title line on the floor.
+ //
+ // Asserting on the exact COLUMNS rather than "contains a +" is the
+ // point: the short "MY FOCUS AND: +" indication contains one too.
+ for cols in [20, 41, 80, 120] {
+ for (what, frame) in live_frames(cols) {
+ assert_eq!(
+ drawn_split_button(&frame),
+ SPLIT_BUTTON_TEXT,
+ "split button not drawn in its clickable columns: {}",
+ what
+ );
+ }
+ }
+ }
+
+ #[test]
+ fn split_button_is_not_drawn_when_the_pane_is_too_narrow() {
+ // the other half of the same invariant: where the hit-test declines to
+ // find a button, nothing may be drawn that looks like one.
+ //
+ // From 2: a 1-column pane renders 2 characters (both corners) on the
+ // left-aligned path, which is upstream behaviour and not this feature's
+ // to fix.
+ for cols in 2..SPLIT_BUTTON_MIN_PANE_COLS {
+ for (what, frame) in live_frames(cols) {
+ assert!(
+ split_button_columns(frame.geom.x, frame.geom.cols).is_none(),
+ "{}: expected no button geometry below {} cols",
+ what,
+ SPLIT_BUTTON_MIN_PANE_COLS
+ );
+ let rendered: Vec<char> = characters_to_string(&frame.render_title().unwrap())
+ .chars()
+ .collect();
+ assert_eq!(rendered.len(), cols, "{}", what);
+ // positional, not `contains`: the short multi-client
+ // indication is itself a "+" between separators, so a
+ // substring search here reports a button that is not one.
+ if cols > SPLIT_BUTTON_WIDTH + 1 {
+ let would_be: String = rendered[cols - 1 - SPLIT_BUTTON_WIDTH..cols - 1]
+ .iter()
+ .collect();
+ assert_ne!(
+ would_be,
+ SPLIT_BUTTON_TEXT,
+ "{}: drew a split button that cannot be clicked: {:?}",
+ what,
+ rendered.iter().collect::<String>()
+ );
+ }
+ }
+ }
+ }
}