diff mbox

Adding wc to Bournish

Message ID 20160607074155.GB32264@debian-netbook
State New
Headers show

Commit Message

Efraim Flashner June 7, 2016, 7:41 a.m. UTC
On Sun, Jun 05, 2016 at 10:37:12PM +0200, Ludovic Courtès wrote:
> Efraim Flashner <efraim@flashner.co.il> skribis:
> > +
> > +(define* (wc-command-implementation file #:optional args)
> > +  (let-values (((lines chars)
> > +                (call-with-input-file file lines+chars)))
> > +    (match args
> > +      (#\l
> > +       (format #t "~a ~a~%" lines file))
> > +      (#\c
> > +       (format #t "~a ~a~%" chars file))
> > +      (_
> > +       (format #t "~a ~a ~a~%" lines chars file)))))
> > +
> > +(define (wc-command args . rest)
> > +  (let* ((flags (cond ((string=? args "-l") #\l)
> > +                      ((string=? args "-c") #\c)
> > +                      (else #\nul)))    ; no flags, "args" is a file
> > +         (files (filter (lambda (file)
> > +                          (catch 'system-error
> > +                            (lambda ()
> > +                              (lstat file))
> > +                            (lambda args
> > +                               (let ((errno (system-error-errno args)))
> > +                                (format (current-error-port) "~a: ~a~%"
> > +                                        file (strerror errno))
> > +                                #f))))
> > +                        (if (char=? flags #\nul) (cons args rest) rest))))
> > +    (for-each
> > +      (lambda (file)
> > +        ((@@ (guix build bournish) wc-command-implementation) file flags))
> > +    files)))
> 
> As discussed at
> <https://lists.gnu.org/archive/html/guix-devel/2016-05/msg00782.html>,
> remember that ‘wc-command’ is called by the compiler to generate Scheme
> code from the input shell code.  Thus, it must emit code that does the
> job.  However, here, it does the job directly, at compilation time, and
> emits the result of ‘for-each’ as code.

copied from that email:
Thus, you must similarly distinguish those two stages by providing:

1. A ‘wc-command-implementation’ procedure that implements ‘wc’;

2. A ‘wc-command’ procedure that emits the code that calls
wc-command-implementation’; so something like:

(define (wc-command args)
 `((@@ (guix build bournish) wc-command-implementation) ,@args))

Better yet, ‘wc-command’ could check for the presence of “-l” or “-c” at
compile time and emit a call to the right thing.

> 
> I’ll commit a couple of fixes for bugs I just found and that prevent us
> from doing:
> 
>   (compile "ls" #:from %bournish-language #:to 'scheme).
> 
> This is useful to clearly understand what code is generated from the
> input.
> 
> Ludo’.

I've refactored the code so now in wc-command it checks if theres a flag
or not, and then passes the list of files to wc-command-implementation
to do the actual computation.

Does it make sense to switch it to something like the case-lambda setup
that `ls' uses or is for-each ok?

Comments

Ludovic Courtès June 8, 2016, 3:43 p.m. UTC | #1
Efraim Flashner <efraim@flashner.co.il> skribis:

> On Sun, Jun 05, 2016 at 10:37:12PM +0200, Ludovic Courtès wrote:

[...]

>> I’ll commit a couple of fixes for bugs I just found and that prevent us
>> from doing:
>> 
>>   (compile "ls" #:from %bournish-language #:to 'scheme).

(This should be be ‘read-and-compile’, not ‘compile’.)

Done in f82c58539e1f7b9b864e68ea2ab0c6a17c15fbb5.  Take a look at
tests/bournish.scm for examples of what is expected.

> From ebce5076177314bfd17a53019b3f6b6888762b01 Mon Sep 17 00:00:00 2001
> From: Efraim Flashner <efraim@flashner.co.il>
> Date: Sun, 22 May 2016 14:56:06 +0300
> Subject: [PATCH] bournish: Add `wc' command.
>
> * guix/build/bournish.scm (file-size, wc-c-command, wc-l-command,
> lines+chars, wc-command, wc-command-implementation): New variables.
> (%commands): Add wc command.

[...]

> +(define* (wc-command-implementation filelist #:optional args)

‘files’, not ‘filelist’.

> +  (let ((files (filter (lambda (file)
> +                         (catch 'system-error
> +                           (lambda ()
> +                             (lstat file))
> +                           (lambda args
> +                              (let ((errno (system-error-errno args)))
> +                               (format (current-error-port) "~a: ~a~%"
> +                                       file (strerror errno))
> +                               #f))))

‘stat’ rather than ‘fstat’.

> +    (for-each
> +      (lambda (file)
> +        (let-values (((lines chars)
> +                      (call-with-input-file file lines+chars)))
> +                    (match args
> +                      (#\l
> +                       (format #t "~a ~a~%" lines file))
> +                      (#\c
> +                       (format #t "~a ~a~%" chars file))
> +                      (_
> +                       (format #t "~a ~a ~a~%" lines chars file)))))
> +      files)))

OK.

> +(define (wc-command args . rest)
> +  (let* ((flags (cond ((string=? args "-l") #\l)
> +                      ((string=? args "-c") #\c)
> +                      (else #\nul))))    ; no flags, "args" is a file

I’d rather make it:

  (define (wc-commands . args)
    (cond ((member "-l" args) …)
          ((member "-c" args) …)
          (else …)))

Instead of the #\nul thing, I think it’d be best to have separate
procedures for -l, -c, and the other case.

> +    ((@@ (guix build bournish) wc-command-implementation)
> +     (if (char=? flags #\nul) (cons args rest) rest) flags)))

This is still not emitting code.  :-)  IOW, there should be a quasiquote
here.

You can see that by running:

  (use-modules (system base compile) (guix build bournish))
  (read-and-compile (open-input-string "wc -l foo")
                    #:from %bournish-language #:to 'scheme)

This should return something like:

  `((@ (guix build bournish) wc-l-command-implementation) '("foo"))

Makes sense?  We’re almost done.

Please take a look at
<https://www.gnu.org/software/guix/manual/html_node/Coding-Style.html>
to make the last review super fast.  ;-)

Thank you!

Ludo’.
diff mbox

Patch

diff --git a/guix/build/bournish.scm b/guix/build/bournish.scm
index 1f17e0a..46e6e1c 100644
--- a/guix/build/bournish.scm
+++ b/guix/build/bournish.scm
@@ -1,5 +1,6 @@ 
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2016 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2016 Efraim Flashner <efraim@flashner.co.il>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -25,7 +26,9 @@ 
   #:use-module (ice-9 match)
   #:use-module (ice-9 ftw)
   #:use-module (srfi srfi-1)
+  #:use-module (srfi srfi-11)
   #:use-module (srfi srfi-26)
+  #:use-module (srfi srfi-41)
   #:export (%bournish-language))
 
 ;;; Commentary:
@@ -103,6 +106,64 @@  characters."
        ((@ (guix build utils) dump-port) port (current-output-port))
        *unspecified*)))
 
+(define (file-size file)
+    (stat:size (stat file)))
+
+(define (wc-c-command file)
+  ;; Faster when only `wc -c' is called
+  (file-size file))
+
+(define (wc-l-command file)
+  ;; Faster when only `wc -l' is called
+  (stream-length
+    (stream-filter
+      (lambda (chr)
+        (char=? chr #\newline))
+      (port->stream (open-file file "r")))))
+
+(define (lines+chars port)
+  ;; Return the number of lines and number of chars read from PORT.
+  ;; TODO: Also return the number of words.
+  (let loop ((lines 0) (chars 0))
+    (match (read-char port) ; get the next char ready
+      ((? eof-object?)              ;done!
+       (values lines chars))
+      (#\newline                    ;recurse
+       (loop (1+ lines) (1+ chars)))
+      (_                            ;recurse
+       (loop lines (1+ chars))))))
+
+(define* (wc-command-implementation filelist #:optional args)
+  (let ((files (filter (lambda (file)
+                         (catch 'system-error
+                           (lambda ()
+                             (lstat file))
+                           (lambda args
+                              (let ((errno (system-error-errno args)))
+                               (format (current-error-port) "~a: ~a~%"
+                                       file (strerror errno))
+                               #f))))
+                       filelist)))
+    (for-each
+      (lambda (file)
+        (let-values (((lines chars)
+                      (call-with-input-file file lines+chars)))
+                    (match args
+                      (#\l
+                       (format #t "~a ~a~%" lines file))
+                      (#\c
+                       (format #t "~a ~a~%" chars file))
+                      (_
+                       (format #t "~a ~a ~a~%" lines chars file)))))
+      files)))
+
+(define (wc-command args . rest)
+  (let* ((flags (cond ((string=? args "-l") #\l)
+                      ((string=? args "-c") #\c)
+                      (else #\nul))))    ; no flags, "args" is a file
+    ((@@ (guix build bournish) wc-command-implementation)
+     (if (char=? flags #\nul) (cons args rest) rest) flags)))
+
 (define (help-command . _)
   (display "\
 Hello, this is Bournish, a minimal Bourne-like shell in Guile!
@@ -129,7 +190,8 @@  commands such as 'ls' and 'cd'; it lacks globbing, pipes---everything.\n"))
     ("help"   ,help-command)
     ("ls"     ,ls-command)
     ("which"  ,which-command)
-    ("cat"    ,cat-command)))
+    ("cat"    ,cat-command)
+    ("wc"     ,wc-command)))
 
 (define (read-bournish port env)
   "Read a Bournish expression from PORT, and return the corresponding Scheme