Subject: Re: [bug#60212] [PATCH] gnu: Add emacs-fzf.
From: Nicolas Goaziou <[email-masked]>
To: jgart via Guix-patches via <[email-masked]>
Cc: [email-masked], jgart <[email-masked]>
Date: Tue, 20 Dec 2022 11:25:58 +0100
List-Id: <debbugs-submit.debbugs.gnu.org>

Hello,

jgart via Guix-patches via <[email-masked]> writes:

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

Thank you.

> +         #~(modify-phases %standard-phases
> +             (add-after 'unpack 'patch-fzf-executable
> +               (lambda* (#:key inputs #:allow-other-keys)
> +                 (make-file-writable "fzf.el")

Nitpick: The expression above is usually not necessary, even though some
package definitions use it.

> +                 (emacs-substitute-variables "fzf.el"
> +                   ("fzf/executable" (search-input-file inputs "/bin/fzf"))))))))
> +      (inputs (list fzf))
> +      (home-page "https://github.com/bling/fzf.el")
> +      (synopsis "Front-end for fzf")

I expounded it a bit.

> +      (description "An @command{emacs} front-end for @command{fzf}.")

Please write full sentences in descriptions, even if upstream does not.

Also, "@command{emacs}" conveys no more information than "Emacs". So
does "@command{fzf}" vs "Fzf". The simpler the better, IMO.

Applied!

Regards,
-- 
Nicolas Goaziou