diff mbox

Tmux Themepack

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

Commit Message

Matthew Jordan June 9, 2016, 5:23 p.m. UTC
Good Day,

This patch to add the tmux themepack.
--
Matthew Jordan
Sent with my mu4e

Comments

Ricardo Wurmus June 11, 2016, 7:45 a.m. UTC | #1
Hi Matthew,

thank you very much for your patch!  Below are some comments.

> From 9d400b025a3d60bc29b58ba0bf1301a418ba7aa2 Mon Sep 17 00:00:00 2001
> From: Matthew Jordan <matthewjordandevops@yandex.com>
> Date: Wed, 18 May 2016 18:20:30 -0400
> Subject: [PATCH] gnu: Add Tmux themepack

> * gnu/packages/tmux.scm: (tmux-themepack): New variable.

The first “:” (after “scm”) should be deleted.

> @@ -21,10 +22,16 @@
>    #:use-module (guix licenses)
>    #:use-module (guix packages)
>    #:use-module (guix download)
> +  #:use-module (guix git-download)
> +  #:use-module (guix build-system trivial)
>    #:use-module (guix build-system gnu)
>    #:use-module (gnu packages)
> +  #:use-module (gnu packages base)
>    #:use-module (gnu packages libevent)
> -  #:use-module (gnu packages ncurses))
> +  #:use-module (gnu packages ncurses)
> +  #:use-module (guix utils)
> +  #:use-module (guix build utils))
> +

This may not be important, but I suggest putting the “use-module”
expressions for “(guix utils)” and “(guix build utils)” further up where
“(guix packages)” is.

> +
> +(define-public tmux-themepack
> +  (package
> +    (name "tmux-themepack")
> +    (version "03a3728") ;; No version tags

Please see “7.6.3 Version Numbers” in the Guix manual.  We use the full
commit (in a let binding) and construct a version string from it.

> +    (source (origin
> +              (method git-fetch)
> +              (uri (git-reference
> +                    (url "https://github.com/jimeh/tmux-themepack.git")
> +                    (commit version)))
> +              (sha256
> +               (base32
> +                "1d3k87mq5lca042jbap5kxskjy3kg79wjhhpnm6jacbn3anc67zl"))))
> +    (build-system trivial-build-system)
> +    (arguments
> +     `(#:modules ((guix build utils))
> +       #:builder (begin
> +                   (use-modules (guix build utils))
> +                   (let* ((out (string-append
> +                                (assoc-ref %outputs "out") "/share/" ,name "-" ,version))
> +                          (env (assoc-ref %build-inputs "coreutils"))
> +                          (in (string-append (assoc-ref %build-inputs "source"))))
> +                     (copy-recursively in ".")
> +                     (substitute* "themepack.tmux"
> +                       (("#!/usr/bin/env") (string-append "#!" env "/bin/env")))
> +                     (mkdir-p out)
> +                     (copy-recursively "." out)))))

I wonder: could you use the gnu-build-system, delete the “configure” and
“build” phases, and override the “install” phase to copy the files?
Instead of running “substitute*” on “themepack.tmux” you can use the
“patch-shebang” procedure.

> +    (propagated-inputs
> +     `(("tmux" ,tmux)
> +       ("coreutils" ,coreutils)))

Why should coreutils be propagated?  And why should tmux be installed
automatically?  I think “tmux” doesn’t even need to be an input at all.

> +    (home-page "https://github.com/jimeh/tmux-themepack")
> +    (synopsis "Pack of various Tmux themes")

How about “Collection of themes for Tmux”.

> +    (description "A pack of various Tmux themes.")

Please add a description with full sentences.

Could you please send an updated patch?

Thank you!

~~ Ricardo
diff mbox

Patch

From 9d400b025a3d60bc29b58ba0bf1301a418ba7aa2 Mon Sep 17 00:00:00 2001
From: Matthew Jordan <matthewjordandevops@yandex.com>
Date: Wed, 18 May 2016 18:20:30 -0400
Subject: [PATCH] gnu: Add Tmux themepack

* gnu/packages/tmux.scm: (tmux-themepack): New variable.
---
 gnu/packages/tmux.scm | 44 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 43 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/tmux.scm b/gnu/packages/tmux.scm
index d89fc16..3617cb4 100644
--- a/gnu/packages/tmux.scm
+++ b/gnu/packages/tmux.scm
@@ -1,6 +1,7 @@ 
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2013 Cyril Roelandt <tipecaml@gmail.com>
 ;;; Copyright © 2016 Efraim Flashner <efraim@flashner.co.il>
+;;; Copyright © 2016 Matthew Jordan <matthewjordandevops@yandex.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -21,10 +22,16 @@ 
   #:use-module (guix licenses)
   #:use-module (guix packages)
   #:use-module (guix download)
+  #:use-module (guix git-download)
+  #:use-module (guix build-system trivial)
   #:use-module (guix build-system gnu)
   #:use-module (gnu packages)
+  #:use-module (gnu packages base)
   #:use-module (gnu packages libevent)
-  #:use-module (gnu packages ncurses))
+  #:use-module (gnu packages ncurses)
+  #:use-module (guix utils)
+  #:use-module (guix build utils))
+
 
 (define-public tmux
   (package
@@ -50,3 +57,38 @@  windows), each running a separate program, to be created, accessed, and
 controlled from a single screen.  tmux may be detached from a screen and
 continue running in the background, then later reattached.")
     (license isc)))
+
+(define-public tmux-themepack
+  (package
+    (name "tmux-themepack")
+    (version "03a3728") ;; No version tags
+    (source (origin
+              (method git-fetch)
+              (uri (git-reference
+                    (url "https://github.com/jimeh/tmux-themepack.git")
+                    (commit version)))
+              (sha256
+               (base32
+                "1d3k87mq5lca042jbap5kxskjy3kg79wjhhpnm6jacbn3anc67zl"))))
+    (build-system trivial-build-system)
+    (arguments
+     `(#:modules ((guix build utils))
+       #:builder (begin
+                   (use-modules (guix build utils))
+                   (let* ((out (string-append
+                                (assoc-ref %outputs "out") "/share/" ,name "-" ,version))
+                          (env (assoc-ref %build-inputs "coreutils"))
+                          (in (string-append (assoc-ref %build-inputs "source"))))
+                     (copy-recursively in ".")
+                     (substitute* "themepack.tmux"
+                       (("#!/usr/bin/env") (string-append "#!" env "/bin/env")))
+                     (mkdir-p out)
+                     (copy-recursively "." out)))))
+    (propagated-inputs
+     `(("tmux" ,tmux)
+       ("coreutils" ,coreutils)))
+    (home-page "https://github.com/jimeh/tmux-themepack")
+    (synopsis "Pack of various Tmux themes")
+    (description "A pack of various Tmux themes.")
+    (license
+     (non-copyleft "http://www.wtfpl.net/txt/copying/"))))
-- 
2.8.3