Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -81,4 +81,9 @@ describe("normalizeDirectiveAttributes / reconstructDirectiveSource", () => {
reconstructDirectiveSource("inline-vis", { file: 'demo"x".html' }),
).toBe('::inline-vis{file="demo\\"x\\".html"}');
});

it("uses the supplied marker for non-leaf directive kinds", () => {
expect(reconstructDirectiveSource("30", {}, ":")).toBe(":30");
expect(reconstructDirectiveSource("note", {}, ":::")).toBe(":::note");
});
});
50 changes: 50 additions & 0 deletions apps/app/src/components/ui/markdown-message-directives.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,56 @@ describe("MarkdownPreview message directives", () => {
expect(screen.getByText('::inline-vis{file="still-streaming')).toBeTruthy();
});

it("renders incidental prose colons literally without dropping text", () => {
// Regression: `remark-directive` parses `13:30`, `a:b`, and `:D` as text
// directives. Before the fix these fell through to mdast-util-to-hast as
// empty block `<div>`s, which split the paragraph and deleted the colon
// text — e.g. "next run 13:30." rendered as "next run 13" + a stray line
// break + ". ...".
const registry = buildMessageDirectiveRegistry([
slot({ id: "inline-vis", pluginId: "demo", component: InlineVis }),
]);
const sentence =
"Watchdog runs every 30 min UTC, next run 13:30. It checks a:b and :D too.";
const { container } = render(
<MarkdownPreview
content={sentence}
messageDirectives={{
registry,
message: MESSAGE,
openWorkspaceFile: null,
}}
/>,
);

const paragraphs = container.querySelectorAll("p");
expect(paragraphs).toHaveLength(1);
// Exact text preserved: no dropped `:30` / `:b` / `:D`.
expect(paragraphs[0]?.textContent).toBe(sentence);
// No block element was injected mid-paragraph to break the line.
expect(paragraphs[0]?.querySelector("div")).toBeNull();
expect(screen.queryByTestId("inline-vis")).toBeNull();
});

it("renders a container directive as literal source text", () => {
const registry = buildMessageDirectiveRegistry([
slot({ id: "inline-vis", pluginId: "demo", component: InlineVis }),
]);
render(
<MarkdownPreview
content={":::note\nhello\n:::"}
messageDirectives={{
registry,
message: MESSAGE,
openWorkspaceFile: null,
}}
/>,
);

expect(screen.queryByTestId("inline-vis")).toBeNull();
expect(screen.getByText(/:::note/)).toBeTruthy();
});

