Fix unnecessary escape characters in markdown output (#800)

matrix-org · Sep 12, 2023 · e1ecd27 · e1ecd27
1 parent 2f1c7cc
commit e1ecd27
Show file tree

Hide file tree

Showing 5 changed files with 17 additions and 37 deletions.
diff --git a/crates/wysiwyg/src/dom/nodes/container_node.rs b/crates/wysiwyg/src/dom/nodes/container_node.rs
@@ -953,7 +953,7 @@ where
             }
 
             CodeBlock => {
-                fmt_code_block(self, buffer, &mut options, as_message)?;
+                fmt_code_block(self, buffer, &options, as_message)?;
             }
 
             Quote => {
@@ -1109,7 +1109,6 @@ where
             buffer.push("`` ");
 
             options.insert(MarkdownOptions::IGNORE_LINE_BREAK);
-            options.insert(MarkdownOptions::NO_ESCAPE);
             fmt_children(this, buffer, options, as_message)?;
 
             buffer.push(" ``");
@@ -1307,14 +1306,13 @@ where
         fn fmt_code_block<S>(
             this: &ContainerNode<S>,
             buffer: &mut S,
-            options: &mut MarkdownOptions,
+            options: &MarkdownOptions,
             as_message: bool,
         ) -> Result<(), MarkdownError<S>>
         where
             S: UnicodeString,
         {
             buffer.push("```\n");
-            options.insert(MarkdownOptions::NO_ESCAPE);
             fmt_children(this, buffer, options, as_message)?;
             buffer.push("\n```\n");
 

diff --git a/crates/wysiwyg/src/dom/nodes/text_node.rs b/crates/wysiwyg/src/dom/nodes/text_node.rs
@@ -293,31 +293,10 @@ where
     fn fmt_markdown(
         &self,
         buffer: &mut S,
-        options: &MarkdownOptions,
+        _options: &MarkdownOptions,
         _as_message: bool,
     ) -> Result<(), MarkdownError<S>> {
-        if options.contains(MarkdownOptions::NO_ESCAPE) {
-            buffer.push(self.data());
-            return Ok(());
-        }
-
-        let mut escaped = S::default();
-
-        for c in self.data().chars() {
-            match c {
-                // https://spec.commonmark.org/0.30/#ascii-punctuation-character
-                '!' | '"' | '#' | '$' | '%' | '&' | '\'' | '(' | ')' | '*'
-                | '+' | ',' | '-' | '.' | '/' | ':' | ';' | '<' | '=' | '>'
-                | '?' | '@' | '[' | '\\' | ']' | '^' | '_' | '`' | '{'
-                | '|' | '}' | '~' => {
-                    escaped.push('\\');
-                    escaped.push(c);
-                }
-                _ => escaped.push(c),
-            }
-        }
-
-        buffer.push(escaped);
+        buffer.push(self.data.to_owned());
 
         Ok(())
     }

diff --git a/crates/wysiwyg/src/dom/to_markdown.rs b/crates/wysiwyg/src/dom/to_markdown.rs
@@ -70,7 +70,6 @@ pub struct MarkdownOptions {
 
 impl MarkdownOptions {
     pub const IGNORE_LINE_BREAK: Self = Self { bits: 0b0001 };
-    pub const NO_ESCAPE: Self = Self { bits: 0b0010 };
 
     pub const fn empty() -> Self {
         Self { bits: 0 }

diff --git a/crates/wysiwyg/src/tests/test_to_markdown.rs b/crates/wysiwyg/src/tests/test_to_markdown.rs
@@ -28,12 +28,16 @@ fn text() {
     assert_to_message_md("abc   def", "abc   def");
 }
 
+// Markdown output contains unescaped special characters but this is ok.
+// Athough the plain text content of a Matrix message _may_ contain markdown,
+// the format is not specified. It is more important for the message to be
+// human readable than valid or machine readable markdown.
 #[test]
 fn text_with_ascii_punctuation() {
-    assert_to_message_md(r"<em>**b**</em>", r"*\*\*b\*\**");
-    assert_to_message_md(
+    assert_to_md_no_roundtrip(r"<em>**b**</em>", r"***b***");
+    assert_to_md_no_roundtrip(
         r##"!&quot;#$%&amp;'()*+,-./:;&lt;=&gt;?@[\]^_`{|}~"##,
-        r##"\!\"\#\$\%\&\'\(\)\*\+\,\-\.\/\:\;\<\=\>\?\@\[\\\]\^\_\`\{\|\}\~"##,
+        r##"!"#$%&'()*+,-./:;<=>?@[\]^_`{|}~"##,
     );
 }
 
@@ -260,7 +264,7 @@ fn room_mention_for_composer() {
 
 #[test]
 fn at_room_mention_for_message() {
-    assert_to_message_md("@room hello!", "@room hello\\!");
+    assert_to_message_md("@room hello!", "@room hello!");
 }
 
 #[test]
@@ -269,8 +273,8 @@ fn at_room_mention_for_composer() {
 
     assert_eq!(tx(&model), "<a data-mention-type=\"at-room\" href=\"#\" contenteditable=\"false\">@room</a> hello!|");
 
-    assert_eq!(model.get_content_as_markdown(), "<a data-mention-type=\"at-room\" href=\"#\" contenteditable=\"false\">@room</a> hello\\!");
-    assert_eq!(model.get_content_as_message_markdown(), "@room hello\\!");
+    assert_eq!(model.get_content_as_markdown(), "<a data-mention-type=\"at-room\" href=\"#\" contenteditable=\"false\">@room</a> hello!");
+    assert_eq!(model.get_content_as_message_markdown(), "@room hello!");
 }
 
 fn assert_to_md_no_roundtrip(html: &str, expected_markdown: &str) {

diff --git a/...ary-compose/src/androidTest/java/io/element/android/wysiwyg/compose/RichTextEditorTest.kt b/...ary-compose/src/androidTest/java/io/element/android/wysiwyg/compose/RichTextEditorTest.kt
@@ -41,7 +41,7 @@ class RichTextEditorTest {
         assertEquals(MenuAction.None, state.menuAction)
         assertEquals(12 to 12, state.selection)
         assertEquals("Hello, world", state.messageHtml)
-        assertEquals("Hello\\, world", state.messageMarkdown)
+        assertEquals("Hello, world", state.messageMarkdown)
     }
 
     @Test
@@ -59,7 +59,7 @@ class RichTextEditorTest {
         assertEquals(12 to 12, state.selection)
         assertEquals(MenuAction.None, state.menuAction)
         assertEquals("Hello, world", state.messageHtml)
-        assertEquals("Hello\\, world", state.messageMarkdown)
+        assertEquals("Hello, world", state.messageMarkdown)
     }
 
     @Test
@@ -84,6 +84,6 @@ class RichTextEditorTest {
         assertEquals(12 to 12, state.selection)
         assertEquals(MenuAction.None, state.menuAction)
         assertEquals("Hello, <b><i>world</i></b>", state.messageHtml)
-        assertEquals("Hello\\, __*world*__", state.messageMarkdown)
+        assertEquals("Hello, __*world*__", state.messageMarkdown)
     }
 }