Re: [elpa] externals/hyperbole faa8294 2/3: Redo hyperbole-mode; new {r} HyRolo search in match buffer

[Stefan Monnier <monnier@iro.umontreal.ca>]

> +(defun enable-hyperbole-mode ()
> +  "Enable Hyperbole global minor mode."
> +  (interactive)

The `hyperbole-mode` command already offers that functionality, so
there's no point providing this as an interactive command.
I'd recommend you use a "--" in the name and keep that function "internal".
Also please use a "hyperbole-" prefix.

> +  ;; Activate hyperbole-mode
> +  (run-hooks 'hyperbole-mode-hook))

`define-minor-mode` runs `hyperbole-mode-hook` for you already, so this
`run-hooks` will cause the hook to be run twice in a row.

>    :keymap hyperbole-mode-map

This argument is redundant.

> +(if after-init-time
> +    ;; Initialize Hyperbole key bindings and hooks.
> +    (hyperb:init)
> +  ;; Initialize after other key bindings are loaded at startup.
> +  (add-hook 'after-init-hook #'hyperb:init t))
>  
>  ;; !! FIXME: Loading a file should not change Emacs's behavior but we
>  ;; need this here for awhile until can ensure Hyperbole users know to

I suspect the above code and the above comment should be swapped?


        Stefan