Re: [PATCH] Emacs Zenburn

[Alex Kost]

Catonano (2016-05-15 17:50 +0300) wrote:

> Hello,

Hello and welcome!

Could you attach the whole patch to your message or use "git send-email"
(to make this work, you need to do "guix package -i git:send-email")
please?  I don't understand how you include the patch to this message,
but it is hard to work with it.

> this is my first attempt at importing a package into Guix. My first attempt
> ever.
>
> It's trivial: the Zenburn theme for Emacs.
>
> The linter claims that lines 44 and 45 are too long. I didn't choose myself
> to make them so long. It was the automatic identation on the Emacs Scheme
> mode. Did it do the wrong thing ?

The lines are so long, because the indentation of
'with-directory-excursion' is not correct.  The guix git checkout
contains ".dir-locals.el" file which sets the proper indentation.  So if
you worked with the git checkout, emacs should have asked you something
about safe local variables.  If you accept it, it means this
".dir-locals.el" is loaded, and the proper indentation is set up.

Alternatively you can configure emacs interface that comes with guix as
described in the manual (info "(guix) Emacs Initial Setup").  In short
this can be done by "guix package -i guix".  Along with the mentioned
guix-specific indentation, it provides many "M-x guix-..." commands.

> It also filled my lines with tabs rather than spaces. I removed them by
> hand !

'indent-tabs-mode' variable is also handled by .dir-locals.

> From a8e28699e4cc02e6213047de1caae00edde06ccb Mon Sep 17 00:00:00 2001
> From: humanitiesNerd <address@hidden>

BTW are you going to use "humanitiesNerd" to identify yourself?  It's
OK, I'm just asking in case you want to use some other name.

> Date: Sun, 15 May 2016 14:55:28 +0200
> Subject: [PATCH] appended the package definition for the Emacs Zenburn theme
>  to the gnu/packages/emacs.scm file

Our convention is to use the following commit message:

--8<---------------cut here---------------start------------->8---
gnu: Add emacs-zenburn-theme.

* gnu/packages/emacs.scm (emacs-zenburn-theme): New variable.
--8<---------------cut here---------------end--------------->8---

> ---
>  gnu/packages/emacs.scm | 48
> ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 48 insertions(+)
>
>
> diff --git a/gnu/packages/emacs.scm b/gnu/packages/emacs.scm
> index 11010b2..e3eeaf4 100644
> --- a/gnu/packages/emacs.scm
> +++ b/gnu/packages/emacs.scm
> @@ -1559,3 +1559,51 @@ to recognize a name like \"RFC 1234\".  This package
> enhances ffap so
>  that it correctly finds RFCs even when a space appears before the
>  number.")
>      (license license:gpl3+)))
> +
> +(define-public emacs-zenburn-theme
> +  (package
> +    (name "emacs-zenburn-theme")
> +    (version "2.4")
> +    (source
> +     (origin
> +       (method url-fetch)
> +       (uri (string-append
> +             "http://stable.melpa.org/packages/zenburn-theme-";
> +             version
> +             ".el"))

We prefer to use the code directly from the upstream.  In this case:
<https://github.com/bbatsov/zenburn-emacs/archive/v2.4.tar.gz>.

> +       (sha256
> +        (base32
> +        "1a65hfkrrlm8ryr2x9zja0h1n8nzk4rnsyzjacqlv4k79yal1fxg"))))
> +    (build-system trivial-build-system)

It is better to use emacs-build-system.  Most likely this is all that
will be required, and you don't have to write 'arguments' at all.

> +    (inputs `(("emacs" ,emacs-no-x)))

With emacs-build-system, this line will not be needed.

> +    (arguments
> +     `(#:modules ((guix build utils)
> +                  (guix build emacs-utils))
> +       #:builder
> +       (begin
> +         (use-modules (guix build utils))
> +         (use-modules (guix build emacs-utils))
> +
> +         (let* ((emacs    (string-append (assoc-ref %build-inputs "emacs")
> +                                         "/bin/emacs"))
> +                (source   (assoc-ref %build-inputs "source"))
> +                (lisp-dir (string-append %output
> +                                         "/share/emacs/site-lisp"))
> +                (target   (string-append lisp-dir "/zenburn-theme.el")))
> +           (mkdir-p lisp-dir)
> +           (copy-file source target)
> +           (with-directory-excursion lisp-dir
> +                                     (parameterize ((%emacs emacs))
> +
> (emacs-generate-autoloads "zenburn-theme" lisp-dir)
> +                                                   (emacs-batch-eval
> '(byte-compile-file "zenburn-theme.el"))))))))
> +
> +    (home-page
> +     "http://github.com/bbatsov/zenburn-emacs";)

I think it's more good-looking to put this (home-page "...") one a
single line.

> +    (synopsis
> +     "Low contrast color theme for Emacs")
> +    (description
> +     "Port of the popular Vim theme Zenburn for Emacs 24, built on top
> +of the new built-in theme support in Emacs 24.
> +")

I don't think we need to specify Emacs version like this.  When Emacs 25
will be out, zenburn theme will work anyway, so this description may be
confusing, what about the following (I don't like it either, I'm not
good in writing descriptions):

  "Zenburn theme is a port of the popular Vim Zenburn theme for Emacs.
It is built on top of the custom theme support in Emacs 24 or later.

FYI theme developers often write "Emacs 24" because this "custom theme"
feature appeared in this emacs version.  Before that, when people said
"theme" they meant "color theme" which is a completely different thing.
Nowadays "custom theme" is the official way to make Emacs themes.

Could you send an updated patch please?

-- 
Alex