Skip to content

Commit 219d0d1

Browse files
authored
Fix block quotes doing mitosis (#893)
should preserve the new lines more accurately and probably fixes a couple other bugs <!-- Please read https://github.com/SableClient/Sable/blob/dev/CONTRIBUTING.md before submitting your pull request --> ### Description <!-- Please include a summary of the change. Please also include relevant motivation and context. List any dependencies that are required for this change. --> Fixes block quotes creating extra block quote lines when editing a message/bio Internal because its covered by the existing changesets for this update. #### Type of change - [x] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] This change requires a documentation update ### Checklist: - [ ] My code follows the style guidelines of this project - [ ] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings ### AI disclosure: - [x] Partially AI assisted (clarify which code was AI assisted and briefly explain what it does). - [ ] Fully AI generated (explain what all the generated code does in moderate detail). <!-- Write any explanation required here, but do not generate the explanation using AI!! You must prove you understand what the code in this PR does. --> Tests were updated by AI
2 parents ed1edab + ed80f65 commit 219d0d1

6 files changed

Lines changed: 111 additions & 25 deletions

File tree

src/app/plugins/markdown/bidirectional.test.ts

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -73,11 +73,16 @@ describe('bidirectional round-trip', () => {
7373
});
7474

7575
it('round-trips blockquotes', () => {
76-
const markdown = '> Quote text';
77-
const html = markdownToHtml(markdown);
78-
const injected = injectDataMd(html);
79-
const result = htmlToMarkdown(injected);
80-
expect(result).toContain('> Quote text');
76+
const roundtrip = (markdown: string) => {
77+
const html = markdownToHtml(markdown);
78+
const injected = injectDataMd(html);
79+
return htmlToMarkdown(injected);
80+
};
81+
expect(roundtrip('> Quote text')).toBe('> Quote text');
82+
expect(roundtrip('> line one\n> line two')).toBe('> line one\n> line two');
83+
expect(roundtrip('> test\ntest')).toBe('> test\ntest');
84+
expect(roundtrip('> test\n\n> test')).toBe('> test\n\n> test');
85+
expect((markdownToHtml('> test\n\n> test').match(/<blockquote/g) ?? []).length).toBe(2);
8186
});
8287

8388
it('round-trips unordered lists', () => {
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
import { describe, expect, it } from 'vitest';
2+
import { expandBlockBoundariesAfterSingleNewlines } from './expandBlockNewlines';
3+
import { markdownToHtml } from './markdownToHtml';
4+
5+
describe('expandBlockBoundariesAfterSingleNewlines', () => {
6+
it('does not expand between consecutive blockquote lines', () => {
7+
const md = '> test\n> test\n> test';
8+
expect(expandBlockBoundariesAfterSingleNewlines(md)).toBe(md);
9+
});
10+
11+
it('still expands before the first blockquote line', () => {
12+
expect(expandBlockBoundariesAfterSingleNewlines('intro\n> quote')).toBe('intro\n\n> quote');
13+
});
14+
15+
it('still expands when a blockquote ends', () => {
16+
expect(expandBlockBoundariesAfterSingleNewlines('> quote\nplain')).toBe('> quote\n\nplain');
17+
});
18+
});
19+
20+
describe('consecutive blockquotes', () => {
21+
it('produces a single blockquote element', () => {
22+
const html = markdownToHtml('> test\n> test\n> test');
23+
expect((html.match(/<blockquote/g) ?? []).length).toBe(1);
24+
});
25+
});

src/app/plugins/markdown/expandBlockNewlines.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,10 @@ function nextLineIsBlockStarter(md: string, newlineIdx: number): boolean {
136136
}
137137

138138
function shouldExpandSingleNewline(md: string, newlineIdx: number): boolean {
139+
// Consecutive `>` lines belong to one blockquote, keep the single `\n` between them.
140+
if (prevLineIsBlockquote(md, newlineIdx) && nextLineContinuesBlockquote(md, newlineIdx)) {
141+
return false;
142+
}
139143
if (nextLineIsBlockStarter(md, newlineIdx)) return true;
140144
// CommonMark lazy continuation keeps non-`>` lines inside blockquotes, close on single `\n`.
141145
if (prevLineIsBlockquote(md, newlineIdx) && !nextLineContinuesBlockquote(md, newlineIdx)) {

src/app/plugins/markdown/htmlToMarkdown.test.ts

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -100,9 +100,17 @@ describe('htmlToMarkdown', () => {
100100
});
101101

102102
it('converts blockquotes', () => {
103-
const result = htmlToMarkdown('<blockquote>Quote text</blockquote>');
104-
expect(result).toContain('>');
105-
expect(result).toContain('Quote text');
103+
expect(htmlToMarkdown('<blockquote>Quote text</blockquote>')).toBe('> Quote text');
104+
expect(htmlToMarkdown('<blockquote><p>test</p></blockquote><p>test</p>')).toBe('> test\ntest');
105+
expect(htmlToMarkdown('<blockquote><p>line one</p><p>line two</p></blockquote>')).toBe(
106+
'> line one\n> line two'
107+
);
108+
expect(htmlToMarkdown('<blockquote><p>line one<br>line two</p></blockquote>')).toBe(
109+
'> line one\n> line two'
110+
);
111+
expect(
112+
htmlToMarkdown('<blockquote><p>first</p></blockquote><blockquote><p>second</p></blockquote>')
113+
).toBe('> first\n\n> second');
106114
});
107115

108116
it('converts unordered lists', () => {

src/app/plugins/markdown/htmlToMarkdown.ts

Lines changed: 55 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -66,15 +66,15 @@ function processNodes(nodes: ChildNode[]): string {
6666
const prev = filtered[i - 1];
6767
// Adjacent <p> blocks must become \n\n in markdown so the editor gets separate Slate
6868
// paragraphs and marked emits <p> per block again on send (single \n would collapse).
69-
if (
70-
i > 0 &&
71-
prev &&
72-
isTag(prev) &&
73-
isTag(cur) &&
74-
prev.name.toLowerCase() === 'p' &&
75-
cur.name.toLowerCase() === 'p'
76-
) {
77-
parts.push('\n');
69+
if (i > 0 && prev && isTag(prev) && isTag(cur)) {
70+
const prevTag = prev.name.toLowerCase();
71+
const curTag = cur.name.toLowerCase();
72+
if (
73+
(prevTag === 'p' && curTag === 'p') ||
74+
(prevTag === 'blockquote' && curTag === 'blockquote')
75+
) {
76+
parts.push('\n');
77+
}
7878
}
7979
parts.push(processNode(cur));
8080
}
@@ -333,19 +333,57 @@ function processParagraph(
333333
return `${content}\n`;
334334
}
335335

336+
function collectBlockquoteBodyLines(
337+
node: Element,
338+
listDepth: number,
339+
insideCode: boolean
340+
): string[] {
341+
const lines: string[] = [];
342+
const pushLine = (line: string) => {
343+
lines.push(line);
344+
};
345+
const pushMultiline = (text: string) => {
346+
for (const part of text.split('\n')) {
347+
pushLine(part);
348+
}
349+
};
350+
351+
for (const child of node.children) {
352+
if (isText(child)) {
353+
if (/^\s*$/.test(child.data)) continue;
354+
const text = insideCode ? child.data : escapeMarkdownInlineSequences(child.data);
355+
pushMultiline(text);
356+
continue;
357+
}
358+
359+
if (!isTag(child)) continue;
360+
361+
const tag = child.name.toLowerCase();
362+
if (tag === 'p') {
363+
pushMultiline(processChildren(child.children, listDepth, insideCode));
364+
} else if (tag === 'br') {
365+
pushLine('');
366+
} else if (tag === 'blockquote') {
367+
lines.push(...collectBlockquoteBodyLines(child, listDepth, insideCode));
368+
} else {
369+
pushMultiline(processNode(child, listDepth, insideCode).trimEnd());
370+
}
371+
}
372+
373+
return lines;
374+
}
375+
336376
function processBlockquote(
337377
node: Element,
338378
listDepth: number = 0,
339379
insideCode: boolean = false
340380
): string {
341-
const content = node.children
342-
.map((child) => {
343-
if (isTag(child) && child.name === 'br') return '\n';
344-
const text = processNode(child, listDepth, insideCode);
345-
return text.replace(/\n/g, '\n> ');
346-
})
347-
.join('');
348-
return `> ${content}\n`;
381+
const marker = node.attribs['data-md'] ? `${node.attribs['data-md']} ` : '> ';
382+
const lines = collectBlockquoteBodyLines(node, listDepth, insideCode);
383+
const body = lines
384+
.map((line) => (line.length === 0 ? marker.trimEnd() : `${marker}${line}`))
385+
.join('\n');
386+
return `${body}\n`;
349387
}
350388

351389
/**

src/app/plugins/markdown/markdownToHtml.test.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,12 @@ describe('markdownToHtml', () => {
163163
expect(html).toContain('<blockquote>');
164164
expect(html).toContain('line one');
165165
expect(html).toContain('line two');
166+
expect((html.match(/<blockquote/g) ?? []).length).toBe(1);
167+
});
168+
169+
it('keeps three or more consecutive blockquote lines in one blockquote', () => {
170+
const html = markdownToHtml('> test\n> test\n> test');
171+
expect((html.match(/<blockquote/g) ?? []).length).toBe(1);
166172
});
167173

168174
it('does not promote -# inside fenced code when the fence follows a single newline', () => {

0 commit comments

Comments
 (0)