Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
cderv opened this issue Jan 4, 2024 · 8 comments · Fixed by #2320
Closed

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

cderv opened this issue Jan 4, 2024 · 8 comments · Fixed by #2320
Labels
quarto Related to quarto specific usage

Comments

@cderv
Copy link
Collaborator

cderv commented Jan 4, 2024

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
#> ```
@cderv cderv added the quarto Related to quarto specific usage label Jan 4, 2024
@kylebutts
Copy link
Contributor

@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
Copy link
Collaborator Author

cderv commented Jan 25, 2024

Support for .qmd was added in 9f3ffca without modifying how spin() works.

Example in comment once done (#2284 (comment)) 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
Copy link
Contributor

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
```
```{r}
#| 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, REditorSupport/vscode-R#1455, so I'll have to add a PR there too

@cderv
Copy link
Collaborator Author

cderv commented Jan 25, 2024

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, REditorSupport/vscode-R#1455, so I'll have to add a PR there too

Oh good to know ! Thanks for linking to this

@kylebutts
Copy link
Contributor

@cderv Fixed in my fork: kylebutts@b8e17d7. 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
Copy link
Collaborator Author

cderv commented Jan 29, 2024

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
Copy link
Owner

yihui commented Mar 28, 2024

I forgot to close this issue when merging #2320.

@yihui yihui closed this as completed Mar 28, 2024
@cderv cderv linked a pull request Mar 28, 2024 that will close this issue
Copy link

github-actions bot commented Oct 2, 2024

This old thread has been automatically locked. If you think you have found something related to this, please open a new issue by following the issue guide (https://yihui.org/issue/), and link to this old issue if necessary.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
quarto Related to quarto specific usage
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants