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

[Robert Weiner]

Thanks, Stefan.  Will implement these updates.

On your last comment, we want Hyperbole initialization deferred until after Emacs init time, so that looks right to me.

Regards,

Bob

On Thu, Apr 29, 2021 at 2:00 AM Stefan Monnier <monnier@iro.umontreal.ca> wrote:

> +(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