[bug#32614] [PATCH] emacs-irony-mode

[Ludovic Courtès]

Hello Tim,

Tim Gesthuizen <address@hidden> skribis:

> The attached patch adds emacs-irony-mode.
> It is also packaged in MELPA so it is definitely free software.
> If there are problems with the description or synopsis just let me know
> and I will change the patch accordingly.

Thanks for the patch!  I have some suggestions and comments below, but
overall it LGTM:

> From 6975ba9e4b005c77f00d7f2187b5d8047f15ba07 Mon Sep 17 00:00:00 2001
> From: Tim Gesthuizen <address@hidden>
> Date: Thu, 30 Aug 2018 17:39:57 +0200
> Subject: [PATCH] gnu: Add emacs-irony-mode
>
> ---
>  gnu/packages/emacs.scm | 49 ++++++++++++++++++++++++++++++++++++------

Please run ‘git log gnu/packages/emacs.scm’ to see our conventions for
commit logs, or see
<https://www.gnu.org/prep/standards/html_node/Change-Logs.html>.  (We can
always fix it up for you though, it’s no big deal.)

> +    (home-page "https://github.com/Sarcasm/irony-mode";)
> +    (synopsis "Clang autocompletion and syntax checking integration for GNU 
> Emacs")

It’s a bit long.  Perhaps: “Code completion and syntax checks for Emacs”?

> +    (description "Provides clang assisted syntax checking and autocompletion
> + for C,C++ and ObjC.")

Please make a full sentence, as per
<https://www.gnu.org/software/guix/manual/en/html_node/Synopses-and-Descriptions.html>.

> +    (license license:gpl3)))

Isn’t it ‘gpl3+’, meaning “version 3 or any later version, at your
option”?

> +(define-public emacs-irony-mode-server
> +  (package (inherit emacs-irony-mode)
> +    (name "emacs-irony-mode-server")
> +    (propagated-inputs
> +     `(("clang" ,clang)))

Instead of propagating Clang, which clutters the user’s profile, do you
think we could patch the .el files such that they refer to ‘clang’ by
its absolute file name?  See ‘emacs-emms’ for an example of that.

> +    (arguments
> +     `(#:phases
> +       (modify-phases %standard-phases
> +         (replace 'configure
> +           (lambda* (#:key outputs #:allow-other-keys)
> +             (let ((out (assoc-ref outputs "out")))
> +               (invoke "cmake"
> +                       "server"
> +                       (string-append "-DCMAKE_INSTALL_PREFIX=" out))))))))

Please return #t at the end of the phase.

>      (source (origin
> -           (method url-fetch)
> -           (uri (string-append "https://github.com/smihica/emmet-mode";
> -                               "/archive/" version ".tar.gz"))
> +              (method url-fetch)
> +              (uri (string-append "https://github.com/smihica/emmet-mode";
> +                                  "/archive/" version ".tar.gz"))
>                (file-name (string-append name "-" version ".tar.gz"))
> -           (sha256
> -            (base32
> -             "0g3p22yabfcp98cfv9dgl9il2m2pd53isq2q11vb3s7qyn31f7zj"))))
> +              (sha256
> +               (base32
> +                "0g3p22yabfcp98cfv9dgl9il2m2pd53isq2q11vb3s7qyn31f7zj"))))

This change is unnecessary and unrelated; could you remove it?

Could you send an updated patch?

Thank you!

Ludo’.