From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.bugs Subject: bug#68114: [PATCH] Make 'advice-remove' interactive Date: Sat, 30 Dec 2023 08:45:11 +0200 Message-ID: <837ckw5gug.fsf@gnu.org> References: <87r0j42353.fsf@stebalien.com> <83a5ps6a5e.fsf@gnu.org> <87r0j491mx.fsf@stebalien.com> Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="28361"; mail-complaints-to="usenet@ciao.gmane.io" Cc: monnier@iro.umontreal.ca, 68114@debbugs.gnu.org To: Steven Allen Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Sat Dec 30 07:46: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 1rJT6u-0007CG-CP for geb-bug-gnu-emacs@m.gmane-mx.org; Sat, 30 Dec 2023 07:46:16 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1rJT6h-0007Y9-UL; Sat, 30 Dec 2023 01:46:03 -0500 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 1rJT6g-0007Xv-4q for bug-gnu-emacs@gnu.org; Sat, 30 Dec 2023 01:46:02 -0500 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 1rJT6f-000073-Tf for bug-gnu-emacs@gnu.org; Sat, 30 Dec 2023 01:46:01 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1rJT6g-0001pO-8f for bug-gnu-emacs@gnu.org; Sat, 30 Dec 2023 01:46:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Eli Zaretskii Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sat, 30 Dec 2023 06:46:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 68114 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 68114-submit@debbugs.gnu.org id=B68114.17039187313834 (code B ref 68114); Sat, 30 Dec 2023 06:46:02 +0000 Original-Received: (at 68114) by debbugs.gnu.org; 30 Dec 2023 06:45:31 +0000 Original-Received: from localhost ([127.0.0.1]:42892 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1rJT6A-0000zB-9d for submit@debbugs.gnu.org; Sat, 30 Dec 2023 01:45:30 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:60354) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1rJT68-0000lG-1w for 68114@debbugs.gnu.org; Sat, 30 Dec 2023 01:45:28 -0500 Original-Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1rJT62-0008Sm-9m; Sat, 30 Dec 2023 01:45:22 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=References:Subject:In-Reply-To:To:From:Date: mime-version; bh=eXnLyI0rA+UinV+/nR6NvIad36u41Oaa5guAQt6nAik=; b=cWbBxjh/FFne sVfd20A41A4ZHvfVz4zCKubf6OXQO791xIRGzZNkGCMIRhFEuiwoCYj1S6L6RDWonrg4OMmeTKo28 C4nQSrQ0RIuAAiTXa9H5LWsTT+IWkafOaHP21DNZYnHLfIQ8BCMN7ITp5ah/h4ZjsHfa56Trm0NYU UeYtX5xTBHNYDKnNAI/VPbgtf8aoapFCf4lZdWi9tVf0RfrUNDTw+aqblIZjrH46tWb4cpp8XJOs8 t2bqDqEm6mQj+5Q/WDrt9rX+Ajhm/VZzVDo4ZnrBaZGyzswS7T0HD7s9uedy07AiJ88YNqyfDR2RR MaMh6SJ8tYfUmGw7/PWPtw==; In-Reply-To: <87r0j491mx.fsf@stebalien.com> (message from Steven Allen on Fri, 29 Dec 2023 12:47:50 -0800) 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:277057 Archived-At: > From: Steven Allen > Cc: 68114@debbugs.gnu.org > Date: Fri, 29 Dec 2023 12:47:50 -0800 > > Thanks for the quick feedback. I've attached an updated patch with a > NEWS and update and some documentation. Let me know if there's a better > place to document this of if I made a mistake somewhere. Thanks, a few minor comments while we wait for Stefan to chime in. > `ad-advice-remove' is already interactive, but it doesn't work with > new-style advice. > > * lisp/emacs-lisp/nadvice.el (advice-remove): Make it interactive. > * doc/lispref/functions.texi (Advising Named Functions): Document that > 'advice-remove' is now an interactive command. Please mention the bug number, since it is now known, in the log message. > -@defun advice-remove symbol function > +@deffn Command advice-remove symbol function > Remove the advice @var{function} from the named function @var{symbol}. > -@var{function} can also be the @code{name} of a piece of advice. > -@end defun > +@var{function} can also be the @code{name} of a piece of advice. When ^^ Two spaces between sentences, please. Btw, what is "the 'name' of a piece of advice"? I realize that this text was there to begin with, but I don't think I understand what it wants to tell me, so maybe we could clarify that. The only reference to a "name" in the preceding text uses "name" to mean a symbol, but then what is "the name of a piece of advice"? I guess this goes back to define-advice, which says: @defmac define-advice symbol (where lambda-list &optional name depth) &rest body This macro defines a piece of advice and adds it to the function named @var{symbol}. The advice is an anonymous function if @var{name} is @code{nil} or a function named @code{symbol@@name}. See @code{advice-add} for explanation of other arguments. which is also a bit mysterious. Does NAME used here serve as "the name of the piece of advice"? if so, should "@code{symbol@@name}" be "@code{symbol@@@var{name}}" instead, i.e. "name" is not a literal string but the reference to NAME? > --- a/lisp/emacs-lisp/nadvice.el > +++ b/lisp/emacs-lisp/nadvice.el > @@ -539,6 +539,16 @@ advice-remove > or an autoload and it preserves `fboundp'. > Instead of the actual function to remove, FUNCTION can also be the `name' > of the piece of advice." > + (interactive > + (let ((symbol (intern (completing-read > + "Advised Function: " I wonder whether "Remove advice from function: " would be a better prompt. > + obarray > + (lambda (sym) (advice--p (advice--symbol-function sym))) > + t nil nil > + (when-let (def (function-called-at-point)) (symbol-name def))))) > + advice) > + (advice-mapc (lambda (f _) (push (cons (prin1-to-string f) f) advice)) symbol) > + (list symbol (cdr (assoc-string (completing-read "Advice: " advice) advice))))) And here I wonder whether "Advice to remove: " would be a better prompt.