GNU bug report logs - #76528
30; Advising functions can cause docstring to be lost

Previous Next

To reply to this bug, email your comments to 76528 AT debbugs.gnu.org.

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

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

From: Jonas Bernoulli <jonas <at> bernoul.li>
To: bug-gnu-emacs <at> gnu.org
Subject: 30; Advising functions can cause docstring to be lost
Date: Mon, 24 Feb 2025 19:25:36 +0100

Hello,

Depending on where the docstring is stored, advising a function can
cause its original docstring to be lost.  This also affects 29.4 at
least.

(defun embeded-docstr () "embeded docstr" nil)

(defun property-docstr () nil)
(put 'property-docstr 'function-documentation "property docstr")

(defalias 'alias-docstr 'identity "alias docstr")

(defun maybe-loose-docstring (symbol)
  (let ((a (documentation symbol))
        (b nil)
        (advice (lambda (fn &rest args) (apply fn args))))
    (advice-add symbol :around advice)
    (setq b (documentation symbol))
    (advice-remove symbol advice)
    `((,a) (,b) (,(documentation symbol)))))

(maybe-loose-docstring 'embeded-docstr)
=> (("embeded docstr\n\n(fn)")
    ("embeded docstr\n\nThis function has :around advice: No documentation\n\n(fn)")
    ("embeded docstr\n\n(fn)"))

(maybe-loose-docstring 'property-docstr)
=> (("property docstr")
    ("\nThis function has :around advice: No documentation\n\n(fn)")
    ("\n\n(fn)"))

(maybe-loose-docstring 'alias-docstr)
=> (("alias docstr")
    ("Return the ARGUMENT unchanged.\n\nThis function has :around advice: No documentation\n\n(fn ARGUMENT)")
    ("Return the ARGUMENT unchanged.\n\n(fn ARGUMENT)"))

Best regards,
Jonas

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jonas Bernoulli <jonas <at> bernoul.li>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 76528 <at> debbugs.gnu.org
Subject: Re: bug#76528: 30; Advising functions can cause docstring to be lost
Date: Mon, 24 Feb 2025 21:20:10 +0200

> Date: Mon, 24 Feb 2025 19:25:36 +0100
> From:  Jonas Bernoulli via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
> 
> Hello,
> 
> Depending on where the docstring is stored, advising a function can
> cause its original docstring to be lost.  This also affects 29.4 at
> least.
> 
> (defun embeded-docstr () "embeded docstr" nil)
> 
> (defun property-docstr () nil)
> (put 'property-docstr 'function-documentation "property docstr")
> 
> (defalias 'alias-docstr 'identity "alias docstr")
> 
> (defun maybe-loose-docstring (symbol)
>   (let ((a (documentation symbol))
>         (b nil)
>         (advice (lambda (fn &rest args) (apply fn args))))
>     (advice-add symbol :around advice)
>     (setq b (documentation symbol))
>     (advice-remove symbol advice)
>     `((,a) (,b) (,(documentation symbol)))))
> 
> (maybe-loose-docstring 'embeded-docstr)
> => (("embeded docstr\n\n(fn)")
>     ("embeded docstr\n\nThis function has :around advice: No documentation\n\n(fn)")
>     ("embeded docstr\n\n(fn)"))
> 
> (maybe-loose-docstring 'property-docstr)
> => (("property docstr")
>     ("\nThis function has :around advice: No documentation\n\n(fn)")
>     ("\n\n(fn)"))
> 
> (maybe-loose-docstring 'alias-docstr)
> => (("alias docstr")
>     ("Return the ARGUMENT unchanged.\n\nThis function has :around advice: No documentation\n\n(fn ARGUMENT)")
>     ("Return the ARGUMENT unchanged.\n\n(fn ARGUMENT)"))

Stefan, any comments?

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Jonas Bernoulli <jonas <at> bernoul.li>, 76528 <at> debbugs.gnu.org
Subject: Re: bug#76528: 30; Advising functions can cause docstring to be lost
Date: Mon, 24 Feb 2025 14:48:09 -0500

> Stefan, any comments?

Just that it's directly related to this part of the code in
`nadvice.el`:

    ;; FIXME: We could use a defmethod on `function-documentation' instead,
    ;; except when (autoloadp nf)!
    (put symbol 'function-documentation `(advice--make-docstring ',symbol))


- Stefan

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

From: Jonas Bernoulli <jonas <at> bernoul.li>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 76528 <at> debbugs.gnu.org
Subject: Re: bug#76528: 30; Advising functions can cause docstring to be lost
Date: Mon, 24 Feb 2025 21:42:20 +0100

Stefan Monnier <monnier <at> iro.umontreal.ca> writes:

>> Stefan, any comments?
>
> Just that it's directly related to this part of the code in
> `nadvice.el`:
>
>     ;; FIXME: We could use a defmethod on `function-documentation' instead,
>     ;; except when (autoloadp nf)!
>     (put symbol 'function-documentation `(advice--make-docstring ',symbol))

Transient (transient--wrap-command) has to advise every command that is
invoked while the menu is active, so this has the potential to cause a
lot of damage.  How should I deal with that?  Maybe essentially?:

  (let ((docstr (documentation this-command))
        (advice (lambda (fn &rest args) (apply fn args))))
    (advice-add this-command :around advice)
    (advice-remove this-command advice)
    (unless (equal (documentation this-command) docstr)
      (put this-command 'function-documentation docstr)))

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Jonas Bernoulli <jonas <at> bernoul.li>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 76528 <at> debbugs.gnu.org
Subject: Re: bug#76528: 30; Advising functions can cause docstring to be lost
Date: Tue, 25 Feb 2025 15:38:09 -0500

>>> Stefan, any comments?
>>
>> Just that it's directly related to this part of the code in
>> `nadvice.el`:
>>
>>     ;; FIXME: We could use a defmethod on `function-documentation' instead,
>>     ;; except when (autoloadp nf)!
>>     (put symbol 'function-documentation `(advice--make-docstring ',symbol))
>
> Transient (transient--wrap-command) has to advise every command that is
> invoked while the menu is active, so this has the potential to cause a
> lot of damage.

Indeed.  🙁
I guess this hasn't been noticed until now because
`function-documentation` is not used very often.

> How should I deal with that?  Maybe essentially?:

Well, we should fix it in `nadvice.el`, of course.  But for
compatibility with older Emacsen...

>   (let ((docstr (documentation this-command))
>         (advice (lambda (fn &rest args) (apply fn args))))
>     (advice-add this-command :around advice)
      [...]
>     (advice-remove this-command advice)
>     (unless (equal (documentation this-command) docstr)
>       (put this-command 'function-documentation docstr)))

That sounds about right, tho you'd have to replace the `(documentation
this-command)` with `(get this-command 'function-documentation)`.

Somewhat related question: can it happen that the command (stored in
`this-command`, apparently) is not a symbol, and if so how do you
wrap it?


        Stefan

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

From: Jonas Bernoulli <jonas <at> bernoul.li>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 76528 <at> debbugs.gnu.org
Subject: Re: bug#76528: 30; Advising functions can cause docstring to be lost
Date: Wed, 26 Feb 2025 17:12:25 +0100

> That sounds about right, tho you'd have to replace the `(documentation
> this-command)` with `(get this-command 'function-documentation)`.

I've decided to instead of destroying and resurrecting the doc-string,
I'm just not gonna destroy it.  ;P

I.e., to switch from advice-add to add-function.  I had somehow
convinced myself before that this does not work, but I must have
been holding it wrong.

(I still have to test it a bit more before pushing to main.)

This also (and really more importantly) helps with:

> Somewhat related question: can it happen that the command (stored in
> `this-command`, apparently) is not a symbol, and if so how do you
> wrap it?

It cannot happen for suffixes, those are always accessed via symbols.

But some menus do not disallow invocation of non-suffixes (commands
bound in other active keymaps).  If such a command is not accessed via
a symbol, then it wasn't properly wrapped before.  This could in rare
cases lead to a menu getting "stuck", which, while unfortunate, was easy
to fix (just invoke any menu).  Of course its better if this can never
happen, and I believe that by switching to add-function I have achieves
that.

     Cheers,
     Jonas

Previous Next

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