GNU bug report logs - #68114
[PATCH] Make 'advice-remove' interactive

Previous Next

Full log

View this message in rfc822 format

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Steven Allen <steven <at> stebalien.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 68114 <at> debbugs.gnu.org
Subject: bug#68114: [PATCH] Make 'advice-remove' interactive
Date: Sat, 30 Dec 2023 00:06:31 -0500

> --- 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: "
> +                           obarray
> +                           (lambda (sym) (advice--p (advice--symbol-function sym)))
> +                           t nil nil
> +                           (when-let (def (function-called-at-point)) (symbol-name def)))))
> +          advice)

You should include the default in the prompt using `format-prompt`.

> +     (advice-mapc (lambda (f _) (push (cons (prin1-to-string f) f) advice)) symbol)

The var name `advice` suggests it holds a single piece of advice.
I'd call it `advices` instead.

Also, some advice's functions are lambda expressions (i.e. closures)
which can be rather ugly/unwieldy when printed.  When code expects to
remove them, we usually provide a `name` property for them, so I suggest
that you use something like

    (lambda (f p)
       (let ((k (or (alist-get 'name p) f)))
         (push (cons (prin1-to-string k) k) advices)))

> +     (list symbol (cdr (assoc-string (completing-read "Advice: " advice) advice)))))

I suspect you want to `require-match` in the `completing-read` call.


        Stefan

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.