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

Bad image links in index #927

Closed
Eluvias opened this Issue Dec 3, 2018 · 15 comments

Comments

Projects
None yet
4 participants
@Eluvias
Copy link

Eluvias commented Dec 3, 2018

The figures generated from README are sourced from the man\figures folder. Should the images be sourced from the copied figures in the reference/figures/ folder within docs?

In the index file, we get the following:

# pkgdown generates
<p><img src="../../R%20-%20Packages/testPkg/man/figures/README-EX1-1.png" width="768"></p>

I would expect the links to point to the reference folder:

# expected?? - this works
<p><img src="reference/figures/README-EX1-1.png" width="768"></p>

I used both cran and dev version.

@jayhesselberth

This comment has been minimized.

Copy link
Collaborator

jayhesselberth commented Dec 4, 2018

Yes they should be linked to reference/figures; this is what happens on the pkgdown home page.

Are you adjusting fig.path in your README? Will need a reproducible example to debug this.

@Eluvias

This comment has been minimized.

Copy link
Author

Eluvias commented Dec 4, 2018

reprex:

library(usethis)
library(pkgdown)

tmp <- file.path(tempdir(), "test")
usethis::create_package(tmp)
usethis::use_readme_rmd()
rmarkdown::render(paste0(tmp, "/README.Rmd"))
pkgdown::build_site(tmp)
Created on 2018-12-04 by the [reprex package](https://reprex.tidyverse.org) (v0.2.1)

Attached home page figure:

image

@hadley

This comment has been minimized.

Copy link
Member

hadley commented Dec 4, 2018

I'm seeing the same problem - it's probably another #787 consequence, so hopefully @goldingn can take a look too.

@Eluvias

This comment has been minimized.

Copy link
Author

Eluvias commented Dec 4, 2018

I missed to report that the figure in man/figures folder was modified when building the site. It won't be noticed unless you have a version control.

Compare the file size from man/figuresCopy folder vs man/figure:

library(usethis)
library(pkgdown)

tmp <- file.path(tempdir(), "test")
usethis::create_package(tmp)
usethis::use_readme_rmd()
rmarkdown::render(paste0(tmp, "/README.Rmd"))

# save original figure
fs::dir_copy(paste0(tmp, "/man/figures"),
            paste0(tmp, "/man/figuresCopy"))

pkgdown::build_site(tmp)
@goldingn

This comment has been minimized.

Copy link
Contributor

goldingn commented Dec 5, 2018

It looks like the issue is that usethis::use_readme_rmd() adds the chunk option fig.path = "man/figures/README-", whereas pkgdown is expecting fig.path not to be set, since it creates docs/index_files to put these images in.

Deleting that line works as a fix for me. Here's a reprex:

library(usethis)
library(pkgdown)

tmp <- file.path(tempdir(), "test")
usethis::create_package(tmp, open = FALSE)
usethis::use_readme_rmd(open = FALSE)

# delete the fig.path chunk option
readme_path <- file.path(tmp, "README.Rmd")
writeLines(
  readLines(readme_path)[-11],
  con = file(readme_path)
)

pkgdown::build_site(tmp)

Though obviously we'd like this to play nicely with usethis, and be helpful if the user directs their outputs elsewhere. Ideally we'd detect where the user is putting their figures, and then copy them over. Though not sure how feasible that is. Perhaps there's a simpler approach that I haven't thought of.

@jayhesselberth

This comment has been minimized.

Copy link
Collaborator

jayhesselberth commented Dec 5, 2018

So this looks like an unfortunate interaction between pkgdown and usethis behaviors, not a regression in pkgdown.

@hadley

This comment has been minimized.

Copy link
Member

hadley commented Dec 6, 2018

I think we need to back out #834 and reconsider: content-home.html is an html template; content-article.html is a pandoc template.

@hadley hadley closed this in d689abf Dec 6, 2018

@hadley

This comment has been minimized.

Copy link
Member

hadley commented Dec 6, 2018

@goldingn you're going to have to take another stab at this; unfortunately your analysis of the problem wasn't quite right.

@goldingn

This comment has been minimized.

Copy link
Contributor

goldingn commented Dec 6, 2018

I'm confused.

Neither #913 nor #918 were caused by #834. The only change #834 made to content-home.html was to de-indent the line with {{index}}.

It looks like there were a bunch of errors in that template since before #834. The only reason they weren't picked up previously is that the article template (instead of the home template) was being used to render the index prior to #834.

@goldingn

This comment has been minimized.

Copy link
Contributor

goldingn commented Dec 6, 2018

Looks like only some of the changes introduced in #834 have been reverted. Not sure if that's deliberate.

Happy to help work this out, but I'm not sure what problem (ie. other than those old problems in content-home.html that @jayhesselberth patched) needs fixing.

@hadley

This comment has been minimized.

Copy link
Member

hadley commented Dec 6, 2018

It changed which template was being used, which I'm reasonably certain caused all these downstream problems. I think you missed the fact that the templates were fundamentally different types (differing on whether or not they passed through pandoc), and they can't simply be interchanged. This is likely why there was two separate templates from day one.

@goldingn

This comment has been minimized.

Copy link
Contributor

goldingn commented Dec 6, 2018

Ah, got it. Thanks for the clarification.

So it looks like we need a separate html template for the Rmd case, based on content-article.html in the first instance in order to maintain the current behaviour.

It's potentially confusing to users to have two distinct templates for the index, but seems like the best option. Forcing conversion from Rmd -> md first is an alternative, but copying linked files would be difficult.

Do you have a preferred naming scheme to disambiguate the Rmd index template from the md index template?

@Eluvias

This comment has been minimized.

Copy link
Author

Eluvias commented Dec 7, 2018

It seems the problem still persists. Do you see the same?

library(usethis)
library(pkgdown)

tmp <- file.path(tempdir(), "test")
usethis::create_package(tmp)
usethis::use_readme_rmd(open = FALSE)
rmarkdown::render(paste0(tmp, "/README.Rmd"))

# figures in  "/man/figures" are being modified  
fs::dir_copy(paste0(tmp, "/man/figures"),
            paste0(tmp, "/man/figuresCopy"))

pkgdown::build_site(tmp)
@hadley

This comment has been minimized.

Copy link
Member

hadley commented Dec 7, 2018

It worked for me in my tests. Do you have the latest version installed?

@Eluvias

This comment has been minimized.

Copy link
Author

Eluvias commented Dec 13, 2018

The problem still persists for me. Does anyone else see the same?
Also the size of figure stored in man/figures is different.

library(usethis)
library(pkgdown)

tmp <- file.path(tempdir(), "test")
usethis::create_package(tmp)
usethis::use_readme_rmd(open = FALSE)

rmarkdown::render(paste0(tmp, "/README.Rmd"))

# different size before and after
fs::dir_copy(paste0(tmp, "/man/figures"),
            paste0(tmp, "/man/figuresCopy"))

pkgdown::build_site(tmp)

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

Session info
devtools::session_info()
#> - Session info ----------------------------------------------------------
#>  setting  value                       
#>  version  R version 3.5.0 (2018-04-23)
#>  os       Windows 7 x64 SP 1          
#>  system   x86_64, mingw32             
#>  ui       RTerm                       
#>  language (EN)                        
#>  collate  English_United Kingdom.1252 
#>  ctype    English_United Kingdom.1252 
#>  tz       Europe/London               
#>  date     2018-12-13                  
#> 
#> - Packages --------------------------------------------------------------
#>  package     * version    date       lib source                           
#>  assertthat    0.2.0      2017-04-11 [1] CRAN (R 3.5.0)                   
#>  backports     1.1.2      2017-12-13 [1] CRAN (R 3.5.0)                   
#>  callr         3.1.0      2018-12-10 [1] CRAN (R 3.5.0)                   
#>  cli           1.0.1      2018-09-25 [1] CRAN (R 3.5.1)                   
#>  clisymbols    1.2.0      2017-05-21 [1] CRAN (R 3.5.0)                   
#>  commonmark    1.7        2018-12-01 [1] CRAN (R 3.5.1)                   
#>  crayon        1.3.4      2017-09-16 [1] CRAN (R 3.5.0)                   
#>  curl          3.2        2018-03-28 [1] CRAN (R 3.5.0)                   
#>  desc          1.2.0      2018-05-01 [1] CRAN (R 3.5.0)                   
#>  devtools      2.0.1      2018-10-26 [1] CRAN (R 3.5.1)                   
#>  digest        0.6.18     2018-10-10 [1] CRAN (R 3.5.1)                   
#>  evaluate      0.12       2018-10-09 [1] CRAN (R 3.5.0)                   
#>  fs            1.2.6      2018-08-23 [1] CRAN (R 3.5.1)                   
#>  gh            1.0.1      2017-07-16 [1] CRAN (R 3.5.0)                   
#>  git2r         0.23.0     2018-07-17 [1] CRAN (R 3.5.1)                   
#>  glue          1.3.0      2018-07-17 [1] CRAN (R 3.5.0)                   
#>  highr         0.7        2018-06-09 [1] CRAN (R 3.5.0)                   
#>  htmltools     0.3.6      2017-04-28 [1] CRAN (R 3.5.0)                   
#>  httr          1.4.0      2018-12-11 [1] CRAN (R 3.5.1)                   
#>  jsonlite      1.6        2018-12-07 [1] CRAN (R 3.5.1)                   
#>  knitr         1.21       2018-12-10 [1] CRAN (R 3.5.1)                   
#>  magrittr      1.5        2014-11-22 [1] CRAN (R 3.5.0)                   
#>  MASS          7.3-51.1   2018-11-01 [2] CRAN (R 3.5.1)                   
#>  memoise       1.1.0      2017-04-21 [1] CRAN (R 3.5.0)                   
#>  mime          0.6        2018-10-05 [1] CRAN (R 3.5.1)                   
#>  pillar        1.3.0      2018-07-14 [1] CRAN (R 3.5.0)                   
#>  pkgbuild      1.0.2      2018-10-16 [1] CRAN (R 3.5.1)                   
#>  pkgdown     * 1.3.0.9000 2018-12-13 [1] Github (r-lib/pkgdown@fa9b750)   
#>  pkgload       1.0.2      2018-10-29 [1] CRAN (R 3.5.1)                   
#>  prettyunits   1.0.2      2015-07-13 [1] CRAN (R 3.5.0)                   
#>  processx      3.2.1      2018-12-05 [1] CRAN (R 3.5.0)                   
#>  ps            1.2.1      2018-11-06 [1] CRAN (R 3.5.1)                   
#>  purrr         0.2.5.9000 2018-12-13 [1] Github (tidyverse/purrr@6dec768) 
#>  R6            2.3.0      2018-10-04 [1] CRAN (R 3.5.0)                   
#>  Rcpp          1.0.0      2018-11-07 [1] CRAN (R 3.5.0)                   
#>  remotes       2.0.2      2018-10-30 [1] CRAN (R 3.5.1)                   
#>  rlang         0.3.0.1    2018-10-25 [1] CRAN (R 3.5.1)                   
#>  rmarkdown     1.11       2018-12-08 [1] CRAN (R 3.5.0)                   
#>  roxygen2      6.1.1      2018-11-07 [1] CRAN (R 3.5.0)                   
#>  rprojroot     1.3-2      2018-01-03 [1] CRAN (R 3.5.0)                   
#>  rstudioapi    0.8        2018-10-02 [1] CRAN (R 3.5.0)                   
#>  sessioninfo   1.1.1      2018-11-05 [1] CRAN (R 3.5.0)                   
#>  stringi       1.2.4      2018-07-20 [1] CRAN (R 3.5.0)                   
#>  stringr       1.3.1      2018-05-10 [1] CRAN (R 3.5.0)                   
#>  testthat      2.0.1      2018-10-13 [1] CRAN (R 3.5.1)                   
#>  tibble        1.4.2.9002 2018-05-31 [1] Github (tidyverse/tibble@3bcc4db)
#>  usethis     * 1.4.0      2018-08-14 [1] CRAN (R 3.5.1)                   
#>  whisker       0.3-2      2013-04-28 [1] CRAN (R 3.5.0)                   
#>  withr         2.1.2      2018-03-15 [1] CRAN (R 3.5.0)                   
#>  xfun          0.4        2018-10-23 [1] CRAN (R 3.5.0)                   
#>  xml2          1.2.0      2018-01-24 [1] CRAN (R 3.5.0)                   
#>  yaml          2.2.0      2018-07-25 [1] CRAN (R 3.5.1)                   
#> 
#> [1] C:/Program Files/R/library
#> [2] C:/Program Files/R/R-3.5.0/library
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment