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

knit() cannot handle variable numbers of backticks #1621

Closed
yutannihilation opened this issue Oct 11, 2018 · 11 comments · Fixed by #2047
Closed

knit() cannot handle variable numbers of backticks #1621

yutannihilation opened this issue Oct 11, 2018 · 11 comments · Fixed by #2047
Labels
feature Feature requests next Issues/PRs considered for the next release

Comments

@yutannihilation
Copy link
Collaborator

(Originally reported here: rstudio/rmarkdown#1444 (comment))

spin() now uses various number of backticks (#1611), but currently knit() cannot handle them.

tmp_rmd <- tempfile(fileext = ".Rmd")
tmp_md <- tempfile(fileext = ".md")

writeLines(
  c('````{r}',
    'x <- "',
    '```',
    '"',
    '````'),
  con = tmp_rmd
)

cat(readLines(tmp_rmd), sep = "\n")
#> ````{r}
#> x <- "
#> ```
#> "
#> ````

res <- knitr::knit(tmp_rmd, output = tmp_md, quiet = TRUE)
cat(readLines(tmp_md), sep = "\n")
#> 
#> ```r
#> x <- "
#> #> Error: <text>:1:6: unexpected INCOMPLETE_STRING
#> #> 1: x <- "
#> #>          ^
#> ```
#> "
#> ````

Created on 2018-10-10 by the reprex package (v0.2.1)

To determine how many backticks are used to represent the end of a chunk, we need to look at the beginning of the chunk (in actual, the rule is a bit more complex). But, in the current implementation of split_file(), begin chunks and end chunks are matched separately, so it seems difficult to fix without introducing a big change to the current implementation.

knitr/R/parser.R

Lines 15 to 16 in 3abb642

blks = grepl(chunk.begin, lines)
txts = filter_chunk_end(blks, grepl(chunk.end, lines))

@yihui
Copy link
Owner

yihui commented Mar 8, 2019

I don't think this particular case will be too complicated to fix. You may take a look at xfun::prose_index(), which does a similar job. In particular, these lines: https://github.com/yihui/xfun/blob/d547831d/R/markdown.R#L17-L23

However, even with that fix, it still won't work for this case (four backticks in the string):

````{r}
x <- "
````
"
````

@yutannihilation
Copy link
Collaborator Author

Oh, thanks. Maybe I thought too much.

That case doesn't seem supposed to work because it's not a valid markdown...?

@yihui
Copy link
Owner

yihui commented Mar 8, 2019

Well, it is valid Markdown: it is a code block with x <- ", followed by ", and another four backticks (will be silently ignored since the fence is not closed). You may say it is invalid R Markdown because the second series of four backticks closed the code chunk too early. The R Markdown parser in knitr is based on regular expressions, and cannot be so smart to know the second four backticks is actually a part of the R code.

@yutannihilation
Copy link
Collaborator Author

Sorry, this is valid as Markdown / R Markdown. What I meant to say was x <- " is not a valid R code.

The R Markdown parser in knitr is based on regular expressions, and cannot be so smart to know the second four backticks is actually a part of the R code.

Sorry, I don't get your point. Do you think knitr's parser should parse this as a code block of the below instead of x <- "?

x <- "
````
"

@yihui
Copy link
Owner

yihui commented Mar 9, 2019

Yes. Well, maybe. I don't really know. Perhaps it should just fail. There isn't a right answer here. Users could write an R code chunk with incomplete R code, e.g.,

```{r, error=TRUE}
x <- "
```

And the output will be

image

So I guess we can ignore the edge case I first mentioned, and only fix the original issue you brought forward.

@yutannihilation
Copy link
Collaborator Author

I believe it should fail; R Markdown itself should not be aware of code inside chunks because the code is not always written in R.

That said, after some thinking, now I feel you are right in that it's nice if knitr can detect these mistakes and raise a helpful error message :)

@atusy
Copy link
Collaborator

atusy commented Jul 8, 2020

I tried to solve this problem.
The result gives no error, but does not properly enclose the code block.
Now I see this issue requires a drastic change so that output code block has as many backticks as the input chunk.

```r
x <- "
```
"
```

https://github.com/atusy/knitr/tree/1621

@yihui
Copy link
Owner

yihui commented Sep 21, 2020

The fix master...atusy:1621 seems to be on the right track to me. I didn't read it carefully or test it.

Now I see this issue requires a drastic change so that output code block has as many backticks as the input chunk.

I'm not sure what that means, but the output blocks will intelligently use N+1 backticks when N backticks are detected in the output:

knitr/R/hooks-md.R

Lines 157 to 162 in 7d2dd40

r = paste0('\n', fence_char, '{3,}')
if (grepl(r, x)) {
l = attr(gregexpr(r, x)[[1]], 'match.length')
l = max(l)
if (l >= 4) fence = paste(rep(fence_char, l), collapse = '')
}
Will a trick like that help?

@atusy
Copy link
Collaborator

atusy commented Sep 27, 2020

@yihui Oh thanks for hint. I'll see if I can implement simply with your trick (maybe next weekend)

@cderv cderv added the feature Feature requests label Jan 28, 2021
@yihui yihui added the next Issues/PRs considered for the next release label Sep 24, 2021
@yutannihilation
Copy link
Collaborator Author

Thanks for fixing!!! 🎉

@github-actions
Copy link

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 Mar 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature Feature requests next Issues/PRs considered for the next release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants