Skip to content

Commit def4986

Browse files
cdervclaude
andauthored
Fix Windows dart-sass path quoting with spaces (#14002)
* Fix Windows dart-sass path quoting with spaces (#13997) Deno has a bug when executing .bat files where both the command path and arguments contain spaces. This causes dart-sass to fail when Quarto is installed in C:\Program Files\ and project paths have spaces. Use `safeWindowsExec` on Windows to write the command to a temp batch file with proper quoting, bypassing Deno's buggy command-line handling. Closes #13997 --------- Co-authored-by: Claude <[email protected]>
1 parent 53cb757 commit def4986

File tree

4 files changed

+299
-26
lines changed

4 files changed

+299
-26
lines changed

news/changelog-1.9.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,4 +164,5 @@ All changes included in 1.9:
164164
- ([#13907](https://github.com/quarto-dev/quarto-cli/issues/13907)): Ignore AI assistant configuration files (`CLAUDE.md`, `AGENTS.md`) when scanning for project input files and in extension templates, similar to how `README.md` is handled.
165165
- ([#13935](https://github.com/quarto-dev/quarto-cli/issues/13935)): Fix `quarto install`, `quarto update`, and `quarto uninstall` interactive tool selection.
166166
- ([#13992](https://github.com/quarto-dev/quarto-cli/issues/13992)): Fix crash when rendering div with both cross-reference ID and conditional visibility to PDF.
167+
- ([#13997](https://github.com/quarto-dev/quarto-cli/issues/13997)): Fix Windows dart-sass theme compilation failing when Quarto is installed in a path with spaces (e.g., `C:\Program Files\`) and the project path also contains spaces.
167168
- ([#13998](https://github.com/quarto-dev/quarto-cli/issues/13998)): Fix YAML validation error with CR-only line terminators (old Mac format). Documents using `\r` line endings no longer fail with "Expected YAML front matter to contain at least 2 lines".

src/core/dart-sass.ts

Lines changed: 63 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,14 @@ import { join } from "../deno_ral/path.ts";
77

88
import { architectureToolsPath } from "./resources.ts";
99
import { execProcess } from "./process.ts";
10+
import { ProcessResult } from "./process-types.ts";
1011
import { TempContext } from "./temp.ts";
1112
import { lines } from "./text.ts";
1213
import { debug, info } from "../deno_ral/log.ts";
1314
import { existsSync } from "../deno_ral/fs.ts";
1415
import { warnOnce } from "./log.ts";
1516
import { isWindows } from "../deno_ral/platform.ts";
17+
import { requireQuoting, safeWindowsExec } from "./windows.ts";
1618

1719
export function dartSassInstallDir() {
1820
return architectureToolsPath("dart-sass");
@@ -53,7 +55,21 @@ export async function dartCompile(
5355
return outputFilePath;
5456
}
5557

56-
export async function dartCommand(args: string[]) {
58+
/**
59+
* Options for dartCommand
60+
*/
61+
export interface DartCommandOptions {
62+
/**
63+
* Override the sass executable path.
64+
* Primarily used for testing with spaced paths.
65+
*/
66+
sassPath?: string;
67+
}
68+
69+
export async function dartCommand(
70+
args: string[],
71+
options?: DartCommandOptions,
72+
) {
5773
const resolvePath = () => {
5874
const dartOverrideCmd = Deno.env.get("QUARTO_DART_SASS");
5975
if (dartOverrideCmd) {
@@ -69,34 +85,55 @@ export async function dartCommand(args: string[]) {
6985
const command = isWindows ? "sass.bat" : "sass";
7086
return architectureToolsPath(join("dart-sass", command));
7187
};
72-
const sass = resolvePath();
88+
const sass = options?.sassPath ?? resolvePath();
7389

74-
const cmd = sass;
75-
// Run the sass compiler
76-
const result = await execProcess(
77-
{
78-
cmd,
79-
args,
80-
stdout: "piped",
81-
stderr: "piped",
82-
},
83-
);
90+
// Process result helper (shared by Windows and non-Windows paths)
91+
const processResult = (result: ProcessResult): string | undefined => {
92+
if (result.success) {
93+
if (result.stderr) {
94+
info(result.stderr);
95+
}
96+
return result.stdout;
97+
} else {
98+
debug(`[DART path] : ${sass}`);
99+
debug(`[DART args] : ${args.join(" ")}`);
100+
debug(`[DART stdout] : ${result.stdout}`);
101+
debug(`[DART stderr] : ${result.stderr}`);
84102

85-
if (result.success) {
86-
if (result.stderr) {
87-
info(result.stderr);
103+
const errLines = lines(result.stderr || "");
104+
// truncate the last 2 lines (they include a pointer to the temp file containing
105+
// all of the concatenated sass, which is more or less incomprehensible for users.
106+
const errMsg = errLines.slice(0, errLines.length - 2).join("\n");
107+
throw new Error("Theme file compilation failed:\n\n" + errMsg);
88108
}
89-
return result.stdout;
90-
} else {
91-
debug(`[DART path] : ${sass}`);
92-
debug(`[DART args] : ${args.join(" ")}`);
93-
debug(`[DART stdout] : ${result.stdout}`);
94-
debug(`[DART stderr] : ${result.stderr}`);
109+
};
95110

96-
const errLines = lines(result.stderr || "");
97-
// truncate the last 2 lines (they include a pointer to the temp file containing
98-
// all of the concatenated sass, which is more or less incomprehensible for users.
99-
const errMsg = errLines.slice(0, errLines.length - 2).join("\n");
100-
throw new Error("Theme file compilation failed:\n\n" + errMsg);
111+
// On Windows, use safeWindowsExec to handle paths with spaces
112+
// (e.g., when Quarto is installed in C:\Program Files\)
113+
// See https://github.com/quarto-dev/quarto-cli/issues/13997
114+
if (isWindows) {
115+
const quoted = requireQuoting([sass, ...args]);
116+
const result = await safeWindowsExec(
117+
quoted.args[0],
118+
quoted.args.slice(1),
119+
(cmd: string[]) => {
120+
return execProcess({
121+
cmd: cmd[0],
122+
args: cmd.slice(1),
123+
stdout: "piped",
124+
stderr: "piped",
125+
});
126+
},
127+
);
128+
return processResult(result);
101129
}
130+
131+
// Non-Windows: direct execution
132+
const result = await execProcess({
133+
cmd: sass,
134+
args,
135+
stdout: "piped",
136+
stderr: "piped",
137+
});
138+
return processResult(result);
102139
}

tests/unit/dart-sass.test.ts

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
/*
2+
* dart-sass.test.ts
3+
*
4+
* Tests for dart-sass functionality.
5+
* Validates fix for https://github.com/quarto-dev/quarto-cli/issues/13997
6+
*
7+
* Copyright (C) 2020-2025 Posit Software, PBC
8+
*/
9+
10+
import { unitTest } from "../test.ts";
11+
import { assert } from "testing/asserts";
12+
import { isWindows } from "../../src/deno_ral/platform.ts";
13+
import { join } from "../../src/deno_ral/path.ts";
14+
import { dartCommand, dartSassInstallDir } from "../../src/core/dart-sass.ts";
15+
16+
// Test that dartCommand handles spaced paths on Windows (issue #13997)
17+
// The bug only triggers when BOTH the executable path AND arguments contain spaces.
18+
unitTest(
19+
"dartCommand - handles spaced paths on Windows (issue #13997)",
20+
async () => {
21+
// Create directories with spaces for both sass and file arguments
22+
const tempBase = Deno.makeTempDirSync({ prefix: "quarto_test_" });
23+
const spacedSassDir = join(tempBase, "Program Files", "dart-sass");
24+
const spacedProjectDir = join(tempBase, "My Project");
25+
const sassInstallDir = dartSassInstallDir();
26+
27+
try {
28+
// Create directories
29+
Deno.mkdirSync(join(tempBase, "Program Files"), { recursive: true });
30+
Deno.mkdirSync(spacedProjectDir, { recursive: true });
31+
32+
// Create junction (Windows directory symlink) to actual dart-sass
33+
const junctionResult = await new Deno.Command("cmd", {
34+
args: ["/c", "mklink", "/J", spacedSassDir, sassInstallDir],
35+
}).output();
36+
37+
if (!junctionResult.success) {
38+
const stderr = new TextDecoder().decode(junctionResult.stderr);
39+
throw new Error(`Failed to create junction: ${stderr}`);
40+
}
41+
42+
// Create test SCSS file in spaced path (args with spaces)
43+
const inputScss = join(spacedProjectDir, "test style.scss");
44+
const outputCss = join(spacedProjectDir, "test style.css");
45+
Deno.writeTextFileSync(inputScss, "body { color: red; }");
46+
47+
const spacedSassPath = join(spacedSassDir, "sass.bat");
48+
49+
// This is the exact bug scenario: spaced exe path + spaced args
50+
// Without the fix, this fails with "C:\...\Program" not recognized
51+
const result = await dartCommand([inputScss, outputCss], {
52+
sassPath: spacedSassPath,
53+
});
54+
55+
// Verify compilation succeeded (no stdout expected for file-to-file compilation)
56+
assert(
57+
result === undefined || result === "",
58+
"Sass compile should succeed (no stdout for file-to-file compilation)",
59+
);
60+
assert(
61+
Deno.statSync(outputCss).isFile,
62+
"Output CSS file should be created",
63+
);
64+
} finally {
65+
// Cleanup: remove junction first (rmdir for junctions), then temp directory
66+
try {
67+
await new Deno.Command("cmd", {
68+
args: ["/c", "rmdir", spacedSassDir],
69+
}).output();
70+
await Deno.remove(tempBase, { recursive: true });
71+
} catch (e) {
72+
// Best effort cleanup - log for debugging if it fails
73+
console.debug("Test cleanup failed:", e);
74+
}
75+
}
76+
},
77+
{ ignore: !isWindows },
78+
);

tests/unit/windows-exec.test.ts

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
/*
2+
* windows-exec.test.ts
3+
*
4+
* Tests for Windows command execution utilities.
5+
* Validates fix for https://github.com/quarto-dev/quarto-cli/issues/13997
6+
*
7+
* Copyright (C) 2020-2025 Posit Software, PBC
8+
*/
9+
10+
import { unitTest } from "../test.ts";
11+
import { assert, assertEquals } from "testing/asserts";
12+
import { isWindows } from "../../src/deno_ral/platform.ts";
13+
import { join } from "../../src/deno_ral/path.ts";
14+
import { requireQuoting, safeWindowsExec } from "../../src/core/windows.ts";
15+
import { execProcess } from "../../src/core/process.ts";
16+
17+
// Test that requireQuoting correctly quotes paths with spaces
18+
unitTest(
19+
"requireQuoting - quotes paths with spaces",
20+
// deno-lint-ignore require-await
21+
async () => {
22+
const result = requireQuoting([
23+
"C:\\Program Files\\dart-sass\\sass.bat",
24+
"C:\\My Project\\style.scss",
25+
"C:\\My Project\\style.css",
26+
]);
27+
28+
assert(result.status === true, "Should indicate quoting was required");
29+
assertEquals(result.args[0], '"C:\\Program Files\\dart-sass\\sass.bat"');
30+
assertEquals(result.args[1], '"C:\\My Project\\style.scss"');
31+
assertEquals(result.args[2], '"C:\\My Project\\style.css"');
32+
},
33+
{ ignore: !isWindows },
34+
);
35+
36+
// Test that requireQuoting does not quote clean paths (no special chars)
37+
// Note: Windows paths with drive letters (C:) ARE quoted due to the colon
38+
unitTest(
39+
"requireQuoting - no quoting for clean paths",
40+
// deno-lint-ignore require-await
41+
async () => {
42+
const result = requireQuoting([
43+
"sass.bat",
44+
"input.scss",
45+
"output.css",
46+
]);
47+
48+
assert(result.status === false, "Should indicate no quoting needed");
49+
assertEquals(result.args[0], "sass.bat");
50+
assertEquals(result.args[1], "input.scss");
51+
assertEquals(result.args[2], "output.css");
52+
},
53+
{ ignore: !isWindows },
54+
);
55+
56+
// Test that safeWindowsExec passes arguments with spaces correctly
57+
unitTest(
58+
"safeWindowsExec - passes spaced args correctly",
59+
async () => {
60+
const tempDir = Deno.makeTempDirSync({ prefix: "quarto-test" });
61+
62+
try {
63+
// Create batch file that echoes args (use %~1 to strip quotes)
64+
const echoArgs = join(tempDir, "echo-args.bat");
65+
Deno.writeTextFileSync(
66+
echoArgs,
67+
`@echo off
68+
echo ARG1: %~1
69+
echo ARG2: %~2
70+
`,
71+
);
72+
73+
const spaced1 = "C:\\My Project\\input.scss";
74+
const spaced2 = "C:\\My Project\\output.css";
75+
const quoted = requireQuoting([echoArgs, spaced1, spaced2]);
76+
77+
const result = await safeWindowsExec(
78+
quoted.args[0],
79+
quoted.args.slice(1),
80+
(cmd) =>
81+
execProcess({
82+
cmd: cmd[0],
83+
args: cmd.slice(1),
84+
stdout: "piped",
85+
stderr: "piped",
86+
}),
87+
);
88+
89+
assert(result.success, "Should execute successfully");
90+
assert(
91+
result.stdout?.includes(spaced1),
92+
`Arg1 should be passed correctly. Got: ${result.stdout}`,
93+
);
94+
assert(
95+
result.stdout?.includes(spaced2),
96+
`Arg2 should be passed correctly. Got: ${result.stdout}`,
97+
);
98+
} finally {
99+
Deno.removeSync(tempDir, { recursive: true });
100+
}
101+
},
102+
{ ignore: !isWindows },
103+
);
104+
105+
// Test that safeWindowsExec handles program paths with spaces (issue #13997)
106+
// This is the core bug: Deno has issues executing .bat files when both the
107+
// command path AND arguments contain spaces.
108+
unitTest(
109+
"safeWindowsExec - handles program path with spaces (issue #13997)",
110+
async () => {
111+
const tempDir = Deno.makeTempDirSync({ prefix: "quarto-test" });
112+
113+
try {
114+
// Create directory with spaces (simulates C:\Program Files\)
115+
const spacedDir = join(tempDir, "Program Files", "tool");
116+
Deno.mkdirSync(spacedDir, { recursive: true });
117+
118+
// Create batch file in spaced path
119+
const program = join(spacedDir, "echo-success.bat");
120+
Deno.writeTextFileSync(
121+
program,
122+
`@echo off
123+
echo SUCCESS
124+
echo ARG: %~1
125+
`,
126+
);
127+
128+
const spacedArg = "C:\\My Project\\file.txt";
129+
const quoted = requireQuoting([program, spacedArg]);
130+
131+
const result = await safeWindowsExec(
132+
quoted.args[0],
133+
quoted.args.slice(1),
134+
(cmd) =>
135+
execProcess({
136+
cmd: cmd[0],
137+
args: cmd.slice(1),
138+
stdout: "piped",
139+
stderr: "piped",
140+
}),
141+
);
142+
143+
assert(result.success, "Should execute program in spaced path");
144+
assert(
145+
result.stdout?.includes("SUCCESS"),
146+
`Program should run. Got: ${result.stdout}`,
147+
);
148+
assert(
149+
result.stdout?.includes(spacedArg),
150+
`Arg should be passed correctly. Got: ${result.stdout}`,
151+
);
152+
} finally {
153+
Deno.removeSync(tempDir, { recursive: true });
154+
}
155+
},
156+
{ ignore: !isWindows },
157+
);

0 commit comments

Comments
 (0)