diff --git a/README.md b/README.md index 4c8149e..f8e2d85 100644 --- a/README.md +++ b/README.md @@ -752,6 +752,27 @@ author starts as a draft. An agent has no drafting step — it posts a review it has already decided on — so `"author":"claude"` is born submitted: an open thread, with no **Submit review** click standing between it and being read. +### Comments are markdown + +A comment body is markdown, rendered where it is read: fenced code (highlighted +by the same theme as the diff beside it), `inline code`, bold, italic, +strikethrough, links, bare URLs, nested and numbered lists with `- [ ]` task +items, headings, quotes and rules. Soft line breaks are hard breaks, as they are +in a GitHub comment box — comments are typed in lines, not in paragraphs. + +It matters most for the comments you did not type. An agent writes the way it +writes — a claim, a fenced snippet of the fix, a link — and before this that +arrived as a wall of asterisks and backticks. Nothing about the stored comment +changed: the body is still plain text, in `reviews.json`, and the editor still +edits the text you wrote. Tables and reference links are not rendered; they show +as the text they were typed as. The renderer is `web/src/lib/markdown.tsx`, five hundred +lines and no dependency — the pane already owns a syntax +highlighter, and the shapes a review comment actually uses are a short list. + +The rail's cards and a collapsed thread's one-line preview strip the markup +instead of rendering it: three clamped lines have no room for a code block, and +raw `**syntax**` reads worse than none. + ### The two skills `mise run install-skills` installs them into `~/.claude/skills`, keeping whatever diff --git a/web/src/components/CommentThread.tsx b/web/src/components/CommentThread.tsx index 9dd46c1..1adcedd 100644 --- a/web/src/components/CommentThread.tsx +++ b/web/src/components/CommentThread.tsx @@ -2,6 +2,7 @@ import { useEffect, useRef, useState } from 'react'; import type { ReactNode } from 'react'; import type { Comment } from '../types'; import { api } from '../api'; +import { markdownToText, renderMarkdown } from '../lib/markdown'; import { Icon } from './Icon'; function initials(author: string): string { @@ -37,8 +38,10 @@ export function CommentThread({ comments, onChanged }: Props) { } // summarize reduces a thread to the single line shown while it is collapsed. +// The markup comes off first: one clamped line has no room to render it, and +// raw `**syntax**` reads worse than none. function summarize(body: string): string { - const line = body.trim().split('\n')[0]; + const line = markdownToText(body).split('\n').find((l) => l.trim()) ?? ''; return line.length > 110 ? line.slice(0, 110) + '…' : line; } @@ -270,7 +273,7 @@ function BodyEditor({ }} />
- ⌘⏎ to save · esc to cancel + markdown · ⌘⏎ to save · esc to cancel @@ -330,7 +333,7 @@ function Bubble({ )}
- {editor ??
{body}
} + {editor ??
{renderMarkdown(body)}
} ); diff --git a/web/src/components/CommentsPanel.tsx b/web/src/components/CommentsPanel.tsx index a11e727..dbeae3b 100644 --- a/web/src/components/CommentsPanel.tsx +++ b/web/src/components/CommentsPanel.tsx @@ -2,6 +2,7 @@ import { useMemo, useState } from 'react'; import type { Comment, Status } from '../types'; import { Icon } from './Icon'; +import { markdownToText } from '../lib/markdown'; // Filters are by status, plus `outdated`, which cuts across status: it's about // whether the diff can still place a comment, not where it is in its lifecycle. @@ -209,7 +210,11 @@ export function CommentsPanel({ {c.author === 'claude' ? 'Claude' : 'You'} - {c.body} + {/* Three clamped lines in a narrow rail: the markup + comes off rather than being rendered into it. */} + + {markdownToText(c.body)} + {c.replies.length > 0 && ( {c.replies.length}{' '} diff --git a/web/src/components/Composer.tsx b/web/src/components/Composer.tsx index 7f972cd..55ccf51 100644 --- a/web/src/components/Composer.tsx +++ b/web/src/components/Composer.tsx @@ -41,7 +41,7 @@ export function Composer({ onSubmit, onCancel }: Props) { }} />
- ⌘⏎ to add · esc to cancel + markdown · ⌘⏎ to add · esc to cancel diff --git a/web/src/lib/language.ts b/web/src/lib/language.ts index 8cf8df7..43aa2f4 100644 --- a/web/src/lib/language.ts +++ b/web/src/lib/language.ts @@ -102,6 +102,29 @@ export function languageForFile(path: string): string | null { return null; } +// languageForName resolves the info string on a markdown fence (```ts) to a +// registered refractor language. Fences and file extensions name languages the +// same way often enough that EXT_TO_LANG does most of the work; the rest are +// the spellings people write in a fence but never in a filename. +const FENCE_ALIASES: Record = { + shell: 'bash', + sh: 'bash', + console: 'bash', + 'c++': 'cpp', + 'c#': 'csharp', + dockerfile: 'docker', + make: 'makefile', + text: '', + plain: '', +}; + +export function languageForName(name: string): string | null { + const key = name.trim().toLowerCase(); + if (!key) return null; + const lang = FENCE_ALIASES[key] ?? EXT_TO_LANG[key] ?? key; + return lang && refractor.registered(lang) ? lang : null; +} + // react-diff-view (v3) expects refractor.highlight() to return an ARRAY of // nodes (refractor v3 behavior). refractor v4 returns a `root` node instead, so // we adapt by handing back its children. Pass this to tokenize(). diff --git a/web/src/lib/markdown.tsx b/web/src/lib/markdown.tsx new file mode 100644 index 0000000..5107a3c --- /dev/null +++ b/web/src/lib/markdown.tsx @@ -0,0 +1,501 @@ +import type { ReactNode } from 'react'; + +import { languageForName, refractor } from './language'; + +/* ============================================================================ + markdown — the subset of GitHub-flavoured markdown a review comment uses + + Comments are stored and edited as plain text; this turns that text into + nodes for display. It is deliberately hand-rolled rather than a markdown + dependency: the review UI already owns its own syntax highlighting, the + bundle is served out of the binary, and the shapes that actually turn up in + a review comment are a small, stable list — + + fenced code (highlighted through refractor, same theme as the diff), + `inline code`, **bold**, *italic*, ~~strike~~, [links](https://…), + bare URLs, bullet/numbered lists (nested, with `- [ ]` task items), + # headings, > quotes, --- rules, and paragraphs. + + Tables and reference-style links are not supported; they render as the raw + text they were typed as, which is the same thing a reader would have seen + before any of this existed. Soft line breaks are hard breaks, as they are in + a GitHub comment box — people type comments in lines, not in paragraphs. + ========================================================================== */ + +const FENCE = /^ {0,3}(`{3,}|~{3,})[ \t]*(\S*)/; +const HEADING = /^ {0,3}(#{1,6})[ \t]+(.*?)[ \t]*#*[ \t]*$/; +const HR = /^ {0,3}(?:(?:-[ \t]*){3,}|(?:\*[ \t]*){3,}|(?:_[ \t]*){3,})$/; +const QUOTE = /^ {0,3}>[ \t]?(.*)$/; +const BULLET = /^([ \t]*)([-*+]|\d{1,9}[.)])[ \t]+(.*)$/; +const TASK = /^\[([ xX])\][ \t]+/; +const LINK = /^\[((?:[^\][\\]|\\.)*)\]\([ \t]*)]*)>?(?:[ \t]+"[^"]*")?[ \t]*\)/; +const AUTOLINK = /^<(https?:\/\/[^\s<>]+|mailto:[^\s<>]+)>/; +const BARE_URL = /^https?:\/\/[^\s<>()[\]"'`]+/; +const PUNCT = /[\\`*_{}[\]()#+\-.!~>|]/; + +// renderMarkdown turns a comment body into nodes. The result is a fragment's +// worth of children, so the caller keeps ownership of the wrapper — it needs +// the `md` class on it for the styles to apply. +export function renderMarkdown(body: string): ReactNode { + // Tabs only ever appear as indentation in practice, and every block rule + // below measures indentation in spaces. + return <>{blocks(body.replace(/\t/g, ' ').split('\n'), 'b')}; +} + +// ---- blocks --------------------------------------------------------------- + +function indentOf(line: string): number { + return line.length - line.trimStart().length; +} + +function startsBlock(line: string): boolean { + return ( + FENCE.test(line) || + HEADING.test(line) || + HR.test(line) || + QUOTE.test(line) || + BULLET.test(line) + ); +} + +function blocks(lines: string[], key: string): ReactNode[] { + const out: ReactNode[] = []; + let i = 0; + let n = 0; + + while (i < lines.length) { + const line = lines[i]; + const k = `${key}-${n++}`; + + if (!line.trim()) { + i++; + continue; + } + + const fence = FENCE.exec(line); + if (fence) { + const marker = fence[1]; + let end = i + 1; + while (end < lines.length) { + const close = lines[end].trim(); + if (close.startsWith(marker[0].repeat(marker.length)) && /^(`+|~+)$/.test(close)) break; + end++; + } + out.push(codeBlock(lines.slice(i + 1, end).join('\n'), fence[2], k)); + // An unterminated fence swallows the rest of the comment, which is what + // every markdown renderer does with it. + i = end + 1; + continue; + } + + const heading = HEADING.exec(line); + if (heading) { + const Tag = `h${heading[1].length}` as 'h1'; + out.push( + + {inline(heading[2], k)} + , + ); + i++; + continue; + } + + // Checked before lists: `- - -` is a rule, not three nested bullets. + if (HR.test(line)) { + out.push(
); + i++; + continue; + } + + if (QUOTE.test(line)) { + const quoted: string[] = []; + while (i < lines.length) { + const m = QUOTE.exec(lines[i]); + if (!m) break; + quoted.push(m[1]); + i++; + } + out.push( +
+ {blocks(quoted, k)} +
, + ); + continue; + } + + if (BULLET.test(line)) { + const [node, next] = list(lines, i, k); + out.push(node); + i = next; + continue; + } + + const para: string[] = []; + while (i < lines.length && lines[i].trim() && !startsBlock(lines[i])) { + para.push(lines[i]); + i++; + } + out.push( +

+ {inline(para.join('\n'), k)} +

, + ); + } + + return out; +} + +// list consumes one whole list — every item at the starting indentation, plus +// each item's continuation lines — and returns it with the index just past it. +function list(lines: string[], start: number, key: string): [ReactNode, number] { + const first = BULLET.exec(lines[start])!; + const base = first[1].length; + const ordered = /\d/.test(first[2]); + // How far an item's continuation lines are indented: past the marker. + const pad = base + first[2].length + 1; + + const items: string[][] = []; + let i = start; + + while (i < lines.length) { + const line = lines[i]; + + if (!line.trim()) { + // A blank line is inside the list only if the list carries on after it. + const next = lines[i + 1]; + if ( + next && + next.trim() && + (indentOf(next) > base || (BULLET.test(next) && indentOf(next) >= base)) + ) { + items[items.length - 1]?.push(''); + i++; + continue; + } + break; + } + + const marker = BULLET.exec(line); + const indent = indentOf(line); + + // A marker at (about) the base indentation opens the next item; anything + // indented past it belongs to the item that is open. + if (marker && !HR.test(line) && indent <= base + 1) { + items.push([marker[3]]); + i++; + continue; + } + if (indent > base && items.length > 0) { + items[items.length - 1].push(line.slice(Math.min(indent, pad))); + i++; + continue; + } + break; + } + + const Tag = (ordered ? 'ol' : 'ul') as 'ol'; + const node = ( + + {items.map((item, idx) => listItem(item, `${key}-${idx}`))} + + ); + return [node, i]; +} + +// listItem renders one list item. Its opening lines stay inline so a tight list +// keeps its tight spacing; anything after them (a nested list, a fenced block, +// a second paragraph) goes through the block parser. +function listItem(lines: string[], key: string): ReactNode { + let split = 0; + while (split < lines.length && lines[split].trim() && !startsBlock(lines[split])) split++; + + let lead = lines.slice(0, split).join('\n'); + const task = TASK.exec(lead); + if (task) lead = lead.slice(task[0].length); + + return ( +
  • + {task && ( + + )} + {lead ? inline(lead, key) : null} + {blocks(lines.slice(split), key)} +
    • + ); +} + +// ---- code ----------------------------------------------------------------- + +// The shape refractor hands back: hast, narrowed to what a highlighted tree +// actually contains. +interface HastNode { + type: string; + value?: string; + properties?: { className?: string[] | string }; + children?: HastNode[]; +} + +function codeBlock(code: string, info: string, key: string): ReactNode { + const lang = languageForName(info); + return ( + 
    +      
