Rewrite ghostel-compile: spawn directly, add opt-in global mode#124
Rewrite ghostel-compile: spawn directly, add opt-in global mode#124
Conversation
Review feedback on #124: - `--prepare-buffer' used to call `ghostel--init-buffer', which spawns a full shell via `ghostel--start-process' and then needed teardown. On TRAMP buffers that's a full remote-integration round-trip (temp file upload, env plumbing) for every compile invocation — and the remote temp files leak because the cleanup only runs at the end of `ghostel--start-process' itself. Replace with direct terminal creation (`ghostel--new' + `ghostel--apply-palette') — no shell spawn, local or remote. - `ghostel-compile-global-mode' advice now also falls through to the stock `compilation-start' when CONTINUE is non-nil, since each ghostel-compile run replaces its buffer from scratch and can't honour append semantics. Added tests for the CONTINUE and MODE=t cases. - Pin `ghostel--managed-buffer-name' to a sentinel in the compile buffer so a compile command's OSC 2 title sequence can't rename the buffer mid-run. - Keep `ghostel-compile-hide-prompts' and `ghostel-compile-clear-buffer' as obsolete no-ops so users' existing `(setq ghostel-compile-... )' forms don't error with void-variable on load. - Convert the zero-arg `--stty-flags' helper to a `defconst'.
There was a problem hiding this comment.
Pull request overview
Reworks ghostel-compile to run compilation commands by spawning shell-file-name -c COMMAND directly under a Ghostel-owned PTY (no interactive shell / OSC 133 dependency), and adds an opt-in global mode to route all compilation-start callers through Ghostel.
Changes:
- Replace OSC-133-driven completion detection with process-sentinel finalization for
ghostel-compile. - Add
ghostel-compile-global-mode(around-advice oncompilation-start) with exclusions for modes likegrep-mode. - Update tests and README to reflect the new lifecycle and global mode behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
ghostel-compile.el |
Rewrites spawn/finalize flow to use a sentinel and introduces ghostel-compile-global-mode + new buffer setup path. |
test/ghostel-test.el |
Removes OSC-133-specific tests and adds tests for sentinel idempotence and global-mode routing/fallthrough behavior. |
README.md |
Documents direct spawning behavior and the new global mode toggle/configuration. |
Comments suppressed due to low confidence (1)
README.md:583
- The README section still states that completion is detected via OSC 133
D;<exit>and thatghostel-shell-integrationmust be active. That’s no longer true forghostel-compileafter this rewrite (it now finalizes via the process sentinel). Please update/relocate this paragraph to clarify that OSC 133 is only required for the generalghostel-command-*-functionshooks, not forghostel-compilecompletion/exit status.
Completion is detected via the OSC 133 `D;<exit>` semantic prompt
marker, so shell integration (`ghostel-shell-integration`, enabled by
default) must be active.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "Regression: multi-line shell paragraphs must be passed verbatim to `sh -c'. | ||
|
|
||
| `ghostel-compile' spawns `sh -c COMMAND' directly, so embedded | ||
| newlines in COMMAND are parsed by the shell as normal script lines | ||
| rather than mangled by a line-editing shell as RET presses." | ||
| (skip-unless (file-executable-p "/bin/sh")) | ||
| (let* ((script "for i in 1 2 3; do\n echo line-$i\ndone\nexit 7")) | ||
| (with-temp-buffer | ||
| (let ((status (call-process "/bin/sh" nil t nil "-c" script))) | ||
| (should (equal 7 status)) | ||
| (let ((out (buffer-string))) | ||
| (should (string-match-p "line-1" out)) | ||
| (should (string-match-p "line-2" out)) | ||
| (should (string-match-p "line-3" out))))))) |
There was a problem hiding this comment.
This test doesn’t exercise the ghostel-compile implementation—it only verifies /bin/sh -c can run a multi-line script. To actually guard the regression, consider stubbing ghostel-compile--spawn/make-process (or ghostel-compile--start) and asserting the spawned argv includes shell-file-name, shell-command-switch, and the exact COMMAND string (including newlines) without re-encoding/splitting.
| "Regression: multi-line shell paragraphs must be passed verbatim to `sh -c'. | |
| `ghostel-compile' spawns `sh -c COMMAND' directly, so embedded | |
| newlines in COMMAND are parsed by the shell as normal script lines | |
| rather than mangled by a line-editing shell as RET presses." | |
| (skip-unless (file-executable-p "/bin/sh")) | |
| (let* ((script "for i in 1 2 3; do\n echo line-$i\ndone\nexit 7")) | |
| (with-temp-buffer | |
| (let ((status (call-process "/bin/sh" nil t nil "-c" script))) | |
| (should (equal 7 status)) | |
| (let ((out (buffer-string))) | |
| (should (string-match-p "line-1" out)) | |
| (should (string-match-p "line-2" out)) | |
| (should (string-match-p "line-3" out))))))) | |
| "Regression: `ghostel-compile--start' must pass multi-line COMMAND verbatim. | |
| This test exercises the Ghostel compile path directly and asserts the | |
| spawned argv contains `shell-file-name', `shell-command-switch', and | |
| the exact COMMAND string including embedded newlines." | |
| (let* ((script "for i in 1 2 3; do\n echo line-$i\ndone\nexit 7") | |
| (captured-command nil) | |
| (test-buffer (generate-new-buffer " *ghostel-test-compile*"))) | |
| (unwind-protect | |
| (cl-letf (((symbol-function 'make-process) | |
| (lambda (&rest args) | |
| (setq captured-command (plist-get args :command)) | |
| 'ghostel-test-process)) | |
| ((symbol-function 'set-process-query-on-exit-flag) | |
| (lambda (&rest _) nil)) | |
| ((symbol-function 'process-put) | |
| (lambda (&rest _) nil)) | |
| ((symbol-function 'set-process-filter) | |
| (lambda (&rest _) nil)) | |
| ((symbol-function 'set-process-sentinel) | |
| (lambda (&rest _) nil)) | |
| ((symbol-function 'process-buffer) | |
| (lambda (&rest _) test-buffer)) | |
| ((symbol-function 'display-buffer) | |
| (lambda (&rest _) test-buffer)) | |
| ((symbol-function 'get-buffer-create) | |
| (lambda (&rest _) test-buffer)) | |
| ((symbol-function 'generate-new-buffer) | |
| (lambda (&rest _) test-buffer))) | |
| (ghostel-compile--start script "*ghostel multiline*" default-directory) | |
| (should (equal shell-file-name (nth 0 captured-command))) | |
| (should (equal shell-command-switch (nth 1 captured-command))) | |
| (should (equal script (nth 2 captured-command)))) | |
| (when (buffer-live-p test-buffer) | |
| (kill-buffer test-buffer))))) |
e5a2904 to
ed21a9e
Compare
The old ghostel-compile typed its command into an already-running
interactive shell via `ghostel--flush-output'. That made every
embedded newline in a multi-line shell paragraph look like a RET
press, so scripts broke. It also tied completion detection to OSC 133
D markers, making shell-integration a hard requirement.
Spawn the command directly. Each invocation runs
`shell-file-name -c COMMAND' via `make-process' through a PTY owned
by the ghostel renderer — no shell between the command and the user.
Multi-line scripts pass through verbatim. The process sentinel
reports the real exit status, so shell-integration is no longer
required. The header ("Compilation started at ...") is written into
the VT terminal before spawn so the user sees the banner live, not
only after finalize. During the run the buffer stays plain
`ghostel-mode' — keystrokes reach the process, which is what makes
interactive programs like `htop', `less', or read prompts work.
When the process exits the sentinel flushes output, trims trailing
grid padding, appends the footer, parses errors over the run's own
output region, and switches the buffer to `ghostel-compile-view-mode'
(read-only, `g'/`n'/`p' bindings, standard error navigation).
Add `ghostel-compile-global-mode', an opt-in global minor mode that
advises `compilation-start' so every caller — `compile', `recompile',
`project-compile', and any other command that goes through
`compilation-start' — automatically runs in a ghostel buffer. Falls
through to the stock implementation for `grep-mode' (output-parser
conflict), `mode=t' (comint), and non-nil `continue' (append
semantics this design can't honour). The excluded set is
customisable via `ghostel-compile-global-mode-excluded-modes'.
`ghostel-recompile' now re-runs into the CURRENT buffer whenever it
holds a local `ghostel-compile--command' — so pressing `g' in a
`*compilation*' buffer (produced by the advice) reuses that buffer and
its window, matching `M-x recompile's behaviour. Previously it always
targeted `*ghostel-compile*', which produced a second unfocused window
side-by-side. `ghostel-compile--prepare-buffer' also resets the
buffer in place (interrupts the old process, erases content, rebuilds
the terminal) rather than killing and recreating it, so the existing
window stays attached across recompiles.
Other details worth noting:
- No more hot shell in `--prepare-buffer'. The previous version
routed through `ghostel--init-buffer' which spawns a shell and in
TRAMP buffers does a full remote-integration round-trip (uploads
temp files and plumbs env); we then killed it immediately, leaking
the remote temp files. Creates the terminal directly now.
- `ghostel--managed-buffer-name' is pinned to a sentinel so an OSC 2
title sequence from the compile command can't rename the buffer
mid-run.
- The scan marker anchors to the end of the rendered header — not
`point-max' of the post-render grid — so `--trim-trailing-blanks'
actually trims the VT grid's blank-row padding between output and
the footer (this padding scales with window height).
- Dropped customs/state that no longer apply: `ghostel-compile-mode'
(and its keymap + `--owns-compilation-minor-mode' flag),
`ghostel-compile-hide-prompts', `ghostel-compile-clear-buffer',
`ghostel-compile--on-start' and `--on-finish' (OSC 133 hooks), the
`--running' state machine, `--finalize-timer' gating, the
`--header-marker' / `--footer-marker' buffer-locals and the
`--clear-markers' helper, and a bash shell-integration exit-status
test.
Tests rewritten to match the new flow. End-to-end native test drives
`ghostel-compile--start' with a multi-line paragraph and asserts
output + exit status. Interactive-input test drives `cat' and asserts
keystrokes round-trip. Global-mode tests cover advice add/remove,
routing, and all three fall-through cases (grep-mode, mode=t,
continue, plus custom-excluded mode).
c3c3dea to
e7164ec
Compare
Two post-#124 issues reported by a user: 1. Initial window width was wrong until the user resized the compile buffer. `ghostel-compile--prepare-buffer' sized the VT from the selected window (the only choice before `display-buffer' runs). `ghostel-compile--start' then called `display-buffer' — which normally splits the frame so the compile buffer lands in a smaller window — and spawned the process with that smaller size. The PTY then agreed with the output window, but the VT still thought it had the selected-window width, so early output (including the header) wrapped at the wrong column and looked garbled until the first `ghostel--window-adjust-process-window-size' fired. `--start' now reconciles the VT to the output window *before* rendering the header, and `--spawn' is handed the same dimensions using `window-max-chars-per-line' (matching `ghostel--spawn-pty'). 2. `M-x kill-compilation' was a no-op. It locates its target via `compilation-buffer-internal-p', which requires `compilation-locs' to be buffer-local. During a ghostel-compile run the buffer stays in `ghostel-mode' (so keys reach the process), so `compilation-mode' never installs that variable. `prepare-buffer' now declares `compilation-locs' buffer-locally, initialized to the same hash-table shape `compilation-mode' uses so any third-party code that reads the value doesn't trip on nil. Finalize switches to `ghostel-compile-view-mode' (derived from `compilation-mode'), which resets `compilation-locs' properly. New tests: - `ghostel-test-compile-reconciles-vt-size-to-outwin' (elisp, mocked): asserts the reconcile fires before header render and spawn, and that VT and PTY end up with identical dimensions. - `ghostel-test-compile-reconciles-skips-when-no-outwin' (elisp, mocked): asserts the `allow-no-window' fallback doesn't call `ghostel--set-size'. - `ghostel-test-compile-kill-compilation-finds-live-buffer' (native, live `cat' run): asserts the buffer is findable via `compilation-buffer-p' / `compilation-find-buffer' (including from an unrelated buffer), then actually invokes `kill-compilation' and waits for the sentinel to finalize the run with a non-zero exit.
Summary
Addresses @dalgong's comment on #104: multi-line shell paragraphs don't work well with the current
ghostel-compile, and the OSC 133 / shell-integration dependency is fragile.ghostel-compilenow runsshell-file-name -c COMMANDviamake-processthrough a PTY owned by the ghostel renderer — no interactive shell between the command and the user. Multi-line scripts are passed verbatim. Completion is detected by the process sentinel, so no OSC 133 markers (or shell integration) are required.ghostel-compile-global-mode. Advisescompilation-startsocompile,recompile,project-compile, and every othercompilation-startcaller run in ghostel automatically.grep-modefalls through by default; customisable viaghostel-compile-global-mode-excluded-modes.ghostel-compile-hide-promptsandghostel-compile-clear-buffer(no shell = no prompts to hide, buffer is always recreated fresh). Drops the--runningstate machine, finalize-timer gating, OSC 133 C/D handlers, and the bash shell-integration exit-status regression test.Behaviour changes for users
ghostel-compileto work.ghostel-compile-hide-prompts,ghostel-compile-clear-buffer.M-x ghostel-compile-global-mode— opt-in, routes all compile-style commands through ghostel.Test plan
make -j4 all— 120 elisp tests + 72 native tests, lint cleanmake test-evil— 30 tests passfor i in 1 2 3; do echo $i; done; exit 7) runs, reports exit 7, output visibleghostel-recompilere-runs the last commandghostel-compile-global-mode 1→M-x compileroutes through ghostel (*compilation*buffer inghostel-compile-view-mode)/tmp/x.c:12:3: error: ...picked up, mode-line shows:exit [1]