[PATCH] gnu: Add beep.

[PATCH] gnu: Add beep.

[José Miguel Sánchez García]

[-- Attachment #1: Type: text/plain, Size: 55 bytes --]

Add beep command.

-- 
José Miguel Sánchez García

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-gnu-Add-beep.patch --]
[-- Type: text/x-diff; name=0001-gnu-Add-beep.patch, Size: 2309 bytes --]

From 62188944da4e42ed46662ca2cc812cced70b0aed Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jos=C3=A9=20Miguel=20S=C3=A1nchez=20Garc=C3=ADa?=
 <jmi2k@openmailbox.org>
Date: Mon, 2 Jan 2017 20:52:24 +0100
Subject: [PATCH] gnu: Add beep.

* gnu/packages/terminals.scm (beep): New variable.
---
 gnu/packages/terminals.scm | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/terminals.scm b/gnu/packages/terminals.scm
index 52767ba6c..641725bda 100644
--- a/gnu/packages/terminals.scm
+++ b/gnu/packages/terminals.scm
@@ -4,7 +4,7 @@
 ;;; Copyright © 2016 Alex Griffin <a@ajgrf.com>
 ;;; Copyright © 2016 David Craven <david@craven.ch>
 ;;; Copyright © 2016 Ludovic Courtès <ludo@gnu.org>
-;;; Copyright © 2016 José Miguel Sánchez García <jmi2k@openmailbox.org>
+;;; Copyright © 2017 José Miguel Sánchez García <jmi2k@openmailbox.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -326,3 +326,29 @@ configuration, testing, and debugging tool.  It has also serves well
 as a low-tech serial communications program to allow access to all
 types of devices that provide serial consoles.")
     (license license:gpl2+)))
