-
-
Notifications
You must be signed in to change notification settings - Fork 872
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
Introduce convert_chunk_header() to convert fenced chunk head to block chunk option #2149
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- You mentioned
getParseData()
would be useful to get one per line, but it seems I manage to have something working without it. Did I miss anything ?- For the label part, do we want unexplicit or explicit ?
I see that you parsed the options. In that case, there is no need to use getParseData()
. Originally I was thinking of simply extracting the chunk options as a single string, and then applying strwrap(prefix = '#| ')
. If users want one option per line, we'll need to figure out which commas are true separators, e.g., getParseData(parse(text = 'alist(foo, bar=1, baz="a,b,c")'))
will tell you.
I feel you don't have to start with Rmd but make a general converter (Rmd does need a tiny special treatment, though). What I'd do is:
- Read the file.
detect_pattern()
and determine the regular expressionchunk.begin
fromall_patterns
.- Extract the chunk options and
strwrap()
them. - Append the wrapped
#|
options and write out the content.
We can consider the two cases later in different PRs: one option per line (comma-separated), and converting to YAML.
- What behavior do we want for the function:
Have an argument output = NULL
, which means writing output to console. Users can specify the output file path, in which case we write to that file. Alternatively, it can take a function that takes the input
value and returns a file path, e.g., output = identity
means overwriting the original input file.
* detect pattern on file extension * simplify for now and use strwrap() * correctly handle output feature: default to console and allow identity for overwriting
Thanks for your feedback. That was mainly my idea at first, but I was not sure if making it generic would be expected.
I have redone a solution with that. I did that at first, but what I don't like is that it is completely splitting in the middle of the code. I don't think anyone would use like that - they would split on comma. Example for long chunks ```{verbatim}
#| lang = "markdown", code =
#| stringr::str_trim(knitr::knit_child("examples/py-engine.Rmd",
#| quiet = TRUE))
``` from ```{verbatim, lang = "markdown", code = stringr::str_trim(knitr::knit_child("examples/py-engine.Rmd", quiet = TRUE))}
```
I always forgot about this trick of using a function. So now
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me overall. Thanks!
as splitting parsed_data on commas does not get nice results. It will also split on comma within R code used.
@yihui after giving more thoughts and trying the This way seems cleaner to me. What do you think ? There is still some room to improve line wrapping for You can review once more maybe, then I'll document on monday, and do further test. We can add YAML support later. |
`wrap_width` can be set to FALSE to deactivate wrapping when `type = "multiline"`
@yihui I just saw that Rnw file does not have an engine unlike Rmd. Is engine setup only for It seems I need to take this into account: Lines 73 to 80 in ca398c2
Probably doing Line 15 in ca398c2
right ? But if no engine, what are we suppose to use inside comment for none markdown document ? I guess we have all of this to support names(knitr:::all_patterns)
#> [1] "rnw" "brew" "tex" "html" "md" "rst" "asciidoc"
#> [8] "textile" Thanks |
This reverts commit 240e790.
Chunk does not have an engine. They defaults to `r` engine for the comment char. (based on `partition_chunk` implementation.
Ok. In fact based on |
Test in knitr-examples went well files <- fs::dir_ls(regexp = "[0-9].*\\.(brew|R(nw|md|tex|html|rst|asciidoc|textile))")
purrr::walk(files, ~ { message(.x) ; convert_chunk_header(.x, output = identity, type = "multiline")}) and same for type = 'wrap' I added a powershell script in knitr example to help me test everything. yihui/knitr-examples@f2cc2eb |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made some cosmetic changes, and I think it's ready to merge now. Thanks!
I might give type = "yaml"
a try later.
Thanks ! |
This PR introduces a new function to allow a user to convert all chunks to new syntax.
https://yihui.org/en/2022/01/knitr-news/
First supported conversion is to multiline R chunk option, but YAML chunk header could be supported next.
To introduce the feature, I refactor some small pieces of already existing parsing code in some utility function.
Target format is one option per line.
@yihui opening this PR so that you can already give some thoughts and feedback
You mentioned
getParseData()
would be useful to get one per line, but it seems I manage to have something working without it. Did I miss anything ?For the label part, do we want unexplicit or explicit ?
or
For now, I did the first;
Also for checks, I am aiming the function to
.Rmd
and.qmd
for this. I think that is ok.I need to test a bit more, and also see if I could derive YAML syntax from this implementation