it("falls back to original directive text when the plugin component crashes", () => {
const registry = buildMessageDirectiveRegistry([
slot({ id: "inline-vis", pluginId: "demo", component: CrashVis }),
Expand Down
122 changes: 95 additions & 27 deletions apps/app/src/components/ui/markdown-message-directives.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,18 @@ export interface ResolvedMessageDirectives {
registry: MessageDirectiveRegistry;
}

interface LeafDirectiveNode {
type: "leafDirective";
/**
* `remark-directive` emits three node kinds from a single `:` grammar. Only the
* leaf form (`::name`) mounts a plugin component; the text and container forms
* are handled solely to rewrite them back to literal prose.
*/
type DirectiveNodeType =
| "textDirective"
| "leafDirective"
| "containerDirective";

interface DirectiveNode {
type: DirectiveNodeType;
name?: string;
attributes?: Record<string, string | null | undefined> | null;
children?: unknown[];
Expand All @@ -87,6 +97,13 @@ interface LeafDirectiveNode {
};
}

/** Source marker for each directive kind (used to reconstruct literal text). */
const DIRECTIVE_MARKERS: Record<DirectiveNodeType, string> = {
textDirective: ":",
leafDirective: "::",
containerDirective: ":::",
};

interface RemarkMessageDirectiveFile {
value: unknown;
}
Expand Down Expand Up @@ -177,22 +194,24 @@ export function normalizeDirectiveAttributes(
export function reconstructDirectiveSource(
name: string,
attributes: Readonly<Record<string, string>>,
marker = "::",
): string {
const keys = Object.keys(attributes);
if (keys.length === 0) {
return `::${name}`;
return `${marker}${name}`;
}
const body = keys
.map((key) => `${key}=${JSON.stringify(attributes[key] ?? "")}`)
.join(" ");
return `::${name}{${body}}`;
return `${marker}${name}{${body}}`;
}

function directiveSourceFromNode(
node: LeafDirectiveNode,
node: DirectiveNode,
markdownSource: string,
name: string,
attributes: Readonly<Record<string, string>>,
marker: string,
): string {
const start = node.position?.start?.offset;
const end = node.position?.end?.offset;
Expand All @@ -205,7 +224,7 @@ function directiveSourceFromNode(
) {
return markdownSource.slice(start, end);
}
return reconstructDirectiveSource(name, attributes);
return reconstructDirectiveSource(name, attributes, marker);
}

function messageDirectiveMountNode(index: number): RootContent {
Expand All @@ -221,25 +240,46 @@ function messageDirectiveMountNode(index: number): RootContent {
};
}

function literalDirectiveNode(source: string): RootContent {
function literalDirectiveNode(
type: DirectiveNodeType,
source: string,
): RootContent {
// A text directive lives inside phrasing content (e.g. mid-paragraph), so it
// must be rewritten to an inline text node — not a block paragraph, which
// would split the surrounding text onto its own line. Leaf and container
// directives are block-level, so a paragraph is the correct literal stand-in.
if (type === "textDirective") {
return { type: "text", value: source };
}
return {
type: "paragraph",
children: [{ type: "text", value: source }],
};
}

function isLeafDirective(node: unknown): node is LeafDirectiveNode {
return (
typeof node === "object" &&
node !== null &&
(node as { type?: unknown }).type === "leafDirective"
);
function asDirectiveNode(node: unknown): DirectiveNode | null {
if (typeof node !== "object" || node === null) {
return null;
}
const type = (node as { type?: unknown }).type;
if (
type === "textDirective" ||
type === "leafDirective" ||
type === "containerDirective"
) {
return node as DirectiveNode;
}
return null;
}

/**
* Remark transformer: rewrite recognized leaf directives into indexed custom
* elements; leave unknown / collision / over-limit directives as literal
* source text. Must run after `remark-directive` has produced leaf nodes.
* Remark transformer: rewrite recognized leaf directives (`::name`) into indexed
* custom elements; leave unknown / collision / over-limit leaf directives as
* literal source text. Text directives (`:name`) and container directives
* (`:::name`) never mount and are always rewritten to their literal source, so
* incidental prose colons (`13:30`, `key:value`, `:D`) render verbatim instead
* of collapsing into `mdast-util-to-hast`'s empty-`<div>` fallback. Must run
* after `remark-directive` has produced the directive nodes.
*
* Mutates `mounts` in document order so indices stay stable when later text
* streams in after an already-complete directive.
Expand All @@ -256,35 +296,63 @@ export function remarkMessageDirectives(args: {
// array reference does not accumulate duplicate indices.
mounts.length = 0;
visit(tree, (node, index, parent: Parent | undefined) => {
if (
!isLeafDirective(node) ||
parent === undefined ||
index === undefined
) {
const directive = asDirectiveNode(node);
if (directive === null || parent === undefined || index === undefined) {
return;
}
const name = typeof node.name === "string" ? node.name : "";
const attributes = normalizeDirectiveAttributes(node.attributes);
const marker = DIRECTIVE_MARKERS[directive.type];
const name = typeof directive.name === "string" ? directive.name : "";
const attributes = normalizeDirectiveAttributes(directive.attributes);
const source = directiveSourceFromNode(
node,
directive,
markdownSource,
name,
attributes,
marker,
);

// Only leaf directives (`::name`) mount a plugin component. Text
// directives (`:name`) and container directives (`:::name`) are almost
// always incidental parses of ordinary prose — a time like `13:30`, a
// `key:value` pair, an emoticon like `:D`. Left in the tree they reach
// `mdast-util-to-hast`, which renders an unknown directive as an empty
// block `<div>`; nested inside a paragraph that both drops the directive's
// text and injects a stray line break. Rewrite them back to literal
// source so the prose renders verbatim.
if (directive.type !== "leafDirective") {
parent.children.splice(
index,
1,
literalDirectiveNode(directive.type, source),
);
return index;
}

if (name.length === 0) {
parent.children.splice(index, 1, literalDirectiveNode(source));
parent.children.splice(
index,
1,
literalDirectiveNode(directive.type, source),
);
return index;
}

const entry = registry.get(name);
if (entry === undefined || entry.status === "collision") {
parent.children.splice(index, 1, literalDirectiveNode(source));
parent.children.splice(
index,
1,
literalDirectiveNode(directive.type, source),
);
return index;
}

if (mounts.length >= MESSAGE_DIRECTIVE_MOUNT_LIMIT) {
parent.children.splice(index, 1, literalDirectiveNode(source));
parent.children.splice(
index,
1,
literalDirectiveNode(directive.type, source),
);
return index;
}

Expand Down