+
+(define-public beep
+  (package
+    (name "beep")
+    (version "1.3")
+    (source (origin
+              (method url-fetch)
+              (uri (string-append "http://www.johnath.com/" name "/"
+                                  name "-" version ".tar.gz"))
+              (sha256
+               (base32 "0bgch6jq5cahakk3kbr9549iysf2dik09afixxy5brbxk1xfzb2r"))))
+    (build-system gnu-build-system)
+    (arguments
+     `(#:tests? #f   ; No tests.
+       #:phases
+       (modify-phases %standard-phases
+         (delete 'configure)
+         (add-after 'unpack 'patch-makefile   ; It doesn't install without this
+           (lambda _
+             (mkdir-p (string-append (assoc-ref %outputs "out") "/bin"))
+             (mkdir-p (string-append (assoc-ref %outputs "out") "/man/man1"))
+             (substitute* "Makefile" (("/usr") (assoc-ref %outputs "out"))))))))
+    (synopsis "Command line utility for Linux that beeps the PC speaker.")
+    (description "Command line utility for Linux that beeps the PC speaker.")
+    (home-page "http://www.johnath.com/beep")
+    (license license:gpl2+)))
-- 
2.11.0

Re: [PATCH] gnu: Add beep.

[José Miguel Sánchez García]

[-- Attachment #1: Type: text/plain, Size: 195 bytes --]

On 2017-01-02 20:10, José Miguel Sánchez García wrote:
> Add beep command.
Quick edit: Remove redundant (assoc-ref %outputs "out") with a let 
binding.

-- 
José Miguel Sánchez García

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-gnu-Add-beep.patch --]
[-- Type: text/x-diff; name=0001-gnu-Add-beep.patch, Size: 2257 bytes --]

From 777c91a14817929d37e4cc60a80f3b3f078fe9a4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jos=C3=A9=20Miguel=20S=C3=A1nchez?= <jmi2k@openmailbox.org>
Date: Mon, 2 Jan 2017 20:27:42 +0100
Subject: [PATCH] gnu: Add beep.

* gnu/packages/terminals (beep): New variable.
---
 gnu/packages/terminals.scm | 28 +++++++++++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/terminals.scm b/gnu/packages/terminals.scm
index 52767ba..0bb75a5 100644
--- a/gnu/packages/terminals.scm
+++ b/gnu/packages/terminals.scm
@@ -4,7 +4,7 @@
 ;;; Copyright © 2016 Alex Griffin <a@ajgrf.com>
 ;;; Copyright © 2016 David Craven <david@craven.ch>
 ;;; Copyright © 2016 Ludovic Courtès <ludo@gnu.org>
-;;; Copyright © 2016 José Miguel Sánchez García <jmi2k@openmailbox.org>
+;;; Copyright © 2017 José Miguel Sánchez García <jmi2k@openmailbox.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -326,3 +326,29 @@ configuration, testing, and debugging tool.  It has also serves well
 as a low-tech serial communications program to allow access to all
 types of devices that provide serial consoles.")
     (license license:gpl2+)))
+
+(define-public beep
+  (package
+    (name "beep")
+    (version "1.3")
+    (source (origin
+              (method url-fetch)
+              (uri (string-append "http://www.johnath.com/" name "/"
+                                  name "-" version ".tar.gz"))
+              (sha256
+               (base32 "0bgch6jq5cahakk3kbr9549iysf2dik09afixxy5brbxk1xfzb2r"))))
+    (build-system gnu-build-system)
+    (arguments
+     `(#:tests? #f   ; No tests.
+       #:phases
+       (modify-phases %standard-phases
+         (delete 'configure)
+         (add-after 'unpack 'patch-makefile   ; It doesn't install without this
+           (let ((out (assoc-ref %outputs "out")))
+             (lambda _
+               (mkdir-p (string-append out "/bin"))
+               (mkdir-p (string-append out "/man/man1"))
+               (substitute* "Makefile" (("/usr") out))))))))
+    (synopsis "Command line utility for Linux that beeps the PC speaker.")
+    (description "Command line utility for Linux that beeps the PC speaker.")
+    (home-page "http://www.johnath.com/beep")
-- 
2.10.1.windows.1

Re: [PATCH] gnu: Add beep.

[Tobias Geerinckx-Rice]

[-- Attachment #1.1: Type: text/plain, Size: 2402 bytes --]

José,

On 02/01/17 20:33, José Miguel Sánchez García wrote:
> Add beep command.

Thanks!

> -;;; Copyright © 2016 José Miguel Sánchez García...
> +;;; Copyright © 2017 José Miguel Sánchez García...

Please keep both years:

  ;;; Copyright © 2016, 2017 José Miguel Sánchez García...

> (base32 "0bgch6jq5cahakk3kbr9549iysf2dik09afixxy5brbxk1xfzb2r"))))

A bit long, easily solved by moving the hash string to its own line:

  (base32
   "0bgch6jq5cahakk3kbr9549iysf2dik09afixxy5brbxk1xfzb2r"))))

> ; It doesn't install without this

I'd consider that implied, and just drop this comment.

(Future note: since in-line comments like this one aren't considered
full sentences, they don't start with a capital letter. Full-line
comments beginning with ;; are, and do.)

