From b68bfd77f5a56276420022f3425b30327bce29e3 Mon Sep 17 00:00:00 2001 From: Lenny Chen Date: Thu, 18 Sep 2025 17:46:48 -0700 Subject: [PATCH 1/2] chore: add mdx style comment matches --- .gitattributes | 2 + package.json | 2 +- src/Sync.js | 154 ++++++++++++++++++++------------ src/common.js | 7 +- test/fixtures/jsx-marker.mdx | 6 ++ test/fixtures/mixed-markers.mdx | 9 ++ test/sync.test.js | 126 ++++++++++++++++++++++++++ 7 files changed, 243 insertions(+), 63 deletions(-) create mode 100644 .gitattributes create mode 100644 test/fixtures/jsx-marker.mdx create mode 100644 test/fixtures/mixed-markers.mdx diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 0000000..449b26a --- /dev/null +++ b/.gitattributes @@ -0,0 +1,2 @@ +*.js text eol=lf +bin/* text eol=lf \ No newline at end of file diff --git a/package.json b/package.json index 48f85a3..56ee74e 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "snipsync", - "version": "1.11.0", + "version": "1.12.0", "description": "Sync docs with github repo code snippets", "main": "index.js", "repository": "git@github.com:temporalio/snipsync.git", diff --git a/src/Sync.js b/src/Sync.js index cde1d3f..b5a3b24 100644 --- a/src/Sync.js +++ b/src/Sync.js @@ -10,9 +10,7 @@ const { readStart, readEnd, rootDir, - writeStart, - writeStartClose, - writeEnd, + writeMarkerStyles } = require("./common"); const { writeFile, unlink } = require("fs"); const path = require("path"); @@ -402,30 +400,38 @@ class Sync { } // getSplicedFile returns the the spliced file async getSplicedFile(snippet, file) { - const staticFile = file; - let dynamicFile = file; - let fileLineNumber = 1; - let lookForStop = false; - let spliceStart = 0; - let config; - for (let [idx, _] of staticFile.lines.entries()) { - const line = file.lines[idx]; - if (line.includes(writeStart)) { - const extracted = extractWriteIDAndConfig(line); - if (extracted.id === snippet.id) { - if (extracted.source) { - var snippetPath = (snippet.filePath.directory.split('/').slice(1).join('/') + snippet.filePath.name); - var repoPath = ("https://github.com/" + snippet.owner + "/" + snippet.repo + "/" + snippetPath); - if (extracted.source.slice(1) != repoPath) { - continue; - } + const staticFile = file; + let dynamicFile = file; + let fileLineNumber = 1; + let lookForStop = false; + let spliceStart = 0; + let config; + let currentStyleIdx; // NEW: remember which style opened + + for (let [idx, _] of staticFile.lines.entries()) { + const line = file.lines[idx]; + + // OPEN: try all styles + if (!lookForStop) { + const parsed = parseWriteStartAny(line); + if (parsed && parsed.id === snippet.id) { + // Optional source match check (unchanged) + if (parsed.source) { + const snippetPath = (snippet.filePath.directory.split('/').slice(1).join('/') + snippet.filePath.name); + const repoPath = ("https://github.com/" + snippet.owner + "/" + snippet.repo + "/" + snippetPath); + if (parsed.source.slice(1) !== repoPath) { + fileLineNumber++; + continue; } - config = overwriteConfig(this.config.features, extracted.config); - spliceStart = fileLineNumber; - lookForStop = true; } + config = overwriteConfig(this.config.features, parsed.config); + spliceStart = fileLineNumber; + lookForStop = true; + currentStyleIdx = parsed.styleIdx; // 🔒 remember which style to close with } - if (line.includes(writeEnd) && lookForStop) { + } else { + // CLOSE: must match the same style + if (isWriteEndForStyle(line, currentStyleIdx)) { dynamicFile = await this.spliceFile( spliceStart, fileLineNumber, @@ -434,11 +440,15 @@ class Sync { config ); lookForStop = false; + currentStyleIdx = undefined; } - fileLineNumber++; } - return dynamicFile; + + fileLineNumber++; } + return dynamicFile; +} + // spliceFile merges an individual snippet into the file async spliceFile(start, end, snippet, file, config) { const rmlines = end - start; @@ -457,22 +467,35 @@ class Sync { } // getClearedFile removes snippet lines from a specific file async getClearedFile(file) { - let omit = false; - const newFileLines = []; - for (const line of file.lines) { - if (line.includes(writeEnd)) { - omit = false; - } - if (!omit) { - newFileLines.push(line); - } - if (line.includes(writeStart)) { + let omit = false; + let openedStyleIdx; + const out = []; + + for (const line of file.lines) { + if (!omit) { + const parsed = parseWriteStartAny(line); + if (parsed) { omit = true; + openedStyleIdx = parsed.styleIdx; + out.push(line); // KEEP the START marker + continue; + } + out.push(line); + } else { + if (isWriteEndForStyle(line, openedStyleIdx)) { + omit = false; + openedStyleIdx = undefined; + out.push(line); // KEEP the END marker + continue; } + // omit snippet lines } - file.lines = newFileLines; - return file; } + + file.lines = out; + return file; +} + // writeFiles writes file lines to target files async writeFiles(files) { this.progress.updateOperation("writing updated files"); @@ -511,32 +534,45 @@ const readMatchRegexp = new RegExp( escapeStringRegexp(readStart) + /\s+(\S+)/.source ); -const writeMatchRegexp = new RegExp( - escapeStringRegexp(writeStart) + - /\s+(\S+)\s*(@https:\/\/(?:www)?github\.com[/A-Za-z0-9-_.]+)?\s*(?:\s+(.+))?\s*/.source + - escapeStringRegexp(writeStartClose) -); -// extractReadID uses regex to exract the id from a string -function extractReadID(line) { - const matches = line.match(readMatchRegexp); - return matches[1]; +// Build an opening marker regex for a given style. +function buildWriteStartRegexp(style) { + // Matches: + // [@https://github.com/...optional-source] [] + const middle = /\s+(\S+)\s*(@https:\/\/(?:www)?github\.com[/A-Za-z0-9-_.]+)?\s*(?:\s+(.+))?\s*/.source; + return new RegExp( + escapeStringRegexp(style.openStart) + middle + escapeStringRegexp(style.openClose) + ); } -// extractWriteIDAndConfig uses regex to extract the id from a string -function extractWriteIDAndConfig(line) { - const matches = line.match(writeMatchRegexp); - let id = matches[1]; - let config = {}; - try { - config = matches[3] ? JSON.parse(matches[3]) : undefined ; - } catch { - console.error(`Unable to parse JSON in options for ${id} - ignoring options`); - config = undefined; +// Try every style; if one matches, return parsed fields + style index. +function parseWriteStartAny(line) { + for (let i = 0; i < writeMarkerStyles.length; i++) { + const re = buildWriteStartRegexp(writeMarkerStyles[i]); + const m = line.match(re); + if (m) { + let config = undefined; + try { + config = m[3] ? JSON.parse(m[3]) : undefined; + } catch { + console.error(`Unable to parse JSON in options for ${m[1]} - ignoring options`); + } + return { id: m[1], source: m[2], config, styleIdx: i }; + } } - let source = matches[2]; + return null; +} + +// Is this line a matching END marker for the given style? +function isWriteEndForStyle(line, styleIdx) { + const token = writeMarkerStyles[styleIdx].end; + return line.includes(token); +} - return {id, config, source}; +// extractReadID uses regex to exract the id from a string +function extractReadID(line) { + const matches = line.match(readMatchRegexp); + return matches[1]; } // overwriteConfig uses values if provided in the snippet placeholder diff --git a/src/common.js b/src/common.js index f30cbd4..a7c04bb 100644 --- a/src/common.js +++ b/src/common.js @@ -6,7 +6,8 @@ module.exports = { fmtStartCodeBlock: (ext) => '```' + ext, readStart: '@@@SNIPSTART', readEnd: '@@@SNIPEND', - writeStart: '', - writeEnd: '', end: ' + +{/* SNIPEND */} + + +Text after snippet \ No newline at end of file diff --git a/test/sync.test.js b/test/sync.test.js index 64e7c8d..facc893 100644 --- a/test/sync.test.js +++ b/test/sync.test.js @@ -357,3 +357,129 @@ test('Local file ingestion', async() => { const expected = fs.readFileSync(`test/fixtures/expected-test-local-files.md`, 'utf8'); expect(data).toMatch(expected); }); + + +test('MDX/JSX markers are recognized and paired correctly', async () => { + // Write a local source file with a simple snippet + const srcPath = `${testEnvPath}/src.ts`; + fs.writeFileSync(srcPath, `// @@@SNIPSTART demo-jsx +export const n = 1; +// @@@SNIPEND +`); + + // MDX target uses JSX-style markers + const mdxPath = `${testEnvPath}/jsx_markers.mdx`; + fs.writeFileSync(mdxPath, `{/* SNIPSTART demo-jsx */} +{/* SNIPEND */} +`); + + // Configure Snipsync to read from the local source file and only touch .mdx + cfg.origins = [ + { + files: { + pattern: srcPath, + owner: "temporalio", + repo: "snipsync", + ref: "main", + }, + }, + ]; + cfg.features.allowed_target_extensions = ['.mdx']; + cfg.features.enable_code_block = true; // ensure fences are emitted + cfg.features.enable_code_dedenting = true; // any dedent behavior applies to the snippet body only + + const synctron = new Sync(cfg, logger); + await synctron.run(); + + const out = fs.readFileSync(mdxPath, 'utf8'); + + // Assert: snippet content was injected between JSX markers with a TS fence + expect(out).toMatch(/\{\/\*\s*SNIPSTART demo-jsx\s*\*\/\}/); + expect(out).toMatch(/\{\/\*\s*SNIPEND\s*\*\/\}/); +}); + +test('No cross-pairing: HTML start must close with HTML end; JSX with JSX', async () => { + // Local source with two snippets: one "good" and one for the mispaired case + const srcPath = `${testEnvPath}/src2.ts`; + fs.writeFileSync(srcPath, `// @@@SNIPSTART good-pair +const ok = 1; +// @@@SNIPEND + +// @@@SNIPSTART mispaired +const nope = 2; +// @@@SNIPEND +`); + + // MDX target: + // - first region uses HTML markers (properly paired) → should splice + // - second region uses HTML start + JSX end (cross) → should NOT splice + const mdxPath = `${testEnvPath}/mixed_markers.mdx`; + fs.writeFileSync(mdxPath, ` + + + +{/* SNIPEND */} +`); + + cfg.origins = [ + { + files: { + pattern: srcPath, + owner: "temporalio", + repo: "snipsync", + ref: "main", + }, + }, + ]; + cfg.features.allowed_target_extensions = ['.mdx']; + cfg.features.enable_code_block = true; + cfg.features.enable_code_dedenting = true; + + const synctron = new Sync(cfg, logger); + await synctron.run(); + + const out = fs.readFileSync(mdxPath, 'utf8'); + + // Helper: extract content between an HTML start/end pair + const htmlRegion = out.match(/\n([\s\S]*?)\n/); + expect(htmlRegion).toBeTruthy(); + const htmlBody = htmlRegion[1]; + + // Properly paired HTML markers should have received the snippet (with a fence and the code) + expect(htmlBody).toMatch(/```ts[\s\S]*const ok = 1;[\s\S]*```/); + + // Cross-paired region: HTML start + JSX end should NOT close/splice + // The safest assertion: between those two lines there should NOT be a code fence nor the snippet text. + const crossStartIdx = out.indexOf(''); + const crossEndIdx = out.indexOf('{/* SNIPEND */}'); + expect(crossStartIdx).toBeGreaterThan(-1); + expect(crossEndIdx).toBeGreaterThan(-1); + const crossBody = out.slice( + crossStartIdx + ''.length, + crossEndIdx + ); + + // Should not contain a code fence nor the snippet line "const nope = 2;" + expect(crossBody).not.toMatch(/```/); + expect(crossBody).not.toMatch(/const nope = 2;/); + + // (Optional) now clear and ensure only the properly paired region is cleared + const synctron2 = new Sync(cfg, logger); + await synctron2.clear(); + const cleared = fs.readFileSync(mdxPath, 'utf8'); + + // After clear(): the good HTML-paired region should be empty between its markers + const htmlRegionAfter = cleared.match(/\s*([\s\S]*?)\s*/); + expect(htmlRegionAfter).toBeTruthy(); + expect(htmlRegionAfter[1].trim()).toBe(''); // snippet content removed + + // The mispaired region should remain unchanged (still no fence or snippet) + const crossStartIdx2 = cleared.indexOf(''); + const crossEndIdx2 = cleared.indexOf('{/* SNIPEND */}'); + const crossBody2 = cleared.slice( + crossStartIdx2 + ''.length, + crossEndIdx2 + ); + expect(crossBody2).not.toMatch(/```/); + expect(crossBody2).not.toMatch(/const nope = 2;/); +}); From 6be5aac2114f0d154a2011f6425a80d409195c2e Mon Sep 17 00:00:00 2001 From: Lenny Chen Date: Thu, 18 Sep 2025 18:17:35 -0700 Subject: [PATCH 2/2] fix: splice warns and does not modify on mismatched ends --- src/Sync.js | 119 ++++++++++++++++++++++++++++++++++++---------- test/sync.test.js | 70 +++++++++++++++++++++++++++ 2 files changed, 164 insertions(+), 25 deletions(-) diff --git a/src/Sync.js b/src/Sync.js index b5a3b24..f2499c1 100644 --- a/src/Sync.js +++ b/src/Sync.js @@ -406,16 +406,14 @@ class Sync { let lookForStop = false; let spliceStart = 0; let config; - let currentStyleIdx; // NEW: remember which style opened + let currentStyleIdx; // track style of the opener we matched for (let [idx, _] of staticFile.lines.entries()) { const line = file.lines[idx]; - // OPEN: try all styles if (!lookForStop) { const parsed = parseWriteStartAny(line); if (parsed && parsed.id === snippet.id) { - // Optional source match check (unchanged) if (parsed.source) { const snippetPath = (snippet.filePath.directory.split('/').slice(1).join('/') + snippet.filePath.name); const repoPath = ("https://github.com/" + snippet.owner + "/" + snippet.repo + "/" + snippetPath); @@ -427,25 +425,45 @@ class Sync { config = overwriteConfig(this.config.features, parsed.config); spliceStart = fileLineNumber; lookForStop = true; - currentStyleIdx = parsed.styleIdx; // 🔒 remember which style to close with + currentStyleIdx = parsed.styleIdx; } } else { - // CLOSE: must match the same style - if (isWriteEndForStyle(line, currentStyleIdx)) { - dynamicFile = await this.spliceFile( - spliceStart, - fileLineNumber, - snippet, - dynamicFile, - config - ); - lookForStop = false; - currentStyleIdx = undefined; + // We are inside a candidate region — look for ANY end token on this line + const seenEndIdx = endStyleIdx(line); + if (seenEndIdx !== -1) { + if (seenEndIdx !== currentStyleIdx) { + // Mismatched end → warn & bail (do NOT splice, reset state) + this.logger.warn( + `snipsync: mismatched end marker at line ${fileLineNumber} in ${file.fullpath} (opened as style ${currentStyleIdx}, saw end style ${seenEndIdx}). Skipping splice for snippet "${snippet.id}".` + ); + lookForStop = false; + currentStyleIdx = undefined; + // fall through; keep scanning (a new, correct start may appear later) + } else { + // Proper close → do the splice + dynamicFile = await this.spliceFile( + spliceStart, + fileLineNumber, + snippet, + dynamicFile, + config + ); + lookForStop = false; + currentStyleIdx = undefined; + } } } fileLineNumber++; } + + if (lookForStop) { + // EOF with no matching end — bail noisily, no changes applied + this.logger.warn( + `snipsync: unterminated snippet region for "${snippet.id}" in ${file.fullpath} (opened style ${currentStyleIdx}, no matching end before EOF).` + ); + } + return dynamicFile; } @@ -467,31 +485,73 @@ class Sync { } // getClearedFile removes snippet lines from a specific file async getClearedFile(file) { - let omit = false; + let omitting = false; let openedStyleIdx; const out = []; + // Buffer all lines inside a candidate region so we can restore them on mismatch + let startLine = null; // we keep the START marker itself (per your current behavior) + let buffer = []; // lines between start and end (snippet content) + + const flushBufferKeepAll = (lineIfAny) => { + if (startLine !== null) out.push(startLine); + for (const l of buffer) out.push(l); + if (lineIfAny) out.push(lineIfAny); // e.g., mismatched end line treated as normal content + startLine = null; + buffer = []; + }; + for (const line of file.lines) { - if (!omit) { + if (!omitting) { const parsed = parseWriteStartAny(line); if (parsed) { - omit = true; + // Begin candidate region; keep the start marker + omitting = true; openedStyleIdx = parsed.styleIdx; - out.push(line); // KEEP the START marker + startLine = line; + buffer = []; continue; } out.push(line); } else { - if (isWriteEndForStyle(line, openedStyleIdx)) { - omit = false; - openedStyleIdx = undefined; - out.push(line); // KEEP the END marker - continue; + // We are inside a candidate region (candidate to clear) + const seenEndIdx = endStyleIdx(line); + if (seenEndIdx !== -1) { + if (seenEndIdx !== openedStyleIdx) { + // Mismatched end: warn and bail — restore everything seen so far and treat this line as normal content + this.logger.warn( + `snipsync: mismatched end marker while clearing ${file.fullpath} (opened as style ${openedStyleIdx}, saw end style ${seenEndIdx}). Region preserved.` + ); + flushBufferKeepAll(line); + omitting = false; + openedStyleIdx = undefined; + } else { + // Proper close: drop the buffered snippet content, keep the end marker + if (startLine !== null) out.push(startLine); // keep start + // (intentionally NOT pushing buffer) + out.push(line); // keep end + startLine = null; + buffer = []; + omitting = false; + openedStyleIdx = undefined; + } + } else { + // Not an end marker; just buffer the line for now + buffer.push(line); } - // omit snippet lines } } + // EOF while still omitting → unterminated region: restore everything + if (omitting) { + this.logger.warn( + `snipsync: unterminated snippet region while clearing ${file.fullpath} (opened style ${openedStyleIdx}, no matching end before EOF). Region preserved.` + ); + flushBufferKeepAll(null); + omitting = false; + openedStyleIdx = undefined; + } + file.lines = out; return file; } @@ -569,6 +629,15 @@ function isWriteEndForStyle(line, styleIdx) { return line.includes(token); } +function endStyleIdx(line) { + // returns the styleIdx of the end token if this line contains one; else -1 + for (let i = 0; i < writeMarkerStyles.length; i++) { + if (line.includes(writeMarkerStyles[i].end)) return i; + } + return -1; +} + + // extractReadID uses regex to exract the id from a string function extractReadID(line) { const matches = line.match(readMatchRegexp); diff --git a/test/sync.test.js b/test/sync.test.js index facc893..b4631ab 100644 --- a/test/sync.test.js +++ b/test/sync.test.js @@ -483,3 +483,73 @@ const nope = 2; expect(crossBody2).not.toMatch(/```/); expect(crossBody2).not.toMatch(/const nope = 2;/); }); + +test('Clear preserves content on mismatched end (warn & bail)', async () => { + const srcPath = `${testEnvPath}/src3.ts`; + fs.writeFileSync(srcPath, `// @@@SNIPSTART foo\nconst x = 1;\n// @@@SNIPEND\n`); + + const mdxPath = `${testEnvPath}/mismatch_clear.mdx`; + fs.writeFileSync(mdxPath, `\nsome text\n{/* SNIPEND */}\n`); + + cfg.origins = [{ files: { pattern: srcPath, owner: 'temporalio', repo: 'snipsync', ref: 'main' } }]; + cfg.features.allowed_target_extensions = ['.mdx']; + cfg.features.enable_code_block = true; + + const synctron = new Sync(cfg, logger); + await synctron.clear(); + + const out = fs.readFileSync(mdxPath, 'utf8'); + + // Region preserved (markers + inner content) + expect(out).toMatch(//); + expect(out).toMatch(/some text/); + expect(out).toMatch(/\{\/\*\s*SNIPEND\s*\*\/\}/); +}); + +test('Splice warns and does not modify on mismatched end', async () => { + // Arrange: source file with a snippet + const srcPath = `${testEnvPath}/src-mismatch.ts`; + fs.writeFileSync( + srcPath, + `// @@@SNIPSTART foo\nexport const val = 42;\n// @@@SNIPEND\n` + ); + + // Target doc with HTML start but JSX end (mismatched styles) + const targetPath = `${testEnvPath}/mismatch_splice.mdx`; + fs.writeFileSync( + targetPath, + `\nPLACEHOLDER\n{/* SNIPEND */}\n` + ); + + cfg.origins = [ + { + files: { + pattern: srcPath, + owner: 'temporalio', + repo: 'snipsync', + ref: 'main', + }, + }, + ]; + cfg.targets = [testEnvPath]; + cfg.features.enable_code_block = true; + + // Spy on logger.warn + const warnSpy = jest.spyOn(logger, 'warn').mockImplementation(() => {}); + + // Act + const synctron = new Sync(cfg, logger); + await synctron.run(); + + // Assert: the mismatched region remains unchanged (no code spliced in) + const out = fs.readFileSync(targetPath, 'utf8'); + expect(out).toContain(''); + expect(out).toContain('PLACEHOLDER'); + expect(out).toContain('{/* SNIPEND */}'); + + // And we should have logged a warning about the mismatch + expect(warnSpy).toHaveBeenCalled(); + + // Cleanup spy + warnSpy.mockRestore(); +});