From 44f16fedeea4e95f8f912d594476a8907dff24ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Sat, 29 Aug 2026 08:07:54 +0100 Subject: [PATCH] Shift+Enter renders as a line break MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit the message was never wrong, which is worth saying first: Shift+Enter puts a real `\n` in the textarea, and a real `\n` is stored, sent, and given to the agent. what loses it is MARKDOWN'S OWN RULE — inside a paragraph a single line ending is whitespace, and only a blank line starts a new one. "one\ntwo" rendering as "one two" is correct CommonMark. it is right for prose and wrong for a chat box, where somebody pressing Shift+Enter is asking for a line. so the fix is at RENDER time, not on the way to disk. appending two spaces to every line as a message is saved would edit what somebody typed, and every consumer downstream — the agent, a transcript, an export — would carry trailing whitespace nobody asked for to satisfy a display concern. this is what `remark-breaks` does, and it is not in the lockfile. `remark-mentions` beside it already records why that is not a reason to add one: installs are frozen, and a dependency for a short tree walk is a deliberate lockfile change for no benefit. same shape, same file, one more local plugin. IT ONLY TOUCHES TEXT NODES, and that is the property that makes it safe rather than a precaution. by the time this runs the tree is parsed, so a newline inside a fence is the VALUE of a `code` node and never a `text` node — fenced code, inline code and tables cannot be reached. tested, not assumed. empty segments are dropped, so a trailing newline gives ONE break rather than a break followed by an empty text node — which renders as a blank line and is the usual way this ends up double-spacing everything after it. 7 tests: the break itself, several breaks, the trailing-newline case, code and inline code untouched, text without newlines unchanged, and nesting — emphasis, links and mentions all wrap text, so the walk has to go in. Co-Authored-By: Claude Opus 5 Officer-Session: 656f5c85-e49a-4f1f-b9f3-042a04b4d043 --- .../apps/Chat/components/MessageBubble.tsx | 9 +-- .../components/remark-line-breaks.test.ts | 61 +++++++++++++++++ .../Chat/components/remark-line-breaks.ts | 67 +++++++++++++++++++ 3 files changed, 133 insertions(+), 4 deletions(-) create mode 100644 src/workspaces/officerdev/src/apps/Chat/components/remark-line-breaks.test.ts create mode 100644 src/workspaces/officerdev/src/apps/Chat/components/remark-line-breaks.ts diff --git a/src/workspaces/officerdev/src/apps/Chat/components/MessageBubble.tsx b/src/workspaces/officerdev/src/apps/Chat/components/MessageBubble.tsx index 25d1fef0..a5f7e084 100644 --- a/src/workspaces/officerdev/src/apps/Chat/components/MessageBubble.tsx +++ b/src/workspaces/officerdev/src/apps/Chat/components/MessageBubble.tsx @@ -3,6 +3,7 @@ import { useState, useRef, useEffect } from 'react'; import ReactMarkdown from 'react-markdown'; import { remarkMentions, MENTION_HREF } from './remark-mentions'; import remarkGfm from 'remark-gfm'; +import { remarkLineBreaks } from './remark-line-breaks'; import rehypeRaw from 'rehype-raw'; import rehypeSanitize, { defaultSchema } from 'rehype-sanitize'; import { @@ -202,7 +203,7 @@ export const MessageBubble = ({ message, onAnswer, onRetry, onDecidePlan, defaul base64 blob pushes the bubble past max-w and out of the pane. */}
@@ -248,7 +249,7 @@ export const MessageBubble = ({ message, onAnswer, onRetry, onDecidePlan, defaul
@@ -546,7 +547,7 @@ const PlanCard = ({
- + {message.plan}
@@ -637,7 +638,7 @@ export const StreamingBubble = ({ text }: StreamingBubbleProps) => { {text ? (
diff --git a/src/workspaces/officerdev/src/apps/Chat/components/remark-line-breaks.test.ts b/src/workspaces/officerdev/src/apps/Chat/components/remark-line-breaks.test.ts new file mode 100644 index 00000000..ceecb044 --- /dev/null +++ b/src/workspaces/officerdev/src/apps/Chat/components/remark-line-breaks.test.ts @@ -0,0 +1,61 @@ +import { expect, test } from 'bun:test'; +import { remarkLineBreaks } from './remark-line-breaks'; + +type MdNode = { type: string; value?: string; children?: MdNode[] }; + +const run = (tree: MdNode): MdNode => { + remarkLineBreaks()(tree); + return tree; +}; + +const para = (...children: MdNode[]): MdNode => ({ type: 'paragraph', children }); +const text = (value: string): MdNode => ({ type: 'text', value }); + +test('a single newline becomes a break', () => { + const tree = run({ type: 'root', children: [para(text('one\ntwo'))] }); + expect(tree.children![0]!.children).toEqual([ + { type: 'text', value: 'one' }, + { type: 'break' }, + { type: 'text', value: 'two' }, + ]); +}); + +test('several newlines make several breaks', () => { + const tree = run({ type: 'root', children: [para(text('a\nb\nc'))] }); + expect(tree.children![0]!.children!.filter((n) => n.type === 'break')).toHaveLength(2); +}); + +// The empty-segment case: a trailing newline must give one break, never a break plus an empty text +// node, which renders as an extra blank line and double-spaces everything after it. +test('a trailing newline does not leave an empty text node', () => { + const tree = run({ type: 'root', children: [para(text('one\n'))] }); + expect(tree.children![0]!.children).toEqual([{ type: 'text', value: 'one' }, { type: 'break' }]); +}); + +// The property that makes this safe: by the time it runs, a newline inside a fence is the VALUE of a +// `code` node, never a `text` node. Walking text alone cannot reach it. +test('code blocks are untouched', () => { + const code: MdNode = { type: 'code', value: 'line one\nline two' }; + const tree = run({ type: 'root', children: [code] }); + expect(tree.children![0]).toEqual({ type: 'code', value: 'line one\nline two' }); +}); + +test('inline code is untouched', () => { + const tree = run({ type: 'root', children: [para({ type: 'inlineCode', value: 'a\nb' })] }); + expect(tree.children![0]!.children![0]).toEqual({ type: 'inlineCode', value: 'a\nb' }); +}); + +test('text without newlines is left exactly as it was', () => { + const tree = run({ type: 'root', children: [para(text('nothing to do here'))] }); + expect(tree.children![0]!.children).toEqual([{ type: 'text', value: 'nothing to do here' }]); +}); + +// Emphasis, links and mentions all nest text — the walk has to go into them, not just the top level. +test('nested nodes are walked', () => { + const tree = run({ type: 'root', children: [para({ type: 'emphasis', children: [text('one\ntwo')] })] }); + expect(tree.children![0]!.children![0]!.children).toEqual([ + { type: 'text', value: 'one' }, + { type: 'break' }, + { type: 'text', value: 'two' }, + ]); +}); diff --git a/src/workspaces/officerdev/src/apps/Chat/components/remark-line-breaks.ts b/src/workspaces/officerdev/src/apps/Chat/components/remark-line-breaks.ts new file mode 100644 index 00000000..2957559e --- /dev/null +++ b/src/workspaces/officerdev/src/apps/Chat/components/remark-line-breaks.ts @@ -0,0 +1,67 @@ +/** + * Make a single newline a line break, the way every chat app does. + * + * ── The message was never wrong ── + * + * Shift+Enter puts a real `\n` in the textarea and a real `\n` is stored, sent and given to the agent. + * What loses it is MARKDOWN'S OWN RULE: inside a paragraph, a single line ending is whitespace, and only + * a blank line starts a new paragraph. So + * + * one + * two + * + * renders as "one two", correctly, per CommonMark. That is right for prose and wrong for a chat box, + * where somebody pressing Shift+Enter is asking for a line. + * + * The fix therefore belongs at RENDER time. The alternative — appending two spaces to every line as the + * message is saved — would edit what somebody typed on its way to disk, and every consumer downstream + * (the agent, a transcript, an export) would carry trailing whitespace nobody asked for to satisfy a + * display concern. + * + * This is what `remark-breaks` does. It is not in the lockfile, and `remark-mentions` beside this file + * records the reasoning for not adding one: installs are frozen, and a dependency for a short tree walk + * is a deliberate lockfile change for no benefit. + * + * ── Why the split is only inside text nodes ── + * + * By the time this runs the tree is already parsed, so a newline inside a code fence is the VALUE of a + * `code` node and never a `text` node — walking text alone means fenced code, inline code and tables are + * untouched. That is the property that makes this safe: it cannot reach anything where a newline means + * something else. + */ + +type MdNode = { type: string; value?: string; children?: MdNode[] }; + +export function remarkLineBreaks() { + return (tree: MdNode) => { + const walk = (node: MdNode) => { + if (!node.children?.length) return; + + const next: MdNode[] = []; + for (const child of node.children) { + if (child.type !== 'text' || !child.value?.includes('\n')) { + walk(child); + next.push(child); + continue; + } + + /* + `one\ntwo` becomes text("one"), break, text("two"). + + Empty segments are dropped rather than emitted, so a trailing newline produces one break and + not a break followed by an empty text node — which renders as an extra blank line and is the + usual way this ends up double-spacing everything. + */ + const parts = child.value.split('\n'); + parts.forEach((part, index) => { + if (index > 0) next.push({ type: 'break' }); + if (part) next.push({ type: 'text', value: part }); + }); + } + + node.children = next; + }; + + walk(tree); + }; +}