Skip to content

Commit 20cce42

Browse files
committed
Fix OSC 51;E eval crashing process filter
`ghostel--osc51-eval` is dispatched synchronously from the native VT parser inside `fnWriteInput`, so anything the callback does — or any error it signals — unwinds back through `ghostel--write-input` into whatever caller drove the flush: the process filter on the immediate-redraw path, or `ghostel--delayed-redraw` on the bulk-timer path. Three distinct regressions from #82 all stem from that: 1. `find-file-other-window` / `dired-other-window` call `select-window` on the newly-popped window, which flips `current-buffer` as a side effect. When control returns to `ghostel--delayed-redraw`, its `with-current-buffer` has already entered its body, and the remaining statements read `ghostel--term` from the *new* file buffer, where the buffer-local is nil. The native module then receives nil and Emacs signals `Wrong type argument: user-ptrp, nil` — attributed to `ghostel--delayed-redraw` on the timer path and to the process filter on the immediate path. Fix: wrap the `ghostel--write-input` call in `ghostel--flush-pending-output` with `save-current-buffer`. Done at the flush site so every caller — delayed-redraw, the filter's color-query fast path, `ghostel-clear`, `ghostel-clear-scrollback`, and the sentinel — is protected in one place. The user-visible window selection still sticks because `save-current-buffer` restores only the elisp current buffer, not `selected-window`. 2. `dow` with no arguments called `dired-other-window` with zero arguments, which signals `Wrong number of arguments`. That error propagates up the same path and again trips the filter. Fix: wrap the `apply` in `ghostel--osc51-eval` with `condition-case`, so any error a whitelisted function signals is reported via `message` instead of crashing the pipeline. Also update the README's `dow()` example to default to `$PWD` so the bare invocation does the sensible thing. 3. After `find-file-other-window` deselected the ghostel window, the subsequent native redraw updated `buffer-point` but not `window-point` for any window showing the ghostel buffer (Emacs only keeps them in sync for the *selected* window). When the user reselected the ghostel window the terminal cursor appeared at a stale position. Fix: after the native redraw in `ghostel--delayed-redraw`, push the new `(point)` into every window displaying the buffer via `set-window-point`. Tests: `ghostel-test-flush-pending-output-preserves-buffer` stubs `ghostel--write-input` to simulate the `select-window` side effect and asserts the flush restores the ghostel buffer; `ghostel-test-osc51-eval-catches-errors` dispatches a whitelisted lambda that raises and asserts the error is messaged rather than propagated. Both run under `ghostel-test-run-elisp` — no native module required. Fixes #82
1 parent c851d48 commit 20cce42

3 files changed

Lines changed: 72 additions & 5 deletions

File tree

README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -358,8 +358,8 @@ if [[ "$INSIDE_EMACS" = 'ghostel' ]]; then
358358
# Open a file in Emacs from the terminal
359359
e() { ghostel_cmd find-file-other-window "$@"; }
360360

361-
# Open dired in another window
362-
dow() { ghostel_cmd dired-other-window "$@"; }
361+
# Open dired in another window, defaulting to the current directory
362+
dow() { ghostel_cmd dired-other-window "${1:-$PWD}"; }
363363

364364
# Open magit for the current directory
365365
gst() { ghostel_cmd magit-status-setup-buffer "$(pwd)"; }

ghostel.el

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1705,7 +1705,15 @@ Parses the command and arguments, looks up the command in
17051705
(args (cdr parts))
17061706
(entry (assoc command ghostel-eval-cmds)))
17071707
(if entry
1708-
(apply (cadr entry) args)
1708+
;; Catch errors from the dispatched function: this callback runs
1709+
;; synchronously inside the native VT parser, so any unhandled
1710+
;; error propagates back up through `ghostel--write-input' and
1711+
;; crashes the process filter / redraw timer.
1712+
(condition-case err
1713+
(apply (cadr entry) args)
1714+
(error
1715+
(message "ghostel: error calling %s: %s"
1716+
command (error-message-string err))))
17091717
(message "ghostel: unknown eval command %S (add to `ghostel-eval-cmds' to allow)"
17101718
command))))
17111719

