michael-o commented on code in PR #185:
URL: https://github.com/apache/maven-doxia/pull/185#discussion_r1438352084
##########
doxia-core/src/test/java/org/apache/maven/doxia/sink/impl/AbstractSinkTest.java:
##########
@@ -644,10 +645,65 @@ public void testTable() {
if (isXmlSink()) {
assertThat(wrapXml(actual), isIdenticalTo(wrapXml(expected)));
} else {
- assertEquals(actual, expected);
+ assertEquals(expected, actual);
+ }
+ }
+
+ /**
+ * Checks that the sequence <code>[table(),
+ * tableRows(Sink.JUSTIFY_CENTER, false), tableRow(), tableCell(),
+ * text(cell), tableCell_(), tableRow_(), tableRows_(), tableCaption(),
+ * text(caption), tableCaption_(), table_()]</code>,
+ * invoked on the current sink, produces the same result as
+ * {@link #getTableBlock getTableBlock}(cell, caption).
+ */
+ @Test
+ public void testTableWithHeader() {
+ int[] justify = {Sink.JUSTIFY_LEFT, Sink.JUSTIFY_RIGHT,
Sink.JUSTIFY_CENTER};
+ sink.table();
+ sink.tableRows(justify, false);
+ try (IntStream cellStream = getCellStreamForNewRow(3)) {
+ cellStream.forEach(col -> {
+ sink.tableHeaderCell();
+ sink.text("header" + col);
Review Comment:
`"header"` should end with an underscope, just like the rows.
##########
doxia-modules/doxia-module-markdown/src/main/java/org/apache/maven/doxia/module/markdown/MarkdownSink.java:
##########
@@ -69,24 +69,21 @@ public class MarkdownSink extends AbstractTextSink
implements MarkdownMarkup {
/** tableCaptionFlag. */
private boolean tableCaptionFlag;
- /** tableCellFlag. */
+ /** tableCellFlag, set to true inside table (header) cells */
private boolean tableCellFlag;
+ /** tableRowHeaderFlag, set to true for table rows containing at least one
table header cell */
+ private boolean tableHeaderCellFlag;
+
/** headerFlag. */
private boolean headerFlag;
- /** bufferFlag. */
+ /** bufferFlag, set to true in certain elements to prevent direct writing
during {@link #text(String, SinkEventAttributes)} */
Review Comment:
Same here
##########
doxia-core/src/test/java/org/apache/maven/doxia/sink/impl/AbstractSinkTest.java:
##########
@@ -644,10 +645,65 @@ public void testTable() {
if (isXmlSink()) {
assertThat(wrapXml(actual), isIdenticalTo(wrapXml(expected)));
} else {
- assertEquals(actual, expected);
+ assertEquals(expected, actual);
Review Comment:
Can you move these to a separate PR? Good catch, BTW.
##########
doxia-modules/doxia-module-xdoc/src/test/java/org/apache/maven/doxia/module/xdoc/XdocSinkTest.java:
##########
@@ -208,6 +208,19 @@ protected String getTableBlock(String cell, String
caption) {
+ "</td></tr></table>";
}
+ @Override
+ protected String getTableWithHeaderBlock(String... rowPrefixes) {
+ return "<table border=\"0\">\n<tr>\n<th>" + rowPrefixes[0] +
"0</th>\n<th>" + rowPrefixes[0] + "1</th>\n<th>"
Review Comment:
That border attribute should already be code by now. Is it still emitted? It
is deprecated and one should use CSS.
##########
doxia-modules/doxia-module-markdown/src/main/java/org/apache/maven/doxia/module/markdown/MarkdownSink.java:
##########
@@ -45,12 +48,9 @@ public class MarkdownSink extends AbstractTextSink
implements MarkdownMarkup {
// Instance fields
// ----------------------------------------------------------------------
- /** A buffer that holds the current text when headerFlag or bufferFlag
set to <code>true</code>. */
+ /** A buffer that holds the current text when headerFlag or bufferFlag
set to <code>true</code>. This value of this buffer is already escaped. */
Review Comment:
The content of this buffer...
##########
doxia-modules/doxia-module-markdown/src/main/java/org/apache/maven/doxia/module/markdown/MarkdownSink.java:
##########
@@ -69,24 +69,21 @@ public class MarkdownSink extends AbstractTextSink
implements MarkdownMarkup {
/** tableCaptionFlag. */
private boolean tableCaptionFlag;
- /** tableCellFlag. */
+ /** tableCellFlag, set to true inside table (header) cells */
private boolean tableCellFlag;
+ /** tableRowHeaderFlag, set to true for table rows containing at least one
table header cell */
Review Comment:
Same here
##########
doxia-modules/doxia-module-markdown/src/main/java/org/apache/maven/doxia/module/markdown/MarkdownSink.java:
##########
@@ -69,24 +69,21 @@ public class MarkdownSink extends AbstractTextSink
implements MarkdownMarkup {
/** tableCaptionFlag. */
private boolean tableCaptionFlag;
- /** tableCellFlag. */
+ /** tableCellFlag, set to true inside table (header) cells */
Review Comment:
`{@code true}`
##########
doxia-modules/doxia-module-apt/src/main/java/org/apache/maven/doxia/module/apt/AptSink.java:
##########
@@ -817,6 +820,9 @@ public void text(String text, SinkEventAttributes
attributes) {
} else {
content(text);
}
+ if (attributes != null) {
+ inline_();
+ }
Review Comment:
Is that a bug we haven't noticed before?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
