diff mbox

Adding wc to Bournish

Message ID 20160614092713.GA2832@debian-netbook
State New
Headers show

Commit Message

Efraim Flashner June 14, 2016, 9:27 a.m. UTC
On Wed, Jun 08, 2016 at 05:43:09PM +0200, Ludovic Courtès wrote:
> 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.
> 

ok

> > 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’.
> 

ok

> > +  (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’.
> 

ok

> > +    (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.
> 

In the end I went with 3 separate functions for the three commands
(`wc', `wc -l', `wc -c') like you mentioned later.

> > +(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.

This also looks nicer, and I was able to strip the `-l' or `-c' before
passing the ,args to the implementation function.

> 
> > +    ((@@ (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"))
> 

got this one fixed, which in turn broke the implementation code ;) In
the end I went with not a list, so it came out like this:

scheme@(guile-user)> (read-and-compile (open-input-string "wc -l foo bar baz") #:from %bournish-language #:to 'scheme)
$1 = ((@@ (guix build bournish) wc-l-command-implementation) "foo" "bar" "baz")

> 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’.

I've attached what hopefully is the last patch for wc :) I took (and put
somewhere safe) the other code I wrote that does wc-c by calling (stat
file) and the one for wc-l that opens the file as a port and reads for
#\newlines.

Comments

Ricardo Wurmus June 14, 2016, 9:57 a.m. UTC | #1
Efraim Flashner <efraim@flashner.co.il> writes:

> +(define wc-command-implementation
> +  (lambda files
> +    (let ((files (filter (lambda (file)
> +                           (catch 'system-error
> +                             (lambda ()
> +                               (stat file))
> +                             (lambda args
> +                               (let ((errno (system-error-errno args)))
> +                                 (format (current-error-port) "~a: ~a~%"
> +                                         file (strerror errno))
> +                                 #f))))
> +                         files)))
> +      (for-each
> +        (lambda (file)
> +          (let-values (((lines chars)
> +                        (call-with-input-file file lines+chars)))
> +                      (format #t "~a ~a ~a~%" lines chars file)))
> +        files))))
> +
> +(define wc-l-command-implementation
> +  (lambda files
> +    (let ((files (filter (lambda (file)
> +                           (catch 'system-error
> +                             (lambda ()
> +                               (stat file))
> +                             (lambda args
> +                               (let ((errno (system-error-errno args)))
> +                                 (format (current-error-port) "~a: ~a~%"
> +                                         file (strerror errno))
> +                                 #f))))
> +                         files)))
> +      (for-each
> +        (lambda (file)
> +          (let-values (((lines chars)
> +                        (call-with-input-file file lines+chars)))
> +                      (format #t "~a ~a~%" lines file)))
> +        files))))
> +
> +(define wc-c-command-implementation
> +  (lambda files
> +    (let ((files (filter (lambda (file)
> +                           (catch 'system-error
> +                             (lambda ()
> +                               (stat file))
> +                             (lambda args
> +                               (let ((errno (system-error-errno args)))
> +                               (format (current-error-port) "~a: ~a~%"
> +                                       file (strerror errno))
> +                               #f))))
> +                         files)))
> +      (for-each
> +        (lambda (file)
> +          (let-values (((lines chars)
> +                        (call-with-input-file file lines+chars)))
> +                      (format #t "~a ~a~%" chars file)))
> +        files))))

It looks to me that the filter function is the same in all of these
procedures.  Even the actual implementation, i.e. the for-each over the
resulting files is almost exactly the same.

This could be simplified.  If only the format expression differs then
you could abstract this difference away.  You could still have three
different procedures, but they can be the result of evaluating a
higher-order function.

It also seems to me that you could use syntactic sugar to simplify
“(define something (lambda ...))” to “(define (something ...))”.

~~ Ricardo
Efraim Flashner June 14, 2016, 10:20 a.m. UTC | #2
On Tue, Jun 14, 2016 at 11:57:26AM +0200, Ricardo Wurmus wrote:
> 
> Efraim Flashner <efraim@flashner.co.il> writes:
> 
> > +(define wc-command-implementation
> > +  (lambda files
> > +    (let ((files (filter (lambda (file)
> > +                           (catch 'system-error
> > +                             (lambda ()
> > +                               (stat file))
> > +                             (lambda args
> > +                               (let ((errno (system-error-errno args)))
> > +                                 (format (current-error-port) "~a: ~a~%"
> > +                                         file (strerror errno))
> > +                                 #f))))
> > +                         files)))
> > +      (for-each
> > +        (lambda (file)
> > +          (let-values (((lines chars)
> > +                        (call-with-input-file file lines+chars)))
> > +                      (format #t "~a ~a ~a~%" lines chars file)))
> > +        files))))
> > +
> > +(define wc-l-command-implementation
> > +  (lambda files
> > +    (let ((files (filter (lambda (file)
> > +                           (catch 'system-error
> > +                             (lambda ()
> > +                               (stat file))
> > +                             (lambda args
> > +                               (let ((errno (system-error-errno args)))
> > +                                 (format (current-error-port) "~a: ~a~%"
> > +                                         file (strerror errno))
> > +                                 #f))))
> > +                         files)))
> > +      (for-each
> > +        (lambda (file)
> > +          (let-values (((lines chars)
> > +                        (call-with-input-file file lines+chars)))
> > +                      (format #t "~a ~a~%" lines file)))
> > +        files))))
> > +
> > +(define wc-c-command-implementation
> > +  (lambda files
> > +    (let ((files (filter (lambda (file)
> > +                           (catch 'system-error
> > +                             (lambda ()
> > +                               (stat file))
> > +                             (lambda args
> > +                               (let ((errno (system-error-errno args)))
> > +                               (format (current-error-port) "~a: ~a~%"
> > +                                       file (strerror errno))
> > +                               #f))))
> > +                         files)))
> > +      (for-each
> > +        (lambda (file)
> > +          (let-values (((lines chars)
> > +                        (call-with-input-file file lines+chars)))
> > +                      (format #t "~a ~a~%" chars file)))
> > +        files))))
> 
> It looks to me that the filter function is the same in all of these
> procedures.  Even the actual implementation, i.e. the for-each over the
> resulting files is almost exactly the same.
> 
> This could be simplified.  If only the format expression differs then
> you could abstract this difference away.  You could still have three
> different procedures, but they can be the result of evaluating a
> higher-order function.
> 
> It also seems to me that you could use syntactic sugar to simplify
> “(define something (lambda ...))” to “(define (something ...))”.
> 
> ~~ Ricardo

It's already calling `((@@ (guix build bournish)
wc-l-command-implementation) ,@(delete "-l" args)), I could try changing
the ,@(delete part to ,@((@@ (guix build bournish) only-files) ,@(delete
"-l" args)) and then the various implementation functions will be just
printing the results
diff mbox

Patch

diff --git a/guix/build/bournish.scm b/guix/build/bournish.scm
index 1f17e0a..a4c5b83 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,6 +26,7 @@ 
   #: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)
   #:export (%bournish-language))
 
@@ -103,6 +105,80 @@  characters."
        ((@ (guix build utils) dump-port) port (current-output-port))
        *unspecified*)))
 
+(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
+  (lambda files
+    (let ((files (filter (lambda (file)
+                           (catch 'system-error
+                             (lambda ()
+                               (stat file))
+                             (lambda args
+                               (let ((errno (system-error-errno args)))
+                                 (format (current-error-port) "~a: ~a~%"
+                                         file (strerror errno))
+                                 #f))))
+                         files)))
+      (for-each
+        (lambda (file)
+          (let-values (((lines chars)
+                        (call-with-input-file file lines+chars)))
+                      (format #t "~a ~a ~a~%" lines chars file)))
+        files))))
+
+(define wc-l-command-implementation
+  (lambda files
+    (let ((files (filter (lambda (file)
+                           (catch 'system-error
+                             (lambda ()
+                               (stat file))
+                             (lambda args
+                               (let ((errno (system-error-errno args)))
+                                 (format (current-error-port) "~a: ~a~%"
+                                         file (strerror errno))
+                                 #f))))
+                         files)))
+      (for-each
+        (lambda (file)
+          (let-values (((lines chars)
+                        (call-with-input-file file lines+chars)))
+                      (format #t "~a ~a~%" lines file)))
+        files))))
+
+(define wc-c-command-implementation
+  (lambda files
+    (let ((files (filter (lambda (file)
+                           (catch 'system-error
+                             (lambda ()
+                               (stat file))
+                             (lambda args
+                               (let ((errno (system-error-errno args)))
+                               (format (current-error-port) "~a: ~a~%"
+                                       file (strerror errno))
+                               #f))))
+                         files)))
+      (for-each
+        (lambda (file)
+          (let-values (((lines chars)
+                        (call-with-input-file file lines+chars)))
+                      (format #t "~a ~a~%" chars file)))
+        files))))
+
+(define (wc-commands . args)
+  (cond ((member "-l" args) `((@@ (guix build bournish) wc-l-command-implementation) ,@(delete "-l" args)))
+        ((member "-c" args) `((@@ (guix build bournish) wc-c-command-implementation) ,@(delete "-c" args)))
+        (else `((@@ (guix build bournish) wc-command-implementation) ,@args))))
+
 (define (help-command . _)
   (display "\
 Hello, this is Bournish, a minimal Bourne-like shell in Guile!
@@ -129,7 +205,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-commands)))
 
 (define (read-bournish port env)
   "Read a Bournish expression from PORT, and return the corresponding Scheme