[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
master f8b710e54fb 3/3: Prevent unwanted recursion in erc-nickbar-disabl
From: |
F. Jason Park |
Subject: |
master f8b710e54fb 3/3: Prevent unwanted recursion in erc-nickbar-disable |
Date: |
Fri, 25 Aug 2023 17:52:04 -0400 (EDT) |
branch: master
commit f8b710e54fb8b29f39bf021ccfb993f881b0134a
Author: F. Jason Park <jp@neverwas.me>
Commit: F. Jason Park <jp@neverwas.me>
Prevent unwanted recursion in erc-nickbar-disable
* lisp/erc/erc-speedbar.el (erc-status-sidebar-mode--unhook): Remove
forward declaration.
(erc-speedbar--toggle-nicknames-sidebar): Inline
`erc-speedbar-close-nicknames-window'. Don't call
`erc-speedbar-browser' and thus avoid adding excess timers.
(erc-speedbar--ensure): Inline `speedbar-enable-update' to avoid
unneeded call to `speedbar-set-timer', and ensure it only runs in
`speedbar-buffer'.
(erc-speedbar--shutting-down-p): New flag variable to avoid recursive
calls to `dframe-close-frame' and friends.
(erc-nickbar-mode, erc-nickbar-enable, erc-nickbar-disable): Move
logic formerly performed by `speedbar-disable-update' to
`erc-speedbar--toggle-nicknames-sidebar'. When disabling, guard
against recursive calls to `dframe-close-frame' and friends.
(erc-speedbar--get-timers): New utility function, also for use in
testing.
(erc-speedbar--dframe-controlled): Bind
`erc-speedbar--shutting-down-p' flag non-nil around call to
`erc-nickbar-mode'. Remove excess timer left behind due to
incompatible behavior from `dframe-close-frame'. Let caller kill
buffer.
(erc-speedbar-close-nicknames-window): Remove unused command, new in
ERC 5.6 and Emacs 30.
* test/lisp/erc/erc-scenarios-status-sidebar.el
(erc-speedbar-close-nicknames-window): Remove forward declaration.
(erc-speedbar--get-timers): Add forward declaration.
(erc-scenarios-status-sidebar--nickbar): Fix faulty expectations of
desired behavior when disabling module. Ensure timers canceled.
* test/lisp/erc/resources/erc-scenarios-common.el
(erc-scenarios-common--make-bindings): Shadow `timer-idle-list' to
avoid polluting global test environment with stray timers.
(Bug#63595)
---
lisp/erc/erc-speedbar.el | 112 +++++++++++++++---------
test/lisp/erc/erc-scenarios-status-sidebar.el | 13 ++-
test/lisp/erc/resources/erc-scenarios-common.el | 1 +
3 files changed, 79 insertions(+), 47 deletions(-)
diff --git a/lisp/erc/erc-speedbar.el b/lisp/erc/erc-speedbar.el
index f5fbaac767d..625d59530b0 100644
--- a/lisp/erc/erc-speedbar.el
+++ b/lisp/erc/erc-speedbar.el
@@ -439,7 +439,6 @@ The INDENT level is ignored."
(defvar erc-status-sidebar-buffer-name)
(declare-function erc-status-sidebar-set-window-preserve-size
"erc-status-sidebar" nil)
-(declare-function erc-status-sidebar-mode--unhook "erc-status-sidebar" nil)
(defvar erc-speedbar--buffer-options
'((speedbar-update-flag . t)
@@ -490,36 +489,64 @@ The INDENT level is ignored."
(cl-assert (buffer-live-p speedbar-buffer))
(if (or (and force (< arg 0))
(and (not force) (get-buffer-window speedbar-buffer nil)))
- (erc-speedbar-close-nicknames-window nil)
+ ;; Close associated windows and stop updating but leave timer.
+ (progn
+ (dolist (window (get-buffer-window-list speedbar-buffer nil t))
+ (unless (frame-root-window-p window)
+ (when erc-speedbar--hidden-speedbar-frame
+ (cl-assert
+ (not (eq (window-frame window)
+ erc-speedbar--hidden-speedbar-frame))))
+ (delete-window window)))
+ (with-current-buffer speedbar-buffer
+ (setq speedbar-update-flag nil)
+ (speedbar-set-mode-line-format)))
(when (or (not force) (>= arg 0))
(with-selected-frame speedbar-frame
(erc-speedbar--emulate-sidebar-set-window-preserve-size)))))
- (when (or (not force) (>= arg 0))
- (let ((speedbar-frame-parameters (backquote-list*
- '(visibility . nil)
- '(no-other-frame . t)
- speedbar-frame-parameters))
- (speedbar-after-create-hook #'erc-speedbar--emulate-sidebar))
- (erc-speedbar-browser)
- ;; If we put the remaining parts in the "create hook" along
- ;; with everything else, the frame with `window-main-window'
- ;; gets raised and steals focus if you've switched away from
- ;; Emacs in the meantime.
- (make-frame-invisible speedbar-frame)
- (select-frame (setq speedbar-frame (previous-frame)))
- (erc-speedbar--emulate-sidebar-set-window-preserve-size))))))
+ (when-let (((or (not force) (>= arg 0)))
+ (speedbar-frame-parameters (backquote-list*
+ '(visibility . nil)
+ '(no-other-frame . t)
+ speedbar-frame-parameters))
+ (speedbar-after-create-hook #'erc-speedbar--emulate-sidebar))
+ (erc-install-speedbar-variables)
+ ;; Run before toggling mode to prevent timer from being
+ ;; created twice.
+ (speedbar-change-initial-expansion-list "ERC")
+ (speedbar-frame-mode 1)
+ ;; If we put the remaining parts in the "create hook" along
+ ;; with everything else, the frame with `window-main-window'
+ ;; gets raised and steals focus if you've switched away from
+ ;; Emacs in the meantime.
+ (make-frame-invisible speedbar-frame)
+ (select-frame (setq speedbar-frame (previous-frame)))
+ (erc-speedbar--emulate-sidebar-set-window-preserve-size))))
+ (cl-assert (not (cdr (erc-speedbar--get-timers))) t))
(defun erc-speedbar--ensure (&optional force)
(when (or (erc-server-buffer) force)
(when erc-track-mode
(cl-pushnew '(derived-mode . speedbar-mode)
erc-track--switch-fallback-blockers :test #'equal))
+ (unless speedbar-update-flag
+ (erc-button--display-error-notice-with-keys
+ (erc-server-buffer)
+ "Module `nickbar' needs `speedbar-update-flag' to be non-nil"
+ (and (not (display-graphic-p)) " in text terminals")
+ ". Setting to t for the current Emacs session."
+ " Customize it permanently to avoid this message.")
+ (setq speedbar-update-flag t))
(erc-speedbar--toggle-nicknames-sidebar +1)
- (speedbar-enable-update)))
+ (with-current-buffer speedbar-buffer
+ (setq speedbar-update-flag t)
+ (speedbar-set-mode-line-format))))
+
+(defvar erc-speedbar--shutting-down-p nil)
;;;###autoload(autoload 'erc-nickbar-mode "erc-speedbar" nil t)
(define-erc-module nickbar nil
- "Show nicknames in a side window.
+ "Show nicknames for current target buffer in a side window.
When enabling, create a speedbar session if one doesn't exist and
show its buffer in an `erc-status-sidebar' window instead of a
separate frame. When disabling, close the window or, with a
@@ -527,8 +554,8 @@ negative prefix arg, destroy the session.
WARNING: this module may produce unwanted side effects, like the
raising of frames or the stealing of input focus. If you witness
-such an occurrence, and can reproduce it, please file a bug
-report with \\[erc-bug]."
+such a thing and can reproduce it, please file a bug report with
+\\[erc-bug]."
((add-hook 'erc--setup-buffer-hook #'erc-speedbar--ensure)
(erc-speedbar--ensure)
(unless (or erc--updating-modules-p
@@ -542,31 +569,44 @@ report with \\[erc-bug]."
(erc-error "Not initializing `erc-nickbar-mode' in %s"
(current-buffer))))))
((remove-hook 'erc--setup-buffer-hook #'erc-speedbar--ensure)
- (speedbar-disable-update)
(when erc-track-mode
(setq erc-track--switch-fallback-blockers
(remove '(derived-mode . speedbar-mode)
erc-track--switch-fallback-blockers)))
(erc-speedbar--toggle-nicknames-sidebar -1)
- (when-let ((arg erc--module-toggle-prefix-arg)
+ (when-let (((not erc-speedbar--shutting-down-p))
+ (arg erc--module-toggle-prefix-arg)
((numberp arg))
((< arg 0)))
- (erc-speedbar-close-nicknames-window 'kill))))
+ (with-current-buffer speedbar-buffer
+ (dframe-close-frame)
+ (setq erc-speedbar--hidden-speedbar-frame nil)))))
+
+(defun erc-speedbar--get-timers ()
+ (cl-remove #'dframe-timer-fn timer-idle-list
+ :key #'timer--function
+ :test-not #'eq))
(defun erc-speedbar--dframe-controlled (arg)
+ (when speedbar-buffer
+ (cl-assert (eq speedbar-buffer (current-buffer))))
(when (and erc-speedbar--hidden-speedbar-frame (numberp arg) (< arg 0))
(when erc-nickbar-mode
- (erc-nickbar-mode -1))
+ (let ((erc-speedbar--shutting-down-p t))
+ (erc-nickbar-mode -1)))
(setq speedbar-frame erc-speedbar--hidden-speedbar-frame
erc-speedbar--hidden-speedbar-frame nil)
;; It's unknown whether leaving the frame invisible interferes
- ;; with the upstream teardown procedure.
+ ;; with the upstream teardown sequence.
(when (display-graphic-p)
(make-frame-visible speedbar-frame))
- (speedbar-frame-mode arg)
- (when speedbar-buffer
- (kill-buffer speedbar-buffer)
- (setq speedbar-buffer nil))))
+ (speedbar-frame-mode arg) ; -1
+ ;; As of Emacs 29, `dframe-set-timer' can't remove `dframe-timer'.
+ (cl-assert (= 1 (length (erc-speedbar--get-timers))) t)
+ (cancel-function-timers #'dframe-timer-fn)
+ ;; `dframe-close-frame' kills the buffer but no function in
+ ;; erc-speedbar.el resets this to nil.
+ (setq speedbar-buffer nil)))
(defun erc-speedbar-toggle-nicknames-window-lock ()
"Toggle whether nicknames window is selectable with \\[other-window]."
@@ -578,20 +618,6 @@ report with \\[erc-bug]."
(set-window-parameter window 'no-other-window (not val))
(message "nick-window: %s" (if val "selectable" "protected")))))
-(defun erc-speedbar-close-nicknames-window (kill)
- (interactive "P")
- (if kill
- (with-current-buffer speedbar-buffer
- (dframe-close-frame)
- (cl-assert (not erc-nickbar-mode))
- (setq erc-speedbar--hidden-speedbar-frame nil))
- (dolist (window (get-buffer-window-list speedbar-buffer nil t))
- (unless (frame-root-window-p window)
- (when erc-speedbar--hidden-speedbar-frame
- (cl-assert (not (eq (window-frame window)
- erc-speedbar--hidden-speedbar-frame))))
- (delete-window window)))))
-
;;;; Nicks integration
diff --git a/test/lisp/erc/erc-scenarios-status-sidebar.el
b/test/lisp/erc/erc-scenarios-status-sidebar.el
index 92229121c9f..3a047bf3983 100644
--- a/test/lisp/erc/erc-scenarios-status-sidebar.el
+++ b/test/lisp/erc/erc-scenarios-status-sidebar.el
@@ -94,7 +94,7 @@
;; terminal, and we lack a fixture for that. Please try running this
;; test interactively with both graphical Emacs and non.
(declare-function erc-nickbar-mode "erc-speedbar" (arg))
-(declare-function erc-speedbar-close-nicknames-window "erc-speedbar" (kill))
+(declare-function erc-speedbar--get-timers "erc-speedbar" nil)
(declare-function speedbar-timer-fn "speedbar" nil)
(defvar erc-nickbar-mode)
(defvar speedbar-buffer)
@@ -154,16 +154,21 @@
(ert-info ("Core toggle and kill commands work")
;; Avoid using API, e.g., `erc-status-sidebar-buffer-exists-p',
;; etc. for testing commands that call those same functions.
- (erc-nickbar-mode -1)
+ (call-interactively #'erc-nickbar-mode)
+ (should-not erc-nickbar-mode)
(should-not (and speedbar-buffer
(get-buffer-window speedbar-buffer)))
+ (should speedbar-buffer)
+
(erc-nickbar-mode +1)
(should (and speedbar-buffer
(get-buffer-window speedbar-buffer)))
(should (get-buffer " SPEEDBAR"))
- (erc-speedbar-close-nicknames-window 'kill)
+ (erc-nickbar-mode -1)
(should-not (get-buffer " SPEEDBAR"))
(should-not erc-nickbar-mode)
- (should-not (cdr (frame-list)))))))
+ (should-not (cdr (frame-list)))))
+
+ (should-not (erc-speedbar--get-timers))))
;;; erc-scenarios-status-sidebar.el ends here
diff --git a/test/lisp/erc/resources/erc-scenarios-common.el
b/test/lisp/erc/resources/erc-scenarios-common.el
index 972faa5c73f..2eb040d28d9 100644
--- a/test/lisp/erc/resources/erc-scenarios-common.el
+++ b/test/lisp/erc/resources/erc-scenarios-common.el
@@ -122,6 +122,7 @@
(inhibit-interaction t)
(auth-source-do-cache nil)
(timer-list (copy-sequence timer-list))
+ (timer-idle-list (copy-sequence timer-idle-list))
(erc-auth-source-parameters-join-function nil)
(erc-autojoin-channels-alist nil)
(erc-server-auto-reconnect nil)