Re: [PATCH] gnu: Add haskell-mode.

[Mark H Weaver]

Federico Beffa <address@hidden> writes:

> From d08df1568688701e20582c74acc85b3d9b5da01a Mon Sep 17 00:00:00 2001
> From: Federico Beffa <address@hidden>
> Date: Fri, 12 Jun 2015 18:58:35 +0200
> Subject: [PATCH] gnu: Add haskell-mode.
>
> * gnu/packages/emacs.scm (haskell-mode): New variable.
> ---
>  gnu/packages/emacs.scm | 62 
> ++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 62 insertions(+)
>
> diff --git a/gnu/packages/emacs.scm b/gnu/packages/emacs.scm
> index fbddff1..2312711 100644
> --- a/gnu/packages/emacs.scm
> +++ b/gnu/packages/emacs.scm
> @@ -430,6 +430,68 @@ operations.")
>  support for Git-SVN.")
>      (license license:gpl3+)))
>  
> +(define-public haskell-mode
> +  (package
> +    (name "haskell-mode")
> +    (version "13.14.2")
> +    (source (origin
> +              (method url-fetch)
> +              (uri (string-append
> +                    "https://github.com/haskell/haskell-mode/archive/v";
> +                    version ".tar.gz"))

Please add a 'file-name' field to this origin, so that the source code
in the store will be called /gnu/store/…-haskell-mode-13.14.2.tar.gz
instead of /gnu/store/…-v13.14.2.tar.gz:

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

> +             (sha256
> +              (base32 
> "1kxc2yj8vb122dv91r68h7c5ladcryx963fr16plfhg71fv7f9av"))))
> +    (inputs `(("emacs" ,emacs-no-x)))
> +    (native-inputs
> +     `(("texinfo" ,texinfo)))
> +    (build-system gnu-build-system)
> +    (arguments
> +     `(#:make-flags (list (string-append "EMACS="
> +                                         (assoc-ref %build-inputs "emacs")
> +                                         "/bin/emacs"))
> +       #:phases
> +       (modify-phases %standard-phases
> +         (delete 'configure)
> +         (add-before
> +          'build 'pre-build
> +          (lambda _
> +            (setenv "SHELL" "sh")
> +            (substitute* (find-files "." ".*\\.el") (("/bin/sh") "sh"))

Please use "\\.el$" instead of ".*\\.el", and
(string-append (assoc-ref inputs "bash") "/bin/sh") instead of "sh" in
the substitution.  Plain "sh" is fine for SHELL.

> +            #t))
> +         (replace
> +          'install
> +          (lambda* (#:key outputs #:allow-other-keys)
> +            (let* ((out (assoc-ref outputs "out"))
> +                   (el-dir (string-append out "/share/emacs/site-lisp"))
> +                   (doc (string-append
> +                         out "/share/doc/haskell-mode-" ,version))
> +                   (info (string-append out "/share/info")))
> +              (with-directory-excursion "doc"
> +                (system* "makeinfo" "haskell-mode.texi")

Please check the result of 'system*':

 (unless (zero? (system* "makeinfo" "haskell-mode.texi"))
   (error "makeinfo failed"))

> +                (mkdir-p info)
> +                (copy-file "haskell-mode.info"
> +                           (string-append info "/haskell-mode.info")))
> +              (for-each
> +               (lambda (dir files)
> +                 (mkdir-p dir)
> +                 (for-each
> +                  (lambda (f)
> +                    (copy-file f (string-append dir "/" (basename f))))
> +                  files))
> +               (list doc el-dir)
> +               (list '("CONTRIBUTING.md" "NEWS" "README.md")
> +                     (find-files "." ".*\\.elc?")))

Please replace ".*\\.elc?" with "\\.elc?$".

Still, this outer 'for-each' is a bit hard to read.  How about something
like this instead?  (with the 'define' as the first thing in the 'let*')

  (define (copy-to-dir dir files)
    (mkdir-p dir)
    (for-each (lambda (f)
                (copy-file f (string-append dir "/" (basename f))))
              files))

[...]

  (copy-to-dir doc '("CONTRIBUTING.md" "NEWS" "README.md"))
  (copy-to-dir el-dir (find-files "." "\\.elc?$"))

> +               ;; these are now distributed with emacs
> +               (with-directory-excursion el-dir
> +                 (for-each delete-file '("cl-lib.el" "ert.el")))

These three lines are indented one more column than they should be,
making them appear to be within the 'for-each'.

> +              #t))))))
> +    (home-page "https://github.com/haskell/haskell-mode";)
> +    (synopsis "Haskell mode for Emacs")
> +    (description
> +     "This is an Emacs mode for editing, debugging and developing Haskell
> +programs.")
> +    (license license:gpl3+)))
> +

Okay to push with these changes.

    Thanks!
      Mark