GNU bug report logs - #16959
24.3.50; defadvice docstring out of date

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 16959 in the body.
You can then email your comments to 16959 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Florian Beck <fb <at> miszellen.de>
To: bug-gnu-emacs <at> gnu.org
Subject: 24.3.50; defadvice docstring out of date
Date: Fri, 07 Mar 2014 16:00:08 +0100

The docstring of defadvice says

See Info node `(elisp)Advising Functions' for comprehensive documentation.

This is no longer true. Also, while the old advice mechanism seems to 
have been be obsoleted, the help doesn't say so.

BTW, the nadvice info could use some more examples and a note on how to 
transition from defadvice. Kind of obvious, I know, but only in hindsight.


-- 
Florian Beck

Message #8 received at 16959 <at> debbugs.gnu.org (full text, mbox):

From: Glenn Morris <rgm <at> gnu.org>
To: Florian Beck <fb <at> miszellen.de>
Cc: 16959 <at> debbugs.gnu.org
Subject: Re: bug#16959: 24.3.50; defadvice docstring out of date
Date: Mon, 10 Mar 2014 14:16:14 -0400

Florian Beck wrote:

> The docstring of defadvice says
>
> See Info node `(elisp)Advising Functions' for comprehensive documentation.
>
> This is no longer true.

OK; removed.

> Also, while the old advice mechanism seems to have been be obsoleted,
> the help doesn't say so.

Why do you think it is obsolete?

> BTW, the nadvice info could use some more examples and a note on how
> to transition from defadvice.

I'll leave this report open till someone does that.

Message #11 received at 16959 <at> debbugs.gnu.org (full text, mbox):

From: Florian Beck <fb <at> miszellen.de>
To: Glenn Morris <rgm <at> gnu.org>
Cc: 16959 <at> debbugs.gnu.org
Subject: Re: bug#16959: 24.3.50; defadvice docstring out of date
Date: Tue, 11 Mar 2014 06:37:17 +0100

On 10.03.2014 19:16, Glenn Morris wrote:

> Why do you think it is obsolete?

Because every hint of its existence has been expunged from the 
documentation and the new advice seems to offer the same functionality.

>> BTW, the nadvice info could use some more examples and a note on how
>> to transition from defadvice.
>
> I'll leave this report open till someone does that.

Some specifics:

 - See the documentation of the old advice: it starts with "altering 
the behavior of an existing function" (yeah! that's what I'm here for), 
then gives a simple example, a use case ("Suppose you wanted ...").

 - In contrast, the beginning of the new "Advising Functions" is a bit 
confusing: appropriate setter function? set-process-filter? What are 
"such hooks"?

 - The difference between add-function and advice-add is not entirely 
clear to me: the latter is for the "function cell of a symbol", a "named 
function and macros", "macros and autoloaded functions". Which is it? I 
can write

