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