Re: Adding wc to Bournish

[Efraim Flashner]

On Wed, Jun 08, 2016 at 05:43:09PM +0200, Ludovic Courtès wrote:
> Efraim Flashner <address@hidden> 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 <address@hidden>
> > 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.

-- 
Efraim Flashner   <address@hidden>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

0001-bournish-Add-wc-command.patch
Description: Text document

signature.asc
Description: PGP signature