bug#25505: 24.5; doc of `define-minor-mode': incorrect for :keymap

[npostavs@users.sourceforge.net]

tags 25505 notabug
quit

Drew Adams <drew.adams@oracle.com> writes:

> For :keymap, the doc string says "Same as the KEYMAP argument."
> Similarly the doc in (elisp) `Defining Minor Modes'.  This does not
> appear to be correct.
>
> For positional arg KEYMAP, the doc string says:
>
>   If non-nil, it should be a variable name (whose value is a keymap),
>   or an expression that returns either a keymap or a list of
>   arguments for `easy-mmode-define-keymap'.
>   ...
>
> But the value of :keymap apparently cannot be a variable name whose
> value is a keymap or an expression that evaluates to a keymap.  The
> :keymap value you pass is apparently not evaluated - unlike other
> keyword values such as :group.

The doc says "variable name" as opposed to "expression", I think it's
clear that expression will be evaluated, and something which is not an
expression will not be evaluated.  I guess we could add "it should be an
unquoted variable name..."

>
> So, for example, the `define-minor-mode' fr `follow-mode' uses this:
>
>   :keymap follow-mode-map

This is correct, follow-mode-map is the name of the variable.

> and not this:
>
>   :keymap 'follow-mode-map

This is not correct, it's an expression which evaluates to the name of
the variable.

>
> and not this:
>
>   (let ((mainmap (make-sparse-keymap))
>         (map (make-sparse-keymap)))
>     (define-key map "\C-v"	'follow-scroll-up)
>     ...
>     (define-key mainmap follow-mode-prefix map)
>     (define-key mainmap [remap end-of-buffer] 'follow-end-of-buffer)
>     ...
>     mainmap)

This is correct, it's an expression which evaluates to a keymap.

> In fact, it is not clear from the doc whether and which keyword values
> are evaluated.  And shouldn't they all be evaluated or else none be so?

I don't see why, it's a macro, some arguments are evaluated, some
aren't.

> Here is allout.el, for example.  It DOES use a quoted map-variable
> (and it jumps through a few hoops).

I don't think it should be doing that, it seems to be using some kind of
needlessly complicated trickery.

> Next up: autoarg-mode.  This uses an UNquoted map variable.

That's correct.

> Those are only the first two grep hits for `define-minor-mode'.

I checked a few more at random, I found no more incorrect cases.