You're also doing more than patching the makefile here. Consider moving
the ‘mkdir-p’ part to a separate, later phase (add-before 'install).
Sorry to ruin your ‘let’ factoring...

> (synopsis "Command line utility for Linux that beeps the PC speaker.")
> (description "Command line utility for Linux that beeps the PC
>               speaker.")

I'll trust you that this is indeed Linux-specific: the manual mentions
ioctls, and I can't get it to make a sound from within X :-(

s/command line/command-line/ when used as an adjective.

Descriptions should be between 5 and 10 lines where possible, use full
sentences, and not repeat the synopsis ($ info guix "Synopses and
Descriptions").

Luckily, the first paragraph of the man page makes a pretty good
starting point:

  beep allows the user to control the PC speaker with precision,
  allowing different sounds to indicate different events.  While it can
  be run quite happily on the command line, it's intended place of
  residence is within shell/perl scripts, notifying the user when
  something interesting occurs.  Of course, it has  no notion of what's
  interesting, but it's real good at that notifying part.

I'd re-write the synopsis as ‘Linux command-line utility to control the
PC speaker’. It does a tad more than just beep, even if it's the musical
equivalent of burping the alphabet.

> (home-page "http://www.johnath.com/beep")
                                          ^^^

Your package is missing a few brackets. Did the patch get truncated?

Kind regards,

T G-R


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 476 bytes --]

Re: [PATCH] gnu: Add beep.

[José Miguel Sánchez García]

[-- Attachment #1: Type: text/plain, Size: 2708 bytes --]

On 2017-01-04 08:10, Tobias Geerinckx-Rice wrote:
> José,
> 
> On 02/01/17 20:33, José Miguel Sánchez García wrote:
>> Add beep command.
> 
> Thanks!
> 
>> -;;; Copyright © 2016 José Miguel Sánchez García...
>> +;;; Copyright © 2017 José Miguel Sánchez García...
> 
> Please keep both years:
> 
>   ;;; Copyright © 2016, 2017 José Miguel Sánchez García...
> 
>> (base32 "0bgch6jq5cahakk3kbr9549iysf2dik09afixxy5brbxk1xfzb2r"))))
> 
> A bit long, easily solved by moving the hash string to its own line:
> 
>   (base32
>    "0bgch6jq5cahakk3kbr9549iysf2dik09afixxy5brbxk1xfzb2r"))))
> 
>> ; It doesn't install without this
> 
> I'd consider that implied, and just drop this comment.
> 
> (Future note: since in-line comments like this one aren't considered
> full sentences, they don't start with a capital letter. Full-line
> comments beginning with ;; are, and do.)
> 
> You're also doing more than patching the makefile here. Consider moving
> the ‘mkdir-p’ part to a separate, later phase (add-before 'install).
> Sorry to ruin your ‘let’ factoring...
> 
>> (synopsis "Command line utility for Linux that beeps the PC speaker.")
>> (description "Command line utility for Linux that beeps the PC
>>               speaker.")
> 
> I'll trust you that this is indeed Linux-specific: the manual mentions
> ioctls, and I can't get it to make a sound from within X :-(
> 
> s/command line/command-line/ when used as an adjective.
> 
> Descriptions should be between 5 and 10 lines where possible, use full
> sentences, and not repeat the synopsis ($ info guix "Synopses and
> Descriptions").
> 
> Luckily, the first paragraph of the man page makes a pretty good
> starting point:
> 
>   beep allows the user to control the PC speaker with precision,
>   allowing different sounds to indicate different events.  While it can
>   be run quite happily on the command line, it's intended place of
>   residence is within shell/perl scripts, notifying the user when
>   something interesting occurs.  Of course, it has  no notion of what's
>   interesting, but it's real good at that notifying part.
> 
> I'd re-write the synopsis as ‘Linux command-line utility to control the
> PC speaker’. It does a tad more than just beep, even if it's the 
> musical
> equivalent of burping the alphabet.
> 
>> (home-page "http://www.johnath.com/beep")
>                                           ^^^
> 
> Your package is missing a few brackets. Did the patch get truncated?
> 
> Kind regards,
> 
> T G-R
I've applied your changes, ran guix lint and tested it. Here you have 
the fixed patch.

-- 
José Miguel Sánchez García

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-gnu-Add-beep.patch --]
[-- Type: text/x-diff; name=0001-gnu-Add-beep.patch, Size: 2705 bytes --]

From ad6b9c735a45738c798a0c677ebc9e4062e17aef Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jos=C3=A9=20Miguel=20S=C3=A1nchez=20Garc=C3=ADa?=
 <jmi2k@openmailbox.org>
Date: Thu, 5 Jan 2017 00:23:18 +0100
Subject: [PATCH] gnu: Add beep.

* gnu/packages/terminals.scm (beep): New variable.
---
 gnu/packages/terminals.scm | 36 +++++++++++++++++++++++++++++++++++-
 1 file changed, 35 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/terminals.scm b/gnu/packages/terminals.scm
index 52767ba6c..dd4142ac8 100644
--- a/gnu/packages/terminals.scm
+++ b/gnu/packages/terminals.scm
@@ -4,7 +4,7 @@
 ;;; Copyright © 2016 Alex Griffin <a@ajgrf.com>
 ;;; Copyright © 2016 David Craven <david@craven.ch>
 ;;; Copyright © 2016 Ludovic Courtès <ludo@gnu.org>
-;;; Copyright © 2016 José Miguel Sánchez García <jmi2k@openmailbox.org>
+;;; Copyright © 2016, 2017 José Miguel Sánchez García <jmi2k@openmailbox.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -326,3 +326,37 @@ configuration, testing, and debugging tool.  It has also serves well
 as a low-tech serial communications program to allow access to all
 types of devices that provide serial consoles.")
     (license license:gpl2+)))
