Skip to content

Commit 9b06e52

Browse files
backnotpropbacknotprop
committed
fix(ui): round-4 review — table pipe escape, callout lists, emoji h-splitter
- TableBlock: buildMarkdownTable now re-escapes literal | as \| in each cell. parseTableContent already unescapes on parse; without the mirror on serialize, the popout's copy-as-markdown produces extra columns for tables with pipes in regex, shell, or boolean content. - AlertBlock + Callout: extract the shared paragraph-and-list body renderer into blocks/proseBody.tsx. Fixes directive callouts (:::note with a bulleted list) rendering as literal hyphens instead of a list. Paragraph lines join with '\n' so InlineMarkdown's hard-break handler still fires. Callout passes an empty text-color class so directive color tokens inherited from the container are preserved. - InlineMarkdown: drop `h` from the plaintext chunk-break class; it was splitting emoji shortcodes like ❤️, 👍, 🤔 at the h, so the :word: pattern never reassembled and transformPlainText couldn't replace the shortcode. Bare URL detection moves inline via emitPlainTextWithBareUrls, which scans chunks for https?:// at word boundaries and emits anchors, passing surrounding text through transformPlainText so emoji + smart punctuation still apply to non-URL slices. - InlineMarkdown: extract trimUrlTail (shared between the top-of-loop URL branch and the new inline scanner) — one balanced-paren trim implementation instead of two. +8 unit tests covering the trim cases (Wikipedia parens, unbalanced brackets, stacked punctuation). - Fixture: section 9 in 13-known-issues.md demonstrates the table copy corruption for manual verification. For provenance purposes, this commit was AI assisted.
1 parent d53dc98 commit 9b06e52

7 files changed

Lines changed: 250 additions & 125 deletions

File tree

‎packages/ui/components/InlineMarkdown.test.ts‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
import { describe, test, expect } from 'bun:test';
2+
import { trimUrlTail } from './InlineMarkdown';
3+
4+
describe('trimUrlTail', () => {
5+
test('trims trailing period', () => {
6+
expect(trimUrlTail('https://foo.com.')).toBe('https://foo.com');
7+
});
8+
9+
test('trims trailing comma / semicolon / question mark', () => {
10+
expect(trimUrlTail('https://foo.com,')).toBe('https://foo.com');
11+
expect(trimUrlTail('https://foo.com;')).toBe('https://foo.com');
12+
expect(trimUrlTail('https://foo.com?')).toBe('https://foo.com?'.replace(/\?$/, ''));
13+
});
14+
15+
test('keeps closing paren when it balances an opener', () => {
16+
expect(trimUrlTail('https://en.wikipedia.org/wiki/Function_(mathematics)')).toBe(
17+
'https://en.wikipedia.org/wiki/Function_(mathematics)',
18+
);
19+
});
20+
21+
test('trims unbalanced closing paren', () => {
22+
expect(trimUrlTail('https://foo.com/path)')).toBe('https://foo.com/path');
23+
});
24+
25+
test('keeps closing bracket when balanced', () => {
26+
expect(trimUrlTail('https://foo.com/[a]')).toBe('https://foo.com/[a]');
27+
});
28+
29+
test('trims unbalanced closing bracket', () => {
30+
expect(trimUrlTail('https://foo.com]')).toBe('https://foo.com');
31+
});
32+
33+
test('trims stacked punctuation', () => {
34+
expect(trimUrlTail('https://foo.com).')).toBe('https://foo.com');
35+
});
36+
37+
test('leaves URL alone when no trailing punctuation', () => {
38+
expect(trimUrlTail('https://foo.com/path')).toBe('https://foo.com/path');
39+
});
40+
});

‎packages/ui/components/InlineMarkdown.tsx‎

Lines changed: 77 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,72 @@ function sanitizeLinkUrl(url: string): string | null {
88
return url;
99
}
1010

