yeah, I did not fixed the tests… sorry :(
but I’m completely sure that adding the % is bad bad bad (bad tree times, or, how we like
to say in france: très bad :P)
Esteban
On 13 Aug 2014, at 16:54, Ben Coman <btc(a)openinworld.com> wrote:
I have identified the problem with the failing Pillar CI development builds, but I'm
not sure what action should be taken. "Pillar-ExporterHTML-EstebanLorenzano.47"
which is commented as...
"bugfix: PRHTMLWriter>>#writeEmbeddedPicture: should not add '%' to
width property. That should be responsibility of author."
made the following change...
PRHTMLWriter>>writeEmbeddedPicture:
- anExternalLink parameterAt: 'width' ifPresent: [ :width | img parameterAt:
'width' put: width greaseString, '%' ].
+ anExternalLink parameterAt: 'width' ifPresent: [ :width | img parameterAt:
'width' put: width greaseString ].
...which causes PRHTMLWriterTest (PRDocumentWriterTest) >> testFigureWithWidth
to fail. It can be fixed as follows (plus some debugging instrumentation)...
PRDocumentWriterTest >> testFigureWithWidth
| item width |
- width := '50'.
+ width := '50%'.
item := PRExternalLink new
reference: 'file://picture.png';
embedded: true;
parameterAt: 'width' put: width;
yourself.
+ Transcript crShow: self printString ; crShow: (self write: item).
self assertWriting: item includesText: self widthFor50percents
...but I'm not sure if modifying #testFigureWithWidth is the right thing to do to fix
PRHTMLWriterTest, since this test is inherited by six other subclasses of
PRDocumentWriterTest. None of the other tests complain, but that doesn't make it
right.
====================
With [width := '50'] running all Pillar tests produces the following
Transcript...
PRLaTeXWriterTest>>#testFigureWithWidth \begin{figure}
\begin{center}
\includegraphics[width=0.5\textwidth]{picture.png}\caption{file:$/$$/$picture.png.\label{picture.png}}\end{center}
\end{figure}
PRHTMLWriterTest>>#testFigureWithWidth
<figure><img src="picture.png" width="50">
</img><figcaption></figcaption></figure>
PRGitHubMarkdownWriterTest>>#testFigureWithWidth <a
name=""></a><figure><img src="picture.png"
width="50%">
</img><figcaption>file://picture.png</figcaption></figure>
PRMarkdownStreamWriterTest>>#testFigureWithWidth <a
name=""></a><figure><img src="picture.png"
width="50%">
</img><figcaption>file://picture.png</figcaption></figure>
PRPillarWriterTest>>#testFigureWithWidth +file://picture.png|width=50+
====================
With [width := '50%'] running all Pillar tests produces the following
Transcript...
PRLaTeXWriterTest>>#testFigureWithWidth \begin{figure}
\begin{center}
\includegraphics[width=0.5\textwidth]{picture.png}\caption{file:$/$$/$picture.png.\label{picture.png}}\end{center}
\end{figure}
PRHTMLWriterTest>>#testFigureWithWidth <figure><img
src="picture.png" width="50%">
</img><figcaption></figcaption></figure>
PRGitHubMarkdownWriterTest>>#testFigureWithWidth <a
name=""></a><figure><img src="picture.png"
width="50%%">
</img><figcaption>file://picture.png</figcaption></figure>
PRMarkdownStreamWriterTest>>#testFigureWithWidth <a
name=""></a><figure><img src="picture.png"
width="50%%">
</img><figcaption>file://picture.png</figcaption></figure>
PRPillarWriterTest>>#testFigureWithWidth +file://picture.png|width=50\%+
====================
Now would existing projects be broken by pictures becoming 50 pixels wide rather 50% wide
? If the goal is to be able to specify pixel width, maybe it would be better to introduce
a Pillar parameter "pixelWidth" .
Presumably the Pillar documentation
https://github.com/pillar-markup/pillar-documentation
would need updating also.
cheers -ben