bug#68294: [PATCH] Set the 'name' prop in 'define-advice'

bug#68294: [PATCH] Set the 'name' prop in 'define-advice'

[Steven Allen]

[-- Attachment #1: Type: text/plain, Size: 193 bytes --]


In addition to naming the advice function 'symbol@name', set the 'name'
property to NAME. The code should be good, but I'm less sure about the
updated documentation.

Requested in Bug#68114.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 3407 bytes --]

From b76e46a2a6e80dcec8845fb216e126068e2c4ac3 Mon Sep 17 00:00:00 2001
From: Steven Allen <steven@stebalien.com>
Date: Sat, 6 Jan 2024 09:19:12 -0800
Subject: [PATCH] Set the 'name' prop in 'define-advice'

In addition to naming the advice function `symbol@name', set the 'name'
property to NAME.

* lisp/emacs-lisp/nadvice.el (define-advice): set the name property to
name (requested in Bug#68114).

* doc/lispref/functions.texi (Advising Named Functions): Document that
'define-advice' installs the advice with the specified name.
---
 doc/lispref/functions.texi | 7 ++++---
 etc/NEWS                   | 6 ++++++
 lisp/emacs-lisp/nadvice.el | 8 +++++---
 3 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/doc/lispref/functions.texi b/doc/lispref/functions.texi
index 29e9f04a076..48f32ec907d 100644
--- a/doc/lispref/functions.texi
+++ b/doc/lispref/functions.texi
@@ -2066,9 +2066,10 @@ Advising Named Functions
 
 @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.
+@var{symbol}.  If @var{name} is non-nil, the advice is named
+@code{symbol@@name} and installed with the name @var{name}; otherwise,
+the advice is anonymous.  See @code{advice-add} for explanation of
+other arguments.
 @end defmac
 
 @defun advice-add symbol where function &optional props
diff --git a/etc/NEWS b/etc/NEWS
index 3a1168f62b3..a26b270d44f 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1393,6 +1393,12 @@ values.
 \f
 * Lisp Changes in Emacs 30.1
 
++++
+** 'define-advice' now sets the new advice's 'name' property to NAME
+Named advice defined with 'define-advice' can now be remove with
+'(advice-remove SYMBOL NAME)' in addition to '(advice-remove SYMBOL
+SYMBOL@NAME)'.
+
 +++
 ** New function 'require-with-check' to detect new versions shadowing.
 This is like 'require', but it checks whether the argument 'feature'
diff --git a/lisp/emacs-lisp/nadvice.el b/lisp/emacs-lisp/nadvice.el
index de287e43b21..7524ab18e58 100644
--- a/lisp/emacs-lisp/nadvice.el
+++ b/lisp/emacs-lisp/nadvice.el
@@ -585,8 +585,8 @@ advice-remove
 (defmacro define-advice (symbol args &rest body)
   "Define an advice and add it to function named SYMBOL.
 See `advice-add' and `add-function' for explanation on the
-arguments.  Note if NAME is nil the advice is anonymous;
-otherwise it is named `SYMBOL@NAME'.
+arguments.  If NAME is non-nil, the advice is named `SYMBOL@NAME'
+and installed with the name NAME; otherwise, the advice is anonymous.
 
 \(fn SYMBOL (HOW LAMBDA-LIST &optional NAME DEPTH) &rest BODY)"
   (declare (indent 2) (doc-string 3) (debug (sexp sexp def-body)))
@@ -597,7 +597,9 @@ define-advice
          (lambda-list   (nth 1 args))
          (name          (nth 2 args))
          (depth         (nth 3 args))
-         (props         (and depth `((depth . ,depth))))
+         (props         (append
+                         (and depth `((depth . ,depth)))
+                         (and name `((name . ,name)))))
          (advice (cond ((null name) `(lambda ,lambda-list ,@body))
                        ((or (stringp name) (symbolp name))
                         (intern (format "%s@%s" symbol name)))
-- 
2.43.0

bug#68294: [PATCH] Set the 'name' prop in 'define-advice'

[Eli Zaretskii]

> From: Steven Allen <steven@stebalien.com>
> Date: Sat, 06 Jan 2024 09:54:36 -0800
> 
> In addition to naming the advice function 'symbol@name', set the 'name'
> property to NAME. The code should be good, but I'm less sure about the
> updated documentation.
> 
> Requested in Bug#68114.

Stefan, any comments?

>  @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.
> +@var{symbol}.  If @var{name} is non-nil, the advice is named
> +@code{symbol@@name} and installed with the name @var{name}; otherwise,
   ^^^^^^^^^^^^^^^^^^^
This should be @code{@var{symbol}@@@var{name}}.

> +** 'define-advice' now sets the new advice's 'name' property to NAME
> +Named advice defined with 'define-advice' can now be remove with
                                                        ^^^^^^
This should be "removed".

bug#68294: [PATCH] Set the 'name' prop in 'define-advice'

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

>> In addition to naming the advice function 'symbol@name', set the 'name'
>> property to NAME. The code should be good, but I'm less sure about the
>> updated documentation.
>> Requested in Bug#68114.
>
> Stefan, any comments?

Sounds fine to me.


        Stefan

bug#68294: [PATCH] Set the 'name' prop in 'define-advice'

[Steven Allen]

[-- Attachment #1: Type: text/plain, Size: 820 bytes --]


>>  @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.
>> +@var{symbol}.  If @var{name} is non-nil, the advice is named
>> +@code{symbol@@name} and installed with the name @var{name}; otherwise,
>    ^^^^^^^^^^^^^^^^^^^
> This should be @code{@var{symbol}@@@var{name}}.

Fixed.

>> +** 'define-advice' now sets the new advice's 'name' property to NAME
>> +Named advice defined with 'define-advice' can now be remove with
>                                                         ^^^^^^
> This should be "removed".

Fixed.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 3437 bytes --]

From 305f74ea832998f10555425225a1d35f137fc489 Mon Sep 17 00:00:00 2001
From: Steven Allen <steven@stebalien.com>
Date: Sat, 6 Jan 2024 09:19:12 -0800
Subject: [PATCH] Set the 'name' prop in 'define-advice'

In addition to naming the advice function `symbol@name', set the 'name'
property to NAME.

* lisp/emacs-lisp/nadvice.el (define-advice): set the name property to
name (requested in Bug#68114). Fixes Bug#68294.

* doc/lispref/functions.texi (Advising Named Functions): Document that
'define-advice' installs the advice with the specified name.
---
 doc/lispref/functions.texi | 7 ++++---
 etc/NEWS                   | 6 ++++++
 lisp/emacs-lisp/nadvice.el | 8 +++++---
 3 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/doc/lispref/functions.texi b/doc/lispref/functions.texi
index 29e9f04a076..29061e6561c 100644
--- a/doc/lispref/functions.texi
+++ b/doc/lispref/functions.texi
@@ -2066,9 +2066,10 @@ Advising Named Functions
 
 @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.
+@var{symbol}.  If @var{name} is non-nil, the advice is named
+@code{@var{symbol}@@@var{name}} and installed with the name @var{name}; otherwise,
+the advice is anonymous.  See @code{advice-add} for explanation of
+other arguments.
 @end defmac
 
 @defun advice-add symbol where function &optional props
diff --git a/etc/NEWS b/etc/NEWS
index 3a1168f62b3..21b1aaee71a 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1393,6 +1393,12 @@ values.
 \f
 * Lisp Changes in Emacs 30.1
 
++++
+** 'define-advice' now sets the new advice's 'name' property to NAME
+Named advice defined with 'define-advice' can now be removed with
+'(advice-remove SYMBOL NAME)' in addition to '(advice-remove SYMBOL
+SYMBOL@NAME)'.
+
 +++
 ** New function 'require-with-check' to detect new versions shadowing.
 This is like 'require', but it checks whether the argument 'feature'
diff --git a/lisp/emacs-lisp/nadvice.el b/lisp/emacs-lisp/nadvice.el
index de287e43b21..7524ab18e58 100644
--- a/lisp/emacs-lisp/nadvice.el
+++ b/lisp/emacs-lisp/nadvice.el
@@ -585,8 +585,8 @@ advice-remove
 (defmacro define-advice (symbol args &rest body)
   "Define an advice and add it to function named SYMBOL.
 See `advice-add' and `add-function' for explanation on the
-arguments.  Note if NAME is nil the advice is anonymous;
-otherwise it is named `SYMBOL@NAME'.
+arguments.  If NAME is non-nil, the advice is named `SYMBOL@NAME'
+and installed with the name NAME; otherwise, the advice is anonymous.
 
 \(fn SYMBOL (HOW LAMBDA-LIST &optional NAME DEPTH) &rest BODY)"
   (declare (indent 2) (doc-string 3) (debug (sexp sexp def-body)))
@@ -597,7 +597,9 @@ define-advice
          (lambda-list   (nth 1 args))
          (name          (nth 2 args))
          (depth         (nth 3 args))
-         (props         (and depth `((depth . ,depth))))
+         (props         (append
+                         (and depth `((depth . ,depth)))
+                         (and name `((name . ,name)))))
          (advice (cond ((null name) `(lambda ,lambda-list ,@body))
                        ((or (stringp name) (symbolp name))
                         (intern (format "%s@%s" symbol name)))
-- 
2.43.0

bug#68294: [PATCH] Set the 'name' prop in 'define-advice'

[Eli Zaretskii]

> From: Steven Allen <steven@stebalien.com>
> Cc: 68294@debbugs.gnu.org
> Date: Mon, 08 Jan 2024 07:42:29 -0800
> 
> >>  @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.
> >> +@var{symbol}.  If @var{name} is non-nil, the advice is named
> >> +@code{symbol@@name} and installed with the name @var{name}; otherwise,
> >    ^^^^^^^^^^^^^^^^^^^
> > This should be @code{@var{symbol}@@@var{name}}.
> 
> Fixed.
> 
> >> +** 'define-advice' now sets the new advice's 'name' property to NAME
> >> +Named advice defined with 'define-advice' can now be remove with
> >                                                         ^^^^^^
> > This should be "removed".
> 
> Fixed.

Thanks, installed on master, and closing the bug.