fix: handling empty line break doc at the end. #1776
Conversation
|
@copilot-robot prerelease |
|
Hi, @a-rena, a pre-release has been published:
|
Our markdown syntax does not support teminal line-breaks, either in blocks like a paragraph or just in text. We had correctly been suppressing rendering the slash-escaped line break when it appeared in a paragraph, but not in text
|
@copilot-robot prerelease |
|
Hi, @a-rena, a pre-release has been published:
|
|
@copilot-robot prerelease |
|
Hi, @a-rena, a pre-release has been published:
|
g changes
|
@copilot-robot prerelease |
|
Hi, @a-rena, a pre-release has been published:
|
|
@copilot-robot prerelease |
|
Hi, @a-rena, a pre-release has been published:
|
@tim-evans I'm not sure what you mean by
Accounting for these cases in the rendering context would be a bit complicated (and I think would have to be specifically for each of the cases) compared to the current approach of rendering somewhat naively and just verifying we don't have this specific markdown pattern. But perhaps I am missing a better way to do this - open to suggestions! |
|
@bachbui yea, I think there may be some alternate ways to do this. One thing I would recommend is to switch the test cases not to use the serialized atjson format for testing. |
|
@bachbui I think making a preProcess hook in the renderer hir base class that will allow us to remove line breaks from the peritext document prior to rendering would make the most sense. That way we don't have to deal with lots of context jumping / etc and can handle it in a single pass. |
|
This hook kind of works for most cases, but strips some markdown that we want to keep for commonmark compatibility: protected preProcess(doc: { text: string; marks: Mark[]; blocks: Block[] }) {
// Line breaks cannot end markdown block elements or paragraphs
// https://spec.commonmark.org/0.29/#example-641
let matches = doc.text.match(/[\uFFFC\s]+$/g);
if (matches) {
let text = matches[0];
let i = 0;
let position = text.lastIndexOf("\uFFFC");
if (position !== -1) {
while (position !== -1) {
let block = doc.blocks[doc.blocks.length - i - 1];
if (block.type !== "line-break" && block.type !== "paragraph") {
break;
}
i++;
position = text.lastIndexOf("\uFFFC", position) - 1;
}
return {
text: doc.text.slice(
0,
doc.text.length - (text.length - position - 1)
),
blocks: doc.blocks.slice(0, doc.blocks.length - i),
marks: doc.marks,
};
}
}
return doc;
} |
|
In leiu of preprocessing, I may recommend changing the markdown syntax for line breaks to the 2-space + newline to solve issues of newlines at the end of text. This should be identical to the backslash + newline sequence but not have any side effects that are visible on sites |
Instead of suppressing the rendering of linebreaks when they terminate a block/document, we can change our linebreak md syntax to an alternative that doesn't break the parser. The alternative linebreak sequence is ` \n` instead of `\\\n`. When this appears at the end of a document, it is still parsed as a linebreak, so it is fine to always render it.
|
What a great suggestion. We can determine from our toggle and round trip tests if this produces any issues |
|
@copilot-robot prerelease |
|
Hi, @bachbui, a pre-release has been published:
|
Issue: When rendering document to markdown, if the document ends with empty line break on new line, its returning an additional "" at the of the document.
Fix: Added additional condition on line break Generator to check specific condition.