Installing R.rsp causes HTML-based help.search() to give an error

[HenrikBengtsson]

From https://github.com/jeroen/jsonlite/issues/196:

Issue

Having R.rsp installed can break HTML-based search to break. For example, with:

options(help_type = "html")
help.search("rsp")

may display

Error in if (nchar(Outfile)) Outfile else File : 
  argument is not interpretable as logical

in the browser.

Troubleshooting

From some of the comments in https://github.com/jeroen/jsonlite/issues/196:

debugonce(tools:::makeVignetteTable)
help.search("rsp")
[...]
Browse[2]> vignettes
     Package File Title PDF R 
[1,] "R.rsp" NA   NA    NA  NA
[2,] "R.rsp" NA   NA    NA  NA
[3,] "R.rsp" NA   NA    NA  NA
[4,] "R.rsp" NA   NA    NA  NA
[5,] "R.rsp" NA   NA    NA  NA

The problem appears to be that the Topic field below contains file extensions:

> temp <- utils::help.search(pattern = "rsp", package = "R.rsp", type = "vignette")$matches
> temp$Topic
 [1] "Dynamic_document_creation_using_RSP.tex" "Dynamic_document_creation_using_RSP.tex"
 [3] "Dynamic_document_creation_using_RSP.tex" "RSP_intro.html"                         
 [5] "RSP_intro.html"                          "RSP_intro.html"                         
 [7] "RSP_refcard.tex"                         "RSP_refcard.tex"                        
 [9] "RSP_refcard.tex"                         "R_packages-RSP_vignettes.md"            
[11] "R_packages-RSP_vignettes.md"             "R_packages-RSP_vignettes.md"            
[13] "R_packages-Vignettes_prior_to_R300.tex" 

[HenrikBengtsson]

Further narrowing in on the problem:

> db <- utils::hsearch_db("R.rsp", types = "vignette")
> temp <- subset(db$Base, Package == "R.rsp" & Type == "vignette")
> temp$Topic
[1] "R_packages-Static_PDF_and_HTML_vignettes.pdf" "R_packages-LaTeX_vignettes"                  
[3] "Dynamic_document_creation_using_RSP.tex"      "RSP_intro.html"                              
[5] "RSP_refcard.tex"                              "R_packages-RSP_vignettes.md"                 
[7] "R_packages-Vignettes_prior_to_R300.tex"      

Note how Name carry file-name extensions, which it shouldn't. This stems from an assumption in utils that vignette files only has one filename extension. Also, Topic <- Name by default, so the key problem is most likely that Name is invalid.

[kevinushey]

@HenrikBengtsson: do you think this is worth reporting to R-core?

[HenrikBengtsson]

We could. I'll try to do some more troubleshooting over the weekend to what's missing and if this is a recent bug or has always been there. (When I said I'm sure I haven't seen it before, it could simply be that I've never done help.search() on topics where this happens).

[HenrikBengtsson]

To continue what I mentioned above, the assumption that vignette files only has one filename extension was only true in R (< 3.0.0). In R (>= 3.0.0), it's possible for vignette engines to specify an arbitrary filename pattern. For instance, engines Sweave, knitr::knitr, and R.rsp::tex use:

 tools::vignetteEngine("Sweave")$pattern
[1] "[.][rRsS](nw|tex)$"

> tools::vignetteEngine("knitr::knitr")$pattern
[1] "[.]([rRsS](nw|tex)|[Rr](md|html|rst))$"

> tools::vignetteEngine("R.rsp::tex")$pattern
[1] "[.](tex|ltx)$"

These meet the assumption of a single level of filename extension such that sub("\\.[^.]*$", "", basename(vDB$File)) suffice.

However, engines such as R.rsp::rsp and R.rsp::asis use filenames with two levels of extensions:

> tools::vignetteEngine("R.rsp::rsp")$pattern
[1] "[.][^.]*[.]rsp$"

> tools::vignetteEngine("R.rsp::asis")$pattern
[1] "[.](pdf|html)[.]asis$"

A proper fix to utils:::merge_vignette_index() would involve replacing:

base[, "Name"] <- sub("\\.[^.]*$", "", basename(vDB$File))

with something like:

engine_pattern <- tools::vignetteEngine(engine_name)$pattern
base[, "Name"] <- sub(engine_pattern, "", basename(vDB$File))

The problem with this "at-runtime" approach is that the following have to happen each time the vignette database is (re-)built:

1. the vignette engine name needs to be inferred
2. the vignette engine package (VignetteBuilder) needs to be loaded

See tools::pkgVignettes() for how this can done (although inefficiently).

However, a more efficient solution would probably be to set the vignette Name when the package vignette database is built, i.e. during installation, which occurs in tools:::.build_vignette_index() called by tools:::.install_package_vignettes2().

Yet another solution would be to record the vignette-engine pattern (also in tools:::.build_vignette_index()) for each vignette such that it's available in vDB;

base[, "Name"] <- sub(vDB$Pattern, "", basename(vDB$File))

[HenrikBengtsson]

A third, and much simpler, alternative could be to replace:

base[, "Name"] <- sub("\\.[^.]*$", "", basename(vDB$File))

with

base[, "Name"] <- sub("\\.[^.]*$", "", basename(vDB$PDF))

This would move the assumption of a single-level of filename extensions on the vignette source file to the same assumption on the vignette output file, which (currently) may only be *.PDF and *.HTML.

[HenrikBengtsson]

Ah! The original error originates from tools:::httpd():

topic <- temp[i, "Topic"]
[...]
vignette <- vigDB[topic == file_path_sans_ext(vigDB$PDF),]

which suggests that utils:::merge_vignette_index() should indeed use:

base[, "Name"] <- sub("\\.[^.]*$", "", basename(vDB$PDF))
base[, "Topic"] <- base[, "Name"]

rather than

base[, "Name"] <- sub("\\.[^.]*$", "", basename(vDB$File))
base[, "Topic"] <- base[, "Name"]

This change / bug fix seems easier to argue for.

PS. The conclusions in this last comment looks awfully familiar to me - I might have discovered this bug in the past for other reasons - it could also just be a jet lag-induced deja vu.

[HenrikBengtsson]

@kevinushey, does the conclusions in https://github.com/HenrikBengtsson/R.rsp/issues/27#issuecomment-325071451 about a bug in utils:::merge_vignette_index() look reasonable to you?

[kevinushey]

Nice sleuthing! It sounds to me like the bug lives there as well.

[HenrikBengtsson]

I just sent a bug report with below patch R-devel (https://stat.ethz.ch/pipermail/r-devel/2017-August/074834.html).

Index: src/library/utils/R/help.search.R
===================================================================
--- src/library/utils/R/help.search.R (revision 73159)
+++ src/library/utils/R/help.search.R (working copy)
@@ -43,7 +43,7 @@
  base[, "LibPath"] <- path
  id <- as.character(1:nrow(vDB) + NROW(hDB[[1L]]))
  base[, "ID"] <- id
- base[, "Name"] <- sub("\\.[^.]*$", "", basename(vDB$File))
+ base[, "Name"] <- tools::file_path_sans_ext(basename(vDB$PDF))
  base[, "Topic"] <- base[, "Name"]
  base[, "Title"] <- vDB$Title
  base[, "Type"] <- "vignette"

[HenrikBengtsson]

UPDATE: Fixed in R-devel (rev 73170).

[HenrikBengtsson]

UPDATE: Fix ported to the R 3.4.x branch (rev 73176), meaning it should be part of the R 3.4.2 release and I guess also R 3.4.1 patched.