Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for inline code with space only character as content #797

Merged
merged 3 commits into from
Sep 10, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 18 additions & 11 deletions __tests__/ExpensiMark-HTML-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ test('Test italic markdown replacement', () => {
const italicTestStartString = 'Note that _this is correctly italicized_\n' + 'Note that `_this is correctly not italicized_` and _following inline contents are italicized_';

const italicTestReplacedString =
'Note that <em>this is correctly italicized</em><br />' + 'Note that <code>_this is correctly not italicized_</code> and <em>following inline contents are italicized</em>';
'Note that <em>this is correctly italicized</em><br />' + 'Note that <code>_this&nbsp;is&nbsp;correctly&nbsp;not&nbsp;italicized_</code> and <em>following inline contents are italicized</em>';

expect(parser.replace(italicTestStartString)).toBe(italicTestReplacedString);
});
Expand Down Expand Up @@ -402,7 +402,7 @@ test('Test critical markdown style links', () => {
'<a href="https://necolas.github.io/react-native-web/docs/?path=/docs/components-pressable--disabled" target="_blank" rel="noreferrer noopener">second no http://</a> ' +
'<a href="https://github.com/Expensify/Expensify.cash/issues/123#:~:text=Please%20work/Expensify.cash" target="_blank" rel="noreferrer noopener">third</a> ' +
'<a href="https://github.com/Expensify/Expensify.cash/issues/123#:~:text=Please%20work/Expensify.cash" target="_blank" rel="noreferrer noopener">third no https://</a> ' +
'<a href="https://google.com" target="_blank" rel="noreferrer noopener">link <code>[inside another link](https://google.com)</code></a> ' +
'<a href="https://google.com" target="_blank" rel="noreferrer noopener">link <code>[inside&nbsp;another&nbsp;link](https://google.com)</code></a> ' +
'<a href="https://google.com" target="_blank" rel="noreferrer noopener">link with an @ in it</a> ' +
'<a href="https://google.com" target="_blank" rel="noreferrer noopener">link with [brackets] inside of it</a> ' +
'<a href="https://google.com" target="_blank" rel="noreferrer noopener">link with smart quotes ‘’“”</a> ' +
Expand Down Expand Up @@ -499,7 +499,7 @@ test('Test inline code with multiple backtick symbols as content', () => {

test('Test inline code blocks with ExpensiMark syntax inside', () => {
const inlineCodeStartString = '`This is how you can write ~strikethrough~, *bold*, and _italics_`';
expect(parser.replace(inlineCodeStartString)).toBe('<code>This is how you can write ~strikethrough~, *bold*, and _italics_</code>');
expect(parser.replace(inlineCodeStartString)).toBe('<code>This&nbsp;is&nbsp;how&nbsp;you&nbsp;can&nbsp;write&nbsp;~strikethrough~,&nbsp;*bold*,&nbsp;and&nbsp;_italics_</code>');
});

test('Test inline code blocks inside ExpensiMark', () => {
Expand Down Expand Up @@ -1243,17 +1243,24 @@ test('Test for backticks with complete escaped backtick characters inside it', (
expect(parser.replace(testString)).toBe(resultString);
});

// Backticks with no content are not replaced with <code>
test('Test for backticks with no content', () => {
const testString = '` `';
const resultString = '&#x60; &#x60;';
// Backticks with only tab characters inside it are not replaced with <code>
test('Test for backticks only tab characters inside it', () => {
const testString = '`\u0009`';
const resultString = '&#x60;\u0009&#x60;';
expect(parser.replace(testString)).toBe(resultString);
});

// Code-fence with no content is not replaced with <pre>
test('Test for codefence with no content', () => {
// Backticks with only space characters are replaced with <code>
test('Test for backticks with only space characters as content', () => {
const testString = '` `';
const resultString = '<code>&nbsp;&nbsp;</code>';
expect(parser.replace(testString)).toBe(resultString);
});

// Code-fence with spaces as content
test('Test for inline code block with triple backtick with spaces as content', () => {
const testString = '``` ```';
const resultString = '&#x60;&#x60;&#x60; &#x60;&#x60;&#x60;';
const resultString = '<code>&#x60;&#x60;&nbsp;&nbsp;&nbsp;&#x60;&#x60;</code>';
expect(parser.replace(testString)).toBe(resultString);
});

Expand Down Expand Up @@ -1410,7 +1417,7 @@ test('Test for user mention with text with codefence style', () => {

test('Test for user mention with text with inlineCodeBlock style', () => {
const testString = '`hi @username@expensify.com`';
const resultString = '<code>hi @username@expensify.com</code>';
const resultString = '<code>hi&nbsp;@username@expensify.com</code>';
expect(parser.replace(testString)).toBe(resultString);
});

Expand Down
8 changes: 6 additions & 2 deletions lib/ExpensiMark.ts
Original file line number Diff line number Diff line change
Expand Up @@ -190,8 +190,12 @@ export default class ExpensiMark {
// Use the url escaped version of a backtick (`) symbol. Mobile platforms do not support lookbehinds,
// so capture the first and third group and place them in the replacement.
// but we should not replace backtick symbols if they include <pre> tags between them.
regex: /(\B|_|)&#x60;((?:&#x60;)*)(?!&#x60;)(.*?\S+?.*?)(?<!&#x60;)((?:&#x60;)*)(&#x60;)(\B|_|)(?!&#x60;|[^<]*<\/pre>|[^<]*<\/video>)/gm,
replacement: '$1<code>$2$3$4</code>$6',
// At least one non-whitespace character or a specific whitespace character (" " and "\u00A0")
// must be present inside the backticks.
regex: /(\B|_|)&#x60;((?:&#x60;)*(?!&#x60;).*?[\S| |\u00A0]+?.*?(?<!&#x60;)(?:&#x60;)*)&#x60;(\B|_|)(?!&#x60;|[^<]*<\/pre>|[^<]*<\/video>)/gm,
replacement: (_extras, _match, g1, g2, g3) => {
return `${g1}<code>${g2.replaceAll(' ', '&nbsp;')}</code>${g3}`;

Choose a reason for hiding this comment

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

Should we only replace string with HTML encoding when a g2 contains only whitespaces? Any thoughts

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I was thinking about it as well. I will modify the code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@hoangzinh I have modified the code.

},
},

/**
Expand Down
Loading