From: Nicolas Goaziou <mail@nicolasgoaziou.fr>
To: pinoaffe <pinoaffe@airmail.cc>
Cc: 40928@debbugs.gnu.org
Subject: [bug#40928] [PATCH 1/2] gnu: Add emacs-ryo-modal.
Date: Tue, 28 Apr 2020 17:46:29 +0200 [thread overview]
Message-ID: <87pnbrhaui.fsf@nicolasgoaziou.fr> (raw)
In-Reply-To: <20200428131817.2141e9e5@airmail.cc> (pinoaffe@airmail.cc's message of "Tue, 28 Apr 2020 13:18:17 +0200")
Hello,
pinoaffe <pinoaffe@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
prev parent reply other threads:[~2020-04-28 15:48 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-28 11:18 [bug#40928] [PATCH 1/2] gnu: Add emacs-ryo-modal pinoaffe
2020-04-28 11:26 ` [bug#40928] Subject: [PATCH 2/2] gnu: Add emacs-kakoune pinoaffe
2020-04-28 15:55 ` Nicolas Goaziou
2020-04-28 18:05 ` [bug#40928] [PATCH 1/2 v2] gnu: Add emacs-ryo-modal pinoaffe
2020-04-29 14:36 ` bug#40928: " Nicolas Goaziou
2020-04-28 18:06 ` [bug#40928] [PATCH 2/2 v2] gnu: Add emacs-kakoune pinoaffe
2020-04-29 14:38 ` Nicolas Goaziou
2020-04-28 15:46 ` Nicolas Goaziou [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://guix.gnu.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87pnbrhaui.fsf@nicolasgoaziou.fr \
--to=mail@nicolasgoaziou.fr \
--cc=40928@debbugs.gnu.org \
--cc=pinoaffe@airmail.cc \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/guix.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).