do not normalize web paths in css argument #2164
Merged
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
yihui
approved these changes
Jun 7, 2021
| @@ -106,7 +106,8 @@ html_document_base <- function(theme = NULL, | |||
| options = sass::sass_options(output_style = "compressed") | |||
| ) | |||
| } | |||
| f <- normalized_relative_to(output_dir, f) | |||
| # do not normalize web path | |||
| if (!xfun::is_web_path(f)) f <- normalized_relative_to(output_dir, f) | |||
There was a problem hiding this comment.
I'm just wonder if it would make more sense to test the existence of the file, but I guess it would be a more risky fix (i.e., there might be cases in which the file doesn't exist given the current working directory---in particular, I'm not sure about Shiny documents):
Suggested change
| if (!xfun::is_web_path(f)) f <- normalized_relative_to(output_dir, f) | |
| if (file.exists(f)) f <- normalized_relative_to(output_dir, f) |
There was a problem hiding this comment.
I don't know about this either. This would require some testing to be sure.
This transformation happens in Pandoc pre processor so right before Pandoc is ran so it should exists.
Unless there is an edge case we missed (like order of pre processor ?) 🤔
There was a problem hiding this comment.
Okay. Let's just be conservative here and merge the PR. Thanks!
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 fix #2163
It was a regression after #2095 and we did not had test for R Markdown document using a url in
cssargument it seems.I took the opportunity to add some tests for this, including with bslib theme activated.