+        {lang
+          ? hast(refractor.highlight(code, lang).children as unknown as HastNode[])
+          : code}
+      
+
    + ); +} + +function hast(nodes: HastNode[]): ReactNode[] { + return nodes.map((node, i) => { + if (node.type === 'text') return node.value ?? ''; + const cls = node.properties?.className; + return ( + + {hast(node.children ?? [])} + + ); + }); +} + +// ---- inline --------------------------------------------------------------- + +// Only schemes a comment has any business linking to. Anything else — and +// `javascript:` above all — renders as the text it was typed as. +function safeUrl(url: string): string | null { + return /^(https?:\/\/|mailto:)/i.test(url) ? url : null; +} + +// closer finds the index of the closing delimiter for one that opened at +// `from`, skipping escaped ones. -1 when the delimiter is never closed, which +// makes it ordinary text. +function closer(src: string, from: number, delim: string): number { + let i = from; + while (i < src.length) { + const at = src.indexOf(delim, i); + if (at < 0) return -1; + if (src[at - 1] === '\\') { + i = at + delim.length; + continue; + } + return at; + } + return -1; +} + +function inline(src: string, key: string): ReactNode[] { + const out: ReactNode[] = []; + let text = ''; + let n = 0; + + const flush = () => { + if (text) out.push(text); + text = ''; + }; + const push = (node: ReactNode) => { + flush(); + out.push(node); + }; + const nextKey = () => `${key}-i${n++}`; + + let i = 0; + while (i < src.length) { + const ch = src[i]; + + if (ch === '\\' && PUNCT.test(src[i + 1] ?? '')) { + text += src[i + 1]; + i += 2; + continue; + } + + // A comment box turns the newlines you typed into the breaks you meant. + if (ch === '\n') { + push(
    ); + i++; + continue; + } + + if (ch === '`') { + const run = /^`+/.exec(src.slice(i))![0]; + const end = src.indexOf(run, i + run.length); + if (end > 0) { + const code = src.slice(i + run.length, end).replace(/\n/g, ' '); + push( + + {code.length > 2 && code.startsWith(' ') && code.endsWith(' ') + ? code.slice(1, -1) + : code} + , + ); + i = end + run.length; + continue; + } + } + + if (ch === '[') { + const link = LINK.exec(src.slice(i)); + const url = link && safeUrl(link[2]); + if (link && url) { + push( + + {inline(link[1], nextKey())} + , + ); + i += link[0].length; + continue; + } + } + + if (ch === '<') { + const auto = AUTOLINK.exec(src.slice(i)); + if (auto) { + push( + + {auto[1]} + , + ); + i += auto[0].length; + continue; + } + } + + // A URL typed on its own, linked where it starts a word so the `//` inside + // some other string is left alone. + if (ch === 'h' && !/[\w/]/.test(src[i - 1] ?? '')) { + const bare = BARE_URL.exec(src.slice(i)); + if (bare) { + // Sentence punctuation right after a URL is the sentence's, not the + // URL's. + const url = bare[0].replace(/[.,;:!?]+$/, ''); + push( + + {url} + , + ); + i += url.length; + continue; + } + } + + if (ch === '~' && src[i + 1] === '~') { + const end = closer(src, i + 2, '~~'); + if (end > i + 2) { + push({inline(src.slice(i + 2, end), nextKey())}); + i = end + 2; + continue; + } + } + + if (ch === '*' || ch === '_') { + // One marker is italic, two are bold, three are both — past that it is + // someone drawing a line of asterisks, and the run is just text. + const run = Math.min(3, /^(?:\*+|_+)/.exec(src.slice(i))![0].length); + const delim = ch.repeat(run); + // `_` only marks emphasis at a word boundary, so snake_case_names survive + // being talked about. + const wordish = ch === '_' && /\w/.test(src[i - 1] ?? ''); + const end = wordish ? -1 : closer(src, i + run, delim); + const body = end < 0 ? '' : src.slice(i + run, end); + // Markers have to sit against what they emphasise, or `2 * 3 * 4` comes + // out italic; `_` also has to close on a word boundary. + const hugs = !!body && !/^\s|\s$/.test(body); + const closes = ch === '*' || !/\w/.test(src[end + run] ?? ' '); + + if (hugs && closes) { + const content = inline(body, nextKey()); + push( + run === 1 ? ( + {content} + ) : run === 2 ? ( + {content} + ) : ( + + {content} + + ), + ); + i = end + run; + continue; + } + + // A run that emphasises nothing is text — all of it. Consuming it whole + // keeps a stray `**` from leaving a `*` behind to pair with the next one. + text += delim; + i += run; + continue; + } + + text += ch; + i++; + } + + flush(); + return out; +} + +// ---- plain text ----------------------------------------------------------- + +// markdownToText strips the markup back off, for the places that show a +// comment as one clamped line of preview: the rail's cards and a collapsed +// resolved thread. Rendering markdown there would fight the clamp, but showing +// the raw `**syntax**` is worse than showing neither. +export function markdownToText(body: string): string { + const lines: string[] = []; + let fenced = false; + + for (const line of body.replace(/\t/g, ' ').split('\n')) { + if (FENCE.test(line)) { + fenced = !fenced; + continue; + } + if (fenced) { + lines.push(line.trim()); + continue; + } + lines.push( + line + .replace(HEADING, '$2') + .replace(QUOTE, '$1') + .replace(BULLET, '$1$3') + .replace(TASK, ''), + ); + } + + return lines + .join('\n') + .replace(/^ {0,3}(?:[-*_][ \t]*){3,}$/gm, '') // rules leave nothing to read + .replace(/!?\[((?:[^\][\\]|\\.)*)\]\([^)]*\)/g, '$1') // links keep their text + .replace(/<(https?:\/\/[^\s<>]+)>/g, '$1') + .replace(/`+/g, '') + .replace(/(\*\*|__|~~)/g, '') + .replace(/(^|[^\w\\])[*_]([^\s*_][^*_]*)[*_]/g, '$1$2') + .replace(/\\([\\`*_{}[\]()#+\-.!~>|])/g, '$1') + .replace(/\n{3,}/g, '\n\n') + .trim(); +} diff --git a/web/src/styles.css b/web/src/styles.css index 1bf76f1..df36ff3 100644 --- a/web/src/styles.css +++ b/web/src/styles.css @@ -1697,68 +1697,69 @@ body.is-resizing { } /* ---- Primer prettylights syntax theme ----------------------------------- */ -/* Prism's token names mapped onto GitHub's syntax colours. */ -.diff .token.comment, -.diff .token.prolog, -.diff .token.doctype, -.diff .token.cdata { +/* Prism's token names mapped onto GitHub's syntax colours. Both places code is + highlighted share them: the diff, and a fenced block in a comment. */ +:is(.diff, .md) .token.comment, +:is(.diff, .md) .token.prolog, +:is(.diff, .md) .token.doctype, +:is(.diff, .md) .token.cdata { color: var(--syn-comment); } -.diff .token.punctuation, -.diff .token.operator, -.diff .token.entity { +:is(.diff, .md) .token.punctuation, +:is(.diff, .md) .token.operator, +:is(.diff, .md) .token.entity { color: var(--text); } -.diff .token.keyword, -.diff .token.rule, -.diff .token.atrule, -.diff .token.important, -.diff .token.doctype .token.name { +:is(.diff, .md) .token.keyword, +:is(.diff, .md) .token.rule, +:is(.diff, .md) .token.atrule, +:is(.diff, .md) .token.important, +:is(.diff, .md) .token.doctype .token.name { color: var(--syn-keyword); } -.diff .token.string, -.diff .token.attr-value, -.diff .token.char, -.diff .token.regex, -.diff .token.url { +:is(.diff, .md) .token.string, +:is(.diff, .md) .token.attr-value, +:is(.diff, .md) .token.char, +:is(.diff, .md) .token.regex, +:is(.diff, .md) .token.url { color: var(--syn-string); } -.diff .token.function, -.diff .token.function-variable, -.diff .token.method { +:is(.diff, .md) .token.function, +:is(.diff, .md) .token.function-variable, +:is(.diff, .md) .token.method { color: var(--syn-entity); } -.diff .token.number, -.diff .token.boolean, -.diff .token.constant, -.diff .token.symbol, -.diff .token.property { +:is(.diff, .md) .token.number, +:is(.diff, .md) .token.boolean, +:is(.diff, .md) .token.constant, +:is(.diff, .md) .token.symbol, +:is(.diff, .md) .token.property { color: var(--syn-constant); } -.diff .token.tag, -.diff .token.selector, -.diff .token.attr-name, -.diff .token.namespace { +:is(.diff, .md) .token.tag, +:is(.diff, .md) .token.selector, +:is(.diff, .md) .token.attr-name, +:is(.diff, .md) .token.namespace { color: var(--syn-tag); } -.diff .token.class-name, -.diff .token.builtin, -.diff .token.variable, -.diff .token.parameter { +:is(.diff, .md) .token.class-name, +:is(.diff, .md) .token.builtin, +:is(.diff, .md) .token.variable, +:is(.diff, .md) .token.parameter { color: var(--syn-variable); } -.diff .token.title, -.diff .token.bold { +:is(.diff, .md) .token.title, +:is(.diff, .md) .token.bold { color: var(--syn-heading); font-weight: 600; } -.diff .token.italic { +:is(.diff, .md) .token.italic { font-style: italic; } -.diff .token.inserted { +:is(.diff, .md) .token.inserted { color: var(--add-fg); } -.diff .token.deleted { +:is(.diff, .md) .token.deleted { color: var(--del-fg); } @@ -1922,10 +1923,119 @@ body.is-resizing { font-size: 14px; line-height: 1.5; color: var(--text); - white-space: pre-wrap; word-break: break-word; } +/* ---- Rendered markdown -------------------------------------------------- */ +/* Primer's markdown body, at the scale a comment is read at. Applies wherever + a comment body is rendered rather than previewed — see lib/markdown. */ +.md > *:first-child { + margin-top: 0; +} +.md > *:last-child { + margin-bottom: 0; +} +/* A nested block's last child sits flush with the bottom of what holds it, + the same way the body's does. */ +.md-quote > *:last-child, +.md-item > *:last-child { + margin-bottom: 0; +} +.md-p { + margin: 0 0 8px; +} +.md-h { + margin: 16px 0 8px; + font-weight: 600; + line-height: 1.25; +} +h1.md-h, +h2.md-h { + padding-bottom: 4px; + border-bottom: 1px solid var(--border-muted); +} +h1.md-h { + font-size: 18px; +} +h2.md-h { + font-size: 16px; +} +h3.md-h { + font-size: 15px; +} +h4.md-h, +h5.md-h, +h6.md-h { + font-size: 14px; +} +h6.md-h { + color: var(--muted); +} +.md-list { + margin: 0 0 8px; + padding-left: 22px; +} +.md-list .md-list { + margin: 4px 0 0; +} +.md-item + .md-item { + margin-top: 4px; +} +/* A task item's marker is its checkbox. */ +.md-item.is-task { + list-style: none; + margin-left: -18px; +} +.md-check { + margin: 0 6px 0 0; + vertical-align: middle; + accent-color: var(--accent); +} +.md-code { + padding: 0.15em 0.4em; + font-family: var(--font-mono); + font-size: 85%; + background: var(--neutral-muted); + border-radius: var(--r-md); +} +.md-pre { + margin: 0 0 8px; + padding: 10px 12px; + overflow-x: auto; + background: var(--surface-2); + border: 1px solid var(--border-muted); + border-radius: var(--r-md); +} +.md-pre code { + font-family: var(--font-mono); + font-size: 12px; + line-height: 1.45; + white-space: pre; + tab-size: 4; +} +.md-quote { + margin: 0 0 8px; + padding: 0 0 0 12px; + border-left: 3px solid var(--border); + color: var(--muted); +} +.md-hr { + height: 1px; + margin: 12px 0; + border: 0; + background: var(--border); +} +.md-link { + color: var(--accent); + text-decoration: none; +} +.md-link:hover { + text-decoration: underline; +} +.md del { + color: var(--muted); +} + /* Primer Label. */ .pill { font-size: 12px;