(add-function :after (symbol-function 'somefun) 'someadvice)

but should I? I have the feeling I should stick with advice-add unless 
dealing with process filters.

 - Indeed, it seems the documentation is in the wrong order: from a 
user's perspective, advice-add is what she usually wants and 
add-function is a special case.

 - The docstring of advice-add is useless: it should give me the info I 
need when writing an advice, which is in add-function. IMO is should be 
the other way around: the fact that you need advice-add for macros, etc 
should be mentioned in the docstring of add-function.

 - The page title "Advising Primitives" is misleading in the context of 
"Advising Functions" and "Advising Named Functions".

 - Interactive functions are only discussed in the docstring of 
add-function.

 - Add some examples that show how arguments work for someone who is 
not familiar with &rest and apply.

 - How about reimplementing the two examples from the old docu ("A 
Simple Advice Example" and "Around Advice")? That would also demonstrate 
how to transition to the new mechanism.


-- 
Florian Beck

Message #14 received at 16959 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Florian Beck <fb <at> miszellen.de>
Cc: Glenn Morris <rgm <at> gnu.org>, 16959 <at> debbugs.gnu.org
Subject: Re: bug#16959: 24.3.50; defadvice docstring out of date
Date: Wed, 12 Mar 2014 09:49:50 -0400

> Some specifics:

Thanks Florian, this list is very helpful.  Just one comment:

>  - Indeed, it seems the documentation is in the wrong order: from a user's
> perspective, advice-add is what she usually wants and add-function
> is a special case.

Actually, add-function is likely to become more common with new hooks
using a `*-function' variable modified via add-function rather than
a `*-functions' modified via add-hook.


        Stefan

Message #17 received at 16959 <at> debbugs.gnu.org (full text, mbox):

From: Josh <josh <at> foxtail.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Florian Beck <fb <at> miszellen.de>, 16959 <at> debbugs.gnu.org
Subject: Re: bug#16959: 24.3.50; defadvice docstring out of date
Date: Wed, 12 Mar 2014 09:30:08 -0700

[Message part 1 (text/plain, inline)]

On Wed, Mar 12, 2014 at 6:49 AM, Stefan Monnier <monnier <at> iro.umontreal.ca>wrote:

> > Some specifics:
>
> Thanks Florian, this list is very helpful.  Just one comment:
>
> >  - Indeed, it seems the documentation is in the wrong order: from a
> user's
> > perspective, advice-add is what she usually wants and add-function
> > is a special case.
>
> Actually, add-function is likely to become more common with new hooks
> using a `*-function' variable modified via add-function rather than
> a `*-functions' modified via add-hook.


Somewhat relatedly, would you consider renaming advice-add to
add-advice (naturally with an alias for backward compatibility)?
It's easier to discover related functionality through tab
completion, apropos, etc. when its names share a common structure.
For example if I was looking for basename functionality and I knew
about file-name-directory, `C-h f file-name- TAB' would lead me to
`file-name-nondirectory' straightaway.

[Message part 2 (text/html, inline)]

Message #20 received at 16959 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Josh <josh <at> foxtail.org>
Cc: Florian Beck <fb <at> miszellen.de>, 16959 <at> debbugs.gnu.org
Subject: Re: bug#16959: 24.3.50; defadvice docstring out of date
Date: Wed, 12 Mar 2014 14:41:52 -0400

> Somewhat relatedly, would you consider renaming advice-add to
> add-advice (naturally with an alias for backward compatibility)?

No, that's not an option.  I could consider getting rid of the
add-function and remove-function exceptions (i.e. renaming them to
something that starts with "advice-" like everything else in
nadvice.el), but then their "natural" name would probably be
advice-function-add and advice-function-remove, neither of which please
me very much.  Especially given the intention to see `add-function' as
a brother to `add-hook'.


        Stefan

Message #25 received at 16959-done <at> debbugs.gnu.org (full text, mbox):

From: Stefan <monnier <at> iro.umontreal.ca>
To: Florian Beck <fb <at> miszellen.de>
Cc: 16959-done <at> debbugs.gnu.org
Subject: Re: bug#16981: 24.3.50;
 electric-pair-delete-adjacent-pairs broken in c-mode,
 python-mode,	maybe-others
Date: Mon, 17 Mar 2014 21:56:25 -0400

>> Why do you think it is obsolete?
> Because every hint of its existence has been expunged from the documentation
> and the new advice seems to offer the same functionality.

Indeed, it's technically obsolete, but it's not declared officially
obsolete yet.  It will come, tho.  But it'll be with us for many years
to come.  OTOH its support is limited to keeping things working.

>>> BTW, the nadvice info could use some more examples and a note on how
>>> to transition from defadvice.
>> I'll leave this report open till someone does that.
> Some specifics:

I've installed some changes to try and address them.


        Stefan

Message #28 received at 16959 <at> debbugs.gnu.org (full text, mbox):

From: Florian Beck <fb <at> miszellen.de>
To: 16959 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca
Subject: Re: bug#16959: bug#16981: 24.3.50; electric-pair-delete-adjacent-pairs
 broken in c-mode, python-mode, maybe-others
Date: Thu, 20 Mar 2014 01:33:00 +0100

> I've installed some changes to try and address them.

Very nice.

Some comments:

Advising Emacs Lisp Functions:

 - The third paragraph could link to "What Is a Function?"
 - also link "in order to trace the calls to the process filter of a
process" to "Filter Functions"
 - in the last paragraph ":above" should be ":around"; the formatting 
"_advice_" is strange
 - mention that in the first example the advising function takes the 
same arguments as the original, and in the second the original function 
as an additional argument (this is the most important info, IMO)

Core Advising Primitives:
 - Should the WHERE specifications have their own node or section? In 
any case, I would move the second example (his-tracing-function) to the 
end of the list and explain it a little bit. Like

"In this case a call to `display-buffer' works like calling

(lambda (BUFFER-OR-NAME &optional ACTION FRAME)
  (apply 'his-tracing-fun 'display-buffer BUFFER-OR-NAME ACTION FRAME))

That is, instead of `display-buffer', the new function `his tracing-fun' 
is called with display-buffer as its first argument."

 - depth: clarify what happens when you have several pieces of 
":override" or ":around" advice that doesn't call the original function 
at the depth 0.

Advising named functions:
 - "Macros can also be advised": this doesn't work when the macro is 
used in a compiled function.
-- 
Florian Beck

Message #31 received at 16959 <at> debbugs.gnu.org (full text, mbox):

From: Stefan <monnier <at> iro.umontreal.ca>
To: Florian Beck <fb <at> miszellen.de>
Cc: 16959 <at> debbugs.gnu.org
Subject: Re: bug#16959: bug#16981: 24.3.50;
 electric-pair-delete-adjacent-pairs broken in c-mode, python-mode,
 maybe-others
Date: Fri, 21 Mar 2014 17:47:55 -0400

>  - also link "in order to trace the calls to the process filter of a
> process" to "Filter Functions"

I find Texinfo links to be a bit heavy, so I prefer not to put too many
of them, so the reader can concentrate on the subject.  Process filters
are not really relevant.

>  - in the last paragraph ":above" should be ":around";

Fixed.

> the formatting "_advice_" is strange

That's how Texinfo renders @emph in Info.

> Core Advising Primitives:
>  - Should the WHERE specifications have their own node or section?

Sizewise, I fully agree, so I moved it to its own node/section.  I'm not
completely happy with the result, but I think it's better.

> In any case, I would move the second example (his-tracing-function) to
> the end of the list and explain it a little bit.

I'd rather keep the advice-add in the first node alongside the
add-function example, since as you say in many cases advice-add is what
the reader is really after.

>  - depth: clarify what happens when you have several pieces of ":override"
> or ":around" advice that doesn't call the original function at the depth 0.

I tried to clarify the notion of depth by spelling out what it means
for :after, :before and :override.

> Advising named functions:
>  - "Macros can also be advised": this doesn't work when the macro is used
> in a compiled function.

Of course it works.  The restriction is that the macro needs to be
advised before it is expanded.  I spelled it out now.


        Stefan

Message #34 received at 16959 <at> debbugs.gnu.org (full text, mbox):

From: Florian Beck <fb <at> miszellen.de>
To: Stefan <monnier <at> iro.umontreal.ca>
Cc: 16959 <at> debbugs.gnu.org
Subject: Re: bug#16959: bug#16981: 24.3.50;
 electric-pair-delete-adjacent-pairs broken in c-mode, python-mode,
 maybe-others
Date: Tue, 25 Mar 2014 18:38:30 +0100

Looks very nice now. Thank you.

>>  - depth: clarify what happens when you have several pieces of ":override"
>> or ":around" advice that doesn't call the original function at the depth 0.
>
> I tried to clarify the notion of depth by spelling out what it means
> for :after, :before and :override.

The depth metaphor actually works pretty well.
The use case I had in mind was this: (1) Someone defines a :override
advice. (2) There already is an :override advice. (3) Neither advice
specifies depth, so it defaults to 0.

The result is that the advice SEEMS to work (the user put it in her
.emacs and presses C-x C-e), but on restarting emacs it depends on the
order advice defintions.

>
>> Advising named functions:
>>  - "Macros can also be advised": this doesn't work when the macro is used
>> in a compiled function.
>
> Of course it works.  The restriction is that the macro needs to be
> advised before it is expanded.  I spelled it out now.

Of course, from a user's point of view, macros usually are already expanded.

How would a user go about advising, say, byte-compile-log in her .emacs?

-- 
Florian Beck

Message #37 received at 16959 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: Florian Beck <fb <at> miszellen.de>
Cc: 16959 <at> debbugs.gnu.org
Subject: Re: bug#16959: bug#16981: 24.3.50;
 electric-pair-delete-adjacent-pairs broken in c-mode, python-mode,
 maybe-others
Date: Tue, 25 Mar 2014 20:34:36 -0400

> The result is that the advice SEEMS to work (the user put it in her
> .emacs and presses C-x C-e), but on restarting emacs it depends on the
> order advice defintions.

That's why the doc says:

   [..]
   should stay as the outermost advice.  When two advices specify the
   same depth, the most recently added advice will be outermost.
   [..]

> Of course, from a user's point of view, macros usually are already expanded.
> How would a user go about advising, say, byte-compile-log in her .emacs?

She can't do it and hope for it to affect already compiled code, indeed.

But we use advice-add in cl.el on the `declare', `dolist', and `dotimes'
macros and it works very well.  Obviously code that uses those needs to
have a (require 'cl) somewhere to make sure the advice is installed
before macro-expansion.


        Stefan

Previous Next

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