GNU bug report logs - #40928
[PATCH 1/2] gnu: Add emacs-ryo-modal.

Previous Next

Full log

Message #11 received at 40928 <at> debbugs.gnu.org (full text, mbox):

From: Nicolas Goaziou <mail <at> nicolasgoaziou.fr>
To: pinoaffe <pinoaffe <at> airmail.cc>
Cc: 40928 <at> debbugs.gnu.org
Subject: Re: [bug#40928] [PATCH 1/2] gnu: Add emacs-ryo-modal.
Date: Tue, 28 Apr 2020 17:46:29 +0200

Hello,

pinoaffe <pinoaffe <at> airmail.cc> writes:

> * gnu/packages/emacs-xyz.scm (emacs-ryo-modal): New variable.

Thank you. 

I have some comments about the patch.

> +(define-public emacs-ryo-modal
> +  (package
> +    (name "emacs-ryo-modal")
> +    (version "20191017.1323")

This version comes from MELPA, but is not official. I suggest to use
"0.4", as defined in the main file, with a "revision" binding and
a "commit" reference. See, e.g., `emacs-uml-mode'.

> +    (source
> +      (origin
> +        (method url-fetch)
> +        (uri (string-append
> +               "https://melpa.org/packages/ryo-modal-"
> +               version
> +               ".el"))

We do not use files from MELPA, as those are not stable, IIRC. You may
want to git-fetch from GitHub directly. Again, see, e.g.,
`emacs-uml-mode'.

> +        (sha256
> +          (base32
> +            "0hslayyfckhirx9sk1pjdrl7acgvwj89aa5spiavz4z6a8rm17js"))))

Nitpick: I suggest to put `base32' on the same line as the hash.

> +    (build-system emacs-build-system)
> +    (home-page
> +      "http://github.com/Kungsgeten/ryo-modal")

This could go on the same line.

> +    (synopsis "Roll your own modal mode")

Could you expound the synopsis a bit? Maybe:

  Emacs minor mode for defining your own modal editing environment

> +    (description
> +      "ryo-modal provides a convenient way of defining modal keybindings in Emacs.

I suggest:

  RYO modal provides a way of defining...

> +The primary way of binding keys is using `ryo-modal-key' and `ryo-modal-keys'.
> +Both of these functions provide useful keyword arguments.
> +`ryo-modal-mode' is used to toggle the modal editing environment.
> +ryo-modal does not come with any predefined bindings!

IMO the description should not be a manual. Could you remove references
to variables and functions?

> +If you want bindings that only should be active when the region is active,
> +please have a look at `selected-minor-mode'
> (https://github.com/Kungsgeten/selected.el).")

I don't think this is needed in the description either.

Could you send an updated patch?

Regards,

-- 
Nicolas Goaziou

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.