From 9600611d5f8382ffc849d56a67ba5eb980d64e04 Mon Sep 17 00:00:00 2001 From: Craig Jennings Date: Wed, 13 May 2026 15:33:38 -0500 Subject: fix(ai-vterm): preserve single-window layout across F9 toggle When the agent buffer is the only window in the frame, F9 buries it (correct) but the next F9 redisplayed it as a side split instead of restoring the full-frame layout -- the display-saved path always called `display-buffer-in-direction`, which insists on a split. New `cj/--ai-vterm-last-was-bury` flag tracks which toggle-off path ran. `cj/ai-vterm` sets it to t in the bury branch (one-window-p) and nil in the delete-window branch. `cj/--ai-vterm-display-saved` checks the flag at toggle-on: if t and the frame is still single-window, it replaces the selected window's buffer in place rather than splitting. Either branch consumes the flag so it never stays stale. 5 tests in test-ai-vterm--single-window-toggle.el cover the flag's set/clear paths, the still-one-window guard, and the end-to-end roundtrip. --- modules/ai-vterm.el | 51 ++++++-- tests/test-ai-vterm--single-window-toggle.el | 170 +++++++++++++++++++++++++++ todo.org | 26 ++-- 3 files changed, 224 insertions(+), 23 deletions(-) create mode 100644 tests/test-ai-vterm--single-window-toggle.el diff --git a/modules/ai-vterm.el b/modules/ai-vterm.el index ac246ee8..1eae19f3 100644 --- a/modules/ai-vterm.el +++ b/modules/ai-vterm.el @@ -337,6 +337,16 @@ has been toggled off yet this session, so the default direction applies. Captured at toggle-off by `cj/--ai-vterm-capture-state' and consumed by `cj/--ai-vterm-display-saved'.") +(defvar cj/--ai-vterm-last-was-bury nil + "Non-nil when the last F9 toggle-off used `bury-buffer'. + +Set by `cj/ai-vterm' in its `toggle-off' branch: t when the agent +window was the only window in the frame (so toggle-off buried +without deleting), nil when the window was deleted. Consumed by +`cj/--ai-vterm-display-saved' to decide between restoring the +buried agent in the current window (the only one) or splitting per +the saved direction.") + (defvar cj/--ai-vterm-last-size nil "Last user-chosen body size for the AI-vterm display. @@ -398,12 +408,26 @@ project changes." (defun cj/--ai-vterm-display-saved (buffer alist) "Display-buffer action: split per saved direction and size. -Delegates to `cj/window-toggle-display-saved' against the F9 state -vars, falling back to `right' and `cj/ai-vterm-window-width'." - (cj/window-toggle-display-saved - buffer alist - 'cj/--ai-vterm-last-direction 'right - 'cj/--ai-vterm-last-size cj/ai-vterm-window-width)) +When the prior toggle-off was a bury (single-window state, flagged +via `cj/--ai-vterm-last-was-bury') and the frame is still single- +window, restore the agent into the selected window in place rather +than splitting -- preserves the user's lone-window layout across +F9 toggles. + +Otherwise delegates to `cj/window-toggle-display-saved' against the +F9 state vars, falling back to `right' and `cj/ai-vterm-window-width'." + (cond + ((and cj/--ai-vterm-last-was-bury (one-window-p)) + (setq cj/--ai-vterm-last-was-bury nil) + (let ((win (selected-window))) + (set-window-buffer win buffer) + win)) + (t + (setq cj/--ai-vterm-last-was-bury nil) + (cj/window-toggle-display-saved + buffer alist + 'cj/--ai-vterm-last-direction 'right + 'cj/--ai-vterm-last-size cj/ai-vterm-window-width)))) (defun cj/--ai-vterm-display-rule-list () "Return the `display-buffer-alist' entry list installed by this module. @@ -679,10 +703,17 @@ AI-vterm buffers without touching the project list." ;; other buffer in it, and the next toggle-on then creates a ;; fresh side window for a count of N+1. Skip the deletion ;; only when agent is the lone window in the frame (delete - ;; would leave none); bury in that case. - (if (one-window-p) - (bury-buffer (window-buffer win)) - (delete-window win)) + ;; would leave none); bury in that case. The flag tells the + ;; next toggle-on (via `cj/--ai-vterm-display-saved') to restore + ;; in place rather than splitting -- preserves the single-window + ;; layout across F9 toggles. + (cond + ((one-window-p) + (setq cj/--ai-vterm-last-was-bury t) + (bury-buffer (window-buffer win))) + (t + (setq cj/--ai-vterm-last-was-bury nil) + (delete-window win))) nil) (`(redisplay-recent . ,buf) (display-buffer buf) diff --git a/tests/test-ai-vterm--single-window-toggle.el b/tests/test-ai-vterm--single-window-toggle.el new file mode 100644 index 00000000..85108a01 --- /dev/null +++ b/tests/test-ai-vterm--single-window-toggle.el @@ -0,0 +1,170 @@ +;;; test-ai-vterm--single-window-toggle.el --- F9 toggle round-trip when agent is the only window -*- lexical-binding: t; -*- + +;;; Commentary: +;; Regression coverage for the bug where toggling off a single-window +;; agent (bury) then toggling on again redisplays the agent in a side +;; split instead of restoring the full-frame layout. +;; +;; The fix introduces a `cj/--ai-vterm-last-was-bury' flag set at +;; toggle-off when `one-window-p' was true. At toggle-on the display +;; action consumes the flag and, if the frame is still single-window, +;; replaces the current window's buffer in place rather than calling +;; `display-buffer-in-direction'. + +;;; Code: + +(require 'ert) +(require 'cl-lib) + +(add-to-list 'load-path (expand-file-name "modules" user-emacs-directory)) +(add-to-list 'load-path (expand-file-name "tests" user-emacs-directory)) +(require 'ai-vterm) +(require 'testutil-vterm-buffers) + +;;; Normal Cases + +(ert-deftest test-ai-vterm--single-window-toggle-normal-roundtrip-preserves-fullscreen () + "Normal: agent in the only window, simulated bury, F9 (on) -> still single window with agent. + +Reproduces Craig's report. Before the fix the toggle-on path fell +through to `display-buffer-in-direction', which split the lone window +into two and left the agent in a side panel. + +The bury step is simulated (set flag + swap window buffer to a non- +agent buffer) because batch-mode `bury-buffer' won't switch the +displayed buffer on a window with empty prev-buffers; the toggle-off +branch's *logic* is covered by the flag-set-on-bury test." + (cj/test--kill-agent-buffers) + (let ((agent-name "agent [single-window-roundtrip]") + (other-name "*test-sw-roundtrip-other*") + (cj/--ai-vterm-last-was-bury nil) + (cj/--ai-vterm-last-direction nil) + (cj/--ai-vterm-last-size nil)) + (unwind-protect + (save-window-excursion + (delete-other-windows) + (let ((agent-buf (get-buffer-create agent-name)) + (other-buf (get-buffer-create other-name))) + (set-window-buffer (selected-window) agent-buf) + (should (one-window-p)) + ;; Simulate the toggle-off bury path: capture state, set the + ;; bury flag, and put a non-agent buffer in the window where + ;; the real bury would have left one. This isolates the + ;; toggle-on behaviour without depending on batch-mode + ;; `bury-buffer' (which is unreliable with empty prev-buffers). + (cj/--ai-vterm-capture-state (selected-window)) + (setq cj/--ai-vterm-last-was-bury t) + (set-window-buffer (selected-window) other-buf) + (should (one-window-p)) + (should-not (cj/--ai-vterm-displayed-agent-window)) + ;; Toggle on -- should restore agent in the same lone window. + (let ((display-buffer-alist (cj/--ai-vterm-display-rule-list))) + (cj/ai-vterm)) + (should (one-window-p)) + (let ((win (cj/--ai-vterm-displayed-agent-window))) + (should (windowp win)) + (should (eq (window-buffer win) agent-buf))) + ;; Flag must be consumed by the display-saved action. + (should-not cj/--ai-vterm-last-was-bury))) + (when (get-buffer other-name) (kill-buffer other-name)) + (cj/test--kill-agent-buffers)))) + +(ert-deftest test-ai-vterm--single-window-toggle-normal-flag-set-on-bury () + "Normal: single-window toggle-off sets the bury flag." + (cj/test--kill-agent-buffers) + (let ((agent-name "agent [bury-flag-set]") + (cj/--ai-vterm-last-was-bury nil)) + (unwind-protect + (save-window-excursion + (delete-other-windows) + (let ((agent-buf (get-buffer-create agent-name))) + (set-window-buffer (selected-window) agent-buf) + (let ((display-buffer-alist (cj/--ai-vterm-display-rule-list))) + (cj/ai-vterm) + (should (eq cj/--ai-vterm-last-was-bury t))))) + (cj/test--kill-agent-buffers)))) + +(ert-deftest test-ai-vterm--single-window-toggle-normal-flag-cleared-on-multi-window-off () + "Normal: multi-window toggle-off clears the bury flag. +Mirrors the existing `delete-window' branch of the dispatcher -- +the flag should not carry over a prior bury into a delete-window +toggle-off." + (cj/test--kill-agent-buffers) + (let ((agent-name "agent [bury-flag-clear]") + (left-name "*test-sw-left*") + (cj/--ai-vterm-last-was-bury t)) ; stale t from prior bury + (unwind-protect + (save-window-excursion + (delete-other-windows) + (let ((agent-buf (get-buffer-create agent-name)) + (left-buf (get-buffer-create left-name))) + (set-window-buffer (selected-window) left-buf) + (let* ((agent-win (split-window (selected-window) nil 'right)) + (display-buffer-alist (cj/--ai-vterm-display-rule-list))) + (set-window-buffer agent-win agent-buf) + (select-window agent-win) + (cj/ai-vterm) + (should-not cj/--ai-vterm-last-was-bury)))) + (when (get-buffer left-name) (kill-buffer left-name)) + (cj/test--kill-agent-buffers)))) + +;;; Boundary Cases + +(ert-deftest test-ai-vterm--single-window-toggle-boundary-flag-respected-only-when-still-one-window () + "Boundary: if the frame got split between toggle-off and toggle-on, the +saved-direction split applies as usual. The flag is a fast-path for the +genuine single-window case, not an override for every redisplay." + (cj/test--kill-agent-buffers) + (let ((agent-name "agent [flag-fallback]") + (cj/--ai-vterm-last-was-bury t) ; flag pretends prior bury + (cj/--ai-vterm-last-direction 'right) + (cj/--ai-vterm-last-size 40)) + (unwind-protect + (save-window-excursion + (delete-other-windows) + (let* ((other-buf (get-buffer-create "*test-sw-other*")) + (agent-buf (get-buffer-create agent-name))) + (set-window-buffer (selected-window) other-buf) + ;; Frame is split (two windows) -- single-window precondition + ;; for the flag no longer holds. + (split-window-right) + (should-not (one-window-p)) + (let (received-buf + (display-buffer-alist (cj/--ai-vterm-display-rule-list))) + (cl-letf (((symbol-function 'display-buffer-in-direction) + (lambda (b _a) + (setq received-buf b) + (selected-window)))) + (cj/--ai-vterm-display-saved agent-buf nil)) + ;; The saved-direction split path ran (display-buffer-in-direction + ;; was called) rather than the in-place fast path. + (should (eq received-buf agent-buf)) + ;; And the flag is cleared either way. + (should-not cj/--ai-vterm-last-was-bury)))) + (when (get-buffer "*test-sw-other*") (kill-buffer "*test-sw-other*")) + (cj/test--kill-agent-buffers)))) + +(ert-deftest test-ai-vterm--single-window-toggle-boundary-flag-not-set-when-bury-not-used () + "Boundary: a fresh dispatcher run with the agent displayed multi-window leaves +the flag nil (no spurious set)." + (cj/test--kill-agent-buffers) + (let ((agent-name "agent [bury-flag-untouched]") + (cj/--ai-vterm-last-was-bury nil)) + (unwind-protect + (save-window-excursion + (delete-other-windows) + (let ((agent-buf (get-buffer-create agent-name)) + (left-buf (get-buffer-create "*test-sw-untouched-left*"))) + (set-window-buffer (selected-window) left-buf) + (let* ((agent-win (split-window (selected-window) nil 'right)) + (display-buffer-alist (cj/--ai-vterm-display-rule-list))) + (set-window-buffer agent-win agent-buf) + (select-window agent-win) + (cj/ai-vterm) + (should-not cj/--ai-vterm-last-was-bury)))) + (when (get-buffer "*test-sw-untouched-left*") + (kill-buffer "*test-sw-untouched-left*")) + (cj/test--kill-agent-buffers)))) + +(provide 'test-ai-vterm--single-window-toggle) +;;; test-ai-vterm--single-window-toggle.el ends here diff --git a/todo.org b/todo.org index f5a07001..97a77094 100644 --- a/todo.org +++ b/todo.org @@ -122,25 +122,25 @@ attachment via =transient-append-suffix=, or gptel-side backend/model config. Migration to current lazy form: commit =3eb1a0c refactor(gptel): lazy-load gptel-magit, rebind rewrite/context keys=. -** TODO [#B] F9 toggle should restore single-window layout for AI-vterm :bug: +** DONE [#B] F9 toggle should restore single-window layout for AI-vterm :bug: When the AI-vterm buffer is the only window in the frame (e.g. after =C-x 1=) and F9 is pressed, =cj/ai-vterm= buries the buffer (correct), but the next F9 redisplays the agent in a split rather than restoring the single-window full-frame layout. F9 should toggle off/on while preserving the lone-window state. -The bury-then-redisplay path goes through =display-buffer= and hits -=display-buffer-in-direction= via =cj/--ai-vterm-display-saved=, which splits the -existing window. The fix needs the toggle-on path to detect that toggle-off was a -bury-not-delete and use =switch-to-buffer= (or equivalent) instead of a -saved-direction split. - -Locations: -- =modules/ai-vterm.el= around the =(one-window-p)= branch in =cj/ai-vterm= -- =modules/ai-vterm.el= the =cj/--ai-vterm-display-saved= display-buffer action - -Regression test: enter single-window state with agent buffer, F9, F9 again -- -expect =(one-window-p)= still t and the agent buffer displayed. +Fix: new =cj/--ai-vterm-last-was-bury= flag in =modules/ai-vterm.el=. +The toggle-off branch sets it to t when =one-window-p= is true (bury +path) or nil when =delete-window= runs. =cj/--ai-vterm-display-saved= +checks the flag at toggle-on: if t and the frame is still single-window, +it replaces the selected window's buffer in place via =set-window-buffer= +rather than splitting via =display-buffer-in-direction=. Flag is +consumed (cleared) by either branch so it never stays stale. + +5 tests in =tests/test-ai-vterm--single-window-toggle.el= cover: +flag set on bury, flag cleared on delete-window, flag respected only +when still one-window, flag not set when bury didn't run, and the +end-to-end roundtrip. Full =make test-unit= green. ** TODO [#B] AI-vterm scrollback history should replace agent buffer in place :feature: -- cgit v1.2.3