Keep {} in Markdown rendering (#11196)

* fix: removed '{' and '}' swallow when not in command in HTMLChars and adapted tests

* doc: updated CHANGELOG.md

* fix: added old test cases and adapted them to apply requested changes

* fix: fixed small rule in HTMLChars formatter to fix faulty tests

* test: added test case in MardownFormatterTest for string with braces

* test: added more test cases in MarkdownFormatterTest

* fix: fixed checkstyle erros in HTMLChars

* test: added test about code between quotes in MardownFormatterTest

* feat: added parameter to keep braces to HTMLChars

* feat: modified default preview to keep braces when formatting markdown

* test: added parameterized test cases to MardownFormatterTest, updated preferences migrations and made keep braces mode name more clear in HTMLChars

* fix: fixed issue about preferences migrations

* fix: rolling back modif. in HTMLCharsTest and enabled case-insensitive equality for braces keep mode in HTMLChars

* test: added test for markdown lists in MarkdowFormatterTest

Author: jeanprbt

CHANGELOG.md

@@ -27,6 +27,7 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv
 ### Fixed
 
 - We fixed an issue where entry type with duplicate fields prevented opening existing libraries with custom entry types [#11127](https://github.com/JabRef/jabref/issues/11127)
+- We fixed an issue where Markdown rendering removed braces from the text. [#10928](https://github.com/JabRef/jabref/issues/10928)
 - We fixed an issue when the file was flagged as changed on disk in the case of content selectors or groups. [#9064](https://github.com/JabRef/jabref/issues/9064)
 - We fixed crash on opening the entry editor when auto completion is enabled. [#11188](https://github.com/JabRef/jabref/issues/11188)
 - We fixed the usage of the key binding for "Clear search" (default: <kbd>Escape</kbd>). [#10764](https://github.com/JabRef/jabref/issues/10764)

src/main/java/org/jabref/logic/layout/format/HTMLChars.java

@@ -4,14 +4,14 @@
 import java.util.Objects;
 import java.util.regex.Pattern;
 
-import org.jabref.logic.layout.LayoutFormatter;
+import org.jabref.logic.layout.ParamLayoutFormatter;
 import org.jabref.logic.util.strings.HTMLUnicodeConversionMaps;
 import org.jabref.model.strings.StringUtil;
 
 /**
- * This formatter escapes characters so they are suitable for HTML.
+ * This formatter escapes characters so that they are suitable for HTML.
  */
-public class HTMLChars implements LayoutFormatter {
+public class HTMLChars implements ParamLayoutFormatter {
 
     private static final Map<String, String> HTML_CHARS = HTMLUnicodeConversionMaps.LATEX_HTML_CONVERSION_MAP;
     /**
@@ -23,6 +23,15 @@ public class HTMLChars implements LayoutFormatter {
      * */
     private static final Pattern HTML_ENTITY_PATTERN = Pattern.compile("&(?!(?:[a-z0-9]+|#[0-9]{1,6}|#x[0-9a-fA-F]{1,6});)");
 
+    private boolean keepCurlyBraces = false;
+
+    @Override
+    public void setArgument(String arg) {
+        if ("keepCurlyBraces".equalsIgnoreCase(arg)) {
+            this.keepCurlyBraces = true;
+        }
+    }
+
     @Override
     public String format(String inField) {
         String field = normalizedField(inField);
@@ -49,7 +58,7 @@ public String format(String inField) {
                 escaped = true;
                 incommand = true;
                 currentCommand = new StringBuilder();
-            } else if (!incommand && ((c == '{') || (c == '}'))) {
+            } else if (!this.keepCurlyBraces && !incommand && ((c == '{') || (c == '}'))) {
                 // Swallow the brace.
             } else if (Character.isLetter(c) || StringUtil.SPECIAL_COMMAND_CHARS.contains(String.valueOf(c))) {
                 escaped = false;
@@ -73,7 +82,7 @@ public String format(String inField) {
                         String commandBody;
                         if (c == '{') {
                             String part = StringUtil.getPart(field, i, false);
-                            i += part.length();
+                            i += this.keepCurlyBraces ? part.length() + 1 : part.length();
                             commandBody = part;
                         } else {
                             commandBody = field.substring(i, i + 1);
@@ -83,7 +92,6 @@ public String format(String inField) {
                         sb.append(Objects.requireNonNullElse(result, commandBody));
 
                         incommand = false;
-                        escaped = false;
                     } else {
                         // Are we already at the end of the string?
                         if ((i + 1) == field.length()) {
@@ -109,11 +117,14 @@ public String format(String inField) {
                     String tag = getHTMLTag(command);
                     if (!tag.isEmpty()) {
                         String part = StringUtil.getPart(field, i, true);
+                        if (this.keepCurlyBraces && (c == '{' || (c == '}'))) {
+                            i++;
+                        }
                         i += part.length();
                         sb.append('<').append(tag).append('>').append(part).append("</").append(tag).append('>');
                     } else if (c == '{') {
                         String argument = StringUtil.getPart(field, i, true);
-                        i += argument.length();
+                        i += this.keepCurlyBraces ? argument.length() + 1 : argument.length();
                         // handle common case of general latex command
                         String result = HTML_CHARS.get(command + argument);
                         // If found, then use translated version. If not, then keep
@@ -133,10 +144,14 @@ public String format(String inField) {
                     } else if (c == '}') {
                         // This end brace terminates a command. This can be the case in
                         // constructs like {\aa}. The correct behaviour should be to
-                        // substitute the evaluated command and swallow the brace:
+                        // substitute the evaluated command.
                         String result = HTML_CHARS.get(command);
                         // If the command is unknown, just print it:
                         sb.append(Objects.requireNonNullElse(result, command));
+                        // We only keep the brace if we are in 'KEEP' mode.
+                        if (this.keepCurlyBraces) {
+                            sb.append(c);
+                        }
                     } else {
                         String result = HTML_CHARS.get(command);
                         sb.append(Objects.requireNonNullElse(result, command));
@@ -170,7 +185,7 @@ private String normalizedField(String inField) {
                                   .replaceAll("[\\n]{2,}", "<p>") // Replace double line breaks with <p>
                                   .replace("\n", "<br>") // Replace single line breaks with <br>
                                   .replace("\\$", "&dollar;") // Replace \$ with &dollar;
-                                  .replaceAll("\\$([^$]*)\\$", "\\{$1\\}");
+                                  .replaceAll("\\$([^$]*)\\$", this.keepCurlyBraces ? "\\\\{$1\\\\}" : "$1}");
     }
 
     private String getHTMLTag(String latexCommand) {

src/main/java/org/jabref/migrations/PreferencesMigrations.java

@@ -331,6 +331,7 @@ protected static void upgradePreviewStyle(JabRefPreferences prefs) {
         String currentPreviewStyle = prefs.get(JabRefPreferences.PREVIEW_STYLE);
         String migratedStyle = currentPreviewStyle.replace("\\begin{review}<BR><BR><b>Review: </b> \\format[HTMLChars]{\\review} \\end{review}", "\\begin{comment}<BR><BR><b>Comment: </b> \\format[Markdown,HTMLChars]{\\comment} \\end{comment}")
                                                   .replace("\\format[HTMLChars]{\\comment}", "\\format[Markdown,HTMLChars]{\\comment}")
+                                                  .replace("\\format[Markdown,HTMLChars]{\\comment}", "\\format[Markdown,HTMLChars(keepCurlyBraces)]{\\comment}")
                                                   .replace("<b><i>\\bibtextype</i><a name=\"\\bibtexkey\">\\begin{bibtexkey} (\\bibtexkey)</a>", "<b><i>\\bibtextype</i><a name=\"\\citationkey\">\\begin{citationkey} (\\citationkey)</a>")
                                                   .replace("\\end{bibtexkey}</b><br>__NEWLINE__", "\\end{citationkey}</b><br>__NEWLINE__");
         prefs.put(JabRefPreferences.PREVIEW_STYLE, migratedStyle);

src/main/java/org/jabref/preferences/JabRefPreferences.java

@@ -821,7 +821,7 @@ private JabRefPreferences() {
                         "\\begin{pages}<BR> p. \\format[FormatPagesForHTML]{\\pages}\\end{pages}__NEWLINE__" +
                         "\\begin{abstract}<BR><BR><b>Abstract: </b>\\format[HTMLChars]{\\abstract} \\end{abstract}__NEWLINE__" +
                         "\\begin{owncitation}<BR><BR><b>Own citation: </b>\\format[HTMLChars]{\\owncitation} \\end{owncitation}__NEWLINE__" +
-                        "\\begin{comment}<BR><BR><b>Comment: </b>\\format[Markdown,HTMLChars]{\\comment}\\end{comment}__NEWLINE__" +
+                        "\\begin{comment}<BR><BR><b>Comment: </b>\\format[Markdown,HTMLChars(keepCurlyBraces)]{\\comment}\\end{comment}__NEWLINE__" +
                         "</font>__NEWLINE__");
 
         // set default theme

src/test/java/org/jabref/logic/layout/format/HTMLCharsTest.java

@@ -3,6 +3,7 @@
 import java.util.stream.Stream;
 
 import org.jabref.logic.layout.LayoutFormatter;
+import org.jabref.logic.layout.ParamLayoutFormatter;
 
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -15,10 +16,13 @@
 public class HTMLCharsTest {
 
     private LayoutFormatter layout;
+    private ParamLayoutFormatter layoutKeep;
 
     @BeforeEach
     public void setUp() {
         layout = new HTMLChars();
+        layoutKeep = new HTMLChars();
+        layoutKeep.setArgument("keepCurlyBraces");
     }
 
     private static Stream<Arguments> provideBasicFormattingData() {
@@ -37,6 +41,7 @@ private static Stream<Arguments> provideBasicFormattingData() {
                 Arguments.of("&auml;", "{\\\"a}"),
                 Arguments.of("&auml;", "\\\"a"),
                 Arguments.of("&Ccedil;", "{\\c{C}}"),
+                Arguments.of("&Ccedil;", "\\c{C}"),
                 Arguments.of("&Oogon;&imath;", "\\k{O}\\i"),
                 Arguments.of("&ntilde; &ntilde; &iacute; &imath; &imath;", "\\~{n} \\~n \\'i \\i \\i")
         );
@@ -144,4 +149,23 @@ private static Stream<Arguments> provideHTMLEntityFormattingData() {
     void hTMLEntityFormatting(String expected, String input) {
         assertEquals(expected, layout.format(input));
     }
+
+    private static Stream<Arguments> provideBracesKeepFormattingData() {
+        return Stream.of(
+            Arguments.of("{&auml;}", "{\\\"{a}}"),
+            Arguments.of("{&auml;}", "{\\\"a}"),
+            Arguments.of("{&Ccedil;}", "{\\c{C}}"),
+            Arguments.of("{<em>hallo</em>}", "{\\emph hallo}"),
+            Arguments.of("{<b>hallo</b>}", "{\\textbf hallo}"),
+            Arguments.of("{<b>hallo</b>}", "{\\bf hallo}"),
+            Arguments.of("{&#39;}", "{\\textquotesingle}"),
+            Arguments.of("{{ test }}", "{{ test }}")
+        );
+    }
+
+    @ParameterizedTest
+    @MethodSource("provideBracesKeepFormattingData")
+    void bracesKeepFormatting(String expected, String input) {
+        assertEquals(expected, layoutKeep.format(input));
+    }
 }

src/test/java/org/jabref/logic/layout/format/MarkdownFormatterTest.java

@@ -1,10 +1,14 @@
 package org.jabref.logic.layout.format;
 
+import java.util.stream.Stream;
+
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.Arguments;
+import org.junit.jupiter.params.provider.MethodSource;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
 class MarkdownFormatterTest {
@@ -17,23 +21,74 @@ void setUp() {
     }
 
     @Test
-    void formatWhenFormattingPlainTextThenReturnsTextWrappedInParagraph() {
-        assertEquals("<p>Hello World</p>", markdownFormatter.format("Hello World"));
-    }
-
-    @Test
-    void formatWhenFormattingComplexMarkupThenReturnsOnlyOneLine() {
-        assertFalse(markdownFormatter.format("Markup\n\n* list item one\n* list item 2\n\n rest").contains("\n"));
+    void formatWhenFormattingNullThenThrowsException() {
+        Exception exception = assertThrows(NullPointerException.class, () -> markdownFormatter.format(null));
+        assertEquals("Field Text should not be null, when handed to formatter", exception.getMessage());
     }
 
-    @Test
-    void formatWhenFormattingEmptyStringThenReturnsEmptyString() {
-        assertEquals("", markdownFormatter.format(""));
+    private static Stream<Arguments> provideMarkdownAndHtml() {
+        return Stream.of(
+                Arguments.of("Hello World", "<p>Hello World</p>"),
+                Arguments.of("""
+                        Markup
+                        
+                        * list item one
+                        * list item two
+                        
+                         rest
+                        """,
+                        "<p>Markup</p> <ul> <li>list item one</li> <li>list item two</li> </ul> <p>rest</p>"
+                ),
+                Arguments.of("""
+                        ```
+                        Hello World
+                        ```
+                        """,
+                        "<pre><code>Hello World </code></pre>"
+                ),
+                Arguments.of("""
+                       First line
+                    
+                       Second line
+                    
+                       ```java
+                       String test;
+                       ```
+                    """,
+                    "<p>First line</p> <p>Second line</p> <pre><code class=\"language-java\">String test; </code></pre>"
+                ),
+                Arguments.of("""
+                       Some text.
+                       ```javascript
+                       let test = "Hello World";
+                       ```
+                    
+                       ```java
+                       String test = "Hello World";
+                       ```
+                       Some more text.
+                    """,
+                    "<p>Some text.</p> <pre><code class=\"language-javascript\">let test = &quot;Hello World&quot;; " +
+                            "</code></pre> <pre><code class=\"language-java\">String test = &quot;Hello World&quot;; " +
+                            "</code></pre> <p>Some more text.</p>"
+                ),
+                Arguments.of("""
+                        Some text.
+                        
+                        ```java
+                        int foo = 0;
+                        foo = 1;
+                        
+                        ```
+                        """,
+                        "<p>Some text.</p> <pre><code class=\"language-java\">int foo = 0; foo = 1;  </code></pre>"
+                )
+        );
     }
 
-    @Test
-    void formatWhenFormattingNullThenThrowsException() {
-        Exception exception = assertThrows(NullPointerException.class, () -> markdownFormatter.format(null));
-        assertEquals("Field Text should not be null, when handed to formatter", exception.getMessage());
+    @ParameterizedTest
+    @MethodSource("provideMarkdownAndHtml")
+    void formatWhenFormattingCodeBlockThenReturnsCodeBlockInHtml(String markdown, String expectedHtml) {
+        assertEquals(expectedHtml, markdownFormatter.format(markdown));
     }
 }

src/test/java/org/jabref/migrations/PreferencesMigrationsTest.java

@@ -84,8 +84,8 @@ void previewStyleReviewToComment() {
 
         String newPreviewStyle = "<font face=\"sans-serif\">__NEWLINE__"
                 + "Customized preview style using reviews and comments:__NEWLINE__"
-                + "\\begin{comment}<BR><BR><b>Comment: </b> \\format[Markdown,HTMLChars]{\\comment} \\end{comment}__NEWLINE__"
-                + "\\begin{comment} Something: \\format[Markdown,HTMLChars]{\\comment} special \\end{comment}__NEWLINE__"
+                + "\\begin{comment}<BR><BR><b>Comment: </b> \\format[Markdown,HTMLChars(keepCurlyBraces)]{\\comment} \\end{comment}__NEWLINE__"
+                + "\\begin{comment} Something: \\format[Markdown,HTMLChars(keepCurlyBraces)]{\\comment} special \\end{comment}__NEWLINE__"
                 + "</font>__NEWLINE__";
 
         when(prefs.get(JabRefPreferences.PREVIEW_STYLE)).thenReturn(oldPreviewStyle);