Wrong figure folder built using custom format due to mutation

[cderv]

Here is the issue with traceback

[smoke] > quarto render docs\smoke-all\2022\09\30\custom-writer-emulation\customwriter-yaml.qmd --to customformat.lua => ./test.ts:126:72
error: AssertionError: Failed assertion:

--------------------------------------------------------------------------------
[smoke] > quarto render docs\smoke-all\2022\09\30\custom-writer-emulation\customwriter-yaml.qmd --to customformat.lua
          run-tests.psl ..\..\..\..\..\..\C:\Users\chris\Documents\DEV_R\quarto-cli\tests\smoke\smoke-all.test.ts

[verify] > unknown

La syntaxe du nom de fichier, de répertoire ou de volume est incorrecte. (os error 123), stat 'docs\smoke-all\2022\09\30\custom-writer-emulation\customwriter-yaml_files\figure-C:\Users\chris\Documents\DEV_R\quarto'
Error: La syntaxe du nom de fichier, de répertoire ou de volume est incorrecte. (os error 123), stat 'docs\smoke-all\2022\09\30\custom-writer-emulation\customwriter-yaml_files\figure-C:\Users\chris\Documents\DEV_R\quarto'
    at Object.lstatSync (deno:runtime/js/30_fs.js:305:9)
    at existsSync (file:///C:/Users/chris/Documents/DEV_R/quarto-cli/src/vendor/deno.land/std@0.166.0/fs/exists.ts:75:10)
    at removeIfEmptyDir (file:///C:/Users/chris/Documents/DEV_R/quarto-cli/src/core/path.ts:47:7)
    at renderCleanup (file:///C:/Users/chris/Documents/DEV_R/quarto-cli/src/command/render/cleanup.ts:101:3)
    at file:///C:/Users/chris/Documents/DEV_R/quarto-cli/src/command/render/render.ts:293:9
    at withTiming (file:///C:/Users/chris/Documents/DEV_R/quarto-cli/src/core/timing.ts:37:20)
    at Object.complete (file:///C:/Users/chris/Documents/DEV_R/quarto-cli/src/command/render/render.ts:292:7)
    at async Object.onPostProcess (file:///C:/Users/chris/Documents/DEV_R/quarto-cli/src/command/render/render-files.ts:568:28)
    at async renderFiles (file:///C:/Users/chris/Documents/DEV_R/quarto-cli/src/command/render/render-files.ts:524:7)
    at async render (file:///C:/Users/chris/Documents/DEV_R/quarto-cli/src/command/render/render-shared.ts:101:18)

We are building wrong path above docs\smoke-all\2022\09\30\custom-writer-emulation\customwriter-yaml_files\figure-C:\Users\chris\Documents\DEV_R\quarto by appending to path not correctly.

The issue is from

quarto-cli/src/command/render/cleanup.ts

Lines 33 to 36 in 104d39d

	// compute figure format
	const figureFormat = isLatexOutput(format.pandoc)
	? extname(output).slice(1)
	: format.pandoc.to;

as format.pandoc.to is a path. I have this in debug mode console

> figureFormat
'C:\Users\chris\Documents\DEV_R\quarto-cli\src\resources\filters\customwriter\customwriter.lua'

This leads to wrong figure directory created

quarto-cli/src/command/render/cleanup.ts

Lines 96 to 97 in 104d39d

	const filesDir = join(dirname(input), inputFilesDir(input));
	const figsDir = join(filesDir, figuresDir(figureFormat));

Maybe the issue is elsewhere though but here the empty directory are deleted even on Linux (no error on Linux , only windows)

[cderv]

FWIW this is what I get locally as directory structure

> tree .\docs\smoke-all\2022\09\30\custom-writer-emulation\
C:\USERS\CHRIS\DOCUMENTS\DEV_R\QUARTO-CLI\TESTS\DOCS\SMOKE-ALL\2022\09\30\CUSTOM-WRITER-EMULATION
└───customwriter-yaml.customformat_files
    └───figure-customformat.lua

[cderv]

Just more information, it may not be what it seems to because by adding some R chunk content in the qmd file, this will not be a absolute file anymore

• Without R chunks in the doc

> figureFormat
'C:\Users\chris\Documents\DEV_R\quarto-cli\src\resources\filters\customwriter\customwriter.lua'

• With R chunks in the doc

> figureFormat
'customformat.lua'

Something else create this issue it seems 🤔

[cderv]

I finally understand why we have a different behavior here. The fact that the test use a customformat.lua which is same name as our internal resource does not help. Below is the detailed analysis I have done to scope the root problem

Why do we have this figureFormat value ?

When using a custom format, it will end up with .lua extension - this will be handle in renderPandoc() here

quarto-cli/src/command/render/pandoc.ts

Lines 706 to 709 in 128c9b0

	if (allDefaults.to?.match(/[.]lua$/)) {
	formatFilterParams["custom-writer"] = allDefaults.to;
	allDefaults.to = resourcePath("filters/customwriter/customwriter.lua");
	}

We will modify the allDefault.to value to be the one from our internal lua filter, and pass the initial to format as a custom-writer parameter.

However, allDefault is not a deep clone by default of the initial options

quarto-cli/src/command/render/pandoc.ts

Line 266 in 128c9b0

let allDefaults = (await generateDefaults(options)) || {};

This means that when allDefault.to is modified, options.format.pandoc.to is too. Mutation will be also in PandocOptions

quarto-cli/src/command/render/render.ts

Line 170 in 128c9b0

const pandocResult = await runPandoc(pandocOptions, executeResult.filters);

which will modified the format in there

quarto-cli/src/command/render/render.ts

Lines 146 to 162 in 128c9b0

	const pandocOptions: PandocOptions = {
	markdown: notebookResult.markdown
	? notebookResult.markdown
	: executeResult.markdown,
	source: context.target.source,
	output: recipe.output,
	keepYaml: recipe.keepYaml,
	mediabagDir,
	libDir: context.libDir,
	format,
	project: context.project,
	args: recipe.args,
	services: context.options.services,
	metadata: executeResult.metadata,
	quiet,
	flags: context.options.flags,
	};

This format is the same as the one used in renderCleanup()

quarto-cli/src/command/render/render.ts

Lines 292 to 299 in 128c9b0

	withTiming("render-cleanup", () =>
	renderCleanup(
	context.target.input,
	finalOutput!,
	format,
	selfContained! ? supporting : undefined,
	executionEngineKeepMd(context.target.input),
	));

And this lead to our initial issue and traceback. The format.pandoc pass in here

quarto-cli/src/command/render/cleanup.ts

Lines 33 to 36 in 104d39d

	// compute figure format
	const figureFormat = isLatexOutput(format.pandoc)
	? extname(output).slice(1)
	: format.pandoc.to;

is not the original format we expect. It is our internal lua filter and thus

> figureFormat
'C:\Users\chris\Documents\DEV_R\quarto-cli\src\resources\filters\customwriter\customwriter.lua'

Why do we have a difference when we include a R chunk in the qmd file ?

This is because using rmarkdown will add a filter rmarkdown/pagebreak.lua file) stored in sysFilters which triggers this condition

quarto-cli/src/command/render/pandoc.ts

Lines 327 to 330 in 128c9b0

	if (
	sysFilters.length > 0 || options.format.formatExtras ||
	options.project?.formatExtras
	) {

Going into this conditional, we will resolve some Extras

quarto-cli/src/command/render/pandoc.ts

Lines 354 to 363 in 128c9b0

	const extras = await resolveExtras(
	projectExtras,
	formatExtras,
	options.format,
	cwd,
	options.libDir,
	options.services.temp,
	dependenciesFile,
	options.project,
	);

we will then have a extras.pandoc which will trigger this part

quarto-cli/src/command/render/pandoc.ts

Line 428 in 128c9b0

if (extras.pandoc) {

The extra is

{
  "highlight-style": "C:\\Users\\chris\\Documents\\DEV_R\\quarto-cli\\src\\resources\\pandoc\\highlight-styles\\arrow-light.theme",
}

And this conditional will modify AllDefaults

quarto-cli/src/command/render/pandoc.ts

Line 447 in 128c9b0

allDefaults = mergeConfigs(extras.pandoc, allDefaults);

which use a cloneDeep() - making AllDefault a deep clone of options, which is was not before

quarto-cli/src/core/config.ts

Lines 10 to 20 in 128c9b0

	export function mergeConfigs<T>(config: T, ...configs: Array<unknown>): T {
	// copy all configs so we don't mutate them
	config = ld.cloneDeep(config);
	configs = ld.cloneDeep(configs);
	return ld.mergeWith(
	config,
	...configs,
	mergeArrayCustomizer,
	);
	}

This is why later when AllDefaults.to is modified for the custom writer, our original options.format.pandoc.to is not modified. Hence the difference for

> figureFormat
'customformat.lua'

which is our original YAML output format.

Conclusion at this point

Several way to see this

• Should we have the extras.pandoc that triggers the cloneDeep ?
• Should we make a deep clone earlier to avoid mutating our original options ?
• Even with this, are we correctly handling format and creating paths based on it ?

I believe this is part of the issue. Even when the format is correctly path to cleanup function, we'll compute a figDir here

quarto-cli/src/command/render/cleanup.ts

Line 98 in b6d4034

const libDir = join(dirname(input), filesDirLibDir(input));

that will be

figsDir
'docs\smoke-all\2022\09\30\custom-writer-emulation\customwriter-yaml_files\figure-customformat.lua'

Adding figure creation in the test qmd file, we can see that the the figure dir is in another folder containing format in the name

>  tree /F .\docs\smoke-all\2022\09\30\custom-writer-emulation\
C:\USERS\CHRIS\DOCUMENTS\DEV_R\QUARTO-CLI\TESTS\DOCS\SMOKE-ALL\2022\09\30\CUSTOM-WRITER-EMULATION
│   createcustomnode.lua
│   customformat.lua
│   customwriter-yaml.qmd
│   customwriter-yaml.txt
│
├───customwriter-yaml.customformat_files
│   └───figure-customformat.lua
│           cars-1.png
│
└───customwriter-yaml_files
    └───mediabag

I haven't look through this at this point. It seems there is multiple issue behind all this.

Hope it helps. Sorry for long and detail explanation, I am also using this thread as notes for myself.

[cderv]

Adding figure creation in the test qmd file, we can see that the the figure dir is in another folder containing format in the name

And so now I have it all covered. Probably another issue but related.

The directory with format in the name is created by rmarkdown when R chunks are added. I don't think Quarto expect this so not sure we are cleaning this correctly.

When no output_file is passed to rmarkdown::render(), one is automatically determined considering the pandoc.to format.

# determine the output file for a pandoc conversion
pandoc_output_file <- function(input, pandoc_options) {
  to <- strsplit(pandoc_options$to, "[+-]")[[1]][[1]]
  ext <- pandoc_output_ext(pandoc_options$ext, to, input)
  output <- paste0(xfun::sans_ext(input), ext)
  basename(output)
}

With to being customwriter.lua, the output file will have a .customwriter suffix as we can see in above comments

Then the files where to store figures for knitr is computed based on output_file

files_dir_slash <- file.path(output_dir, knitr_files_dir(basename(output_file)))

which will lead to the other folder customwriter-yaml.customformat_files when R code are into this.

I believe this will happen with any format no known by rmarkdown function pandoc_output_ext

pandoc_output_ext <- function(ext, to, input) {
  if (!is.null(ext)) return(ext)
  if (to %in% c("latex", "beamer")) return(".pdf")
  if (to %in% c("html", "html4", "html5", "s5", "slidy", "slideous", "dzslides", "revealjs"))
    return(".html")
  if (to == "markdown" && tolower(xfun::file_ext(input)) != "md") return(".md")
  paste0(".", to)
}

I'll probably look into that in another issue too, but that explains the behavior and differences seen above