* bug#72487: calculator-electric-mode causes freeze on subsequent use of minibuffer
2024-08-24 9:27 ` Eli Zaretskii
@ 2024-08-28 8:59 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-09-11 8:45 ` martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors
0 siblings, 1 reply; 8+ messages in thread
From: martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-08-28 8:59 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 72487, kim.jet.wav
[-- Attachment #1: Type: text/plain, Size: 1390 bytes --]
Attached find a patch that should guarantee that only minibuffers are
recorded in a minibuffer window's list of previous buffers. A ChangeLog
appears below. If there are no objections, I'll install this on master
in two weeks.
martin
For minibuffer windows record minibuffers only (Bug#72487)
* src/minibuf.c (zip_minibuffer_stacks): Use wset type
functions. Call 'record-window-buffer' instead of
'push-window-buffer-onto-prev' to handle all sorts of buffers
shown in minibuffer windows in a uniform way.
(read_minibuf): Call 'record-window-buffer' instead of
'push-window-buffer-onto-prev' for same reason as previous.
* lisp/calculator.el (calculator-update-display)
(calculator-save-and-quit): Make sure calculator buffer is live
before operating on it.
* lisp/window.el (record-window-buffer): Handle case where
WINDOW is a minibuffer window: Unconditionally remove WINDOW's
buffer from WINDOW's list of previous buffers and push it if
and only if it is a live minibuffer (Bug#72487). Do not run
'buffer-list-update-hook' if WINDOW is a minibuffer window.
(push-window-buffer-onto-prev): Make it an alias of
'record-window-buffer' so it will run the latter's checks.
(replace-buffer-in-windows): Handle minibuffer windows and
rewrite doc-string accordingly.
* doc/lispref/windows.texi (Buffers and Windows): Explain
handling of minibuffer windows in 'replace-buffer-in-windows'.
[-- Attachment #2: minibuffer-windows.diff --]
[-- Type: text/x-patch, Size: 20091 bytes --]
index 656a44dfcbf..541c91ddae2 100644
--- a/doc/lispref/windows.texi
+++ b/doc/lispref/windows.texi
@@ -2353,6 +2353,12 @@ Buffers and Windows
that are only referenced by window configurations) and remove any
@code{quit-restore} or @code{quit-restore-prev} parameters
(@pxref{Window Parameters}) referencing that buffer.
+
+This function does not replace the buffer specified by
+@var{buffer-or-name} in any minibuffer window showing it, nor does it
+delete minibuffer windows or minibuffer frames. It removes, however,
+that buffer from the lists of previous and next buffers of all
+minibuffer windows.
@end deffn
By default, @code{replace-buffer-in-windows} deletes only windows
diff --git a/lisp/calculator.el b/lisp/calculator.el
index ef1e6d8dbc3..a9fe76259a8 100644
--- a/lisp/calculator.el
+++ b/lisp/calculator.el
@@ -1059,49 +1059,50 @@ calculator-number-to-string
(defun calculator-update-display (&optional force)
"Update the display.
If optional argument FORCE is non-nil, don't use the cached string."
- (set-buffer calculator-buffer)
- ;; update calculator-stack-display
- (when (or force (not (eq (car calculator-stack-display)
- calculator-stack)))
- (setq calculator-stack-display
- (cons calculator-stack
- (if calculator-stack
- (concat
- (let ((calculator-displayer
- (if (and calculator-displayers
- (= 1 (length calculator-stack)))
- ;; customizable display for a single value
- (caar calculator-displayers)
- calculator-displayer)))
- (mapconcat 'calculator-number-to-string
- (reverse calculator-stack)
- " "))
- " "
- (and calculator-display-fragile
- calculator-saved-list
- ;; Hack: use `eq' to compare the number: it's a
- ;; flonum, so `eq' means that its the actual
- ;; number rather than a computation that had an
- ;; equal result (eg, enter 1,3,2, use "v" to see
- ;; the average -- it now shows "2" instead of
- ;; "2 [3]").
- (eq (car calculator-stack)
- (nth calculator-saved-ptr
- calculator-saved-list))
- (if (= 0 calculator-saved-ptr)
- (format "[%s]" (length calculator-saved-list))
- (format "[%s/%s]"
- (- (length calculator-saved-list)
- calculator-saved-ptr)
- (length calculator-saved-list)))))
- ""))))
- (let ((inhibit-read-only t))
- (erase-buffer)
- (insert (calculator-get-display)))
- (set-buffer-modified-p nil)
- (goto-char (if calculator-display-fragile
- (1+ (length calculator-prompt))
- (1- (point)))))
+ (when (buffer-live-p calculator-buffer)
+ (set-buffer calculator-buffer)
+ ;; update calculator-stack-display
+ (when (or force (not (eq (car calculator-stack-display)
+ calculator-stack)))
+ (setq calculator-stack-display
+ (cons calculator-stack
+ (if calculator-stack
+ (concat
+ (let ((calculator-displayer
+ (if (and calculator-displayers
+ (= 1 (length calculator-stack)))
+ ;; customizable display for a single value
+ (caar calculator-displayers)
+ calculator-displayer)))
+ (mapconcat 'calculator-number-to-string
+ (reverse calculator-stack)
+ " "))
+ " "
+ (and calculator-display-fragile
+ calculator-saved-list
+ ;; Hack: use `eq' to compare the number: it's a
+ ;; flonum, so `eq' means that its the actual
+ ;; number rather than a computation that had an
+ ;; equal result (eg, enter 1,3,2, use "v" to see
+ ;; the average -- it now shows "2" instead of
+ ;; "2 [3]").
+ (eq (car calculator-stack)
+ (nth calculator-saved-ptr
+ calculator-saved-list))
+ (if (= 0 calculator-saved-ptr)
+ (format "[%s]" (length calculator-saved-list))
+ (format "[%s/%s]"
+ (- (length calculator-saved-list)
+ calculator-saved-ptr)
+ (length calculator-saved-list)))))
+ ""))))
+ (let ((inhibit-read-only t))
+ (erase-buffer)
+ (insert (calculator-get-display)))
+ (set-buffer-modified-p nil)
+ (goto-char (if calculator-display-fragile
+ (1+ (length calculator-prompt))
+ (1- (point))))))
;;;---------------------------------------------------------------------
;;; Stack computations
@@ -1553,17 +1554,18 @@ calculator-help
(defun calculator-quit ()
"Quit calculator."
(interactive)
- (set-buffer calculator-buffer)
- (let ((inhibit-read-only t)) (erase-buffer))
- (unless calculator-electric-mode
- (ignore-errors
- (while (get-buffer-window calculator-buffer)
- (delete-window (get-buffer-window calculator-buffer)))))
- (kill-buffer calculator-buffer)
- (message "Calculator done.")
- (if calculator-electric-mode
- (throw 'calculator-done nil) ; will kill the buffer
- (setq calculator-buffer nil)))
+ (when (buffer-live-p calculator-buffer)
+ (set-buffer calculator-buffer)
+ (let ((inhibit-read-only t)) (erase-buffer))
+ (unless calculator-electric-mode
+ (ignore-errors
+ (while (get-buffer-window calculator-buffer)
+ (delete-window (get-buffer-window calculator-buffer)))))
+ (kill-buffer calculator-buffer)
+ (message "Calculator done.")
+ (if calculator-electric-mode
+ (throw 'calculator-done nil) ; will kill the buffer
+ (setq calculator-buffer nil))))
(defun calculator-save-and-quit ()
"Quit the calculator, saving the result on the `kill-ring'."
diff --git a/lisp/window.el b/lisp/window.el
index f4226fa4428..368444fbee8 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -4469,75 +4469,86 @@ delete-other-windows-vertically
;;; Windows and buffers.
-;; `prev-buffers' and `next-buffers' are two reserved window slots used
+;; 'prev-buffers' and 'next-buffers' are two reserved window slots used
;; for (1) determining which buffer to show in the window when its
;; buffer shall be buried or killed and (2) which buffer to show for
-;; `switch-to-prev-buffer' and `switch-to-next-buffer'.
+;; 'switch-to-prev-buffer' and 'switch-to-next-buffer'.
-;; `prev-buffers' consists of <buffer, window-start, window-point>
+;; 'prev-buffers' consists of <buffer, window-start, window-point>
;; triples. The entries on this list are ordered by the time their
;; buffer has been removed from the window, the most recently removed
;; buffer's entry being first. The window-start and window-point
-;; components are `window-start' and `window-point' at the time the
+;; components are 'window-start' and 'window-point' at the time the
;; buffer was removed from the window which implies that the entry must
-;; be added when `set-window-buffer' removes the buffer from the window.
+;; be added when 'set-window-buffer' removes the buffer from the window.
-;; `next-buffers' is the list of buffers that have been replaced
-;; recently by `switch-to-prev-buffer'. These buffers are the least
-;; preferred candidates of `switch-to-prev-buffer' and the preferred
-;; candidates of `switch-to-next-buffer' to switch to. This list is
+;; 'next-buffers' is the list of buffers that have been replaced
+;; recently by 'switch-to-prev-buffer'. These buffers are the least
+;; preferred candidates of 'switch-to-prev-buffer' and the preferred
+;; candidates of 'switch-to-next-buffer' to switch to. This list is
;; reset to nil by any action changing the window's buffer with the
-;; exception of `switch-to-prev-buffer' and `switch-to-next-buffer'.
-;; `switch-to-prev-buffer' pushes the buffer it just replaced on it,
-;; `switch-to-next-buffer' pops the last pushed buffer from it.
-
-;; Both `prev-buffers' and `next-buffers' may reference killed buffers
-;; if such a buffer was killed while the window was hidden within a
-;; window configuration. Such killed buffers get removed whenever
-;; `switch-to-prev-buffer' or `switch-to-next-buffer' encounter them.
-
-;; The following function is called by `set-window-buffer' _before_ it
-;; replaces the buffer of the argument window with the new buffer.
-(defun push-window-buffer-onto-prev (&optional window)
- "Push entry for WINDOW's buffer onto WINDOW's prev-buffers list.
-WINDOW must be a live window and defaults to the selected one.
-
-Any duplicate entries for the buffer in the list are removed."
+;; exception of 'switch-to-prev-buffer' and 'switch-to-next-buffer'.
+;; 'switch-to-prev-buffer' pushes the buffer it just replaced on it,
+;; 'switch-to-next-buffer' pops the last pushed buffer from it.
+
+;; The following function is called by 'set-window-buffer' _before_ it
+;; replaces the buffer of the argument window with the new buffer. It
+;; does not record a non-minibuffer buffer (like the one created by
+;; 'calculator' in Electric mode) in a minibuffer window since the code
+;; in minibuf.c cannot handle that. The minibuf.c code calls this
+;; function exclusively to arrange minibuffers shown in minibuffer
+;; windows.
+(defun record-window-buffer (&optional window)
+ "Record WINDOW's buffer.
+Add the buffer currently shown in WINDOW to the list of WINDOW's
+previous buffers. WINDOW must be a live window and defaults to the
+selected one.
+
+If WINDOW is not a minibuffer window, do not record insignificant
+buffers (buffers whose name starts with a space). If WINDOW is a
+minibuffer window, record its buffer if and only if that buffer is a
+live minibuffer (`minibufferp' with LIVE argument non-nil must return
+non-nil for it).
+
+Run `buffer-list-update-hook' if and only if WINDOW is not a minibuffer
+window."
(let* ((window (window-normalize-window window t))
+ (mini (window-minibuffer-p window))
(buffer (window-buffer window))
- (w-list (window-prev-buffers window))
- (entry (assq buffer w-list)))
+ (prev-buffers (window-prev-buffers window))
+ (entry (assq buffer prev-buffers)))
(when entry
- (setq w-list (assq-delete-all buffer w-list)))
- (let ((start (window-start window))
- (point (window-point window)))
- (setq entry
- (cons buffer
- (with-current-buffer buffer
- (if entry
- ;; We have an entry, update marker positions.
- (list (set-marker (nth 1 entry) start)
- (set-marker (nth 2 entry) point))
- (list (copy-marker start)
- (copy-marker
- ;; Preserve window-point-insertion-type
- ;; (Bug#12855)
- point window-point-insertion-type))))))
- (set-window-prev-buffers window (cons entry w-list)))))
+ (setq prev-buffers (assq-delete-all buffer prev-buffers)))
-(defun record-window-buffer (&optional window)
- "Record WINDOW's buffer.
-WINDOW must be a live window and defaults to the selected one."
- (let* ((window (window-normalize-window window t))
- (buffer (window-buffer window)))
;; Reset WINDOW's next buffers. If needed, they are resurrected by
;; `switch-to-prev-buffer' and `switch-to-next-buffer'.
(set-window-next-buffers window nil)
- ;; Don't record insignificant buffers.
- (when (not (eq (aref (buffer-name buffer) 0) ?\s))
- (push-window-buffer-onto-prev window)
- (run-hooks 'buffer-list-update-hook))))
+ ;; For minibuffer windows record live minibuffers only. For normal
+ ;; windows do not record insignificant buffers.
+ (when (if mini
+ (minibufferp buffer t)
+ (eq (aref (buffer-name buffer) 0) ?\s))
+ (let ((start (window-start window))
+ (point (window-point window)))
+ (setq entry
+ (cons buffer
+ (with-current-buffer buffer
+ (if entry
+ ;; We have an entry, update marker positions.
+ (list (set-marker (nth 1 entry) start)
+ (set-marker (nth 2 entry) point))
+ (list (copy-marker start)
+ (copy-marker
+ ;; Preserve window-point-insertion-type
+ ;; (Bug#12855)
+ point window-point-insertion-type))))))
+ (set-window-prev-buffers window (cons entry prev-buffers))
+
+ (unless mini
+ (run-hooks 'buffer-list-update-hook))))))
+
+(defalias 'push-window-buffer-onto-prev 'record-window-buffer)
(defun unrecord-window-buffer (&optional window buffer all)
"Unrecord BUFFER in WINDOW.
@@ -5160,10 +5171,19 @@ delete-windows-on
;; If a window doesn't show BUFFER, unrecord BUFFER in it.
(unrecord-window-buffer window buffer t)))))
+;; Conceptually, 'replace-buffer-in-windows' would not have to touch the
+;; list of previous buffers of a minibuffer window: As a rule,
+;; minibuffers are never deleted and any other buffers shown in a
+;; minibuffer window are not recorded by 'record-window'. To be on the
+;; safe side, 'replace-buffer-in-windows' now scans minibuffer windows
+;; too to make sure that any killed buffer gets removed from all lists
+;; of previous and next buffers. 'replace-buffer-in-windows' still does
+;; _not_ replace the buffer itself in any minibuffer window showing it.
+;; That case is still handled only in 'kill-buffer' itself.
(defun replace-buffer-in-windows (&optional buffer-or-name)
"Replace BUFFER-OR-NAME with some other buffer in all windows showing it.
BUFFER-OR-NAME may be a buffer or the name of an existing buffer and
-defaults to the current buffer. Minibuffer windows are not considered.
+defaults to the current buffer.
If the option `kill-buffer-quit-windows' is nil, behave as follows: With
the exception of side windows, when a window showing BUFFER-OR-NAME is
@@ -5180,21 +5200,29 @@ replace-buffer-in-windows
lists of previous and next buffers of all windows and remove any
`quit-restore' or `quit-restore-prev' parameters mentioning it.
+This function does not replace the buffer specified by BUFFER-OR-NAME in
+any minibuffer window showing it, nor does it delete minibuffer windows
+or minibuffer frames. It removes, however, that buffer from the lists
+of previous and next buffers of all minibuffer windows.
+
If, for any window showing BUFFER-OR-NAME running the abnormal hook
`window-deletable-functions' returns nil, do not delete that window but
show some other buffer in that window.
-This function is called by `kill-buffer' which kills the buffer
-specified by `buffer-or-name' afterwards. It never kills a buffer by
-itself."
+This function is called by `kill-buffer' which effectively kills the
+buffer specified by `buffer-or-name' afterwards. It never kills a
+buffer by itself."
(interactive "bBuffer to replace: ")
(let ((buffer (window-normalize-buffer buffer-or-name)))
- ;; Scan all windows. We have to unrecord BUFFER-OR-NAME in those
- ;; not showing it.
- (dolist (window (window-list-1 nil nil t))
+ ;; Scan all windows including minibuffer windows. We have to
+ ;; unrecord BUFFER-OR-NAME even in those not showing it.
+ (dolist (window (window-list-1 nil t t))
(when (eq (window-buffer window) buffer)
- (if kill-buffer-quit-windows
- (quit-restore-window window 'killing)
+ (cond
+ ((window-minibuffer-p window))
+ (kill-buffer-quit-windows
+ (quit-restore-window window 'killing))
+ (t
(let ((dedicated-side (eq (window-dedicated-p window) 'side)))
(when (or dedicated-side (not (window--delete window t 'kill)))
;; Switch to another buffer in that window.
@@ -5212,7 +5240,7 @@ replace-buffer-in-windows
;; element of the parameter, 'quit-restore-window' cannot
;; possibly show BUFFER instead; so this parameter becomes
;; useless too.
- (unrecord-window-buffer window buffer t)))))
+ (unrecord-window-buffer window buffer t))))))
(defcustom quit-window-hook nil
"Hook run before performing any other actions in the `quit-window' command."
diff --git a/src/minibuf.c b/src/minibuf.c
index f16880011f7..1f94e0e650e 100644
--- a/src/minibuf.c
+++ b/src/minibuf.c
@@ -160,16 +160,15 @@ zip_minibuffer_stacks (Lisp_Object dest_window, Lisp_Object source_window)
set_window_buffer (dest_window, sw->contents, 0, 0);
Fset_window_start (dest_window, Fwindow_start (source_window), Qnil);
Fset_window_point (dest_window, Fwindow_point (source_window));
- dw->prev_buffers = sw->prev_buffers;
+ wset_prev_buffers (dw, sw->prev_buffers);
set_window_buffer (source_window, nth_minibuffer (0), 0, 0);
- sw->prev_buffers = Qnil;
+ wset_prev_buffers (sw, Qnil);
return;
}
- if (live_minibuffer_p (dw->contents))
- call1 (Qpush_window_buffer_onto_prev, dest_window);
- if (live_minibuffer_p (sw->contents))
- call1 (Qpush_window_buffer_onto_prev, source_window);
+ call1 (Qrecord_window_buffer, dest_window);
+ call1 (Qrecord_window_buffer, source_window);
+
acc = merge_c (dw->prev_buffers, sw->prev_buffers, minibuffer_ent_greater);
if (!NILP (acc))
@@ -180,8 +179,9 @@ zip_minibuffer_stacks (Lisp_Object dest_window, Lisp_Object source_window)
Fset_window_start (dest_window, Fcar (Fcdr (d_ent)), Qnil);
Fset_window_point (dest_window, Fcar (Fcdr (Fcdr (d_ent))));
}
- dw->prev_buffers = acc;
- sw->prev_buffers = Qnil;
+
+ wset_prev_buffers (dw, acc);
+ wset_prev_buffers (sw, Qnil);
set_window_buffer (source_window, nth_minibuffer (0), 0, 0);
}
@@ -688,8 +688,8 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt,
Fframe_first_window (MB_frame), Qnil);
}
MB_frame = XWINDOW (XFRAME (selected_frame)->minibuffer_window)->frame;
- if (live_minibuffer_p (XWINDOW (minibuf_window)->contents))
- call1 (Qpush_window_buffer_onto_prev, minibuf_window);
+
+ call1 (Qrecord_window_buffer, minibuf_window);
record_unwind_protect_void (minibuffer_unwind);
if (read_minibuffer_restore_windows)
diff --git a/src/window.c b/src/window.c
index 35092ddd582..34968ac824f 100644
--- a/src/window.c
+++ b/src/window.c
@@ -3647,9 +3647,17 @@ replace_buffer_in_windows (Lisp_Object buffer)
call1 (Qreplace_buffer_in_windows, buffer);
}
-/* If BUFFER is shown in a window, safely replace it with some other
- buffer in all windows of all frames, even those on other keyboards. */
-
+/** If BUFFER is shown in any window, safely replace it with some other
+ buffer in all windows of all frames, even those on other keyboards.
+ Do not delete any window.
+
+ This function is called by Fkill_buffer when it detects that
+ replacing BUFFER in some window showing BUFFER has failed. It
+ assumes that ‘replace-buffer-in-windows’ has removed any entry
+ referencing BUFFER from any window's lists of previous and next
+ buffers and that window's ‘quit-restore’ and 'quit-restore-prev'
+ parameters.
+*/
void
replace_buffer_in_windows_safely (Lisp_Object buffer)
{
^ permalink raw reply related [flat|nested] 8+ messages in thread