From e1ecd27e86d60a62e4eae42b2a58d7c3c733f8ae Mon Sep 17 00:00:00 2001 From: jonnyandrew Date: Tue, 12 Sep 2023 09:27:31 +0100 Subject: [PATCH] Fix unnecessary escape characters in markdown output (#800) --- .../wysiwyg/src/dom/nodes/container_node.rs | 6 ++--- crates/wysiwyg/src/dom/nodes/text_node.rs | 25 ++----------------- crates/wysiwyg/src/dom/to_markdown.rs | 1 - crates/wysiwyg/src/tests/test_to_markdown.rs | 16 +++++++----- .../wysiwyg/compose/RichTextEditorTest.kt | 6 ++--- 5 files changed, 17 insertions(+), 37 deletions(-) diff --git a/crates/wysiwyg/src/dom/nodes/container_node.rs b/crates/wysiwyg/src/dom/nodes/container_node.rs index 9377a0627..2a838f2d2 100644 --- 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( this: &ContainerNode, buffer: &mut S, - options: &mut MarkdownOptions, + options: &MarkdownOptions, as_message: bool, ) -> Result<(), MarkdownError> 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 index 046814dd8..f6315be17 100644 --- 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> { - 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 index 95154e058..c5f7877a0 100644 --- 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 index 95c474b86..c993d9bba 100644 --- 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"**b**", r"*\*\*b\*\**"); - assert_to_message_md( + assert_to_md_no_roundtrip(r"**b**", r"***b***"); + assert_to_md_no_roundtrip( r##"!"#$%&'()*+,-./:;<=>?@[\]^_`{|}~"##, - 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), "@room hello!|"); - assert_eq!(model.get_content_as_markdown(), "@room hello\\!"); - assert_eq!(model.get_content_as_message_markdown(), "@room hello\\!"); + assert_eq!(model.get_content_as_markdown(), "@room 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/platforms/android/library-compose/src/androidTest/java/io/element/android/wysiwyg/compose/RichTextEditorTest.kt b/platforms/android/library-compose/src/androidTest/java/io/element/android/wysiwyg/compose/RichTextEditorTest.kt index e3d0cbe95..0188da429 100644 --- a/platforms/android/library-compose/src/androidTest/java/io/element/android/wysiwyg/compose/RichTextEditorTest.kt +++ b/platforms/android/library-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, world", state.messageHtml) - assertEquals("Hello\\, __*world*__", state.messageMarkdown) + assertEquals("Hello, __*world*__", state.messageMarkdown) } }