Hi Alex, Alex Sassmannshausen <[email protected]> skribis:
> The first patch tidies the CSS in (insert-css). > > The second patch uses fold-values from (sxml fold) to achieve what my > previous patch did without the use of set! (essentially change the page > so that it does not assume JavaScript to display all content elegantly). > > Output is visible at www.atheia.org/guix/list-packages.html again, and > it validates correctly. Looks great! > From 361b7aa0eff3681a8b0a47b7139fb7e84d7c8f71 Mon Sep 17 00:00:00 2001 > From: Alex Sassmannshausen <[email protected]> > Date: Mon, 26 Aug 2013 15:53:38 +0200 > Subject: [PATCH 1/2] list-packages: Tidy CSS in preparation for split into > external file. > > * build-aux/list-packages.scm (insert-css): Tidy CSS alignment etc. Applied. > From ed5771c1d5a18c31634ef075d6fe1eee70b32d6b Mon Sep 17 00:00:00 2001 > From: Alex Sassmannshausen <[email protected]> > Date: Mon, 26 Aug 2013 15:55:28 +0200 > Subject: [PATCH 2/2] list-packages: Progressive Enhancement approach to JS. > > * build-aux/list-packages.scm (package->sxml): Modify formal params, > docstring. Introduce logic for fold-values process. Instead of “Modify...”, please write “Add parameters foo and bar.”. > --- a/build-aux/list-packages.scm > +++ b/build-aux/list-packages.scm > @@ -29,6 +29,7 @@ exec guile -l "$0" \ > #:use-module (guix gnu-maintenance) > #:use-module (gnu packages) > #:use-module (sxml simple) > + #:use-module (sxml fold) > #:use-module (web uri) > #:use-module (ice-9 match) > #:use-module (srfi srfi-1) > @@ -48,8 +49,10 @@ exec guile -l "$0" \ > (equal? (gnu-package-name package) name)) > gnu)))) > > -(define (package->sxml package) > - "Return HTML-as-SXML representing PACKAGE." > +(define (package->sxml package previous lid l) > + "Return built HTML-as-SXML representing PACKAGEs, collected in > +PREVIOUS. Include a call to the JavaScript prep_pkg_descs function, every > time > +the length of LID (increasing) is 15 or when L (decreasing) is 1." It should be “Return 3 values: the HTML-as-SXML for PACKAGE, etc.” Also, PREVIOUS is a list of previously processed packages right? That should be mentioned in the docstring. A more descriptive name for ‘lid’ would be ‘description-ids’, and its type should be mentioned in the docstring. ‘l’ could be renamed to ‘remaining’, since it’s a count of remaining packages, IIUC. > + (define (insert-tr description-id js?) > + (define (insert-js-call lid) > + "Return an sxml call to prep_pkg_descs, with up to 15 elements of lid > as > +formal parameters." > + (define (lid->js-argl) > + "Parse a Scheme list into a JavaScript style arguments list." > + (define (helper l) > + (if (null? l) > + "" ; No more args, done with > list. > + (string-append ", '" (car l) "'" ; Append a further arg. > + (helper (cdr l))))) > + > + (string-append "'" (car lid) "'" ; Initial arg. > + (helper (cdr lid)))) This looks nicer with (ice-9 match) pattern matching and ‘string-join’: (define (list->js-argl) (match lid (() ; empty list (is it possible?) "") ((lid ...) (string-append "'" (string-join lid "', '") "'")))) > + (cond ((= l 1) ; Last package in packages > + (reverse ; Fold has reversed packages > + (cons (insert-tr description-id 'js) ; Only return sxml > + previous))) This code path returns a single value, whereas the others return 3. In general it’s better for procedures to always return the same number of values, to avoid confusion. > + ,@(fold-values package->sxml packages '() '() (length packages))) Great that you converted it to functional style. :-) So I think these comments are mostly cosmetic, and then we should be done with this patch. Thanks, Ludo’.
