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



Reply via email to