From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.ciao.gmane.io!not-for-mail From: =?UTF-8?Q?=C5=A0t=C4=9Bp=C3=A1n_?= =?UTF-8?Q?N=C4=9Bmec?= Newsgroups: gmane.emacs.bugs Subject: bug#5293: 23.1; unload-feature on buffer-local hooks Date: Mon, 06 Apr 2020 19:24:40 +0200 Message-ID: <87zhbojzmv.fsf@gmail.com> References: <87hbr4p67t.fsf@blah.blah> <871uxsl778.fsf@blah.blah> <87aabnnxna.fsf@blah.blah> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Injection-Info: ciao.gmane.io; posting-host="ciao.gmane.io:159.69.161.202"; logging-data="33099"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux) Cc: Juanma Barranquero , Stefan Monnier , 5293@debbugs.gnu.org To: Kevin Ryde Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Mon Apr 06 19:25:14 2020 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 1jLVUm-0008RA-1m for geb-bug-gnu-emacs@m.gmane-mx.org; Mon, 06 Apr 2020 19:25:12 +0200 Original-Received: from localhost ([::1]:35574 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jLVUl-0008Q0-40 for geb-bug-gnu-emacs@m.gmane-mx.org; Mon, 06 Apr 2020 13:25:11 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:59373) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1jLVUd-0008Pj-TI for bug-gnu-emacs@gnu.org; Mon, 06 Apr 2020 13:25:05 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1jLVUc-00064u-Bg for bug-gnu-emacs@gnu.org; Mon, 06 Apr 2020 13:25:03 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:37618) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1jLVUc-00064k-50 for bug-gnu-emacs@gnu.org; Mon, 06 Apr 2020 13:25:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1jLVUc-0002AC-0H for bug-gnu-emacs@gnu.org; Mon, 06 Apr 2020 13:25:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: =?UTF-8?Q?=C5=A0t=C4=9Bp=C3=A1n_?= =?UTF-8?Q?N=C4=9Bmec?= Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Mon, 06 Apr 2020 17:25:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 5293 X-GNU-PR-Package: emacs Original-Received: via spool by 5293-submit@debbugs.gnu.org id=B5293.15861938638198 (code B ref 5293); Mon, 06 Apr 2020 17:25:01 +0000 Original-Received: (at 5293) by debbugs.gnu.org; 6 Apr 2020 17:24:23 +0000 Original-Received: from localhost ([127.0.0.1]:49164 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1jLVTz-000287-6M for submit@debbugs.gnu.org; Mon, 06 Apr 2020 13:24:23 -0400 Original-Received: from mail-wr1-f43.google.com ([209.85.221.43]:34304) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1jLVTx-00027e-25 for 5293@debbugs.gnu.org; Mon, 06 Apr 2020 13:24:21 -0400 Original-Received: by mail-wr1-f43.google.com with SMTP id 65so452597wrl.1 for <5293@debbugs.gnu.org>; Mon, 06 Apr 2020 10:24:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:in-reply-to:references:user-agent:date :message-id:mime-version; bh=XDcIPMcHHW2xL8wDI1ZR5PWLrAN9Z653lOLqJfToGxE=; b=n6euSnCC6DGJPUgHWqK9TqaIscYUsiDjaWyw2CQGzQGL25lxMCAQBlRtV6i/Ei0jzl pvfbQVAJE2vTycsaEx/eCDqtBgnotbWwzIXDFRjC1Y9xmFl1dW+28IHahoA2p+Qd3BqW +MF6glK55OAD1PIhnxiLl+NqFTZCTcugNhH7mSTrx1jRKbntqVGxVf0f7YzAk/lqlI9j Rog9+R+OYptEuZ6uj0E+mDi7NT+Qt50svkUyvQBLQJctUoJPLZsCIRejfixfPXRzDSf4 tqa0m0yNB+GgMbGTxxi8QF2e9agmYcIg53VSQqPcQRjaZur8vcKKDksZM/8UivRJzqCp T4yA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:in-reply-to:references :user-agent:date:message-id:mime-version; bh=XDcIPMcHHW2xL8wDI1ZR5PWLrAN9Z653lOLqJfToGxE=; b=e8zJRI+LHReU5JqmLX/FBkvFz3oBjK2kM0ZYtDYPvGQnHsOT0FUF+AcuIWVfs4CWcA gNyaJ12hPKpmMCQyUsRK3KnU7hdw6fbbAP6wuNeQKSdJwpninZfB8691SG5eU3zzIJ6E oFR1xPiZahcINE+mvTrml6lS0c6eVo0MHmefchdguq/J0r0pyRv1+CLqZnx6VFVPxeT4 XnRx2E5U3MHBNt28iw1IOb42HTm0edXxcgeLHBuUoJrV798Zp0Ehs9Vjj1gkxYbvZQHH CvUXXye6h/3rL8FzU+X9deL7JpjF5292KhE8kD3yqDTKf5rWHj0mrbgjZQIIcxp2Yfb5 auvw== X-Gm-Message-State: AGi0PuZ9/ZAzN3nThdrEGEKvx7CW3sF5dfzc041TFyQOkgofVrm+9s0U tYIixJSPVEkc0A6Rbwkkyn0= X-Google-Smtp-Source: APiQypKJF45fIcgy/X8fUsrxH/M1e3/uhIvGT+AXvLGJ7nt3H/h9UbUv9fHW5TLDH/QRQWWI4lxgKg== X-Received: by 2002:adf:ea82:: with SMTP id s2mr216061wrm.407.1586193854851; Mon, 06 Apr 2020 10:24:14 -0700 (PDT) Original-Received: from localhost ([185.112.167.47]) by smtp.gmail.com with ESMTPSA id z3sm294985wma.22.2020.04.06.10.24.13 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 06 Apr 2020 10:24:13 -0700 (PDT) In-Reply-To: <87aabnnxna.fsf@blah.blah> (Kevin Ryde's message of "Sat, 06 Aug 2011 11:20:41 +1000") X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.51.188.43 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" Xref: news.gmane.io gmane.emacs.bugs:178094 Archived-At: --=-=-= Content-Type: text/plain On Sat, 06 Aug 2011 11:20:41 +1000 Kevin Ryde wrote: > reopen 5293 > thanks > > Stefan Monnier writes: >> >>> No, just the hooks. If it makes sense to remove unloaded funcs from >>> hook global values then surely the same rationale applies to remove them >>> from the buffer-local values too. >> >> Agreed. Someone would have to try it out to see if it can be >> done efficiently. ^^^^^^^^^^^ :-D > Reopened for that, or failing that then for clarifying the docstring. > > I imagine there's not normally many hooks or many buffers, or many > buffer-local variables, whichever one of those was the efficient way to > scan ... and unload-feature isn't used very much anyway. Coming back to this after 10 years, it appears that we (I, certainly) underestimated the computation involved. While the attached patch(es) work for emacs -Q toy examples like Kevin's or more recently the one from bug#34686, when I tried M-x load-library allout RET M-x unload-feature allout RET in my normal Emacs session with ~300 buffers and ~1000 features, it took my venerable laptop 8 minutes to complete. Based on that experience, unless someone has better ideas, I suggest we close this and clarify the (henceforth intentional) lack of attention to buffer-local hook values in the documentation instead. Actually, I wonder if ignoring even the global hooks (as opined by Juanma) and enforcing more widespread usage of FEATURE-unload-function wouldn't be better; or/also, couldn't stray undefined functions on hooks be handled similarly to how it's done for errors e.g. in `post-command-hook', i.e. auto-removed when encountered? I guess wrapping all hooks like that would be overkill? (That said, I think [1/3] and [3/3] could/should be applied nonetheless.) --=-=-= Content-Type: text/x-patch Content-Disposition: inline; filename=0001-unload-feature-Improve-logic-don-t-repeat-computatio.patch >From 394f2bf6821196a4171fd79fc9a10dc626619a58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0t=C4=9Bp=C3=A1n=20N=C4=9Bmec?= Date: Mon, 6 Apr 2020 13:25:41 +0200 Subject: [PATCH 1/3] unload-feature: Improve logic (don't repeat computation) * lisp/loadhist.el (unload-feature): Don't do the same computation twice. --- lisp/loadhist.el | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/lisp/loadhist.el b/lisp/loadhist.el index a1ff2f6270..60da00cceb 100644 --- a/lisp/loadhist.el +++ b/lisp/loadhist.el @@ -287,22 +287,23 @@ unload-feature ;; functions which the package might just have installed, and ;; there might be other important state, but this tactic ;; normally works. - (mapatoms - (lambda (x) - (when (and (boundp x) - (or (and (consp (symbol-value x)) ; Random hooks. - (string-match "-hooks?\\'" (symbol-name x))) - (memq x unload-feature-special-hooks))) ; Known abnormal hooks etc. - (dolist (y unload-function-defs-list) - (when (and (eq (car-safe y) 'defun) - (not (get (cdr y) 'autoload))) - (remove-hook x (cdr y))))))) - ;; Remove any feature-symbols from auto-mode-alist as well. - (dolist (y unload-function-defs-list) - (when (and (eq (car-safe y) 'defun) - (not (get (cdr y) 'autoload))) - (setq auto-mode-alist - (rassq-delete-all (cdr y) auto-mode-alist))))) + (let ((removables (cl-loop for def in unload-function-defs-list + when (and (eq (car-safe def) 'defun) + (not (get (cdr def) 'autoload))) + collect (cdr def)))) + (mapatoms + (lambda (x) + (when (and (boundp x) + (or (and (consp (symbol-value x)) ; Random hooks. + (string-match "-hooks?\\'" (symbol-name x))) + ;; Known abnormal hooks etc. + (memq x unload-feature-special-hooks))) + (dolist (func removables) + (remove-hook x func))))) + ;; Remove any feature-symbols from auto-mode-alist as well. + (dolist (func removables) + (setq auto-mode-alist + (rassq-delete-all func auto-mode-alist))))) ;; Change major mode in all buffers using one defined in the feature being unloaded. (unload--set-major-mode) -- 2.26.0 --=-=-= Content-Type: text/x-patch Content-Disposition: inline; filename=0002-unload-feature-Handle-local-hooks.patch >From d2372157c031b79cad4e3bf331aa6e167ff48199 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0t=C4=9Bp=C3=A1n=20N=C4=9Bmec?= Date: Mon, 6 Apr 2020 13:30:11 +0200 Subject: [PATCH 2/3] unload-feature: Handle local hooks Buffer-local hooks were introduced in 1994-09-30T20:47:13+00:00!rms@gnu.org 0e4d378b32 (add-hook): Initialize default value and local value. but `unload-feature' has not been updated to handle them. * lisp/loadhist.el (unload-feature): Handle local hooks. --- etc/NEWS | 3 +++ lisp/loadhist.el | 6 +++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/etc/NEWS b/etc/NEWS index 765a923bf7..fa90edc4d1 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -297,6 +297,9 @@ optional argument specifying whether to follow symbolic links. ** 'parse-time-string' can now parse ISO 8601 format strings, such as "2020-01-15T16:12:21-08:00". +--- +** 'unload-feature' now also tries to undo additions to buffer-local hooks. + * Changes in Emacs 28.1 on Non-Free Operating Systems diff --git a/lisp/loadhist.el b/lisp/loadhist.el index 60da00cceb..2fd2d3f6be 100644 --- a/lisp/loadhist.el +++ b/lisp/loadhist.el @@ -299,7 +299,11 @@ unload-feature ;; Known abnormal hooks etc. (memq x unload-feature-special-hooks))) (dolist (func removables) - (remove-hook x func))))) + (remove-hook x func) + (save-current-buffer + (dolist (buffer (buffer-list)) + (set-buffer buffer) + (remove-hook x func t))))))) ;; Remove any feature-symbols from auto-mode-alist as well. (dolist (func removables) (setq auto-mode-alist -- 2.26.0 --=-=-= Content-Type: text/x-patch Content-Disposition: inline; filename=0003-unload-feature-Correct-doc-string-to-match-info-manu.patch >From a418282b70e514065159a217a4106226395aa98e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=A0t=C4=9Bp=C3=A1n=20N=C4=9Bmec?= Date: Mon, 6 Apr 2020 17:05:33 +0200 Subject: [PATCH 3/3] unload-feature: Correct doc string to match info manual and reality 'unload-feature' doesn't try to "undo any additions the library has made" to hooks, it tries to remove functions defined by the library from hooks, no matter how they got there. * lisp/loadhist.el (unload-feature): Correct the doc string. * doc/lispref/loading.texi (Unloading): Clarify, fix typo. --- doc/lispref/loading.texi | 4 ++-- lisp/loadhist.el | 9 ++++----- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/doc/lispref/loading.texi b/doc/lispref/loading.texi index 2894282079..f1ee638357 100644 --- a/doc/lispref/loading.texi +++ b/doc/lispref/loading.texi @@ -1063,7 +1063,7 @@ Unloading (Loading saves these in the @code{autoload} property of the symbol.) Before restoring the previous definitions, @code{unload-feature} runs -@code{remove-hook} to remove functions in the library from certain +@code{remove-hook} to remove functions defined by the library from certain hooks. These hooks include variables whose names end in @samp{-hook} (or the deprecated suffix @samp{-hooks}), plus those listed in @code{unload-feature-special-hooks}, as well as @@ -1071,7 +1071,7 @@ Unloading function because important hooks refer to functions that are no longer defined. -Standard unloading activities also undoes ELP profiling of functions +Standard unloading activities also undo ELP profiling of functions in that library, unprovides any features provided by the library, and cancels timers held in variables defined by the library. diff --git a/lisp/loadhist.el b/lisp/loadhist.el index 2fd2d3f6be..9718a4840d 100644 --- a/lisp/loadhist.el +++ b/lisp/loadhist.el @@ -234,11 +234,10 @@ unload-feature is nil, raise an error. Standard unloading activities include restoring old autoloads for -functions defined by the library, undoing any additions that the -library has made to hook variables or to `auto-mode-alist', undoing -ELP profiling of functions in that library, unproviding any features -provided by the library, and canceling timers held in variables -defined by the library. +functions defined by the library, removing such functions from +hooks and `auto-mode-alist', undoing their ELP profiling, +unproviding any features provided by the library, and canceling +timers held in variables defined by the library. If a function `FEATURE-unload-function' is defined, this function calls it with no arguments, before doing anything else. That function -- 2.26.0 --=-=-=--