all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* bug#13305: 24.2; describe-function gives incorrect function location information (for functions changed in a hook?)
@ 2012-12-29 20:16 Mark Lillibridge
  2012-12-31  2:57 ` Stefan Monnier
  2012-12-31  2:59 ` Stefan Monnier
  0 siblings, 2 replies; 8+ messages in thread
From: Mark Lillibridge @ 2012-12-29 20:16 UTC (permalink / raw)
  To: 13305


    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





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#13305: 24.2; describe-function gives incorrect function location information (for functions changed in a hook?)
  2012-12-29 20:16 bug#13305: 24.2; describe-function gives incorrect function location information (for functions changed in a hook?) Mark Lillibridge
@ 2012-12-31  2:57 ` Stefan Monnier
  2013-01-01 19:34   ` Mark Lillibridge
  2012-12-31  2:59 ` Stefan Monnier
  1 sibling, 1 reply; 8+ messages in thread
From: Stefan Monnier @ 2012-12-31  2:57 UTC (permalink / raw)
  To: mdl; +Cc: 13305

>     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





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#13305: 24.2; describe-function gives incorrect function location information (for functions changed in a hook?)
  2012-12-29 20:16 bug#13305: 24.2; describe-function gives incorrect function location information (for functions changed in a hook?) Mark Lillibridge
  2012-12-31  2:57 ` Stefan Monnier
@ 2012-12-31  2:59 ` Stefan Monnier
  2013-01-01 19:35   ` Mark Lillibridge
  1 sibling, 1 reply; 8+ messages in thread
From: Stefan Monnier @ 2012-12-31  2:59 UTC (permalink / raw)
  To: mdl; +Cc: 13305

> (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





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#13305: 24.2; describe-function gives incorrect function location information (for functions changed in a hook?)
  2012-12-31  2:57 ` Stefan Monnier
@ 2013-01-01 19:34   ` Mark Lillibridge
  2013-01-03 17:22     ` Stefan Monnier
  2013-01-23  2:16     ` Stefan Monnier
  0 siblings, 2 replies; 8+ messages in thread
From: Mark Lillibridge @ 2013-01-01 19:34 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 13305


Stefan Monnier <monnier@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





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#13305: 24.2; describe-function gives incorrect function location information (for functions changed in a hook?)
  2012-12-31  2:59 ` Stefan Monnier
@ 2013-01-01 19:35   ` Mark Lillibridge
  2013-01-03 17:19     ` Stefan Monnier
  0 siblings, 1 reply; 8+ messages in thread
From: Mark Lillibridge @ 2013-01-01 19:35 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 13305


Stefan Monnier <monnier@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






^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#13305: 24.2; describe-function gives incorrect function location information (for functions changed in a hook?)
  2013-01-01 19:35   ` Mark Lillibridge
@ 2013-01-03 17:19     ` Stefan Monnier
  0 siblings, 0 replies; 8+ messages in thread
From: Stefan Monnier @ 2013-01-03 17:19 UTC (permalink / raw)
  To: Mark Lillibridge; +Cc: 13305

>> > (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





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#13305: 24.2; describe-function gives incorrect function location information (for functions changed in a hook?)
  2013-01-01 19:34   ` Mark Lillibridge
@ 2013-01-03 17:22     ` Stefan Monnier
  2013-01-23  2:16     ` Stefan Monnier
  1 sibling, 0 replies; 8+ messages in thread
From: Stefan Monnier @ 2013-01-03 17:22 UTC (permalink / raw)
  To: Mark Lillibridge; +Cc: 13305

>     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





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#13305: 24.2; describe-function gives incorrect function location information (for functions changed in a hook?)
  2013-01-01 19:34   ` Mark Lillibridge
  2013-01-03 17:22     ` Stefan Monnier
@ 2013-01-23  2:16     ` Stefan Monnier
  1 sibling, 0 replies; 8+ messages in thread
From: Stefan Monnier @ 2013-01-23  2:16 UTC (permalink / raw)
  To: mdl; +Cc: 13305

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





^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2013-01-23  2:16 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-12-29 20:16 bug#13305: 24.2; describe-function gives incorrect function location information (for functions changed in a hook?) Mark Lillibridge
2012-12-31  2:57 ` Stefan Monnier
2013-01-01 19:34   ` Mark Lillibridge
2013-01-03 17:22     ` Stefan Monnier
2013-01-23  2:16     ` Stefan Monnier
2012-12-31  2:59 ` Stefan Monnier
2013-01-01 19:35   ` Mark Lillibridge
2013-01-03 17:19     ` Stefan Monnier

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.