305 lines
10 KiB
Markdown
305 lines
10 KiB
Markdown
# cl-tty v1.0.0 Bug Fix Iteration
|
|
|
|
> **For Hermes:** Use subagent-driven-development + bug-fix-iteration pattern.
|
|
> Each task: inspect → write regression test → fix → verify → commit.
|
|
> Do NOT skip tests. Do NOT combine tasks.
|
|
|
|
**Goal:** Fix all known bugs and blindspots before v1.0.0 release.
|
|
|
|
**Architecture:** cl-tty is a pure CL terminal UI library. No FFI, no ncurses.
|
|
Components: backend (modern/simple escape seq), input (byte reader + event parser),
|
|
rendering (framebuffer diff pipeline), layout (flexbox), widgets.
|
|
|
|
**Verification command after each fix:**
|
|
```bash
|
|
cd /mnt/hermes/projects/cl-tty && sbcl --script run-all-tests.lisp && python3 scripts/verify-api.py && python3 scripts/verify-demo-pty.py
|
|
```
|
|
|
|
---
|
|
|
|
### Task 1: Fix `read-raw-byte` timeout (CRITICAL BUG)
|
|
|
|
**Objective:** The timeout mechanism uses `get-universal-time` which returns
|
|
integer seconds. Adding a float timeout like 0.05 produces a deadline that
|
|
equals the current second — the loop terminates immediately. The 50ms escape
|
|
ambiguity timeout never actually works.
|
|
|
|
**Files:**
|
|
- Modify: `src/components/input.lisp:84-111`
|
|
- Test: `tests/input-tests.lisp` (add regression test)
|
|
|
|
**Root cause:** Line 99: `(let ((deadline (+ (get-universal-time) timeout)))` —
|
|
`get-universal-time` returns integer seconds, so `(+ (integer) 0.05)` = `(+ integer 0)` = integer.
|
|
The loop `(while (< (get-universal-time) deadline))` runs zero iterations for any
|
|
sub-second timeout.
|
|
|
|
**Fix:** Use `sb-ext:get-time-of-day` (microsecond precision) or `(/ (get-internal-real-time)
|
|
internal-time-units-per-second)` to get fractional seconds. Replace:
|
|
|
|
```lisp
|
|
(let ((deadline (+ (get-universal-time) timeout)))
|
|
(loop while (< (get-universal-time) deadline) ...))
|
|
```
|
|
|
|
with:
|
|
|
|
```lisp
|
|
(let* ((start (get-internal-real-time))
|
|
(ticks (round (* timeout internal-time-units-per-second)))
|
|
(deadline (+ start ticks)))
|
|
(loop while (< (get-internal-real-time) deadline) ...))
|
|
```
|
|
|
|
Or simpler: use `(/ (- (get-internal-real-time) start) internal-time-units-per-second)`
|
|
to check elapsed time in a loop.
|
|
|
|
**Verification:**
|
|
1. Write a test that calls `read-raw-byte` with :timeout 0.05 and verifies it
|
|
returns `(values nil :timeout)` within ~100ms (not instantly).
|
|
2. All existing tests still pass.
|
|
3. The demo's Escape key works (tested by verify-demo-pty.py).
|
|
|
|
---
|
|
|
|
### Task 2: Fix `draw-border` ignoring title in modern backend (BUG)
|
|
|
|
**Objective:** The `modern-backend`'s `draw-border` method has
|
|
`(declare (ignore title title-align))` on line 194. The framebuffer backend
|
|
renders titles correctly. The simple backend also ignores titles.
|
|
This means titled borders don't show titles in the modern backend.
|
|
|
|
**Files:**
|
|
- Modify: `backend/modern.lisp:192-219`
|
|
- Add test: `backend/modern-tests.lisp`
|
|
|
|
**Fix:** In `draw-border` for `modern-backend`, insert the title text into the
|
|
top border line after the first character. The title should be centered or
|
|
left-aligned based on `title-align`.
|
|
|
|
The title rendering logic should extract from the framebuffer backend's
|
|
draw-border (framebuffer.lisp lines 114-117) and adapt for escape sequences:
|
|
- The top border line is constructed as: `tl + h*N + tr`
|
|
- Before writing top: if title is non-nil, insert it: `tl + " " + title + " " + h*fill + tr`
|
|
- Truncate title if it exceeds width-4
|
|
|
|
---
|
|
|
|
### Task 3: Fix `backend-size` to query real terminal size (MISSING FEATURE)
|
|
|
|
**Objective:** `backend-size` for `modern-backend` returns hardcoded (80 24).
|
|
Should query the terminal via TIOCGWINSZ ioctl or `ESC[18t` query.
|
|
|
|
**Files:**
|
|
- Modify: `backend/modern.lisp:163-165`
|
|
- Add test: `backend/modern-tests.lisp` (test that values are positive integers)
|
|
|
|
**Fix:** Use SBCL's `sb-alien` to call `ioctl` with `TIOCGWINSZ` on the
|
|
stdout fd (or /dev/tty):
|
|
|
|
```lisp
|
|
(defmethod backend-size ((b modern-backend))
|
|
(sb-unix:unix-ioctl (sb-sys:fd-stream-fd
|
|
(or (ignore-errors
|
|
(open "/dev/tty" :direction :input
|
|
:if-does-not-exist nil))
|
|
*standard-output*))
|
|
sb-unix:TIOCGWINSZ ...)
|
|
;; Or fallback to query-terminal with ESC[18t
|
|
;; Fallback: (values 80 24))
|
|
```
|
|
|
|
Simpler approach: Use `sb-unix:unix-ioctl` with the `TIOCGWINSZ` request.
|
|
The winsize struct is: (rows columns) as two 16-bit values. In SBCL,
|
|
`sb-unix:unix-ioctl` can be used with `sb-unix:TIOCGWINSZ`.
|
|
|
|
If ioctl is complex, implement via OSC Terminal query: `query-terminal` with
|
|
`ESC[18t` returns `ESC[8;rows;colst`. Parse the response.
|
|
|
|
---
|
|
|
|
### Task 4: Enable kitty keyboard protocol in `initialize-backend` (MISSING FEATURE)
|
|
|
|
**Objective:** `modern-backend` declares `:kitty-keyboard` in `capable-p`
|
|
but never sends the escape sequence to enable it (`ESC[?u`).
|
|
|
|
**Files:**
|
|
- Modify: `backend/modern.lisp:142-151`
|
|
|
|
**Fix:** Add to `initialize-backend`:
|
|
```lisp
|
|
(backend-write b (format nil "~C[?u" #\Esc)) ; kitty keyboard
|
|
```
|
|
|
|
And add to `shutdown-backend`:
|
|
```lisp
|
|
(backend-write b (format nil "~C[?u" #\Esc)) ; restore default keyboard
|
|
```
|
|
|
|
---
|
|
|
|
### Task 5: Fix text-input cursor rendering (MISSING VISUAL FEEDBACK)
|
|
|
|
**Objective:** The `text-input.lisp` render method declares `(declare (ignore cursor))`.
|
|
The cursor position is tracked but never drawn, so users can't see where
|
|
they're typing.
|
|
|
|
**Files:**
|
|
- Modify: `src/components/text-input.lisp` (render method)
|
|
- Add test: `tests/input-tests.lisp` or existing test file
|
|
|
|
**Fix:** In the text-input render method, after drawing the value/placeholder,
|
|
draw a cursor block (█ or reversed ▓) at the cursor position. Use
|
|
`draw-rect` or `draw-text` with a visual cursor character at the cursor column.
|
|
|
|
When the cursor would be beyond the visible area (scrolled past the right edge),
|
|
show it at the rightmost position.
|
|
|
|
---
|
|
|
|
### Task 6: Fix SS3 branch reading without timeout (POTENTIAL HANG)
|
|
|
|
**Objective:** In `%read-escape-sequence`, the SS3 branch (when b=#x4f) calls
|
|
`(read-raw-byte)` without a timeout parameter. If the terminal sends a partial
|
|
ESC O with no follow-up byte, the read blocks forever.
|
|
|
|
**Files:**
|
|
- Modify: `src/components/input.lisp:210`
|
|
|
|
**Fix:** Change line 210 from:
|
|
```lisp
|
|
(let ((b2 (read-raw-byte)))
|
|
```
|
|
to:
|
|
```lisp
|
|
(let ((b2 (read-raw-byte :timeout 0.1)))
|
|
```
|
|
And handle the nil case: if b2 is nil, return a key-event for the lone Escape.
|
|
|
|
---
|
|
|
|
### Task 7: Add Wayland support to `copy-to-clipboard` (PLATFORM GAP)
|
|
|
|
**Objective:** `copy-to-clipboard` in `mouse.lisp` only supports X11 (xclip)
|
|
and macOS (pbcopy). Wayland users (wl-copy) get no clipboard.
|
|
|
|
**Files:**
|
|
- Modify: `src/components/mouse.lisp:51-54`
|
|
|
|
**Fix:** Add `#+wayland` or detect Wayland via `$WAYLAND_DISPLAY` env var:
|
|
|
|
```lisp
|
|
(defun copy-to-clipboard (text)
|
|
#+linux
|
|
(cond
|
|
((sb-ext:posix-getenv "WAYLAND_DISPLAY")
|
|
(sb-ext:run-program "wl-copy" nil :input text :wait nil))
|
|
(t
|
|
(sb-ext:run-program "xclip" (list "-selection" "clipboard")
|
|
:input text :wait nil)))
|
|
#+darwin
|
|
(sb-ext:run-program "pbcopy" nil :input text :wait nil))
|
|
```
|
|
|
|
---
|
|
|
|
### Task 8: Add SIGWINCH handler for terminal resize (MISSING FEATURE)
|
|
|
|
**Objective:** When the terminal is resized, the demo and any cl-tty app
|
|
will render with stale dimensions. The `backend-size` (Task 3) helps but
|
|
apps need to be notified of resizes.
|
|
|
|
**Files:**
|
|
- Create: `src/components/notification.lisp` OR modify existing components
|
|
|
|
**Approach:**
|
|
This is a design decision. Options:
|
|
a) Install a SIGWINCH handler that sets a flag checked each frame
|
|
b) Provide a `register-resize-callback` API
|
|
c) Only fix in the demo layer (demo.lisp)
|
|
|
|
Keep it minimal: install a simple signal handler that sets
|
|
`*terminal-resized-p*` to T. The app checks this flag each frame.
|
|
|
|
Add to `input.lisp` or a new file:
|
|
```lisp
|
|
(defvar *terminal-resized-p* nil
|
|
"Set to T by SIGWINCH handler when terminal resizes.")
|
|
|
|
(defun %handle-sigwinch (signal info context)
|
|
(declare (ignore signal info context))
|
|
(setf *terminal-resized-p* t))
|
|
|
|
;; Install handler
|
|
#+sbcl
|
|
(sb-sys:enable-interrupt sb-unix:sigwinch #'%handle-sigwinch)
|
|
```
|
|
|
|
---
|
|
|
|
### Bug Blindspots Verified as NOT Bugs (justifying "won't fix"):
|
|
|
|
These were investigated and are fine:
|
|
- **Framebuffer diff link-url**: `cells-equal-p` compares `cell-link-url` with `equal` — covered.
|
|
- **Select with empty options**: `(if (zerop count) (setf (select-selected-index sel) 0)` — handled.
|
|
- **Dialog pop from empty stack**: `(when *dialog-stack*` — guarded.
|
|
- **`parse-csi-params`**: reads raw bytes, handles EOF gracefully.
|
|
- **Thread safety of globals**: out of scope for v1.0.0 (single-threaded TUI).
|
|
- **ScrollBox horizontal scrolling**: actually implemented (uses sx in render).
|
|
- **Redundant tests removed**: cleanup already done in uncommitted diff.
|
|
|
|
---
|
|
|
|
### BLINDSPOT: The `parse-csi-params` function also uses `(read-raw-byte)` without timeout.
|
|
|
|
Line 122: `(multiple-value-bind (b reason) (read-raw-byte)` — while parsing
|
|
a CSI sequence, if the terminal sends ESC[ but never completes the sequence,
|
|
this blocks forever. This should use a timeout similar to the escape sequence
|
|
reader. Same fix pattern as Task 6.
|
|
|
|
Adding as Task 9.
|
|
|
|
---
|
|
|
|
### Task 9: Fix `parse-csi-params` to use timeout (POTENTIAL HANG)
|
|
|
|
**Objective:** `parse-csi-params` (input.lisp line 122) reads bytes without
|
|
timeout. A partial CSI sequence (ESC[ without final byte) blocks forever.
|
|
|
|
**Files:**
|
|
- Modify: `src/components/input.lisp:116-149`
|
|
|
|
**Fix:** Add a timeout to the read inside `parse-csi-params`. Use a total
|
|
timeout of ~500ms for the entire CSI sequence (generous given terminals
|
|
respond within a few ms). If the timeout fires, return nil for final-byte.
|
|
|
|
Similar to `%read-escape-sequence`, pass `:timeout` parameter to `parse-csi-params`
|
|
and have `%read-escape-sequence` pass a timeout to it.
|
|
|
|
---
|
|
|
|
### Task 10: Fix `draw-border` ignoring title in simple backend (BUG)
|
|
|
|
**Objective:** Same as Task 2 but for `simple-backend`. The
|
|
`%simple-border-char` function just got refactored (uncommitted diff), and
|
|
`draw-border` in simple.lisp also ignores title.
|
|
|
|
**Files:**
|
|
- Modify: `backend/simple.lisp` (draw-border method)
|
|
- Add test: `backend/tests.lisp`
|
|
|
|
**Fix:** In `simple-backend`'s `draw-border`, when a title is provided,
|
|
insert it into the top border line. Use ASCII chars (the simple backend
|
|
doesn't use Unicode).
|
|
|
|
---
|
|
|
|
### Task 11: Add `detect-backend` export to backend package (API GAP)
|
|
|
|
**Objective:** The README shows `(cl-tty.backend:detect-backend)` as the
|
|
entry point, but verify this is actually exported from the backend package.
|
|
|
|
**Files:**
|
|
- Check: `backend/package.lisp`
|
|
|
|
**Fix:** Ensure `#:detect-backend` is in the package's `:export` list.
|