allow escaping via env variable, document and test

Author: lorenzwalthert

.github/workflows/R-CMD-check.yaml

@@ -30,6 +30,7 @@ jobs:
 
     env:
       R_REMOTES_NO_ERRORS_FROM_WARNINGS: true
+      R_STYLER_FUTURE_NO_OVERRIDE: true
       RSPM: ${{ matrix.config.rspm }}
       GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}
       _R_CHECK_FORCE_SUGGESTS_: false

.github/workflows/benchmarking.yaml

@@ -18,6 +18,7 @@ jobs:
 
     env:
       R_REMOTES_NO_ERRORS_FROM_WARNINGS: true
+      R_STYLER_FUTURE_NO_OVERRIDE: true
       RSPM: ${{ matrix.config.rspm }}
       GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}
     steps:

.github/workflows/test-coverage.yaml

@@ -13,6 +13,7 @@ jobs:
     runs-on: macOS-latest
     env:
       GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}
+      R_STYLER_FUTURE_NO_OVERRIDE: true
     steps:
       - uses: actions/checkout@v2
 

DESCRIPTION

@@ -57,6 +57,7 @@ Collate:
     'detect-alignment.R'
     'environments.R'
     'expr-is.R'
+    'future.R'
     'indent.R'
     'initialize.R'
     'io.R'

NEWS.md

@@ -16,6 +16,10 @@
 
 ## Major changes
 
+- `style_file()`, `style_dir()` and `style_pkg()` now process input files in 
+  parallel and display process bars if `{furrr}` is installed. This feature is
+  experimental, please see `help(styler_future, package = "styler")` for 
+  details. 
 - blank lines in function calls and headers are now removed, for the former only 
   when there are no comments before or after the blank line (#629, #630, #635).
 - speed improvements: (~10%) when cache is activated because transformers are not 

R/future.R

@@ -0,0 +1,20 @@
+#' Parallelization of styling
+#'
+#' [style_file()], [style_pkg()] and [style_dir()] leverage the `{future}`
+#' framework for parallelization. To make use of parallel processing, you need
+#' to have the package `{furrr}` installed. In that case, the strategy
+#' [future::multiprocess()] is set temporarily during styling (only if there is
+#' more than two files to style, because of start-up costs of parallelization).
+#' You can prevent this temporary change in the future strategy by setting the
+#' environment variable `R_STYLER_FUTURE_NO_OVERRIDE` to `TRUE` (case ignored). In
+#' that case, the future strategy set before calling styler will be used, if
+#' you have not set any, the future `{framework}` falls back to
+#' [future::sequential()].
+#'
+#' @section Life-cycle:
+#' The parallelization feature is experimental, in particular how its governed
+#' with `R_STYLER_FUTURE_NO_OVERRIDE` and how progress bars are displayed with
+#' `{furrr}`. In the future, most likely the `{progressr}` package will be used
+#' to handle progress bars.
+#' @name styler_future
+NULL

R/transform-files.R

@@ -17,21 +17,33 @@ transform_files <- function(files, transformers, include_roxygen_examples, base_
   max_char <- min(max(nchar(files), 0), getOption("width"))
   len_files <- length(files)
   if (len_files > 0L && !getOption("styler.quiet", FALSE)) {
-    cat("Styling ", len_files, " files:\n")
+    cat("Styling ", len_files, " files:\n") # TODO should only have one space between words
   }
 
-  if (!identical(Sys.getenv("TESTTHAT"), "true") && rlang::is_installed("furrr")) {
-    if (inherits(future::plan(), "uniprocess")) {
+  future_strategy_no_override <- tolower(Sys.getenv("R_STYLER_FUTURE_NO_OVERRIDE", "FALSE")) == "true"
+  if (rlang::is_installed("furrr")) {
+    # only parallelise when furrr is installed
+    if (!future_strategy_no_override && length(files) > 2) {
+      # only override when env not set and more than two files.
       local_options(future.supportsMulticore.unstable = "quiet")
-      oplan <- future::plan("multiprocess")
-      on.exit(future::plan(oplan), add = TRUE)
-    }
+      oplan <- future::plan("multisession") # not sure we should use
 
+      # withr::defer() gives C stack overflow on macOS, on.exit not.
+      on.exit(future::plan(oplan), add = TRUE, after = FALSE)
+    }
     changed <- furrr::future_map_lgl(files, transform_file,
       fun = transformer, max_char_path = max_char, dry = dry,
       .progress = TRUE
     )
   } else {
+    if (future_strategy_no_override) {
+      rlang::warn(paste(
+        "Environment variable `R_STYLER_FUTURE_NO_OVERRIDE` is not respected",
+        "because package {furrr} is not installed. Falling back to ",
+        "`purrr::map_lgl()` to iterate over files to style instead of",
+        "`furrr::future_map_lgl()`."
+      ))
+    }
     changed <- map_lgl(files, transform_file,
       fun = transformer, max_char_path = max_char, dry = dry
     )

inst/WORDLIST

@@ -54,6 +54,7 @@ fileext
 filetype
 forcond
 funct
+furrr
 gcc
 getChecksum
 getOption
@@ -91,6 +92,7 @@ macOS
 magrittr
 md
 Müller
+multiprocess
 mutli
 navbar
 netlify
@@ -101,6 +103,8 @@ ourself
 packagemanager
 packrat
 pandoc
+parallelization
+Parallelization
 params
 parsable
 parsesum
@@ -113,6 +117,7 @@ pre
 precommit
 prefill
 prettycode
+progressr
 PRs
 purrr
 rcmdcheck

man/styler_future.Rd

@@ -0,0 +1,53 @@
+path_rel <- c(
+  "dirty-sample-with-scope-tokens.R",
+  "dirty-sample-with-scope-spaces.R",
+  "clean-sample-with-scope-tokens.R"
+)
+
+path_dir <- file.path(
+    "public-api", "xyzdir-dirty", path_rel
+)
+
+test_that("can style in parallel when no strategy is set and overriding is deactivated", {
+  withr::local_envvar("R_STYLER_FUTURE_NO_OVERRIDE" = "TRUE")
+  expect_match(
+    catch_style_file_output(path_dir),
+    'Styling +3',
+    all = FALSE
+  )
+})
+
+test_that("can style in parallel when no strategy is set and overriding is not deactivated", {
+  withr::local_envvar("R_STYLER_FUTURE_NO_OVERRIDE" = "FALSE")
+  expect_match(
+    catch_style_file_output(path_dir),
+    'Styling +3',
+    all = FALSE
+  )
+})
+
+test_that("can style in parallel when no strategy is set", {
+
+  expect_match(
+    catch_style_file_output(path_dir),
+    'Styling +3',
+    all = FALSE
+  )
+})
+
+test_that("can style in parallel when strategy is set", {
+  future::plan('sequential')
+  expect_match(
+    catch_style_file_output(path_dir),
+    'Styling +3',
+    all = FALSE
+  )
+})
+
+test_that("with fewer than two files, the plan is not modified", {
+  expect_match(
+    catch_style_file_output(path_dir[1]),
+    'Styling +1',
+    all = FALSE
+  )
+})