Vectorise replacement function

[hadley]

This breaks 7 revdeps but I think it's worth it because the performance improvement is immense.

[hadley]

It also breaks rmarkdown 😬

Error in `if (grepl("^[.][.]", in_file)) ...`:
! the condition has length > 1
---
Backtrace:
     ▆
  1. └─pkgdown::build_article("from-base", new_process = FALSE)
  2.   └─pkgdown:::render_rmarkdown(...) at pkgdown/R/build-articles.R:281:2
  3.     ├─rlang::inject(rmarkdown::render(!!!args)) at pkgdown/R/rmarkdown.R:52:4
  4.     └─rmarkdown::render(...) at rlang/R/eval.R:195:2
  5.       └─output_format$post_processor(...)
  6.         └─rmarkdown (local) base(metadata, input_file, output_file, ...)
  7.           └─rmarkdown:::process_images(output_str, image_relative)
  8.             └─rmarkdown:::process_html_res(...)
  9.               └─stringr::str_replace_all(html, reg, process_img_src)
 10.                 └─stringr:::str_transform_all(string, pattern, replacement) at stringr/R/replace.r:91:4
 11.                   └─rmarkdown (local) replacement(old_flat) at stringr/R/replace.r:201:4
 12.                     └─rmarkdown (local) processor(img_src, src)

[hadley]

rmarkdown patch in rstudio/rmarkdown#2416. Breaking rmarkdown is a big deal, so I don't think we should merge this until after rmarkdown is released, so it probably won't make it for this release.

[DavisVaughan]

I guess pkgdown is failing because rmarkdown is broken? Very meta - ah yes i see that in your backtrace

[DavisVaughan]

I guess the broken packages are like rmarkdown? They didn't expect multiple strings at once?

[DavisVaughan]

Depending on how much you value readability over performance, you don't actually need to pass idx here since you are unchopping in the order the input was provided

[hadley]

In dev vctrs:

xs <- list("a", c("b", "c"), c("d", "e", "f"))

x <- list_unchop(xs)
x <- toupper(x)
x
#> [1] "A" "B" "C" "D" "E" "F"

sizes <- list_sizes(xs)
vec_partition(x, sizes)

[DavisVaughan]

💀

[DavisVaughan]

Alternate idea is to actually apply replacement() and then check if length(new_flat) == 1 && length(old_flat) == 0. In that special case you could issue a warning (possibly even mentioning paste0()) and "fix" it for them.

[hadley]

Yeah, we could do that, I just don't think it's worth the effort since we do know what the correct answer is in the 0 element case.

[hadley]

Yeah, the failures seem to be cases that expected a single string. So there's always a trivial fix, which is to vectorise with vapply() or similar. When I do the revdep fixes, I'll try and vectorise properly so they can benefit from the performance boost.

[hadley]

Fixed in rmarkdown 2.17, but I think this will have to wait until the next release since otherwise installing stringr will break older (i.e. most) rmarkdown installed.