From aec7af075700479e9125fb17a342bfadef10b493 Mon Sep 17 00:00:00 2001 From: Wayn_Liu Date: Wed, 9 Sep 2026 10:03:50 +0800 Subject: [PATCH 1/2] feat(diff): --latexdiff and --pdf for marked-up revisions Follow-up to #45, left open when the core diff command shipped in 0.10.0. A unified diff is the right artifact for a developer and the wrong one for a thesis advisor, who expects the revision marked up in the document. --latexdiff runs on the two sides diff has already fetched, so the markup means what the patch output means and costs no extra request. --pdf compiles it with Overleaf's compiler, so a reviewable PDF needs no local TeX installation. Root document detection, argument construction, output naming and failure interpretation are functions over data in src/latexdiff.ts, unit-tested with no Overleaf account and no latexdiff binary. Refs #55 --- CHANGELOG.md | 16 +++ README.md | 41 ++++++ docs/ARCHITECTURE.md | 9 +- src/cli.ts | 319 ++++++++++++++++++++++++++++++++++++++++- src/latexdiff.ts | 310 +++++++++++++++++++++++++++++++++++++++ test/e2e.sh | 70 +++++++++ test/latexdiff.test.ts | 212 +++++++++++++++++++++++++++ 7 files changed, 973 insertions(+), 4 deletions(-) create mode 100644 src/latexdiff.ts create mode 100644 test/latexdiff.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index b292541..980000e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,22 @@ All notable changes to this project will be documented in this file. +## [Unreleased] + +### Added +- **`olcli diff --latexdiff` marks the revision up inside the document** ([#55](https://github.com/aloth/olcli/issues/55)) - the follow-up left open when the core `diff` command shipped in 0.10.0. A unified diff is the right artifact for a developer and the wrong one for a thesis advisor, who expects deletions struck through and additions underlined. It runs on the two sides `diff` has already fetched, so the markup describes exactly what the patch output does - struck through is what a push would overwrite, underlined is what it would upload - and costs no extra request + - Requires `latexdiff` on PATH, which ships with TeX Live and MacTeX. A missing binary is reported as a setup problem with the install command for the platform, not as a failure of `diff`; nothing else in the command needs an external tool + - `\input` and `\include` are inlined before comparing, with `--no-flatten` to opt out. Without it a remote compile resolves those against the files in the project - the *old* content - and produces a PDF marking up the root document while showing every input file as unchanged. Wrong in a way that is very hard to notice + - Output goes to `.olcli-diff/`, dotted because `scanLocalFiles` skips dotted entries before any ignore rule is consulted. A plain `main-diff.tex` next to the document would be uploaded to Overleaf by the next `push` + - The root document is the `.tex` file declaring `\documentclass`. Several candidates are reported and listed rather than resolved by preferring `main.tex`: marking up the wrong document produces a plausible PDF describing the wrong revision + - `--latexdiff-opt` passes options straight through (`--latexdiff-opt --math-markup=0`), since the remote tree is on disk only for the duration of the run and cannot be handed to `latexdiff` by hand afterwards +- **`--pdf` compiles the marked-up document on Overleaf**, so a reviewable PDF needs no local TeX installation - the reason the flag was proposed in [#45](https://github.com/aloth/olcli/issues/45) + - Overleaf's compile endpoint takes a path that must already be in the project; there is no way to compile a document that is not. So the markup is uploaded as `olcli-latexdiff.tex` next to the root document, compiled, downloaded, and removed. That is a real mutation of the project for the duration of one compile, and the command says so before it does it + - It refuses rather than overwrites if a file of that name already exists, deletes from a `finally` with nothing in between able to terminate the process first, and prints the exact `olcli rm` command if the delete itself fails or the run is interrupted + - Placed next to the root document rather than at the project root, so relative `\includegraphics` and `\bibliography` paths resolve exactly as they do for the document it was built from + - A compile failure writes the CLSI log next to the marked-up source instead of reporting only a status, and deletes a PDF left by an earlier run rather than leaving one that describes a different revision. The compile runs against the project, so a `.sty` or `.cls` that exists only locally is the usual cause and the message says so. A missing *figure* is not: Overleaf draws a placeholder box naming the file and still reports success +- **`src/latexdiff.ts`** - root document detection, argument construction, output naming and failure interpretation are functions over data, unit-tested with no Overleaf account and no `latexdiff` binary. 21 tests, in the suite CI already runs + ## [0.12.0] - 2026-09-06 ### Added diff --git a/README.md b/README.md index 4cfe382..4296009 100644 --- a/README.md +++ b/README.md @@ -23,6 +23,7 @@ Work with Overleaf projects directly from your command line. Edit locally with y - ⬆️ **Push** local changes back to Overleaf - 🔄 **Sync** bidirectionally with smart conflict detection - 🔍 **Diff** local files against the live remote before pushing +- 📝 **Marked-up revisions** — `diff --latexdiff` produces the struck-through/underlined PDF advisors and journals ask for - 🔀 **Git remote** — use Overleaf as a native git remote ([docs](docs/GIT-REMOTE.md)) - ✌️ **Two-way deletions** — files removed locally are deleted on Overleaf on next sync - 🗑️ **Delete** and ✏️ **rename** remote files by path @@ -240,6 +241,46 @@ than the last pull, because that is what `push` uploads; `diff` lists files whose **contents** actually differ. A file you touched without editing appears in the first and not the second. +#### Marked-up revisions with latexdiff + +A unified diff is the right artifact for a developer and the wrong one for a +thesis advisor. `--latexdiff` marks the same revision up inside the document +instead — deletions struck through, additions underlined — which is what +advisors and journals ask for. + +```bash +olcli diff --latexdiff # write .olcli-diff/main-diff.tex +olcli diff --latexdiff --pdf # ...and compile it, download .olcli-diff/main-diff.pdf +``` + +Requires `latexdiff` on your PATH. It ships with TeX Live and MacTeX; nothing +else in `olcli diff` needs an external tool. + +**`--pdf` compiles on Overleaf, so you do not need a local TeX installation.** +The compile endpoint can only build a file that is in the project, so the +marked-up document is uploaded as `olcli-latexdiff.tex` next to your root +document, compiled, downloaded, and then removed. The command says so before it +does it, refuses to overwrite a file of that name if one already exists, and +prints the exact `olcli rm` command if the cleanup itself fails. + +`\input` and `\include` are inlined before comparing (`--no-flatten` to opt +out). Without that, a remote compile would resolve those against the files +sitting in the project — the old content — and quietly produce a PDF showing +every input file as unchanged. + +Output goes to `.olcli-diff/`, which is dotted so that `push` and `sync` never +pick it up; `-o ` puts it elsewhere. The root document is the `.tex` file +declaring `\documentclass`; if several do, `--main ` picks one rather +than the command guessing. `--latexdiff-opt` passes anything else straight +through, e.g. `--latexdiff-opt --math-markup=0`. + +One limit worth knowing: `--pdf` compiles against the project, not against your +working directory, so anything the markup needs must already be on Overleaf. A +`.sty` or `.cls` you added locally fails the compile — the compiler log is +written next to the marked-up source when that happens. A *figure* you added +locally does not fail it; Overleaf draws a placeholder box naming the missing +file and the rest of the PDF is fine. + #### How deletion propagation works `olcli` records a manifest of remote files in `.olcli.json`. On next sync: diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index e3ee121..32cee83 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -115,11 +115,13 @@ Which files need an Overleaf account to exercise, and which do not. This is the main thing to know before adding a feature, because it decides where the logic should go. -**Pure — data in, data out. No network, no filesystem, unit-tested:** +**Needs no Overleaf account, so unit-tested directly. Data in, data out; +`scan.ts` and `latexdiff.ts` also touch the local filesystem:** | Module | Responsibility | |---|---| | `diff.ts` | Compare two file trees; render unified diffs | +| `latexdiff.ts` | Root document detection; build and run the `latexdiff` command | | `ignore.ts` | The three ignore layers and the `.pdf`-next-to-`.tex` rule | | `paths.ts` | Remote path normalization; zip-slip containment | | `rename-plan.ts` | Plan bulk project renames before applying any | @@ -147,6 +149,11 @@ exists at all: `push` and `sync` each carried their own copy of the same walk loop and had already drifted apart, and `diff` would have made a third. The same reasoning produced `rename-plan.ts` and `diff.ts`. +`latexdiff.ts` is the one module that shells out to something olcli does not +ship. That is confined to a single `execFile` call with an argv array — never a +shell — and a missing binary is reported as a setup problem with a fix rather +than as a failure of the command. + `client.ts` request *construction* can also be tested without an account, by pointing the client at a local HTTP server that captures the outgoing request — see `test/client.test.ts`. diff --git a/src/cli.ts b/src/cli.ts index 70a59e1..f03d3c3 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -9,14 +9,28 @@ import { Command } from 'commander'; import chalk from 'chalk'; import ora from 'ora'; -import { writeFileSync, readFileSync, existsSync, mkdirSync } from 'node:fs'; +import { writeFileSync, readFileSync, existsSync, mkdirSync, mkdtempSync, rmSync } from 'node:fs'; import { join, dirname, basename } from 'node:path'; +import { tmpdir } from 'node:os'; import { fileURLToPath } from 'node:url'; import { OverleafClient } from './client.js'; import { resolveRemotePath, resolveWithin, normalizeRemotePath } from './paths.js'; import { planProjectRenames } from './rename-plan.js'; import { scanLocalFiles } from './scan.js'; -import { compareTrees, filterRemoteTree, renderFileDiff, statusLetter } from './diff.js'; +import { compareTrees, filterRemoteTree, renderFileDiff, statusLetter, type FileDiff } from './diff.js'; +import { + DIFF_OUTPUT_DIR, + LatexdiffError, + RootDocumentError, + buildLatexdiffArgs, + diffOutputPath, + isTexPath, + latexdiffInstallHint, + materializeTree, + remoteScratchPath, + resolveRootDocument, + runLatexdiff, +} from './latexdiff.js'; import { loadIgnore } from './ignore.js'; // Read version from package.json @@ -1786,6 +1800,13 @@ program .option('--name-only', 'List changed paths instead of printing patches') .option('--file ', 'Diff a single file') .option('-U, --unified ', 'Lines of context around each hunk (default: 3)', parseInt) + .option('--latexdiff', 'Mark the revision up inside the document with latexdiff (requires latexdiff on PATH)') + .option('--pdf', 'Compile the marked-up document on Overleaf and download the PDF (implies --latexdiff)') + .option('--main ', 'Root .tex document to mark up (default: the only file declaring \\documentclass)') + .option('-o, --output ', `Where to write the marked-up .tex (default: ${DIFF_OUTPUT_DIR}/-diff.tex)`) + .option('--no-flatten', 'Leave \\input/\\include in place instead of inlining them') + .option('--latexdiff-opt ', 'Pass an option straight through to latexdiff (repeatable)', + (value: string, previous: string[] = []) => [...previous, value]) .option('--no-default-ignore', 'Disable built-in LaTeX artifact ignore list (only .olignore applies)') .option('--no-ignore', 'Disable all ignore filtering') .option('--cookie ', 'Session cookie override') @@ -1794,7 +1815,13 @@ The remote side is fetched fresh on every run, so the diff describes the project as it is right now - which is what a subsequent push would overwrite. It is not a comparison against the last pull. A collaborator editing between diff and push can still change the outcome; the fetch time is printed for that -reason.`) +reason. + +--latexdiff hands those same two sides to latexdiff and marks the revision up +inside the document instead: struck through is what a push would overwrite, +underlined is what it would upload. --pdf additionally uploads the marked-up +document to the project for one compile, downloads the PDF, and removes it +again - so a reviewable PDF needs no local TeX installation.`) .action(async (project, dir, options) => { const targetDir = dir || '.'; @@ -1803,6 +1830,35 @@ reason.`) process.exit(1); } + // Checked before connecting: an unusable combination of flags should not + // cost a login and a full project download first. + const latexdiffMode = Boolean(options.latexdiff || options.pdf); + const conflicting = [ + options.nameOnly ? '--name-only' : null, + options.file ? '--file' : null, + options.unified !== undefined ? '-U/--unified' : null, + ].filter(Boolean); + + if (latexdiffMode && conflicting.length > 0) { + console.error(chalk.red(`--latexdiff cannot be combined with ${conflicting.join(', ')}`)); + console.error('It marks up one root document; those options select and shape unified patch output.'); + process.exit(1); + } + + if (!latexdiffMode) { + // Accepting these silently would produce a normal patch and no markup, + // with nothing in the output saying the flag was dropped. + const latexdiffOnly = [ + options.main ? '--main' : null, + options.output ? '--output' : null, + options.latexdiffOpt?.length ? '--latexdiff-opt' : null, + ].filter(Boolean); + if (latexdiffOnly.length > 0) { + console.error(chalk.red(`${latexdiffOnly.join(', ')} only applies with --latexdiff`)); + process.exit(1); + } + } + const spinner = ora('Connecting...').start(); try { const client = await getClient(options.cookie); @@ -1850,6 +1906,23 @@ reason.`) let entries = compareTrees(localFiles, remoteFiles).filter((e) => e.status !== 'unchanged'); + if (latexdiffMode) { + await runLatexdiffMode({ + client, + projectId, + projectName, + targetDir, + localFiles, + remoteFiles, + entries, + fetchedAt, + options, + spinner, + }); + setLastProject(projectId); + return; + } + if (options.file) { const wanted = normalizeRemotePath(options.file); entries = entries.filter((e) => e.path === wanted); @@ -1917,6 +1990,246 @@ reason.`) } }); +/** + * `--latexdiff` / `--pdf`: mark the revision up inside the document. + * + * Runs on the two sides `diff` has already fetched, so the semantics are the + * ones documented for the command - old is the remote as of this run, new is + * the working directory - and no extra request is made to produce the markup. + * + * The remote side has to reach the filesystem before `latexdiff` can read it, + * and the whole tree is written rather than the root document alone, because + * `--flatten` resolves each `\input` relative to its own side. + */ +async function runLatexdiffMode(params: { + client: OverleafClient; + projectId: string; + projectName: string; + targetDir: string; + localFiles: Map; + remoteFiles: Map; + entries: FileDiff[]; + fetchedAt: Date; + /** Only the flags this mode reads; the rest of `diff`'s options are rejected. */ + options: { + main?: string; + output?: string; + pdf?: boolean; + flatten?: boolean; + latexdiffOpt?: string[]; + }; + spinner: ReturnType; +}): Promise { + const { + client, projectId, projectName, targetDir, + localFiles, remoteFiles, entries, fetchedAt, options, spinner, + } = params; + + let root = ''; + try { + root = resolveRootDocument(localFiles, options.main ? normalizeRemotePath(options.main) : undefined); + } catch (error) { + if (!(error instanceof RootDocumentError)) throw error; + spinner.stop(); + console.error(chalk.red(error.message)); + for (const candidate of error.candidates) { + console.error(chalk.dim(` ${candidate}`)); + } + process.exit(1); + } + + // latexdiff needs two versions of the same document. A root document that + // only exists locally has one, and diffing it against an empty file would + // mark up the entire paper as an addition. + if (!remoteFiles.has(root)) { + spinner.stop(); + console.error(chalk.red(`${root} is not in "${projectName}" yet, so there is no earlier version to mark up.`)); + console.error(chalk.dim(' Push it first, or use --main to name a document that exists on both sides.')); + process.exit(1); + } + + if (!entries.some((e) => isTexPath(e.path))) { + spinner.info(`No .tex file differs from "${projectName}" - the markup will show no changes`); + } + + const tmpRoot = mkdtempSync(join(tmpdir(), 'olcli-latexdiff-')); + const cleanupTmp = () => rmSync(tmpRoot, { recursive: true, force: true }); + + try { + spinner.start('Writing the remote side to a temporary directory...'); + materializeTree(tmpRoot, remoteFiles); + + spinner.text = `Running latexdiff on ${root}...`; + let markup = ''; + let warnings = ''; + try { + const result = await runLatexdiff(buildLatexdiffArgs( + join(tmpRoot, root), + join(targetDir, root), + { flatten: options.flatten !== false, extra: options.latexdiffOpt }, + )); + markup = result.markup; + warnings = result.stderr.trim(); + } catch (error) { + if (!(error instanceof LatexdiffError)) throw error; + spinner.fail(error.message); + if (error.missing) { + console.error(chalk.dim(` ${latexdiffInstallHint()}`)); + console.error(chalk.dim(' Everything else in olcli diff needs no external tools.')); + } else if (error.stderr) { + console.error(error.stderr); + } + cleanupTmp(); + process.exit(1); + } + + // An explicit --output is a path the user chose, so it is taken relative to + // the current directory; the default belongs to the project directory being + // diffed. Either way the PDF sits next to the source it was built from. + const texPath = options.output || join(targetDir, diffOutputPath(root, 'tex')); + const pdfPath = texPath.replace(/\.(tex|ltx)$/i, '') + '.pdf'; + + mkdirSync(dirname(texPath), { recursive: true }); + writeFileSync(texPath, markup, 'utf-8'); + spinner.succeed(`Marked up ${root} (${(Buffer.byteLength(markup) / 1024).toFixed(1)} KB)`); + + if (warnings) { + for (const line of warnings.split('\n')) { + console.log(chalk.yellow(` latexdiff: ${line}`)); + } + } + + if (options.pdf) { + await compileMarkupOnOverleaf({ client, projectId, projectName, root, markup, texPath, pdfPath, spinner }); + } + + console.log(); + console.log(chalk.bold(`Revision of ${root} against "${projectName}"`)); + console.log(` ${texPath}`); + if (options.pdf) console.log(` ${pdfPath}`); + console.log(chalk.dim( + ` struck through = remote as of ${fetchedAt.toISOString()}, underlined = local`, + )); + if (options.flatten !== false) { + console.log(chalk.dim(' \\input/\\include were inlined; pass --no-flatten to keep them')); + } + } finally { + cleanupTmp(); + } +} + +/** + * Compile a marked-up document with Overleaf's compiler and download the PDF. + * + * The compile endpoint takes a `rootResourcePath` that has to already exist in + * the project - there is no way to compile a document that is not in it. So + * the markup is uploaded, compiled, and removed again, which is a real + * mutation of someone's project for the duration of one compile and is + * announced before it happens. + * + * Three things follow from that and are not incidental: + * + * - The upload refuses to overwrite. If the scratch path is taken, that file + * belongs to the user, and clobbering it to produce a diff would be a worse + * outcome than not producing one. + * - The delete runs from a `finally`, and nothing between the upload and it + * calls `process.exit` - that would terminate before the cleanup and leave + * the file on the project. Failures are collected and reported afterwards. + * - An interrupt cannot be cleaned up after reliably, so it prints the exact + * command that removes the file rather than leaving it to be discovered. + */ +async function compileMarkupOnOverleaf(params: { + client: OverleafClient; + projectId: string; + projectName: string; + root: string; + markup: string; + texPath: string; + pdfPath: string; + spinner: ReturnType; +}): Promise { + const { client, projectId, projectName, root, markup, texPath, pdfPath, spinner } = params; + const scratch = remoteScratchPath(root); + + spinner.start(`Checking ${scratch} is free...`); + if (await client.findEntityByPath(projectId, scratch)) { + spinner.fail(`"${projectName}" already has a file named ${scratch}`); + console.error(chalk.dim(' --pdf uploads the marked-up document under that name for one compile and')); + console.error(chalk.dim(' removes it again; it will not overwrite a file that is already there.')); + console.error(chalk.dim(` The marked-up source was still written: ${texPath}`)); + process.exit(1); + } + + spinner.stop(); + console.log(chalk.dim(` uploading ${scratch} to "${projectName}" for one compile, then removing it`)); + + const onInterrupt = () => { + console.error(chalk.yellow(`\nInterrupted with ${scratch} still in "${projectName}".`)); + console.error(chalk.yellow(`Remove it with: olcli rm ${scratch}`)); + process.exit(130); + }; + + spinner.start('Uploading the marked-up document...'); + await client.uploadFile(projectId, null, scratch, Buffer.from(markup, 'utf-8')); + process.once('SIGINT', onInterrupt); + + let failure: string[] | null = null; + + try { + spinner.text = 'Compiling on Overleaf...'; + const compile = await client.compileWithOutputs(projectId, scratch); + + if (compile.status !== 'success' || !compile.pdfUrl) { + failure = [`Overleaf reported "${compile.status}" compiling ${scratch}`]; + + // The log is the only thing that explains a LaTeX failure, and it stops + // being reachable as soon as the scratch file is removed below. + const logFile = compile.outputFiles.find((f) => f.path === 'output.log'); + if (logFile) { + const logPath = pdfPath.replace(/\.pdf$/, '.log'); + writeFileSync(logPath, await client.downloadOutputFile(logFile.url)); + failure.push(` compiler log: ${logPath}`); + } + failure.push(` marked-up source: ${texPath}`); + // A .sty or .cls that only exists locally is the common one: Overleaf + // compiles against the project, so anything the markup needs has to be + // in the project. A missing *figure* does not fail - Overleaf draws a + // placeholder box naming the file and reports success. + failure.push(' A class, style or input file that exists only locally is the usual cause;'); + failure.push(' Overleaf compiles against the project, not against your working directory.'); + + // A PDF from an earlier run would otherwise sit next to the log, dated + // now by the directory listing and describing a different revision. + rmSync(pdfPath, { force: true }); + } else { + spinner.text = 'Downloading the PDF...'; + const pdf = await client.downloadOutputFile(compile.pdfUrl); + writeFileSync(pdfPath, pdf); + spinner.succeed(`Compiled ${basename(pdfPath)} (${(pdf.length / 1024).toFixed(1)} KB)`); + } + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + failure = [`Compiling ${scratch} failed: ${message}`, ` marked-up source: ${texPath}`]; + } finally { + process.off('SIGINT', onInterrupt); + spinner.start(`Removing ${scratch} from "${projectName}"...`); + try { + await client.deleteByPath(projectId, scratch); + spinner.stop(); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + spinner.warn(`${scratch} is still in "${projectName}": ${message}`); + console.error(chalk.yellow(` remove it with: olcli rm ${scratch}`)); + } + } + + if (failure) { + spinner.fail(failure[0]); + for (const line of failure.slice(1)) console.error(chalk.dim(line)); + process.exit(1); + } +} + /** * Colourize one line of a rendered patch. chalk already no-ops when stdout is * not a TTY, so this needs no flag of its own. diff --git a/src/latexdiff.ts b/src/latexdiff.ts new file mode 100644 index 0000000..52ec26c --- /dev/null +++ b/src/latexdiff.ts @@ -0,0 +1,310 @@ +/** + * `latexdiff` integration for `olcli diff`. + * + * `diff` already computes both sides of the comparison - the project as it is + * on Overleaf right now, and the working directory - and prints them as + * unified patches. That is the right artifact for a developer and the wrong + * one for a thesis advisor, who expects the revision marked up in the document + * itself. `latexdiff` produces that, and the only parts a user cannot do by + * hand are getting the remote side onto disk and compiling the result without + * a local TeX installation. + * + * Everything decidable from data is decided here as a pure function so it can + * be unit-tested with no Overleaf account and no `latexdiff` binary; the spawn + * and the upload/compile/delete sequence stay in `cli.ts` with the rest of the + * IO. Same split as `diff.ts`. + * + * Orientation matches `diff.ts` and is not negotiable: **old is the remote, + * new is local**. Struck-through text in the markup is content a subsequent + * push would overwrite, underlined text is content it would upload. + */ + +import { execFile } from 'node:child_process'; +import { promisify } from 'node:util'; +import { mkdirSync, writeFileSync } from 'node:fs'; +import { dirname } from 'node:path'; +import { resolveWithin } from './paths.js'; + +const execFileAsync = promisify(execFile); + +/** Binary looked up on PATH. Not configurable; `latexdiff` is its only name. */ +export const LATEXDIFF_BIN = 'latexdiff'; + +/** + * Directory the marked-up files are written to, relative to the target + * directory. + * + * Dotted on purpose. `scanLocalFiles` skips dotted entries unconditionally, + * before any ignore rule is consulted, so this is the one location whose + * contents cannot leak into a later `push`. A plain `main-diff.tex` next to + * the document would be uploaded to Overleaf on the next sync. + */ +export const DIFF_OUTPUT_DIR = '.olcli-diff'; + +/** + * Name of the file `--pdf` uploads to the project for the duration of one + * compile. + * + * Fixed rather than randomized: a name a user can recognize and delete by hand + * is worth more than one that never collides, and a collision is refused + * rather than resolved anyway. + */ +export const REMOTE_SCRATCH_NAME = 'olcli-latexdiff.tex'; + +/** Extensions treated as LaTeX source when looking for the root document. */ +const TEX_EXTENSIONS = ['.tex', '.ltx']; + +/** + * Drop the comment part of a single line of TeX. + * + * A `%` starts a comment unless it is escaped, and `\\%` is an escaped + * backslash followed by a comment, so the run of backslashes before the `%` + * has to be counted rather than just the character in front of it. + */ +function stripTexComment(line: string): string { + for (let i = 0; i < line.length; i++) { + if (line[i] !== '%') continue; + let backslashes = 0; + for (let j = i - 1; j >= 0 && line[j] === '\\'; j--) backslashes++; + if (backslashes % 2 === 0) return line.slice(0, i); + } + return line; +} + +/** + * True when the file declares a document class outside of a comment. + * + * This is what separates a root document from the chapters it inputs. A + * commented-out `\documentclass` is common in subfiles - people leave the + * standalone preamble behind when they split a document up - and counting it + * would report every chapter as a candidate root. + */ +export function declaresDocumentClass(content: Buffer): boolean { + const text = content.toString('utf-8'); + return text + .split('\n') + .some((line) => /\\documentclass\s*[[{]/.test(stripTexComment(line))); +} + +export function isTexPath(path: string): boolean { + const lower = path.toLowerCase(); + return TEX_EXTENSIONS.some((ext) => lower.endsWith(ext)); +} + +/** + * Every `.tex` file in the tree that declares a document class, sorted by path + * so the list a user is shown is stable across runs. + */ +export function findRootCandidates(files: Map): string[] { + return [...files.keys()] + .filter((path) => isTexPath(path)) + .filter((path) => declaresDocumentClass(files.get(path)!)) + .sort(); +} + +/** Raised when the root document cannot be determined; carries the advice. */ +export class RootDocumentError extends Error { + constructor(message: string, readonly candidates: string[] = []) { + super(message); + this.name = 'RootDocumentError'; + } +} + +/** + * Decide which document to mark up. + * + * An explicit `--main` always wins, including for layouts this heuristic + * cannot read. Otherwise the document class is the signal, and ambiguity is + * reported rather than guessed at: a project holding both `paper.tex` and + * `poster.tex` has no correct default, and marking up the wrong one produces a + * plausible PDF describing the wrong revision - the failure that is hardest to + * notice. + * + * @param files The local tree, already filtered the same way `diff` filters it. + */ +export function resolveRootDocument(files: Map, explicit?: string): string { + if (explicit) { + if (!files.has(explicit)) { + throw new RootDocumentError( + `Root document not found locally: ${explicit}\n` + + ' It must be a file in the target directory that diff also reports ' + + '(an ignore rule can hide it).', + ); + } + return explicit; + } + + const candidates = findRootCandidates(files); + + if (candidates.length === 1) return candidates[0]; + + if (candidates.length === 0) { + throw new RootDocumentError( + 'No .tex file declaring \\documentclass was found, so there is no root document to mark up.\n' + + ' Pass --main to name one.', + ); + } + + throw new RootDocumentError( + `${candidates.length} files declare \\documentclass, so the root document is ambiguous.\n` + + ' Pass --main to choose one.', + candidates, + ); +} + +export interface LatexdiffArgsOptions { + /** + * Inline `\input` and `\include` before comparing. On by default; see + * `buildLatexdiffArgs`. + */ + flatten?: boolean; + /** Verbatim `latexdiff` options from `--latexdiff-opt`, passed through. */ + extra?: string[]; +} + +/** + * Build the `latexdiff` argument list. + * + * `--flatten` is the default, and the reason is the remote compile rather than + * taste. Without it the marked-up file still contains `\input{sections/intro}`, + * and compiling it on Overleaf resolves those against the files sitting in the + * project - the old content - so a multi-file thesis would produce a PDF that + * marks up the root document and silently shows every input file as unchanged. + * `--no-flatten` is there for anyone who wants the raw single-file markup. + * + * Extra options come before the file arguments because `latexdiff` reads the + * last two positional arguments as old and new. + */ +export function buildLatexdiffArgs( + oldPath: string, + newPath: string, + options: LatexdiffArgsOptions = {}, +): string[] { + const args: string[] = []; + if (options.flatten !== false) args.push('--flatten'); + args.push(...(options.extra ?? [])); + args.push(oldPath, newPath); + return args; +} + +/** + * Where a marked-up artifact is written, relative to the target directory. + * + * Flat rather than mirroring the root document's folder: the output of a + * flattened run is a single self-contained file, and burying it under + * `.olcli-diff/chapters/` would only make it harder to find. + */ +export function diffOutputPath(rootPath: string, extension: string): string { + const base = rootPath.split('/').pop()!.replace(/\.(tex|ltx)$/i, ''); + return `${DIFF_OUTPUT_DIR}/${base}-diff.${extension}`; +} + +/** + * Project path `--pdf` uploads the marked-up file to. + * + * Placed next to the root document rather than at the project root, so that + * relative `\includegraphics` and `\bibliography` paths in the flattened file + * resolve exactly as they do for the document it was built from. + */ +export function remoteScratchPath(rootPath: string): string { + const folder = rootPath.includes('/') ? rootPath.slice(0, rootPath.lastIndexOf('/')) : ''; + return folder ? `${folder}/${REMOTE_SCRATCH_NAME}` : REMOTE_SCRATCH_NAME; +} + +/** + * Write a file tree to a directory, for use as `latexdiff`'s old side. + * + * `latexdiff --flatten` resolves `\input` relative to each side's own file, so + * the remote tree has to exist on disk with its folder structure intact - + * handing it just the root document would break every input in a multi-file + * project. + * + * Entry names are re-checked against the destination even though `diff` has + * already filtered them against the target directory: this writes files, and a + * path that was safe relative to one base is not automatically safe relative + * to another. See #44. + */ +export function materializeTree(destination: string, files: Map): number { + let written = 0; + for (const [path, data] of files) { + const target = resolveWithin(destination, path); + if (!target) continue; + mkdirSync(dirname(target), { recursive: true }); + writeFileSync(target, data); + written++; + } + return written; +} + +export interface LatexdiffResult { + /** The marked-up document, as printed to stdout. */ + markup: string; + /** Warnings `latexdiff` printed while succeeding. */ + stderr: string; +} + +/** + * Raised when `latexdiff` could not be run or exited non-zero. `missing` + * separates "not installed" - a setup problem with a known fix - from a real + * failure on the documents, which needs the tool's own message. + */ +export class LatexdiffError extends Error { + constructor(message: string, readonly missing: boolean, readonly stderr = '') { + super(message); + this.name = 'LatexdiffError'; + } +} + +/** + * Run `latexdiff`, returning its stdout. + * + * `execFile`, never a shell: the arguments include user-supplied paths and + * pass-through options, and an argv array cannot be talked into running + * something else. + * + * The default `maxBuffer` is 1 MB, which a flattened thesis passes without + * trying. Exceeding it truncates stdout and reports it as an error, which + * would read as "latexdiff failed" on exactly the documents this feature + * exists for. + */ +export async function runLatexdiff(args: string[]): Promise { + try { + const { stdout, stderr } = await execFileAsync(LATEXDIFF_BIN, args, { + encoding: 'utf-8', + maxBuffer: 256 * 1024 * 1024, + }); + return { markup: stdout, stderr }; + } catch (error) { + // execFile rejects with the spawn error for a missing binary and with the + // captured streams for a non-zero exit; `code` carries the errno in the + // first case and the exit status in the second. + const failure = error as NodeJS.ErrnoException & { stderr?: string }; + if (failure.code === 'ENOENT') { + throw new LatexdiffError(`${LATEXDIFF_BIN} was not found on PATH`, true); + } + const stderr = typeof failure.stderr === 'string' ? failure.stderr.trim() : ''; + throw new LatexdiffError( + `${LATEXDIFF_BIN} exited with status ${failure.code ?? 'unknown'}`, + false, + stderr, + ); + } +} + +/** + * Install advice for a missing binary, by platform. + * + * `latexdiff` ships with TeX Live and MacTeX, so on most machines the answer + * is that the whole distribution is missing rather than this one tool - which + * is also the case where `--pdf` is most useful, since it needs no local TeX. + */ +export function latexdiffInstallHint(platform: string = process.platform): string { + switch (platform) { + case 'darwin': + return 'Install MacTeX (brew install --cask mactex-no-gui) or brew install latexdiff'; + case 'win32': + return 'Install MiKTeX or TeX Live; both ship latexdiff'; + default: + return 'Install it from your TeX distribution (apt install latexdiff, or TeX Live)'; + } +} diff --git a/test/e2e.sh b/test/e2e.sh index 09cfd86..ff1a5dc 100755 --- a/test/e2e.sh +++ b/test/e2e.sh @@ -607,6 +607,76 @@ run_test "diff is clean again after restoring the tree" \ sleep 1 # Rate limit +####################################### +# Test: latexdiff +####################################### + +log_section "latexdiff Tests" + +if ! command -v latexdiff >/dev/null 2>&1; then + log_warn "latexdiff not on PATH - skipping the --latexdiff section" +else + # A root document of our own, so the section does not depend on what the + # target project happens to contain, and --main keeps it unambiguous even in + # a project that already has one. + LD_NAME="${TEST_ID}_ld.tex" + LD_FILE="$PULL_DIR/$LD_NAME" + cat > "$LD_FILE" < Buffer.from(s, 'utf-8'); +const ROOT = '\\documentclass{article}\n\\begin{document}\nhi\n\\end{document}\n'; + +// ───────────────────────────────────────────────────────────────────────────── +// Root document detection +// ───────────────────────────────────────────────────────────────────────────── + +test('declaresDocumentClass: finds a document class with and without options', () => { + assert.equal(declaresDocumentClass(buf('\\documentclass{article}\n')), true); + assert.equal(declaresDocumentClass(buf('\\documentclass[12pt,a4paper]{report}\n')), true); + assert.equal(declaresDocumentClass(buf('\\documentclass {book}\n')), true); +}); + +test('declaresDocumentClass: a commented-out declaration does not count', () => { + // Chapters split out of a standalone document routinely keep their old + // preamble commented out; counting it would make every chapter a candidate. + assert.equal(declaresDocumentClass(buf('% \\documentclass{article}\n\\section{One}\n')), false); + assert.equal(declaresDocumentClass(buf('\\section{One} % \\documentclass{article}\n')), false); +}); + +test('declaresDocumentClass: an escaped percent does not start a comment', () => { + assert.equal(declaresDocumentClass(buf('50\\% off \\documentclass{article}\n')), true); + // An escaped backslash before the percent means the percent is a comment again. + assert.equal(declaresDocumentClass(buf('a\\\\% \\documentclass{article}\n')), false); +}); + +test('declaresDocumentClass: prose mentioning the command is not a declaration', () => { + assert.equal(declaresDocumentClass(buf('The documentclass matters.\n')), false); + assert.equal(declaresDocumentClass(buf('\\documentclasses are described below\n')), false); +}); + +test('isTexPath: .tex and .ltx, case-insensitive', () => { + assert.equal(isTexPath('main.tex'), true); + assert.equal(isTexPath('sections/Intro.TeX'), true); + assert.equal(isTexPath('paper.ltx'), true); + assert.equal(isTexPath('refs.bib'), false); + assert.equal(isTexPath('figures/plot.pdf'), false); +}); + +test('findRootCandidates: only .tex files that declare a class, sorted', () => { + const files = new Map([ + ['z.tex', buf(ROOT)], + ['a.tex', buf(ROOT)], + ['sections/intro.tex', buf('\\section{Intro}\n')], + ['refs.bib', buf('@book{x}\n')], + ['notes.txt', buf('\\documentclass{article}\n')], + ]); + assert.deepEqual(findRootCandidates(files), ['a.tex', 'z.tex']); +}); + +test('resolveRootDocument: a single candidate is used without asking', () => { + const files = new Map([ + ['main.tex', buf(ROOT)], + ['sections/intro.tex', buf('\\section{Intro}\n')], + ]); + assert.equal(resolveRootDocument(files), 'main.tex'); +}); + +test('resolveRootDocument: ambiguity is reported, never guessed', () => { + // Silently picking one produces a plausible PDF describing the wrong + // revision, which is the failure hardest for a reviewer to notice. + const files = new Map([['paper.tex', buf(ROOT)], ['poster.tex', buf(ROOT)]]); + assert.throws( + () => resolveRootDocument(files), + (error: unknown) => { + assert.ok(error instanceof RootDocumentError); + assert.deepEqual(error.candidates, ['paper.tex', 'poster.tex']); + assert.match(error.message, /--main/); + return true; + }, + ); +}); + +test('resolveRootDocument: no candidate at all is an error naming the flag', () => { + const files = new Map([['sections/intro.tex', buf('\\section{Intro}\n')]]); + assert.throws(() => resolveRootDocument(files), /--main/); +}); + +test('resolveRootDocument: --main wins, including over the heuristic', () => { + const files = new Map([['main.tex', buf(ROOT)], ['sections/intro.tex', buf('\\section{Intro}\n')]]); + assert.equal(resolveRootDocument(files, 'sections/intro.tex'), 'sections/intro.tex'); +}); + +test('resolveRootDocument: --main for a file that is not in the tree explains why', () => { + const files = new Map([['main.tex', buf(ROOT)]]); + assert.throws(() => resolveRootDocument(files, 'missing.tex'), /ignore rule/); +}); + +// ───────────────────────────────────────────────────────────────────────────── +// Argument construction +// ───────────────────────────────────────────────────────────────────────────── + +test('buildLatexdiffArgs: flattens by default, old side first', () => { + // Orientation is the whole meaning of the output: old is the remote. + assert.deepEqual( + buildLatexdiffArgs('/tmp/remote/main.tex', 'local/main.tex'), + ['--flatten', '/tmp/remote/main.tex', 'local/main.tex'], + ); +}); + +test('buildLatexdiffArgs: --no-flatten drops the flag and nothing else', () => { + assert.deepEqual( + buildLatexdiffArgs('old.tex', 'new.tex', { flatten: false }), + ['old.tex', 'new.tex'], + ); +}); + +test('buildLatexdiffArgs: pass-through options come before the file arguments', () => { + // latexdiff reads the last two positional arguments as old and new, so an + // option appended after them would be taken for a filename. + assert.deepEqual( + buildLatexdiffArgs('old.tex', 'new.tex', { extra: ['--math-markup=0', '--type=CFONT'] }), + ['--flatten', '--math-markup=0', '--type=CFONT', 'old.tex', 'new.tex'], + ); +}); + +// ───────────────────────────────────────────────────────────────────────────── +// Output locations +// ───────────────────────────────────────────────────────────────────────────── + +test('diffOutputPath: named after the root document, inside the dotted directory', () => { + assert.equal(diffOutputPath('main.tex', 'tex'), `${DIFF_OUTPUT_DIR}/main-diff.tex`); + assert.equal(diffOutputPath('main.tex', 'pdf'), `${DIFF_OUTPUT_DIR}/main-diff.pdf`); +}); + +test('diffOutputPath: a nested root document still writes to one flat directory', () => { + // The flattened markup is a single self-contained file; mirroring the source + // folder would only make it harder to find. + assert.equal(diffOutputPath('thesis/paper.ltx', 'tex'), `${DIFF_OUTPUT_DIR}/paper-diff.tex`); +}); + +test('diffOutputPath: the output directory is dotted so a push cannot pick it up', () => { + // scanLocalFiles skips dotted entries before any ignore rule is consulted. + assert.ok(DIFF_OUTPUT_DIR.startsWith('.')); +}); + +test('remoteScratchPath: sits next to the root document', () => { + // Relative \includegraphics and \bibliography paths in the flattened file + // resolve from the root document's folder, so the upload has to share it. + assert.equal(remoteScratchPath('main.tex'), REMOTE_SCRATCH_NAME); + assert.equal(remoteScratchPath('thesis/paper.tex'), `thesis/${REMOTE_SCRATCH_NAME}`); + assert.equal(remoteScratchPath('a/b/c.tex'), `a/b/${REMOTE_SCRATCH_NAME}`); +}); + +test('latexdiffInstallHint: names something installable on each platform', () => { + assert.match(latexdiffInstallHint('darwin'), /brew/); + assert.match(latexdiffInstallHint('win32'), /MiKTeX|TeX Live/); + assert.match(latexdiffInstallHint('linux'), /apt|TeX Live/); +}); + +// ───────────────────────────────────────────────────────────────────────────── +// Materializing the remote side +// ───────────────────────────────────────────────────────────────────────────── + +test('materializeTree: writes the whole tree, folders included', () => { + // --flatten resolves each \input relative to its own side, so the remote + // needs its structure on disk - the root document alone is not enough. + const dir = mkdtempSync(join(tmpdir(), 'olcli-materialize-')); + try { + const written = materializeTree(dir, new Map([ + ['main.tex', buf(ROOT)], + ['sections/intro.tex', buf('\\section{Intro}\n')], + ['figures/plot.pdf', Buffer.from([0x25, 0x50, 0x44, 0x46, 0x00])], + ])); + + assert.equal(written, 3); + assert.equal(readFileSync(join(dir, 'main.tex'), 'utf-8'), ROOT); + assert.equal(readFileSync(join(dir, 'sections', 'intro.tex'), 'utf-8'), '\\section{Intro}\n'); + assert.equal(readFileSync(join(dir, 'figures', 'plot.pdf')).length, 5); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + +test('materializeTree: entries escaping the destination are dropped, not written', () => { + // This writes files, and a path that was safe relative to the target + // directory is not automatically safe relative to a temporary one. See #44. + const dir = mkdtempSync(join(tmpdir(), 'olcli-materialize-')); + try { + const written = materializeTree(dir, new Map([ + ['main.tex', buf(ROOT)], + ['../escaped.tex', buf('nope\n')], + ['../../escaped-twice.tex', buf('nope\n')], + ])); + + assert.equal(written, 1); + assert.ok(existsSync(join(dir, 'main.tex'))); + assert.equal(existsSync(join(dir, '..', 'escaped.tex')), false); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); From 86b078b21038253ea47f86f3fd1f18e6bea48c46 Mon Sep 17 00:00:00 2001 From: Wayn_Liu Date: Wed, 9 Sep 2026 10:38:43 +0800 Subject: [PATCH 2/2] feat(diff): --exit-code so olcli diff can gate CI Second follow-up left open by #48, alongside --latexdiff. Reports diff(1)'s statuses: 0 when nothing differs, 1 when something does, 2 when the run itself failed. The 1-vs-2 split is the whole feature. A pipeline that cannot separate "the project differs" from "the run failed" reads an expired session cookie as a content change, and a job that goes red for the wrong reason sends whoever reads the log looking for a diff that was never computed. All ten failure paths in the command move to 2 together, so no failure can be mistaken for a difference. Without the flag they stay 1, as in every other command, so existing scripts are unaffected. The status is set rather than exited on. process.exit discards whatever is still buffered on a non-TTY stdout, and `olcli diff --exit-code > patch.txt` is exactly a large patch going into a pipe: exiting outright truncated a 1.5 MB patch to the 128 KB pipe buffer here, losing 91% of it mid-hunk. The gate covers whatever was compared, so --file narrows it the way a git diff --exit-code pathspec does, and under --latexdiff it reports on the project rather than on the markup - a changed figure is a real difference even though a marked-up root document cannot show one. Refs #48 --- CHANGELOG.md | 6 +++++ README.md | 35 +++++++++++++++++++++++++ SKILL.md | 8 ++++++ src/cli.ts | 66 ++++++++++++++++++++++++++++++++++++----------- src/diff.ts | 36 ++++++++++++++++++++++++++ test/diff.test.ts | 47 +++++++++++++++++++++++++++++++++ test/e2e.sh | 35 +++++++++++++++++++++++++ 7 files changed, 218 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 980000e..7da2bfb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,12 @@ All notable changes to this project will be documented in this file. - Placed next to the root document rather than at the project root, so relative `\includegraphics` and `\bibliography` paths resolve exactly as they do for the document it was built from - A compile failure writes the CLSI log next to the marked-up source instead of reporting only a status, and deletes a PDF left by an earlier run rather than leaving one that describes a different revision. The compile runs against the project, so a `.sty` or `.cls` that exists only locally is the usual cause and the message says so. A missing *figure* is not: Overleaf draws a placeholder box naming the file and still reports success - **`src/latexdiff.ts`** - root document detection, argument construction, output naming and failure interpretation are functions over data, unit-tested with no Overleaf account and no `latexdiff` binary. 21 tests, in the suite CI already runs +- **`olcli diff --exit-code` makes the command a CI gate** ([#48](https://github.com/aloth/olcli/pull/48)) - the second follow-up left open when the core `diff` command shipped in 0.10.0. It reports `diff(1)`'s statuses: `0` when nothing differs, `1` when something does, `2` when the run itself failed + - The `1` vs `2` split is the whole feature. A pipeline that cannot separate "the project differs" from "the run failed" reads an expired session cookie as a content change, and a job that goes red for the wrong reason sends whoever reads the log looking for a diff that was never computed. All ten failure paths in the command move to `2` together - bad flag combinations, a missing directory, an unresolvable project, a `latexdiff` that is not installed, a failed remote compile - so no failure can be mistaken for a difference + - Failures stay `1` when the flag is absent, which is what every other command exits, so scripts that check `olcli diff` for success are unaffected + - The status is *set* rather than exited on. `process.exit` discards whatever is still buffered on a non-TTY stdout, and `olcli diff --exit-code > patch.txt` is exactly a large patch going into a pipe: measured here, exiting outright truncated a 1.5 MB patch to the 128 KB pipe buffer and lost 91% of it mid-hunk. Returning lets node flush first + - The gate covers whatever was compared, so `--file` narrows it to one file the way a `git diff --exit-code` pathspec does, and a `--file` matching nothing is `0` rather than an error. Under `--latexdiff` it reports on the project rather than on the markup - a changed figure is a real difference even though a marked-up root document cannot show one + - Exit statuses and the "unchanged files are not differences" rule are pinned by unit tests in `test/diff.test.ts`; `test/e2e.sh` gains eight cases covering the clean tree, a difference, `--file` narrowing, the redirected-output case and both failure statuses ## [0.12.0] - 2026-09-06 diff --git a/README.md b/README.md index 4296009..18728d2 100644 --- a/README.md +++ b/README.md @@ -218,6 +218,7 @@ olcli diff # every changed file, as patches olcli diff --name-only # just the changed paths olcli diff --file main.tex # one file olcli diff -U 8 # wider context +olcli diff --exit-code # exit 1 if anything differs, for CI ``` **The remote side is fetched fresh on every run.** The diff describes the @@ -281,6 +282,40 @@ written next to the marked-up source when that happens. A *figure* you added locally does not fail it; Overleaf draws a placeholder box naming the missing file and the rest of the PDF is fine. +#### Using `diff` as a CI gate + +`--exit-code` turns the command into a check, with the statuses `diff(1)` uses: + +| Status | Meaning | +|--------|---------| +| `0` | Nothing differs | +| `1` | Something differs | +| `2` | The run failed — bad flags, no session, project unreachable | + +```yaml +- name: Fail if the paper on Overleaf has drifted from the repo + run: olcli diff --exit-code --name-only + env: + OVERLEAF_SESSION: ${{ secrets.OVERLEAF_SESSION }} +``` + +**The `1` versus `2` split is the point of the flag.** Without it a pipeline +cannot tell a changed file from an expired session cookie, and a job that goes +red for the second reason sends whoever reads the log hunting for a diff that +was never computed. Every other olcli command reports failure as `1`, and +`olcli diff` still does when `--exit-code` is absent, so adding the flag does +not change what existing scripts see. + +The gate covers whatever was compared: `--file main.tex` narrows it to one +file, the way a `git diff --exit-code` pathspec does, and a `--file` that +matches nothing is `0` rather than an error. Under `--latexdiff` it still +reports on the project as a whole — a changed figure is a real difference even +though a marked-up root document cannot show it. + +Note that this compares against Overleaf **now**, not against your last pull, +so the check is "has anyone drifted from what is committed here", which is what +makes it worth running on a schedule as well as on a push. + #### How deletion propagation works `olcli` records a manifest of remote files in `.olcli.json`. On next sync: diff --git a/SKILL.md b/SKILL.md index 16f7b93..a7e5a9c 100644 --- a/SKILL.md +++ b/SKILL.md @@ -157,12 +157,19 @@ olcli sync --no-delete # Sync without propagating local deletions to remote olcli diff # unified diff of every changed file olcli diff --name-only # changed paths only olcli diff --file main.tex # a single file +olcli diff --exit-code # CI gate: 0 same, 1 differs, 2 failed ``` The remote side is fetched fresh each run, so this shows what a subsequent `push` would overwrite — not a comparison against the last `pull`. `a/` is the remote, `b/` is local. Binary files are reported as differing without a patch. +`--exit-code` uses `diff(1)`'s statuses so the command can gate a pipeline: +`0` nothing differs, `1` something does, `2` the run itself failed. The last +one matters — without it a job cannot tell a changed file from an expired +session. Failures stay `1` when the flag is absent, so existing scripts are +unaffected. + ### Delete or rename remote files ```bash @@ -304,6 +311,7 @@ zip arxiv.zip *.tex main.bbl figures/*.pdf - **Auto-detect project**: Run commands from a synced directory (contains `.olcli.json`) to skip the project argument - **Dry run**: Use `olcli push --dry-run` or `olcli sync --dry-run` to preview before applying - **Preview content**: `push --dry-run` lists files by modification time; `olcli diff` compares actual contents, so the two lists can differ +- **CI gate**: `olcli diff --exit-code` exits 1 when anything differs and 2 when the run failed, so a pipeline can distinguish drift from breakage - **Force overwrite**: Use `olcli pull --force` to overwrite local changes - **Two-way deletes**: `olcli sync` propagates *local* deletions to the remote; use `--no-delete` to opt out per run - **Build artifacts**: `.aux`, `.bbl`, `.log`, `.synctex.gz` etc. are filtered by default. Add custom patterns to a `.olignore` file (gitignore-style) diff --git a/src/cli.ts b/src/cli.ts index f03d3c3..d6d8b9f 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -17,7 +17,15 @@ import { OverleafClient } from './client.js'; import { resolveRemotePath, resolveWithin, normalizeRemotePath } from './paths.js'; import { planProjectRenames } from './rename-plan.js'; import { scanLocalFiles } from './scan.js'; -import { compareTrees, filterRemoteTree, renderFileDiff, statusLetter, type FileDiff } from './diff.js'; +import { + compareTrees, + DIFF_EXIT_FAILURE, + differencesExitCode, + filterRemoteTree, + renderFileDiff, + statusLetter, + type FileDiff, +} from './diff.js'; import { DIFF_OUTPUT_DIR, LatexdiffError, @@ -1800,6 +1808,7 @@ program .option('--name-only', 'List changed paths instead of printing patches') .option('--file ', 'Diff a single file') .option('-U, --unified ', 'Lines of context around each hunk (default: 3)', parseInt) + .option('--exit-code', 'Exit 1 if anything differs, 0 if nothing does, 2 on failure (for CI)') .option('--latexdiff', 'Mark the revision up inside the document with latexdiff (requires latexdiff on PATH)') .option('--pdf', 'Compile the marked-up document on Overleaf and download the PDF (implies --latexdiff)') .option('--main ', 'Root .tex document to mark up (default: the only file declaring \\documentclass)') @@ -1821,13 +1830,26 @@ reason. inside the document instead: struck through is what a push would overwrite, underlined is what it would upload. --pdf additionally uploads the marked-up document to the project for one compile, downloads the PDF, and removes it -again - so a reviewable PDF needs no local TeX installation.`) +again - so a reviewable PDF needs no local TeX installation. + +--exit-code makes the command a CI gate, with diff(1)'s statuses: 0 when +nothing differs, 1 when something does, and 2 when the run itself failed. The +last one matters - without it a pipeline cannot tell a changed file from an +expired session cookie. It applies to whatever was compared, so --file narrows +the gate to one file, and it reports on the project even under --latexdiff, +where the markup only covers the root document.`) .action(async (project, dir, options) => { const targetDir = dir || '.'; + // Every way this command can fail, as opposed to finding differences. + // Under --exit-code that has to be distinguishable from status 1, or a + // pipeline reads "could not reach Overleaf" as "the paper changed"; with + // the flag absent it stays 1, which is what every other command exits. + const failureCode = options.exitCode ? DIFF_EXIT_FAILURE : 1; + if (!existsSync(targetDir)) { console.error(chalk.red(`Directory not found: ${targetDir}`)); - process.exit(1); + process.exit(failureCode); } // Checked before connecting: an unusable combination of flags should not @@ -1842,7 +1864,7 @@ again - so a reviewable PDF needs no local TeX installation.`) if (latexdiffMode && conflicting.length > 0) { console.error(chalk.red(`--latexdiff cannot be combined with ${conflicting.join(', ')}`)); console.error('It marks up one root document; those options select and shape unified patch output.'); - process.exit(1); + process.exit(failureCode); } if (!latexdiffMode) { @@ -1855,7 +1877,7 @@ again - so a reviewable PDF needs no local TeX installation.`) ].filter(Boolean); if (latexdiffOnly.length > 0) { console.error(chalk.red(`${latexdiffOnly.join(', ')} only applies with --latexdiff`)); - process.exit(1); + process.exit(failureCode); } } @@ -1869,7 +1891,7 @@ again - so a reviewable PDF needs no local TeX installation.`) } catch (error: any) { spinner.fail(error.message); console.error('Either run from a directory with .olcli.json or pass a project name/ID'); - process.exit(1); + process.exit(failureCode); } const { id: projectId, name: projectName } = resolved; @@ -1916,10 +1938,15 @@ again - so a reviewable PDF needs no local TeX installation.`) remoteFiles, entries, fetchedAt, + failureCode, options, spinner, }); setLastProject(projectId); + // Reports on the comparison, not on the markup: a changed figure is a + // difference in the project even though a marked-up root document + // cannot show it. The `No .tex file differs` line above says as much. + if (options.exitCode) process.exitCode = differencesExitCode(entries); return; } @@ -1984,9 +2011,14 @@ again - so a reviewable PDF needs no local TeX installation.`) console.log(chalk.dim(` a/ = remote as of ${fetchedAt.toISOString()}, b/ = local`)); setLastProject(projectId); + // Set rather than exited: `process.exit` drops whatever is still + // buffered on a non-TTY stdout, and `olcli diff --exit-code > patch.txt` + // is precisely a large patch going into a pipe. Returning lets node + // flush and then exit with this status on its own. + if (options.exitCode) process.exitCode = differencesExitCode(entries); } catch (error: any) { spinner.fail(`Failed: ${error.message}`); - process.exit(1); + process.exit(failureCode); } }); @@ -2010,6 +2042,8 @@ async function runLatexdiffMode(params: { remoteFiles: Map; entries: FileDiff[]; fetchedAt: Date; + /** What to exit with when this mode fails; 2 under --exit-code, else 1. */ + failureCode: number; /** Only the flags this mode reads; the rest of `diff`'s options are rejected. */ options: { main?: string; @@ -2022,7 +2056,7 @@ async function runLatexdiffMode(params: { }): Promise { const { client, projectId, projectName, targetDir, - localFiles, remoteFiles, entries, fetchedAt, options, spinner, + localFiles, remoteFiles, entries, fetchedAt, failureCode, options, spinner, } = params; let root = ''; @@ -2035,7 +2069,7 @@ async function runLatexdiffMode(params: { for (const candidate of error.candidates) { console.error(chalk.dim(` ${candidate}`)); } - process.exit(1); + process.exit(failureCode); } // latexdiff needs two versions of the same document. A root document that @@ -2045,7 +2079,7 @@ async function runLatexdiffMode(params: { spinner.stop(); console.error(chalk.red(`${root} is not in "${projectName}" yet, so there is no earlier version to mark up.`)); console.error(chalk.dim(' Push it first, or use --main to name a document that exists on both sides.')); - process.exit(1); + process.exit(failureCode); } if (!entries.some((e) => isTexPath(e.path))) { @@ -2080,7 +2114,7 @@ async function runLatexdiffMode(params: { console.error(error.stderr); } cleanupTmp(); - process.exit(1); + process.exit(failureCode); } // An explicit --output is a path the user chose, so it is taken relative to @@ -2100,7 +2134,7 @@ async function runLatexdiffMode(params: { } if (options.pdf) { - await compileMarkupOnOverleaf({ client, projectId, projectName, root, markup, texPath, pdfPath, spinner }); + await compileMarkupOnOverleaf({ client, projectId, projectName, root, markup, texPath, pdfPath, failureCode, spinner }); } console.log(); @@ -2146,9 +2180,11 @@ async function compileMarkupOnOverleaf(params: { markup: string; texPath: string; pdfPath: string; + /** What to exit with when the compile fails; 2 under --exit-code, else 1. */ + failureCode: number; spinner: ReturnType; }): Promise { - const { client, projectId, projectName, root, markup, texPath, pdfPath, spinner } = params; + const { client, projectId, projectName, root, markup, texPath, pdfPath, failureCode, spinner } = params; const scratch = remoteScratchPath(root); spinner.start(`Checking ${scratch} is free...`); @@ -2157,7 +2193,7 @@ async function compileMarkupOnOverleaf(params: { console.error(chalk.dim(' --pdf uploads the marked-up document under that name for one compile and')); console.error(chalk.dim(' removes it again; it will not overwrite a file that is already there.')); console.error(chalk.dim(` The marked-up source was still written: ${texPath}`)); - process.exit(1); + process.exit(failureCode); } spinner.stop(); @@ -2226,7 +2262,7 @@ async function compileMarkupOnOverleaf(params: { if (failure) { spinner.fail(failure[0]); for (const line of failure.slice(1)) console.error(chalk.dim(line)); - process.exit(1); + process.exit(failureCode); } } diff --git a/src/diff.ts b/src/diff.ts index 1e0c38a..a2af0ad 100644 --- a/src/diff.ts +++ b/src/diff.ts @@ -217,3 +217,39 @@ export function statusLetter(status: FileStatus): string { case 'unchanged': return ' '; } } + +/** + * Exit statuses for `diff --exit-code`, following diff(1) rather than the + * plain success/failure every other olcli command reports. + * + * The 1-vs-2 split is the entire point of the flag. A gate that cannot + * separate "the project differs" from "the run failed" reads an expired + * cookie or a typo'd flag as a content change, and a pipeline that fails for + * the wrong reason is worse than one that does not fail at all - it sends + * whoever reads the log looking for a diff that was never computed. + * + * Only `--exit-code` moves failures to 2. Without it every failure stays 1, + * so existing scripts that check `olcli diff` for success keep working. + */ +export const DIFF_EXIT_CLEAN = 0; +export const DIFF_EXIT_DIFFERENCES = 1; +export const DIFF_EXIT_FAILURE = 2; + +/** + * The status `diff --exit-code` should finish with, given the comparison it + * just rendered. + * + * Takes the entries rather than a count so an unfiltered `compareTrees` + * result answers the same as the filtered list the command prints from: a + * gate that fired because every *unchanged* file was counted would report a + * difference on every run and be switched off within a day. + * + * The entries passed in are whatever was reported, so `--file main.tex` + * narrows the question to that file, the same way `git diff --exit-code` + * narrows to its pathspec. + */ +export function differencesExitCode(entries: FileDiff[]): number { + return entries.some((e) => e.status !== 'unchanged') + ? DIFF_EXIT_DIFFERENCES + : DIFF_EXIT_CLEAN; +} diff --git a/test/diff.test.ts b/test/diff.test.ts index 99c07c4..f50c58e 100644 --- a/test/diff.test.ts +++ b/test/diff.test.ts @@ -2,6 +2,10 @@ import { test } from 'node:test'; import assert from 'node:assert/strict'; import { compareTrees, + DIFF_EXIT_CLEAN, + DIFF_EXIT_DIFFERENCES, + DIFF_EXIT_FAILURE, + differencesExitCode, filterRemoteTree, renderFileDiff, isBinary, @@ -217,3 +221,46 @@ test('renderFileDiff: a non-numeric context falls back to the default', () => { assert.equal(bad, good); assert.match(bad, /^\+line TEN$/m); }); + +// ─── --exit-code ────────────────────────────────────────────────────────────── + +const entry = (status: 'added' | 'deleted' | 'modified' | 'unchanged') => + ({ path: `${status}.tex`, status, binary: false }); + +test('differencesExitCode: nothing to report is a clean gate', () => { + assert.equal(differencesExitCode([]), DIFF_EXIT_CLEAN); +}); + +test('differencesExitCode: unchanged files are not differences', () => { + // The command filters these out before printing, but the gate is fed + // whatever it is given: counting entries instead of inspecting their status + // would fire on every run of a project that matches its remote exactly. + const local = new Map([['a.tex', buf('same\n')], ['b.tex', buf('same\n')]]); + const unfiltered = compareTrees(local, new Map(local)); + + assert.equal(unfiltered.length, 2); + assert.equal(differencesExitCode(unfiltered), DIFF_EXIT_CLEAN); +}); + +test('differencesExitCode: every non-unchanged status is a difference', () => { + // A remote-only file included on purpose: plain `push` leaves it alone, but + // the two sides still do not match, and a gate that passed would be saying + // they do. + for (const status of ['added', 'deleted', 'modified'] as const) { + assert.equal(differencesExitCode([entry(status)]), DIFF_EXIT_DIFFERENCES, status); + } +}); + +test('differencesExitCode: one difference among unchanged files still fires', () => { + const entries = [entry('unchanged'), entry('modified'), entry('unchanged')]; + assert.equal(differencesExitCode(entries), DIFF_EXIT_DIFFERENCES); +}); + +test('exit statuses follow diff(1), and failure is distinguishable', () => { + // The whole point of the flag: a CI job has to be able to tell a changed + // file from a run that never got as far as comparing anything. + assert.equal(DIFF_EXIT_CLEAN, 0); + assert.equal(DIFF_EXIT_DIFFERENCES, 1); + assert.equal(DIFF_EXIT_FAILURE, 2); + assert.notEqual(DIFF_EXIT_FAILURE, DIFF_EXIT_DIFFERENCES); +}); diff --git a/test/e2e.sh b/test/e2e.sh index ff1a5dc..0570ac6 100755 --- a/test/e2e.sh +++ b/test/e2e.sh @@ -574,6 +574,9 @@ log_section "Diff Tests" run_test "diff reports no changes on a freshly pulled directory" \ "cd '$PULL_DIR' && olcli diff | grep -q 'No differences'" +run_test "diff --exit-code exits 0 when nothing differs" \ + "cd '$PULL_DIR' && olcli diff --exit-code >/dev/null" + DIFF_TEST_FILE="$PULL_DIR/${TEST_ID}.txt" DIFF_ORIGINAL_CONTENT=$(cat "$DIFF_TEST_FILE") echo "diff test modification - $TIMESTAMP" >> "$DIFF_TEST_FILE" @@ -587,6 +590,38 @@ run_test "diff shows the added line as a local addition" \ run_test "diff --file limits output to the requested file" \ "cd '$PULL_DIR' && test \$(olcli diff --file '${TEST_ID}.txt' | grep -c '^diff --olcli') -eq 1" +run_test "diff --exit-code exits 1 when a file differs" \ + "cd '$PULL_DIR' && olcli diff --exit-code >/dev/null; test \$? -eq 1" + +# The gate is opt-in: a script that checks plain `olcli diff` for success must +# keep seeing success when the project simply has changes in it. +run_test "diff without --exit-code still exits 0 when a file differs" \ + "cd '$PULL_DIR' && olcli diff >/dev/null" + +run_test "diff --exit-code narrows the gate to --file" \ + "cd '$PULL_DIR' && olcli diff --exit-code --file '${TEST_ID}.txt' >/dev/null; test \$? -eq 1" + +# Same as a git pathspec that matches nothing: no difference was found, so the +# gate is green rather than an error. +run_test "diff --exit-code exits 0 for a --file that differs in nothing" \ + "cd '$PULL_DIR' && olcli diff --exit-code --file 'no-such-file-here.tex' >/dev/null" + +# A large patch redirected to a file is the case the gate is written for, and +# the case where exiting outright would truncate stdout mid-hunk. The capture +# file is dotted so the diff it is capturing does not report it. +run_test "diff --exit-code writes its whole patch when redirected" \ + "cd '$PULL_DIR' && { olcli diff --exit-code > '$PULL_DIR/.diff-gate.out'; test \$? -eq 1; } && tail -1 '$PULL_DIR/.diff-gate.out' | grep -q 'b/ = local'" + +# The distinction the flag exists for: a run that failed must not look like a +# run that found changes. No network needed - a bad directory fails early. +run_test "diff --exit-code exits 2 when the run itself fails" \ + "olcli diff someproject /nonexistent-olcli-dir --exit-code >/dev/null 2>&1; test \$? -eq 2" + +run_test "diff without --exit-code still reports failure as 1" \ + "olcli diff someproject /nonexistent-olcli-dir >/dev/null 2>&1; test \$? -eq 1" + +rm -f "$PULL_DIR/.diff-gate.out" + # Restore, so the push tests below see the tree they expect. printf '%s\n' "$DIFF_ORIGINAL_CONTENT" > "$DIFF_TEST_FILE"