Re: [Patch] Updated patch for emacs-helm

[Alex Kost <alezost@gmail.com>]

Matthew Jordan (2016-05-31 23:39 +0300) wrote:

> Updated patch for review, emacs-helm.
>
>
> From d6e128ccb7e2511a474e32dda0d0227e13ed811b Mon Sep 17 00:00:00 2001
> From: Matthew Jordan <matthewjordandevops@yandex.com>
> Date: Wed, 18 May 2016 17:39:45 -0400
> Subject: [PATCH] gnu: Add emacs-helm
                                      ^
(Not a big deal but) period here: "gnu: Add emacs-helm."

> * gnu/packages/emacs.scm: Modified file.

Our convention is to write it like this:

* gnu/packages/emacs.scm (emacs-helm): New variable.

> ---
>  gnu/packages/emacs.scm | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
>
>
> diff --git a/gnu/packages/emacs.scm b/gnu/packages/emacs.scm
> index 5d6db5a..8771500 100644
> --- a/gnu/packages/emacs.scm
> +++ b/gnu/packages/emacs.scm
> @@ -9,6 +9,7 @@
>  ;;; Copyright © 2016 Chris Marusich <cmmarusich@gmail.com>
>  ;;; Copyright © 2015, 2016 Christopher Allan Webber <cwebber@dustycloud.org>
>  ;;; Copyright © 2016 humanitiesNerd <catonano@gmail.com>
> +;;; Copyright © 2016 Matthew Jordan <matthewjordandevops@yandex.com>
>  ;;;
>  ;;; This file is part of GNU Guix.
>  ;;;
> @@ -38,6 +39,7 @@
>    #:use-module (guix build-system glib-or-gtk)
>    #:use-module (guix build-system trivial)
>    #:use-module (gnu packages)
> +  #:use-module (gnu packages version-control)

Not needed as git is not needed (see below).

>    #:use-module (gnu packages guile)
>    #:use-module (gnu packages gtk)
>    #:use-module (gnu packages gnome)
> @@ -1706,3 +1708,31 @@ It is recommended to use @code{clojure-mode} with paredit or smartparens.")
>  The purpose of this library is to wrap all the quirks and hassle of
>  @code{package.el} into a sane API.")
>      (license license:gpl3+)))
> +
> +(define-public emacs-helm
> +  (package
> +    (name "emacs-helm")
> +    (version "1.9.4")

The current version is "1.9.6".

> +    (source (origin
> +              (method url-fetch)
> +              (uri (string-append
> +                    "https://github.com/" name "/helm/archive/v"
> +                    version ".tar.gz"))

Please add this line here:

  (file-name (string-append name "-" version ".tar.gz"))

See <http://lists.gnu.org/archive/html/guix-devel/2016-05/msg00512.html>
for the explanation.

> +              (sha256
> +               (base32 "1fjgmjxjwqp55gh4z9jxh28gzbdixkdkg9869dn3c4g8qzy2hg35"))))
> +    (inputs `(("git" ,git)))

I see that helm has some functionality related to "git grep", but adding
"git" here does nothing by itself.  If helm had some kind of
"helm-git-executable" variable, we could patch it (see how it is done
for "magit" package), but it is not so simple here, because "git" is
hardcoded in several places in the helm source.

So I think this "git" input should be removed: if a user will want to
use helm for "git-grepping", I think (s)he will guess that git needs to
be installed.

> +    (propagated-inputs
> +     `(("dash" ,emacs-dash)
> +       ("texinfo" ,texinfo)))

I wonder why you added these inputs, they are not needed.  Actually,
helm depends on the following emacs packages:

    (propagated-inputs
     `(("emacs-async" ,emacs-async)
       ("emacs-popup" ,emacs-popup)))

> +    (build-system emacs-build-system)
> +    (home-page "https://emacs-helm.github.io/helm/")
> +    (synopsis "Helm is incremental completion and selection narrowing
> +framework for Emacs")

In synopses we don't write "Foo is ...".  It is a good start for
descriptions, but it is not needed for synopses, so:

  "Incremental completion and selection narrowing framework for Emacs"

or maybe even:

  "Incremental and narrowing framework for Emacs"

> +    (description "Helm is incremental completion and selection narrowing
> +framework for Emacs.  It will help steer you in the right direction when you're
> +looking for stuff in Emacs (like buffers, files, etc).  Helm is a fork of
> +anything.el originally written by Tamas Patrovic and can be considered to be

I think it's better to wrap "anything.el" in @code: "@code{anything.el}".

> +its successor.  Helm sets out to clean up the legacy code in anything.el and
> +provide a cleaner, leaner and more modular tool, that's not tied in the trap
> +of backward compatibility.")
> +    (license license:gpl3+)))

Could you send an updated patch?  Also please make sure your patch can
be applied to the current master (this one cannot :-))

And thanks!

-- 
Alex