cal-x.el/diary-lib.el diary-mode-hook

cal-x.el/diary-lib.el diary-mode-hook

[Karl Chen]

Hi, this is regarding calendar.  I experience the symptoms in
Emacs 22.2, and by inspecting CVS HEAD I suspect it is still
affected despite the code having been significantly updated.

emacs -q
(setq diary-display-hook 'fancy-diary-display)
(setq calendar-setup 'one-frame)
(calendar)

A couple places in calendar/*.el seem to assume that
diary-mode-hook is a list of functions, e.g. when it uses ``(memq
'diary-fancy-display diary-display-hook)''.

In my setup I had ``(setq diary-display-hook
'fancy-schedule-display-desk-calendar)'' [instead of add-hook or
``setq .. '(..)''] and this would trigger a type error.

I realize the documentation for diary-display-hook says "list of
functions", but a couple places in the code let-bind
diary-display-hook to non-lists, and the documentation in 22.2 has
sample code with non-lists. 

Thanks,
Karl

Re: cal-x.el/diary-lib.el diary-mode-hook

[Stefan Monnier]

> Hi, this is regarding calendar.  I experience the symptoms in
> Emacs 22.2, and by inspecting CVS HEAD I suspect it is still
> affected despite the code having been significantly updated.

> emacs -q
> (setq diary-display-hook 'fancy-diary-display)
> (setq calendar-setup 'one-frame)
> (calendar)

> A couple places in calendar/*.el seem to assume that
> diary-mode-hook is a list of functions, e.g. when it uses ``(memq
> 'diary-fancy-display diary-display-hook)''.

> In my setup I had ``(setq diary-display-hook
> 'fancy-schedule-display-desk-calendar)'' [instead of add-hook or
> ``setq .. '(..)''] and this would trigger a type error.

> I realize the documentation for diary-display-hook says "list of
> functions", but a couple places in the code let-bind
> diary-display-hook to non-lists, and the documentation in 22.2 has
> sample code with non-lists.

Hooks accept either a function or a list of functions, where the former
is only accepted for historical reasons.  So all the code that sets
hooks to a single function should be reported as (minor) bugs, and all
the documentation that does that should be reported as (non-minor) bugs.
A quick grep in the EMACS_22_BASE branch disn't show me the sample code
you refer to, so could you please tell us where you found it?

This said, using `memq' on a hook is indeed an error.


        Stefan


PS: You should "never" modify a hook with setq, always use `add-hook'.

Re: cal-x.el/diary-lib.el diary-mode-hook

[Karl Chen]

>>>>> On 2008-05-15 07:52 PDT, Stefan Monnier writes:

    >> I realize the documentation for diary-display-hook says
    >> "list of functions", but a couple places in the code
    >> let-bind diary-display-hook to non-lists, and the
    >> documentation in 22.2 has sample code with non-lists.

    Stefan> Hooks accept either a function or a list of functions,
    Stefan> where the former is only accepted for historical
    Stefan> reasons.  So all the code that sets hooks to a single
    Stefan> function should be reported as (minor) bugs, and all
    Stefan> the documentation that does that should be reported as
    Stefan> (non-minor) bugs.  A quick grep in the EMACS_22_BASE
    Stefan> branch disn't show me the sample code you refer to, so
    Stefan> could you please tell us where you found it?

Sure.

Emacs 22.2  (Debian)

sample code in docstring:
    calendar.el:769:  (setq diary-display-hook 'fancy-diary-display) 

Let-binds:
    appt.el:346:      (let* ((diary-display-hook 'appt-make-list)
    diary-lib.el:581: (let* ... (diary-display-hook 'ignore)
    diary-lib.el:915: (let ((diary-display-hook 'fancy-diary-display))

    Stefan> This said, using `memq' on a hook is indeed an error.

Memq on hook:
    cal-x.el:90:   (if (not (memq 'fancy-diary-display diary-display-hook))
    cal-x.el:148:  (if (not (memq 'fancy-diary-display diary-display-hook))

Docstring could be tweaked slightly:
    diary-lib.el:670: (defun fancy-diary-display ()
    diary-lib.el:671:   "Prepare a diary buffer with relevant entries in a fancy, noneditable form.
    diary-lib.el:672: This function is provided for optional use as the `diary-display-hook'."



    Stefan> PS: You should "never" modify a hook with setq, always
    Stefan> use `add-hook'.

I see.  I think the whole confusion arises because
diary-display-hook doesn't act like a regular hook, where a bunch
of hooks are typically stacked and called sequentially.  The
various functions for diary display are really alternates to each
other, not stackable extras.  If diary-display-hook already
contains fancy-diary-display, then when add-hook
fancy-schedule-display-desk-calendar has no effect.  An nil value
for diary-display-hook also has different semantics than for
regular hooks.  Perhaps it should have been called
"diary-display-function" to begin with, though there was probably
a reason for converting it into a list-of-functions hook other
than just its name.

Re: cal-x.el/diary-lib.el diary-mode-hook

[Glenn Morris]

Thanks; I will tidy it up somehow.