Details
-
Bug
-
Status: Closed
-
Major
-
Resolution: Fixed
-
None
-
None
Description
Table justification is currently not implemented at all (i.e. all table columns have default alignment) and figures are using the XHTML markup instead of the native Markdown markup.
Attachments
Attachments
Issue Links
- links to
Activity
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())
+ }
+
+ /**
+ * 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)
+ @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:
`
`
##########
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
+ if (attributes != null)
{ + inline_(); + }Review Comment:
Is that a bug we haven't noticed before?
kwin commented on code in PR #185:
URL: https://github.com/apache/maven-doxia/pull/185#discussion_r1438355558
##########
doxia-core/src/test/java/org/apache/maven/doxia/sink/impl/AbstractSinkTest.java:
##########
@@ -644,10 +645,65 @@ public void testTable() {
if (isXmlSink())
else {
- assertEquals(actual, expected);
+ assertEquals(expected, actual);
Review Comment:
No, sorry, too much effort for just fixing a glitch in a test class.
kwin commented on code in PR #185:
URL: https://github.com/apache/maven-doxia/pull/185#discussion_r1438355892
##########
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
+ if (attributes != null)
{ + inline_(); + }Review Comment:
Yes, similar to https://issues.apache.org/jira/browse/DOXIA-706
kwin commented on code in PR #185:
URL: https://github.com/apache/maven-doxia/pull/185#discussion_r1438356143
##########
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)
+ @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:
This is just what is emitted currently for the newly added test in the `AbstractParserTest`. Fixing the output should be done separately if you consider it wrong.
michael-o commented on code in PR #185:
URL: https://github.com/apache/maven-doxia/pull/185#discussion_r1438360736
##########
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
+ if (attributes != null)
{ + inline_(); + }Review Comment:
Shouldn't this be separate then?
michael-o commented on code in PR #185:
URL: https://github.com/apache/maven-doxia/pull/185#discussion_r1438360834
##########
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)
+ @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:
Alright, let's leave as-is.
kwin commented on code in PR #185:
URL: https://github.com/apache/maven-doxia/pull/185#discussion_r1438369940
##########
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
+ if (attributes != null)
{ + inline_(); + }Review Comment:
Ok -> https://issues.apache.org/jira/browse/DOXIA-715
kwin commented on code in PR #185:
URL: https://github.com/apache/maven-doxia/pull/185#discussion_r1438391211
##########
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:
Fixed.
##########
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:
Fixed
michael-o commented on code in PR #185:
URL: https://github.com/apache/maven-doxia/pull/185#discussion_r1438393908
##########
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 content of this buffer is already escaped. */
Review Comment:
The content...
kwin merged PR #185:
URL: https://github.com/apache/maven-doxia/pull/185
kwin opened a new pull request, #185:
URL: https://github.com/apache/maven-doxia/pull/185
Fix markup for inline styles for "em" and "strong"