Re: Patches: Progressive Enhancement

[Alex Sassmannshausen]

Hello again,

It's been a while, but if still desired, here the cosmetic revisions for
the list-packages page.

Ludovic Courtès writes:

>> From ed5771c1d5a18c31634ef075d6fe1eee70b32d6b Mon Sep 17 00:00:00 2001
>> From: Alex Sassmannshausen <address@hidden>
>> 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.”.

Updated the commit message.

>> -(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.

Docstring's now updated.

> A more descriptive name for ‘lid’ would be ‘description-ids’, and its
> type should be mentioned in the docstring.

I've changed lid to description-ids as suggested.

> ‘l’ could be renamed to ‘remaining’, since it’s a count of remaining
> packages, IIUC.

Correct. I've changed the name to remaining.

>> +  (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’:

I've used string-join. (ice-9 match) looks awesome, but I can't seem to
get it to work in this context. Must be lack of experience — string-join
has already made a great improvement, so thanks for the tip.

>   (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.

This path now returns three values (full PREVIOUS, '() for
description-ids and 0 for remaining.

>> +           ,@(fold-values package->sxml packages '() '() (length packages)))
>
> Great that you converted it to functional style.  :-)

Yes, it's much better.

As a slight complication, I no longer have access to my test
environment, so I wasn't able to validate the resulting page at
http://validator.w3.org/nu/. The changes I made should not affect the
output so it should still be valid. I also had a look at the resulting
HTML and it looks alright, but you may want to run it through the
validator.

> Thanks,
> Ludo’.

Thanks for the feedback.

I still intend to at some point split out the js and css into separate
files to be imported through cut(e) — mostly because I want to get to
grips with the mechanism.

Best wishes,

Alex

0001-list-packages-Progressive-Enhancement-approach-to-JS.patch
Description: Text Data