Adding wc to Bournish

  On Tue, Jun 14, 2016 at 01:20:29PM +0300, Efraim Flashner wrote:
> 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
> 

It turns out I forgot that calling only-files from wc-commands would
make it evaluate too soon, so I stuck it inside the different
implementations. Factoring out the check if a file exists or not could
also apply to the ls-command-implementation too later.
  

Efraim Flashner <efraim@flashner.co.il> writes:

>> 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
>> 
>
> It turns out I forgot that calling only-files from wc-commands would
> make it evaluate too soon, so I stuck it inside the different
> implementations. Factoring out the check if a file exists or not could
> also apply to the ls-command-implementation too later.

I don’t understand this (but don’t let this delay you).  I don’t see any
“only-files” in your patch.  To delay evaluation you can always wrap
expressions in “(lambda _ ...)”.

I was thinking of a first simplification like this:


(define (existing-files 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))))

(define (wc-print file)
  (let-values (((lines chars)
                (call-with-input-file file lines+chars)))
    (format #t "~a ~a ~a~%" lines chars file)))

(define (wc-l-print file)
  (let-values (((lines chars)
                (call-with-input-file file lines+chars)))
    (format #t "~a ~a~%" lines file)))

(define (wc-c-print file)
  (let-values (((lines chars)
                (call-with-input-file file lines+chars)))
    (format #t "~a ~a~%" chars file)))

(define (wc-command-implementation files)
  (for-each wc-print (filter existing-files files)))

(define (wc-l-command-implementation files)
  (for-each wc-l-print (filter existing-files files)))

(define (wc-c-command-implementation files)
  (for-each wc-c-print (filter existing-files files)))


Even at this point there’s a lot of repetition that we could get rid of,
but maybe that’s too zealous.  Note that I didn’t follow this whole
discussion, so I could be completely off target.

~~ Ricardo
  

Efraim Flashner <efraim@flashner.co.il> skribis:

> From 09eef9cd841a7d212e024be0609168611923696b 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 (lines+chars, only-files, wc-commands,
> wc-command-implementation, wc-l-command-implementation,
> wc-c-command-implementation): New variables.

s/variables/procedures/ :-)

> (%commands): Add wc command.

[...]

> +(define (only-files 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))
> +
> +(define (wc-command-implementation . files)
> +  (for-each
> +    (lambda (file)
> +      (let-values (((lines chars)
> +                    (call-with-input-file file lines+chars)))
> +                  (format #t "~a ~a ~a~%" lines chars file)))
> +    ((@@ (guix build bournish) only-files) files)))

I prefer the approach Ricardo suggested, I think it’s more concise and
clearer:

  https://lists.gnu.org/archive/html/guix-devel/2016-06/msg00525.html

Also, note that @@ would not be needed here; @@ serves to access private
bindings within a specific module:

  https://www.gnu.org/software/guile/manual/html_node/Using-Guile-Modules.html

Last, do not miss the bit about docstrings at:

  https://www.gnu.org/software/guix/manual/html_node/Formatting-Code.html

:-)

With these changes, we’re all set.  Thanks!

From a GSoC viewpoint, I think we must move to the compilation part
now.  Specifically, I think the next step is to start parsing Bash
code.

For that we could use SILex + parse-lalr, but these are not the nicest
tools for the job.  Better tools would be “parsing expression grammars”
(the (ice-9 peg) module in Guile 2.1) or parser combinators, though I
don’t think there’s a directly usable Guile library for that.  Maybe
Eric or David can comment?

The goal is to have a parser that returns an abstract syntax tree (AST)
as an sexp:

  (parse "(cd /foo; ls $HOME) && echo *.a ; echo done")
  =>
  '(sequence
     (success-sequence
       (subshell
         (sequence (command "cd" "/foo")
                   (command "ls" (variable-ref "HOME"))))
       (command "echo" (glob "*.a")))
     (command "echo" "done"))

Thoughts?

Ludo’.