yihui / knitr

A general-purpose tool for dynamic report generation in R
https://yihui.org/knitr/
2.36k stars 873 forks source link

[spin] Better support for `#|` to split cell in .qmd #2314

Closed cderv closed 3 months ago

cderv commented 6 months ago
knitr::spin(text = c('#| echo: false','1+1', '#| label: test','1+1'), format = 'qmd', knit = F) |> xfun::raw_string()
#> 
#> ```{r}
#> #| echo: false
#> 1+1
#> #| eval: false
#> 1+1
#> ```

This is producing wrong syntax in .qmd really correct or ideal, considering #+ does correcty split

knitr::spin(text = c('#+ echo = FALSE','1+1', '#+ eval=FALSE','1+1'), format = 'qmd', knit = F) |> xfun::raw_string()
#> 
#> ```{r echo = FALSE}
#> 1+1
#> ```
#> ```{r eval=FALSE}
#> 1+1
#> ```
kylebutts commented 5 months ago

@cderv I'm confused by this. What is the intended output of the first example?

From my understanding of the internals of knitr::spin, #| are ignored, so they work sort of by accident. Is your expectation that a lone #| would produce a code chunk?

In the second example, #+ marks code cells, so they're demarkating code chunks.

cderv commented 5 months ago

Support for .qmd was added in https://github.com/yihui/knitr/commit/9f3ffcac41de6f4c4502d2858a941ebc65708eb5 without modifying how spin() works.

Example in comment once done (https://github.com/yihui/knitr/issues/2284#issuecomment-1699311083) was

spin(text = c('#| echo: false','1+1'), format = 'qmd', knit = F)

It produces the right syntax for a .qmd as we expected; But as their was no special handling, it misses some cases by just ignoring the special comment.

For full support of knitr::spin() we need to handle #| correctly and just create different chunk when we see some comment.

knitr::spin() is used by quarto render script.R right now - so this is an issue to fix.

Hope it helps understand the meaning of this. I'll probably do it soon, unless someone wants to give it a try

kylebutts commented 5 months ago

Ahh, yes! I agree. That #| works is just because it is left alone by knitr::spin().

Just to be clear, you think the result of your first code block should be:

```{r}
#| echo: false
1+1
#| eval: false
1+1


I can submit a PR for this.  I think there's two modifications that should occur:
1. `#|` should be treated as a code chunk marker
2. Any `#|` that are immediately following *any* code marker should be left as is. 

Does that seem right to you? And would this would only be with `format = 'qmd'`? 

BTW, this change would also touch the R VSCode extension, https://github.com/REditorSupport/vscode-R/pull/1455, so I'll have to add a PR there too
cderv commented 5 months ago

just to be clear, you think the result of your first code block should be:

Yes

And would this would only be with format = 'qmd'?

Yes I think this is better.

BTW, this change would also touch the R VSCode extension, https://github.com/REditorSupport/vscode-R/pull/1455, so I'll have to add a PR there too

Oh good to know ! Thanks for linking to this

kylebutts commented 5 months ago

@cderv Fixed in my fork: https://github.com/kylebutts/knitr/commit/b8e17d736ee557acc5f5714c5a4289ef20416857. What do you think about the approach? It basically inserts a # %% whenever a code chunk starts with #| and then runs spin as normal.

I was thinking of refactoring that part of spin to loop through the lines and mark chunk_start and chunk_end. This might make the code more readable / simple, but is more of an invasive/bug-prone change.

cderv commented 5 months ago

Thanks for giving it a try ! Can you open a PR even as draft ? This makes it more easy to compare and review. @yihui will also be able to jump in and give its thoughts.

Thanks!

yihui commented 3 months ago

I forgot to close this issue when merging https://github.com/yihui/knitr/pull/2320.