Re: Patches: Progressive Enhancement

[Alex Sassmannshausen <alex.sassmannshausen@gmail.com>]

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