11+
// Trim trailing sentence punctuation from a bare URL, but keep closing
12+
// brackets when they balance an opener inside the URL (Wikipedia-style
13+
// https://…/Function_(mathematics) should keep its closing paren).
14+
export function trimUrlTail(url: string): string {
15+
const balanced = (u: string, close: string, open: string): boolean => {
16+
let opens = 0, closes = 0;
17+
for (const c of u) {
18+
if (c === open) opens++;
19+
else if (c === close) closes++;
20+
}
21+
return opens >= closes;
22+
};
23+
while (url.length > 0) {
24+
const last = url[url.length - 1];
25+
if (!/[.,;:!?)\]}>"']/.test(last)) break;
26+
if (last === ')' && balanced(url, ')', '(')) break;
27+
if (last === ']' && balanced(url, ']', '[')) break;
28+
if (last === '}' && balanced(url, '}', '{')) break;
29+
url = url.slice(0, -1);
30+
}
31+
return url;
32+
}
33+
34+
// Scan a plain-text chunk for bare https?:// URLs at word boundaries and
35+
// emit them as anchor nodes, passing surrounding text through
36+
// transformPlainText so emoji shortcodes and smart punctuation still apply
37+
// to the non-URL slices.
38+
function emitPlainTextWithBareUrls(
39+
text: string,
40+
previousChar: string,
41+
parts: React.ReactNode[],
42+
nextKey: () => number,
43+
): void {
44+
if (text.length === 0) return;
45+
const re = /https?:\/\/[^\s<>"']+/g;
46+
let last = 0;
47+
let m: RegExpExecArray | null;
48+
while ((m = re.exec(text)) !== null) {
49+
const before = m.index === 0 ? previousChar : text[m.index - 1];
50+
if (/\w/.test(before)) continue;
51+
const raw = m[0];
52+
const url = trimUrlTail(raw);
53+
const safe = url.length > 0 ? sanitizeLinkUrl(url) : null;
54+
if (!safe) continue;
55+
if (m.index > last) {
56+
parts.push(transformPlainText(text.slice(last, m.index)));
57+
}
58+
parts.push(
59+
<a
60+
key={nextKey()}
61+
href={safe}
62+
target="_blank"
63+
rel="noopener noreferrer"
64+
className="text-primary underline underline-offset-2 hover:text-primary/80"
65+
>
66+
{url}
67+
</a>,
68+
);
69+
last = m.index + url.length;
70+
re.lastIndex = last;
71+
}
72+
if (last < text.length) {
73+
parts.push(transformPlainText(text.slice(last)));
74+
}
75+
}
76+
1177
/**
1278
* Scanner that walks a text string and emits React nodes for inline markdown:
1379
* emphasis (**bold**, *italic*, _italic_, ***both***), `code`, ~~strikethrough~~,
@@ -46,23 +112,7 @@ export const InlineMarkdown: React.FC<{
46112
if (!/\w/.test(previousChar)) {
47113
const bareMatch = remaining.match(/^https?:\/\/[^\s<>"']+/);
48114
if (bareMatch) {
49-
let url = bareMatch[0];
50-
const balanced = (u: string, close: string, open: string): boolean => {
51-
let opens = 0, closes = 0;
52-
for (const c of u) {
53-
if (c === open) opens++;
54-
else if (c === close) closes++;
55-
}
56-
return opens >= closes;
57-
};
58-
while (url.length > 0) {
59-
const last = url[url.length - 1];
60-
if (!/[.,;:!?)\]}>"']/.test(last)) break;
61-
if (last === ')' && balanced(url, ')', '(')) break;
62-
if (last === ']' && balanced(url, ']', '[')) break;
63-
if (last === '}' && balanced(url, '}', '{')) break;
64-
url = url.slice(0, -1);
65-
}
115+
const url = trimUrlTail(bareMatch[0]);
66116
const safe = url.length > 0 ? sanitizeLinkUrl(url) : null;
67117
if (safe) {
68118
parts.push(
@@ -531,18 +581,19 @@ export const InlineMarkdown: React.FC<{
531581
continue;
532582
}
533583

534-
// Find next special character or consume one regular character
535-
const nextSpecial = remaining.slice(1).search(/[\*_`\[!~\\<#h@]/);
584+
// Find next special character or consume one regular character.
585+
// `h` is intentionally NOT in this class — plain-text chunks may contain
586+
// `h` mid-word (e.g. ":heart:", "hello"), and splitting on it breaks
587+
// multi-char patterns like emoji shortcodes. Bare URLs are instead
588+
// detected inline via emitPlainTextWithBareUrls() below.
589+
const nextSpecial = remaining.slice(1).search(/[\*_`\[!~\\<#@]/);
590+
const plainText = nextSpecial === -1 ? remaining : remaining.slice(0, nextSpecial + 1);
591+
emitPlainTextWithBareUrls(plainText, previousChar, parts, () => key++);
592+
previousChar = plainText[plainText.length - 1] || previousChar;
536593
if (nextSpecial === -1) {
537-
parts.push(transformPlainText(remaining));
538-
previousChar = remaining[remaining.length - 1] || previousChar;
539594
break;
540-
} else {
541-
const plainText = remaining.slice(0, nextSpecial + 1);
542-
parts.push(transformPlainText(plainText));
543-
remaining = remaining.slice(nextSpecial + 1);
544-
previousChar = plainText[plainText.length - 1] || previousChar;
545595
}
596+
remaining = remaining.slice(nextSpecial + 1);
546597
}
547598

548599
return <>{parts}</>;

‎packages/ui/components/blocks/AlertBlock.tsx‎

Lines changed: 2 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import React from 'react';
22
import type { AlertKind } from '../../types';
3-
import { InlineMarkdown } from '../InlineMarkdown';
3+
import { renderProseBody } from './proseBody';
44

55
interface AlertBlockProps {
66
blockId: string;
@@ -50,87 +50,7 @@ export const AlertBlock: React.FC<AlertBlockProps> = ({
5050
<Icon kind={kind} />
5151
<span>{TITLE[kind]}</span>
5252
</div>
53-
{renderAlertBody({ body, imageBaseDir, onImageClick, onOpenLinkedDoc, githubRepo })}
53+
{renderProseBody({ body, imageBaseDir, onImageClick, onOpenLinkedDoc, githubRepo })}
5454
</div>
5555
);
5656
};
57-
58-
// Lightweight block-level renderer for alert bodies. Handles paragraphs,
59-
// unordered lists, and ordered lists — the shapes GitHub alerts commonly
60-
// carry. More exotic content (headings, tables, code fences, nested
61-
// alerts) falls back to inline text; add cases here when a real use
62-
// case turns up.
63-
function renderAlertBody(args: {
64-
body: string;
65-
imageBaseDir?: string;
66-
onImageClick?: (src: string, alt: string) => void;
67-
onOpenLinkedDoc?: (path: string) => void;
68-
githubRepo?: string;
69-
}): React.ReactNode {
70-
const { body, imageBaseDir, onImageClick, onOpenLinkedDoc, githubRepo } = args;
71-
const inline = (text: string) => (
72-
<InlineMarkdown
73-
imageBaseDir={imageBaseDir}
74-
onImageClick={onImageClick}
75-
text={text}
76-
onOpenLinkedDoc={onOpenLinkedDoc}
77-
githubRepo={githubRepo}
78-
/>
79-
);
80-
81-
const lines = body.split('\n');
82-
const out: React.ReactNode[] = [];
83-
let paraLines: string[] = [];
84-
let list: { ordered: boolean; items: string[] } | null = null;
85-
let key = 0;
86-
87-
const flushPara = () => {
88-
if (paraLines.length === 0) return;
89-
const text = paraLines.join(' ');
90-
if (text.trim()) {
91-
out.push(
92-
<p key={`p-${key++}`} className={`text-[15px] leading-relaxed text-foreground/90 ${out.length > 0 ? 'mt-2' : ''}`}>
93-
{inline(text)}
94-
</p>,
95-
);
96-
}
97-
paraLines = [];
98-
};
99-
const flushList = () => {
100-
if (!list) return;
101-
const Tag = list.ordered ? 'ol' : 'ul';
102-
const className = `${list.ordered ? 'list-decimal' : 'list-disc'} pl-5 text-[15px] leading-relaxed text-foreground/90 ${out.length > 0 ? 'mt-2' : ''}`;
103-
out.push(
104-
<Tag key={`l-${key++}`} className={className}>
105-
{list.items.map((item, i) => (
106-
<li key={i} className="my-0.5">{inline(item)}</li>
107-
))}
108-
</Tag>,
109-
);
110-
list = null;
111-
};
112-
113-
for (const line of lines) {
114-
if (line.trim() === '') {
115-
flushPara();
116-
flushList();
117-
continue;
118-
}
119-
const listMatch = line.match(/^\s*(\*|-|\d+\.)\s+(.*)$/);
120-
if (listMatch) {
121-
flushPara();
122-
const ordered = /\d/.test(listMatch[1]);
123-
if (!list || list.ordered !== ordered) {
124-
flushList();
125-
list = { ordered, items: [] };
126-
}
127-
list.items.push(listMatch[2]);
128-
} else {
129-
flushList();
130-
paraLines.push(line);
131-
}
132-
}
133-
flushPara();
134-
flushList();
135-
return out;
136-
}

‎packages/ui/components/blocks/Callout.tsx‎

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import React from 'react';
2-
import { InlineMarkdown } from '../InlineMarkdown';
2+
import { renderProseBody } from './proseBody';
33

44
interface CalloutProps {
55
blockId: string;
@@ -26,7 +26,6 @@ export const Callout: React.FC<CalloutProps> = ({
2626
onImageClick,
2727
githubRepo,
2828
}) => {
29-
const paragraphs = body.split(/\n\n+/);
3029
const kindAttr =
3130
blockType === 'alert' ? { 'data-alert-kind': kindAttribute } : { 'data-directive-kind': kindAttribute };
3231
return (
@@ -39,19 +38,17 @@ export const Callout: React.FC<CalloutProps> = ({
3938
<div className={`${blockType}-title text-xs font-semibold uppercase tracking-wide mb-1`}>
4039
{kind}
4140
</div>
42-
{paragraphs.map((para, i) =>
43-
para ? (
44-
<p key={i} className={`text-[15px] leading-relaxed ${i > 0 ? 'mt-2' : ''}`}>
45-
<InlineMarkdown
46-
imageBaseDir={imageBaseDir}
47-
onImageClick={onImageClick}
48-
text={para}
49-
onOpenLinkedDoc={onOpenLinkedDoc}
50-
githubRepo={githubRepo}
51-
/>
52-
</p>
53-
) : null,
54-
)}
41+
{renderProseBody({
42+
body,
43+
// Callout inherits text color from the container (directive tint per
44+
// kind). Only pass size/leading classes, not a text-foreground value.
45+
paragraphClassName: 'text-[15px] leading-relaxed',
46+
listClassName: 'text-[15px] leading-relaxed',
47+
imageBaseDir,
48+
onImageClick,
49+
onOpenLinkedDoc,
50+
githubRepo,
51+
})}
5552
</div>
5653
);
5754
};

‎packages/ui/components/blocks/TableBlock.tsx‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,10 +56,15 @@ export const buildCsvFromRows = (headers: string[], rows: string[][]): string =>
5656
// Inverse of parseTableContent. Whitespace normalized (one space per cell
5757
// padding). The popout uses this for copy-as-markdown when filter or sort
5858
// might change what the user sees.
59+
// parseTableContent unescapes `\|` → `|`, so cells may hold literal pipes.
60+
// Re-escape on the way out — otherwise the serialized table sprouts extra
61+
// columns wherever a cell had a pipe.
62+
const mdCellEscape = (value: string): string => value.replace(/\|/g, '\\|');
63+
5964
export const buildMarkdownTable = (headers: string[], rows: string[][]): string => {
60-
const headerLine = `| ${headers.join(' | ')} |`;
65+
const headerLine = `| ${headers.map(mdCellEscape).join(' | ')} |`;
6166
const separator = `| ${headers.map(() => '---').join(' | ')} |`;
62-
const bodyLines = rows.map((row) => `| ${row.join(' | ')} |`);
67+
const bodyLines = rows.map((row) => `| ${row.map(mdCellEscape).join(' | ')} |`);
6368
return [headerLine, separator, ...bodyLines].join('\n');
6469
};
6570

0 commit comments

Comments
 (0)