Avoid positional arguments to define-minor-mode

[tarsius]

Back in Emacs-21.1, define-minor-mode grew keyword arguments to replace its old positional arguments. Starting with Emacs-28.1 warning will be omitted if positional arguments are still used.

[purcell]

Generally in favour, but doesn't this cause symbol-overlay-mode-map to no longer be defined?

[purcell]

(Not that it's used in the code or examples, but I - for one - use it instead of binding keys globally, see here

[tarsius]

If the keymap is left unspecified, then a sparse keymap is created and bound to MODE-map.

:keymap MAP Keymap bound to the mode keymap.  Defaults to `MODE-map'.
        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 (KEY . BINDING) pairs where KEY and BINDING are
        suitable for `define-key'.  If you supply a KEYMAP argument
        that is not a symbol, this macro defines the variable MODE-map
        and gives it the value that KEYMAP specifies.

[purcell]

I think that's a misunderstanding of the docstring, given what I see when I macroexpand it:

(define-minor-mode foo-mode "")

=>

                (add-minor-mode 'foo-mode 'nil
                    (if
                        (boundp 'foo-mode-map)
                        foo-mode-map)
                    nil nil)

[tarsius]

You're right. I've fixed the pr. Instead of doing it exactly like this you could also or instead add:

(defvar symbol-overlay-mode-map (make-sparse-keymap)
  "Keymap for `symbol-overlay-mode'.")

in order to be more explicit. Would you prefer that?

[purcell]

Yeah, that'd be ideal, probably. But a defcustom I guess.

[tarsius]

But a defcustom I guess.

Whaaaaa?!

[tarsius]

I've added the defvar, so now it is doppelt gemoppelt.

[purcell]

But a defcustom I guess.

Whaaaaa?!

There's a :type 'keymap, no? But whatever, I'll go ahead and merge this. Thanks!