3:47 a.m.
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(a)av-test.de>
> ---
> 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(a)users.sourceforge.net */
> +/* 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