From: ludo@gnu.org (Ludovic Courtès)
To: Alex Sassmannshausen <alex.sassmannshausen@gmail.com>
Cc: guix-devel@gnu.org
Subject: Re: Patches: Progressive Enhancement
Date: Wed, 28 Aug 2013 14:34:26 +0200 [thread overview]
Message-ID: <87ppsydmel.fsf@gnu.org> (raw)
In-Reply-To: <87vc2silld.fsf_-_@honeybear.home> (Alex Sassmannshausen's message of "Mon, 26 Aug 2013 16:16:14 +0200")
Hi Alex,
Alex Sassmannshausen <alex.sassmannshausen@gmail.com> 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 <alex.sassmannshausen@gmail.com>
> 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 <alex.sassmannshausen@gmail.com>
> 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’.
next prev parent reply other threads:[~2013-08-28 12:35 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-08-18 12:55 Patches: Trivial (add div), Progressive Enhancement, Externalise CSS/JS alex sassmannshausen
2013-08-18 22:43 ` Ludovic Courtès
2013-08-19 23:21 ` Alex Sassmannshausen
2013-08-19 23:55 ` Alex Sassmannshausen
2013-08-26 14:16 ` Patches: Progressive Enhancement Alex Sassmannshausen
2013-08-26 14:25 ` Alex Sassmannshausen
2013-08-28 12:34 ` Ludovic Courtès [this message]
2013-09-22 14:11 ` Alex Sassmannshausen
2013-09-23 15:54 ` Ludovic Courtès
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87ppsydmel.fsf@gnu.org \
--to=ludo@gnu.org \
--cc=alex.sassmannshausen@gmail.com \
--cc=guix-devel@gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/guix.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.