@@ -2269,7 +2277,13 @@ frame after idle to improve interactive responsiveness."
22692277
(when ghostel--pending-output
22702278
(let ((combined (apply #'concat (nreverse ghostel--pending-output))))
22712279
(setq ghostel--pending-output nil)
2272-
(ghostel--write-input ghostel--term combined))))
2280+
;; An OSC 51;E callback dispatched synchronously from the native
2281+
;; parser (e.g. `find-file-other-window') can change the current
2282+
;; buffer via `select-window'. Isolate that so callers keep
2283+
;; reading buffer-locals — notably `ghostel--term' — from the
2284+
;; ghostel buffer after this returns.
2285+
(save-current-buffer
2286+
(ghostel--write-input ghostel--term combined)))))
22732287

22742288
(defun ghostel--delayed-redraw (buffer)
22752289
"Perform the actual redraw in BUFFER."
@@ -2289,7 +2303,16 @@ frame after idle to improve interactive responsiveness."
22892303
(inhibit-modification-hooks t))
22902304
(ghostel--redraw ghostel--term ghostel-full-redraw))
22912305
(when ghostel--has-wide-chars
2292-
(ghostel--compensate-wide-chars)))))))
2306+
(ghostel--compensate-wide-chars))
2307+
;; Native redraw updates buffer-point via `goto-char', which
2308+
;; only propagates to `window-point' for the selected window.
2309+
;; If an OSC 51;E callback moved selection elsewhere (e.g.
2310+
;; `find-file-other-window'), the ghostel window's
2311+
;; window-point is stale and the terminal cursor will display
2312+
;; at the wrong place when the user reselects it. Sync it.
2313+
(let ((pt (point)))
2314+
(dolist (win (get-buffer-window-list buffer nil t))
2315+
(set-window-point win pt))))))))
22932316

22942317
(defun ghostel-force-redraw ()
22952318
"Force a full terminal redraw (for debugging)."

test/ghostel-test.el

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1454,6 +1454,48 @@ rendered by `ghostel--delayed-redraw'. This is the exact real-world path."
14541454
(should (car messages))
14551455
(should (string-match-p "unknown eval command" (car messages))))))
14561456

1457+
(ert-deftest ghostel-test-osc51-eval-catches-errors ()
1458+
"Errors signaled by a dispatched OSC 51;E function must not
1459+
propagate out of `ghostel--osc51-eval' — otherwise they crash the
1460+
process filter / redraw timer that invoked the native parser.
1461+
Regression for a follow-up to #82 where `dow' with no args called
1462+
`dired-other-window' with 0 arguments and signaled up through the
1463+
filter."
1464+
(let* ((ghostel-eval-cmds
1465+
`(("boom" ,(lambda (&rest _) (error "kaboom")))))
1466+
(messages nil))
1467+
(cl-letf (((symbol-function 'message)
1468+
(lambda (fmt &rest args) (push (apply #'format fmt args) messages))))
1469+
;; Must not raise.
1470+
(ghostel--osc51-eval "\"boom\"")
1471+
(should (car messages))
1472+
(should (string-match-p "error calling boom" (car messages)))
1473+
(should (string-match-p "kaboom" (car messages))))))
1474+
1475+
(ert-deftest ghostel-test-flush-pending-output-preserves-buffer ()
1476+
"Regression for #82: a buffer switch performed by a synchronous
1477+
native callback (as OSC 51;E dispatch does when it calls
1478+
`find-file-other-window') must not leak out of
1479+
`ghostel--flush-pending-output'. Otherwise callers such as
1480+
`ghostel--delayed-redraw' read `ghostel--term' from the wrong
1481+
buffer and hand nil to the native module."
1482+
(let ((ghostel-buf (generate-new-buffer " *ghostel-test-flush-buf*"))
1483+
(other-buf (generate-new-buffer " *ghostel-test-flush-other*")))
1484+
(unwind-protect
1485+
(with-current-buffer ghostel-buf
1486+
(setq-local ghostel--term 'fake-handle)
1487+
(setq-local ghostel--pending-output (list "payload"))
1488+
(cl-letf (((symbol-function 'ghostel--write-input)
1489+
(lambda (_term _data)
1490+
;; Simulate `find-file-other-window' flipping
1491+
;; the current buffer via `select-window'.
1492+
(set-buffer other-buf))))
1493+
(ghostel--flush-pending-output))
1494+
(should (eq (current-buffer) ghostel-buf))
1495+
(should (null ghostel--pending-output)))
1496+
(kill-buffer ghostel-buf)
1497+
(kill-buffer other-buf))))
1498+
14571499
;; -----------------------------------------------------------------------
14581500
;; Test: copy-mode cursor visibility
14591501
;; -----------------------------------------------------------------------
@@ -2496,6 +2538,8 @@ while :; do sleep 0.1; done'\n")
24962538
ghostel-test-apply-palette-default-colors
24972539
ghostel-test-osc51-eval
24982540
ghostel-test-osc51-eval-unknown
2541+
ghostel-test-osc51-eval-catches-errors
2542+
ghostel-test-flush-pending-output-preserves-buffer
24992543
ghostel-test-copy-mode-cursor
25002544
ghostel-test-copy-mode-hl-line
25012545
ghostel-test-project-buffer-name

0 commit comments

Comments
 (0)