From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alex Kost Subject: Re: [Patch] Updated patch for emacs-helm Date: Thu, 02 Jun 2016 13:14:23 +0300 Message-ID: <87r3cfnbi8.fsf@gmail.com> References: <87lh2qx8qw.fsf@mailerver.i-did-not-set--mail-host-address--so-tickle-me> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:39958) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b8Pe0-0005fi-Mm for guix-devel@gnu.org; Thu, 02 Jun 2016 06:14:30 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b8Pdv-0004QB-NQ for guix-devel@gnu.org; Thu, 02 Jun 2016 06:14:27 -0400 Received: from mail-lf0-x242.google.com ([2a00:1450:4010:c07::242]:34652) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b8Pdv-0004Py-0p for guix-devel@gnu.org; Thu, 02 Jun 2016 06:14:23 -0400 Received: by mail-lf0-x242.google.com with SMTP id 65so4613752lfq.1 for ; Thu, 02 Jun 2016 03:14:22 -0700 (PDT) In-Reply-To: <87lh2qx8qw.fsf@mailerver.i-did-not-set--mail-host-address--so-tickle-me> (Matthew Jordan's message of "Tue, 31 May 2016 16:39:19 -0400") List-Id: "Development of GNU Guix and the GNU System distribution." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org Sender: "Guix-devel" To: Matthew Jordan Cc: guix-devel 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 > 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 =C2=A9 2016 Chris Marusich > ;;; Copyright =C2=A9 2015, 2016 Christopher Allan Webber > ;;; Copyright =C2=A9 2016 humanitiesNerd > +;;; Copyright =C2=A9 2016 Matthew Jordan > ;;; > ;;; 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 for the explanation. > + (sha256 > + (base32 "1fjgmjxjwqp55gh4z9jxh28gzbdixkdkg9869dn3c4g8qzy2= hg35")))) > + (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 t= o 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! --=20 Alex