Sounds good to me, +1
And thanks for the detailed report on your investigation Alex!
Bruno
On Monday, 18 November 2019, 3:08:49 am NZDT, Alex Herbert
<alex.d.herb...@gmail.com> wrote:
> On 15 Nov 2019, at 14:40, Alex Herbert <alex.d.herb...@gmail.com> wrote:
>
> […]
I have finished testing different solutions to the site rendering of RNG to
included MathJax and render the code using prettyprint.
I tried a full upgrade to commons-skin to base it on Fluido 1.8. This changed a
lot of element class names such that the style sheet work was too time
consuming. I fixed it for RNG but there is more to the commons-skin
customisation that I have not yet tested. It allowed dynamic rendering of the
left menu width. This is fixed at 250px for all published commons sites. Thus
the new and old skins looked different enough that it would require more
comprehensive testing.
In this process I now understand where the problems are with common-skin:
1. <head> processing
The commons-skin uses the default <head> processing from Fluido 1.3. This was
written pre maven site plugin 3.5 where <head> element allows XHTML. Now this
has to be escaped. The velocity template code thus does not work. This requires
a change to the velocity template to use the rendering engine to run the
velocity engine on the injected XHTML. This is what is does in Fluido 1.8.
Given the current commons-skin functionality for <head> is broken this is a
simple fix that should be made.
2. The footer
The footer for all commons pages in defined in the site.xml in commons parent.
This is within a CDATA escape tag. This should not be done.
The commons-skin template does not work with the CDATA escaped footer. Thus
more recent site for commons components have no copyright footer.
If you remove the CDATA escape tag from commons-parent then you see that the
macro for the footer is not working correctly and the footer although now
included has some incorrect text (duplicate 'Apache Apache’ in the text).
The fix is simple and should be applied to the current commons-skin
3. Menu icons
I noticed the right arrow menu icon has a white background. The icon is 7x7
pixels and the background is light grey so it is hard to notice. The down arrow
has a transparent background. So this should be fixed.
4. Pretty print of source code
The site.js in commons skin has a workaround for the fact that any <pre> tag
are wrapped in a <div> tag.
Thus in XDOC:
<source class=“prettyprint”>
Turns into
<div class=“prettyprint”><pre>
This is handled in site.js by javascript that moves the prettyprint class from
the <div> tag to the <pre> tag.
The issue with commons RNG is the use of APT to write the user guide.
Thus in APT:
*-------------
String s = “”;
*-------------
Turns into
<div class=“source”><pre>
The <pre> tag is not pretty printed.
Fluido 1.8 handles the second case with this replacement in the body of the
document:
$bodyContent.replaceAll( "<div class=${esc.q}source${esc.q}>(\r?\n)?<pre>",
"<div class=${esc.q}source${esc.q}><pre class=${esc.q}$sourceStyle${esc.q}>" )
Replacing:
<div class=“source”><pre>
With
<div class=“source”><pre class="prettyprint">
Proposal:
I propose to fix items 1, 2, 3 in commons-skin. These are all bugs where the
intended functionality is not working.
Item 4 would be a new change to increase the coverage of prettyprint to more
<pre> tags. This should have a vote if it should be opt out or opt in.
If the regex replacement is done in the site.vm velocity template (like Fluido
1.8) then opt out/in could be done by passing a properties to the commons-skin
plugin using the site.xml:
<custom>
<commonsSkin>
<prettyPrintSourcePreTags>true</prettyPrintSourcePreTags>
<sourceLineNumbersEnabled>false</sourceLineNumbersEnabled>
</commonsSkin >
</custom>
The second property allows the class “linenums” to be added to the tag as well
for different prettyprint rendering. This option idea is copied from Fluido 1.8
[1].
If I make this change then all existing functionality of commons-skin should be
restored. Commons RNG would be able to opt-in configure the pretty print for
the APT written site documentation.
In my local git repos for commons only commons-math uses APT documentation. In
this document there are no source snippets. So it may be OK to make this opt
out functionality that is on by default. But I am missing a lot of the projects
and some may be effected.
I have confirmed that if I put
<prettyPrintSourcePreTags>true</prettyPrintSourcePreTags> in commons-parent
site.xml and then <prettyPrintSourcePreTags>false</prettyPrintSourcePreTags> in
commons-rng then the functionality is turned off. So opt out would work for any
projects that upgrade and have an issue with the prettyprint rendering.
I would vote for fixing the bugs in commons-skin and making the new
functionality opt-out via properties in the site.xml. The skin can then be
released and then used with the most recent commons-parent. This would fix the
problem with the CDATA wrapper footer in the site.xml.
Alex
[1] https://maven.apache.org/skins/maven-fluido-skin/#SourceLineNumbers
<https://maven.apache.org/skins/maven-fluido-skin/#SourceLineNumbers>
