From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: "J.P." Newsgroups: gmane.emacs.bugs Subject: bug#63595: 30.0.50; ERC 5.6: Add buffer-list and nick-list modules Date: Wed, 16 Aug 2023 07:04:26 -0700 Message-ID: <87jztvdqxh.fsf__30741.4362834365$1692194723$gmane$org@neverwas.me> References: <87lehkt97a.fsf@neverwas.me> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="27181"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Cc: emacs-erc@gnu.org To: 63595@debbugs.gnu.org Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Wed Aug 16 16:05:16 2023 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1qWH99-0006q5-2r for geb-bug-gnu-emacs@m.gmane-mx.org; Wed, 16 Aug 2023 16:05:15 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1qWH8z-0002NG-Ft; Wed, 16 Aug 2023 10:05:06 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1qWH8x-0002Gw-1I for bug-gnu-emacs@gnu.org; Wed, 16 Aug 2023 10:05:03 -0400 Original-Received: from debbugs.gnu.org ([2001:470:142:5::43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1qWH8w-0007MD-MU for bug-gnu-emacs@gnu.org; Wed, 16 Aug 2023 10:05:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1qWH8w-0003iP-79 for bug-gnu-emacs@gnu.org; Wed, 16 Aug 2023 10:05:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: "J.P." Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Wed, 16 Aug 2023 14:05:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 63595 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 63595-submit@debbugs.gnu.org id=B63595.169219468314253 (code B ref 63595); Wed, 16 Aug 2023 14:05:02 +0000 Original-Received: (at 63595) by debbugs.gnu.org; 16 Aug 2023 14:04:43 +0000 Original-Received: from localhost ([127.0.0.1]:41735 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1qWH8c-0003hn-TM for submit@debbugs.gnu.org; Wed, 16 Aug 2023 10:04:43 -0400 Original-Received: from mail-108-mta140.mxroute.com ([136.175.108.140]:33815) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1qWH8a-0003he-QF for 63595@debbugs.gnu.org; Wed, 16 Aug 2023 10:04:42 -0400 Original-Received: from mail-111-mta2.mxroute.com ([136.175.111.2] filter006.mxroute.com) (Authenticated sender: mN4UYu2MZsgR) by mail-108-mta140.mxroute.com (ZoneMTA) with ESMTPSA id 189fea99aa0000d7b6.001 for <63595@debbugs.gnu.org> (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384); Wed, 16 Aug 2023 14:04:35 +0000 X-Zone-Loop: 41c0d243dc9e67aaa847a0cac01ace96775dad027fc9 X-Originating-IP: [136.175.111.2] DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=neverwas.me ; s=x; h=Content-Type:MIME-Version:Message-ID:Date:References:In-Reply-To: Subject:Cc:To:From:Sender:Reply-To:Content-Transfer-Encoding:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=8M8J8n5QBlJWd1eRSDhxaMy16vKzx73KIA9lBvT0/pQ=; b=hbbGPqAAHeOSEQK9L9rztrylBl mLclJtAdRaNsWxDMeWVkaKYkrmglEIlh0ieCdA4SCa+/6rkf6ZHGj7mFRkWasY8KH+RYdAb6XSz/W 7etuQlDn9UahfwzuXCVUB/e5Ftx4TXpTcfD5YXQhgdqrkwNj9frskiut7WweyTqZ3FLF7i00Tu28U 0OYJ8l/beyMICf4a4GZrkSQyhkwaDkNbamEqboo9kOMafxJzu64DezhL3K68GI+vbVVf6GKYw7eOu n5hdWZrQ5hxiy7IsyylzZFt0vRtLSlUtk8OKNMmDQVWZDsz3XDIq2GiPCV/3fDeZ/a7wjSi3vlJmo nCFlvY3w==; In-Reply-To: <87lehkt97a.fsf@neverwas.me> (J. P.'s message of "Fri, 19 May 2023 12:25:29 -0700") X-Authenticated-Id: masked@neverwas.me X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:267589 Archived-At: --=-=-= Content-Type: text/plain Some issues with the `nickbar' module have surfaced that may be related to another issue involving unexpected connection loss. Thanks to Corwin for reporting this. It appears that the teardown logic in `erc-nickbar-disable' was calling `dframe-close-frame', `speedbar-set-timer', and `speedbar-timer-fn' repeatedly and unnecessarily, which resulted in redundant scheduling of the same update timer (there should only be one per speedbar instance). Other problems involving the variable `speedbar-update-flag' were also discovered, as well as a possible bug in `dframe-set-timer', which I'll leave for future people familiar with dframe to deal with if necessary. --=-=-= Content-Type: text/x-patch Content-Disposition: attachment; filename=0001-5.6-Prevent-unwanted-recursion-in-erc-nickbar-disabl.patch >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 --=-=-=--