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

Avoid pagetitle warning from pandoc2.0 when title is missing. #1355

Merged
merged 1 commit into from May 18, 2018

Conversation

@jdblischak
Copy link
Contributor

@jdblischak jdblischak commented May 18, 2018

I noticed the warning produced by pandoc 2+ when the R Markdown file does not have a title in the YAML header. This is the same warning as demonstrated for find_external_resources in #1290. The fix in #1298 removes the warning only for find_external_resources.

This PR makes the change to the pre_processor function of html_document_base, and thus it removes the warning for anything that inherits from this function (including find_external_resources).

I also made two other minor improvements:

  1. Don't specify pagetitle if there is a title specified in the YAML header. This keeps the text displayed in the browser tab the same as the document title (which is the same behavior as pandoc 1.0)
  2. Instead of using "PREVIEW", use the basename of the input file. This will make it easier to identify the browser tab. Also, this is similar to pandoc's fallback option.

cc @rich-iannone

I tested this with R 3.3.3, pandoc 2.2.1, and macOS 10.10.5. I'm happy to test this on other operating systems if there is interest in merging this PR.

I've previously signed the individual contributor agreement.

@rich-iannone rich-iannone requested a review from yihui May 18, 2018
@yihui yihui added this to the v1.10 milestone May 18, 2018
@yihui
yihui approved these changes May 18, 2018
Copy link
Member

@yihui yihui left a comment

Looks great to me! Thanks!

@rich-iannone rich-iannone merged commit 7ea4f08 into rstudio:master May 18, 2018
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
jdblischak added a commit to jdblischak/workflowr that referenced this pull request Mar 25, 2019
After my change was included in rmarkdown 1.10, the pagetitle was
specified twice, which pandoc does not accept. Thus workflowr should
only apply its fix for previous versions of rmarkdown.

rstudio/rmarkdown#1355
rstudio/rmarkdown#1434
jdblischak added a commit to jdblischak/workflowr that referenced this pull request Mar 25, 2019
After my change was included in rmarkdown 1.10, the pagetitle was
specified twice, which pandoc does not accept. Thus workflowr should
only apply its fix for previous versions of rmarkdown.

rstudio/rmarkdown#1355
rstudio/rmarkdown#1434
jdblischak added a commit to jdblischak/workflowr that referenced this pull request Mar 25, 2019
After my change was included in rmarkdown 1.10, the pagetitle was
specified twice, which pandoc does not accept. Thus workflowr should
only apply its fix for previous versions of rmarkdown.

rstudio/rmarkdown#1355
rstudio/rmarkdown#1434
yihui added a commit to RLesur/rmarkdown that referenced this pull request Apr 1, 2019
yihui added a commit that referenced this pull request Nov 26, 2019
…#1355)" and revert "fix #1434: use --metadata:pagetitle only if both title and pagetitle in YAML are empty"

This reverts commits 7ea4f08 and 19008bf.

This is because applying `--metadata pagetitle=...` to all HTML output documents is too aggressive, and can cause new issues like #1607, which are even worse than the warning about the missing title.

Fix #1607 and close #1608.
@yihui
Copy link
Member

@yihui yihui commented Nov 26, 2019

@jdblischak Sorry I decided to revert this PR, because it has brought a couple of new issues that would be too hard to fix. The first one was #1434, which was not too bad. The second one was #1607, which is not trivial to fix. I think the case #1607 is bad enough that it is worth reverting this PR.

I don't know what your original motivation for this PR was (i.e. why the title could be missing in workflowr). If I understand the problem better, I might be able to come up with a direct fix to that problem.

@jdblischak
Copy link
Contributor Author

@jdblischak jdblischak commented Dec 4, 2019

@yihui No worries. I apologize that my PR caused you and others so much difficulty.

My original motivation was to remove the pandoc2 warning about a missing title:

f <- tempfile()
writeLines(c("```{r}", "1 + 1", "```"), con = f)
rmarkdown::render(f, quiet = TRUE)
## [WARNING] This document format requires a nonempty <title> element.
##   Please specify either 'title' or 'pagetitle' in the metadata.
##   Falling back to 'file63fd452f6059.utf8'
sessioninfo::package_info("rmarkdown")["rmarkdown", ]
##  package   * version date       lib source                            
##  rmarkdown   1.18.4  2019-12-04 [1] Github (rstudio/rmarkdown@3d648d3)
## 
## [1] /home/jdb-work/R/x86_64-pc-linux-gnu-library/3.6
## [2] /usr/local/lib/R/site-library
## [3] /usr/lib/R/site-library
## [4] /usr/lib/R/library

I don't bother adding a title to most of the Rmd files I use to test workflowr, so all of those pandoc2 warnings were obscuring the true error messages whenever a test failed. I'll continue to handle this downstream in workflowr.

@yihui
Copy link
Member

@yihui yihui commented Dec 4, 2019

Sounds good. Thanks @jdblischak! I also added titles to our tests: c38c5c8

jdblischak added a commit to jdblischak/workflowr that referenced this pull request Dec 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.