Patches: Trivial (add div), Progressive Enhancement, Externalise CSS/JS

Patches: Trivial (add div), Progressive Enhancement, Externalise CSS/JS

[alex sassmannshausen]

[-- Attachment #1.1: Type: text/plain, Size: 2906 bytes --]

Hello,

Right, thanks for your feedback — and Cyril, for finding an HTML5 validator
that works. The NU validator seems to be well hidden if your starting
point is validator.w3.org.

Please find attached 3 patches. First is trivial, continuing Cyril's
work to make the package-list validate. It simply adds a missing closing
div, just after the footer.

The second patch re-introduces changes to the SXML and JavaScript to
fulfill the criteria of Progressive Enhancement:
- All content is shown when JavaScript is not enabled on GUI browsers.
In addition the patch implements JS that carries out the changes to the
HTML document successively rather than all at the end, which, with the
current size of the page, would cause a 'flicker'.

I have improved the changelog, as well as, I believe, made the functions
that these changes require, clearer.
Ludo, I appreciate set! being evil (maybe not the true extent yet
though…), but I believe it to be necessary on this occasion:
show_hide-grouper collects package description IDs in the local lid list
variable. This is achieved through repeated set!-ing. That list needs to
be cleared after each occasion that show_hide-grouper generates an sxml
JavaScript call to bulk_show_hide, with the collected package-IDs as
formals, to be inserted in the final HTML. bulk_show_hide initially
hides the package descriptions and creates the <a> element that allows
for the package description to be 'expanded' or 'collapsed'.

Currently show_hide-grouper inserts roughly 31 JavaScript calls to
bulk_show hide (1 every 15 package descriptions + 1 at the end to
collect the remaining package descriptions) in the final HTML
document. Hope this makes sense; if you can think of a better way of
doing it please let me know.

The third patch finally moves the CSS and JS into separate files, as
suggested by Ludo. I have implemented this so that by default, the page
simply links to those external files (this allows browsers to cache the
JS and CSS independently from the HTML). This does mean that those 2
additional files (package-list.js and package-list.css) need to be
pushed to the Guix site whenever changes are carried out to the CSS/JS.
The alternative, inlining the CSS and JS in the resulting HTML, which is
I think what was proposed, has been implemented as suggested, but I'm
running into an error — and I'm having difficulty locating it. It seems
to be happenning either during the (cute) evaluation, or the (dump-port)
evaluation. This functionality is currently disabled, and can be enabled
by calling insert-css or insert-js with an optional additional formal
parameter if you want to have a go at testing.

I'd be happy with the latter functionality removed or fixed — in terms
of HTML practice, I think the default, that is linking to external
files, is generally the done thing.

Best Wishes,

Alex

[-- Attachment #1.2: Type: text/html, Size: 3153 bytes --]

[-- Attachment #2: 0001-list-packages-Add-missing-closing-div-after-footer-i.patch --]
[-- Type: application/octet-stream, Size: 1021 bytes --]

From 01d1183d310a404500ecfda2e3bee4c996d580f8 Mon Sep 17 00:00:00 2001
From: Alex Sassmannshausen <alex.sassmannshausen@gmail.com>
Date: Sun, 18 Aug 2013 13:34:05 +0200
Subject: [PATCH 1/3] list-packages: Add missing closing </div> after footer
 include.

* build-aux/list-packages.scm (list-packages): Add missing closing </div>
  after footer include.
---
 build-aux/list-packages.scm |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/build-aux/list-packages.scm b/build-aux/list-packages.scm
index bae25ac..5fbe359 100755
--- a/build-aux/list-packages.scm
+++ b/build-aux/list-packages.scm
@@ -244,7 +244,8 @@ with gnu.org server-side include and all that."
    (format #t "<!--#include virtual=\"/server/banner.html\" -->")
 
    (sxml->xml (packages->sxml packages))
-   (format #t "<!--#include virtual=\"/server/footer.html\" -->
+   (format #t "</div>
+<!--#include virtual=\"/server/footer.html\" -->
 <div id=\"footer\">
 
 <p>Please send general FSF &amp; GNU inquiries to
-- 
1.7.10.4


[-- Attachment #3: 0002-list-packages-Progressive-Enhancement-approach-to-JS.patch --]
[-- Type: application/octet-stream, Size: 5756 bytes --]

From fcbf81a5164e49a385bd1cd28a67e59d868a1cbf Mon Sep 17 00:00:00 2001
From: Alex Sassmannshausen <alex.sassmannshausen@gmail.com>
Date: Sun, 18 Aug 2013 13:43:05 +0200
Subject: [PATCH 2/3] list-packages: Progressive Enhancement approach to JS.

* build-aux/list-packages.scm (package->sxml): Remove <a> element and JS
  function calls. These are now provided through (show_hide-grouper) below.
  Add call to (show_hide-grouper) for every package added to the table.
  (show_hide-grouper): New function.
  (packages->sxml): Add final call to (show_hide-grouper).
  (insert-js): show_hide: add compatibility check, introduce, use thingLink.
               prep: new JS function.
               bulk_show_hide: new JS function.
---
 build-aux/list-packages.scm |   80 ++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 72 insertions(+), 8 deletions(-)

diff --git a/build-aux/list-packages.scm b/build-aux/list-packages.scm
index 5fbe359..161116b 100755
--- a/build-aux/list-packages.scm
+++ b/build-aux/list-packages.scm
@@ -103,13 +103,8 @@ exec guile -l "$0"                              \
                   (title "Link to the Guix package source code"))
                ,(package-name package) " "
                ,(package-version package)))
-        (td (a (@ (href "javascript:void(0)")
-                  (title "show/hide package description")
-                  (onClick ,(format #f "javascript:show_hide('~a')"
-                                    description-id)))
-               ,(package-synopsis package))
-            (div (@ (id ,description-id)
-                    (style "display: none;"))
+        (td (span (strong ,(package-synopsis package)))
+            (div (@ (id ,description-id))
                  ,(match (package-logo (package-name package))
                     ((? string? url)
                      `(img (@ (src ,url)
@@ -122,7 +117,44 @@ exec guile -l "$0"                              \
                  (a (@ (href ,(package-home-page package))
                        (title "Link to the package's website"))
                     ,(package-home-page package))
-                 ,(status package))))))
+                 ,(status package))
+            ,(show_hide-grouper description-id)))))
+
+(define show_hide-grouper
+  (let ((lid '())
+        (group-counter 15))
+    (lambda (id)
+      "Collect GROUP-COUNTER element IDs, then apply them to
+bulk_show_hide. If ID is #f, force the application of collected IDs to
+bulk_show_hide even when the number of IDs is smaller than GROUP-COUNTER."
+      (define (insert-js-call)
+        "Return an sxml call to bulk_show_hide."
+        (define (lid->js-argl)
+          "Parse a Scheme list into a JavaScript style arguments list."
+          (define (helper l)
+            (if (null? l)
+                (begin
+                  (set! lid '()) ; lid, now processed, safe to reset.
+                  "")
+                (string-append ", '" (car l) "'"      ; further args.
+                               (helper (cdr l)))))
+          (string-append "'" (car lid) "'"             ; initial arg.
+                         (helper (cdr lid))))
+        `(script (@ (type "text/javascript"))
+                 ,(format #f "bulk_show_hide(~a)"
+                          (lid->js-argl))))
+      (if id
+          (begin
+            ;; If ID is not false, then we add ID to LID.
+            (set! lid (cons id lid))
+            ;; If LID is now equal to GROUP-COUNTER it is time to insert a
+            ;; call to bulk_show_hide in the HTML.
+            (if (= (length lid) group-counter)
+                (insert-js-call)
+                ""))
+          ;; if ID is false then we force the insert of a call to
+          ;; bulk_show_hide for (< n GROUP-COUNTER) packages in the HTML.
+          (insert-js-call)))))
 
 (define (packages->sxml packages)
   "Return an HTML page as SXML describing PACKAGES."
@@ -146,6 +178,7 @@ exec guile -l "$0"                              \
                (th "Package version")
                (th "Package details"))
            ,@(map package->sxml packages))
+    ,(show_hide-grouper #f)
     (a (@ (href "#intro")
           (title "Back to top.")
           (id "top"))
@@ -210,14 +243,45 @@ color:#fff;
 // license: CC0
 function show_hide(idThing)
 {
+  if(document.getElementById && document.createTextNode) {
     var thing = document.getElementById(idThing);
+    /* Used to change the link text, depending on whether description is
+       collapsed or expanded */
+    var thingLink = thing.previousSibling.lastChild.firstChild;
     if (thing) {
       if (thing.style.display == \"none\") {
         thing.style.display = \"\";
+        thingLink.data = 'Collapse';
       } else {
         thing.style.display = \"none\";
+        thingLink.data = 'Expand';
       }
     }
+  }
+}
+/* Add controllers used for collapse/expansion of package descriptions */
+function prep(idThing)
+{
+  var tdThing = document.getElementById(idThing).parentNode;
+  if (tdThing) {
+    var aThing = tdThing.firstChild.appendChild(document.createElement('a'));
+    aThing.setAttribute('href', 'javascript:void(0)');
+    aThing.setAttribute('title', 'show/hide package description');
+    aThing.appendChild(document.createTextNode('Expand'));
+    aThing.onclick=function(){show_hide(idThing);};
+    /* aThing.onkeypress=function(){show_hide(idThing);}; */
+  }
+}
+/* Take n element IDs, prepare them for javascript enhanced
+display and hide the IDs by default. */
+function bulk_show_hide()
+{
+  if(document.getElementById && document.createTextNode) {
+    for(var i=0; i<arguments.length; i++) {
+      prep(arguments[i])
+      show_hide(arguments[i]);
+    }
+  }
 }
 </script>"))
 
-- 
1.7.10.4


[-- Attachment #4: 0003-list-packages-Externalise-CSS-and-JavaScript.patch --]
[-- Type: application/octet-stream, Size: 9774 bytes --]

From 3c3bfb6dea1b20447ba8bb48ec95a8cc4b556129 Mon Sep 17 00:00:00 2001
From: Alex Sassmannshausen <alex.sassmannshausen@gmail.com>
Date: Sun, 18 Aug 2013 15:23:03 +0200
Subject: [PATCH 3/3] list-packages: Externalise CSS and JavaScript.

* build-aux/list-packages.scm (%load-directory): New variable.
  (%css-file): New variable.
  (%js-file): New variable.
  (insert-css): Rewrite to, by default, generate a <link> element calling the
  external CSS stylesheet. Contains structure for inlining the CSS from the
  external stylesheet.
  (insert-js): Rewrite to, by default, call external JS file, rather than
  inlining it. Contains structure for inlining the JS from the external file.
  (list-packages): generate sxml from insert-css and insert-js.
* package-list.css: New CSS file, with minor tweaks.
* package-list.js: New JS file, identical to last commit.
---
 build-aux/list-packages.scm |  134 +++++++++++--------------------------------
 build-aux/package-list.css  |   81 ++++++++++++++++++++++++++
 build-aux/package-list.js   |   43 ++++++++++++++
 3 files changed, 158 insertions(+), 100 deletions(-)
 create mode 100644 build-aux/package-list.css
 create mode 100644 build-aux/package-list.js

diff --git a/build-aux/list-packages.scm b/build-aux/list-packages.scm
index 161116b..883c32b 100755
--- a/build-aux/list-packages.scm
+++ b/build-aux/list-packages.scm
@@ -27,11 +27,13 @@ exec guile -l "$0"                              \
   #:use-module (guix packages)
   #:use-module (guix licenses)
   #:use-module (guix gnu-maintenance)
+  #:use-module (guix build utils)
   #:use-module (gnu packages)
   #:use-module (sxml simple)
   #:use-module (web uri)
   #:use-module (ice-9 match)
   #:use-module (srfi srfi-1)
+  #:use-module (srfi srfi-26)
   #:export (list-packages))
 
 ;;; Commentary:
@@ -185,105 +187,37 @@ bulk_show_hide even when the number of IDs is smaller than GROUP-COUNTER."
        "^")))
 
 \f
-(define (insert-css)
-  "Return the CSS for the list-packages page."
-  (format #t
-"<style>
-a {transition: all 0.3s}
-div#intro {margin-bottom: 5em}
-div#intro div, div#intro p {padding:0.5em}
-div#intro div {float:left}
-table#packages, table#packages tr, table#packages tbody, table#packages td,
-table#packages th {border: 0px solid black}
-div.package-description {position: relative}
-table#packages tr:nth-child(even) {background-color: #FFF}
-table#packages tr:nth-child(odd) {background-color: #EEE}
-table#packages tr:hover, table#packages tr:focus, table#packages tr:active {background-color: #DDD}
-table#packages tr:first-child, table#packages tr:first-child:hover, table#packages tr:first-child:focus, table#packages tr:first-child:active {
-background-color: #333;
-color: #fff;
-}
-table#packages td
-{
-margin:0px;
-padding:0.2em 0.5em;
-}
-table#packages td:first-child {
-width:10%;
-text-align:center;
-}
-table#packages td:nth-child(2){width:30%;}
-table#packages td:last-child {width:60%}
-img.package-logo {
-float: left;
-padding-right: 1em;
-}
-table#packages span a {float: right}
-a#top {
-position:fixed;
-right:2%;
-bottom:2%;
-font-size:150%;
-background-color:#EEE;
-padding:1.125% 0.75% 0% 0.75%;
-text-decoration:none;
-color:#000;
-border-radius:5px;
-}
-a#top:hover, a#top:focus {
-background-color:#333;
-color:#fff;
-}
-</style>"))
+(define %load-directory
+  (string-append (dirname (current-filename)) "/"))
 
-(define (insert-js)
-  "Return the JavaScript for the list-packages page."
-  (format #t
-"<script type=\"text/javascript\">
-// license: CC0
-function show_hide(idThing)
-{
-  if(document.getElementById && document.createTextNode) {
-    var thing = document.getElementById(idThing);
-    /* Used to change the link text, depending on whether description is
-       collapsed or expanded */
-    var thingLink = thing.previousSibling.lastChild.firstChild;
-    if (thing) {
-      if (thing.style.display == \"none\") {
-        thing.style.display = \"\";
-        thingLink.data = 'Collapse';
-      } else {
-        thing.style.display = \"none\";
-        thingLink.data = 'Expand';
-      }
-    }
-  }
-}
-/* Add controllers used for collapse/expansion of package descriptions */
-function prep(idThing)
-{
-  var tdThing = document.getElementById(idThing).parentNode;
-  if (tdThing) {
-    var aThing = tdThing.firstChild.appendChild(document.createElement('a'));
-    aThing.setAttribute('href', 'javascript:void(0)');
-    aThing.setAttribute('title', 'show/hide package description');
-    aThing.appendChild(document.createTextNode('Expand'));
-    aThing.onclick=function(){show_hide(idThing);};
-    /* aThing.onkeypress=function(){show_hide(idThing);}; */
-  }
-}
-/* Take n element IDs, prepare them for javascript enhanced
-display and hide the IDs by default. */
-function bulk_show_hide()
-{
-  if(document.getElementById && document.createTextNode) {
-    for(var i=0; i<arguments.length; i++) {
-      prep(arguments[i])
-      show_hide(arguments[i]);
-    }
-  }
-}
-</script>"))
+(define %css-file
+  "package-list.css")
+(define %js-file
+  "package-list.js")
+
+(define (insert-css . inline)
+  "Return an sxml link to the CSS file for the list-packages page.  If
+the optional inline argument is present, inline the CSS instead.
+Inlining currently fail with wrong-num-args error."
+  (if (null? inline)
+      `(link (@ (type "text/css")
+		(rel "stylesheet")
+		(href ,%css-file)))
+      `(style (@ (type "text/css"))
+	 ,(with-input-from-file (string-append %load-directory %css-file)
+	    (cute dump-port <> (current-output-port))))))
+
+(define (insert-js . inline)
+  "Return an sxml link to the CSS file for the list-packages page.  If
+the optional inline argument is present, inline the CSS instead.
+Inlining currently fail with wrong-num-args error."
+  (if (null? inline)
+      `(script (@ (type "text/javascript")
+                  (src ,%js-file))
+               " ")
+      `(style (@ (type "text/javascript"))
+	 ,(with-input-from-file (string-append %load-directory %js-file)
+	    (cute dump-port <> (current-output-port))))))
 
 \f
 (define (list-packages . args)
@@ -303,8 +237,8 @@ with gnu.org server-side include and all that."
 <!-- Parent-Version: 1.70 $ -->
 <title>GNU Guix - GNU Distribution - GNU Project</title>
 ")
-   (insert-css)
-   (insert-js)
+   (sxml->xml `(,(insert-css)
+                ,(insert-js)))
    (format #t "<!--#include virtual=\"/server/banner.html\" -->")
 
    (sxml->xml (packages->sxml packages))
diff --git a/build-aux/package-list.css b/build-aux/package-list.css
new file mode 100644
index 0000000..67d423a
--- /dev/null
+++ b/build-aux/package-list.css
@@ -0,0 +1,81 @@
+/* license: CC0 */
+a {
+  transition: all 0.3s;
+}
+
+div#intro {
+  margin-bottom: 2em;
+}
+div#intro div, div#intro p {
+  padding:0.5em;
+}
+div#intro div {
+  float:left;
+}
+div#intro img {
+  float:left;
+  padding:0.75em;
+}
+
+table#packages, table#packages tr, table#packages tbody, table#packages td, table#packages th {
+  border: 0px solid black;
+  clear: both;
+}
+
+div.package-description {
+  position: relative;
+}
+
+table#packages tr:nth-child(even) {
+  background-color: #FFF;
+}
+table#packages tr:nth-child(odd) {
+  background-color: #EEE;
+}
+table#packages tr:hover, table#packages tr:focus, table#packages tr:active {
+  background-color: #DDD;
+}
+table#packages tr:first-child, table#packages tr:first-child:hover, table#packages tr:first-child:focus, table#packages tr:first-child:active {
+  background-color: #333;
+  color: #fff;
+}
+table#packages td
+{
+ margin:0px;
+ padding:0.2em 0.5em;
+}
+table#packages td:first-child {
+width:10%;
+text-align:center;
+}
+table#packages td:nth-child(2){
+  width:30%;
+}
+table#packages td:last-child {
+  width:60%;
+}
+
+img.package-logo {
+float: left;
+padding: 0.75em;
+}
+
+table#packages span a {
+  float: right;
+}
+
+a#top {
+position:fixed;
+right:10px;
+bottom:10px;
+font-size:150%;
+background-color:#EEE;
+padding:10px 7.5px 0 7.5px;
+text-decoration:none;
+color:#000;
+border-radius:5px;
+}
+a#top:hover, a#top:focus {
+background-color:#333;
+color:#fff;
+}
\ No newline at end of file
diff --git a/build-aux/package-list.js b/build-aux/package-list.js
new file mode 100644
index 0000000..8b2971a
--- /dev/null
+++ b/build-aux/package-list.js
@@ -0,0 +1,43 @@
+// license: CC0
+function show_hide(idThing)
+{
+  if(document.getElementById && document.createTextNode) {
+    var thing = document.getElementById(idThing);
+    /* Used to change the link text, depending on whether description is
+       collapsed or expanded */
+    var thingLink = thing.previousSibling.lastChild.firstChild;
+    if (thing) {
+      if (thing.style.display == "none") {
+        thing.style.display = "";
+        thingLink.data = 'Collapse';
+      } else {
+        thing.style.display = "none";
+        thingLink.data = 'Expand';
+      }
+    }
+  }
+}
+/* Add controllers used for collapse/expansion of package descriptions */
+function prep(idThing)
+{
+  var tdThing = document.getElementById(idThing).parentNode;
+  if (tdThing) {
+    var aThing = tdThing.firstChild.appendChild(document.createElement('a'));
+    aThing.setAttribute('href', 'javascript:void(0)');
+    aThing.setAttribute('title', 'show/hide package description');
+    aThing.appendChild(document.createTextNode('Expand'));
+    aThing.onclick=function(){show_hide(idThing);};
+    /* aThing.onkeypress=function(){show_hide(idThing);}; */
+  }
+}
+/* Take n element IDs, prepare them for javascript enhanced
+display and hide the IDs by default. */
+function bulk_show_hide()
+{
+  if(document.getElementById && document.createTextNode) {
+    for(var i=0; i<arguments.length; i++) {
+      prep(arguments[i])
+      show_hide(arguments[i]);
+    }
+  }
+}
-- 
1.7.10.4

Re: Patches: Trivial (add div), Progressive Enhancement, Externalise CSS/JS

[Ludovic Courtès]

alex sassmannshausen <alex.sassmannshausen@gmail.com> skribis:

> The second patch re-introduces changes to the SXML and JavaScript to
> fulfill the criteria of Progressive Enhancement:
> - All content is shown when JavaScript is not enabled on GUI browsers.
> In addition the patch implements JS that carries out the changes to the
> HTML document successively rather than all at the end, which, with the
> current size of the page, would cause a 'flicker'.

Nice.  (What’s Progressive Enhancement?)

> I have improved the changelog, as well as, I believe, made the functions
> that these changes require, clearer.
> Ludo, I appreciate set! being evil (maybe not the true extent yet
> though…), but I believe it to be necessary on this occasion:

More on that below.

> Currently show_hide-grouper inserts roughly 31 JavaScript calls to
> bulk_show hide (1 every 15 package descriptions + 1 at the end to
> collect the remaining package descriptions) in the final HTML
> document. Hope this makes sense; if you can think of a better way of
> doing it please let me know.

I trust you.  :-)

> The third patch finally moves the CSS and JS into separate files, as
> suggested by Ludo. I have implemented this so that by default, the page
> simply links to those external files (this allows browsers to cache the
> JS and CSS independently from the HTML). This does mean that those 2
> additional files (package-list.js and package-list.css) need to be
> pushed to the Guix site whenever changes are carried out to the CSS/JS.

I’d prefer inlining, to simplify web site maintenance.

> The alternative, inlining the CSS and JS in the resulting HTML, which is
> I think what was proposed, has been implemented as suggested, but I'm
> running into an error

Can you send the backtrace that you get, and/or the error message?

> From 01d1183d310a404500ecfda2e3bee4c996d580f8 Mon Sep 17 00:00:00 2001
> From: Alex Sassmannshausen <alex.sassmannshausen@gmail.com>
> Date: Sun, 18 Aug 2013 13:34:05 +0200
> Subject: [PATCH 1/3] list-packages: Add missing closing </div> after footer
>  include.
>
> * build-aux/list-packages.scm (list-packages): Add missing closing </div>
>   after footer include.

Applied.

> From fcbf81a5164e49a385bd1cd28a67e59d868a1cbf Mon Sep 17 00:00:00 2001
> From: Alex Sassmannshausen <alex.sassmannshausen@gmail.com>
> Date: Sun, 18 Aug 2013 13:43:05 +0200
> Subject: [PATCH 2/3] list-packages: Progressive Enhancement approach to JS.
>
> * build-aux/list-packages.scm (package->sxml): Remove <a> element and JS
>   function calls. These are now provided through (show_hide-grouper) below.
>   Add call to (show_hide-grouper) for every package added to the table.
>   (show_hide-grouper): New function.
>   (packages->sxml): Add final call to (show_hide-grouper).
>   (insert-js): show_hide: add compatibility check, introduce, use thingLink.
>                prep: new JS function.
>                bulk_show_hide: new JS function.

[...]

> +        (td (span (strong ,(package-synopsis package)))

Shouldn’t we use CSS instead of <strong>?

> +(define show_hide-grouper

I should have mentioned it: the name of a procedure should describe its
result or computation (when it’s a pure function, like ‘expt’), or its
effect (like ‘display’).  Otherwise it’s harder to tell what it does etc.

> +  (let ((lid '())
> +        (group-counter 15))
> +    (lambda (id)
> +      "Collect GROUP-COUNTER element IDs, then apply them to
> +bulk_show_hide. If ID is #f, force the application of collected IDs to
> +bulk_show_hide even when the number of IDs is smaller than GROUP-COUNTER."
> +      (define (insert-js-call)
> +        "Return an sxml call to bulk_show_hide."
> +        (define (lid->js-argl)
> +          "Parse a Scheme list into a JavaScript style arguments list."
> +          (define (helper l)
> +            (if (null? l)
> +                (begin
> +                  (set! lid '()) ; lid, now processed, safe to reset.
> +                  "")
> +                (string-append ", '" (car l) "'"      ; further args.
> +                               (helper (cdr l)))))
> +          (string-append "'" (car lid) "'"             ; initial arg.
> +                         (helper (cdr lid))))
> +        `(script (@ (type "text/javascript"))
> +                 ,(format #f "bulk_show_hide(~a)"
> +                          (lid->js-argl))))
> +      (if id
> +          (begin
> +            ;; If ID is not false, then we add ID to LID.

But this function still has a single call with #f as its argument.  Can
you remove the parameter and the dead code?  Or am I missing something?

[...]

> +/* Take n element IDs, prepare them for javascript enhanced
> +display and hide the IDs by default. */
> +function bulk_show_hide()

Align ‘display’ with ‘Take’.

> From 3c3bfb6dea1b20447ba8bb48ec95a8cc4b556129 Mon Sep 17 00:00:00 2001
> From: Alex Sassmannshausen <alex.sassmannshausen@gmail.com>
> Date: Sun, 18 Aug 2013 15:23:03 +0200
> Subject: [PATCH 3/3] list-packages: Externalise CSS and JavaScript.
>
> * build-aux/list-packages.scm (%load-directory): New variable.
>   (%css-file): New variable.
>   (%js-file): New variable.
>   (insert-css): Rewrite to, by default, generate a <link> element calling the
>   external CSS stylesheet. Contains structure for inlining the CSS from the
>   external stylesheet.
>   (insert-js): Rewrite to, by default, call external JS file, rather than
>   inlining it. Contains structure for inlining the JS from the external file.
>   (list-packages): generate sxml from insert-css and insert-js.
> * package-list.css: New CSS file, with minor tweaks.
> * package-list.js: New JS file, identical to last commit.

Looks good!  A couple of details:

> +(define %load-directory
> +  (string-append (dirname (current-filename)) "/"))

No need for the trailing slash.

> +(define %css-file
> +  "package-list.css")
> +(define %js-file
> +  "package-list.js")

Should be (string-append %load-directory "/package-list.css"), to make
sure it works regardless of the current working directory.

> +(define (insert-css . inline)
> +  "Return an sxml link to the CSS file for the list-packages page.  If
> +the optional inline argument is present, inline the CSS instead.
> +Inlining currently fail with wrong-num-args error."
> +  (if (null? inline)
> +      `(link (@ (type "text/css")
> +		(rel "stylesheet")
> +		(href ,%css-file)))
> +      `(style (@ (type "text/css"))
> +	 ,(with-input-from-file (string-append %load-directory %css-file)
> +	    (cute dump-port <> (current-output-port))))))

Remove the ‘inline’ parameter, and keep only the second arm of the ‘if’
for now.

> +(define (insert-js . inline)
> +  "Return an sxml link to the CSS file for the list-packages page.  If
> +the optional inline argument is present, inline the CSS instead.
> +Inlining currently fail with wrong-num-args error."
> +  (if (null? inline)
> +      `(script (@ (type "text/javascript")
> +                  (src ,%js-file))
> +               " ")
> +      `(style (@ (type "text/javascript"))
> +	 ,(with-input-from-file (string-append %load-directory %js-file)
> +	    (cute dump-port <> (current-output-port))))))

Ditto.

>  <!-- Parent-Version: 1.70 $ -->
>  <title>GNU Guix - GNU Distribution - GNU Project</title>
>  ")
> -   (insert-css)
> -   (insert-js)
> +   (sxml->xml `(,(insert-css)
> +                ,(insert-js)))

‘insert-css’ and ‘insert-js’ return the unspecified value; instead, they
write to the current output port.

So I guess you rather want to keep the two calls as before, no?

> diff --git a/build-aux/package-list.css b/build-aux/package-list.css
> new file mode 100644
> index 0000000..67d423a
> --- /dev/null
> +++ b/build-aux/package-list.css

Can you indent this file in C-mode in Emacs or something similar?

Thanks for following up!

Ludo’.

Re: Patches: Trivial (add div), Progressive Enhancement, Externalise CSS/JS

[Alex Sassmannshausen]

[-- Attachment #1: Type: text/plain, Size: 3906 bytes --]



Hi,

Sooner or later we will get there! :-)

To make the process easier, I've split the patches into 2 series. For
now, the JavaScript Progressive Enhancement patches.

>>>>> "Ludovic" == Ludovic Courtès <ludo@gnu.org> writes:

 alex sassmannshausen <alex.sassmannshausen@gmail.com>
 > The second patch re-introduces changes to the SXML and JavaScript
 > to fulfill the criteria of Progressive Enhancement: - All content
 > is shown when JavaScript is not enabled on GUI browsers.  In
 > addition the patch implements JS that carries out the changes to
 > the HTML document successively rather than all at the end, which,
 > with the current size of the page, would cause a 'flicker'.

 Ludovic> Nice.  (What’s Progressive Enhancement?)

Progressive Enhancement is a web design/development approach where you
make sure that all functionality is available using a very low baseline
— i.e. no JS, smallest files possible, attempt to take old browsers into
account — and then add functionality as 'icing on the cake'. In this
way, users are (almost) never left behind.

 alex sassmannshausen <alex.sassmannshausen@gmail.com> skribis:
 > The second patch re-introduces changes to the SXML and JavaScript to

 [...]

 > +        (td (span (strong ,(package-synopsis package)))

 Ludovic> Shouldn’t we use CSS instead of <strong>?

You are right. I've now changed this (patch 1 no longer uses strong,
patch 3 implements CSS font-weight heaviness.

 > +(define show_hide-grouper

 Ludovic> I should have mentioned it: the name of a procedure should
 Ludovic> describe its result or computation (when it’s a pure function,
 Ludovic> like ‘expt’), or its effect (like ‘display’).  Otherwise it’s
 Ludovic> harder to tell what it does etc.

In an attempt to do this I have now renamed the JS function
prep_pkg_descs (for prepare_package_descriptions), and the Scheme
function you are discussing here is now called
insert-prep_pkg_descs. Hope that does the trick.

 > +  (let ((lid '())
 > +        (group-counter 15))
 > +    (lambda (id)

 [...]

 > +      (if id
 > +          (begin
 > +            ;; If ID is not false, then we add ID to LID.

 Ludovic> But this function still has a single call with #f as its
 Ludovic> argument.  Can you remove the parameter and the dead code?  Or
 Ludovic> am I missing something?

Actually that function (now called insert-prep_pkg_descs is called in 2
places in the Scheme code, once with #f as argument (in packages->sxml),
and once (at the end) of package->sxml, with each package-id as
argument. The code is not dead, as it is what accumulates up to 15
package-ids, which are then used when prep_pkg_descriptions is inserted
into the HTML table (do a search for insert-prep_pkg_descs, and you
should get three hits, one being the definition.

 [...]

 > +/* Take n element IDs, prepare them for javascript enhanced
 > +display and hide the IDs by default. */
 > +function bulk_show_hide()

 Ludovic> Align ‘display’ with ‘Take’.

Done.

 > From 3c3bfb6dea1b20447ba8bb48ec95a8cc4b556129 Mon Sep 17 00:00:00 2001
 > From: Alex Sassmannshausen <alex.sassmannshausen@gmail.com>

 [...]

This part deals with the splitting of the CSS and JS into separate
source files; I'll address that with a second patch series later (or
tomorrow).

 [...]

 > diff --git a/build-aux/package-list.css b/build-aux/package-list.css
 > new file mode 100644
 > index 0000000..67d423a
 > --- /dev/null
 > +++ b/build-aux/package-list.css

 Ludovic> Can you indent this file in C-mode in Emacs or something
 Ludovic> similar?

This has been done in patch no. 2, if I understood you correctly.

 Ludovic> Thanks for following up!

No problem, thanks for the thorough feedback — let me know if these
patches cause any problems.

Best wishes,

Alex


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-list-packages-Progressive-Enhancement-approach-to-JS.patch --]
[-- Type: text/x-patch, Size: 5742 bytes --]

From 145d2c1dfd05c8c7182df4d6383629bfee070ba9 Mon Sep 17 00:00:00 2001
From: Alex Sassmannshausen <alex.sassmannshausen@gmail.com>
Date: Tue, 20 Aug 2013 00:26:37 +0200
Subject: [PATCH 1/3] list-packages: Progressive Enhancement approach to JS.

* build-aux/list-packages.scm (package->sxml): Remove <a> elements and JS
  function calls. These are created through JS (prep_pkg_descs). Add call to
  insert-prep_pkg_descs for every package in the table.
  (insert-prep_pkg_descs): New function.
  (packages->sxml): Add final call to insert-prep_pkg_descs.
  (insert-js): show_hide: add compatibility check, introduce, use thingLink.
  	       prep: new JS function.
	       bulk_show_hide: new JS function.
---
 build-aux/list-packages.scm |   80 ++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 72 insertions(+), 8 deletions(-)

diff --git a/build-aux/list-packages.scm b/build-aux/list-packages.scm
index 9cb07c1..1964c82 100755
--- a/build-aux/list-packages.scm
+++ b/build-aux/list-packages.scm
@@ -103,13 +103,8 @@ exec guile -l "$0"                              \
                   (title "Link to the Guix package source code"))
                ,(package-name package) " "
                ,(package-version package)))
-        (td (a (@ (href "javascript:void(0)")
-                  (title "show/hide package description")
-                  (onClick ,(format #f "javascript:show_hide('~a')"
-                                    description-id)))
-               ,(package-synopsis package))
-            (div (@ (id ,description-id)
-                    (style "display: none;"))
+        (td (span ,(package-synopsis package))
+            (div (@ (id ,description-id))
                  ,(match (package-logo (package-name package))
                     ((? string? url)
                      `(img (@ (src ,url)
@@ -122,7 +117,44 @@ exec guile -l "$0"                              \
                  (a (@ (href ,(package-home-page package))
                        (title "Link to the package's website"))
                     ,(package-home-page package))
-                 ,(status package))))))
+                 ,(status package))
+            ,(insert-prep_pkg_descs description-id)))))
+
+(define insert-prep_pkg_descs
+  (let ((lid '())
+        (group-counter 15))
+    (lambda (id)
+      "Collect GROUP-COUNTER element IDs, then apply them to
+prep_pkg_descs. If ID is #f, force the application of collected IDs to
+prep_pkg_descs even when the number of IDs is smaller than GROUP-COUNTER."
+      (define (insert-js-call)
+        "Return an sxml call to prep_pkg_descs."
+        (define (lid->js-argl)
+          "Parse a Scheme list into a JavaScript style arguments list."
+          (define (helper l)
+            (if (null? l)
+                (begin
+                  (set! lid '()) ; lid, now processed, safe to reset.
+                  "")
+                (string-append ", '" (car l) "'"      ; further args.
+                               (helper (cdr l)))))
+          (string-append "'" (car lid) "'"             ; initial arg.
+                         (helper (cdr lid))))
+        `(script (@ (type "text/javascript"))
+                 ,(format #f "prep_pkg_descs(~a)"
+                          (lid->js-argl))))
+      (if id
+          (begin
+            ;; If ID is not false, then we add ID to LID.
+            (set! lid (cons id lid))
+            ;; If LID is now equal to GROUP-COUNTER it is time to insert a
+            ;; call to prep_pkg_descs in the HTML.
+            (if (= (length lid) group-counter)
+                (insert-js-call)
+                ""))
+          ;; if ID is false then we force the insert of a call to
+          ;; prep_pkg_descs for (< n GROUP-COUNTER) packages in the HTML.
+          (insert-js-call)))))
 
 (define (packages->sxml packages)
   "Return an HTML page as SXML describing PACKAGES."
@@ -146,6 +178,7 @@ exec guile -l "$0"                              \
                (th "Package version")
                (th "Package details"))
            ,@(map package->sxml packages))
+    ,(insert-prep_pkg_descs #f)
     (a (@ (href "#intro")
           (title "Back to top.")
           (id "top"))
@@ -210,14 +243,45 @@ color:#fff;
 // license: CC0
 function show_hide(idThing)
 {
+  if(document.getElementById && document.createTextNode) {
     var thing = document.getElementById(idThing);
+    /* Used to change the link text, depending on whether description is
+       collapsed or expanded */
+    var thingLink = thing.previousSibling.lastChild.firstChild;
     if (thing) {
       if (thing.style.display == \"none\") {
         thing.style.display = \"\";
+        thingLink.data = 'Collapse';
       } else {
         thing.style.display = \"none\";
+        thingLink.data = 'Expand';
       }
     }
+  }
+}
+/* Add controllers used for collapse/expansion of package descriptions */
+function prep(idThing)
+{
+  var tdThing = document.getElementById(idThing).parentNode;
+  if (tdThing) {
+    var aThing = tdThing.firstChild.appendChild(document.createElement('a'));
+    aThing.setAttribute('href', 'javascript:void(0)');
+    aThing.setAttribute('title', 'show/hide package description');
+    aThing.appendChild(document.createTextNode('Expand'));
+    aThing.onclick=function(){show_hide(idThing);};
+    /* aThing.onkeypress=function(){show_hide(idThing);}; */
+  }
+}
+/* Take n element IDs, prepare them for javascript enhanced
+   display and hide the IDs by default. */
+function prep_pkg_descs()
+{
+  if(document.getElementById && document.createTextNode) {
+    for(var i=0; i<arguments.length; i++) {
+      prep(arguments[i])
+      show_hide(arguments[i]);
+    }
+  }
 }
 </script>"))
 
-- 
1.7.10.4


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-list-packages-Tidy-CSS-in-preparation-for-split-into.patch --]
[-- Type: text/x-patch, Size: 3383 bytes --]

From 5b6e848d26204df0b7a0e3ee6a915ba13f903bb8 Mon Sep 17 00:00:00 2001
From: Alex Sassmannshausen <alex.sassmannshausen@gmail.com>
Date: Tue, 20 Aug 2013 00:37:39 +0200
Subject: [PATCH 2/3] list-packages: Tidy CSS in preparation for split into
 external file.

* build-aux/list-packages.scm (insert-css): Tidy CSS alignment etc.
---
 build-aux/list-packages.scm |   96 ++++++++++++++++++++++++++++---------------
 1 file changed, 62 insertions(+), 34 deletions(-)

diff --git a/build-aux/list-packages.scm b/build-aux/list-packages.scm
index 1964c82..4ef1ce9 100755
--- a/build-aux/list-packages.scm
+++ b/build-aux/list-packages.scm
@@ -189,50 +189,78 @@ prep_pkg_descs even when the number of IDs is smaller than GROUP-COUNTER."
   "Return the CSS for the list-packages page."
   (format #t
 "<style>
-a {transition: all 0.3s}
-div#intro {margin-bottom: 5em}
-div#intro div, div#intro p {padding:0.5em}
-div#intro div {float:left}
-table#packages, table#packages tr, table#packages tbody, table#packages td,
-table#packages th {border: 0px solid black}
-div.package-description {position: relative}
-table#packages tr:nth-child(even) {background-color: #FFF}
-table#packages tr:nth-child(odd) {background-color: #EEE}
-table#packages tr:hover, table#packages tr:focus, table#packages tr:active {background-color: #DDD}
+/* license: CC0 */
+a {
+    transition: all 0.3s;
+}
+div#intro {
+    margin-bottom: 2em;
+}
+div#intro div, div#intro p {
+    padding:0.5em;
+}
+div#intro div {
+    float:left;
+}
+div#intro img {
+    float:left;
+    padding:0.75em;
+}
+table#packages, table#packages tr, table#packages tbody, table#packages td, table#packages th {
+    border: 0px solid black;
+    clear: both;
+}
+div.package-description {
+    position: relative;
+}
+table#packages tr:nth-child(even) {
+    background-color: #FFF;
+}
+table#packages tr:nth-child(odd) {
+    background-color: #EEE;
+}
+table#packages tr:hover, table#packages tr:focus, table#packages tr:active {
+    background-color: #DDD;
+}
 table#packages tr:first-child, table#packages tr:first-child:hover, table#packages tr:first-child:focus, table#packages tr:first-child:active {
-background-color: #333;
-color: #fff;
+    background-color: #333;
+    color: #fff;
 }
-table#packages td
-{
-margin:0px;
-padding:0.2em 0.5em;
+table#packages td {
+    margin:0px;
+    padding:0.2em 0.5em;
 }
 table#packages td:first-child {
-width:10%;
-text-align:center;
+    width:10%;
+    text-align:center;
+}
+table#packages td:nth-child(2) {
+    width:30%;
+}
+table#packages td:last-child {
+    width:60%;
 }
-table#packages td:nth-child(2){width:30%;}
-table#packages td:last-child {width:60%}
 img.package-logo {
-float: left;
-padding-right: 1em;
+    float: left;
+    padding: 0.75em;
+}
+table#packages span a {
+    float: right;
 }
-table#packages span a {float: right}
 a#top {
-position:fixed;
-right:2%;
-bottom:2%;
-font-size:150%;
-background-color:#EEE;
-padding:1.125% 0.75% 0% 0.75%;
-text-decoration:none;
-color:#000;
-border-radius:5px;
+    position:fixed;
+    right:10px;
+    bottom:10px;
+    font-size:150%;
+    background-color:#EEE;
+    padding:10px 7.5px 0 7.5px;
+    text-decoration:none;
+    color:#000;
+    border-radius:5px;
 }
 a#top:hover, a#top:focus {
-background-color:#333;
-color:#fff;
+    background-color:#333;
+    color:#fff;
 }
 </style>"))
 
-- 
1.7.10.4


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0003-list-packages-Improve-CSS-for-legibility-remove-redu.patch --]
[-- Type: text/x-patch, Size: 1140 bytes --]

From be839fada47678b869199760eb351ff8151d2be6 Mon Sep 17 00:00:00 2001
From: Alex Sassmannshausen <alex.sassmannshausen@gmail.com>
Date: Tue, 20 Aug 2013 00:56:37 +0200
Subject: [PATCH 3/3] list-packages: Improve CSS for legibility, remove
 redundant CSS.

* build-aux/list-packages.scm (insert-css): Improve CSS for legibility, remove redundant CSS.
---
 build-aux/list-packages.scm |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/build-aux/list-packages.scm b/build-aux/list-packages.scm
index 4ef1ce9..6303066 100755
--- a/build-aux/list-packages.scm
+++ b/build-aux/list-packages.scm
@@ -210,9 +210,6 @@ table#packages, table#packages tr, table#packages tbody, table#packages td, tabl
     border: 0px solid black;
     clear: both;
 }
-div.package-description {
-    position: relative;
-}
 table#packages tr:nth-child(even) {
     background-color: #FFF;
 }
@@ -244,8 +241,12 @@ img.package-logo {
     float: left;
     padding: 0.75em;
 }
+table#packages span {
+    font-weight: 700;
+}
 table#packages span a {
     float: right;
+    font-weight: 500;
 }
 a#top {
     position:fixed;
-- 
1.7.10.4

Re: Patches: Trivial (add div), Progressive Enhancement, Externalise CSS/JS

[Alex Sassmannshausen]

[-- Attachment #1: Type: text/plain, Size: 602 bytes --]


Hello,

Working on the second patch series, I still run into the problem with
(cute) or (dump-port).

    >> The alternative, inlining the CSS and JS in the resulting HTML,
    >> which is I think what was proposed, has been implemented as
    >> suggested, but I'm running into an error

    Ludovic> Can you send the backtrace that you get, and/or the error
    Ludovic> message?

Please find attached the error I get during compilation, and the patch
to get the code to the state where I'm at (to be applied after the
previous 3 patches I sent tonight).

Any help appreciated.

Best wishes,

Alex


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-list-packages-Externalise-CSS-file.patch --]
[-- Type: text/x-patch, Size: 9892 bytes --]

From d7aaff23102a5ecfe1f6a579b13360385b8f004b Mon Sep 17 00:00:00 2001
From: Alex Sassmannshausen <alex.sassmannshausen@gmail.com>
Date: Tue, 20 Aug 2013 01:38:30 +0200
Subject: [PATCH] list-packages: Externalise CSS file.

* build-aux/list-packages.scm (%load-directory): New variable.
  (%css-file): New variable.
  (%js-file): New variable.
  (insert-css): Read CSS from external file, and return object for sxml->xml.
  (insert-js): Read JS from external file, and return object for sxml->xml.
* build-aux/package-list.css: New CSS file.
* build-aux/package-list.js: New JS file.
---
 build-aux/list-packages.scm |  156 +++++++------------------------------------
 build-aux/package-list.css  |   74 ++++++++++++++++++++
 build-aux/package-list.js   |   43 ++++++++++++
 3 files changed, 142 insertions(+), 131 deletions(-)
 create mode 100644 build-aux/package-list.css
 create mode 100644 build-aux/package-list.js

diff --git a/build-aux/list-packages.scm b/build-aux/list-packages.scm
index 6303066..6ad66d4 100755
--- a/build-aux/list-packages.scm
+++ b/build-aux/list-packages.scm
@@ -27,11 +27,13 @@ exec guile -l "$0"                              \
   #:use-module (guix packages)
   #:use-module (guix licenses)
   #:use-module (guix gnu-maintenance)
+  #:use-module (guix build utils)
   #:use-module (gnu packages)
   #:use-module (sxml simple)
   #:use-module (web uri)
   #:use-module (ice-9 match)
   #:use-module (srfi srfi-1)
+  #:use-module (srfi srfi-26)
   #:export (list-packages))
 
 ;;; Commentary:
@@ -185,134 +187,25 @@ prep_pkg_descs even when the number of IDs is smaller than GROUP-COUNTER."
        "^")))
 
 \f
+(define %load-directory
+  (string-append (dirname (current-filename))))
+
+(define %css-file
+  "/package-list.css")
+(define %js-file
+  "/package-list.js")
+
 (define (insert-css)
-  "Return the CSS for the list-packages page."
-  (format #t
-"<style>
-/* license: CC0 */
-a {
-    transition: all 0.3s;
-}
-div#intro {
-    margin-bottom: 2em;
-}
-div#intro div, div#intro p {
-    padding:0.5em;
-}
-div#intro div {
-    float:left;
-}
-div#intro img {
-    float:left;
-    padding:0.75em;
-}
-table#packages, table#packages tr, table#packages tbody, table#packages td, table#packages th {
-    border: 0px solid black;
-    clear: both;
-}
-table#packages tr:nth-child(even) {
-    background-color: #FFF;
-}
-table#packages tr:nth-child(odd) {
-    background-color: #EEE;
-}
-table#packages tr:hover, table#packages tr:focus, table#packages tr:active {
-    background-color: #DDD;
-}
-table#packages tr:first-child, table#packages tr:first-child:hover, table#packages tr:first-child:focus, table#packages tr:first-child:active {
-    background-color: #333;
-    color: #fff;
-}
-table#packages td {
-    margin:0px;
-    padding:0.2em 0.5em;
-}
-table#packages td:first-child {
-    width:10%;
-    text-align:center;
-}
-table#packages td:nth-child(2) {
-    width:30%;
-}
-table#packages td:last-child {
-    width:60%;
-}
-img.package-logo {
-    float: left;
-    padding: 0.75em;
-}
-table#packages span {
-    font-weight: 700;
-}
-table#packages span a {
-    float: right;
-    font-weight: 500;
-}
-a#top {
-    position:fixed;
-    right:10px;
-    bottom:10px;
-    font-size:150%;
-    background-color:#EEE;
-    padding:10px 7.5px 0 7.5px;
-    text-decoration:none;
-    color:#000;
-    border-radius:5px;
-}
-a#top:hover, a#top:focus {
-    background-color:#333;
-    color:#fff;
-}
-</style>"))
+  "Return inlined CSS, sourced from %css-file."
+  `(style (@ (type "text/css"))
+     ,(with-input-from-file (string-append %load-directory %css-file)
+        (cute dump-port <> (current-output-port)))))
 
 (define (insert-js)
-  "Return the JavaScript for the list-packages page."
-  (format #t
-"<script type=\"text/javascript\">
-// license: CC0
-function show_hide(idThing)
-{
-  if(document.getElementById && document.createTextNode) {
-    var thing = document.getElementById(idThing);
-    /* Used to change the link text, depending on whether description is
-       collapsed or expanded */
-    var thingLink = thing.previousSibling.lastChild.firstChild;
-    if (thing) {
-      if (thing.style.display == \"none\") {
-        thing.style.display = \"\";
-        thingLink.data = 'Collapse';
-      } else {
-        thing.style.display = \"none\";
-        thingLink.data = 'Expand';
-      }
-    }
-  }
-}
-/* Add controllers used for collapse/expansion of package descriptions */
-function prep(idThing)
-{
-  var tdThing = document.getElementById(idThing).parentNode;
-  if (tdThing) {
-    var aThing = tdThing.firstChild.appendChild(document.createElement('a'));
-    aThing.setAttribute('href', 'javascript:void(0)');
-    aThing.setAttribute('title', 'show/hide package description');
-    aThing.appendChild(document.createTextNode('Expand'));
-    aThing.onclick=function(){show_hide(idThing);};
-    /* aThing.onkeypress=function(){show_hide(idThing);}; */
-  }
-}
-/* Take n element IDs, prepare them for javascript enhanced
-   display and hide the IDs by default. */
-function prep_pkg_descs()
-{
-  if(document.getElementById && document.createTextNode) {
-    for(var i=0; i<arguments.length; i++) {
-      prep(arguments[i])
-      show_hide(arguments[i]);
-    }
-  }
-}
-</script>"))
+  "Return inlined JS, sourced from %js-file."
+  `(style (@ (type "text/javascript"))
+     ,(with-input-from-file (string-append %load-directory %js-file)
+        (cute dump-port <> (current-output-port)))))
 
 \f
 (define (list-packages . args)
@@ -328,16 +221,17 @@ with gnu.org server-side include and all that."
   (let ((packages (sort (fold-packages cons '())
                         (lambda (p1 p2)
                           (string<? (package-name p1) (package-name p2))))))
-   (format #t "<!--#include virtual=\"/server/html5-header.html\" -->
+    (format #t "<!--#include virtual=\"/server/html5-header.html\" -->
 <!-- Parent-Version: 1.70 $ -->
 <title>GNU Guix - GNU Distribution - GNU Project</title>
 ")
-   (insert-css)
-   (insert-js)
-   (format #t "<!--#include virtual=\"/server/banner.html\" -->")
+    ;; Inline CSS and JS into the header of the HTML document.
+    (sxml->xml (insert-css))
+    (sxml->xml (insert-js))
+    (format #t "<!--#include virtual=\"/server/banner.html\" -->")
 
-   (sxml->xml (packages->sxml packages))
-   (format #t "</div>
+    (sxml->xml (packages->sxml packages))
+    (format #t "</div>
 <!--#include virtual=\"/server/footer.html\" -->
 <div id=\"footer\">
 
diff --git a/build-aux/package-list.css b/build-aux/package-list.css
new file mode 100644
index 0000000..6596aa6
--- /dev/null
+++ b/build-aux/package-list.css
@@ -0,0 +1,74 @@
+/* license: CC0 */
+a {
+    transition: all 0.3s;
+}
+div#intro {
+    margin-bottom: 2em;
+}
+div#intro div, div#intro p {
+    padding:0.5em;
+}
+div#intro div {
+    float:left;
+}
+div#intro img {
+    float:left;
+    padding:0.75em;
+}
+table#packages, table#packages tr, table#packages tbody, table#packages td, table#packages th {
+    border: 0px solid black;
+    clear: both;
+}
+table#packages tr:nth-child(even) {
+    background-color: #FFF;
+}
+table#packages tr:nth-child(odd) {
+    background-color: #EEE;
+}
+table#packages tr:hover, table#packages tr:focus, table#packages tr:active {
+    background-color: #DDD;
+}
+table#packages tr:first-child, table#packages tr:first-child:hover, table#packages tr:first-child:focus, table#packages tr:first-child:active {
+    background-color: #333;
+    color: #fff;
+}
+table#packages td {
+    margin:0px;
+    padding:0.2em 0.5em;
+}
+table#packages td:first-child {
+    width:10%;
+    text-align:center;
+}
+table#packages td:nth-child(2) {
+    width:30%;
+}
+table#packages td:last-child {
+    width:60%;
+}
+img.package-logo {
+    float: left;
+    padding: 0.75em;
+}
+table#packages span {
+    font-weight: 700;
+}
+table#packages span a {
+    float: right;
+    font-weight: 500;
+}
+a#top {
+    position:fixed;
+    right:10px;
+    bottom:10px;
+    font-size:150%;
+    background-color:#EEE;
+    padding:10px 7.5px 0 7.5px;
+    text-decoration:none;
+    color:#000;
+    border-radius:5px;
+}
+a#top:hover, a#top:focus {
+    background-color:#333;
+    color:#fff;
+}
diff --git a/build-aux/package-list.js b/build-aux/package-list.js
new file mode 100644
index 0000000..8b2971a
--- /dev/null
+++ b/build-aux/package-list.js
@@ -0,0 +1,43 @@
+// license: CC0
+function show_hide(idThing)
+{
+  if(document.getElementById && document.createTextNode) {
+    var thing = document.getElementById(idThing);
+    /* Used to change the link text, depending on whether description is
+       collapsed or expanded */
+    var thingLink = thing.previousSibling.lastChild.firstChild;
+    if (thing) {
+      if (thing.style.display == "none") {
+        thing.style.display = "";
+        thingLink.data = 'Collapse';
+      } else {
+        thing.style.display = "none";
+        thingLink.data = 'Expand';
+      }
+    }
+  }
+}
+/* Add controllers used for collapse/expansion of package descriptions */
+function prep(idThing)
+{
+  var tdThing = document.getElementById(idThing).parentNode;
+  if (tdThing) {
+    var aThing = tdThing.firstChild.appendChild(document.createElement('a'));
+    aThing.setAttribute('href', 'javascript:void(0)');
+    aThing.setAttribute('title', 'show/hide package description');
+    aThing.appendChild(document.createTextNode('Expand'));
+    aThing.onclick=function(){show_hide(idThing);};
+    /* aThing.onkeypress=function(){show_hide(idThing);}; */
+  }
+}
+/* Take n element IDs, prepare them for javascript enhanced
+display and hide the IDs by default. */
+function bulk_show_hide()
+{
+  if(document.getElementById && document.createTextNode) {
+    for(var i=0; i<arguments.length; i++) {
+      prep(arguments[i])
+      show_hide(arguments[i]);
+    }
+  }
+}
-- 
1.7.10.4


[-- Attachment #3: error.log --]
[-- Type: text/plain, Size: 1250 bytes --]

Backtrace:
In ice-9/boot-9.scm:
 157: 11 [catch #t #<catch-closure 915df70> ...]
In unknown file:
   ?: 10 [apply-smob/1 #<catch-closure 915df70>]
In ice-9/boot-9.scm:
  63: 9 [call-with-prompt prompt0 ...]
In ice-9/eval.scm:
 432: 8 [eval # #]
In unknown file:
   ?: 7 [call-with-input-string "(apply (@ (list-packages) list-packages)\n             (cdr (command-line)))" ...]
In ice-9/command-line.scm:
 180: 6 [#<procedure 9163ca0 at ice-9/command-line.scm:175:6 (port)> #<input: string 903da50>]
In unknown file:
   ?: 5 [eval (apply (@ # list-packages) (cdr #)) #<directory (guile-user) 90ec630>]
In build-aux/list-packages.scm:
 229: 4 [list-packages]
 201: 3 [insert-css]
In ice-9/boot-9.scm:
 793: 2 [call-with-input-file "/home/alex/projects/guix/build-aux/package-list.css" ...]
In ice-9/r4rs.scm:
 172: 1 [with-input-from-port #<variable 98b7c40 value: #<input: file /dev/pts/3>> ...]
In build-aux/list-packages.scm:
 202: 0 [#<procedure 993d4b0 at build-aux/list-packages.scm:202:8 (t-839)>]

build-aux/list-packages.scm:202:8: In procedure #<procedure 993d4b0 at build-aux/list-packages.scm:202:8 (t-839)>:
build-aux/list-packages.scm:202:8: Wrong number of arguments to #<procedure 993d4b0 at build-aux/list-packages.scm:202:8 (t-839)>

Patches: Progressive Enhancement

[Alex Sassmannshausen]

[-- Attachment #1: Type: text/plain, Size: 616 bytes --]

Hello,

As discussed at the GHM, please find attached revised patches for
list-packages.

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.

Let me know if you spot any problems. I will have another go at
externalising the CSS and JS after these patches have been approved and
merged.

Best wishes,

Alex


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-list-packages-Tidy-CSS-in-preparation-for-split-into.patch --]
[-- Type: text/x-patch, Size: 3375 bytes --]

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.
---
 build-aux/list-packages.scm |   97 ++++++++++++++++++++++++++++---------------
 1 file changed, 63 insertions(+), 34 deletions(-)

diff --git a/build-aux/list-packages.scm b/build-aux/list-packages.scm
index 9cb07c1..3e798fc 100755
--- a/build-aux/list-packages.scm
+++ b/build-aux/list-packages.scm
@@ -156,50 +156,79 @@ exec guile -l "$0"                              \
   "Return the CSS for the list-packages page."
   (format #t
 "<style>
-a {transition: all 0.3s}
-div#intro {margin-bottom: 5em}
-div#intro div, div#intro p {padding:0.5em}
-div#intro div {float:left}
-table#packages, table#packages tr, table#packages tbody, table#packages td,
-table#packages th {border: 0px solid black}
-div.package-description {position: relative}
-table#packages tr:nth-child(even) {background-color: #FFF}
-table#packages tr:nth-child(odd) {background-color: #EEE}
-table#packages tr:hover, table#packages tr:focus, table#packages tr:active {background-color: #DDD}
+/* license: CC0 */
+a {
+    transition: all 0.3s;
+}
+div#intro {
+    margin-bottom: 2em;
+}
+div#intro div, div#intro p {
+    padding:0.5em;
+}
+div#intro div {
+    float:left;
+}
+div#intro img {
+    float:left;
+    padding:0.75em;
+}
+table#packages, table#packages tr, table#packages tbody, table#packages td, table#packages th {
+    border: 0px solid black;
+    clear: both;
+}
+table#packages tr:nth-child(even) {
+    background-color: #FFF;
+}
+table#packages tr:nth-child(odd) {
+    background-color: #EEE;
+}
+table#packages tr:hover, table#packages tr:focus, table#packages tr:active {
+    background-color: #DDD;
+}
 table#packages tr:first-child, table#packages tr:first-child:hover, table#packages tr:first-child:focus, table#packages tr:first-child:active {
-background-color: #333;
-color: #fff;
+    background-color: #333;
+    color: #fff;
 }
-table#packages td
-{
-margin:0px;
-padding:0.2em 0.5em;
+table#packages td {
+    margin:0px;
+    padding:0.2em 0.5em;
 }
 table#packages td:first-child {
-width:10%;
-text-align:center;
+    width:10%;
+    text-align:center;
+}
+table#packages td:nth-child(2) {
+    width:30%;
+}
+table#packages td:last-child {
+    width:60%;
 }
-table#packages td:nth-child(2){width:30%;}
-table#packages td:last-child {width:60%}
 img.package-logo {
-float: left;
-padding-right: 1em;
+    float: left;
+    padding: 0.75em;
+}
+table#packages span {
+    font-weight: 700;
+}
+table#packages span a {
+    float: right;
+    font-weight: 500;
 }
-table#packages span a {float: right}
 a#top {
-position:fixed;
-right:2%;
-bottom:2%;
-font-size:150%;
-background-color:#EEE;
-padding:1.125% 0.75% 0% 0.75%;
-text-decoration:none;
-color:#000;
-border-radius:5px;
+    position:fixed;
+    right:10px;
+    bottom:10px;
+    font-size:150%;
+    background-color:#EEE;
+    padding:10px 7.5px 0 7.5px;
+    text-decoration:none;
+    color:#000;
+    border-radius:5px;
 }
 a#top:hover, a#top:focus {
-background-color:#333;
-color:#fff;
+    background-color:#333;
+    color:#fff;
 }
 </style>"))
 
-- 
1.7.10.4


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-list-packages-Progressive-Enhancement-approach-to-JS.patch --]
[-- Type: text/x-patch, Size: 8777 bytes --]

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.
  (insert-tr): Moved sxml package table-row generation to new function; remove
  <a> elements and JS function calls. These are created through JS
  (prep_pkg_descs). Add insert-js-call for every 15th package, and the last.
  (insert-js-call): New function.
  (packages->sxml): Change map to fold values; add init params.
  (insert-js): show_hide: add compatibility check, introduce, use thingLink
           prep: new JS function.
           bulk_show_hide: new JS function.
---
 build-aux/list-packages.scm |  133 ++++++++++++++++++++++++++++++++-----------
 1 file changed, 101 insertions(+), 32 deletions(-)

diff --git a/build-aux/list-packages.scm b/build-aux/list-packages.scm
index 3e798fc..b32b595 100755
--- 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."
   (define (source-url package)
     (let ((loc (package-location package)))
       (and loc
@@ -92,37 +95,72 @@ exec guile -l "$0"                              \
     (and=> (lookup-gnu-package name)
            gnu-package-logo))
 
+  (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))))
+
+      `(script (@ (type "text/javascript")) ; Insert JS call
+               ,(format #f "prep_pkg_descs(~a)"
+                        (lid->js-argl))))
+
+    (let ((lid (cons description-id lid)))
+      `(tr (td ,(if (gnu-package? package)
+                    `(img (@ (src "/graphics/gnu-head-mini.png")
+                             (alt "Part of GNU")
+                             (title "Part of GNU")))
+                    ""))
+           (td (a (@ (href ,(source-url package))
+                     (title "Link to the Guix package source code"))
+                  ,(package-name package) " "
+                  ,(package-version package)))
+           (td (span ,(package-synopsis package))
+               (div (@ (id ,description-id))
+                    ,(match (package-logo (package-name package))
+                       ((? string? url)
+                        `(img (@ (src ,url)
+                                 (height "35")
+                                 (class "package-logo")
+                                 (alt ("Logo of " ,(package-name package))))))
+                       (_ #f))
+                    (p ,(package-description package))
+                    ,(license package)
+                    (a (@ (href ,(package-home-page package))
+                          (title "Link to the package's website"))
+                       ,(package-home-page package))
+                    ,(status package)
+                    ,(if js?
+                         (insert-js-call lid)
+                         ""))))))
+
   (let ((description-id (symbol->string
                          (gensym (package-name package)))))
-   `(tr (td ,(if (gnu-package? package)
-                 `(img (@ (src "/graphics/gnu-head-mini.png")
-                          (alt "Part of GNU")
-                          (title "Part of GNU")))
-                 ""))
-        (td (a (@ (href ,(source-url package))
-                  (title "Link to the Guix package source code"))
-               ,(package-name package) " "
-               ,(package-version package)))
-        (td (a (@ (href "javascript:void(0)")
-                  (title "show/hide package description")
-                  (onClick ,(format #f "javascript:show_hide('~a')"
-                                    description-id)))
-               ,(package-synopsis package))
-            (div (@ (id ,description-id)
-                    (style "display: none;"))
-                 ,(match (package-logo (package-name package))
-                    ((? string? url)
-                     `(img (@ (src ,url)
-                              (height "35")
-                              (class "package-logo")
-                              (alt ("Logo of " ,(package-name package))))))
-                    (_ #f))
-                 (p ,(package-description package))
-                 ,(license package)
-                 (a (@ (href ,(package-home-page package))
-                       (title "Link to the package's website"))
-                    ,(package-home-page package))
-                 ,(status package))))))
+    (cond ((= l 1)                               ; Last package in packages
+           (reverse                              ; Fold has reversed packages
+            (cons (insert-tr description-id 'js) ; Only return sxml
+                  previous)))
+          ((= (length lid) 15)          ; Time for a JS call
+           (values
+            (cons (insert-tr description-id 'js) ; Prefix new sxml
+                  previous)
+            '()                         ; Reset lid
+            (1- l)))                    ; Reduce l
+          (else                         ; Insert another row, and build lid
+           (values
+            (cons (insert-tr description-id #f) ; Prefix new sxml
+                  previous)
+            (cons description-id lid)   ; Pass along updated lid
+            (1- l))))))                 ; Reduce l
 
 (define (packages->sxml packages)
   "Return an HTML page as SXML describing PACKAGES."
@@ -145,7 +183,7 @@ exec guile -l "$0"                              \
            (tr (th "GNU?")
                (th "Package version")
                (th "Package details"))
-           ,@(map package->sxml packages))
+           ,@(fold-values package->sxml packages '() '() (length packages)))
     (a (@ (href "#intro")
           (title "Back to top.")
           (id "top"))
@@ -239,14 +277,45 @@ a#top:hover, a#top:focus {
 // license: CC0
 function show_hide(idThing)
 {
+  if(document.getElementById && document.createTextNode) {
     var thing = document.getElementById(idThing);
+    /* Used to change the link text, depending on whether description is
+       collapsed or expanded */
+    var thingLink = thing.previousSibling.lastChild.firstChild;
     if (thing) {
       if (thing.style.display == \"none\") {
         thing.style.display = \"\";
+        thingLink.data = 'Collapse';
       } else {
         thing.style.display = \"none\";
+        thingLink.data = 'Expand';
       }
     }
+  }
+}
+/* Add controllers used for collapse/expansion of package descriptions */
+function prep(idThing)
+{
+  var tdThing = document.getElementById(idThing).parentNode;
+  if (tdThing) {
+    var aThing = tdThing.firstChild.appendChild(document.createElement('a'));
+    aThing.setAttribute('href', 'javascript:void(0)');
+    aThing.setAttribute('title', 'show/hide package description');
+    aThing.appendChild(document.createTextNode('Expand'));
+    aThing.onclick=function(){show_hide(idThing);};
+    /* aThing.onkeypress=function(){show_hide(idThing);}; */
+  }
+}
+/* Take n element IDs, prepare them for javascript enhanced
+   display and hide the IDs by default. */
+function prep_pkg_descs()
+{
+  if(document.getElementById && document.createTextNode) {
+    for(var i=0; i<arguments.length; i++) {
+      prep(arguments[i])
+      show_hide(arguments[i]);
+    }
+  }
 }
 </script>"))
 
-- 
1.7.10.4

Re: Patches: Progressive Enhancement

[Alex Sassmannshausen]

…and the link should of course have been
http://www.atheia.org/guix/package-list.html

Thanks to Amirouche for spotting that.

Alex


>>>>> "Alex" == Alex Sassmannshausen <alex.sassmannshausen@gmail.com> writes:

    Alex> Hello, As discussed at the GHM, please find attached revised
    Alex> patches for list-packages.

    Alex> The first patch tidies the CSS in (insert-css).

    Alex> The second patch uses fold-values from (sxml fold) to achieve
    Alex> what my previous patch did without the use of set!
    Alex> (essentially change the page so that it does not assume
    Alex> JavaScript to display all content elegantly).

    Alex> Output is visible at www.atheia.org/guix/list-packages.html
    Alex> again, and it validates correctly.

    Alex> Let me know if you spot any problems. I will have another go
    Alex> at externalising the CSS and JS after these patches have been
    Alex> approved and merged.

    Alex> Best wishes,

    Alex> Alex

Re: Patches: Progressive Enhancement

[Ludovic Courtès]

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

Re: Patches: Progressive Enhancement

[Alex Sassmannshausen]

[-- Attachment #1: Type: text/plain, Size: 3984 bytes --]

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

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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-list-packages-Progressive-Enhancement-approach-to-JS.patch --]
[-- Type: text/patch, Size: 8954 bytes --]

From 6a043706faf3f2fc0de85b735327859e74a77bce Mon Sep 17 00:00:00 2001
From: Alex Sassmannshausen <alex.sassmannshausen@gmail.com>
Date: Sun, 22 Sep 2013 15:43:23 +0200
Subject: [PATCH] list-packages: Progressive Enhancement approach to JS.

* build-aux/list-packages.scm (package->sxml): Add parameters previous,
  description-ids and remaining, update docstring accordingly. Introduce logic
  for fold-values process.
  (insert-tr): Moved sxml package table-row generation to new function; remove
  <a> elements and JS function calls. These are created through JS
  (prep_pkg_descs). Add insert-js-call for every 15th package, and the last.
  (insert-js-call): New function.
  (packages->sxml): Change map to fold values; add init params.
  (insert-js): show_hide: add compatibility check, introduce, use thingLink
               prep: new JS function.
               bulk_show_hide: new JS function.
---
 build-aux/list-packages.scm |  130 ++++++++++++++++++++++++++++++++-----------
 1 file changed, 98 insertions(+), 32 deletions(-)

diff --git a/build-aux/list-packages.scm b/build-aux/list-packages.scm
index 3e798fc..60c9bc3 100755
--- 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,13 @@ 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 description-ids remaining)
+  "Return 3 values: the HTML-as-SXML for PACKAGE added to all previously
+collected package output in PREVIOUS, a list of DESCRIPTION-IDS and the number
+of packages still to be processed in REMAINING.  Also Introduces a call to the
+JavaScript prep_pkg_descs function as part of the output of PACKAGE, every
+time the length of DESCRIPTION-IDS, increasing, is 15 or when REMAINING,
+decreasing, is 1."
   (define (source-url package)
     (let ((loc (package-location package)))
       (and loc
@@ -92,37 +98,66 @@ exec guile -l "$0"                              \
     (and=> (lookup-gnu-package name)
            gnu-package-logo))
 
+  (define (insert-tr description-id js?)
+    (define (insert-js-call description-ids)
+      "Return an sxml call to prep_pkg_descs, with up to 15 elements of
+description-ids as formal parameters."
+      `(script (@ (type "text/javascript"))
+               ,(format #f "prep_pkg_descs(~a)"
+                        (string-append "'"
+                                       (string-join description-ids "', '")
+                                       "'"))))
+
+    (let ((description-ids (cons description-id description-ids)))
+      `(tr (td ,(if (gnu-package? package)
+                    `(img (@ (src "/graphics/gnu-head-mini.png")
+                             (alt "Part of GNU")
+                             (title "Part of GNU")))
+                    ""))
+           (td (a (@ (href ,(source-url package))
+                     (title "Link to the Guix package source code"))
+                  ,(package-name package) " "
+                  ,(package-version package)))
+           (td (span ,(package-synopsis package))
+               (div (@ (id ,description-id))
+                    ,(match (package-logo (package-name package))
+                       ((? string? url)
+                        `(img (@ (src ,url)
+                                 (height "35")
+                                 (class "package-logo")
+                                 (alt ("Logo of " ,(package-name package))))))
+                       (_ #f))
+                    (p ,(package-description package))
+                    ,(license package)
+                    (a (@ (href ,(package-home-page package))
+                          (title "Link to the package's website"))
+                       ,(package-home-page package))
+                    ,(status package)
+                    ,(if js?
+                         (insert-js-call description-ids)
+                         ""))))))
+
   (let ((description-id (symbol->string
                          (gensym (package-name package)))))
-   `(tr (td ,(if (gnu-package? package)
-                 `(img (@ (src "/graphics/gnu-head-mini.png")
-                          (alt "Part of GNU")
-                          (title "Part of GNU")))
-                 ""))
-        (td (a (@ (href ,(source-url package))
-                  (title "Link to the Guix package source code"))
-               ,(package-name package) " "
-               ,(package-version package)))
-        (td (a (@ (href "javascript:void(0)")
-                  (title "show/hide package description")
-                  (onClick ,(format #f "javascript:show_hide('~a')"
-                                    description-id)))
-               ,(package-synopsis package))
-            (div (@ (id ,description-id)
-                    (style "display: none;"))
-                 ,(match (package-logo (package-name package))
-                    ((? string? url)
-                     `(img (@ (src ,url)
-                              (height "35")
-                              (class "package-logo")
-                              (alt ("Logo of " ,(package-name package))))))
-                    (_ #f))
-                 (p ,(package-description package))
-                 ,(license package)
-                 (a (@ (href ,(package-home-page package))
-                       (title "Link to the package's website"))
-                    ,(package-home-page package))
-                 ,(status package))))))
+    (cond ((= remaining 1)              ; Last package in packages
+           (values
+            (reverse                              ; Fold has reversed packages
+             (cons (insert-tr description-id 'js) ; Prefix final sxml
+                   previous))
+            '()                            ; No more work to do
+            0))                            ; End of the line
+          ((= (length description-ids) 15) ; Time for a JS call
+           (values
+            (cons (insert-tr description-id 'js)
+                  previous)    ; Prefix new sxml
+            '()                ; Reset description-ids
+            (1- remaining)))   ; Reduce remaining
+          (else                ; Insert another row, and build description-ids
+           (values
+            (cons (insert-tr description-id #f)
+                  previous)                       ; Prefix new sxml
+            (cons description-id description-ids) ; Update description-ids
+            (1- remaining))))))                   ; Reduce remaining
 
 (define (packages->sxml packages)
   "Return an HTML page as SXML describing PACKAGES."
@@ -145,7 +180,7 @@ exec guile -l "$0"                              \
            (tr (th "GNU?")
                (th "Package version")
                (th "Package details"))
-           ,@(map package->sxml packages))
+           ,@(fold-values package->sxml packages '() '() (length packages)))
     (a (@ (href "#intro")
           (title "Back to top.")
           (id "top"))
@@ -239,14 +274,45 @@ a#top:hover, a#top:focus {
 // license: CC0
 function show_hide(idThing)
 {
+  if(document.getElementById && document.createTextNode) {
     var thing = document.getElementById(idThing);
+    /* Used to change the link text, depending on whether description is
+       collapsed or expanded */
+    var thingLink = thing.previousSibling.lastChild.firstChild;
     if (thing) {
       if (thing.style.display == \"none\") {
         thing.style.display = \"\";
+        thingLink.data = 'Collapse';
       } else {
         thing.style.display = \"none\";
+        thingLink.data = 'Expand';
       }
     }
+  }
+}
+/* Add controllers used for collapse/expansion of package descriptions */
+function prep(idThing)
+{
+  var tdThing = document.getElementById(idThing).parentNode;
+  if (tdThing) {
+    var aThing = tdThing.firstChild.appendChild(document.createElement('a'));
+    aThing.setAttribute('href', 'javascript:void(0)');
+    aThing.setAttribute('title', 'show/hide package description');
+    aThing.appendChild(document.createTextNode('Expand'));
+    aThing.onclick=function(){show_hide(idThing);};
+    /* aThing.onkeypress=function(){show_hide(idThing);}; */
+  }
+}
+/* Take n element IDs, prepare them for javascript enhanced
+   display and hide the IDs by default. */
+function prep_pkg_descs()
+{
+  if(document.getElementById && document.createTextNode) {
+    for(var i=0; i<arguments.length; i++) {
+      prep(arguments[i])
+      show_hide(arguments[i]);
+    }
+  }
 }
 </script>"))
 
-- 
1.7.10.4

Re: Patches: Progressive Enhancement

[Ludovic Courtès]

Hi Alex!

Alex Sassmannshausen <alex.sassmannshausen@gmail.com> skribis:

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

Of course it’s still desired!  :-)

> Ludovic Courtès writes:

[...]

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

Well, we’ll put it online, and see if anything’s wrong.

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

Great.

‘cut’ is just syntactic sugar for function “currification”:

  http://srfi.schemers.org/srfi-26/srfi-26.html

I’ve applied the patch.  The page at
<http://www.gnu.org/software/guix/package-list.html> has been updated,
so you’re welcome to check for any problems.  It displays correctly in
Conkeror (Gecko) and Emacs-w3m.

Sorry the review has taken so long.  I think it’s a necessary step to
learn to work together and be satisfied with the result; I just hope
it’s not been too discouraging.  Anyway, your help has been and remains
very welcome!  :-)

Thanks,
Ludo’.