unofficial mirror of guix-devel@gnu.org 
 help / color / mirror / code / Atom feed
* patch for list-packages
@ 2013-08-04 20:22 Alex Sassmannshausen
  2013-08-11 20:28 ` Alex Sassmannshausen
  0 siblings, 1 reply; 8+ messages in thread
From: Alex Sassmannshausen @ 2013-08-04 20:22 UTC (permalink / raw)
  To: guix-devel

Hello,

Sorry it's taken a while to actually work on this. Been a bit busy, and
had a minor data-loss disaster!

Please find attached a small patch that centralises CSS where possible
and moves the JavaScript into the <head> section.

Next I plan to invert the JavaScript, so it defaults to showing package
descriptions for people with JavaScript disabled, without negatively
affecting JavaScript users' experience. I also want to improve the CSS
and add a table-header and a JavaScript snippet to make the header
follow the top of the window downwards when scrolling down the page
(i.e., set the table-header position to 'fixed').

Hope this all makes sense — and let me know if you have any problems
with the patch.

Best wishes,
Alex


===File
~/projects/guix/0001-list-packages-Centralise-CSS-styling-in-head.patch===
From c3a9ba4635e0af47423391b9777ec64f872bd2ab Mon Sep 17 00:00:00 2001
From: Alex Sassmannshausen <alex.sassmannshausen@gmail.com>
Date: Sun, 4 Aug 2013 21:46:26 +0200
Subject: [PATCH] list-packages: Centralise CSS styling in <head>.

* build-aux/list-packages.scm (package-logo): Assign class of
  'package-description' to package synopsis div; 'package-logo'.  Move inline
  CSS where possible.
  (packages->sxml): Assign id of 'intro' to intro div, 'packages' to the
  table. Move inline CSS.
  (list-packages): Create new <style> section, containing all inline CSS.
  Move JavaScript <script> section to above banner include to place it in
  <head>.
---
 build-aux/list-packages.scm |   29 ++++++++++++++++++++++-------
 1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/build-aux/list-packages.scm b/build-aux/list-packages.scm
index 8d38728..ceadbef 100755
--- a/build-aux/list-packages.scm
+++ b/build-aux/list-packages.scm
@@ -104,12 +104,13 @@ exec guile -l "$0"                              \
                                     description-id)))
                ,(package-synopsis package))
             (div (@ (id ,description-id)
-                    (style "position: relative; display: none;"))
+                    (class "package-description")
+                    (style "display: none;"))
                  ,(match (package-logo (package-name package))
                     ((? string? url)
                      `(img (@ (src ,url)
                               (height "35em")
-                              (style "float: left; padding-right: 1em;"))))
+                              (class "package-logo"))))
                     (_ #f))
                  (p ,(package-description package))
                  ,(license package)
@@ -121,7 +122,7 @@ exec guile -l "$0"                              \
   "Return an HTML page as SXML describing PACKAGES."
   `(div
     (h2 "GNU Guix Package List")
-    (div (@ (style "margin-bottom: 5em;"))
+    (div (@ (id "intro"))
          (div
           (img (@ (src "graphics/guix-logo.small.png")
                   (alt "GNU Guix and the GNU System")
@@ -134,7 +135,7 @@ exec guile -l "$0"                              \
          "Our " (a (@ (href "http://hydra.gnu.org/jobset/gnu/master"))
                    "continuous integration system")
          " shows their current build status.")
-    (table (@ (style "border: none;"))
+    (table (@ (id "packages"))
            ,@(map package->sxml packages))))
 
 \f
@@ -155,8 +156,6 @@ with gnu.org server-side include and all that."
 <!-- Parent-Version: 1.70 $ -->
 
 <title>GNU Guix - GNU Distribution - GNU Project</title>
-<!--#include virtual=\"/server/banner.html\" -->
-
 <script language=\"javascript\" type=\"text/javascript\">
 // license: CC0
 function show_hide(idThing)
@@ -170,7 +169,23 @@ function show_hide(idThing)
     }
   }
 }
-</script>")
+</script>
+<style>
+div#intro {
+margin-bottom: 5em;
+}
+table#packages {
+border: none;
+}
+div.package-description {
+position: relative;
+}
+img.package-logo {
+float: left; padding-right: 1em;
+}
+</style>
+<!--#include virtual=\"/server/banner.html\" -->
+")
    (display (sxml->xml (packages->sxml packages)))
    (format #t "<!--#include virtual=\"/server/footer.html\" -->
 <div id=\"footer\">
-- 
1.7.10.4

============================================================

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: patch for list-packages
  2013-08-04 20:22 patch for list-packages Alex Sassmannshausen
@ 2013-08-11 20:28 ` Alex Sassmannshausen
  2013-08-11 23:38   ` Cyril Roelandt
  0 siblings, 1 reply; 8+ messages in thread
From: Alex Sassmannshausen @ 2013-08-11 20:28 UTC (permalink / raw)
  To: guix-devel

Hello,

I have now been able to finish (in my mind) the work I wanted to carry
out for list-packages.scm's output. I have attached the complete patch
series (3 files, including the first one I sent on 4 August).

As well as the patch series you can visualise what the results of the
final 2 patches are at http://www.atheia.org/guix/output.html (2nd
patch) and http://www.atheia.org/guix/output-final.html (3rd patch).

I've stuck to what I set out to do in my original email. I ended up
generating JavaScript function calls (using <script> elements) in the
final HTML to ensure that the package descriptions would be hidden from
the beginning if JavaScript is enabled (it's a compromise between having
the descriptions hidden from the outset, which is not desirable for
users without JavaScript, but with CSS and hiding package descriptions
after the page has been loaded, which, given the size of the page, would
cause a substantial 'flicker' and a usability issue).

I'm quite happy with the result. Feedback and comments welcome —
especially as I'm still pretty new to the collective work-flow.

Best wishes,

Alex


===File
~/projects/guix/0001-list-packages-Centralise-CSS-styling-in-head.patch===
From c3a9ba4635e0af47423391b9777ec64f872bd2ab Mon Sep 17 00:00:00 2001
From: Alex Sassmannshausen <alex.sassmannshausen@gmail.com>
Date: Sun, 4 Aug 2013 21:46:26 +0200
Subject: [PATCH 1/3] list-packages: Centralise CSS styling in <head>.

* build-aux/list-packages.scm (package-logo): Assign class of
  'package-description' to package synopsis div; 'package-logo'.  Move inline
  CSS where possible.
  (packages->sxml): Assign id of 'intro' to intro div, 'packages' to the
  table. Move inline CSS.
  (list-packages): Create new <style> section, containing all inline CSS.
  Move JavaScript <script> section to above banner include to place it in
  <head>.
---
 build-aux/list-packages.scm |   29 ++++++++++++++++++++++-------
 1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/build-aux/list-packages.scm b/build-aux/list-packages.scm
index 8d38728..ceadbef 100755
--- a/build-aux/list-packages.scm
+++ b/build-aux/list-packages.scm
@@ -104,12 +104,13 @@ exec guile -l "$0"                              \
                                     description-id)))
                ,(package-synopsis package))
             (div (@ (id ,description-id)
-                    (style "position: relative; display: none;"))
+                    (class "package-description")
+                    (style "display: none;"))
                  ,(match (package-logo (package-name package))
                     ((? string? url)
                      `(img (@ (src ,url)
                               (height "35em")
-                              (style "float: left; padding-right: 1em;"))))
+                              (class "package-logo"))))
                     (_ #f))
                  (p ,(package-description package))
                  ,(license package)
@@ -121,7 +122,7 @@ exec guile -l "$0"                              \
   "Return an HTML page as SXML describing PACKAGES."
   `(div
     (h2 "GNU Guix Package List")
-    (div (@ (style "margin-bottom: 5em;"))
+    (div (@ (id "intro"))
          (div
           (img (@ (src "graphics/guix-logo.small.png")
                   (alt "GNU Guix and the GNU System")
@@ -134,7 +135,7 @@ exec guile -l "$0"                              \
          "Our " (a (@ (href "http://hydra.gnu.org/jobset/gnu/master"))
                    "continuous integration system")
          " shows their current build status.")
-    (table (@ (style "border: none;"))
+    (table (@ (id "packages"))
            ,@(map package->sxml packages))))
 
 \f
@@ -155,8 +156,6 @@ with gnu.org server-side include and all that."
 <!-- Parent-Version: 1.70 $ -->
 
 <title>GNU Guix - GNU Distribution - GNU Project</title>
-<!--#include virtual=\"/server/banner.html\" -->
-
 <script language=\"javascript\" type=\"text/javascript\">
 // license: CC0
 function show_hide(idThing)
@@ -170,7 +169,23 @@ function show_hide(idThing)
     }
   }
 }
-</script>")
+</script>
+<style>
+div#intro {
+margin-bottom: 5em;
+}
+table#packages {
+border: none;
+}
+div.package-description {
+position: relative;
+}
+img.package-logo {
+float: left; padding-right: 1em;
+}
+</style>
+<!--#include virtual=\"/server/banner.html\" -->
+")
    (display (sxml->xml (packages->sxml packages)))
    (format #t "<!--#include virtual=\"/server/footer.html\" -->
 <div id=\"footer\">
-- 
1.7.10.4

============================================================

===File
~/projects/guix/0002-list-packages-tidying-tidying-and-refactoring-in-pre.patch===
From 1bc1c348e8a792d3ac7b22bbb82ea9285d03f1ea Mon Sep 17 00:00:00 2001
From: Alex Sassmannshausen <alex.sassmannshausen@gmail.com>
Date: Sun, 11 Aug 2013 19:53:15 +0200
Subject: [PATCH 2/3] list-packages tidying: tidying and refactoring in
 preparation for substantive changes.

* build-aux/list-packages.scm (license package): add title for <a> element.
  (status package): add title for <a> element.
  (package->sxml package): add alt and title for gnu-logo <img> element.
  (package->sxml package): add title to package website <a> element.
  (packages->sxml packages): wrap <div id="intro"> intro paragraph in <p> element.
  (packages->sxml packages): add table header row to <table id="packages">
  (packages->sxml packages): add <a> back to top of the page beneath table.
  (insert-css): create new function returning page's CSS; apply whole load of new CSS.
  (insert-js): create new function returning page's JavaScript.
  (list-packages . args): move JavaScript to (insert-js).
  (list-packages . args): move CSS to (insert-css).
---
 build-aux/list-packages.scm |  148 +++++++++++++++++++++++++++++--------------
 1 file changed, 102 insertions(+), 46 deletions(-)

diff --git a/build-aux/list-packages.scm b/build-aux/list-packages.scm
index ceadbef..b598b97 100755
--- a/build-aux/list-packages.scm
+++ b/build-aux/list-packages.scm
@@ -65,7 +65,8 @@ exec guile -l "$0"                              \
         (let ((uri (license-uri license)))
           (case (and=> (and uri (string->uri uri)) uri-scheme)
             ((http https)
-             `(div (a (@ (href ,uri))
+             `(div (a (@ (href ,uri)
+                         (title "Link to the full license"))
                       ,(license-name license))))
             (else
              `(div ,(license-name license) " ("
@@ -78,7 +79,8 @@ exec guile -l "$0"                              \
     (define (url system)
       `(a (@ (href ,(string-append "http://hydra.gnu.org/job/gnu/master/"
                                    (package-full-name package) "."
-                                   system)))
+                                   system))
+             (title "View the status of this architecture's build at Hydra"))
           ,system))
 
     `(div "status: "
@@ -92,9 +94,12 @@ exec guile -l "$0"                              \
   (let ((description-id (symbol->string
                          (gensym (package-name package)))))
    `(tr (td ,(if (gnu-package? package)
-                 `(img (@ (src "/graphics/gnu-head-mini.png")))
+                 `(img (@ (src "/graphics/gnu-head-mini.png")
+                          (alt "Part of GNU")
+                          (title "Part of GNU")))
                  ""))
-        (td (a (@ (href ,(source-url package)))
+        (td (a (@ (href ,(source-url package))
+                  (title "Link to the Guix package source code"))
                ,(package-name package) " "
                ,(package-version package)))
         (td (@ (colspan "2") (height "0"))
@@ -104,7 +109,6 @@ exec guile -l "$0"                              \
                                     description-id)))
                ,(package-synopsis package))
             (div (@ (id ,description-id)
-                    (class "package-description")
                     (style "display: none;"))
                  ,(match (package-logo (package-name package))
                     ((? string? url)
@@ -114,7 +118,8 @@ exec guile -l "$0"                              \
                     (_ #f))
                  (p ,(package-description package))
                  ,(license package)
-                 (a (@ (href ,(package-home-page package)))
+                 (a (@ (href ,(package-home-page package))
+                       (title "Link to the package's website"))
                     ,(package-home-page package))
                  ,(status package))))))
 
@@ -127,16 +132,93 @@ exec guile -l "$0"                              \
           (img (@ (src "graphics/guix-logo.small.png")
                   (alt "GNU Guix and the GNU System")
                   (height "83em"))))
-         "This web page lists the packages currently provided by the "
-         (a (@ (href "manual/guix.html#GNU-Distribution"))
-            "GNU system distribution")
-         " of "
-         (a (@ (href "/software/guix/guix.html")) "GNU Guix") ".  "
-         "Our " (a (@ (href "http://hydra.gnu.org/jobset/gnu/master"))
-                   "continuous integration system")
-         " shows their current build status.")
+         (p "This web page lists the packages currently provided by the "
+            (a (@ (href "manual/guix.html#GNU-Distribution"))
+               "GNU system distribution")
+            " of "
+            (a (@ (href "/software/guix/guix.html")) "GNU Guix") ".  "
+            "Our " (a (@ (href "http://hydra.gnu.org/jobset/gnu/master"))
+                      "continuous integration system")
+            " shows their current build status."))
     (table (@ (id "packages"))
-           ,@(map package->sxml packages))))
+           (tr (th "GNU?")
+               (th "Package version")
+               (th "Package details"))
+           ,@(map package->sxml packages))
+    (a (@ (href "#intro")
+          (title "Back to top.")
+          (id "top"))
+       "^")))
+
+\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 (insert-js)
+  "Return the JavaScript for the list-packages page."
+  (format #t
+"<script language=\"javascript\" type=\"text/javascript\">
+// license: CC0
+function show_hide(idThing)
+{
+    var thing = document.getElementById(idThing);
+    if (thing) {
+      if (thing.style.display == \"none\") {
+        thing.style.display = \"\";
+      } else {
+        thing.style.display = \"none\";
+      }
+    }
+}
+</script>"))
 
 \f
 (define (list-packages . args)
@@ -154,39 +236,13 @@ with gnu.org server-side include and all that."
                           (string<? (package-name p1) (package-name p2))))))
    (format #t "<!--#include virtual=\"/server/html5-header.html\" -->
 <!-- Parent-Version: 1.70 $ -->
-
 <title>GNU Guix - GNU Distribution - GNU Project</title>
-<script language=\"javascript\" type=\"text/javascript\">
-// license: CC0
-function show_hide(idThing)
-{
-  var thing = document.getElementById(idThing);
-  if (thing) {
-    if (thing.style.display == \"none\") {
-      thing.style.display = \"\";
-    } else {
-      thing.style.display = \"none\";
-    }
-  }
-}
-</script>
-<style>
-div#intro {
-margin-bottom: 5em;
-}
-table#packages {
-border: none;
-}
-div.package-description {
-position: relative;
-}
-img.package-logo {
-float: left; padding-right: 1em;
-}
-</style>
-<!--#include virtual=\"/server/banner.html\" -->
 ")
-   (display (sxml->xml (packages->sxml packages)))
+   (insert-css)
+   (insert-js)
+   (format #t "<!--#include virtual=\"/server/banner.html\" -->")
+
+   (sxml->xml (packages->sxml packages))
    (format #t "<!--#include virtual=\"/server/footer.html\" -->
 <div id=\"footer\">
 
-- 
1.7.10.4

============================================================

===File
~/projects/guix/0003-list-packages.scm-cause-description-to-be-shown-and-.patch===
From 2b9542cfcc180a6b5fc3b0ed28217eb0896c0d2b Mon Sep 17 00:00:00 2001
From: Alex Sassmannshausen <alex.sassmannshausen@gmail.com>
Date: Sun, 11 Aug 2013 21:40:29 +0200
Subject: [PATCH 3/3] list-packages.scm: cause description to be shown, and
 'void' links to not be shown when JS is disabled.

* build-aux/list-packages.scm (package->sxml package): no longer use
  package-synopsis as link as we cannot assume the JavaScript in the link will
  work (no JS safety net); instead use a span.
  (package->sxml package): remove default invisibility of package
  descriptions.
  (package->sxml package): call (show_hide-grouper description-id) after each
  package is processed to collect the package description-ids to be made
  invisible.
  (show_hide-grouper id-or-force): new function; collect n package IDs, and to
  insert a JS call to bulk_show_hide with the package IDs to be processed into
  the xml.
  (packages->sxml packages): call (show_hide-grouper #f) after (map
  package->sxml packages) returns to force the remaining package IDs to be
  processed, even if n have not been reached yet.
  (insert-js): (show_hide(idThing)): add general clause to check whether
  working, DOM conforming JS exists; introduce thingLink variable to allow its
  text to be modified depending on the show/hide package description state of
  a given package.
  (insert-js): (prep(idThing)): new JS function that generates a link in the
  span containing package-synopsis. New link provides show/hide
  package-description functionality.
  (insert-js): (bulk_show_hide()): new JS function that is able to process
  (first call prep, then show_hide on a package-ID) any number of package-IDs
  sequentially (for now defaults to 15, as provided by (show_hide-grouper
  id-or-force)).
---
 build-aux/list-packages.scm |   72 +++++++++++++++++++++++++++++++++++++------
 1 file changed, 63 insertions(+), 9 deletions(-)

diff --git a/build-aux/list-packages.scm b/build-aux/list-packages.scm
index b598b97..2aa6194 100755
--- a/build-aux/list-packages.scm
+++ b/build-aux/list-packages.scm
@@ -102,14 +102,8 @@ exec guile -l "$0"                              \
                   (title "Link to the Guix package source code"))
                ,(package-name package) " "
                ,(package-version package)))
-        (td (@ (colspan "2") (height "0"))
-            (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)
@@ -121,7 +115,35 @@ 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-or-force)
+        "Collect GROUP-COUNTER element IDs, then apply them to
+bulk_show_hide. Pass ID-OR-FORCE #f to apply collected IDs to bulk_show_hide
+even when GROUP-COUNTER IDs have not been collected."
+        (letrec
+            ((lid->js-argl
+              (lambda (l separator)
+                "Parse a list into JavaScript style arguments."
+                (if (null? l)
+                    ""
+                    (string-append separator "'" (car l) "'"
+                                   (lid->js-argl (cdr l) ", "))))))
+          (cond ((or (not id-or-force)
+                     (> (length lid) group-counter))
+                 (let ((t lid))
+                   (if id-or-force
+                       (set! lid (list id-or-force))
+                       (set! lid '()))
+                   `(script (@ (type "text/javascript"))
+                            ,(format #f "bulk_show_hide(~a)"
+                                     (lid->js-argl t "")))))
+                (else (set! lid (cons id-or-force lid))
+                      ""))))))
 
 (define (packages->sxml packages)
   "Return an HTML page as SXML describing PACKAGES."
@@ -145,6 +167,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"))
@@ -209,14 +232,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

============================================================

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: patch for list-packages
  2013-08-11 20:28 ` Alex Sassmannshausen
@ 2013-08-11 23:38   ` Cyril Roelandt
  2013-08-12 15:42     ` Nikita Karetnikov
  0 siblings, 1 reply; 8+ messages in thread
From: Cyril Roelandt @ 2013-08-11 23:38 UTC (permalink / raw)
  To: guix-devel

On 08/11/2013 10:28 PM, Alex Sassmannshausen wrote:
> I'm quite happy with the result. Feedback and comments welcome —
> especially as I'm still pretty new to the collective work-flow.

This looks really nice, except for the "back to top" icon that comes on 
top of the "expand" links (yes, my browser window is not very wide). I 
believe it's a very common issue that should not be treated as a bug 
though :)

I don't think I'll have time to look at the code before Thursday, 
though. Btw, I can't see the patches as attachements, only inlined. Is 
it a problem with my mail client or does everybody witness the same 
behaviour ?

Regards,
Cyril Roelandt.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: patch for list-packages
  2013-08-11 23:38   ` Cyril Roelandt
@ 2013-08-12 15:42     ` Nikita Karetnikov
  2013-08-12 22:33       ` Alex Sassmannshausen
  0 siblings, 1 reply; 8+ messages in thread
From: Nikita Karetnikov @ 2013-08-12 15:42 UTC (permalink / raw)
  To: Cyril Roelandt; +Cc: guix-devel

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

> Btw, I can't see the patches as attachements, only inlined. Is
> it a problem with my mail client or does everybody witness the same
> behaviour ?

I see the same.  They also appear inline in the archives [1].

[1] https://lists.gnu.org/archive/html/guix-devel/2013-08/msg00004.html

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: patch for list-packages
  2013-08-12 15:42     ` Nikita Karetnikov
@ 2013-08-12 22:33       ` Alex Sassmannshausen
  2013-08-15 14:54         ` Ludovic Courtès
  0 siblings, 1 reply; 8+ messages in thread
From: Alex Sassmannshausen @ 2013-08-12 22:33 UTC (permalink / raw)
  To: Nikita Karetnikov; +Cc: guix-devel

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


Hello,

>>>>> Cyril Roelandt <tipecaml@gmail.com> writes:

  > This looks really nice, except for the "back to top" icon that comes
  > on top of the "expand" links (yes, my browser window is not very
  > wide). I believe it's a very common issue that should not be treated
  > as a bug though :)

mhmm, I can see what you mean. I'll see if I can improve this next time
I work on it.

  > I don't think I'll have time to look at the code before Thursday,
  > though.

No problem — thanks for feedback so far.

>>>>> Nikita Karetnikov <nikita@karetnikov.org> writes:

  >> Btw, I can't see the patches as attachements, only inlined. Is it
  >> a problem with my mail client or does everybody witness the same
  >> behaviour ?

  > I see the same.  They also appear inline in the archives [1].

  > [1]
  > https://lists.gnu.org/archive/html/guix-devel/2013-08/msg00004.html

Yes, sorry — still getting used to gnus as my mail client.

I've attached the patches once more (hopefully this time they shouldn't
be inlined).

Best wishes,

Alex


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-list-packages-Centralise-CSS-styling-in-head.patch --]
[-- Type: text/x-diff, Size: 3313 bytes --]

From c3a9ba4635e0af47423391b9777ec64f872bd2ab Mon Sep 17 00:00:00 2001
From: Alex Sassmannshausen <alex.sassmannshausen@gmail.com>
Date: Sun, 4 Aug 2013 21:46:26 +0200
Subject: [PATCH 1/3] list-packages: Centralise CSS styling in <head>.

* build-aux/list-packages.scm (package-logo): Assign class of
  'package-description' to package synopsis div; 'package-logo'.  Move inline
  CSS where possible.
  (packages->sxml): Assign id of 'intro' to intro div, 'packages' to the
  table. Move inline CSS.
  (list-packages): Create new <style> section, containing all inline CSS.
  Move JavaScript <script> section to above banner include to place it in
  <head>.
---
 build-aux/list-packages.scm |   29 ++++++++++++++++++++++-------
 1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/build-aux/list-packages.scm b/build-aux/list-packages.scm
index 8d38728..ceadbef 100755
--- a/build-aux/list-packages.scm
+++ b/build-aux/list-packages.scm
@@ -104,12 +104,13 @@ exec guile -l "$0"                              \
                                     description-id)))
                ,(package-synopsis package))
             (div (@ (id ,description-id)
-                    (style "position: relative; display: none;"))
+                    (class "package-description")
+                    (style "display: none;"))
                  ,(match (package-logo (package-name package))
                     ((? string? url)
                      `(img (@ (src ,url)
                               (height "35em")
-                              (style "float: left; padding-right: 1em;"))))
+                              (class "package-logo"))))
                     (_ #f))
                  (p ,(package-description package))
                  ,(license package)
@@ -121,7 +122,7 @@ exec guile -l "$0"                              \
   "Return an HTML page as SXML describing PACKAGES."
   `(div
     (h2 "GNU Guix Package List")
-    (div (@ (style "margin-bottom: 5em;"))
+    (div (@ (id "intro"))
          (div
           (img (@ (src "graphics/guix-logo.small.png")
                   (alt "GNU Guix and the GNU System")
@@ -134,7 +135,7 @@ exec guile -l "$0"                              \
          "Our " (a (@ (href "http://hydra.gnu.org/jobset/gnu/master"))
                    "continuous integration system")
          " shows their current build status.")
-    (table (@ (style "border: none;"))
+    (table (@ (id "packages"))
            ,@(map package->sxml packages))))
 
 \f
@@ -155,8 +156,6 @@ with gnu.org server-side include and all that."
 <!-- Parent-Version: 1.70 $ -->
 
 <title>GNU Guix - GNU Distribution - GNU Project</title>
-<!--#include virtual=\"/server/banner.html\" -->
-
 <script language=\"javascript\" type=\"text/javascript\">
 // license: CC0
 function show_hide(idThing)
@@ -170,7 +169,23 @@ function show_hide(idThing)
     }
   }
 }
-</script>")
+</script>
+<style>
+div#intro {
+margin-bottom: 5em;
+}
+table#packages {
+border: none;
+}
+div.package-description {
+position: relative;
+}
+img.package-logo {
+float: left; padding-right: 1em;
+}
+</style>
+<!--#include virtual=\"/server/banner.html\" -->
+")
    (display (sxml->xml (packages->sxml packages)))
    (format #t "<!--#include virtual=\"/server/footer.html\" -->
 <div id=\"footer\">
-- 
1.7.10.4


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-list-packages-tidying-tidying-and-refactoring-in-pre.patch --]
[-- Type: text/x-diff, Size: 8383 bytes --]

From 1bc1c348e8a792d3ac7b22bbb82ea9285d03f1ea Mon Sep 17 00:00:00 2001
From: Alex Sassmannshausen <alex.sassmannshausen@gmail.com>
Date: Sun, 11 Aug 2013 19:53:15 +0200
Subject: [PATCH 2/3] list-packages tidying: tidying and refactoring in
 preparation for substantive changes.

* build-aux/list-packages.scm (license package): add title for <a> element.
  (status package): add title for <a> element.
  (package->sxml package): add alt and title for gnu-logo <img> element.
  (package->sxml package): add title to package website <a> element.
  (packages->sxml packages): wrap <div id="intro"> intro paragraph in <p> element.
  (packages->sxml packages): add table header row to <table id="packages">
  (packages->sxml packages): add <a> back to top of the page beneath table.
  (insert-css): create new function returning page's CSS; apply whole load of new CSS.
  (insert-js): create new function returning page's JavaScript.
  (list-packages . args): move JavaScript to (insert-js).
  (list-packages . args): move CSS to (insert-css).
---
 build-aux/list-packages.scm |  148 +++++++++++++++++++++++++++++--------------
 1 file changed, 102 insertions(+), 46 deletions(-)

diff --git a/build-aux/list-packages.scm b/build-aux/list-packages.scm
index ceadbef..b598b97 100755
--- a/build-aux/list-packages.scm
+++ b/build-aux/list-packages.scm
@@ -65,7 +65,8 @@ exec guile -l "$0"                              \
         (let ((uri (license-uri license)))
           (case (and=> (and uri (string->uri uri)) uri-scheme)
             ((http https)
-             `(div (a (@ (href ,uri))
+             `(div (a (@ (href ,uri)
+                         (title "Link to the full license"))
                       ,(license-name license))))
             (else
              `(div ,(license-name license) " ("
@@ -78,7 +79,8 @@ exec guile -l "$0"                              \
     (define (url system)
       `(a (@ (href ,(string-append "http://hydra.gnu.org/job/gnu/master/"
                                    (package-full-name package) "."
-                                   system)))
+                                   system))
+             (title "View the status of this architecture's build at Hydra"))
           ,system))
 
     `(div "status: "
@@ -92,9 +94,12 @@ exec guile -l "$0"                              \
   (let ((description-id (symbol->string
                          (gensym (package-name package)))))
    `(tr (td ,(if (gnu-package? package)
-                 `(img (@ (src "/graphics/gnu-head-mini.png")))
+                 `(img (@ (src "/graphics/gnu-head-mini.png")
+                          (alt "Part of GNU")
+                          (title "Part of GNU")))
                  ""))
-        (td (a (@ (href ,(source-url package)))
+        (td (a (@ (href ,(source-url package))
+                  (title "Link to the Guix package source code"))
                ,(package-name package) " "
                ,(package-version package)))
         (td (@ (colspan "2") (height "0"))
@@ -104,7 +109,6 @@ exec guile -l "$0"                              \
                                     description-id)))
                ,(package-synopsis package))
             (div (@ (id ,description-id)
-                    (class "package-description")
                     (style "display: none;"))
                  ,(match (package-logo (package-name package))
                     ((? string? url)
@@ -114,7 +118,8 @@ exec guile -l "$0"                              \
                     (_ #f))
                  (p ,(package-description package))
                  ,(license package)
-                 (a (@ (href ,(package-home-page package)))
+                 (a (@ (href ,(package-home-page package))
+                       (title "Link to the package's website"))
                     ,(package-home-page package))
                  ,(status package))))))
 
@@ -127,16 +132,93 @@ exec guile -l "$0"                              \
           (img (@ (src "graphics/guix-logo.small.png")
                   (alt "GNU Guix and the GNU System")
                   (height "83em"))))
-         "This web page lists the packages currently provided by the "
-         (a (@ (href "manual/guix.html#GNU-Distribution"))
-            "GNU system distribution")
-         " of "
-         (a (@ (href "/software/guix/guix.html")) "GNU Guix") ".  "
-         "Our " (a (@ (href "http://hydra.gnu.org/jobset/gnu/master"))
-                   "continuous integration system")
-         " shows their current build status.")
+         (p "This web page lists the packages currently provided by the "
+            (a (@ (href "manual/guix.html#GNU-Distribution"))
+               "GNU system distribution")
+            " of "
+            (a (@ (href "/software/guix/guix.html")) "GNU Guix") ".  "
+            "Our " (a (@ (href "http://hydra.gnu.org/jobset/gnu/master"))
+                      "continuous integration system")
+            " shows their current build status."))
     (table (@ (id "packages"))
-           ,@(map package->sxml packages))))
+           (tr (th "GNU?")
+               (th "Package version")
+               (th "Package details"))
+           ,@(map package->sxml packages))
+    (a (@ (href "#intro")
+          (title "Back to top.")
+          (id "top"))
+       "^")))
+
+\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 (insert-js)
+  "Return the JavaScript for the list-packages page."
+  (format #t
+"<script language=\"javascript\" type=\"text/javascript\">
+// license: CC0
+function show_hide(idThing)
+{
+    var thing = document.getElementById(idThing);
+    if (thing) {
+      if (thing.style.display == \"none\") {
+        thing.style.display = \"\";
+      } else {
+        thing.style.display = \"none\";
+      }
+    }
+}
+</script>"))
 
 \f
 (define (list-packages . args)
@@ -154,39 +236,13 @@ with gnu.org server-side include and all that."
                           (string<? (package-name p1) (package-name p2))))))
    (format #t "<!--#include virtual=\"/server/html5-header.html\" -->
 <!-- Parent-Version: 1.70 $ -->
-
 <title>GNU Guix - GNU Distribution - GNU Project</title>
-<script language=\"javascript\" type=\"text/javascript\">
-// license: CC0
-function show_hide(idThing)
-{
-  var thing = document.getElementById(idThing);
-  if (thing) {
-    if (thing.style.display == \"none\") {
-      thing.style.display = \"\";
-    } else {
-      thing.style.display = \"none\";
-    }
-  }
-}
-</script>
-<style>
-div#intro {
-margin-bottom: 5em;
-}
-table#packages {
-border: none;
-}
-div.package-description {
-position: relative;
-}
-img.package-logo {
-float: left; padding-right: 1em;
-}
-</style>
-<!--#include virtual=\"/server/banner.html\" -->
 ")
-   (display (sxml->xml (packages->sxml packages)))
+   (insert-css)
+   (insert-js)
+   (format #t "<!--#include virtual=\"/server/banner.html\" -->")
+
+   (sxml->xml (packages->sxml packages))
    (format #t "<!--#include virtual=\"/server/footer.html\" -->
 <div id=\"footer\">
 
-- 
1.7.10.4


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0003-list-packages.scm-cause-description-to-be-shown-and-.patch --]
[-- Type: text/x-diff, Size: 6428 bytes --]

From 2b9542cfcc180a6b5fc3b0ed28217eb0896c0d2b Mon Sep 17 00:00:00 2001
From: Alex Sassmannshausen <alex.sassmannshausen@gmail.com>
Date: Sun, 11 Aug 2013 21:40:29 +0200
Subject: [PATCH 3/3] list-packages.scm: cause description to be shown, and
 'void' links to not be shown when JS is disabled.

* build-aux/list-packages.scm (package->sxml package): no longer use
  package-synopsis as link as we cannot assume the JavaScript in the link will
  work (no JS safety net); instead use a span.
  (package->sxml package): remove default invisibility of package
  descriptions.
  (package->sxml package): call (show_hide-grouper description-id) after each
  package is processed to collect the package description-ids to be made
  invisible.
  (show_hide-grouper id-or-force): new function; collect n package IDs, and to
  insert a JS call to bulk_show_hide with the package IDs to be processed into
  the xml.
  (packages->sxml packages): call (show_hide-grouper #f) after (map
  package->sxml packages) returns to force the remaining package IDs to be
  processed, even if n have not been reached yet.
  (insert-js): (show_hide(idThing)): add general clause to check whether
  working, DOM conforming JS exists; introduce thingLink variable to allow its
  text to be modified depending on the show/hide package description state of
  a given package.
  (insert-js): (prep(idThing)): new JS function that generates a link in the
  span containing package-synopsis. New link provides show/hide
  package-description functionality.
  (insert-js): (bulk_show_hide()): new JS function that is able to process
  (first call prep, then show_hide on a package-ID) any number of package-IDs
  sequentially (for now defaults to 15, as provided by (show_hide-grouper
  id-or-force)).
---
 build-aux/list-packages.scm |   72 +++++++++++++++++++++++++++++++++++++------
 1 file changed, 63 insertions(+), 9 deletions(-)

diff --git a/build-aux/list-packages.scm b/build-aux/list-packages.scm
index b598b97..2aa6194 100755
--- a/build-aux/list-packages.scm
+++ b/build-aux/list-packages.scm
@@ -102,14 +102,8 @@ exec guile -l "$0"                              \
                   (title "Link to the Guix package source code"))
                ,(package-name package) " "
                ,(package-version package)))
-        (td (@ (colspan "2") (height "0"))
-            (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)
@@ -121,7 +115,35 @@ 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-or-force)
+        "Collect GROUP-COUNTER element IDs, then apply them to
+bulk_show_hide. Pass ID-OR-FORCE #f to apply collected IDs to bulk_show_hide
+even when GROUP-COUNTER IDs have not been collected."
+        (letrec
+            ((lid->js-argl
+              (lambda (l separator)
+                "Parse a list into JavaScript style arguments."
+                (if (null? l)
+                    ""
+                    (string-append separator "'" (car l) "'"
+                                   (lid->js-argl (cdr l) ", "))))))
+          (cond ((or (not id-or-force)
+                     (> (length lid) group-counter))
+                 (let ((t lid))
+                   (if id-or-force
+                       (set! lid (list id-or-force))
+                       (set! lid '()))
+                   `(script (@ (type "text/javascript"))
+                            ,(format #f "bulk_show_hide(~a)"
+                                     (lid->js-argl t "")))))
+                (else (set! lid (cons id-or-force lid))
+                      ""))))))
 
 (define (packages->sxml packages)
   "Return an HTML page as SXML describing PACKAGES."
@@ -145,6 +167,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"))
@@ -209,14 +232,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


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: patch for list-packages
  2013-08-12 22:33       ` Alex Sassmannshausen
@ 2013-08-15 14:54         ` Ludovic Courtès
  2013-08-15 14:59           ` Cyril Roelandt
  0 siblings, 1 reply; 8+ messages in thread
From: Ludovic Courtès @ 2013-08-15 14:54 UTC (permalink / raw)
  To: Alex Sassmannshausen; +Cc: guix-devel

Hi Alex,

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

> Yes, sorry — still getting used to gnus as my mail client.

FWIW, in Gnus I make inlined text/x-patch attachments when posting
patches.  Yours in this message were marked as text/x-diff, and not
inlined, so I typed ‘t text/x-patch RET’ on each of them, and that’s
fine.  :-)

Regarding your work, I really like
<http://www.atheia.org/guix/output-final.html>!

> From c3a9ba4635e0af47423391b9777ec64f872bd2ab Mon Sep 17 00:00:00 2001
> From: Alex Sassmannshausen <alex.sassmannshausen@gmail.com>
> Date: Sun, 4 Aug 2013 21:46:26 +0200
> Subject: [PATCH 1/3] list-packages: Centralise CSS styling in <head>.
>
> * build-aux/list-packages.scm (package-logo): Assign class of
>   'package-description' to package synopsis div; 'package-logo'.  Move inline
>   CSS where possible.
>   (packages->sxml): Assign id of 'intro' to intro div, 'packages' to the
>   table. Move inline CSS.
>   (list-packages): Create new <style> section, containing all inline CSS.
>   Move JavaScript <script> section to above banner include to place it in
>   <head>.

Applied.

> From 1bc1c348e8a792d3ac7b22bbb82ea9285d03f1ea Mon Sep 17 00:00:00 2001
> From: Alex Sassmannshausen <alex.sassmannshausen@gmail.com>
> Date: Sun, 11 Aug 2013 19:53:15 +0200
> Subject: [PATCH 2/3] list-packages tidying: tidying and refactoring in
>  preparation for substantive changes.
>
> * build-aux/list-packages.scm (license package): add title for <a> element.
>   (status package): add title for <a> element.
>   (package->sxml package): add alt and title for gnu-logo <img> element.
>   (package->sxml package): add title to package website <a> element.
>   (packages->sxml packages): wrap <div id="intro"> intro paragraph in <p> element.
>   (packages->sxml packages): add table header row to <table id="packages">
>   (packages->sxml packages): add <a> back to top of the page beneath table.
>   (insert-css): create new function returning page's CSS; apply whole load of new CSS.
>   (insert-js): create new function returning page's JavaScript.
>   (list-packages . args): move JavaScript to (insert-js).
>   (list-packages . args): move CSS to (insert-css).

Applied, with an additional copyright line for you, and with commit log
slightly modified to match GNU ChangeLog conventions (for instance, only
the procedure name is shown, not its formal parameters, and it doesn’t
need to be repeated; new procedures/vars do not need to be documented in
the change log, just mentioned.)

One comment:

> +(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;
> +}

I would rather indent it in C style, for improved readability.

More importantly, I think it’s worth moving to a separate file now,
like:

  ...
    #:use-module (guix build utils)
  ...

  (define %load-directory
    (dirname (current-filename)))

  (define %css-file
    (string-append %load-directory "/package-list.css"))

  (define (insert-css)
    (with-input-from-file %css-file
      (cute dump-port <> (current-output-port))))

WDYT?  Could you do this?  I think the CSS itself can be put under CC0,
so the file would need the appropriate header.

> From 2b9542cfcc180a6b5fc3b0ed28217eb0896c0d2b Mon Sep 17 00:00:00 2001
> From: Alex Sassmannshausen <alex.sassmannshausen@gmail.com>
> Date: Sun, 11 Aug 2013 21:40:29 +0200
> Subject: [PATCH 3/3] list-packages.scm: cause description to be shown, and
>  'void' links to not be shown when JS is disabled.
>
> * build-aux/list-packages.scm (package->sxml package): no longer use
>   package-synopsis as link as we cannot assume the JavaScript in the link will
>   work (no JS safety net); instead use a span.
>   (package->sxml package): remove default invisibility of package
>   descriptions.
>   (package->sxml package): call (show_hide-grouper description-id) after each
>   package is processed to collect the package description-ids to be made
>   invisible.
>   (show_hide-grouper id-or-force): new function; collect n package IDs, and to
>   insert a JS call to bulk_show_hide with the package IDs to be processed into
>   the xml.
>   (packages->sxml packages): call (show_hide-grouper #f) after (map
>   package->sxml packages) returns to force the remaining package IDs to be
>   processed, even if n have not been reached yet.
>   (insert-js): (show_hide(idThing)): add general clause to check whether
>   working, DOM conforming JS exists; introduce thingLink variable to allow its
>   text to be modified depending on the show/hide package description state of
>   a given package.
>   (insert-js): (prep(idThing)): new JS function that generates a link in the
>   span containing package-synopsis. New link provides show/hide
>   package-description functionality.
>   (insert-js): (bulk_show_hide()): new JS function that is able to process
>   (first call prep, then show_hide on a package-ID) any number of package-IDs
>   sequentially (for now defaults to 15, as provided by (show_hide-grouper
>   id-or-force)).

Overall OK, modulo two things:

  1. Could you adjust the commit log as mentioned above?

  2. See below:

> +  (define show_hide-grouper
> +    (let ((lid '())
> +          (group-counter 15))
> +      (lambda (id-or-force)
> +        "Collect GROUP-COUNTER element IDs, then apply them to
> +bulk_show_hide. Pass ID-OR-FORCE #f to apply collected IDs to bulk_show_hide
> +even when GROUP-COUNTER IDs have not been collected."

Apparently this procedure is only called with #f.  Can you remove the
‘id-or-force’ parameter and the ‘else’ case of the ‘cond’ then?

Seems like this would allow you to get rid of ‘lid’ and the ‘set!’
statements, right?  (‘set!’ is evil, you know.  ;-))

> +        (letrec
> +            ((lid->js-argl
> +              (lambda (l separator)
> +                "Parse a list into JavaScript style arguments."
> +                (if (null? l)
> +                    ""
> +                    (string-append separator "'" (car l) "'"
> +                                   (lid->js-argl (cdr l) ", "))))))

For readability, better use ‘define’ here (internal defines are
equivalent to ‘letrec’.)

Thank you for taking care of this!

Ludo’.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: patch for list-packages
  2013-08-15 14:54         ` Ludovic Courtès
@ 2013-08-15 14:59           ` Cyril Roelandt
  2013-08-15 16:40             ` Ludovic Courtès
  0 siblings, 1 reply; 8+ messages in thread
From: Cyril Roelandt @ 2013-08-15 14:59 UTC (permalink / raw)
  To: guix-devel

On 08/15/2013 04:54 PM, Ludovic Courtès wrote:
> I would rather indent it in C style, for improved readability.

Agreed, it was much better in the first patch.

Also, after applying those 3 patches, we go from 627 errors/warnings to 
955 in the W3C validator 
(http://validator.w3.org/nu/?doc=http%3A%2F%2Fwww.atheia.org%2Fguix%2Foutput.html). 
I'll try and send some patches. Ludo, where is the repository for 
gnu.org ? I'd like to get the images and the other html files.

Cyril.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: patch for list-packages
  2013-08-15 14:59           ` Cyril Roelandt
@ 2013-08-15 16:40             ` Ludovic Courtès
  0 siblings, 0 replies; 8+ messages in thread
From: Ludovic Courtès @ 2013-08-15 16:40 UTC (permalink / raw)
  To: Cyril Roelandt; +Cc: guix-devel

Cyril Roelandt <tipecaml@gmail.com> skribis:

> Also, after applying those 3 patches, we go from 627 errors/warnings
> to 955 in the W3C validator
> (http://validator.w3.org/nu/?doc=http%3A%2F%2Fwww.atheia.org%2Fguix%2Foutput.html). 

Oops.

> I'll try and send some patches. Ludo, where is the repository for
> gnu.org ?  I'd like to get the images and the other html files.

What do you mean?

See <http://savannah.gnu.org/cvs/?group=guix> for the web page CVS repo.

Ludo’.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2013-08-15 16:46 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-08-04 20:22 patch for list-packages Alex Sassmannshausen
2013-08-11 20:28 ` Alex Sassmannshausen
2013-08-11 23:38   ` Cyril Roelandt
2013-08-12 15:42     ` Nikita Karetnikov
2013-08-12 22:33       ` Alex Sassmannshausen
2013-08-15 14:54         ` Ludovic Courtès
2013-08-15 14:59           ` Cyril Roelandt
2013-08-15 16:40             ` Ludovic Courtès

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).