syntax-tree
-
|
I noticed that Here's a test case: import { toMarkdown } from "mdast-util-to-markdown";
const tree = {
type: "root",
children: [
{
type: "paragraph",
children: [
{ type: "text", value: "foo" },
{
type: "strong",
children: [{ type: "text", value: " this is bold " }],
},
{ type: "text", value: "bar" },
],
},
],
};
console.log(toMarkdown(tree));Current output: Expected output: |
Beta Was this translation helpful? Give feedback.
Replies: 6 comments 24 replies
-
|
An interesting question @gschlager. The content example in mdast shared itself is not valid markdown. const tree = {
type: "root",
children: [
{
type: "paragraph",
children: [
{ type: "text", value: "foo" },
{
type: "strong",
children: [{ type: "text", value: " this is bold " }],
},
{ type: "text", value: "bar" },
],
},
],
};adds spaces around I'm not sure I see the stringifier as being the place to put content validation/content fixing. |
Beta Was this translation helpful? Give feedback.
-
|
Hi there! From the perspective of
Which ones do you mean?
In the case of
I think this case is similar to |
Beta Was this translation helpful? Give feedback.
-
I was thinking about the escaping of unsafe characters which have special meaning in Markdown. Or this handling of a special case inside lists. Or the escaping and padding with spaces in inline-code. The existing code tries very hard to output valid Markdown. That makes a lot of sense to me. I don't have to know the Markdown spec in order to create a I'd argue that handling leading/trailing whitespaces in
Sure, I can definitely implement it in my mapping function if the decision is that this doesn't belong into |
Beta Was this translation helpful? Give feedback.
-
|
All those cases you list solve valid markdown though. Things can be escaped in markdown. Lists can exist in lists. And code can be padded. Leading/trailing whitespace in emphasis, strong, and strikethrough is not valid markdown. |
Beta Was this translation helpful? Give feedback.
-
|
I fully understand your point. I wasn't thinking about it in the context of valid/invalid Markdown. For me it's more about transforming text based on where it's used.
In my mind that's similar to the escaping that's already implemented.
That's why I thought it would make sense to implement it in |
Beta Was this translation helpful? Give feedback.
-
|
new thread: #60 (comment) |
Beta Was this translation helpful? Give feedback.
-
|
Hey again @gschlager! Sooo, it took me hours and hours of thinking but I managed to reproduce your AST in markdown 😅 -> Pretty doubtful that anyone would ever write that though. But even before I figured it out I already become more understanding of the use cases you mention, where folks are working on the ASTs and injecting punctuation/whitespace/whatever in text or adding/removing emphasis nodes. We already use character references in a couple places. When things can be input:  this -> <p> this </p>…we try and output it too: import {toMarkdown} from 'mdast-util-to-markdown'
/** @type {import('mdast').Root} */
const tree = {
type: 'root',
children: [
{type: 'paragraph', children: [{type: 'text', value: ' this '}]}
]
}
console.log(toMarkdown(tree))->  this That way, it roundtrips. We can turn anything into a character reference. x * this *
x * this *
x *.this.*
x *.this.*-> <p>x * this *</p>
<p>x <em> this </em></p>
<p>x <em>.this.</em></p>
<p>x <em>.this.</em></p>Emphasis can form based on the kind of character before and after the “run”, which can be whitespace, punctuation, or anything else. A bigger example (though still reduced, because left and right runs are the same and only looking at asterisks): | | A (letter inside) | B (punctuation inside) | C (whitespace inside) | D (nothing inside) |
| ----------------------- | ------------- | ------------------ | ----------------- | -------------- |
| 1 (letter outside) | x*y*z | x*.*z | x* *z | x**z |
| 2 (punctuation outside) | .*y*. | .*.*. | .* *. | .**. |
| 3 (whitespace outside) | x *y* z | x *.* z | x * * z | x ** z |
| 4 (nothing outside) | *x* | *.* | * * | ** |->
Inspecting that, we can divide them into two groups: Now given our magic trick: we can turn letters (1 and A) and whitespace (3 and C) into punctuation (2 and B), by turning them into references (which start with
Visualizing that:
We observe some interesting aspects:
In this case though, we only care about list X: we’re looking at an AST import {toMarkdown} from 'mdast-util-from-markdown'
/** @type {import('mdast').Root} */
const tree = {type: 'root', children: [
{type: 'paragraph', children: [{type: 'text', value: 'a '},
{type: 'emphasis', children: []},
{type: 'text', value: ' b'}]}
]}
console.log(toMarkdown(tree))-> (current) a ** b-> From the interesting aspects above, we found that we can add an encoded whitespace (a normal space, zero-width space? No break space?) inside it: punctuation around:
a.* *.b (space)
a.*​*.b (zwsp)
a.* *.b (nbsp)
whitespace around:
a * * b (space)
a *​* b (zwsp)
a * * b (nbsp)-> whitespace around: It’s not perfect, adding that space, as expressed before. a b? |
Beta Was this translation helpful? Give feedback.
-
|
Wow, that's a very detailed analysis. Thank you for that. The workaround I was planning to implement would have removed the leading/trailing whitespace and put it outside of the emphasis. It results in cleaner Markdown, but I understand that you want to be able to roundtrip, so encoding some chars is probably the only solution. It's ugly, but it works, so I'd be fine with it. 🙂
For simplicity I'd go with a normal space.
An empty emphasis doesn't make much sense in my book. I'd drop it. |
Beta Was this translation helpful? Give feedback.
-
|
I stumbled upon this again but I forgot whether this has landed already? Is this still a problem? |
Beta Was this translation helpful? Give feedback.
-
|
Done in https://github.com/syntax-tree/mdast-util-to-markdown/releases/tag/2.1.1. |
Beta Was this translation helpful? Give feedback.
-
|
I stumbled upon this thread trying to figure out how to deal with whitespace in inline HTML elements (created in some wysiwyg editor) when converting it to markdown. I'm running this: ... through Which doesn't work as markdown:
Similarly if So unlike @gschlager I didn't get it from parsing markdown but from parsing HTML to hast to mdast to markdown, still leading to the same invalid output. I'm trying to figure out if I can somehow "sanitize" the whitespace characters to lift them out of the inline elements to prepare for the markdown transformation but I haven't figured it out yet. |
Beta Was this translation helpful? Give feedback.
-
|
I have now come up with this: export function cleanUpSpaces(htmlTree: HastNode): HastNode {
visit(htmlTree, (node, index, parent) => {
if (!index) {
return;
}
if (node.type !== "element") {
return;
}
if (node.tagName !== "strong" && node.tagName !== "em") {
return;
}
if (node.children.length === 0) {
return;
}
const firstChild = node.children[0];
if (firstChild.type === "text") {
const [, startTrim, trimmedValue] =
firstChild.value.match(/^(\s*)([^\s].*)/) || [];
if (startTrim && startTrim.length > 0) {
firstChild.value = trimmedValue;
parent?.children.splice(index, 0, {
type: "text",
value: startTrim,
});
}
}
const lastChild = node.children[node.children.length - 1];
if (lastChild.type === "text") {
const [, trimmedValue, endTrim] =
lastChild.value.match(/([^\s]*)(\s*)$/) || [];
if (endTrim && endTrim.length > 0) {
lastChild.value = trimmedValue;
parent?.children.splice(index + 1, 0, {
type: "text",
value: endTrim,
});
}
}
});
return htmlTree;
}which at least passes these tests: test("sanitizes spaces at the start of emphasis", () => {
expect(
toHtml(
cleanUpSpaces(
fromHtml("<p>A<em> word</em> with emphasis.</p>", { fragment: true })
)
)
).toBe("<p>A <em>word</em> with emphasis.</p>");
});
test("sanitizes spaces at the start of strong", () => {
expect(
toHtml(
cleanUpSpaces(
fromHtml("<p>A<strong> word</strong> with strength.</p>", {
fragment: true,
})
)
)
).toBe("<p>A <strong>word</strong> with strength.</p>");
});
test("sanitizes spaces at the end of emphasis", () => {
expect(
toHtml(
cleanUpSpaces(
fromHtml("<p>A <em>word </em>with emphasis.</p>", { fragment: true })
)
)
).toBe("<p>A <em>word</em> with emphasis.</p>");
});
test("sanitizes spaces at the end of strong", () => {
expect(
toHtml(
cleanUpSpaces(
fromHtml("<p>A <strong>word </strong>with strength.</p>", {
fragment: true,
})
)
)
).toBe("<p>A <strong>word</strong> with strength.</p>");
}); |
Beta Was this translation helpful? Give feedback.
-
|
Also stumbled on this problem recently. Apparently it’s really easy to produce trailing spaces inside emphasis nodes with WYSIWYG editors. I’ve fixed it for my case at the MDAST level with a riff on @bakkerjoeri’s code: /*
Remark plugin to fix trailing whitespace in inline nodes.
See: https://github.com/orgs/syntax-tree/discussions/60
*/
function remarkTrailingWhitespace() {
return function (tree, file) {
visit(tree, ['strong', 'emphasis'], (node, index, parent) => {
// Remove empty nodes
if (!node.children.length) {
parent.children.splice(index, 1);
return SKIP;
}
const first = node.children[0];
// Remove trailing whitespace at the beginning
if (first.type === 'text') {
const start_match = first.value.match(/^(\s+)(.*)/);
if (start_match) {
const [_, start_ws, start_text] = start_match;
parent.children.splice(index, 0, {
type: 'text',
value: start_ws
});
if (start_text) {
// has non-whitespace content
first.value = start_text;
} else {
// all-whitespace content
node.children.shift();
}
// Re-visit this node
return [SKIP, index - 1];
}
}
const last = node.children[node.children.length - 1];
if (last.type === 'text') {
const end_match = last.value.match(/(.*?)(\s+)$/);
if (end_match) {
const [_, end_text, end_ws] = end_match;
parent.children.splice(index + 1, 0, {
type: 'text',
value: end_ws
});
if (end_text) {
last.value = end_text;
} else {
node.children.pop();
}
}
}
return SKIP;
});
};
}It doesn’t cover all the scenarios but was good enough to solve the problem at hand. |
Beta Was this translation helpful? Give feedback.
-
|
I didn't use any of the above solutions because of the edge cases (bold inside italic) etc., So I came up with this very hacky solution just adding a 0 width space to any offending text. import {visit, SKIP, CONTINUE} from 'unist-util-visit';
const REMOVE_TYPES = ['strong', 'emphasis'];
/*
Remark plugin to fix trailing whitespace in inline nodes.
See: https://github.com/orgs/syntax-tree/discussions/60
*/
const START_SPACE = /^\s/;
const END_SPACE = /\s$/;
const ZERO_WIDTH_SPACE = '\u200b';
export function remarkTrailingWhitespace() {
//
// Append zero-width space to start of text nodes that have leading or trailing whitespace
//
return ast => {
visit(ast, REMOVE_TYPES, (node, index, parent) => {
// Remove empty nodes
if (!node.children.length) {
parent.children.splice(index, 1);
return SKIP;
}
// Make certain that empty text nodes are not empty, if they are add a zero-width space to make certain they don't end up as dividers
// Ideally we just delete these.
if(node.children.length === 1 && node.children[0].type === 'text' && node.children[0].value.trim() === ''){
node.children[0].value = ZERO_WIDTH_SPACE;
return CONTINUE;
}
const first = node.children[0];
const last = node.children[node.children.length - 1];
// Append zero-width space to start of text nodes that have leading whitespace
if (first.type === 'text' && START_SPACE.test(first.value)) {
first.value = `${ZERO_WIDTH_SPACE}${first.value.replace(ZERO_WIDTH_SPACE,'')}`;
}
// Append zero-width space to end of text nodes that have trailing whitespace
if (last.type === 'text' && END_SPACE.test(last.value)) {
last.value = `${last.value.replace('\u200b','')}${ZERO_WIDTH_SPACE}`;
}
return CONTINUE;
});
};
} |
Beta Was this translation helpful? Give feedback.
-
|
Done in https://github.com/syntax-tree/mdast-util-to-markdown/releases/tag/2.1.1. |
Beta Was this translation helpful? Give feedback.
-
|
Thanks, @wooorm, works great! |
Beta Was this translation helpful? Give feedback.
-
|
I initially tried to find a solution based on the solution by @danburzo but found out that there were some edge cases in which it also failed. So I wrote my own recursion algorithm that bubbles the space up to the root node and also took care of a number of other possibilities where the translation can fail. If this is a valid solution I would like to contribute this code to the library where the translation is happening. Any help would be welcome const cleanUpSpaces = () => {
return /** @param {import('hast').Nodes | import('mdast').TopLevelContent} htmlTree */ (htmlTree) => {
/**
* @param {import('hast').Parent} node
* @param {import('hast').Parent | null} parent
* @param {Number} index
* @returns
*/
const visitNode = (node, parent, index) => {
// text nodes will not have any children property, so this will do an early return for all such tags.
if (!node.children) {
return;
}
/**
* if strong, del or em tags doesn't have any children(children array is present,
* but is an empty array),remove them as it can cause the translation to break.
* eg: <p><strong></strong></p> -----> <p></p>
*/
if (node.children.length === 0) {
parent?.children.splice(index, 1);
return;
}
/**
* Traversing in reverse order because the visitNode function will change the array in which the node is present,
* and in the case of leading spaces, the space gets extracted and inserted at the index of the node and the node gets displaced
* into the next place, so in the case of forward iteration, this node is again processed.
*
* Also in the case when one node is deleted from the array, the next node will take it's place, and in the case
* of forward iteration, this sibling node can get omitted.
*/
for (let i = node.children.length - 1; i >= 0; i--) {
const child = node.children[i];
visitNode(child, node, i);
}
if (node.type !== 'strong' && node.type !== 'emphasis' && node.type !== 'delete') {
return;
}
const firstChild = node.children[0];
const lastChild = node.children[node.children.length - 1];
if (firstChild.type === 'text') {
/**
* Looking for leading spaces:
* <p><strong><text> Hello</text></strong></p> -----> <p><text> </text><strong><text>Hello</text></strong></p>
*/
const [, leadingSpaces, textValue] = firstChild.value.match(/^(\s+)(.*?)$/) || [];
if (leadingSpaces && leadingSpaces.length > 0) {
firstChild.value = textValue;
parent?.children.splice(index, 0, {
type: 'text',
value: leadingSpaces,
});
// index is incremented because the space was inserted in the place of the node, so the index of the node will change.
index += 1;
}
}
if (lastChild.type === 'text') {
/**
* Looking for trailing spaces:
* <p><strong><text>Hello </text></strong></p> -----> <p><strong><text>Hello</text></strong><text> </text></p>
*/
const [, textValue, trailingSpaces] = lastChild.value.match(/^(.*?)(\s+)$/) || [];
if (trailingSpaces && trailingSpaces.length > 0) {
lastChild.value = textValue;
parent?.children.splice(index + 1, 0, {
type: 'text',
value: trailingSpaces,
});
}
}
/**
* whenever a strong/del/em node has a single text node as a child and the value of the text node is an empty string,
* remove the node.
* eg: <p><strong><text></text></strong></p> ----> <p></p>; This can happen when there was a whitespace in the text
* node, which was identified as a leading space and was extracted outside.
*/
if (firstChild === lastChild && firstChild.type === 'text' && firstChild.value === '') {
parent?.children.splice(index, 1);
}
return;
};
visitNode(htmlTree, null, 0);
};
}; |
Beta Was this translation helpful? Give feedback.
-
|
See https://github.com/orgs/syntax-tree/discussions/60#discussioncomment-2111096 above. In it, I explain in depth what the cause is and how it can be solved. It would need to be implemented in https://github.com/syntax-tree/mdast-util-to-markdown. I’d estimate 1, 2 days of work? |
Beta Was this translation helpful? Give feedback.
-
|
Thanks for the reply, I will go through this thread ! |
Beta Was this translation helpful? Give feedback.
-
|
Hey @abelrasheed / @wooorm , is there a plan of getting this |
Beta Was this translation helpful? Give feedback.
-
|
No, see my earlier comment. It changes ASTs. It changes meaning. It changes content. That is what you can choose to do. It is not something I currently have a bit more time to look at open issues, due to sponsorship, so I may look at it soon. |
Beta Was this translation helpful? Give feedback.
-
|
Done in https://github.com/syntax-tree/mdast-util-to-markdown/releases/tag/2.1.1. |
Beta Was this translation helpful? Give feedback.
-
|
Hi @wooorm , version 2.1.1 or commit syntax-tree/mdast-util-to-markdown@97fb8181 has a breaking change on Markdown round trip. |
Beta Was this translation helpful? Give feedback.
-
|
Oh my god, thank you so much @wooorm 💜 |
Beta Was this translation helpful? Give feedback.
-
|
Thanks for the kind words Anton! :) |
Beta Was this translation helpful? Give feedback.
Hey again @gschlager! Sooo, it took me hours and hours of thinking but I managed to reproduce your AST in markdown 😅
->
foo this is bold bar
Pretty doubtful that anyone would ever write that though. But even before I figured it out I already become more understanding of the use cases you mention, where folks are working on the ASTs and injecting punctuation/whitespace/whatever in text or adding/removing emphasis nodes.
We already use character references in a couple places. When things can be input:
->
…we try and output it too: