Skip to content

Commit 7892fd3

Browse files
Merge branch 'quarto-dev:main' into fix-11189
2 parents b7d491c + 9e01d1c commit 7892fd3

File tree

26 files changed

+1296
-1006
lines changed

26 files changed

+1296
-1006
lines changed

.github/pull_request_template.md

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,4 +24,7 @@ I have (if applicable):
2424

2525
- [ ] filed a [contributor agreement](https://github.com/quarto-dev/quarto-cli/blob/main/CONTRIBUTING.md).
2626
- [ ] referenced the GitHub issue this PR closes
27-
- [ ] updated the appropriate changelog
27+
- [ ] updated the appropriate changelog in the PR
28+
- [ ] ensured the present test suite passes
29+
- [ ] added new tests
30+
- [ ] created a separate documentation PR in [Quarto's website repo](https://github.com/quarto-dev/quarto-web/) and linked it to this PR

news/changelog-1.6.md

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ All changes included in 1.6:
22

33
## Breaking changes
44

5-
- The syntax for standard library imports in `quarto run` TypeScript files (`*.ts`) changed. Please see https://prerelease.quarto.org/docs/projects/scripts.html#deno-scripts for how to make the necessary changes.
5+
- The syntax for standard library imports in `quarto run` TypeScript files (`*.ts`) changed. Please see <https://prerelease.quarto.org/docs/projects/scripts.html#deno-scripts> for how to make the necessary changes.
66

77
## Shortcodes
88

@@ -16,6 +16,7 @@ All changes included in 1.6:
1616
## Lua Filters and extensions
1717

1818
- ([#8179](https://github.com/quarto-dev/quarto-cli/issues/8179)): When merging code cells for complex layouts, do not merge cells with different languages.
19+
- ([#8428](https://github.com/quarto-dev/quarto-cli/issues/8428)): only forward cell labels to tables when tables will be cross-referenceable.
1920
- ([#10004](https://github.com/quarto-dev/quarto-cli/issues/10004)): Resolve callout titles, theorem names, and `code-summary` content through `quarto_ast_pipeline()` and `process_shortcodes()`.
2021
- ([#10196](https://github.com/quarto-dev/quarto-cli/issues/10196)): Protect against nil values in `float.caption_long`.
2122
- ([#10328](https://github.com/quarto-dev/quarto-cli/issues/10328)): Interpret subcells as subfloats when subcap count matches subcell count.
@@ -77,6 +78,7 @@ All changes included in 1.6:
7778
## Projects
7879

7980
- ([#10268](https://github.com/quarto-dev/quarto-cli/issues/10268)): `quarto create` supports opening project in Positron, in addition to VS Code and RStudio IDE.
81+
- ([#10285](https://github.com/quarto-dev/quarto-cli/issues/10285)): Include text from before the first chapter sections in search indices. In addition, include text of every element with `.quarto-include-in-search-index` class in search indices.
8082
- ([#10566](https://github.com/quarto-dev/quarto-cli/issues/10566)): Ensure that `quarto run` outputs `stdout` and `stderr` to the correct streams.
8183

8284
### Websites
@@ -105,6 +107,12 @@ All changes included in 1.6:
105107

106108
- ([#9134](https://github.com/quarto-dev/quarto-cli/issues/9134)): Add proper fix for `multiprocessing` in notebooks with the Python kernel.
107109

110+
## Chromium support
111+
112+
- ([#11135](https://github.com/quarto-dev/quarto-cli/issues/11135)): Use `--headless=old` mode for Chromium to avoid recent issues with the new `--headless` mode. Setting `--headless=new` can be configured with `QUARTO_CHROMIUM_HEADLESS_MODE=new` environment variable, however it is not recommended new headless mode seems to be unstable. Only use to be unblocked of a situation (like `QUARTO_CHROMIUM_HEADLESS_MODE="none"` if you use an old chrome version somehow that don't support `--headless=old`).
113+
- ([#10170](https://github.com/quarto-dev/quarto-cli/issues/10170)): Quarto should find chrome executable automatically on most OS. If this is does not find it, or a specific version is needed, set `QUARTO_CHROMIUM` environment variable to the executable path.
114+
- Quarto now makes sure that all started chromium instances are closed when the process ends, no matter how it ends (success, error, or interruption).
115+
108116
## Other Fixes and Improvements
109117

110118
- Upgrade `mermaidjs` to 11.2.0.

src/core/cri/cri.ts

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,13 @@ import { findOpenPort } from "../port.ts";
1414
import { getNamedLifetime, ObjectWithLifetime } from "../lifetimes.ts";
1515
import { sleep } from "../async.ts";
1616
import { InternalError } from "../lib/error.ts";
17+
import { getenv } from "../env.ts";
1718
import { kRenderFileLifetime } from "../../config/constants.ts";
19+
import { debug } from "../../deno_ral/log.ts";
20+
import {
21+
registerForExitCleanup,
22+
unregisterForExitCleanup,
23+
} from "../process.ts";
1824

1925
async function waitForServer(port: number, timeout = 3000) {
2026
const interval = 50;
@@ -78,20 +84,36 @@ export async function criClient(appPath?: string, port?: number) {
7884
}
7985
const app: string = appPath || await getBrowserExecutablePath();
8086

87+
// Allow to adapt the headless mode depending on the Chrome version
88+
const headlessMode = getenv("QUARTO_CHROMIUM_HEADLESS_MODE", "old");
89+
8190
const cmd = [
8291
app,
83-
"--headless",
92+
// TODO: Chrome v128 changed the default from --headless=old to --headless=new
93+
// in 2024-08. Old headless mode was effectively a separate browser render,
94+
// and while more performant did not share the same browser implementation as
95+
// headful Chrome. New headless mode will likely be useful to some, but in Quarto use cases
96+
// like printing to PDF or screenshoting, we need more work to
97+
// move to the new mode. We'll use `--headless=old` as the default for now
98+
// until the new mode is more stable, or until we really pin a version as default to be used.
99+
// This is also impacting in chromote and pagedown R packages and we could keep syncing with them.
100+
`--headless${headlessMode == "none" ? "" : "=" + headlessMode}`,
84101
"--no-sandbox",
85102
"--disable-gpu",
86103
"--renderer-process-limit=1",
87104
`--remote-debugging-port=${port}`,
88105
];
89106
const browser = Deno.run({ cmd, stdout: "piped", stderr: "piped" });
90107

108+
// Register for cleanup inside exitWithCleanup() in case something goes wrong
109+
const thisProcessId = registerForExitCleanup(browser);
110+
91111
if (!(await waitForServer(port as number))) {
92112
let msg = "Couldn't find open server.";
93113
// Printing more error information if chrome process errored
94114
if (!(await browser.status()).success) {
115+
debug(`[CHROMIUM path] : ${app}`);
116+
debug(`[CHROMIUM cmd] : ${cmd}`);
95117
const rawError = await browser.stderrOutput();
96118
const errorString = new TextDecoder().decode(rawError);
97119
msg = msg + "\n" + `Chrome process error: ${errorString}`;
@@ -106,7 +128,13 @@ export async function criClient(appPath?: string, port?: number) {
106128
const result = {
107129
close: async () => {
108130
await client.close();
109-
browser.close();
131+
// FIXME: 2024-10
132+
// We have a bug where `client.close()` doesn't return properly and we don't go below
133+
// meaning the `browser` process is not killed here, and it will be handled in exitWithCleanup().
134+
135+
browser.kill(); // Chromium headless won't terminate on its own, so we need to send kill signal
136+
browser.close(); // Closing the browser Deno process
137+
unregisterForExitCleanup(thisProcessId); // All went well so not need to cleanup on quarto exit
110138
},
111139

112140
rawClient: () => client,

src/core/process.ts

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,16 @@ const processList = new Map<number, Deno.Process>();
1414
let processCount = 0;
1515
let cleanupRegistered = false;
1616

17+
export function registerForExitCleanup(process: Deno.Process) {
18+
const thisProcessId = ++processCount; // don't risk repeated PIDs
19+
processList.set(thisProcessId, process);
20+
return thisProcessId;
21+
}
22+
23+
export function unregisterForExitCleanup(processId: number) {
24+
processList.delete(processId);
25+
}
26+
1727
function ensureCleanup() {
1828
if (!cleanupRegistered) {
1929
cleanupRegistered = true;
@@ -61,12 +71,11 @@ export async function execProcess(
6171
stdout: typeof (options.stdout) === "number" ? options.stdout : "piped",
6272
stderr: typeof (options.stderr) === "number" ? options.stderr : "piped",
6373
});
64-
const thisProcessId = ++processCount; // don't risk repeated PIDs
65-
processList.set(thisProcessId, process);
74+
const thisProcessId = registerForExitCleanup(process);
6675

6776
if (stdin !== undefined) {
6877
if (!process.stdin) {
69-
processList.delete(processCount);
78+
unregisterForExitCleanup(thisProcessId);
7079
throw new Error("Process stdin not available");
7180
}
7281
// write in 4k chunks (deno observed to overflow at > 64k)
@@ -170,7 +179,7 @@ export async function execProcess(
170179
// close the process
171180
process.close();
172181

173-
processList.delete(thisProcessId);
182+
unregisterForExitCleanup(thisProcessId);
174183

175184
debug(`[execProcess] Success: ${status.success}, code: ${status.code}`);
176185

src/core/puppeteer.ts

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
import { readRegistryKey } from "./windows.ts";
88
import { safeExistsSync, which } from "./path.ts";
9-
import { error, info } from "../deno_ral/log.ts";
9+
import { debug, error, info } from "../deno_ral/log.ts";
1010
import { existsSync } from "../deno_ral/fs.ts";
1111
import { UnreachableError } from "./lib/error.ts";
1212
import { quartoDataDir } from "./appdirs.ts";
@@ -201,12 +201,26 @@ export async function withHeadlessBrowser<T>(
201201

202202
async function findChrome(): Promise<string | undefined> {
203203
let path;
204+
// First check env var and use this path if specified
205+
const envPath = Deno.env.get("QUARTO_CHROMIUM");
206+
if (envPath) {
207+
debug("[CHROMIUM] Using path specified in QUARTO_CHROMIUM");
208+
if (safeExistsSync(envPath)) {
209+
debug(`[CHROMIUM] Found at ${envPath}, and will be used.`);
210+
return envPath;
211+
} else {
212+
debug(
213+
`[CHROMIUM] Not found at ${envPath}. Check your environment variable valye. Searching now for another binary.`,
214+
);
215+
}
216+
}
217+
// Otherwise, try to find the path based on OS.
204218
if (Deno.build.os === "darwin") {
205219
const programs = [
206220
"/Applications/Google Chrome.app/Contents/MacOS/Google Chrome",
207221
"/Applications/Microsoft Edge.app/Contents/MacOS/Microsoft Edge",
208222
];
209-
return programs.find(safeExistsSync);
223+
path = programs.find(safeExistsSync);
210224
} else if (Deno.build.os === "windows") {
211225
// Try the HKLM key
212226
const programs = ["chrome.exe", "msedge.exe"];
@@ -216,7 +230,7 @@ async function findChrome(): Promise<string | undefined> {
216230
programs[i],
217231
"(Default)",
218232
);
219-
if (path && existsSync(path)) break;
233+
if (path && safeExistsSync(path)) break;
220234
}
221235

222236
// Try the HKCR key
@@ -240,6 +254,12 @@ async function findChrome(): Promise<string | undefined> {
240254
path = await which("chromium-browser");
241255
}
242256
}
257+
if (path) {
258+
debug("[CHROMIUM] Found Chromium on OS known location");
259+
debug(`[CHROMIUM] Path: ${path}`);
260+
} else {
261+
debug("[CHROMIUM] Chromium not found on OS known location");
262+
}
243263
return path;
244264
}
245265

src/project/types/website/website-search.ts

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -332,19 +332,31 @@ export async function updateSearchIndex(
332332

333333
// If there are any paragraphs residing outside a section, just
334334
// include that in the document entry
335-
const pararaphNodes = doc.querySelectorAll(
336-
`${mainSelector} > p, ${mainSelector} > div.cell`,
335+
const paragraphNodes = doc.querySelectorAll(
336+
`${mainSelector} > p, ${mainSelector} > div.cell, main > p`,
337337
);
338338

339-
for (const paragraphNode of pararaphNodes) {
339+
const addToIndex = (paragraphNode: Element) => {
340340
const text = paragraphNode.textContent.trim();
341341
if (text) {
342342
pageText.push(text);
343343
}
344344

345345
// Since these are already indexed with the main entry, remove them
346346
// so they are not indexed again
347-
(paragraphNode as Element).remove();
347+
paragraphNode.remove();
348+
};
349+
350+
for (const paragraphNode of paragraphNodes) {
351+
addToIndex(paragraphNode as Element);
352+
}
353+
354+
// Add forced inclusions to the index
355+
const forcedInclusions = doc.querySelectorAll(
356+
`.quarto-include-in-search-index`,
357+
);
358+
for (const forcedInclusion of forcedInclusions) {
359+
addToIndex(forcedInclusion as Element);
348360
}
349361

350362
if (pageText.length > 0) {

src/resources/filters/normalize/flags.lua

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ function compute_flags()
106106

107107
-- FIXME: are we actually triggering this with FloatRefTargets?
108108
-- table captions
109+
local kTblCap = "tbl-cap"
109110
local tblCap = extractTblCapAttrib(node,kTblCap)
110111
if hasTableRef(node) or tblCap then
111112
flags.has_table_captions = true

src/resources/filters/quarto-pre/table-captions.lua

Lines changed: 51 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -3,66 +3,64 @@
33

44
local patterns = require("modules/patterns")
55

6-
kTblCap = "tbl-cap"
7-
kTblSubCap = "tbl-subcap"
8-
9-
function table_captions()
10-
return {
6+
function table_captions()
7+
local kTblCap = "tbl-cap"
8+
local kTblSubCap = "tbl-subcap"
9+
return {
1110
Div = function(el)
1211
if tcontains(el.attr.classes, "cell") then
1312
-- extract table attributes
14-
local tblCap = extractTblCapAttrib(el,kTblCap)
13+
local tblCap = extractTblCapAttrib(el, kTblCap)
1514
local tblSubCap = extractTblCapAttrib(el, kTblSubCap, true)
16-
if hasTableRef(el) or tblCap then
17-
local tables = countTables(el)
18-
if tables > 0 then
19-
20-
-- apply captions and labels if we have a tbl-cap or tbl-subcap
21-
if tblCap or tblSubCap then
22-
23-
-- special case: knitr::kable will generate a \begin{tablular} without
24-
-- a \begin{table} wrapper -- put the wrapper in here if need be
25-
if _quarto.format.isLatexOutput() then
26-
el = _quarto.ast.walk(el, {
27-
RawBlock = function(raw)
28-
if _quarto.format.isRawLatex(raw) then
29-
local tabular_match = _quarto.modules.patterns.match_all_in_table(_quarto.patterns.latexTabularPattern)
30-
local table_match = _quarto.modules.patterns.match_all_in_table(_quarto.patterns.latexTablePattern)
31-
if tabular_match(raw.text) and not table_match(raw.text) then
32-
raw.text = raw.text:gsub(
33-
_quarto.modules.patterns.combine_patterns(_quarto.patterns.latexTabularPattern),
34-
"\\begin{table}\n\\centering\n%1%2%3\n\\end{table}\n",
35-
1)
36-
return raw
37-
end
38-
end
39-
end
40-
})
41-
end
42-
43-
-- compute all captions and labels
44-
local label = el.attr.identifier
45-
local mainCaption, tblCaptions, mainLabel, tblLabels = table_captionsAndLabels(
46-
label,
47-
tables,
48-
tblCap,
49-
tblSubCap
50-
)
51-
-- apply captions and label
52-
el.attr.identifier = mainLabel
53-
if mainCaption then
54-
el.content:insert(pandoc.Para(mainCaption))
55-
end
56-
if #tblCaptions > 0 then
57-
el = applyTableCaptions(el, tblCaptions, tblLabels)
15+
if not (tblCap or hasTableRef(el)) then
16+
return
17+
end
18+
if not (tblCap or tblSubCap) then
19+
return
20+
end
21+
local tables = countTables(el)
22+
if tables <= 0 then
23+
return
24+
end
25+
26+
-- special case: knitr::kable will generate a \begin{tabular} without
27+
-- a \begin{table} wrapper -- put the wrapper in here if need be
28+
if _quarto.format.isLatexOutput() then
29+
el = _quarto.ast.walk(el, {
30+
RawBlock = function(raw)
31+
if _quarto.format.isRawLatex(raw) then
32+
local tabular_match = _quarto.modules.patterns.match_all_in_table(_quarto.patterns.latexTabularPattern)
33+
local table_match = _quarto.modules.patterns.match_all_in_table(_quarto.patterns.latexTablePattern)
34+
if tabular_match(raw.text) and not table_match(raw.text) then
35+
raw.text = raw.text:gsub(
36+
_quarto.modules.patterns.combine_patterns(_quarto.patterns.latexTabularPattern),
37+
"\\begin{table}\n\\centering\n%1%2%3\n\\end{table}\n",
38+
1)
39+
return raw
40+
end
5841
end
59-
return el
6042
end
61-
end
43+
})
44+
end
45+
46+
-- compute all captions and labels
47+
local label = el.attr.identifier
48+
local mainCaption, tblCaptions, mainLabel, tblLabels = table_captionsAndLabels(
49+
label,
50+
tables,
51+
tblCap,
52+
tblSubCap
53+
)
54+
-- apply captions and label
55+
el.attr.identifier = mainLabel
56+
if mainCaption then
57+
el.content:insert(pandoc.Para(mainCaption))
58+
end
59+
if #tblCaptions > 0 then
60+
el = applyTableCaptions(el, tblCaptions, tblLabels)
6261
end
62+
return el
6363
end
64-
65-
6664
end
6765
}
6866

@@ -138,7 +136,7 @@ function applyTableCaptions(el, tblCaptions, tblLabels)
138136
cap:extend(tblCaptions[idx])
139137
cap:insert(pandoc.Space())
140138
end
141-
if #tblLabels[idx] > 0 then
139+
if #tblLabels[idx] > 0 and tblLabels[idx]:match("^tbl%-") then
142140
cap:insert(pandoc.Str("{#" .. tblLabels[idx] .. "}"))
143141
end
144142
idx = idx + 1

0 commit comments

Comments
 (0)