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

Fix markdown formatting for bold #7257

Merged
merged 2 commits into from
Dec 2, 2021
Merged

Fix markdown formatting for bold #7257

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
14b12c0
Fix markdown formatting for bold
Dec 2, 2021
40649dd
I hate you too eslint
Dec 2, 2021
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
18 changes: 12 additions & 6 deletions src/Markdown.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,11 @@ function getTextUntilEndOrLinebreak(node: commonmark.Node) {
return text;
}

const formattingChangesByNodeType = {
'emph': '_',
'strong': '__',
};

/**
* Class that wraps commonmark, adding the ability to see whether
* a given message actually uses any markdown syntax or whether
Expand Down Expand Up @@ -128,7 +133,7 @@ export default class Markdown {
let text = '';
let isInPara = false;
let previousNode: commonmark.Node | null = null;
let shouldUnlinkEmphasisNode = false;
let shouldUnlinkFormattingNode = false;
while ((event = walker.next())) {
const { node } = event;
if (node.type === 'paragraph') {
Expand All @@ -152,7 +157,7 @@ export default class Markdown {
text += node.literal;
}
// We should not do this if previous node was not a textnode, as we can't combine it then.
if (node.type === 'emph' && previousNode.type === 'text') {
if ((node.type === 'emph' || node.type === 'strong') && previousNode.type === 'text') {
if (event.entering) {
const foundLinks = linkify.find(text);
for (const { value } of foundLinks) {
Expand All @@ -161,7 +166,8 @@ export default class Markdown {
* NOTE: This technically should unlink the emph node and create LINK nodes instead, adding all the next elements as siblings
* but this solution seems to work well and is hopefully slightly easier to understand too
*/
const nonEmphasizedText = `_${node.firstChild.literal}_`;
const format = formattingChangesByNodeType[node.type];
const nonEmphasizedText = `${format}${node.firstChild.literal}${format}`;
const f = getTextUntilEndOrLinebreak(node);
const newText = value + nonEmphasizedText + f;
const newLinks = linkify.find(newText);
Expand All @@ -175,7 +181,7 @@ export default class Markdown {
// Remove `em` opening and closing nodes
node.unlink();
previousNode.insertAfter(event.node);
shouldUnlinkEmphasisNode = true;
shouldUnlinkFormattingNode = true;
} else {
logger.error(
"Markdown links escaping found too many links for following text: ",
Expand All @@ -189,9 +195,9 @@ export default class Markdown {
}
}
} else {
if (shouldUnlinkEmphasisNode) {
if (shouldUnlinkFormattingNode) {
node.unlink();
shouldUnlinkEmphasisNode = false;
shouldUnlinkFormattingNode = false;
}
}
}
Expand Down
28 changes: 26 additions & 2 deletions test/Markdown-test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ describe("Markdown parser test", () => {
"https://riot.im/app/#/room/#_foonetic_xkcd:matrix.org",
].join("\n");

it('tests that links are getting properly HTML formatted', () => {
it('tests that links with markdown empasis in them are getting properly HTML formatted', () => {
/* eslint-disable max-len */
const expectedResult = [
"<p>Test1:<br />#_foonetic_xkcd:matrix.org<br />http://google.com/_thing_<br />https://matrix.org/_matrix/client/foo/123_<br />#_foonetic_xkcd:matrix.org</p>",
Expand Down Expand Up @@ -125,7 +125,7 @@ describe("Markdown parser test", () => {
expect(md.toHTML()).toEqual(expectedResult);
});

it('expects that links in one line will be "escaped" properly', () => {
it('expects that links with emphasis are "escaped" correctly', () => {
/* eslint-disable max-len */
const testString = [
'http://domain.xyz/foo/bar-_stuff-like-this_-in-it.jpg' + " " + 'http://domain.xyz/foo/bar-_stuff-like-this_-in-it.jpg',
Expand All @@ -139,5 +139,29 @@ describe("Markdown parser test", () => {
const md = new Markdown(testString);
expect(md.toHTML()).toEqual(expectedResult);
});

it('expects that the link part will not be accidentally added to <strong>', () => {
/* eslint-disable max-len */
const testString = `https://github.com/matrix-org/synapse/blob/develop/synapse/module_api/__init__.py`;
const expectedResult = 'https://github.com/matrix-org/synapse/blob/develop/synapse/module_api/__init__.py';
/* eslint-enable max-len */
const md = new Markdown(testString);
expect(md.toHTML()).toEqual(expectedResult);
});

it('expects that the link part will not be accidentally added to <strong> for multiline links', () => {
/* eslint-disable max-len */
const testString = [
'https://github.com/matrix-org/synapse/blob/develop/synapse/module_api/__init__.py' + " " + 'https://github.com/matrix-org/synapse/blob/develop/synapse/module_api/__init__.py',
'https://github.com/matrix-org/synapse/blob/develop/synapse/module_api/__init__.py' + " " + 'https://github.com/matrix-org/synapse/blob/develop/synapse/module_api/__init__.py',
].join('\n');
const expectedResult = [
'https://github.com/matrix-org/synapse/blob/develop/synapse/module_api/__init__.py' + " " + 'https://github.com/matrix-org/synapse/blob/develop/synapse/module_api/__init__.py',
'https://github.com/matrix-org/synapse/blob/develop/synapse/module_api/__init__.py' + " " + 'https://github.com/matrix-org/synapse/blob/develop/synapse/module_api/__init__.py',
].join('<br />');
/* eslint-enable max-len */
const md = new Markdown(testString);
expect(md.toHTML()).toEqual(expectedResult);
});
});
});