From ae5a5298e01664d1d437bf7c5d60897508547c82 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Tue, 8 Sep 2026 18:05:21 +0000 Subject: [PATCH] fix(design-md): text-level edits keep CRLF, one section-boundary rule, control characters quoted - insertMarker and spliceSection normalized every line ending to LF, so a CRLF DESIGN.md came back rewritten beyond the one line they promised to touch. Both detect the file's dominant line ending and restore it. - parseDesignMd and spliceSection each walked headings with their own fence tracking; they now share headingLines (and upsertSection shares headingMatches). An unclosed ``` is treated as prose for that file: it used to swallow every later section on a splice. - A token value carrying a control character (an LLM-extracted font family with an embedded newline) was emitted as a bare multi-line scalar that Bun.YAML rejects, turning a freshly written DESIGN.md into frontmatter-unparsable; needsQuotes routes it through the quoted form. - The marker-line regex variants are built once beside YAML_MARKER_RE; the dead setMarker export and a no-op ternary are gone; LEGACY_HEADINGS derives from the identity list; the header diagram names the text-level editors as the write path for user-owned files; the bin validates and prints the mark choices from FORMAT_CHOICES. Tests: CRLF round-trips for both editors, a fenced ## inside a section and an unclosed fence, and a newline-bearing scalar parsing back. Co-Authored-By: Claude Fable 5.1 --- bin/gstack-design-md.ts | 9 ++- lib/design-md.ts | 120 +++++++++++++++++++++++----------------- test/design-md.test.ts | 36 +++++++++++- 3 files changed, 107 insertions(+), 58 deletions(-) diff --git a/bin/gstack-design-md.ts b/bin/gstack-design-md.ts index 23f8c7335..c27ad1232 100755 --- a/bin/gstack-design-md.ts +++ b/bin/gstack-design-md.ts @@ -28,8 +28,7 @@ import { SENTINEL } from '../lib/design-detect-contract'; import { atomicWriteSync } from '../lib/fs-atomic'; import { parseDesignMd, detectFormat, convertLegacy, renderDesignMd, tokensFlat, insertMarker, - type DesignMdDoc, type FormatChoice, -} from '../lib/design-md'; + type DesignMdDoc, type FormatChoice, FORMAT_CHOICES } from '../lib/design-md'; function resolveFile(arg?: string): string { return path.resolve(arg ?? 'DESIGN.md'); @@ -86,8 +85,8 @@ export function main(argv = process.argv.slice(2)): number { } case 'mark': { const choice = positional[0] as FormatChoice | undefined; - if (choice !== 'spec' && choice !== 'legacy-keep') { - process.stderr.write('usage: gstack-design-md.ts mark [DESIGN.md]\n'); + if (!(FORMAT_CHOICES as readonly string[]).includes(choice)) { + process.stderr.write(`usage: gstack-design-md.ts mark <${FORMAT_CHOICES.join('|')}> [DESIGN.md]\n`); return 2; } const file = resolveFile(positional[1]); @@ -103,7 +102,7 @@ export function main(argv = process.argv.slice(2)): number { return 0; } default: - process.stderr.write('usage: gstack-design-md.ts check [file] | convert [file] [--write] | tokens [file] | mark [file]\n'); + process.stderr.write(`usage: gstack-design-md.ts check [file] | convert [file] [--write] | tokens [file] | mark <${FORMAT_CHOICES.join('|')}> [file]\n`); return 2; } } diff --git a/lib/design-md.ts b/lib/design-md.ts index e19875051..c191f7f32 100644 --- a/lib/design-md.ts +++ b/lib/design-md.ts @@ -11,8 +11,12 @@ // ──► convertLegacy ──► gstack's pre-spec DESIGN.md (Product Context, Aesthetic Direction, // Typography, Color, Spacing, Layout, Motion, Decisions Log) becomes // tokens + canonical sections; Motion and Decisions Log survive as extras -// ──► upsertSection ──► body-only splice; front matter bytes are never re-emitted +// ──► upsertSection ──► body-only splice on the parsed doc (files gstack writes from scratch) // ──► renderDesignMd ──► marker, front matter, preamble, canonical sections in order, extras +// text ──► spliceSection / insertMarker ──► text-level edits of a file the USER owns: one section +// body or one marker line changes, every other byte and +// the file's line endings are untouched (the `mark` verb, +// the design binary's extraction section) // ──► tokensFlat ──► "colors.primary" → "#F59E0B"; {path} refs resolved to primitives // // Format marker (the user's one-time conversion answer, persisted in the file): @@ -51,6 +55,10 @@ const MARKER_RE_BODY = FORMAT_MARKER_PREFIX.replace(/[.*+?^${}()|[\]\\]/g, '\\$& const LEGACY_MARKER_RE = new RegExp('^\\n?'); /** `# gstack: design-md-format=...` as a YAML comment inside the front matter (spec files) */ const YAML_MARKER_RE = new RegExp('^# ' + MARKER_RE_BODY + '\\s*$', 'm'); +/** The marker line inside front matter, newline included (renderDesignMd drops it before re-emitting the marker itself). */ +const YAML_MARKER_LINE_RE = new RegExp(YAML_MARKER_RE.source + '\\n', 'm'); +/** A front matter opener immediately followed by the marker line (insertMarker replaces the old choice). */ +const FRONTMATTER_OPEN_WITH_MARKER_RE = new RegExp('^---\\n' + YAML_MARKER_RE.source.replace(/^\^/, '') + '\\n', 'm'); /** Maximum `{path}` reference hops before a chain counts as a cycle. */ export const TOKEN_REF_MAX_HOPS = 8; /** Headings that mark gstack's pre-spec file by themselves (either one is enough evidence of a legacy shape). */ @@ -60,7 +68,7 @@ export type DesignMdFormat = 'spec' | 'legacy' | 'unknown' | 'missing'; export type FormatCode = 'spec' | 'legacy' | 'missing' | 'frontmatter-unparsable' | 'ambiguous' | 'no-token-groups' | 'no-shape'; /** Headings that identify gstack's pre-spec DESIGN.md. */ -export const LEGACY_HEADINGS = ['Product Context', 'Aesthetic Direction', 'Color', 'Spacing', 'Decisions Log']; +export const LEGACY_HEADINGS = [...LEGACY_IDENTITY_HEADINGS, 'Color', 'Spacing', 'Decisions Log']; export interface Section { heading: string; @@ -128,29 +136,44 @@ export function parseDesignMd(text: string): DesignMdDoc { } const lines = rest.split('\n'); - const sections: Section[] = []; - const preambleLines: string[] = []; - let cur: { heading: string; lines: string[] } | null = null; - let inFence = false; - for (const line of lines) { - if (/^```/.test(line)) inFence = !inFence; - const h = !inFence ? line.match(/^## (.+?)\s*$/) : null; - if (h) { - if (cur) sections.push(finish(cur)); - cur = { heading: h[1], lines: [] }; - } else if (cur) { - cur.lines.push(line); - } else { - preambleLines.push(line); - } - } - if (cur) sections.push(finish(cur)); + const heads = headingLines(lines); + const preambleLines = lines.slice(0, heads[0]?.index ?? lines.length); + const sections: Section[] = heads.map((h, k) => { + const canonical = canonicalFor(h.heading); + const body = lines.slice(h.index + 1, heads[k + 1]?.index ?? lines.length).join('\n').replace(/\s+$/, ''); + return { heading: h.heading, ...(canonical ? { canonical } : {}), body }; + }); return { frontmatterText, frontmatter, frontmatterError, marker, preamble: preambleLines.join('\n').trim(), sections }; +} - function finish(c: { heading: string; lines: string[] }): Section { - const canonical = canonicalFor(c.heading); - return { heading: c.heading, ...(canonical ? { canonical } : {}), body: c.lines.join('\n').replace(/\s+$/, '') }; +/** + * The `## ` headings of a body, with code fences skipped. The one section- + * boundary rule, shared by parseDesignMd and spliceSection so they cannot drift. + * An unclosed fence is treated as prose (fence tracking off for that file): + * a stray ``` must never swallow every later section of a file gstack edits. + */ +function headingLines(lines: string[]): Array<{ index: number; heading: string }> { + const fences = lines.filter(l => /^```/.test(l)).length; + const trackFences = fences % 2 === 0; + const out: Array<{ index: number; heading: string }> = []; + let inFence = false; + for (let i = 0; i < lines.length; i++) { + if (trackFences && /^```/.test(lines[i])) { inFence = !inFence; continue; } + if (inFence) continue; + const h = lines[i].match(/^## (.+?)\s*$/); + if (h) out.push({ index: i, heading: h[1] }); } + return out; +} + +/** Does a section heading name the requested section? By canonical name when the request has one, else by exact (case-insensitive) heading. */ +function headingMatches(heading: string, wanted: string, canonical: CanonicalSection | null): boolean { + return canonical ? canonicalFor(heading) === canonical : heading.trim().toLowerCase() === wanted.trim().toLowerCase(); +} + +/** The file's dominant line ending; text-level editors restore it so a CRLF file stays CRLF. */ +function eolOf(text: string): string { + return text.includes('\r\n') ? '\r\n' : '\n'; } // ── Format detection ───────────────────────────────────────────────────────── @@ -184,7 +207,9 @@ export function detectFormat(doc: DesignMdDoc | null): { format: DesignMdFormat; // ── YAML block emitter ─────────────────────────────────────────────────────── function needsQuotes(s: string): boolean { - return s === '' || /^[\s#&*!|>'"%@`{[\]},:?-]|[:#]\s|\s$|^(true|false|null|yes|no|on|off|~)$|^[-+]?(\d+\.?\d*|\.\d+)([eE][-+]?\d+)?$/i.test(s); + // Control characters (an LLM-extracted font family with an embedded newline) must + // go through the double-quoted form: a bare multi-line scalar does not parse back. + return s === '' || /[\x00-\x1f\x7f]/.test(s) || /^[\s#&*!|>'"%@`{[\]},:?-]|[:#]\s|\s$|^(true|false|null|yes|no|on|off|~)$|^[-+]?(\d+\.?\d*|\.\d+)([eE][-+]?\d+)?$/i.test(s); } function yamlScalar(v: unknown): string { @@ -224,7 +249,7 @@ export function renderDesignMd(doc: DesignMdDoc, opts: RenderOptions = {}): stri const parts: string[] = []; const fm = opts.emitFrontmatter && doc.frontmatter ? emitYamlBlock(doc.frontmatter) + '\n' : doc.frontmatterText; if (fm !== null) { - const body = fm.replace(new RegExp(YAML_MARKER_RE.source + '\\n', 'm'), ''); + const body = fm.replace(YAML_MARKER_LINE_RE, ''); parts.push('---'); if (doc.marker) parts.push(`# ${FORMAT_MARKER_PREFIX}${doc.marker}`); parts.push(body.replace(/\n$/, '')); @@ -259,29 +284,24 @@ export function renderDesignMd(doc: DesignMdDoc, opts: RenderOptions = {}): stri * writes from scratch (convert, skeletons). */ export function spliceSection(text: string, heading: string, body: string): string { + const eol = eolOf(text); const src = text.replace(/\r\n/g, '\n'); const canonical = canonicalFor(heading); const lines = src.split('\n'); - let inFence = false; - let start = -1; - let end = lines.length; - for (let i = 0; i < lines.length; i++) { - if (/^```/.test(lines[i])) inFence = !inFence; - if (inFence) continue; - const h = lines[i].match(/^## (.+?)\s*$/); - if (!h) continue; - if (start === -1) { - const matches = canonical ? canonicalFor(h[1]) === canonical : h[1].trim().toLowerCase() === heading.trim().toLowerCase(); - if (matches) start = i; - } else { end = i; break; } - } + const heads = headingLines(lines); + const k = heads.findIndex(h => headingMatches(h.heading, heading, canonical)); const block = `## ${canonical ?? heading}\n\n${body.replace(/\s+$/, '')}\n`; - if (start === -1) { - return src.replace(/\s*$/, '') + '\n\n' + block; + let out: string; + if (k === -1) { + out = src.replace(/\s*$/, '') + '\n\n' + block; + } else { + const start = heads[k].index; + const end = heads[k + 1]?.index ?? lines.length; + const before = lines.slice(0, start).join('\n'); + const after = lines.slice(end).join('\n'); + out = before + (before ? '\n' : '') + block + (after.trim() ? '\n' + after.replace(/^\n+/, '') : ''); } - const before = lines.slice(0, start).join('\n'); - const after = lines.slice(end).join('\n'); - return before + (before ? '\n' : '') + block + (after.trim() ? '\n' + after.replace(/^\n+/, '') : (after.endsWith('\n') ? '' : '')); + return eol === '\n' ? out : out.replace(/\n/g, eol); } /** @@ -290,29 +310,29 @@ export function spliceSection(text: string, heading: string, body: string): stri * marker; every other byte is untouched. */ export function insertMarker(text: string, choice: FormatChoice): string { + const eol = eolOf(text); const src = text.replace(/\r\n/g, '\n'); const stripped = src.replace(LEGACY_MARKER_RE, ''); + let out: string; if (stripped.startsWith('---\n')) { - const withoutOld = stripped.replace(new RegExp('^---\\n' + YAML_MARKER_RE.source.replace(/^\^/, '') + '\\n', 'm'), '---\n'); - return withoutOld.replace(/^---\n/, `---\n# ${FORMAT_MARKER_PREFIX}${choice}\n`); + const withoutOld = stripped.replace(FRONTMATTER_OPEN_WITH_MARKER_RE, '---\n'); + out = withoutOld.replace(/^---\n/, `---\n# ${FORMAT_MARKER_PREFIX}${choice}\n`); + } else { + out = `\n` + stripped; } - return `\n` + stripped; + return eol === '\n' ? out : out.replace(/\n/g, eol); } /** Replace or add a section; canonical names slot into spec order, extras append. Body-only: front matter bytes untouched. */ export function upsertSection(doc: DesignMdDoc, heading: string, body: string): DesignMdDoc { const canonical = canonicalFor(heading); const sections = doc.sections.map(s => ({ ...s })); - const idx = sections.findIndex(s => (canonical ? s.canonical === canonical : s.heading.trim().toLowerCase() === heading.trim().toLowerCase())); + const idx = sections.findIndex(s => headingMatches(s.heading, heading, canonical)); const next: Section = { heading: canonical ?? heading, ...(canonical ? { canonical } : {}), body: body.replace(/\s+$/, '') }; if (idx >= 0) sections[idx] = next; else sections.push(next); return { ...doc, sections }; } -export function setMarker(doc: DesignMdDoc, marker: FormatChoice): DesignMdDoc { - return { ...doc, marker }; -} - // ── Tokens ─────────────────────────────────────────────────────────────────── export interface FlatTokens { diff --git a/test/design-md.test.ts b/test/design-md.test.ts index d2794ae5d..72c152ca8 100644 --- a/test/design-md.test.ts +++ b/test/design-md.test.ts @@ -15,7 +15,7 @@ import * as path from 'path'; import { spawnSync } from 'child_process'; import { parseDesignMd, detectFormat, renderDesignMd, upsertSection, convertLegacy, tokensFlat, - emitYamlBlock, setMarker, specSkeleton, spliceSection, insertMarker, CANONICAL_SECTIONS, TOKEN_GROUPS, isLegacyGstackFormat, + emitYamlBlock, specSkeleton, spliceSection, insertMarker, CANONICAL_SECTIONS, TOKEN_GROUPS, isLegacyGstackFormat, } from '../lib/design-md'; import { updateDesignMd, readDesignConstraints } from '../design/src/memory'; @@ -196,9 +196,9 @@ describe('render + upsert', () => { expect(insertMarker(kept, 'legacy-keep')).toBe(kept); }); - test('setMarker on a spec file writes the YAML comment on line 2 and nothing else moves', () => { + test('a marker on the parsed doc renders as the YAML comment on line 2 and nothing else moves', () => { const noMarker = SPEC.replace('# gstack: design-md-format=spec\n', ''); - const out = renderDesignMd(setMarker(parseDesignMd(noMarker), 'spec')); + const out = renderDesignMd({ ...parseDesignMd(noMarker), marker: 'spec' }); expect(out.split('\n').slice(0, 3)).toEqual(['---', '# gstack: design-md-format=spec', 'name: Heritage']); }); }); @@ -545,3 +545,33 @@ describe('design binary: updateDesignMd is frontmatter-safe', () => { } finally { fs.rmSync(dir, { recursive: true, force: true }); } }); }); + +describe('text-level editors keep line endings and respect fences', () => { + const SPEC_LF = ['---', 'name: x', 'colors:', ' a: "#fff"', '---', '', '## Overview', '', 'o', '', '## Colors', '', 'c', ''].join('\n'); + + test('insertMarker and spliceSection preserve CRLF line endings', () => { + const crlf = SPEC_LF.replace(/\n/g, '\r\n'); + const marked = insertMarker(crlf, 'spec'); + expect(marked).toBe(crlf.replace('---\r\n', '---\r\n# gstack: design-md-format=spec\r\n')); + expect(marked).not.toMatch(/[^\r]\n/); + const legacyCrlf = '# T\r\n\r\n## Product Context\r\n\r\np\r\n'; + expect(insertMarker(legacyCrlf, 'legacy-keep')).toBe('\r\n' + legacyCrlf); + const spliced = spliceSection(crlf, 'Colors', 'Ink only.'); + expect(spliced).toBe(SPEC_LF.replace('## Colors\n\nc\n', '## Colors\n\nInk only.\n').replace(/\n/g, '\r\n')); + expect(spliceSection(SPEC_LF, 'Colors', 'Ink only.')).not.toContain('\r'); + }); + + test('a fenced ## inside a section does not end it; an unclosed fence is prose, so later sections survive', () => { + const src = '## A\n\nbody\n\n```md\n## Not a heading\n```\n\n## B\n\nb body\n'; + expect(spliceSection(src, 'A', 'x')).toBe('## A\n\nx\n\n## B\n\nb body\n'); + expect(parseDesignMd(src).sections.map(s => s.heading)).toEqual(['A', 'B']); + const unclosed = '## A\n\nbody\n\n```\nunclosed\n\n## B\n\nb body\n'; + expect(spliceSection(unclosed, 'A', 'x')).toBe('## A\n\nx\n\n## B\n\nb body\n'); + expect(parseDesignMd(unclosed).sections.map(s => s.heading)).toEqual(['A', 'B']); + }); + + test('a token value with an embedded newline is quoted and parses back', () => { + const yaml = emitYamlBlock({ typography: { body: { fontFamily: 'Foo\nBar', fontSize: '16px\tx' } } }); + expect(Bun.YAML.parse(yaml)).toEqual({ typography: { body: { fontFamily: 'Foo\nBar', fontSize: '16px\tx' } } }); + }); +});