feat(diff): --latexdiff and --pdf for marked-up revisions - #56
Conversation
Follow-up to aloth#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 aloth#55
|
Built and tested locally on top of On design question 1: go with the temp file in the project, as implemented. A scratch project trades one stray file for a stray project, and uploading the whole tree file by file to compile one document is a lot of requests for a worse failure mode. Compiling locally gives up the reason The three properties you call structural are the ones that make that acceptable: refusing rather than overwriting, deleting from a Correcting yourself on the missing-figure case in the PR body is worth more than getting it right first time. A placeholder box and a successful compile is exactly the kind of detail that would otherwise have shipped as a wrong error message. Taking this out of draft and merging before #57. |
CI gate from #48: olcli diff --exit-code reports diff(1)'s statuses, 0 when nothing differs, 1 when something does, 2 when the run itself failed. Author: @Waynting. Stacked on #56, so only 86b078b is new here. Verified before merging: npm ci, lint and build clean, 84 tests passing. The 1 versus 2 split is the feature. All ten failure paths move to 2 together, so a pipeline cannot read an expired session cookie as a content change. Without the flag every failure stays 1, which is what every other command exits, so existing scripts are unaffected.
Draft PR for
--latexdiffand--pdffrom #55 — the follow-up left open when the corediffcommand shipped in 0.10.0.Opening it as a draft because design question 1 in #55 is still yours to settle:
--pdfcannot compile without writing to the project. Everything below implements the option I leaned to there (a temp file in the project, cleaned up); if you would rather it created a scratch project or compiled locally, the change is contained to one function.What it does
It runs on the two sides
diffhas already fetched, so the markup means exactly what the patch output means — struck through is what a push would overwrite, underlined is what it would upload — and it costs no extra request.The decisions from #55, and what testing changed about them
--pdfuploads, compiles, deletes. The compile endpoint takes arootResourcePaththat must already be in the project, so there is no way to compile a document that is not in it. The markup goes up asolcli-latexdiff.texnext to the root document, and the command says so before it does it:Three properties are structural rather than incidental, and each is exercised below:
finally, and nothing between the upload and it callsprocess.exit— that would terminate before the cleanup and leave the file on the project. Failures are collected and reported after the cleanup instead.SIGINTprints the exactolcli rmcommand rather than leaving the file to be discovered.--flattenby default. Without it the markup still contains\input{sections/intro}, and a remote compile resolves those against the files in the project — the old content — producing a PDF that marks up the root document and shows every input file as unchanged. Wrong in a way that is hard to notice.latexdiff --flattenresolves each\inputrelative to its own side, so pointing it at the extracted remote tree and the working directory needs no cwd juggling.Output goes to
.olcli-diff/.scanLocalFilesskips dotted entries before any ignore rule is consulted, so this is the one place output cannot leak into a later push. A plainmain-diff.texnext to the document would be uploaded on the next sync.Ambiguity is reported, not guessed. The root document is the
.texdeclaring\documentclass; several candidates are listed and--mainis asked for. Preferringmain.texwould silently mark up the wrong document in a project that also holds a poster, which produces a plausible PDF describing the wrong revision.One thing in #55 I had wrong, found by testing it. I wrote that a figure existing only locally would fail the
--pdfcompile. It does not: Overleaf draws a placeholder box naming the missing file and reports success. What actually fails is a.styor.clsthat only exists locally, because the compile runs against the project. The message, the README and the changelog now say that instead, and the failure path also deletes a PDF left by an earlier run rather than leaving one dated now that describes a different revision.Rejected flag combinations, before connecting
--latexdiffwith--name-only,--fileor-Uis refused, as is--main/--output/--latexdiff-optwithout--latexdiff— accepting those silently would print a normal patch with nothing saying a flag had been dropped. All of it is checked before the login and the project download, so a typo costs no requests.--latexdiff-optOne addition to the flag list in #55:
--latexdiff-opt --math-markup=0passes options straight through. Without it there is no way to tunelatexdiffat all, because the remote tree exists on disk only for the duration of the run and cannot be handed to the tool by hand afterwards. Arguments go throughexecFileas an argv array, never a shell.Structure and tests
src/latexdiff.tsholds root document detection, argument construction, output naming, tree materialization and failure interpretation as functions over data — 21 unit tests, no Overleaf account and nolatexdiffbinary required. 79 tests total undernpm test.npm run lintadds no new warnings to the budget (62 before and after; the module itself contributes none).docs/ARCHITECTURE.mdgets the new row, and its module-map heading is corrected: that column claimed "no filesystem", whichscan.tsalready contradicted andlatexdiff.ts— the one module that shells out to something olcli does not ship — would have contradicted further.test/e2e.shgains a latexdiff section between the diff and push tests, skipped with a warning whenlatexdiffis not on PATH. It uploads a root document of its own so it does not depend on what the target project contains, checks the markup strikes through the remote wording and underlines the local one, checks the output is invisible topush --dry-run, and checks--pdfcleaned up after itself — usingdiff --name-only, since a leftover scratch file is exactly whatdiffreports as remote-only.Verified against a real project
npm run lint,npm run build,npm testand the entry-point check all pass. Beyond that I ran the feature by hand against a throwaway Overleaf project created from the example template —main.tex,sample.bibandfrog.jpg, so the compile had a figure and a bibliography to resolve:--latexdiffon an editedmain.texproduced the markup;latexdiff's own warning about not flattening\bibliographywas passed through rather than swallowed, and is harmless here because the remote compile hassample.bib.--pdfreturned a 272 KB PDF with the title, abstract and introduction changes struck through in red and underlined in blue, the frog figure and the hyperlinks intact.diff --name-onlyafterwards reported0 remote-only, so the scratch file was gone;push --dry-runlisted onlymain.tex, so.olcli-diff/stayed invisible to push.olcli-latexdiff.texby hand and re-running--pdfwas refused, and the file came back byte-identical..stymade the compile fail: exit 1, the CLSI log written to.olcli-diff/main-diff.logwith! LaTeX Error: File 'mylocalstyle.sty' not found., the stale PDF removed, and the scratch file still deleted from the project.I have not run the full
test/e2e.sh; the account I tested with has noolcli testproject, and I did not want to point the suite at a real paper.Follow-ups, not in this PR
--exit-code, still open from feat(diff): content-level diff between local files and the remote project #48Refs #55