Re: New version of todo-mode.el (announcement + user guide)

[Stephen Berman <stephen.berman@gmx.net>]

On Thu, 13 Jun 2013 20:21:01 -0400 Stefan Monnier <monnier@iro.umontreal.ca> wrote:

>> Thank you for this excellent "advice" ;-).  It works very nicely --
>> except for one wrinkle: after loading todos.el (which requires
>> 'diary-lib) I have to eval diary-goto-entry (or load diary-lib) again in
>> order for the advice to take effect.  Without doing that it's as if the
>> advice were not activated.  But I see nothing about activating advice in
>> nadvice.el.  Here's what I added to diary-lib.el:
>
> Contrary to advice, which has all kinds of subtle distinction between
> defining an advice, enabling an advice, and activating an advice,
> nadvice just has add-function (or advice-add) which
> works more like add-hook (i.e. kind of defines/enables/activates all at
> once).

Ah, ok, thanks for clarifying.

>> (define-derived-mode todos-mode special-mode "Todos"
>>   "Major mode for displaying, navigating and editing Todo lists.
>
>> \\{todos-mode-map}"
>> ;; other initializations ...
>>   (add-function :around diary-goto-location-function
>> 		(lambda (orig-fun &rest args)
>> 		  (when (derived-mode-p 'todos-mode) (widen))
>> 		  (apply orig-fun args)
>> 		  (todos-diary-goto-entry))))
>
> This doesn't make sense: you `add-function' to the global value of
> diary-goto-location-function, but you do it in the mode function,
> i.e. once per todo-mode buffer.
>
> Either do it locally (i.e. in the todo-mode function but with
>
>    (add-function :around (local diary-goto-location-function)
>  		(lambda (orig-fun &rest args)
>  		  (widen)
>  		  (apply orig-fun args)
>  		  (todos-diary-goto-entry)))
>
> Or do it globally, (i.e. at the top-level of todo-mode.el).

Thanks.  Doing it locally still doesn't work, I guess because the
function is called from the Fancy Diary buffer, not from Todos mode.
But it works when it's added at top-level.  Moreover, it turns out that
using :around isn't right, at least not without making it much more
complicated AFAICT.  It's much more straightforward to use :override.
So I'd like to make the following change to diary-lib.el:


*** /data/steve/bzr/emacs/trunk/lisp/calendar/diary-lib.el	2013-06-05 11:41:31.000000000 +0200
--- /data/steve/bzr/emacs/quickfixes/lisp/calendar/diary-lib.el	2013-06-14 23:34:12.000000000 +0200
***************
*** 1032,1038 ****
  (define-obsolete-function-alias 'simple-diary-display
    'diary-simple-display "23.1")
  
! (define-button-type 'diary-entry 'action #'diary-goto-entry
    'face 'diary-button 'help-echo "Find this diary entry"
    'follow-link t)
  
--- 1032,1045 ----
  (define-obsolete-function-alias 'simple-diary-display
    'diary-simple-display "23.1")
  
! (defvar diary-goto-entry-function 'diary-goto-entry
!   "Function called to jump to a diary entry.
! Modes that require special handling of the included file
! containing the diary entry can assign a suitable function to this
! variable.")
! 
! (define-button-type 'diary-entry
!   'action (lambda (button) (funcall diary-goto-entry-function button))
    'face 'diary-button 'help-echo "Find this diary entry"
    'follow-link t)

  
Any objections?

Steve Berman