You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "elharo (via GitHub)" <gi...@apache.org> on 2023/02/19 14:41:39 UTC

[GitHub] [maven-doxia] elharo commented on a diff in pull request #146: Bugfix/newlines

elharo commented on code in PR #146:
URL: https://github.com/apache/maven-doxia/pull/146#discussion_r1111249956


##########
doxia-modules/doxia-module-markdown/src/main/java/org/apache/maven/doxia/module/markdown/MarkdownMarkup.java:
##########
@@ -92,10 +91,7 @@ public interface MarkdownMarkup extends TextMarkup {
     String VERBATIM_START_MARKUP = "```";
 
     /** Syntax for the non breaking space: "\ " */
-    String NON_BREAKING_SPACE_MARKUP = String.valueOf(BACKSLASH) + SPACE;
-
-    /** Syntax for the page break: "\f" */
-    String PAGE_BREAK_MARKUP = String.valueOf(PAGE_BREAK);
+    String NON_BREAKING_SPACE_MARKUP = "&npsp;";

Review Comment:
   should this be `&nbsp;`?



##########
doxia-core/src/test/java/org/apache/maven/doxia/sink/impl/SinkEventElement.java:
##########
@@ -80,4 +80,22 @@ public String toString() {
         builder.append(']');
         return builder.toString();
     }
+
+    @Override
+    public int hashCode() {

Review Comment:
   These methods could use tests



##########
doxia-modules/doxia-module-markdown/src/test/java/org/apache/maven/doxia/module/markdown/MarkdownSinkTest.java:
##########
@@ -349,11 +350,34 @@ public void testMultipleAuthors() {
                 + "author: " + EOL
                 + "  - first author" + EOL
                 + "  - second author" + EOL
-                + MarkdownMarkup.METADATA_MARKUP + EOL;
+                + MarkdownMarkup.METADATA_MARKUP + EOL + EOL;
 
         assertEquals(expected, getSinkContent(), "Wrong metadata section");
     }
 
+    @Test
+    public void testRoundtrip() throws IOException, ParseException {
+        parseFile(parser, "test", getSink());
+
+        final SinkEventTestingSink regeneratedSink = new SinkEventTestingSink();
+        try (Reader reader = new StringReader(getSinkContent())) {
+            parser.parse(reader, regeneratedSink);
+        }
+
+        System.out.println(getSinkContent());

Review Comment:
   delete this line; passing tests should generate no output



##########
doxia-modules/doxia-module-markdown/src/test/java/org/apache/maven/doxia/module/markdown/MarkdownSinkTest.java:
##########
@@ -212,24 +214,23 @@ protected String getDivisionBlock(String text) {
 
     /** {@inheritDoc} */
     protected String getVerbatimSourceBlock(String text) {
-        return EOL
-                + EOL
-                + MarkdownMarkup.VERBATIM_START_MARKUP
+        return MarkdownMarkup.VERBATIM_START_MARKUP
                 + EOL
                 + text
                 + EOL
                 + MarkdownMarkup.VERBATIM_START_MARKUP
+                + EOL
                 + EOL;
     }
 
     /** {@inheritDoc} */
     protected String getHorizontalRuleBlock() {
-        return EOL + MarkdownMarkup.HORIZONTAL_RULE_MARKUP + EOL;
+        return MarkdownMarkup.HORIZONTAL_RULE_MARKUP + EOL + EOL;
     }
 
     /** {@inheritDoc} */
     protected String getPageBreakBlock() {
-        return EOL + MarkdownMarkup.PAGE_BREAK_MARKUP + EOL;
+        return "";

Review Comment:
   I prefer "". I definitely don't want to add additional dependencies on Plexus StringUtils



-- 
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-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org