GNU bug report logs - #13305
24.2; describe-function gives incorrect function location information (for functions changed in a hook?)

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 13305 in the body.
You can then email your comments to 13305 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: Mark Lillibridge <mdl <at> alum.mit.edu>
To: bug-gnu-emacs <at> gnu.org
Subject: 24.2;
	describe-function gives incorrect function location information (for
	functions changed in a hook?)
Date: Sat, 29 Dec 2012 12:16:09 -0800

    While working on a patch, I ran into a really weird and misleading
case where describe-function lied about where a function was located,
making me think my redefinition of the function had been ignored.

Here's a recipe to reproduce the problem:

    Start with emacs -q, do M-: (require 'rmailsum), then use C-h f on
rmail-summary-output; you should (correctly) see:

    rmail-summary-output is an interactive compiled Lisp function in
    `rmailsum.el'.
    
    (rmail-summary-output &optional FILE-NAME N)
    
    Append this message to mail file FILE-NAME.
    This works with both mbox format and Babyl format files,
    outputting in the appropriate format for each.
    The default file name comes from `rmail-default-file',
    which is updated to the name you use in this command.
    
    A prefix argument N says to output that many consecutive messages
    from those in the summary, starting with the current one.
    Deleted messages are skipped and don't count.
    When called from Lisp code, N may be omitted and defaults to 1.
    
    This command always outputs the complete message header,
    even the header display is currently pruned.

Note that the location of the function is correctly given as in rmailsum.el.


    Now start again with emacs -q then evaluate the following elisp
code:

====================  cut here for elisp ====================
(add-hook 'rmail-mode-hook
  (function (lambda()

(require 'rmailsum)

(defun rmail-summary-output (&optional file-name n)
  "MODIFIED!
Append this message to mail file FILE-NAME.
This works with both mbox format and Babyl format files,
outputting in the appropriate format for each.
The default file name comes from `rmail-default-file',
which is updated to the name you use in this command.

A prefix argument N says to output that many consecutive messages
from those in the summary, starting with the current one.
Deleted messages are skipped and don't count.
When called from Lisp code, N may be omitted and defaults to 1.

This command always outputs the complete message header,
even the header display is currently pruned.
[mdl: patched for bug #12214]"
  (interactive
   (progn (require 'rmailout)
	  (list (with-current-buffer rmail-buffer (rmail-output-read-file-name))
		(prefix-numeric-value current-prefix-arg))))
  (let ((i 0) prev-msg)
    (while
	(and (< i n)
	     (progn (rmail-summary-goto-msg)
		    (not (eq prev-msg
			     (setq prev-msg
				   (with-current-buffer rmail-buffer
				     rmail-current-message))))))
      (setq i (1+ i))
      (with-current-buffer rmail-buffer
	(let ((rmail-delete-after-output nil))
	  (with-no-warnings (rmail-output file-name 1))))
      (if rmail-delete-after-output
	  (rmail-summary-delete-forward nil)
	(if (< i n)
	    (rmail-summary-next-msg 1))))))

)))
====================  cut here for elisp ====================

    Basically, this replaces the built-in version of
rmail-summary-output with a patched version; the patching is done only
when Rmail started via the Rmail mode hook (I don't always use Rmail
mode and don't want to unnecessarily load things to save on start up
speed with slow connections).

    Now do M-x rmail then once again C-h f on rmail-summary-output; you
should (incorrectly) see:

    rmail-summary-output is an interactive Lisp function in `rmailsum.el'.
    
    (rmail-summary-output &optional FILE-NAME N)
    
    MODIFIED!
    Append this message to mail file FILE-NAME.
    This works with both mbox format and Babyl format files,
    outputting in the appropriate format for each.
    The default file name comes from `rmail-default-file',
    which is updated to the name you use in this command.
    
    A prefix argument N says to output that many consecutive messages
    from those in the summary, starting with the current one.
    Deleted messages are skipped and don't count.
    When called from Lisp code, N may be omitted and defaults to 1.
    
    This command always outputs the complete message header,
    even the header display is currently pruned.
    [mdl: patched for bug #12214]

Note that the modification has been made (see the MODIFIED! at the top)
*BUT* help still thinks the function is located in rmailsum.el.


    By contrast, if I do the patching outside of the Rmail mode hook,
(emacs -q, eval all but first 2 and last line, M-x rmail, then C-h f) I
get:

    rmail-summary-output is an interactive Lisp function in `temporary'.
    
    (rmail-summary-output &optional FILE-NAME N)
    
    MODIFIED!
    Append this message to mail file FILE-NAME.
    This works with both mbox format and Babyl format files,
    outputting in the appropriate format for each.
    The default file name comes from `rmail-default-file',
    which is updated to the name you use in this command.
    
    A prefix argument N says to output that many consecutive messages
    from those in the summary, starting with the current one.
    Deleted messages are skipped and don't count.
    When called from Lisp code, N may be omitted and defaults to 1.
    
    This command always outputs the complete message header,
    even the header display is currently pruned.
    [mdl: patched for bug #12214]

(temporary is the name of the file I evaluated the list from).

- Mark

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

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: mdl <at> alum.mit.edu
Cc: 13305 <at> debbugs.gnu.org
Subject: Re: bug#13305: 24.2;
	describe-function gives incorrect function location information (for
	functions changed in a hook?)
Date: Sun, 30 Dec 2012 21:57:42 -0500

>     Basically, this replaces the built-in version of
> rmail-summary-output with a patched version; the patching is done only

Don't do that.  Use defadvice or advice-add instead.


        Stefan

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

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: mdl <at> alum.mit.edu
Cc: 13305 <at> debbugs.gnu.org
Subject: Re: bug#13305: 24.2;
	describe-function gives incorrect function location information (for
	functions changed in a hook?)
Date: Sun, 30 Dec 2012 21:59:08 -0500

> (add-hook 'rmail-mode-hook
>   (function (lambda()
> (require 'rmailsum)
> (defun rmail-summary-output (&optional file-name n)

I also *strongly* discourage the use of `defun' within another function.


        Stefan

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

From: Mark Lillibridge <mdl <at> alum.mit.edu>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 13305 <at> debbugs.gnu.org
Subject: Re: bug#13305: 24.2;
	describe-function gives incorrect function location information (for
	functions changed in a hook?)
Date: Tue, 01 Jan 2013 11:34:22 -0800

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

>  >     Basically, this replaces the built-in version of
>  > rmail-summary-output with a patched version; the patching is done only
>  
>  Don't do that.  Use defadvice or advice-add instead.

    For some reason I was thinking that changes to interactive didn't
work properly with advice.  Just using defun can be more convenient than
advice, however, when you are in a hurry for a temporary patch.

    It's too bad there isn't a macro for replace-defun that acts
syntactically identical to defun except that it uses advice to patch
lazily the built-in (e.g., part of Emacs distribution) function.  This
would be a lot easier for users to understand and use.  For that matter,
what would be really useful is a small (10-20 page?) guide to patching
Emacs with this sort of useful information.  Yes, I know there is a 800+
page elisp manual that contains a lot of this information somewhere
(maybe?)  But the reality is that the average person trying to patch
Emacs doesn't have time to read the manual and limits themselves to
Google searches.  You don't discover advice that way.

- Mark

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

From: Mark Lillibridge <mdl <at> alum.mit.edu>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 13305 <at> debbugs.gnu.org
Subject: Re: bug#13305: 24.2;
	describe-function gives incorrect function location information (for
	functions changed in a hook?)
Date: Tue, 01 Jan 2013 11:35:30 -0800

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

>  > (add-hook 'rmail-mode-hook
>  >   (function (lambda()
>  > (require 'rmailsum)
>  > (defun rmail-summary-output (&optional file-name n)
>  
>  I also *strongly* discourage the use of `defun' within another function.

    Is this in the elisp manual somewhere?  It's exactly what one would
do given experience with scheme or other lisps.

- Mark

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Mark Lillibridge <mdl <at> alum.mit.edu>
Cc: 13305 <at> debbugs.gnu.org
Subject: Re: bug#13305: 24.2;
	describe-function gives incorrect function location information (for
	functions changed in a hook?)
Date: Thu, 03 Jan 2013 12:19:51 -0500

>> > (add-hook 'rmail-mode-hook
>> >   (function (lambda()
>> > (require 'rmailsum)
>> > (defun rmail-summary-output (&optional file-name n)
>> I also *strongly* discourage the use of `defun' within another function.
>     Is this in the elisp manual somewhere?  It's exactly what one would
> do given experience with scheme or other lisps.

In Scheme, a local `define' does not affect the global definition.
So it's a completely different operation (it's more like a `let').


        Stefan

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Mark Lillibridge <mdl <at> alum.mit.edu>
Cc: 13305 <at> debbugs.gnu.org
Subject: Re: bug#13305: 24.2;
	describe-function gives incorrect function location information (for
	functions changed in a hook?)
Date: Thu, 03 Jan 2013 12:22:34 -0500

>     For some reason I was thinking that changes to interactive didn't
> work properly with advice.  Just using defun can be more convenient than
> advice, however, when you are in a hurry for a temporary patch.

Fine, but don't expect everything else to handle it just right.

>     It's too bad there isn't a macro for replace-defun that acts
> syntactically identical to defun except that it uses advice to patch
> lazily the built-in (e.g., part of Emacs distribution) function.

Instead of (replace-defun foo (ARGS) BODY), you can write
(advice-add foo :around (lambda (_ignore ARGS) BODY))
If needed we could add a :replace to advice-add, so it'd be
(advice-add foo :replace (lambda (ARGS) BODY)), but I'm not convinced
it's worth the trouble.


        Stefan

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

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: mdl <at> alum.mit.edu
Cc: 13305 <at> debbugs.gnu.org
Subject: Re: bug#13305: 24.2;
	describe-function gives incorrect function location information (for
	functions changed in a hook?)
Date: Tue, 22 Jan 2013 21:16:57 -0500

BTW, now that `defun' is macro-expanded when loading the file (thanks to
eager macro-expansion), we could actually use it to record the file where
this `defun' was found, even if that `defun' is (as in your case) only
executed at some later time.


        Stefan

Previous Next

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