Bring support for rendering from R file

[cderv]

This closes #6660

It does the same logic as for Jupyter in #6700

Though proposal here is to use knitr::spin() directly, which has support for .qmd since v1.44.

There is question about performance, but we could easily improve by implementing the knitr::spin() logic internally in TypeScript. I'll suggest we wait for feedback on usage with calling R directly.

It works ok though little thing I noticed : It seems we run the asMappedString() from a input file at two places

Inside target() in our call

quarto-cli/src/execute/engine.ts

Line 214 in 1a8c1af

const target = await engine.target(file, flags?.quiet, markdown, project);

where we run

quarto-cli/src/execute/rmd.ts

Line 84 in 74391fc

markdown = mappedStringFromFile(file);

and then a second time when rendering project as we do run

quarto-cli/src/project/project-config.ts

Line 97 in 1a8c1af

export async function partitionedMarkdownForInput(

to find resources and this calls

quarto-cli/src/project/project-config.ts

Line 110 in 74391fc

return await engine.partitionedMarkdown(inputPath);

which needs to get markdown from input file too.

I am not sure why we need to run all this twice instead of using a stored version from the first run. But I am probably missing something.

Should we try caching the results of first spin ? Or that could cause issue ?

On other topic, I'll add a documentation file for the website. Let's note that #+ is not specifically needed and YAML syntax can directly be used this way.

#| echo: false
#| fig-with: 7
#  This is a normal R comment.
plot(cars)

This makes it quite coherent with percent script syntax I believe. I'll probably document only this way on Quarto Web, but the #+ still works.

Thanks

[jjallaire]

Is there something more we could look for? (as I think this would trigger for R code that uses any type of roxygen comment e.g. a package source file or plumber file)

[cderv]

Oh indeed. I did not thought package files would be found while rendering with quarto but probably better to be safe here.

There is no specific spin syntax except #+ for chunk options but I believe we prefer to use yaml.

Probably best is requiring a yaml header in the file. First like would need to be #´ ---. This would require a user to add one.

Otherwise we could require a specific fields for quarto. # /* is a special comment that spin will ignore and won't affect output.
So something like

# /* quarto */

but this is quite new and different syntax.

Maybe requiring yaml header is the simplest here and not too much to ask.

[jjallaire]

Yes, I think the YAML header is the thing to do

…

On Fri, Nov 24, 2023 at 3:33 PM Christophe Dervieux < ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/execute/rmd.ts <#7696 (comment)> : > @@ -353,3 +371,35 @@ function resolveInlineExecute(code: string) { (expr) => `${"`"}r .QuartoInlineRender(${expr})${"`"}`, )(code); } + +export function isKnitrSpinScript(file: string) { + const ext = extname(file).toLowerCase(); + if (ext == ".r") { + const text = Deno.readTextFileSync(file); + // Consider a .R script that can be spinned if first line starts with a special `#'` comment + return /^\s*#'/.test(text); Oh indeed. I did not thought package files would be found while rendering with quarto but probably better to be safe here. There is no specific spin syntax except #+ for chunk options but I believe we prefer to use yaml. Probably best is requiring a yaml header in the file. First like would need to be #´ ---. This would require a user to add one. Otherwise we could require a specific fields for quarto. # /* is a special comment that spin will ignore and won't affect output. So something like # /* quarto */ but this is quite new and different syntax. Maybe requiring yaml header is the simplest here and not too much to ask. — Reply to this email directly, view it on GitHub <#7696 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAZPRY23DJ37CGPAOG5ARDYGEAARAVCNFSM6AAAAAA7ZP2S4OVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTONBYGQYDSNBQG4> . You are receiving this because your review was requested.Message ID: ***@***.***>

[cderv]

Ok. I agree too. That is better.

About the two passes we do for the markdown content import which leads to two runs of spin(), do you have insights ?

I am thinking of caching first spin results in temp folder but not sure this is robust. It feels unneeded but I may be missing something.
I did not check if we do two percent script import too in this case.

[jjallaire]

I wouldn't try to untangle why that is called twice or do any caching. I actually tried to do this already for Jupyter and I broke something I didn't understand related to includes. I think the better solution is to just write a typescript version that is cheaper (don't need to do this for this PR but perhaps a follow-up PR?)

[cscheid]

I wouldn't try to untangle why that is called twice or do any caching. I actually tried to do this already for Jupyter and I broke something I didn't understand related to includes.

Let's try to fix that before 1.4 is in - I'll open an issue.

[kylebutts]

So exciting y'all. Made a little demo video here: https://x.com/kylefbutts/status/1729145164881674539?s=20

[jjallaire]

@cderv We should also make sure that RStudio knows to make "Preview" available for R scripts.