I missed this reply of yours the first time. Let's see. At Tue, 29 Jan 2013 11:38:41 -0700, Eric Blake wrote: > > On 01/22/2013 07:31 AM, Claudio Bley wrote: > > Add minimized CSS and Javascript files of SHJS > > (http://shjs.sourceforge.net/) required for highlighting C code. > > > > Call sh_highlightDocument() in onload event handler. > > > > Signed-off-by: Claudio Bley <cbley@xxxxxxxxxx> > > --- > > docs/newapi.xsl | 2 +- > > docs/page.xsl | 5 ++++- > > docs/sh_c.min.js | 1 + > > Just one line? Eww. Can we add some whitespace to make it legible? It's not intended to be legible, and it is not inteded to be edited. It's just output produced by a compiler. > > docs/sh_emacs.min.css | 1 + > > Likewise. Likewise. > > docs/sh_main.min.js | 4 ++++ > > 5 files changed, 11 insertions(+), 2 deletions(-) > > create mode 100644 docs/sh_c.min.js > > create mode 100644 docs/sh_emacs.min.css > > create mode 100644 docs/sh_main.min.js > > Are these .js files used only during generation of the html pages (that > is, only important for the person creating the rpm), or are they > intended to be installed alongside the html files (all end users end up > downloading them)? These files are to be installed along with the HTML files. > At any rate, this patch is missing patches to docs/Makefile.am, so I > can't reproduce the build from a 'make dist' tarball. I see. I'll address that. > > +++ b/docs/page.xsl > > @@ -136,10 +136,13 @@ > > <head> > > <link rel="stylesheet" type="text/css" href="{$href_base}main.css"/> > > <link rel="SHORTCUT ICON" href="{$href_base}32favicon.png"/> > > + <script type="text/javascript" src="{$href_base}sh_main.min.js"/> > > + <script type="text/javascript" src="{$href_base}sh_c.min.js"/> > > + <link rel="stylesheet" type="text/css" href="{$href_base}sh_emacs.min.css"/> > > <title>libvirt: <xsl:value-of select="html/body/h1"/></title> > > <meta name="description" content="libvirt, virtualization, virtualization API"/> > > </head> > > - <body> > > + <body onload="sh_highlightDocument();"> > > Oh, it looks like you are actually installing the .js files alongside > the .html, and that end users would be required to run javascript to see > the benefits. Have you tested that things still render nicely when > scripts are disabled in the user's browser? Yes, it just doesn't get highlighted then. > Can we avoid user-side scripting, and instead get the coloration > done at 'make dist' time? It would be possible, no doubt. But it would further complicate things since we either would have to pre-process the libvirt-api.xml file into some intermediate format, looking for code blocks and feeding them to a code highlighter and afterwards XSL process this file OR post-process the HTML output file extracting code blocks, feeding them into a source code highlighter and splicing them back in. Then we'd require an external highlighting tool as part of the build to begin with. > > <div id="header"> > > <div id="headerLogo"/> > > <div id="headerSearch"> > > diff --git a/docs/sh_c.min.js b/docs/sh_c.min.js > > new file mode 100644 > > index 0000000..fd91118 > > --- /dev/null > > +++ b/docs/sh_c.min.js > > @@ -0,0 +1 @@ > > +if(!this.sh_languages) [...] > > \ No newline at end of file > > This file needs a newline at the end; and whitespace would help > immensely. Keeping it at 80 columns is a good goal. This patch > probably fails 'make syntax-check'. It did. I've added an exception to the syntax check rules. See the new patch at https://www.redhat.com/archives/libvir-list/2013-January/msg02104.html > This file is missing a copyright and license statement; this is > particularly important if it is going to be shipped alongside .html > and used client-side. > > > diff --git a/docs/sh_emacs.min.css b/docs/sh_emacs.min.css > > new file mode 100644 > > index 0000000..b7aed87 > > --- /dev/null > > +++ b/docs/sh_emacs.min.css > > @@ -0,0 +1 @@ > > +pre.sh_sourceCode [...] > > \ No newline at end of file > > Again, adding whitespace and a trailing newline would help, and it needs > a copyright and license. IMO, these files are "object files", as far as the GPL v3 is concerned. ,----[ GPL v3 1. Source Code ] | The "source code" for a work means the preferred form of the work | for making modifications to it. "Object code" means any non-source | form of a work. `---- > Also, since you are copying from somewhere else, a comment about the > original source would be appropriate. Fair enough, where should I put it? > > diff --git a/docs/sh_main.min.js b/docs/sh_main.min.js > > new file mode 100644 > > index 0000000..31d1ba0 > > --- /dev/null > > +++ b/docs/sh_main.min.js > > @@ -0,0 +1,4 @@ > > +/* Copyright (C) 2007, 2008 gnombat@xxxxxxxxxxxxxxxxxxxxx */ > > +/* License: http://shjs.sourceforge.net/doc/gplv3.html */ > > This has a copyright, but the license is incomplete - the FSF states > that it is insufficient to point to a URL when using the GPLv3 (and a > non-canonical URL at that); while a URL is helpful, any package shipping > a GPLv3 file must also ship the full GPLv3 text as part of the > package. OK, I'll add a GPLv3 license file. > > + > > +if(!this.sh_languag [...] > > \ No newline at end of file > > What does upstream have against whitespace? It is a compressed version of the original code. Intended to cut down on download time, snappier page loading. Claudio -- AV-Test GmbH, Henricistraße 20, 04155 Leipzig, Germany Phone: +49 341 265 310 19 Web:<http://www.av-test.org> Eingetragen am / Registered at: Amtsgericht Stendal (HRB 114076) Geschaeftsfuehrer (CEO): Andreas Marx, Guido Habicht, Maik Morgenstern -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list