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 |
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 <[hidden email]> 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 > > > > > > > > |
Free forum by Nabble | Edit this page |