Skip to content

Commit 894f43f

Browse files
authored
fix(manuscript): keep knitr figures on Windows on first freeze render (#14633)
* 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.
1 parent 532be5b commit 894f43f

7 files changed

Lines changed: 126 additions & 4 deletions

File tree

news/changelog-1.10.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,10 @@ All changes included in 1.10:
4545

4646
## Projects
4747

48+
### Manuscripts
49+
50+
- ([#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`.
51+
4852
### Websites
4953

5054
- ([#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.

src/command/preview/preview-shiny.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ import {
3333
} from "../../core/http.ts";
3434
import { findOpenPort } from "../../core/port.ts";
3535
import { handleHttpRequests } from "../../core/http-server.ts";
36-
import { normalizePath } from "../../core/path.ts";
36+
import { pathsEqual } from "../../core/path.ts";
3737
import { previewMonitorResources } from "../../core/quarto.ts";
3838
import { renderServices } from "../render/render-services.ts";
3939
import { RenderFlags } from "../render/types.ts";
@@ -123,7 +123,7 @@ function runPreviewControlService(
123123
// helper to check whether a render request is compatible
124124
// with the original render
125125
const isCompatibleRequest = async (prevReq: PreviewRenderRequest) => {
126-
return normalizePath(options.input) === normalizePath(prevReq.path) &&
126+
return pathsEqual(options.input, prevReq.path) &&
127127
await previewRenderRequestIsCompatible(
128128
prevReq,
129129
options.project,

src/command/preview/preview.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ import { projectOutputDir } from "../../project/project-shared.ts";
7272
import { projectContext } from "../../project/project-context.ts";
7373
import {
7474
normalizePath,
75+
pathsEqual,
7576
pathWithForwardSlashes,
7677
safeExistsSync,
7778
} from "../../core/path.ts";
@@ -788,7 +789,7 @@ function htmlFileRequestHandlerOptions(
788789
!invalidateDevServerReRender &&
789790
prevReq &&
790791
existsSync(prevReq.path) &&
791-
normalizePath(prevReq.path) === normalizePath(inputFile) &&
792+
pathsEqual(prevReq.path, inputFile) &&
792793
await previewRenderRequestIsCompatible(prevReq, project, flags.to)
793794
) {
794795
// don't wait for the promise so the

src/command/render/cleanup.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import * as ld from "../../core/lodash.ts";
1111

1212
import {
1313
normalizePath,
14+
pathsEqual,
1415
removeIfEmptyDir,
1516
removeIfExists,
1617
} from "../../core/path.ts";
@@ -73,7 +74,7 @@ export function renderCleanup(
7374
filesDir = normalizePath(filesDir);
7475
}
7576
supporting = supporting.map((supportingDir) => {
76-
if (filesDir === supportingDir) {
77+
if (pathsEqual(filesDir, supportingDir)) {
7778
return join(filesDir, figuresDir(figureFormat));
7879
} else {
7980
return supportingDir;

src/core/path.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -327,6 +327,14 @@ export function normalizePath(path: string | URL): string {
327327
return file.replace(/^\w:\\/, (m: string) => m[0].toUpperCase() + ":\\");
328328
}
329329

330+
// Compares two filesystem paths for equality in a separator-agnostic way by
331+
// normalizing both sides first. Use this instead of comparing raw path strings:
332+
// engines (notably knitr) can report paths with forward slashes on Windows even
333+
// when other paths use the platform separator, so a raw === comparison fails.
334+
export function pathsEqual(a: string, b: string): boolean {
335+
return normalizePath(a) === normalizePath(b);
336+
}
337+
330338
// Moved here from env.ts to avoid circular dependency
331339
export function suggestUserBinPaths() {
332340
if (!isWindows) {

tests/unit/path.test.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { join, resolve } from "../../src/deno_ral/path.ts";
1111
import { isWindows } from "../../src/deno_ral/platform.ts";
1212
import {
1313
dirAndStem,
14+
pathsEqual,
1415
removeIfEmptyDir,
1516
removeIfExists,
1617
resolvePathGlobs,
@@ -90,6 +91,29 @@ unitTest("path - dirAndStem", async () => {
9091
});
9192
});
9293

94+
// Path equality must be separator-agnostic: the knitr engine reports paths
95+
// with forward slashes on Windows while other paths are normalized to the
96+
// platform separator. Comparing the raw strings then fails on Windows (#14613).
97+
// deno-lint-ignore require-await
98+
unitTest("path - pathsEqual is separator-agnostic (#14613)", async () => {
99+
const dir = Deno.makeTempDirSync({ prefix: "quarto-pathsequal-test" });
100+
try {
101+
const filesDir = join(dir, "index_files");
102+
const forwardSlash = filesDir.replaceAll("\\", "/");
103+
104+
assert(
105+
pathsEqual(filesDir, forwardSlash),
106+
"same path with different separators must compare equal",
107+
);
108+
assert(
109+
!pathsEqual(filesDir, join(dir, "other_files")),
110+
"different paths must not compare equal",
111+
);
112+
} finally {
113+
Deno.removeSync(dir, { recursive: true });
114+
}
115+
});
116+
93117
interface GlobTest {
94118
name: string;
95119
globs: string[];

tests/unit/render/cleanup.test.ts

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
/*
2+
* cleanup.test.ts
3+
*
4+
* Copyright (C) 2026 Posit Software, PBC
5+
*/
6+
7+
import { assert } from "testing/asserts";
8+
import { ensureDirSync, existsSync } from "../../../src/deno_ral/fs.ts";
9+
import { join } from "../../../src/deno_ral/path.ts";
10+
import { unitTest } from "../../test.ts";
11+
import { renderCleanup } from "../../../src/command/render/cleanup.ts";
12+
import { Format } from "../../../src/config/types.ts";
13+
import { createMockProjectContext } from "../project/utils.ts";
14+
15+
// Minimal ipynb format: ipynb is a self-contained (standalone) output, so the
16+
// self-contained cleanup path in render.ts passes the supporting dirs here.
17+
function ipynbFormat(): Format {
18+
return {
19+
pandoc: { to: "ipynb" },
20+
execute: {},
21+
render: {},
22+
metadata: {},
23+
language: {},
24+
} as unknown as Format;
25+
}
26+
27+
// Regression test for #14613.
28+
//
29+
// On a manuscript first render with freeze:auto, the ipynb completion triggers
30+
// the self-contained supporting cleanup. The supporting dir reported by the
31+
// engine uses forward slashes; renderCleanup normalizes the computed filesDir
32+
// with normalizePath (backslashes on Windows). The separator mismatch made the
33+
// equality check fail, so the parent index_files dir (instead of the narrowed
34+
// figure-<to> dir) was handed to safeRemoveDirSync, deleting ALL figures.
35+
// Subsequent format completions then found no index_files and copied nothing
36+
// into _manuscript.
37+
//
38+
// deno-lint-ignore require-await
39+
unitTest("renderCleanup - narrows ipynb supporting despite path separator mismatch (#14613)", async () => {
40+
const projectDir = Deno.makeTempDirSync({ prefix: "quarto-cleanup-test" });
41+
const project = createMockProjectContext({ dir: projectDir });
42+
try {
43+
const input = join(projectDir, "index.qmd");
44+
Deno.writeTextFileSync(input, "");
45+
46+
const filesDir = join(projectDir, "index_files");
47+
const figureHtmlDir = join(filesDir, "figure-html");
48+
const figureIpynbDir = join(filesDir, "figure-ipynb");
49+
ensureDirSync(figureHtmlDir);
50+
ensureDirSync(figureIpynbDir);
51+
const figureHtmlPlot = join(figureHtmlDir, "plot.png");
52+
Deno.writeTextFileSync(figureHtmlPlot, "html-figure");
53+
Deno.writeTextFileSync(join(figureIpynbDir, "plot.png"), "ipynb-figure");
54+
55+
// Engine reports the supporting dir with forward slashes (as knitr does).
56+
const supportingForwardSlash = filesDir.replaceAll("\\", "/");
57+
58+
renderCleanup(
59+
input,
60+
join(projectDir, "_manuscript", "index.out.ipynb"),
61+
ipynbFormat(),
62+
project,
63+
[supportingForwardSlash],
64+
);
65+
66+
// Cleanup must narrow to figure-ipynb only; the html figures the other
67+
// manuscript formats depend on must survive.
68+
assert(
69+
existsSync(figureHtmlPlot),
70+
"index_files/figure-html must survive ipynb supporting cleanup",
71+
);
72+
assert(
73+
!existsSync(figureIpynbDir),
74+
"index_files/figure-ipynb should be removed by narrowed cleanup",
75+
);
76+
} finally {
77+
project.cleanup();
78+
try {
79+
Deno.removeSync(projectDir, { recursive: true });
80+
} catch {
81+
// ignore
82+
}
83+
}
84+
});

0 commit comments

Comments
 (0)