+
+(define-public beep
+  (package
+    (name "beep")
+    (version "1.3")
+    (source (origin
+              (method url-fetch)
+              (uri (string-append "http://www.johnath.com/" name "/"
+                                  name "-" version ".tar.gz"))
+              (sha256
+               (base32
+                 "0bgch6jq5cahakk3kbr9549iysf2dik09afixxy5brbxk1xfzb2r"))))
+    (build-system gnu-build-system)
+    (arguments
+     `(#:tests? #f   ; no tests.
+       #:phases
+       (modify-phases %standard-phases
+         (delete 'configure)
+         (add-after 'unpack 'patch-makefile
+           (lambda _
+             (substitute* "Makefile" (("/usr") (assoc-ref %outputs "out")))))
+         (add-before 'install 'create-dest-dirs
+           (lambda _
+             (mkdir-p (string-append (assoc-ref %outputs "out") "/bin"))
+             (mkdir-p (string-append (assoc-ref %outputs "out") "/man/man1")))))))
+    (synopsis "Linux command-line utilitu to control the PC speaker")
+    (description "beep allows the user to control the PC speaker with precision,
+allowing different sounds to indicate different events.  While it can be run
+quite happily on the command line, its intended place of residence is within
+shell/perl scripts, notifying the user when something interesting occurs.  Of
+course, it has no notion of what's interesing, but it's real good at that
+notifying part.")
+    (home-page "http://www.johnath.com/beep")
+    (license license:gpl2+)))
-- 
2.11.0

Re: [PATCH] gnu: Add beep.

[Tobias Geerinckx-Rice]

[-- Attachment #1.1: Type: text/plain, Size: 1363 bytes --]

José,

On 04/01/17 23:32, José Miguel Sánchez García wrote:
> I've applied your changes, ran guix lint and tested it. Here you have
> the fixed patch.

Thank you!

Pushed as abe2ec7 with small adjustments below.
I hope that's all right.

Kind regards,

T G-R

---

Fix indentation:
  (base32
-   "0bgch6jq5cahakk3kbr9549iysf2dik09afixxy5brbxk1xfzb2r"))))
+  "0bgch6jq5cahakk3kbr9549iysf2dik09afixxy5brbxk1xfzb2r"))))


Use lambda* instead of %outputs directly, don't abbreviate phase names,
(partially) restore use of let:
-   (lambda _
-     (substitute* "Makefile" (("/usr") (assoc-ref %outputs "out")))))
- (add-before 'install 'create-dest-dirs
-   (lambda _
-     (mkdir-p (string-append (assoc-ref %outputs "out") "/bin"))
-     (mkdir-p (string-append (assoc-ref %outputs "out") "/man/man1")))))))
+   (lambda* (#:key outputs #:allow-other-keys)
+     (substitute* "Makefile" (("/usr") (assoc-ref outputs "out")))))
+ (add-before 'install 'create-output-directories
+   (lambda* (#:key outputs #:allow-other-keys)
+     (let ((out (assoc-ref %outputs "out")))
+       (mkdir-p (string-append out "/bin"))
+       (mkdir-p (string-append out "/man/man1"))))))))

Fix typo:
- (synopsis "Linux command-line utilitu to control the PC speaker")
+ (synopsis "Linux command-line utility to control the PC speaker")


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 476 bytes --]