Details

    • Bug
    • Status: Closed
    • Major
    • Resolution: Fixed
    • None
    • 2.0.0, 2.0.0-M9
    • Module - Markdown
    • 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

        Activity

          githubbot ASF GitHub Bot added a comment -

          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"

          githubbot ASF GitHub Bot added a comment - 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"
          githubbot ASF GitHub Bot added a comment -

          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?

          githubbot ASF GitHub Bot added a comment - 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?
          githubbot ASF GitHub Bot added a comment -

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

          { assertThat(wrapXml(actual), isIdenticalTo(wrapXml(expected))); }

          else {

          • assertEquals(actual, expected);
            + assertEquals(expected, actual);

          Review Comment:
          No, sorry, too much effort for just fixing a glitch in a test class.

          githubbot ASF GitHub Bot added a comment - 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()) { assertThat(wrapXml(actual), isIdenticalTo(wrapXml(expected))); } else { assertEquals(actual, expected); + assertEquals(expected, actual); Review Comment: No, sorry, too much effort for just fixing a glitch in a test class.
          githubbot ASF GitHub Bot added a comment -

          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

          { content(text); }

          + if (attributes != null)

          { + inline_(); + }

          Review Comment:
          Yes, similar to https://issues.apache.org/jira/browse/DOXIA-706

          githubbot ASF GitHub Bot added a comment - 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 { content(text); } + if (attributes != null) { + inline_(); + } Review Comment: Yes, similar to https://issues.apache.org/jira/browse/DOXIA-706
          githubbot ASF GitHub Bot added a comment -

          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)

          { + "</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:
          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.

          githubbot ASF GitHub Bot added a comment - 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) { + "</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: 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.
          githubbot ASF GitHub Bot added a comment -

          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

          { content(text); }

          + if (attributes != null)

          { + inline_(); + }

          Review Comment:
          Shouldn't this be separate then?

          githubbot ASF GitHub Bot added a comment - 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 { content(text); } + if (attributes != null) { + inline_(); + } Review Comment: Shouldn't this be separate then?
          githubbot ASF GitHub Bot added a comment -

          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)

          { + "</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:
          Alright, let's leave as-is.

          githubbot ASF GitHub Bot added a comment - 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) { + "</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: Alright, let's leave as-is.
          githubbot ASF GitHub Bot added a comment -

          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

          { content(text); }

          + if (attributes != null)

          { + inline_(); + }

          Review Comment:
          Ok -> https://issues.apache.org/jira/browse/DOXIA-715

          githubbot ASF GitHub Bot added a comment - 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 { content(text); } + if (attributes != null) { + inline_(); + } Review Comment: Ok -> https://issues.apache.org/jira/browse/DOXIA-715
          githubbot ASF GitHub Bot added a comment -

          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

          githubbot ASF GitHub Bot added a comment - 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
          githubbot ASF GitHub Bot added a comment -

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

          githubbot ASF GitHub Bot added a comment - 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...
          githubbot ASF GitHub Bot added a comment - kwin merged PR #185: URL: https://github.com/apache/maven-doxia/pull/185
          kwin Konrad Windszus added a comment - Fixed in https://github.com/apache/maven-doxia/commit/7fc56190e58715022c9b3967e79517d387ac9b27 .

          People

            kwin Konrad Windszus
            kwin Konrad Windszus
            Votes:
            0 Vote for this issue
            Watchers:
            Stop watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Slack

                In order to see discussions, first confirm access to your Slack account(s) in the following workspace(s): ASF