Loop correctly on splitted file by file_scope function #2150
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This caused an issue with file_scope foor loop
yihui
approved these changes
May 28, 2021
There was a problem hiding this comment.
To do a proper unit test (and not a big rendering test), I refactored by moving out the file scope logic into a function.
That's great! To be honest, render() is a too big function and I'm all for splitting it into smaller ones, which can be good for both testing and avoiding inadvertent name conflicts.
Co-authored-by: Yihui Xie <[email protected]>
[skip CI]
jonathan-g
added a commit
to jonathan-g/rmarkdown
that referenced
this pull request
Jun 18, 2021
* rstudio_origin/master: (90 commits) start the next version CRAN release v2.9 fix rstudio#2163: do not normalize web paths in the css argument of html output formats (rstudio#2164) Add a css file for future tests Restore previous shiny theme when run() exits (rstudio#2160) Correct name in workflow file Add a workflow to test Pandoc nighly (rstudio#2153) bslib 0.2.5.1 is now on CRAN use `stop2()` from knitr internal instead of `stop(..., call. = FALSE)` (rstudio#2152) 74e2f2f changed the error message, and a test relies on the content of this message (it probably shouldn't be so specific) tweak wording Support 'href' dependencies (rstudio#2151) Loop correctly on splitted file by file_scope function (rstudio#2150) update snapshot file following change in test name in a8aad75 amend 84ff469: clarify the meaning of fig_crop = 'auto' in the doc Add the position of the last character (rstudio#2146) don't put the checklist in the comment since it is required Fix for lost encoding in shiny_prerendered_html. (rstudio#2140) Insure fontawesome V4 compatibility in navbar (rstudio#1994) fix rstudio#2043: replace parse(text) with xfun::parse_only() to avoid hanging the R session when the input is empty ...
jonathan-g
added a commit
to jonathan-g/rmarkdown
that referenced
this pull request
Jun 18, 2021
Merge remote-tracking branch 'rstudio_origin/master' into jg-devel * rstudio_origin/master: (64 commits) start the next version CRAN release v2.9 fix rstudio#2163: do not normalize web paths in the css argument of html output formats (rstudio#2164) Add a css file for future tests Restore previous shiny theme when run() exits (rstudio#2160) Correct name in workflow file Add a workflow to test Pandoc nighly (rstudio#2153) bslib 0.2.5.1 is now on CRAN use `stop2()` from knitr internal instead of `stop(..., call. = FALSE)` (rstudio#2152) 74e2f2f changed the error message, and a test relies on the content of this message (it probably shouldn't be so specific) tweak wording Support 'href' dependencies (rstudio#2151) Loop correctly on splitted file by file_scope function (rstudio#2150) update snapshot file following change in test name in a8aad75 amend 84ff469: clarify the meaning of fig_crop = 'auto' in the doc Add the position of the last character (rstudio#2146) don't put the checklist in the comment since it is required Fix for lost encoding in shiny_prerendered_html. (rstudio#2140) Insure fontawesome V4 compatibility in navbar (rstudio#1994) fix rstudio#2043: replace parse(text) with xfun::parse_only() to avoid hanging the R session when the input is empty ... # Conflicts: # DESCRIPTION # R/html_dependencies.R
jonathan-g
added a commit
to jonathan-g/rmarkdown
that referenced
this pull request
Jun 18, 2021
Merge remote-tracking branch 'rstudio_origin/master' into minimal-tree-fix * rstudio_origin/master: (223 commits) start the next version CRAN release v2.9 fix rstudio#2163: do not normalize web paths in the css argument of html output formats (rstudio#2164) Add a css file for future tests Restore previous shiny theme when run() exits (rstudio#2160) Correct name in workflow file Add a workflow to test Pandoc nighly (rstudio#2153) bslib 0.2.5.1 is now on CRAN use `stop2()` from knitr internal instead of `stop(..., call. = FALSE)` (rstudio#2152) 74e2f2f changed the error message, and a test relies on the content of this message (it probably shouldn't be so specific) tweak wording Support 'href' dependencies (rstudio#2151) Loop correctly on splitted file by file_scope function (rstudio#2150) update snapshot file following change in test name in a8aad75 amend 84ff469: clarify the meaning of fig_crop = 'auto' in the doc Add the position of the last character (rstudio#2146) don't put the checklist in the comment since it is required Fix for lost encoding in shiny_prerendered_html. (rstudio#2140) Insure fontawesome V4 compatibility in navbar (rstudio#1994) fix rstudio#2043: replace parse(text) with xfun::parse_only() to avoid hanging the R session when the input is empty ... # Conflicts: # DESCRIPTION # NEWS.md # R/html_dependencies.R
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This will fix #2149 (at least the error mentioned but I don't see the relation to Lua filters 🤷 )
It resolve a name conflict after change in 92b437a (#2098) that caused an issue with file_scope for loop.
I took the time to add some test so that we can detect this next time (if any).
To do a proper unit test (and not a big rendering test), I refactored by moving out the file scope logic into a function. Also, I used snapshot tests on the dummy example as it is simple and works well in this case.
Hope this is fine.