Skip to content
This repository has been archived by the owner on Sep 27, 2024. It is now read-only.

Allow markdown in message format #740

Merged
merged 14 commits into from
Jul 14, 2023

Conversation

artcodespace
Copy link
Contributor

@artcodespace artcodespace commented Jun 30, 2023

This work brings the markdown output in line with html output and allows the client to get markdown either for the composer (in which case it will keep mentions as html) or for a message (in which case it will format the mentions as it currently does). I believe that the changes will also help us to unbreak the iOS implementation of mentions in plain text mode.

This PR:

  • updates bindings
  • adds a new function get_content_as_message_markdown
  • passes an as_message argument through calls to functions that render the model as markdown
  • uses that new argument to conditionally render mentions in the desired way

@artcodespace artcodespace requested a review from a team June 30, 2023 14:39
@codecov-commenter
Copy link

codecov-commenter commented Jun 30, 2023

Codecov Report

Patch coverage: 86.32% and project coverage change: -0.01 ⚠️

Comparison is base (61cdc44) 89.95% compared to head (78db72d) 89.94%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #740      +/-   ##
==========================================
- Coverage   89.95%   89.94%   -0.01%     
==========================================
  Files          83       83              
  Lines       14598    14678      +80     
==========================================
+ Hits        13131    13202      +71     
- Misses       1467     1476       +9     
Flag Coverage Δ
unittests 89.94% <86.32%> (-0.01%) ⬇️
unittests-rust 89.94% <86.32%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
bindings/wysiwyg-ffi/src/ffi_composer_model.rs 27.54% <0.00%> (-0.65%) ⬇️
bindings/wysiwyg-wasm/src/lib.rs 19.38% <0.00%> (-0.10%) ⬇️
crates/wysiwyg/src/dom/nodes/container_node.rs 94.06% <84.61%> (-0.10%) ⬇️
crates/wysiwyg/src/composer_model/base.rs 94.79% <100.00%> (+1.66%) ⬆️
crates/wysiwyg/src/dom/dom_struct.rs 91.24% <100.00%> (+0.01%) ⬆️
crates/wysiwyg/src/dom/nodes/dom_node.rs 88.81% <100.00%> (+0.09%) ⬆️
crates/wysiwyg/src/dom/nodes/line_break_node.rs 96.20% <100.00%> (+0.04%) ⬆️
crates/wysiwyg/src/dom/nodes/mention_node.rs 99.50% <100.00%> (+0.12%) ⬆️
crates/wysiwyg/src/dom/nodes/text_node.rs 94.02% <100.00%> (+0.02%) ⬆️
crates/wysiwyg/src/dom/to_markdown.rs 73.07% <100.00%> (+6.41%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@artcodespace artcodespace force-pushed the alunturner/allow-markdown-in-message-format branch from 43d6440 to b7b5ba5 Compare July 10, 2023 15:16
@sonarcloud
Copy link

sonarcloud bot commented Jul 11, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@@ -669,8 +669,9 @@ where
&self,
buffer: &mut S,
options: &MarkdownOptions,
as_message: bool,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could be done later, but maybe it makes more sense to use a context enum for this to make it clearer what the options are. 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I think that's a fair point, although perhaps that would make sense to do that work if/when the number of formats that could be output extends past two?

@artcodespace artcodespace merged commit 69e0e48 into main Jul 14, 2023
@artcodespace artcodespace deleted the alunturner/allow-markdown-in-message-format branch July 14, 2023 09:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants