diff mbox

Updated patch for emacs-helm

Message ID 87lh2qx8qw.fsf@mailerver.i-did-not-set--mail-host-address--so-tickle-me
State New
Headers show

Commit Message

Matthew Jordan May 31, 2016, 8:39 p.m. UTC
Updated patch for review, emacs-helm.
--
Matthew Jordan
Sent with my mu4e

Comments

Alex Kost June 2, 2016, 10:14 a.m. UTC | #1
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!
diff mbox

Patch

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

* gnu/packages/emacs.scm: Modified file.
---
 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)
   #: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")
+    (source (origin
+              (method url-fetch)
+              (uri (string-append
+                    "https://github.com/" name "/helm/archive/v"
+                    version ".tar.gz"))
+              (sha256
+               (base32 "1fjgmjxjwqp55gh4z9jxh28gzbdixkdkg9869dn3c4g8qzy2hg35"))))
+    (inputs `(("git" ,git)))
+    (propagated-inputs
+     `(("dash" ,emacs-dash)
+       ("texinfo" ,texinfo)))
+    (build-system emacs-build-system)
+    (home-page "https://emacs-helm.github.io/helm/")
+    (synopsis "Helm is incremental completion and selection 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
+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+)))
-- 
2.8.3