From 06f833a7c8f5bb89716c68a46b977750a5177134 Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Mon, 29 Jun 2026 11:23:32 +0200 Subject: [PATCH 1/2] fix(manuscript): keep knitr figures on Windows freeze render (#14613) On a manuscript first render with freeze, renderCleanup compared the engine-reported supporting dir to the normalized filesDir with a raw string equality. The knitr engine reports paths with forward slashes on Windows while filesDir uses the platform separator, so the comparison failed there and the parent index_files dir was removed wholesale instead of being narrowed to the figure dir. Subsequent format completions then found no index_files and copied nothing into _manuscript. Only knitr is affected; the Jupyter engine builds the path with the platform separator, which already matches. Compare paths separator-agnostically via a new pathsEqual helper, and adopt it at the two existing preview call sites that hand-rolled the same normalize-both-sides comparison. --- news/changelog-1.10.md | 4 ++ src/command/preview/preview-shiny.ts | 4 +- src/command/preview/preview.ts | 3 +- src/command/render/cleanup.ts | 8 ++- src/core/path.ts | 8 +++ tests/unit/path.test.ts | 24 ++++++++ tests/unit/render/cleanup.test.ts | 84 ++++++++++++++++++++++++++++ 7 files changed, 131 insertions(+), 4 deletions(-) create mode 100644 tests/unit/render/cleanup.test.ts diff --git a/news/changelog-1.10.md b/news/changelog-1.10.md index 6dc3f66aae6..b8d7ebec3a6 100644 --- a/news/changelog-1.10.md +++ b/news/changelog-1.10.md @@ -45,6 +45,10 @@ All changes included in 1.10: ## Projects +### Manuscripts + +- ([#14613](https://github.com/quarto-dev/quarto-cli/issues/14613)): Fix dynamically generated figures missing from the rendered manuscript on Windows on the first render when using the knitr engine with `freeze: auto`. + ### Websites - ([#13565](https://github.com/quarto-dev/quarto-cli/issues/13565), [#14353](https://github.com/quarto-dev/quarto-cli/issues/14353)): Fix sidebar logo not appearing on secondary sidebars in multi-sidebar website layouts. diff --git a/src/command/preview/preview-shiny.ts b/src/command/preview/preview-shiny.ts index 8c55ed1906e..224c46be0b1 100644 --- a/src/command/preview/preview-shiny.ts +++ b/src/command/preview/preview-shiny.ts @@ -33,7 +33,7 @@ import { } from "../../core/http.ts"; import { findOpenPort } from "../../core/port.ts"; import { handleHttpRequests } from "../../core/http-server.ts"; -import { normalizePath } from "../../core/path.ts"; +import { pathsEqual } from "../../core/path.ts"; import { previewMonitorResources } from "../../core/quarto.ts"; import { renderServices } from "../render/render-services.ts"; import { RenderFlags } from "../render/types.ts"; @@ -123,7 +123,7 @@ function runPreviewControlService( // helper to check whether a render request is compatible // with the original render const isCompatibleRequest = async (prevReq: PreviewRenderRequest) => { - return normalizePath(options.input) === normalizePath(prevReq.path) && + return pathsEqual(options.input, prevReq.path) && await previewRenderRequestIsCompatible( prevReq, options.project, diff --git a/src/command/preview/preview.ts b/src/command/preview/preview.ts index 868259e6073..948867203ea 100644 --- a/src/command/preview/preview.ts +++ b/src/command/preview/preview.ts @@ -72,6 +72,7 @@ import { projectOutputDir } from "../../project/project-shared.ts"; import { projectContext } from "../../project/project-context.ts"; import { normalizePath, + pathsEqual, pathWithForwardSlashes, safeExistsSync, } from "../../core/path.ts"; @@ -788,7 +789,7 @@ function htmlFileRequestHandlerOptions( !invalidateDevServerReRender && prevReq && existsSync(prevReq.path) && - normalizePath(prevReq.path) === normalizePath(inputFile) && + pathsEqual(prevReq.path, inputFile) && await previewRenderRequestIsCompatible(prevReq, project, flags.to) ) { // don't wait for the promise so the diff --git a/src/command/render/cleanup.ts b/src/command/render/cleanup.ts index 40390e594b7..1c39d5eae3e 100644 --- a/src/command/render/cleanup.ts +++ b/src/command/render/cleanup.ts @@ -11,6 +11,7 @@ import * as ld from "../../core/lodash.ts"; import { normalizePath, + pathsEqual, removeIfEmptyDir, removeIfExists, } from "../../core/path.ts"; @@ -73,7 +74,12 @@ export function renderCleanup( filesDir = normalizePath(filesDir); } supporting = supporting.map((supportingDir) => { - if (filesDir === supportingDir) { + // Compare separator-agnostically: the knitr engine reports supportingDir + // with forward slashes on Windows (R path convention) while filesDir uses + // the platform separator. A raw === comparison fails on Windows, so the + // parent index_files dir gets removed wholesale instead of being narrowed + // to the figure dir (#14613). + if (pathsEqual(filesDir, supportingDir)) { return join(filesDir, figuresDir(figureFormat)); } else { return supportingDir; diff --git a/src/core/path.ts b/src/core/path.ts index fab741516cc..06f14be96ec 100644 --- a/src/core/path.ts +++ b/src/core/path.ts @@ -327,6 +327,14 @@ export function normalizePath(path: string | URL): string { return file.replace(/^\w:\\/, (m: string) => m[0].toUpperCase() + ":\\"); } +// Compares two filesystem paths for equality in a separator-agnostic way by +// normalizing both sides first. Use this instead of comparing raw path strings: +// engines (notably knitr) can report paths with forward slashes on Windows even +// when other paths use the platform separator, so a raw === comparison fails. +export function pathsEqual(a: string, b: string): boolean { + return normalizePath(a) === normalizePath(b); +} + // Moved here from env.ts to avoid circular dependency export function suggestUserBinPaths() { if (!isWindows) { diff --git a/tests/unit/path.test.ts b/tests/unit/path.test.ts index e0af8aaf776..32f9913c8db 100644 --- a/tests/unit/path.test.ts +++ b/tests/unit/path.test.ts @@ -11,6 +11,7 @@ import { join, resolve } from "../../src/deno_ral/path.ts"; import { isWindows } from "../../src/deno_ral/platform.ts"; import { dirAndStem, + pathsEqual, removeIfEmptyDir, removeIfExists, resolvePathGlobs, @@ -90,6 +91,29 @@ unitTest("path - dirAndStem", async () => { }); }); +// Path equality must be separator-agnostic: the knitr engine reports paths +// with forward slashes on Windows while other paths are normalized to the +// platform separator. Comparing the raw strings then fails on Windows (#14613). +// deno-lint-ignore require-await +unitTest("path - pathsEqual is separator-agnostic (#14613)", async () => { + const dir = Deno.makeTempDirSync({ prefix: "quarto-pathsequal-test" }); + try { + const filesDir = join(dir, "index_files"); + const forwardSlash = filesDir.replaceAll("\\", "/"); + + assert( + pathsEqual(filesDir, forwardSlash), + "same path with different separators must compare equal", + ); + assert( + !pathsEqual(filesDir, join(dir, "other_files")), + "different paths must not compare equal", + ); + } finally { + Deno.removeSync(dir, { recursive: true }); + } +}); + interface GlobTest { name: string; globs: string[]; diff --git a/tests/unit/render/cleanup.test.ts b/tests/unit/render/cleanup.test.ts new file mode 100644 index 00000000000..f5c5e1a91ff --- /dev/null +++ b/tests/unit/render/cleanup.test.ts @@ -0,0 +1,84 @@ +/* + * cleanup.test.ts + * + * Copyright (C) 2026 Posit Software, PBC + */ + +import { assert } from "testing/asserts"; +import { ensureDirSync, existsSync } from "../../../src/deno_ral/fs.ts"; +import { join } from "../../../src/deno_ral/path.ts"; +import { unitTest } from "../../test.ts"; +import { renderCleanup } from "../../../src/command/render/cleanup.ts"; +import { Format } from "../../../src/config/types.ts"; +import { createMockProjectContext } from "../project/utils.ts"; + +// Minimal ipynb format: ipynb is a self-contained (standalone) output, so the +// self-contained cleanup path in render.ts passes the supporting dirs here. +function ipynbFormat(): Format { + return { + pandoc: { to: "ipynb" }, + execute: {}, + render: {}, + metadata: {}, + language: {}, + } as unknown as Format; +} + +// Regression test for #14613. +// +// On a manuscript first render with freeze:auto, the ipynb completion triggers +// the self-contained supporting cleanup. The supporting dir reported by the +// engine uses forward slashes; renderCleanup normalizes the computed filesDir +// with normalizePath (backslashes on Windows). The separator mismatch made the +// equality check fail, so the parent index_files dir (instead of the narrowed +// figure- dir) was handed to safeRemoveDirSync, deleting ALL figures. +// Subsequent format completions then found no index_files and copied nothing +// into _manuscript. +// +// deno-lint-ignore require-await +unitTest("renderCleanup - narrows ipynb supporting despite path separator mismatch (#14613)", async () => { + const projectDir = Deno.makeTempDirSync({ prefix: "quarto-cleanup-test" }); + const project = createMockProjectContext({ dir: projectDir }); + try { + const input = join(projectDir, "index.qmd"); + Deno.writeTextFileSync(input, ""); + + const filesDir = join(projectDir, "index_files"); + const figureHtmlDir = join(filesDir, "figure-html"); + const figureIpynbDir = join(filesDir, "figure-ipynb"); + ensureDirSync(figureHtmlDir); + ensureDirSync(figureIpynbDir); + const figureHtmlPlot = join(figureHtmlDir, "plot.png"); + Deno.writeTextFileSync(figureHtmlPlot, "html-figure"); + Deno.writeTextFileSync(join(figureIpynbDir, "plot.png"), "ipynb-figure"); + + // Engine reports the supporting dir with forward slashes (as knitr does). + const supportingForwardSlash = filesDir.replaceAll("\\", "/"); + + renderCleanup( + input, + join(projectDir, "_manuscript", "index.out.ipynb"), + ipynbFormat(), + project, + [supportingForwardSlash], + ); + + // Cleanup must narrow to figure-ipynb only; the html figures the other + // manuscript formats depend on must survive. + assert( + existsSync(figureHtmlPlot), + "index_files/figure-html must survive ipynb supporting cleanup", + ); + assert( + !existsSync(figureIpynbDir), + "index_files/figure-ipynb should be removed by narrowed cleanup", + ); + } finally { + project.cleanup(); + try { + Deno.removeSync(projectDir, { recursive: true }); + } catch { + // ignore + } + } +}); From 2699fa764f44b20a35cd5e748dcdc15efcfe8462 Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Mon, 29 Jun 2026 11:31:01 +0200 Subject: [PATCH 2/2] Remove unnecessary comment --- src/command/render/cleanup.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/command/render/cleanup.ts b/src/command/render/cleanup.ts index 1c39d5eae3e..ce2a75be12d 100644 --- a/src/command/render/cleanup.ts +++ b/src/command/render/cleanup.ts @@ -74,11 +74,6 @@ export function renderCleanup( filesDir = normalizePath(filesDir); } supporting = supporting.map((supportingDir) => { - // Compare separator-agnostically: the knitr engine reports supportingDir - // with forward slashes on Windows (R path convention) while filesDir uses - // the platform separator. A raw === comparison fails on Windows, so the - // parent index_files dir gets removed wholesale instead of being narrowed - // to the figure dir (#14613). if (pathsEqual(filesDir, supportingDir)) { return join(filesDir, figuresDir(figureFormat)); } else {