From 645af24230bdf9032f1f0d79ff7e62f5ecf7b72a Mon Sep 17 00:00:00 2001 From: "F. Jason Park" Date: Tue, 15 Aug 2023 21:14:07 -0700 Subject: [PATCH] [5.6] 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' to avoid excess timers being added. (erc-speedbar--ensure): Inline `speedbar-enable-update' to avoid unneeded call to `speedbar-set-timer', and ensure it 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 helper function. (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. (erc-speedbar--destroy-nicknames-frame): New function to perform common teardown. * 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 +- .../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..88400f034ba 100644 --- a/lisp/erc/erc-speedbar.el +++ b/lisp/erc/erc-speedbar.el @@ -439,7 +439,6 @@ erc-track--switch-fallback-blockers (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 @@ erc-speedbar--toggle-nicknames-sidebar (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 (display-graphic-p) + (unless speedbar-update-flag + (erc-button--display-error-notice-with-keys + (erc-server-buffer) + "Module `nickbar' needs `speedbar-update-flag' to be non-nil" + " in non-graphic terminals. Setting to t for the current" + " Emacs session. Customize it 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 @@ nickbar 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 @@ nickbar (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 @@ erc-speedbar-toggle-nicknames-window-lock (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 @@ erc-scenarios-status-sidebar--bufbar ;; 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 @@ erc-scenarios-status-sidebar--nickbar (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 @@ erc-scenarios-common--print-trace (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) -- 2.41.0