Preserve code fence line breaks and ensure backticks occupy a separate line #496
Conversation
|
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
melvin-bot
bot
requested review from
marcochavezf
and removed request for
a team
|
I have read the CLA Document and I hereby sign the CLA |
|
recheck |
|
@marcochavezf This PR is a part of the App PR which requires changes in the ExpensifyMark (expensify-common). Also tagging @bondydaa. |
| @@ -24,7 +24,7 @@ test('Test multi-line bold markdown replacement', () => { | |||
| // Sections starting with > are successfully wrapped with <blockquote></blockquote> | |||
| test('Test quote markdown replacement', () => { | |||
| const quoteTestStartString = '>This is a *quote* that started on a new line.\nHere is a >quote that did not\n```\nhere is a codefenced quote\n>it should not be quoted\n```'; | |||
| const quoteTestReplacedString = '<blockquote>This is a <strong>quote</strong> that started on a new line.</blockquote>Here is a >quote that did not <pre>here is a codefenced quote<br />>it should not be quoted</pre>'; | |||
| const quoteTestReplacedString = '<blockquote>This is a <strong>quote</strong> that started on a new line.</blockquote>Here is a >quote that did not <pre>here is a codefenced quote<br />>it should not be quoted<br /></pre>'; | |||
There was a problem hiding this comment.
Hmm having to add all of these makes me question if we might have purposefully been removing new lines at the end of codefences.
Going to double check this real quick.
There was a problem hiding this comment.
@bondydaa I think it should not strip the line when doing this conversion, because the context is not known. By that I don't suggest that it should strip it when adding the comment either, given that is the reported issue.
There was a problem hiding this comment.
@bondydaa One possible purpose that comes to mind is the HTML to MD conversion, which always added a new line. This has been fixed as well.
There was a problem hiding this comment.
I've elaborated here Expensify/App#14694 (comment) on the changes
There was a problem hiding this comment.
and here Expensify/App#14694 (comment)
bondydaa
changed the title
Fixed Issues
$ Expensify/App#14694
Tests
The change updates the tests in
ExpensiMark-HTML-test.js, which depended on the previous incorrect behavior (i.e., the line break was always stripped). It also introduces a new test inExpensiMark-Markdown-test.js, which validates the behavior described in the test, which was not covered by existing tests.QA
Steps (validation + regressions)
textonce added.text:New.Expensify.-.Google.Chrome.2023-02-02.20-08-04.mp4