From 938c9f95c88a4169dd7b10575155e8c5792fdca4 Mon Sep 17 00:00:00 2001 From: lorenzwalthert Date: Thu, 15 Aug 2019 09:16:30 +0200 Subject: [PATCH 01/67] implement basic caching --- DESCRIPTION | 4 +- R/transform-files.R | 38 +++++++++++++----- R/ui.R | 42 ++++++++++++++++++++ R/utils-cache.R | 29 ++++++++++++++ R/zzz.R | 3 +- man/hash_standardize.Rd | 17 ++++++++ tests/testthat/helpers-devel-options.R | 3 ++ tests/testthat/reference-objects/caching.R | 45 ++++++++++++++++++++++ tests/testthat/test-cache.R | 21 ++++++++++ 9 files changed, 191 insertions(+), 11 deletions(-) create mode 100644 R/utils-cache.R create mode 100644 man/hash_standardize.Rd create mode 100644 tests/testthat/helpers-devel-options.R create mode 100644 tests/testthat/reference-objects/caching.R create mode 100644 tests/testthat/test-cache.R diff --git a/DESCRIPTION b/DESCRIPTION index 3a78d9b9c..2d353dc6d 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -36,7 +36,8 @@ Suggests: prettycode, rmarkdown, rstudioapi (>= 0.7), - testthat (>= 2.1.0) + testthat (>= 2.1.0), + R.cache VignetteBuilder: knitr Encoding: UTF-8 LazyData: true @@ -81,6 +82,7 @@ Collate: 'transform-files.R' 'ui.R' 'unindent.R' + 'utils-cache.R' 'utils-files.R' 'utils-navigate-nest.R' 'utils-strings.R' diff --git a/R/transform-files.R b/R/transform-files.R index 5b1c9788c..a28506cd4 100644 --- a/R/transform-files.R +++ b/R/transform-files.R @@ -72,17 +72,37 @@ transform_file <- function(path, #' @inheritParams parse_transform_serialize_r #' @keywords internal #' @importFrom purrr when -make_transformer <- function(transformers, include_roxygen_examples, warn_empty = TRUE) { +make_transformer <- function(transformers, + include_roxygen_examples, + warn_empty = TRUE) { force(transformers) + cache_dir <- c("styler", get_cache_subdir()) + function(text) { - transformed_code <- text %>% - parse_transform_serialize_r(transformers, warn_empty = warn_empty) %>% - when( - include_roxygen_examples ~ - parse_transform_serialize_roxygen(., transformers), - ~. - ) - transformed_code + is_cached <- !is.null( + R.cache::loadCache(key = hash_standardize(text), dir = cache_dir) + ) + should_use_cache <- getOption("styler.use_cache", FALSE) + can_use_cache <- is_cached && should_use_cache + if (!can_use_cache) { + transformed_code <- text %>% + parse_transform_serialize_r(transformers, warn_empty = warn_empty) %>% + when( + include_roxygen_examples ~ + parse_transform_serialize_roxygen(., transformers), + ~. + ) + if (should_use_cache) { + R.cache::saveCache( + Sys.time(), + key = hash_standardize(transformed_code), + dir = cache_dir + ) + } + transformed_code + } else { + text + } } } diff --git a/R/ui.R b/R/ui.R index 02d2a84c3..4da377fb5 100644 --- a/R/ui.R +++ b/R/ui.R @@ -242,3 +242,45 @@ style_file <- function(path, changed <- transform_files(path, transformers, include_roxygen_examples) invisible(changed) } + +#' Clear the cache +#' +#' Clears the cache that stores which files' styling is up-to-date. You won't be +#' able to undo this. Note that the file corresponding to the cache won't be +#' deleted. +#' If the cache is used at all or not is determined via the R option +#' `styler.use_cache`. +#' @param cache_subdir Each version of styler has it's own cache, because +#' styling is potentially different with different versions of styler. If +#' `cache_subdir` is `NULL`, the option "styler.cache_subdir" is considered. +#' If unset, the value is resolved the currently installed version of styler. +#' @param ask Whether or not to interactively ask the user again. +#' @family cache managers +#' @export +cache_clear <- function(cache_subdir = NULL, ask = TRUE) { + path_cache <- cache_find_path(cache_subdir) + R.cache::clearCache(path_cache, prompt = ask) +} + +#' Show information about the styler cache +#' +#' If the cache is used at all or not is determined via the R option +#' `styler.use_cache`. +#' @inheritParams cache_delete +#' @param format Either "lucid" for a printed summary or "tabular" for a +#' tabular summary. +#' @export +cache_info <- function(cache_subdir = NULL, format = "lucid") { + rlang::arg_match(format, c("tabular", "lucid")) + path_cache <- cache_find_path(cache_subdir) + tbl <- fs::file_info(path_cache) + if (format == "lucid") { + cat( + "Size:\t\t", tbl$size, " bytes\nLast modified:\t", + as.character(tbl$modification_time), "\nCreated:\t", + as.character(tbl$birth_time), "\nLocation:\t", tbl$path + ) + } else { + tbl + } +} diff --git a/R/utils-cache.R b/R/utils-cache.R new file mode 100644 index 000000000..74bb48d00 --- /dev/null +++ b/R/utils-cache.R @@ -0,0 +1,29 @@ +#' Make sure text after styling results in the same hash as text before styling +#' if it is indeed identical. +#' @param x A character vector. +#' @keywords internal +hash_standardize <- function(x) { + x <- ensure_last_is_empty(x) + Encoding(x) <- "UTF-8" + list(x) +} + +#' Where is the cache +#' +#' Finds the path to the cache and creates it if it does not exist +#' @param cache_subdir The subdir of the cache. Is equivalent to the +#' R.cache subdir when adding "styler" as a parent directory to +#' `cache_subdir`. +cache_find_path <- function(cache_subdir = NULL) { + if (is.null(cache_subdir)) { + cache_subdir <- get_cache_subdir() + } + R.cache::getCachePath(c("styler", cache_subdir)) +} + +get_cache_subdir <- function() { + getOption( + "styler.cache_subdir", + packageDescription("styler", fields = "Version") + ) +} diff --git a/R/zzz.R b/R/zzz.R index 10b074fc4..1d36beb87 100644 --- a/R/zzz.R +++ b/R/zzz.R @@ -2,7 +2,8 @@ backports::import(pkgname, "trimws") op <- options() op.styler <- list( - styler.colored_print.vertical = TRUE + styler.colored_print.vertical = TRUE, + styler.use_cache = TRUE ) toset <- !(names(op.styler) %in% names(op)) if (any(toset)) options(op.styler[toset]) diff --git a/man/hash_standardize.Rd b/man/hash_standardize.Rd new file mode 100644 index 000000000..546dc1300 --- /dev/null +++ b/man/hash_standardize.Rd @@ -0,0 +1,17 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/utils-cache.R +\name{hash_standardize} +\alias{hash_standardize} +\title{Make sure text after styling results in the same hash as text before styling +if it is indeed identical.} +\usage{ +hash_standardize(x) +} +\arguments{ +\item{x}{A character vector.} +} +\description{ +Make sure text after styling results in the same hash as text before styling +if it is indeed identical. +} +\keyword{internal} diff --git a/tests/testthat/helpers-devel-options.R b/tests/testthat/helpers-devel-options.R new file mode 100644 index 000000000..c1b1fcf51 --- /dev/null +++ b/tests/testthat/helpers-devel-options.R @@ -0,0 +1,3 @@ +options(styler.use_cache = TRUE) +options(styler.cache_subdir = "testthat") +cat("Setting options `styler.cache_subdir` as well as `styler.use_cache` from tests/testthat/helpers-devel-options.R") diff --git a/tests/testthat/reference-objects/caching.R b/tests/testthat/reference-objects/caching.R new file mode 100644 index 000000000..8e92f97cd --- /dev/null +++ b/tests/testthat/reference-objects/caching.R @@ -0,0 +1,45 @@ +#' Style `.R` and/or `.Rmd` files +#' +#' Performs various substitutions in the files specified. +#' Carefully examine the results after running this function! +#' @param path A character vector with paths to files to style. +#' @inheritParams style_pkg +#' @inheritSection transform_files Value +#' @inheritSection style_pkg Warning +#' @inheritSection style_pkg Roundtrip Validation +#' @examples +#' # the following is identical but the former is more convenient: +#' file <- tempfile("styler", +#' fileext = ".R" +#' ) +#' \dontrun{ +#' xfun::write_utf8("1++1", file) +#' } +#' style_file( +#' file, +#' style = tidyverse_style, strict = TRUE +#' ) +#' style_file(file, transformers = tidyverse_style(strict = TRUE)) +#' xfun::read_utf8(file) +#' \dontrun{ +#' unlink(file2) +#' } +#' \dontrun{ +#' { +#' x +#' } +#' unlink(file2) +#' } +#' @family stylers +#' @export +style_file <- function(path, + ..., + style = tidyverse_style, + transformers = style(...), + include_roxygen_examples = TRUE) { + changed <- withr::with_dir( + dirname(path), + transform_files(basename(path), transformers) + ) + invisible(changed) +} diff --git a/tests/testthat/test-cache.R b/tests/testthat/test-cache.R new file mode 100644 index 000000000..6bbc807a2 --- /dev/null +++ b/tests/testthat/test-cache.R @@ -0,0 +1,21 @@ +test_that("activated cache brings speedup", { + withr::with_options( + list("styler.use_cache" = TRUE, "styler.cache_subdir" = "testthat"), { + cache_clear(ask = FALSE) + first <- system.time(styler::style_file(test_path("reference-objects/caching.R"))) + second <- system.time(styler::style_file(test_path("reference-objects/caching.R"))) + expect_true(first["elapsed"] / 2 > second["elapsed"]) + } + ) +}) + +test_that("unactivated cache does not bring speedup", { + withr::with_options( + list("styler.use_cache" = FALSE, "styler.cache_subdir" = "testthat"), { + cache_clear(ask = FALSE) + first <- system.time(styler::style_file(test_path("reference-objects/caching.R"))) + second <- system.time(styler::style_file(test_path("reference-objects/caching.R"))) + expect_false(first["elapsed"] / 2 > second["elapsed"]) + } + ) +}) From 94c48ae181f349e2a90e2be63fc874efb3fd33ca Mon Sep 17 00:00:00 2001 From: lorenzwalthert Date: Thu, 15 Aug 2019 09:18:16 +0200 Subject: [PATCH 02/67] cache administration tools --- API | 2 ++ NAMESPACE | 2 ++ R/utils-cache.R | 1 + man/cache_clear.Rd | 24 ++++++++++++++++++++++++ man/cache_find_path.Rd | 17 +++++++++++++++++ man/cache_info.Rd | 16 ++++++++++++++++ 6 files changed, 62 insertions(+) create mode 100644 man/cache_clear.Rd create mode 100644 man/cache_find_path.Rd create mode 100644 man/cache_info.Rd diff --git a/API b/API index 072513601..18cc6c523 100644 --- a/API +++ b/API @@ -2,6 +2,8 @@ ## Exported functions +cache_clear(cache_subdir = NULL, ask = TRUE) +cache_info(cache_subdir = NULL, format = "lucid") create_style_guide(initialize = default_style_guide_attributes, line_break = NULL, space = NULL, token = NULL, indention = NULL, use_raw_indention = FALSE, reindention = tidyverse_reindention()) default_style_guide_attributes(pd_flat) specify_math_token_spacing(zero = "'^'", one = c("'+'", "'-'", "'*'", "'/'")) diff --git a/NAMESPACE b/NAMESPACE index 74a303c2f..8e9a023cc 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -1,6 +1,8 @@ # Generated by roxygen2: do not edit by hand S3method(print,vertical) +export(cache_clear) +export(cache_info) export(create_style_guide) export(default_style_guide_attributes) export(specify_math_token_spacing) diff --git a/R/utils-cache.R b/R/utils-cache.R index 74bb48d00..8dbe13115 100644 --- a/R/utils-cache.R +++ b/R/utils-cache.R @@ -14,6 +14,7 @@ hash_standardize <- function(x) { #' @param cache_subdir The subdir of the cache. Is equivalent to the #' R.cache subdir when adding "styler" as a parent directory to #' `cache_subdir`. +#' @keywords internal cache_find_path <- function(cache_subdir = NULL) { if (is.null(cache_subdir)) { cache_subdir <- get_cache_subdir() diff --git a/man/cache_clear.Rd b/man/cache_clear.Rd new file mode 100644 index 000000000..c4b08dd1f --- /dev/null +++ b/man/cache_clear.Rd @@ -0,0 +1,24 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/ui.R +\name{cache_clear} +\alias{cache_clear} +\title{Clear the cache} +\usage{ +cache_clear(cache_subdir = NULL, ask = TRUE) +} +\arguments{ +\item{cache_subdir}{Each version of styler has it's own cache, because +styling is potentially different with different versions of styler. If +\code{cache_subdir} is \code{NULL}, the option "styler.cache_subdir" is considered. +If unset, the value is resolved the currently installed version of styler.} + +\item{ask}{Whether or not to interactively ask the user again.} +} +\description{ +Clears the cache that stores which files' styling is up-to-date. You won't be +able to undo this. Note that the file corresponding to the cache won't be +deleted. +If the cache is used at all or not is determined via the R option +\code{styler.use_cache}. +} +\concept{cache managers} diff --git a/man/cache_find_path.Rd b/man/cache_find_path.Rd new file mode 100644 index 000000000..928c5b5c4 --- /dev/null +++ b/man/cache_find_path.Rd @@ -0,0 +1,17 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/utils-cache.R +\name{cache_find_path} +\alias{cache_find_path} +\title{Where is the cache} +\usage{ +cache_find_path(cache_subdir = NULL) +} +\arguments{ +\item{cache_subdir}{The subdir of the cache. Is equivalent to the +R.cache subdir when adding "styler" as a parent directory to +\code{cache_subdir}.} +} +\description{ +Finds the path to the cache and creates it if it does not exist +} +\keyword{internal} diff --git a/man/cache_info.Rd b/man/cache_info.Rd new file mode 100644 index 000000000..070e0f7ed --- /dev/null +++ b/man/cache_info.Rd @@ -0,0 +1,16 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/ui.R +\name{cache_info} +\alias{cache_info} +\title{Show information about the styler cache} +\usage{ +cache_info(cache_subdir = NULL, format = "lucid") +} +\arguments{ +\item{format}{Either "lucid" for a printed summary or "tabular" for a +tabular summary.} +} +\description{ +If the cache is used at all or not is determined via the R option +\code{styler.use_cache}. +} From f57a7c5b876a8e8ddb4ec249a713b3119ba33d5c Mon Sep 17 00:00:00 2001 From: lorenzwalthert Date: Thu, 15 Aug 2019 19:57:07 +0200 Subject: [PATCH 03/67] set all options in .onLoad(), do not use the default in getOptions(). --- R/addins.R | 2 +- R/transform-files.R | 2 +- R/utils-cache.R | 5 +---- R/zzz.R | 4 +++- 4 files changed, 6 insertions(+), 7 deletions(-) diff --git a/R/addins.R b/R/addins.R index 367b9ce9e..aa2a38157 100644 --- a/R/addins.R +++ b/R/addins.R @@ -141,7 +141,7 @@ set_style_transformers <- function() { #' #' @keywords internal get_addins_style_transformer_name <- function() { - getOption("styler.addins_style_transformer", default = "styler::tidyverse_style()") + getOption("styler.addins_style_transformer") } #' @rdname get_addins_style_transformer_name diff --git a/R/transform-files.R b/R/transform-files.R index a28506cd4..1d7a6e4dc 100644 --- a/R/transform-files.R +++ b/R/transform-files.R @@ -82,7 +82,7 @@ make_transformer <- function(transformers, is_cached <- !is.null( R.cache::loadCache(key = hash_standardize(text), dir = cache_dir) ) - should_use_cache <- getOption("styler.use_cache", FALSE) + should_use_cache <- getOption("styler.use_cache") can_use_cache <- is_cached && should_use_cache if (!can_use_cache) { transformed_code <- text %>% diff --git a/R/utils-cache.R b/R/utils-cache.R index 8dbe13115..59fb51e1e 100644 --- a/R/utils-cache.R +++ b/R/utils-cache.R @@ -23,8 +23,5 @@ cache_find_path <- function(cache_subdir = NULL) { } get_cache_subdir <- function() { - getOption( - "styler.cache_subdir", - packageDescription("styler", fields = "Version") - ) + getOption("styler.cache_subdir") } diff --git a/R/zzz.R b/R/zzz.R index 1d36beb87..660ae3bb8 100644 --- a/R/zzz.R +++ b/R/zzz.R @@ -3,7 +3,9 @@ op <- options() op.styler <- list( styler.colored_print.vertical = TRUE, - styler.use_cache = TRUE + styler.use_cache = TRUE, + styler.cache_subdir = packageDescription("styler", fields = "Version"), + styler.addins_style_transformer = "styler::tidyverse_style()" ) toset <- !(names(op.styler) %in% names(op)) if (any(toset)) options(op.styler[toset]) From cc02ccd398121c8ad88d55af7f50bad49bb6b0d7 Mon Sep 17 00:00:00 2001 From: lorenzwalthert Date: Thu, 15 Aug 2019 20:00:25 +0200 Subject: [PATCH 04/67] don't use cache in general --- tests/testthat/helpers-devel-options.R | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/testthat/helpers-devel-options.R b/tests/testthat/helpers-devel-options.R index c1b1fcf51..271a50bde 100644 --- a/tests/testthat/helpers-devel-options.R +++ b/tests/testthat/helpers-devel-options.R @@ -1,3 +1,5 @@ -options(styler.use_cache = TRUE) -options(styler.cache_subdir = "testthat") -cat("Setting options `styler.cache_subdir` as well as `styler.use_cache` from tests/testthat/helpers-devel-options.R") +options(styler.use_cache = FALSE) +cat( + "Setting options `styler.use_cache` ", + "from tests/testthat/helpers-devel-options.R" +) From bf658be1100bb60b763f9694a37813ca597e477b Mon Sep 17 00:00:00 2001 From: lorenzwalthert Date: Thu, 15 Aug 2019 20:06:51 +0200 Subject: [PATCH 05/67] handle situation where R.cache is not available. --- R/transform-files.R | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/R/transform-files.R b/R/transform-files.R index 1d7a6e4dc..3f94fbc11 100644 --- a/R/transform-files.R +++ b/R/transform-files.R @@ -77,7 +77,14 @@ make_transformer <- function(transformers, warn_empty = TRUE) { force(transformers) cache_dir <- c("styler", get_cache_subdir()) - + if (!rlang::is_installed("R.cache") && getOption("styler.use_cache")) { + rlang::abort(paste( + "R package R.cache is not installed, which is needed when the option ", + "`styler.use_cache` is `TRUE`. Please install the package to enable the ", + "caching feature of styler or set `options(styler.use_cache = FALSE)` ", + "in your .Rprofile to use styler without that feature." + )) + } function(text) { is_cached <- !is.null( R.cache::loadCache(key = hash_standardize(text), dir = cache_dir) From f2a8d2e39907cdc7792dd7e0602428b7722f29d3 Mon Sep 17 00:00:00 2001 From: lorenzwalthert Date: Thu, 15 Aug 2019 21:26:36 +0200 Subject: [PATCH 06/67] assert that the R.cache is installed --- R/communicate.R | 16 ++++++++++++++++ R/transform-files.R | 9 +-------- R/ui.R | 4 +++- man/assert_R.cache_installation.Rd | 12 ++++++++++++ man/cache_clear.Rd | 2 +- 5 files changed, 33 insertions(+), 10 deletions(-) create mode 100644 man/assert_R.cache_installation.Rd diff --git a/R/communicate.R b/R/communicate.R index 3e9328b8a..f3aa6745a 100644 --- a/R/communicate.R +++ b/R/communicate.R @@ -34,3 +34,19 @@ assert_data.tree_installation <- function() { abort("The package data.tree needs to be installed for this functionality.") } } + +#' R.cache needs to be installed if caching functionality is enabled +#' +#' @keywords internal +assert_R.cache_installation <- function(installation_only = FALSE) { + if (!rlang::is_installed("R.cache") && + ifelse(installation_only, TRUE, getOption("styler.use_cache") + )) { + rlang::abort(paste( + "R package R.cache is not installed, which is needed when the option ", + "`styler.use_cache` is `TRUE`. Please install the package to enable the ", + "caching feature of styler or set `options(styler.use_cache = FALSE)` ", + "in your .Rprofile to use styler without that feature." + )) + } +} diff --git a/R/transform-files.R b/R/transform-files.R index 3f94fbc11..f4801c06d 100644 --- a/R/transform-files.R +++ b/R/transform-files.R @@ -77,14 +77,7 @@ make_transformer <- function(transformers, warn_empty = TRUE) { force(transformers) cache_dir <- c("styler", get_cache_subdir()) - if (!rlang::is_installed("R.cache") && getOption("styler.use_cache")) { - rlang::abort(paste( - "R package R.cache is not installed, which is needed when the option ", - "`styler.use_cache` is `TRUE`. Please install the package to enable the ", - "caching feature of styler or set `options(styler.use_cache = FALSE)` ", - "in your .Rprofile to use styler without that feature." - )) - } + assert_R.cache_installation() function(text) { is_cached <- !is.null( R.cache::loadCache(key = hash_standardize(text), dir = cache_dir) diff --git a/R/ui.R b/R/ui.R index 4da377fb5..034c0f264 100644 --- a/R/ui.R +++ b/R/ui.R @@ -249,7 +249,7 @@ style_file <- function(path, #' able to undo this. Note that the file corresponding to the cache won't be #' deleted. #' If the cache is used at all or not is determined via the R option -#' `styler.use_cache`. +#' `styler.use_cache`, defaulting to `TRUE`. #' @param cache_subdir Each version of styler has it's own cache, because #' styling is potentially different with different versions of styler. If #' `cache_subdir` is `NULL`, the option "styler.cache_subdir" is considered. @@ -258,6 +258,7 @@ style_file <- function(path, #' @family cache managers #' @export cache_clear <- function(cache_subdir = NULL, ask = TRUE) { + assert_R.cache_installation(installation_only = TRUE) path_cache <- cache_find_path(cache_subdir) R.cache::clearCache(path_cache, prompt = ask) } @@ -271,6 +272,7 @@ cache_clear <- function(cache_subdir = NULL, ask = TRUE) { #' tabular summary. #' @export cache_info <- function(cache_subdir = NULL, format = "lucid") { + assert_R.cache_installation(installation_only = TRUE) rlang::arg_match(format, c("tabular", "lucid")) path_cache <- cache_find_path(cache_subdir) tbl <- fs::file_info(path_cache) diff --git a/man/assert_R.cache_installation.Rd b/man/assert_R.cache_installation.Rd new file mode 100644 index 000000000..1c166abb9 --- /dev/null +++ b/man/assert_R.cache_installation.Rd @@ -0,0 +1,12 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/communicate.R +\name{assert_R.cache_installation} +\alias{assert_R.cache_installation} +\title{R.cache needs to be installed if caching functionality is enabled} +\usage{ +assert_R.cache_installation(installation_only = FALSE) +} +\description{ +R.cache needs to be installed if caching functionality is enabled +} +\keyword{internal} diff --git a/man/cache_clear.Rd b/man/cache_clear.Rd index c4b08dd1f..0cd0a101f 100644 --- a/man/cache_clear.Rd +++ b/man/cache_clear.Rd @@ -19,6 +19,6 @@ Clears the cache that stores which files' styling is up-to-date. You won't be able to undo this. Note that the file corresponding to the cache won't be deleted. If the cache is used at all or not is determined via the R option -\code{styler.use_cache}. +\code{styler.use_cache}, defaulting to \code{TRUE}. } \concept{cache managers} From fc9ec71a2f95c9a4cb1e141b0cea0e2dacfe8898 Mon Sep 17 00:00:00 2001 From: lorenzwalthert Date: Fri, 16 Aug 2019 00:00:13 +0200 Subject: [PATCH 07/67] drop fs dependency. --- R/ui.R | 8 ++++---- R/utils-files.R | 5 +++++ man/cache_info.Rd | 2 +- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/R/ui.R b/R/ui.R index 034c0f264..528702f67 100644 --- a/R/ui.R +++ b/R/ui.R @@ -269,18 +269,18 @@ cache_clear <- function(cache_subdir = NULL, ask = TRUE) { #' `styler.use_cache`. #' @inheritParams cache_delete #' @param format Either "lucid" for a printed summary or "tabular" for a -#' tabular summary. +#' tabular summary from [base::file.info()]. #' @export cache_info <- function(cache_subdir = NULL, format = "lucid") { assert_R.cache_installation(installation_only = TRUE) rlang::arg_match(format, c("tabular", "lucid")) path_cache <- cache_find_path(cache_subdir) - tbl <- fs::file_info(path_cache) + tbl <- file_info(path_cache) if (format == "lucid") { cat( "Size:\t\t", tbl$size, " bytes\nLast modified:\t", - as.character(tbl$modification_time), "\nCreated:\t", - as.character(tbl$birth_time), "\nLocation:\t", tbl$path + as.character(tbl$mtime), "\nCreated:\t", + as.character(tbl$ctime), "\nLocation:\t", path_cache ) } else { tbl diff --git a/R/utils-files.R b/R/utils-files.R index d1c0c6db4..7e326775d 100644 --- a/R/utils-files.R +++ b/R/utils-files.R @@ -26,3 +26,8 @@ is_unsaved_file <- function(path) { map_filetype_to_pattern <- function(filetype) { paste0("(", paste(set_and_assert_arg_filetype(filetype), collapse = "|"), ")$") } + +file_info <- function(path) { + file.info(path) %>% + as_tibble() +} diff --git a/man/cache_info.Rd b/man/cache_info.Rd index 070e0f7ed..916fada6c 100644 --- a/man/cache_info.Rd +++ b/man/cache_info.Rd @@ -8,7 +8,7 @@ cache_info(cache_subdir = NULL, format = "lucid") } \arguments{ \item{format}{Either "lucid" for a printed summary or "tabular" for a -tabular summary.} +tabular summary from \code{\link[base:file.info]{base::file.info()}}.} } \description{ If the cache is used at all or not is determined via the R option From 4ceb7c5246360092c2348567436fed4baaef4814 Mon Sep 17 00:00:00 2001 From: lorenzwalthert Date: Fri, 16 Aug 2019 00:01:22 +0200 Subject: [PATCH 08/67] fix naming. --- R/ui.R | 2 +- man/cache_info.Rd | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/R/ui.R b/R/ui.R index 528702f67..298f601dd 100644 --- a/R/ui.R +++ b/R/ui.R @@ -267,7 +267,7 @@ cache_clear <- function(cache_subdir = NULL, ask = TRUE) { #' #' If the cache is used at all or not is determined via the R option #' `styler.use_cache`. -#' @inheritParams cache_delete +#' @inheritParams cache_clear #' @param format Either "lucid" for a printed summary or "tabular" for a #' tabular summary from [base::file.info()]. #' @export diff --git a/man/cache_info.Rd b/man/cache_info.Rd index 916fada6c..1e11c24d2 100644 --- a/man/cache_info.Rd +++ b/man/cache_info.Rd @@ -7,6 +7,11 @@ cache_info(cache_subdir = NULL, format = "lucid") } \arguments{ +\item{cache_subdir}{Each version of styler has it's own cache, because +styling is potentially different with different versions of styler. If +\code{cache_subdir} is \code{NULL}, the option "styler.cache_subdir" is considered. +If unset, the value is resolved the currently installed version of styler.} + \item{format}{Either "lucid" for a printed summary or "tabular" for a tabular summary from \code{\link[base:file.info]{base::file.info()}}.} } From 59168c21b09253f84954304b1dd59c18b0772ba6 Mon Sep 17 00:00:00 2001 From: lorenzwalthert Date: Fri, 16 Aug 2019 00:03:34 +0200 Subject: [PATCH 09/67] fix R cmd check --- R/io.R | 2 +- R/zzz.R | 2 +- man/invalid_utf8.Rd | 3 +++ 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/R/io.R b/R/io.R index 8b9ed49b4..495650ca5 100644 --- a/R/io.R +++ b/R/io.R @@ -80,7 +80,7 @@ read_utf8_bare <- function(con, warn = TRUE) { x } -#' Drop-in replacement for `xfun:::invalid_utf8()`. +#' Drop-in replacement for [xfun:::invalid_utf8()] #' @keywords internal invalid_utf8 <- function(x) { which(!is.na(x) & is.na(iconv(x, "UTF-8", "UTF-8"))) diff --git a/R/zzz.R b/R/zzz.R index 660ae3bb8..ecc03353e 100644 --- a/R/zzz.R +++ b/R/zzz.R @@ -4,7 +4,7 @@ op.styler <- list( styler.colored_print.vertical = TRUE, styler.use_cache = TRUE, - styler.cache_subdir = packageDescription("styler", fields = "Version"), + styler.cache_subdir = utils::packageDescription("styler", fields = "Version"), styler.addins_style_transformer = "styler::tidyverse_style()" ) toset <- !(names(op.styler) %in% names(op)) diff --git a/man/invalid_utf8.Rd b/man/invalid_utf8.Rd index 9c9479add..4f2c74224 100644 --- a/man/invalid_utf8.Rd +++ b/man/invalid_utf8.Rd @@ -6,6 +6,9 @@ \usage{ invalid_utf8(x) } +\arguments{ +\item{x}{A character vector.} +} \description{ Drop-in replacement for \code{xfun:::invalid_utf8()}. } From 219a73c6441f9d8d694fc93ad572250c4b04c6a5 Mon Sep 17 00:00:00 2001 From: lorenzwalthert Date: Fri, 16 Aug 2019 14:04:01 +0200 Subject: [PATCH 10/67] res-style with stylermd --- README.Rmd | 23 +++++++++++++++++++---- README.md | 15 +++++++++++++++ 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/README.Rmd b/README.Rmd index 5ababb0c2..1a882d7d2 100644 --- a/README.Rmd +++ b/README.Rmd @@ -58,8 +58,11 @@ style_text(ugly_code) There are a few variants of `style_text()`: * `style_file()` styles .R and/or .Rmd files. + * `style_dir()` styles all .R and/or .Rmd files in a directory. + * `style_pkg()` styles the source files of an R package. + * RStudio Addins for styling the active file, styling the current package and styling the highlighted code region. @@ -75,8 +78,11 @@ You can decide on the level of invasiveness with the scope argument. You can style: * just spaces. + * spaces and indention. + * spaces, indention and line breaks. + * spaces, indention, line breaks and tokens. ```{r} @@ -87,6 +93,7 @@ style_text(ugly_code, scope = "spaces") Note that compared to the default used above `scope = "tokens"`: * no line breaks were added. + * `<-` was not replaced with `=`. While spaces still got styled (around `=` in `(x)`). @@ -112,23 +119,28 @@ not flexible enough for you, you can implement your own style guide, as explained in the corresponding [vignette](https://styler.r-lib.org/articles/customizing_styler.html). + ## Adaption of styler styler functionality is made available through other packages, most notably * `usethis::use_tidy_style()` styles your project according to the tidyverse style guide. + * `reprex::reprex(style = TRUE)` to prettify reprex code before printing. To - permanently use `style = TRUE` without specifying it every time, you can add the - following line to your `.Rprofile` (via `usethis::edit_r_profile()`): + permanently use `style = TRUE` without specifying it every time, you can add + the following line to your `.Rprofile` (via `usethis::edit_r_profile()`): `options(reprex.styler = TRUE)`. + * you can pretty-print your R code in RMarkdown reports without having styler modifying the source. This feature is implemented as a code chunk option in knitr. use `tidy = "styler"` in the header of a code chunks (e.g. ` ```{r name-of-the-chunk, tidy = "styler"}`), or `knitr::opts_chunk$set(tidy = "styler")` at the top of your RMarkdown script. + * as a pre-commit hook `style-files` in https://github.com/lorenzwalthert/pre-commit-hooks. + * pretty-printing of [drake](https://github.com/ropensci/drake) workflow data frames with `drake::drake_plan_source()`. * Adding styler as a fixer to the @@ -142,12 +154,15 @@ styler functionality is made available through other packages, most notably * The official [web documentation](https://styler.r-lib.org/) of styler, containing various vignettes function documentation as well as a change-log. + * [Blog post](https://lorenzwalthert.netlify.com/post/customizing-styler-the-quick-way/) about how you can customize styler without being an expert. + * A [tidyverse.org blog - post](https://www.tidyverse.org/articles/2017/12/styler-1.0.0/) introducing the - functionality of styler. + post](https://www.tidyverse.org/articles/2017/12/styler-1.0.0/) introducing + the functionality of styler. + * The wiki of [Google Summer of Code 2017](https://github.com/rstats-gsoc/gsoc2017/wiki/Noninvasive-source-code-formatting) or the [pkgdown](https://r-lib.github.io/styler/) page contain information diff --git a/README.md b/README.md index 4f676f206..fe21f1ab3 100644 --- a/README.md +++ b/README.md @@ -48,8 +48,11 @@ style_text(ugly_code) There are a few variants of `style_text()`: - `style_file()` styles .R and/or .Rmd files. + - `style_dir()` styles all .R and/or .Rmd files in a directory. + - `style_pkg()` styles the source files of an R package. + - RStudio Addins for styling the active file, styling the current package and styling the highlighted code region. @@ -64,8 +67,11 @@ You can decide on the level of invasiveness with the scope argument. You can style: - just spaces. + - spaces and indention. + - spaces, indention and line breaks. + - spaces, indention, line breaks and tokens. @@ -79,6 +85,7 @@ style_text(ugly_code, scope = "spaces") Note that compared to the default used above `scope = "tokens"`: - no line breaks were added. + - `<-` was not replaced with `=`. While spaces still got styled (around `=` in `(x)`). @@ -113,20 +120,25 @@ notably - `usethis::use_tidy_style()` styles your project according to the tidyverse style guide. + - `reprex::reprex(style = TRUE)` to prettify reprex code before printing. To permanently use `style = TRUE` without specifying it every time, you can add the following line to your `.Rprofile` (via `usethis::edit_r_profile()`): `options(reprex.styler = TRUE)`. + - you can pretty-print your R code in RMarkdown reports without having styler modifying the source. This feature is implemented as a code chunk option in knitr. use `tidy = "styler"` in the header of a code chunks (e.g. ` ```{r name-of-the-chunk, tidy = "styler"}`), or `knitr::opts_chunk$set(tidy = "styler")` at the top of your RMarkdown script. + - as a pre-commit hook `style-files` in . + - pretty-printing of [drake](https://github.com/ropensci/drake) workflow data frames with `drake::drake_plan_source()`. + - Adding styler as a fixer to the [ale Plug-in](https://github.com/w0rp/ale/pull/2401#issuecomment-485942966) for VIM. @@ -139,12 +151,15 @@ notably - The official [web documentation](https://styler.r-lib.org/) of styler, containing various vignettes function documentation as well as a change-log. + - [Blog post](https://lorenzwalthert.netlify.com/post/customizing-styler-the-quick-way/) about how you can customize styler without being an expert. + - A [tidyverse.org blog post](https://www.tidyverse.org/articles/2017/12/styler-1.0.0/) introducing the functionality of styler. + - The wiki of [Google Summer of Code 2017](https://github.com/rstats-gsoc/gsoc2017/wiki/Noninvasive-source-code-formatting) or the [pkgdown](https://r-lib.github.io/styler/) page contain From b721904cd38749c1649d316306eceded709d12c9 Mon Sep 17 00:00:00 2001 From: lorenzwalthert Date: Fri, 16 Aug 2019 14:08:14 +0200 Subject: [PATCH 11/67] add more cache manipulators. --- API | 1 + NAMESPACE | 1 + R/ui.R | 53 +++++++++++++++++++++++++++++++++++------- R/utils-cache.R | 4 ++-- man/cache_activate.Rd | 25 ++++++++++++++++++++ man/cache_clear.Rd | 13 +++++++---- man/cache_find_path.Rd | 4 ++-- man/cache_info.Rd | 9 +++++-- 8 files changed, 92 insertions(+), 18 deletions(-) create mode 100644 man/cache_activate.Rd diff --git a/API b/API index 18cc6c523..1ea161a4f 100644 --- a/API +++ b/API @@ -2,6 +2,7 @@ ## Exported functions +cache_activate(cache_subdir = NULL) cache_clear(cache_subdir = NULL, ask = TRUE) cache_info(cache_subdir = NULL, format = "lucid") create_style_guide(initialize = default_style_guide_attributes, line_break = NULL, space = NULL, token = NULL, indention = NULL, use_raw_indention = FALSE, reindention = tidyverse_reindention()) diff --git a/NAMESPACE b/NAMESPACE index 8e9a023cc..cec420c57 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -1,6 +1,7 @@ # Generated by roxygen2: do not edit by hand S3method(print,vertical) +export(cache_activate) export(cache_clear) export(cache_info) export(create_style_guide) diff --git a/R/ui.R b/R/ui.R index 298f601dd..ab657e64a 100644 --- a/R/ui.R +++ b/R/ui.R @@ -246,14 +246,15 @@ style_file <- function(path, #' Clear the cache #' #' Clears the cache that stores which files' styling is up-to-date. You won't be -#' able to undo this. Note that the file corresponding to the cache won't be -#' deleted. +#' able to undo this. Note that the file corresponding to the cache (a folder +#' on our file stystem) won't be deleted, but it will be empty after calling +#' `cache_clear`. #' If the cache is used at all or not is determined via the R option #' `styler.use_cache`, defaulting to `TRUE`. #' @param cache_subdir Each version of styler has it's own cache, because #' styling is potentially different with different versions of styler. If -#' `cache_subdir` is `NULL`, the option "styler.cache_subdir" is considered. -#' If unset, the value is resolved the currently installed version of styler. +#' `cache_subdir` is `NULL`, the option "styler.cache_subdir" is considered +#' which defaults to the version of styler used. #' @param ask Whether or not to interactively ask the user again. #' @family cache managers #' @export @@ -270,19 +271,55 @@ cache_clear <- function(cache_subdir = NULL, ask = TRUE) { #' @inheritParams cache_clear #' @param format Either "lucid" for a printed summary or "tabular" for a #' tabular summary from [base::file.info()]. +#' @family cache managers #' @export cache_info <- function(cache_subdir = NULL, format = "lucid") { assert_R.cache_installation(installation_only = TRUE) rlang::arg_match(format, c("tabular", "lucid")) path_cache <- cache_find_path(cache_subdir) - tbl <- file_info(path_cache) + files <- list.files(path_cache, full.names = TRUE) + file_info <- file_info(files) + tbl <- tibble( + n = nrow(file_info), + size = sum(file_info$size), + last_modified = suppressWarnings(max(file_info$mtime)), + created = file.info(path_cache)$ctime + ) if (format == "lucid") { cat( - "Size:\t\t", tbl$size, " bytes\nLast modified:\t", - as.character(tbl$mtime), "\nCreated:\t", - as.character(tbl$ctime), "\nLocation:\t", path_cache + "Size:\t\t", tbl$size, " bytes (", tbl$n, " cached expressions)", + "\nLast modified:\t", as.character(tbl$last_modified), + "\nCreated:\t", as.character(tbl$created), + "\nLocation:\t", path_cache, + "\nActivated:\t", getOption("styler.use_cache"), + sep = "" ) } else { tbl } } + +#' Activate or deactivate the styler cache +#' +#' Helper functions to control the behavior of caching. Simple wrappers around +#' [base::options()]. +#' @param cache_subdir The sub-directory you want to use under the R.cache +#' location. If `NULL`, the option "styler.cache_subdir" is considered +#' which defaults to the version of styler used. +#' @family cache managers +#' @export +cache_activate <- function(cache_subdir = NULL) { + options("styler.use_cache" = TRUE) + if (!is.null(cache_subdir)) { + options("styler.cache_subdir" = cache_subdir) + } + cat("Using cache at", cache_find_path(cache_subdir), ".") +} + +#' @rdname cache_activate +cache_deactivate <- function() { + options("styler.use_cache" = FALSE) + options("styler.cache_subdir" = NULL) + + cat("Deactivated cache.") +} diff --git a/R/utils-cache.R b/R/utils-cache.R index 59fb51e1e..5a4b47900 100644 --- a/R/utils-cache.R +++ b/R/utils-cache.R @@ -8,9 +8,9 @@ hash_standardize <- function(x) { list(x) } -#' Where is the cache +#' Where is the cache? #' -#' Finds the path to the cache and creates it if it does not exist +#' Finds the path to the cache and creates it if it does not exist. #' @param cache_subdir The subdir of the cache. Is equivalent to the #' R.cache subdir when adding "styler" as a parent directory to #' `cache_subdir`. diff --git a/man/cache_activate.Rd b/man/cache_activate.Rd new file mode 100644 index 000000000..eaa07c089 --- /dev/null +++ b/man/cache_activate.Rd @@ -0,0 +1,25 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/ui.R +\name{cache_activate} +\alias{cache_activate} +\alias{cache_deactivate} +\title{Activate or deactivate the styler cache} +\usage{ +cache_activate(cache_subdir = NULL) + +cache_deactivate() +} +\arguments{ +\item{cache_subdir}{The sub-directory you want to use under the R.cache +location. If \code{NULL}, the option "styler.cache_subdir" is considered +which defaults to the version of styler used.} +} +\description{ +Helper functions to control the behavior of caching. Simple wrappers around +\code{\link[base:options]{base::options()}}. +} +\seealso{ +Other cache managers: \code{\link{cache_clear}}, + \code{\link{cache_info}} +} +\concept{cache managers} diff --git a/man/cache_clear.Rd b/man/cache_clear.Rd index 0cd0a101f..c155d9137 100644 --- a/man/cache_clear.Rd +++ b/man/cache_clear.Rd @@ -9,16 +9,21 @@ cache_clear(cache_subdir = NULL, ask = TRUE) \arguments{ \item{cache_subdir}{Each version of styler has it's own cache, because styling is potentially different with different versions of styler. If -\code{cache_subdir} is \code{NULL}, the option "styler.cache_subdir" is considered. -If unset, the value is resolved the currently installed version of styler.} +\code{cache_subdir} is \code{NULL}, the option "styler.cache_subdir" is considered +which defaults to the version of styler used.} \item{ask}{Whether or not to interactively ask the user again.} } \description{ Clears the cache that stores which files' styling is up-to-date. You won't be -able to undo this. Note that the file corresponding to the cache won't be -deleted. +able to undo this. Note that the file corresponding to the cache (a folder +on our file stystem) won't be deleted, but it will be empty after calling +\code{cache_clear}. If the cache is used at all or not is determined via the R option \code{styler.use_cache}, defaulting to \code{TRUE}. } +\seealso{ +Other cache managers: \code{\link{cache_activate}}, + \code{\link{cache_info}} +} \concept{cache managers} diff --git a/man/cache_find_path.Rd b/man/cache_find_path.Rd index 928c5b5c4..3bbfda4ae 100644 --- a/man/cache_find_path.Rd +++ b/man/cache_find_path.Rd @@ -2,7 +2,7 @@ % Please edit documentation in R/utils-cache.R \name{cache_find_path} \alias{cache_find_path} -\title{Where is the cache} +\title{Where is the cache?} \usage{ cache_find_path(cache_subdir = NULL) } @@ -12,6 +12,6 @@ R.cache subdir when adding "styler" as a parent directory to \code{cache_subdir}.} } \description{ -Finds the path to the cache and creates it if it does not exist +Finds the path to the cache and creates it if it does not exist. } \keyword{internal} diff --git a/man/cache_info.Rd b/man/cache_info.Rd index 1e11c24d2..bfb1e9d46 100644 --- a/man/cache_info.Rd +++ b/man/cache_info.Rd @@ -9,8 +9,8 @@ cache_info(cache_subdir = NULL, format = "lucid") \arguments{ \item{cache_subdir}{Each version of styler has it's own cache, because styling is potentially different with different versions of styler. If -\code{cache_subdir} is \code{NULL}, the option "styler.cache_subdir" is considered. -If unset, the value is resolved the currently installed version of styler.} +\code{cache_subdir} is \code{NULL}, the option "styler.cache_subdir" is considered +which defaults to the version of styler used.} \item{format}{Either "lucid" for a printed summary or "tabular" for a tabular summary from \code{\link[base:file.info]{base::file.info()}}.} @@ -19,3 +19,8 @@ tabular summary from \code{\link[base:file.info]{base::file.info()}}.} If the cache is used at all or not is determined via the R option \code{styler.use_cache}. } +\seealso{ +Other cache managers: \code{\link{cache_activate}}, + \code{\link{cache_clear}} +} +\concept{cache managers} From 3fe9bbd0d0b20640e91fc223bfa12b4c97521643 Mon Sep 17 00:00:00 2001 From: lorenzwalthert Date: Fri, 16 Aug 2019 14:23:22 +0200 Subject: [PATCH 12/67] don't use technical name cache_subdir (as it would be from the R.cache terminology). Simply use cache_name. --- API | 6 +++--- R/transform-files.R | 2 +- R/ui.R | 32 ++++++++++++++++---------------- R/utils-cache.R | 16 +++++++--------- R/zzz.R | 2 +- man/cache_activate.Rd | 8 ++++---- man/cache_clear.Rd | 13 ++++++++----- man/cache_find_path.Rd | 8 ++++---- man/cache_info.Rd | 9 ++++----- tests/testthat/test-cache.R | 4 ++-- 10 files changed, 50 insertions(+), 50 deletions(-) diff --git a/API b/API index 1ea161a4f..480056898 100644 --- a/API +++ b/API @@ -2,9 +2,9 @@ ## Exported functions -cache_activate(cache_subdir = NULL) -cache_clear(cache_subdir = NULL, ask = TRUE) -cache_info(cache_subdir = NULL, format = "lucid") +cache_activate(cache_name = NULL) +cache_clear(cache_name = NULL, ask = TRUE) +cache_info(cache_name = NULL, format = "lucid") create_style_guide(initialize = default_style_guide_attributes, line_break = NULL, space = NULL, token = NULL, indention = NULL, use_raw_indention = FALSE, reindention = tidyverse_reindention()) default_style_guide_attributes(pd_flat) specify_math_token_spacing(zero = "'^'", one = c("'+'", "'-'", "'*'", "'/'")) diff --git a/R/transform-files.R b/R/transform-files.R index f4801c06d..c9113c29f 100644 --- a/R/transform-files.R +++ b/R/transform-files.R @@ -76,7 +76,7 @@ make_transformer <- function(transformers, include_roxygen_examples, warn_empty = TRUE) { force(transformers) - cache_dir <- c("styler", get_cache_subdir()) + cache_dir <- c("styler", get_cache_name()) assert_R.cache_installation() function(text) { is_cached <- !is.null( diff --git a/R/ui.R b/R/ui.R index ab657e64a..4029a9eb5 100644 --- a/R/ui.R +++ b/R/ui.R @@ -251,16 +251,18 @@ style_file <- function(path, #' `cache_clear`. #' If the cache is used at all or not is determined via the R option #' `styler.use_cache`, defaulting to `TRUE`. -#' @param cache_subdir Each version of styler has it's own cache, because -#' styling is potentially different with different versions of styler. If -#' `cache_subdir` is `NULL`, the option "styler.cache_subdir" is considered -#' which defaults to the version of styler used. +#' @param cache_name The name of the cache to use. If +#' `NULL`, the option "styler.cache_name" is considered which defaults to +#' the version of styler used. +#' @details +#' Each version of styler has it's own cache by default, because styling is +#' potentially different with different versions of styler. #' @param ask Whether or not to interactively ask the user again. #' @family cache managers #' @export -cache_clear <- function(cache_subdir = NULL, ask = TRUE) { +cache_clear <- function(cache_name = NULL, ask = TRUE) { assert_R.cache_installation(installation_only = TRUE) - path_cache <- cache_find_path(cache_subdir) + path_cache <- cache_find_path(cache_name) R.cache::clearCache(path_cache, prompt = ask) } @@ -273,10 +275,10 @@ cache_clear <- function(cache_subdir = NULL, ask = TRUE) { #' tabular summary from [base::file.info()]. #' @family cache managers #' @export -cache_info <- function(cache_subdir = NULL, format = "lucid") { +cache_info <- function(cache_name = NULL, format = "lucid") { assert_R.cache_installation(installation_only = TRUE) rlang::arg_match(format, c("tabular", "lucid")) - path_cache <- cache_find_path(cache_subdir) + path_cache <- cache_find_path(cache_name) files <- list.files(path_cache, full.names = TRUE) file_info <- file_info(files) tbl <- tibble( @@ -303,23 +305,21 @@ cache_info <- function(cache_subdir = NULL, format = "lucid") { #' #' Helper functions to control the behavior of caching. Simple wrappers around #' [base::options()]. -#' @param cache_subdir The sub-directory you want to use under the R.cache -#' location. If `NULL`, the option "styler.cache_subdir" is considered -#' which defaults to the version of styler used. +#' @inheritParams cache_clear #' @family cache managers #' @export -cache_activate <- function(cache_subdir = NULL) { +cache_activate <- function(cache_name = NULL) { options("styler.use_cache" = TRUE) - if (!is.null(cache_subdir)) { - options("styler.cache_subdir" = cache_subdir) + if (!is.null(cache_name)) { + options("styler.cache_name" = cache_name) } - cat("Using cache at", cache_find_path(cache_subdir), ".") + cat("Using cache at", cache_find_path(cache_name), ".") } #' @rdname cache_activate cache_deactivate <- function() { options("styler.use_cache" = FALSE) - options("styler.cache_subdir" = NULL) + options("styler.cache_name" = NULL) cat("Deactivated cache.") } diff --git a/R/utils-cache.R b/R/utils-cache.R index 5a4b47900..a2db175f8 100644 --- a/R/utils-cache.R +++ b/R/utils-cache.R @@ -11,17 +11,15 @@ hash_standardize <- function(x) { #' Where is the cache? #' #' Finds the path to the cache and creates it if it does not exist. -#' @param cache_subdir The subdir of the cache. Is equivalent to the -#' R.cache subdir when adding "styler" as a parent directory to -#' `cache_subdir`. +#' @inheritParams cache_clear #' @keywords internal -cache_find_path <- function(cache_subdir = NULL) { - if (is.null(cache_subdir)) { - cache_subdir <- get_cache_subdir() +cache_find_path <- function(cache_name = NULL) { + if (is.null(cache_name)) { + cache_name <- get_cache_name() } - R.cache::getCachePath(c("styler", cache_subdir)) + R.cache::getCachePath(c("styler", cache_name)) } -get_cache_subdir <- function() { - getOption("styler.cache_subdir") +get_cache_name <- function() { + getOption("styler.cache_name") } diff --git a/R/zzz.R b/R/zzz.R index ecc03353e..453e91b18 100644 --- a/R/zzz.R +++ b/R/zzz.R @@ -4,7 +4,7 @@ op.styler <- list( styler.colored_print.vertical = TRUE, styler.use_cache = TRUE, - styler.cache_subdir = utils::packageDescription("styler", fields = "Version"), + styler.cache_name = utils::packageDescription("styler", fields = "Version"), styler.addins_style_transformer = "styler::tidyverse_style()" ) toset <- !(names(op.styler) %in% names(op)) diff --git a/man/cache_activate.Rd b/man/cache_activate.Rd index eaa07c089..144af9804 100644 --- a/man/cache_activate.Rd +++ b/man/cache_activate.Rd @@ -5,14 +5,14 @@ \alias{cache_deactivate} \title{Activate or deactivate the styler cache} \usage{ -cache_activate(cache_subdir = NULL) +cache_activate(cache_name = NULL) cache_deactivate() } \arguments{ -\item{cache_subdir}{The sub-directory you want to use under the R.cache -location. If \code{NULL}, the option "styler.cache_subdir" is considered -which defaults to the version of styler used.} +\item{cache_name}{The name of the cache to use. If +\code{NULL}, the option "styler.cache_name" is considered which defaults to +the version of styler used.} } \description{ Helper functions to control the behavior of caching. Simple wrappers around diff --git a/man/cache_clear.Rd b/man/cache_clear.Rd index c155d9137..73fa07c34 100644 --- a/man/cache_clear.Rd +++ b/man/cache_clear.Rd @@ -4,13 +4,12 @@ \alias{cache_clear} \title{Clear the cache} \usage{ -cache_clear(cache_subdir = NULL, ask = TRUE) +cache_clear(cache_name = NULL, ask = TRUE) } \arguments{ -\item{cache_subdir}{Each version of styler has it's own cache, because -styling is potentially different with different versions of styler. If -\code{cache_subdir} is \code{NULL}, the option "styler.cache_subdir" is considered -which defaults to the version of styler used.} +\item{cache_name}{The name of the cache to use. If +\code{NULL}, the option "styler.cache_name" is considered which defaults to +the version of styler used.} \item{ask}{Whether or not to interactively ask the user again.} } @@ -22,6 +21,10 @@ on our file stystem) won't be deleted, but it will be empty after calling If the cache is used at all or not is determined via the R option \code{styler.use_cache}, defaulting to \code{TRUE}. } +\details{ +Each version of styler has it's own cache by default, because styling is +potentially different with different versions of styler. +} \seealso{ Other cache managers: \code{\link{cache_activate}}, \code{\link{cache_info}} diff --git a/man/cache_find_path.Rd b/man/cache_find_path.Rd index 3bbfda4ae..3038c8acf 100644 --- a/man/cache_find_path.Rd +++ b/man/cache_find_path.Rd @@ -4,12 +4,12 @@ \alias{cache_find_path} \title{Where is the cache?} \usage{ -cache_find_path(cache_subdir = NULL) +cache_find_path(cache_name = NULL) } \arguments{ -\item{cache_subdir}{The subdir of the cache. Is equivalent to the -R.cache subdir when adding "styler" as a parent directory to -\code{cache_subdir}.} +\item{cache_name}{The name of the cache to use. If +\code{NULL}, the option "styler.cache_name" is considered which defaults to +the version of styler used.} } \description{ Finds the path to the cache and creates it if it does not exist. diff --git a/man/cache_info.Rd b/man/cache_info.Rd index bfb1e9d46..80e53efa9 100644 --- a/man/cache_info.Rd +++ b/man/cache_info.Rd @@ -4,13 +4,12 @@ \alias{cache_info} \title{Show information about the styler cache} \usage{ -cache_info(cache_subdir = NULL, format = "lucid") +cache_info(cache_name = NULL, format = "lucid") } \arguments{ -\item{cache_subdir}{Each version of styler has it's own cache, because -styling is potentially different with different versions of styler. If -\code{cache_subdir} is \code{NULL}, the option "styler.cache_subdir" is considered -which defaults to the version of styler used.} +\item{cache_name}{The name of the cache to use. If +\code{NULL}, the option "styler.cache_name" is considered which defaults to +the version of styler used.} \item{format}{Either "lucid" for a printed summary or "tabular" for a tabular summary from \code{\link[base:file.info]{base::file.info()}}.} diff --git a/tests/testthat/test-cache.R b/tests/testthat/test-cache.R index 6bbc807a2..42b23777e 100644 --- a/tests/testthat/test-cache.R +++ b/tests/testthat/test-cache.R @@ -1,6 +1,6 @@ test_that("activated cache brings speedup", { withr::with_options( - list("styler.use_cache" = TRUE, "styler.cache_subdir" = "testthat"), { + list("styler.use_cache" = TRUE, "styler.cache_name" = "testthat"), { cache_clear(ask = FALSE) first <- system.time(styler::style_file(test_path("reference-objects/caching.R"))) second <- system.time(styler::style_file(test_path("reference-objects/caching.R"))) @@ -11,7 +11,7 @@ test_that("activated cache brings speedup", { test_that("unactivated cache does not bring speedup", { withr::with_options( - list("styler.use_cache" = FALSE, "styler.cache_subdir" = "testthat"), { + list("styler.use_cache" = FALSE, "styler.cache_name" = "testthat"), { cache_clear(ask = FALSE) first <- system.time(styler::style_file(test_path("reference-objects/caching.R"))) second <- system.time(styler::style_file(test_path("reference-objects/caching.R"))) From 6aef9ffdc504ffd9a679deb39399bceb8785c909 Mon Sep 17 00:00:00 2001 From: lorenzwalthert Date: Fri, 16 Aug 2019 14:25:19 +0200 Subject: [PATCH 13/67] fix spacing. --- R/ui.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/ui.R b/R/ui.R index 4029a9eb5..ffe3094c4 100644 --- a/R/ui.R +++ b/R/ui.R @@ -313,7 +313,7 @@ cache_activate <- function(cache_name = NULL) { if (!is.null(cache_name)) { options("styler.cache_name" = cache_name) } - cat("Using cache at", cache_find_path(cache_name), ".") + cat("Using cache at ", cache_find_path(cache_name), ".", sep = "") } #' @rdname cache_activate From 390d2b4967543efb52b4e5d546797fb1df86e6e6 Mon Sep 17 00:00:00 2001 From: lorenzwalthert Date: Fri, 16 Aug 2019 14:30:35 +0200 Subject: [PATCH 14/67] be consistent: all cache functions start with cache_ --- R/transform-files.R | 2 +- R/ui.R | 2 ++ R/utils-cache.R | 8 ++++++-- R/zzz.R | 2 +- 4 files changed, 10 insertions(+), 4 deletions(-) diff --git a/R/transform-files.R b/R/transform-files.R index c9113c29f..32c6e5ad3 100644 --- a/R/transform-files.R +++ b/R/transform-files.R @@ -76,7 +76,7 @@ make_transformer <- function(transformers, include_roxygen_examples, warn_empty = TRUE) { force(transformers) - cache_dir <- c("styler", get_cache_name()) + cache_dir <- c("styler", cache_get_name()) assert_R.cache_installation() function(text) { is_cached <- !is.null( diff --git a/R/ui.R b/R/ui.R index ffe3094c4..06fce8b86 100644 --- a/R/ui.R +++ b/R/ui.R @@ -312,6 +312,8 @@ cache_activate <- function(cache_name = NULL) { options("styler.use_cache" = TRUE) if (!is.null(cache_name)) { options("styler.cache_name" = cache_name) + } else { + options("styler.cache_name" = cache_derive_name()) } cat("Using cache at ", cache_find_path(cache_name), ".", sep = "") } diff --git a/R/utils-cache.R b/R/utils-cache.R index a2db175f8..0d8aa484a 100644 --- a/R/utils-cache.R +++ b/R/utils-cache.R @@ -15,11 +15,15 @@ hash_standardize <- function(x) { #' @keywords internal cache_find_path <- function(cache_name = NULL) { if (is.null(cache_name)) { - cache_name <- get_cache_name() + cache_name <- cache_get_name() } R.cache::getCachePath(c("styler", cache_name)) } -get_cache_name <- function() { +cache_derive_name <- function() { + utils::packageDescription("styler", fields = "Version") +} + +cache_get_name <- function() { getOption("styler.cache_name") } diff --git a/R/zzz.R b/R/zzz.R index 453e91b18..531ab7122 100644 --- a/R/zzz.R +++ b/R/zzz.R @@ -4,7 +4,7 @@ op.styler <- list( styler.colored_print.vertical = TRUE, styler.use_cache = TRUE, - styler.cache_name = utils::packageDescription("styler", fields = "Version"), + styler.cache_name = cache_derive_name(), styler.addins_style_transformer = "styler::tidyverse_style()" ) toset <- !(names(op.styler) %in% names(op)) From fa3dc164eab74b21f47f964a402a4ce658c6fd56 Mon Sep 17 00:00:00 2001 From: lorenzwalthert Date: Fri, 16 Aug 2019 14:34:56 +0200 Subject: [PATCH 15/67] also export deactivate --- API | 1 + NAMESPACE | 1 + R/ui.R | 1 + 3 files changed, 3 insertions(+) diff --git a/API b/API index 480056898..43fc16c41 100644 --- a/API +++ b/API @@ -4,6 +4,7 @@ cache_activate(cache_name = NULL) cache_clear(cache_name = NULL, ask = TRUE) +cache_deactivate() cache_info(cache_name = NULL, format = "lucid") create_style_guide(initialize = default_style_guide_attributes, line_break = NULL, space = NULL, token = NULL, indention = NULL, use_raw_indention = FALSE, reindention = tidyverse_reindention()) default_style_guide_attributes(pd_flat) diff --git a/NAMESPACE b/NAMESPACE index cec420c57..7e7197660 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -3,6 +3,7 @@ S3method(print,vertical) export(cache_activate) export(cache_clear) +export(cache_deactivate) export(cache_info) export(create_style_guide) export(default_style_guide_attributes) diff --git a/R/ui.R b/R/ui.R index 06fce8b86..fce7bd576 100644 --- a/R/ui.R +++ b/R/ui.R @@ -319,6 +319,7 @@ cache_activate <- function(cache_name = NULL) { } #' @rdname cache_activate +#' @export cache_deactivate <- function() { options("styler.use_cache" = FALSE) options("styler.cache_name" = NULL) From 3ff182a1545dd86372b454c470f766d1be30029a Mon Sep 17 00:00:00 2001 From: lorenzwalthert Date: Sat, 17 Aug 2019 18:47:11 +0200 Subject: [PATCH 16/67] don't read the cache, just check if it exists --- R/transform-files.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/transform-files.R b/R/transform-files.R index 32c6e5ad3..27fff5926 100644 --- a/R/transform-files.R +++ b/R/transform-files.R @@ -80,7 +80,7 @@ make_transformer <- function(transformers, assert_R.cache_installation() function(text) { is_cached <- !is.null( - R.cache::loadCache(key = hash_standardize(text), dir = cache_dir) + R.cache::findCache(key = hash_standardize(text), dir = cache_dir) ) should_use_cache <- getOption("styler.use_cache") can_use_cache <- is_cached && should_use_cache From 1ab8be0a168dca8097dce19e0057017a2a067c0a Mon Sep 17 00:00:00 2001 From: lorenzwalthert Date: Sat, 17 Aug 2019 20:23:03 +0200 Subject: [PATCH 17/67] make styling work without R.chache installed. --- R/transform-files.R | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/R/transform-files.R b/R/transform-files.R index 27fff5926..130d90825 100644 --- a/R/transform-files.R +++ b/R/transform-files.R @@ -77,9 +77,9 @@ make_transformer <- function(transformers, warn_empty = TRUE) { force(transformers) cache_dir <- c("styler", cache_get_name()) - assert_R.cache_installation() + assert_R.cache_installation(action = "warn") function(text) { - is_cached <- !is.null( + is_cached <- rlang::is_installed("R.cache") && !is.null( R.cache::findCache(key = hash_standardize(text), dir = cache_dir) ) should_use_cache <- getOption("styler.use_cache") From e48b5ec04a693ef484529a9c77c26d3d6bfc0a75 Mon Sep 17 00:00:00 2001 From: lorenzwalthert Date: Sat, 17 Aug 2019 20:23:38 +0200 Subject: [PATCH 18/67] add line break at end of message. --- R/ui.R | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/R/ui.R b/R/ui.R index fce7bd576..418bca97a 100644 --- a/R/ui.R +++ b/R/ui.R @@ -315,7 +315,7 @@ cache_activate <- function(cache_name = NULL) { } else { options("styler.cache_name" = cache_derive_name()) } - cat("Using cache at ", cache_find_path(cache_name), ".", sep = "") + cat("Using cache at ", cache_find_path(cache_name), ".\n", sep = "") } #' @rdname cache_activate @@ -324,5 +324,5 @@ cache_deactivate <- function() { options("styler.use_cache" = FALSE) options("styler.cache_name" = NULL) - cat("Deactivated cache.") + cat("Deactivated cache.\n") } From bb52ea0597337b532f8e6c9b3177b5ae45a34def Mon Sep 17 00:00:00 2001 From: lorenzwalthert Date: Sat, 17 Aug 2019 20:24:22 +0200 Subject: [PATCH 19/67] add action = "abort" as optional. --- R/communicate.R | 35 ++++++++++++++++++++++-------- man/assert_R.cache_installation.Rd | 7 +++++- 2 files changed, 32 insertions(+), 10 deletions(-) diff --git a/R/communicate.R b/R/communicate.R index f3aa6745a..1ddffa96d 100644 --- a/R/communicate.R +++ b/R/communicate.R @@ -36,17 +36,34 @@ assert_data.tree_installation <- function() { } #' R.cache needs to be installed if caching functionality is enabled -#' +#' @param installation_only Whether or not to only check if R.cache is +#' installed. #' @keywords internal -assert_R.cache_installation <- function(installation_only = FALSE) { +assert_R.cache_installation <- function(installation_only = FALSE, + action = "abort") { + browser() if (!rlang::is_installed("R.cache") && - ifelse(installation_only, TRUE, getOption("styler.use_cache") + !ifelse(installation_only, TRUE, getOption("styler.use_cache") )) { - rlang::abort(paste( - "R package R.cache is not installed, which is needed when the option ", - "`styler.use_cache` is `TRUE`. Please install the package to enable the ", - "caching feature of styler or set `options(styler.use_cache = FALSE)` ", - "in your .Rprofile to use styler without that feature." - )) + msg_basic <- paste( + "R package R.cache is not installed, which is needed when the caching ", + "feature is activated. Please install the package with ", + "`install.packages('R.cache')` and then restart R to enable the ", + "caching feature of styler or permanently deactivate the feature by ", + "adding `styler::cache_deactivate()` to your .Rprofile, e.g. via ", + "`usethis::edit_r_profile()`." + ) + + if (action == "abort") { + rlang::abort(msg_basic) + } else { + rlang::warn(paste0( + msg_basic, " ", + "Deactivating the caching feature for the current session.", + "" + )) + cache_deactivate() + } + } } diff --git a/man/assert_R.cache_installation.Rd b/man/assert_R.cache_installation.Rd index 1c166abb9..c9d5d9166 100644 --- a/man/assert_R.cache_installation.Rd +++ b/man/assert_R.cache_installation.Rd @@ -4,7 +4,12 @@ \alias{assert_R.cache_installation} \title{R.cache needs to be installed if caching functionality is enabled} \usage{ -assert_R.cache_installation(installation_only = FALSE) +assert_R.cache_installation(installation_only = FALSE, + action = "abort") +} +\arguments{ +\item{installation_only}{Whether or not to only check if R.cache is +installed.} } \description{ R.cache needs to be installed if caching functionality is enabled From fb98b79b3c0149aec83ea2e692f598444214a1a8 Mon Sep 17 00:00:00 2001 From: lorenzwalthert Date: Mon, 19 Aug 2019 22:59:41 +0200 Subject: [PATCH 20/67] return a default path stlyer/:version instead instead of just styler in the case no option is set (which is the case after deactivating the cache). --- R/utils-cache.R | 3 +++ 1 file changed, 3 insertions(+) diff --git a/R/utils-cache.R b/R/utils-cache.R index 0d8aa484a..39e3633e5 100644 --- a/R/utils-cache.R +++ b/R/utils-cache.R @@ -16,6 +16,9 @@ hash_standardize <- function(x) { cache_find_path <- function(cache_name = NULL) { if (is.null(cache_name)) { cache_name <- cache_get_name() + if (is.null(cache_name)) { + cache_name <- cache_derive_name() + } } R.cache::getCachePath(c("styler", cache_name)) } From 0d0fa9262257cabc123942ed9b00b008c4fe17d3 Mon Sep 17 00:00:00 2001 From: lorenzwalthert Date: Mon, 19 Aug 2019 23:34:20 +0200 Subject: [PATCH 21/67] remove option styler.use_cache. If styler.cache_name is NULL, this is equivalent to not use the cache. Make cache_info congruent for tabular and lucid output. --- R/communicate.R | 10 ++++----- R/transform-files.R | 2 +- R/ui.R | 30 +++++++++++++++++--------- R/zzz.R | 1 - tests/testthat/helpers-devel-options.R | 7 ++---- 5 files changed, 28 insertions(+), 22 deletions(-) diff --git a/R/communicate.R b/R/communicate.R index 1ddffa96d..198a34435 100644 --- a/R/communicate.R +++ b/R/communicate.R @@ -41,17 +41,18 @@ assert_data.tree_installation <- function() { #' @keywords internal assert_R.cache_installation <- function(installation_only = FALSE, action = "abort") { - browser() + # fail if R.cache is not installed but feature is actiavted. if (!rlang::is_installed("R.cache") && - !ifelse(installation_only, TRUE, getOption("styler.use_cache") - )) { + ifelse(installation_only, TRUE, cache_is_activated()) + ) { msg_basic <- paste( "R package R.cache is not installed, which is needed when the caching ", "feature is activated. Please install the package with ", "`install.packages('R.cache')` and then restart R to enable the ", "caching feature of styler or permanently deactivate the feature by ", "adding `styler::cache_deactivate()` to your .Rprofile, e.g. via ", - "`usethis::edit_r_profile()`." + "`usethis::edit_r_profile()`.", + sep = "" ) if (action == "abort") { @@ -64,6 +65,5 @@ assert_R.cache_installation <- function(installation_only = FALSE, )) cache_deactivate() } - } } diff --git a/R/transform-files.R b/R/transform-files.R index 130d90825..516d9e2db 100644 --- a/R/transform-files.R +++ b/R/transform-files.R @@ -82,7 +82,7 @@ make_transformer <- function(transformers, is_cached <- rlang::is_installed("R.cache") && !is.null( R.cache::findCache(key = hash_standardize(text), dir = cache_dir) ) - should_use_cache <- getOption("styler.use_cache") + should_use_cache <- cache_is_activated() can_use_cache <- is_cached && should_use_cache if (!can_use_cache) { transformed_code <- text %>% diff --git a/R/ui.R b/R/ui.R index 418bca97a..b11c699ad 100644 --- a/R/ui.R +++ b/R/ui.R @@ -249,8 +249,6 @@ style_file <- function(path, #' able to undo this. Note that the file corresponding to the cache (a folder #' on our file stystem) won't be deleted, but it will be empty after calling #' `cache_clear`. -#' If the cache is used at all or not is determined via the R option -#' `styler.use_cache`, defaulting to `TRUE`. #' @param cache_name The name of the cache to use. If #' `NULL`, the option "styler.cache_name" is considered which defaults to #' the version of styler used. @@ -268,9 +266,10 @@ cache_clear <- function(cache_name = NULL, ask = TRUE) { #' Show information about the styler cache #' -#' If the cache is used at all or not is determined via the R option -#' `styler.use_cache`. -#' @inheritParams cache_clear +#' Gives information about the cache +#' @param cache_name The name of the cache for which to show details. If +#' `NULL`, the option "styler.cache_name" is considered which defaults to +#' the version of styler used. #' @param format Either "lucid" for a printed summary or "tabular" for a #' tabular summary from [base::file.info()]. #' @family cache managers @@ -285,7 +284,9 @@ cache_info <- function(cache_name = NULL, format = "lucid") { n = nrow(file_info), size = sum(file_info$size), last_modified = suppressWarnings(max(file_info$mtime)), - created = file.info(path_cache)$ctime + created = file.info(path_cache)$ctime, + location = path_cache, + activated = cache_is_activated() ) if (format == "lucid") { cat( @@ -293,7 +294,7 @@ cache_info <- function(cache_name = NULL, format = "lucid") { "\nLast modified:\t", as.character(tbl$last_modified), "\nCreated:\t", as.character(tbl$created), "\nLocation:\t", path_cache, - "\nActivated:\t", getOption("styler.use_cache"), + "\nActivated:\t", cache_is_activated(), sep = "" ) } else { @@ -309,20 +310,29 @@ cache_info <- function(cache_name = NULL, format = "lucid") { #' @family cache managers #' @export cache_activate <- function(cache_name = NULL) { - options("styler.use_cache" = TRUE) + assert_R.cache_installation(installation_only = TRUE) if (!is.null(cache_name)) { options("styler.cache_name" = cache_name) } else { options("styler.cache_name" = cache_derive_name()) } - cat("Using cache at ", cache_find_path(cache_name), ".\n", sep = "") + path <- cache_find_path(cache_name) + cat( + "Using cache ", cache_get_name(), " at ", + path, ".\n", + sep = "" + ) + path } #' @rdname cache_activate #' @export cache_deactivate <- function() { - options("styler.use_cache" = FALSE) options("styler.cache_name" = NULL) cat("Deactivated cache.\n") } + +cache_is_activated <- function() { + !is.null(cache_get_name()) +} diff --git a/R/zzz.R b/R/zzz.R index 531ab7122..bedd9ea5b 100644 --- a/R/zzz.R +++ b/R/zzz.R @@ -3,7 +3,6 @@ op <- options() op.styler <- list( styler.colored_print.vertical = TRUE, - styler.use_cache = TRUE, styler.cache_name = cache_derive_name(), styler.addins_style_transformer = "styler::tidyverse_style()" ) diff --git a/tests/testthat/helpers-devel-options.R b/tests/testthat/helpers-devel-options.R index 271a50bde..8751489f7 100644 --- a/tests/testthat/helpers-devel-options.R +++ b/tests/testthat/helpers-devel-options.R @@ -1,5 +1,2 @@ -options(styler.use_cache = FALSE) -cat( - "Setting options `styler.use_cache` ", - "from tests/testthat/helpers-devel-options.R" -) +cat("In tests/testthat/helpers-devel-options: ") +cache_deactivate() From 43c58c95b5f41849b2169fcc7d2847f6d0fd42ba Mon Sep 17 00:00:00 2001 From: lorenzwalthert Date: Mon, 19 Aug 2019 23:34:54 +0200 Subject: [PATCH 22/67] also test when R.cache is not installed. --- .travis.yml | 5 ++- tests/testthat/test-cache-with-r-cache.R | 49 +++++++++++++++++++++ tests/testthat/test-cache-without-r-cache.R | 35 +++++++++++++++ tests/testthat/test-cache.R | 21 --------- 4 files changed, 88 insertions(+), 22 deletions(-) create mode 100644 tests/testthat/test-cache-with-r-cache.R create mode 100644 tests/testthat/test-cache-without-r-cache.R delete mode 100644 tests/testthat/test-cache.R diff --git a/.travis.yml b/.travis.yml index 68273b8ea..4e7aff5fd 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,7 +3,7 @@ # Usually you shouldn't need to change the first part of the file # DO NOT CHANGE THE CODE BELOW -before_install: R -q -e 'install.packages(c("remotes", "curl", "knitr", "rmarkdown")); remotes::install_github("ropenscilabs/tic"); tic::prepare_all_stages(); remotes::install_deps(dependencies = TRUE); tic::before_install()' +before_install: R -q -e 'install.packages(c("remotes", "curl", "knitr", "rmarkdown")); remotes::install_github("ropenscilabs/tic"); tic::prepare_all_stages(); remotes::install_deps(dependencies = TRUE); if (as.logical(toupper(Sys.getenv("R_REMOVE_RCACHE")))) remove.packages("R.cache"); tic::before_install()' install: R -q -e 'tic::install()' after_install: R -q -e 'tic::after_install()' before_script: R -q -e 'tic::before_script()' @@ -38,6 +38,9 @@ matrix: - r: release env: - BUILD_PKGDOWN: true + - r: release + env: + - R_REMOVE_RCACHE: true - r: devel #env diff --git a/tests/testthat/test-cache-with-r-cache.R b/tests/testthat/test-cache-with-r-cache.R new file mode 100644 index 000000000..de4ab1fa8 --- /dev/null +++ b/tests/testthat/test-cache-with-r-cache.R @@ -0,0 +1,49 @@ +test_that("No warnings are issued when R.cache is installed", { + skip_if_not_installed("R.cache") + on.exit(cache_deactivate()) + expect_silent(assert_R.cache_installation(installation_only = TRUE)) + expect_silent(assert_R.cache_installation()) + expect_warning(style_text("1+1"), NA) + cache_activate() + assert_R.cache_installation() + expect_warning(style_text("1+1"), NA) +}) + + +test_that("Cache management works when R.cache is installed", { + skip_if_not_installed("R.cache") + on.exit(cache_deactivate()) + cache_activate() + # at fresh startup, with R.cache installed + expect_s3_class(cache_info(format = "tabular"), "tbl_df") + expect_error(cache_info(), NA) + expect_equal(basename(cache_activate()), utils::packageDescription("styler", fields = "Version")) + expect_equal(basename(cache_activate("xyz")), "xyz") + expect_equal(getOption("styler.cache_name"), "xyz") + cache_deactivate() + expect_false(cache_info(format = "tabular")$activated) + expect_equal(getOption("styler.cache_location"), NULL) + expect_error(cache_clear(ask = FALSE), NA) +}) + + + +test_that("activated cache brings speedup", { + skip_if_not_installed("R.cache") + cache_clear(ask = FALSE) + cache_activate() + on.exit(cache_deactivate()) + first <- system.time(styler::style_file(test_path("reference-objects/caching.R"))) + second <- system.time(styler::style_file(test_path("reference-objects/caching.R"))) + expect_true(first["elapsed"] / 2 > second["elapsed"]) +}) + + +test_that("unactivated cache does not bring speedup", { + skip_if_not_installed("R.cache") + cache_clear(ask = FALSE) + cache_deactivate() + first <- system.time(styler::style_file(test_path("reference-objects/caching.R"))) + second <- system.time(styler::style_file(test_path("reference-objects/caching.R"))) + expect_false(first["elapsed"] / 2 > second["elapsed"]) +}) diff --git a/tests/testthat/test-cache-without-r-cache.R b/tests/testthat/test-cache-without-r-cache.R new file mode 100644 index 000000000..c9cbc107c --- /dev/null +++ b/tests/testthat/test-cache-without-r-cache.R @@ -0,0 +1,35 @@ +test_that("Cache management fails mostly when R.cache is not installed", { + skip_if(rlang::is_installed("R.cache")) + expect_error(cache_info(), "is needed when the caching feature is activated") + expect_error(cache_activate(), "is needed when the caching feature is activated") + expect_error(cache_clear(), "is needed when the caching feature is activated") + expect_error(capture.output(cache_deactivate()), NA) + expect_silent(assert_R.cache_installation()) + expect_error( + assert_R.cache_installation(installation_only = TRUE), + "is needed when the caching feature is activated" + ) +}) + + +test_that("styling works when R.cache is not installed", { + # warning for first time + expect_warning(capture.output( + withr::with_options( + # simulate .onLoad() in fresh R session + list(styler.cache_name = cache_derive_name()), + style_text("1+1") + )), + "Deactivating the caching feature for the current session" + ) + + # No warnings subsequently + expect_warning(capture.output( + withr::with_options( + list(styler.cache_name = cache_derive_name()), { + suppressWarnings(style_text("1+1")) + style_text("1+1") + })), + NA + ) +}) diff --git a/tests/testthat/test-cache.R b/tests/testthat/test-cache.R deleted file mode 100644 index 42b23777e..000000000 --- a/tests/testthat/test-cache.R +++ /dev/null @@ -1,21 +0,0 @@ -test_that("activated cache brings speedup", { - withr::with_options( - list("styler.use_cache" = TRUE, "styler.cache_name" = "testthat"), { - cache_clear(ask = FALSE) - first <- system.time(styler::style_file(test_path("reference-objects/caching.R"))) - second <- system.time(styler::style_file(test_path("reference-objects/caching.R"))) - expect_true(first["elapsed"] / 2 > second["elapsed"]) - } - ) -}) - -test_that("unactivated cache does not bring speedup", { - withr::with_options( - list("styler.use_cache" = FALSE, "styler.cache_name" = "testthat"), { - cache_clear(ask = FALSE) - first <- system.time(styler::style_file(test_path("reference-objects/caching.R"))) - second <- system.time(styler::style_file(test_path("reference-objects/caching.R"))) - expect_false(first["elapsed"] / 2 > second["elapsed"]) - } - ) -}) From 87d782a706d39b626b2dcb856d699030227d78a2 Mon Sep 17 00:00:00 2001 From: lorenzwalthert Date: Thu, 22 Aug 2019 09:08:52 +0200 Subject: [PATCH 23/67] add new option "both" for cache_info(), argument "format". --- API | 2 +- R/ui.R | 22 +++++++++++++--------- man/cache_info.Rd | 12 ++++++------ 3 files changed, 20 insertions(+), 16 deletions(-) diff --git a/API b/API index 43fc16c41..e4c7a6b14 100644 --- a/API +++ b/API @@ -5,7 +5,7 @@ cache_activate(cache_name = NULL) cache_clear(cache_name = NULL, ask = TRUE) cache_deactivate() -cache_info(cache_name = NULL, format = "lucid") +cache_info(cache_name = NULL, format = "both") create_style_guide(initialize = default_style_guide_attributes, line_break = NULL, space = NULL, token = NULL, indention = NULL, use_raw_indention = FALSE, reindention = tidyverse_reindention()) default_style_guide_attributes(pd_flat) specify_math_token_spacing(zero = "'^'", one = c("'+'", "'-'", "'*'", "'/'")) diff --git a/R/ui.R b/R/ui.R index b11c699ad..f8c2c4bcb 100644 --- a/R/ui.R +++ b/R/ui.R @@ -270,13 +270,14 @@ cache_clear <- function(cache_name = NULL, ask = TRUE) { #' @param cache_name The name of the cache for which to show details. If #' `NULL`, the option "styler.cache_name" is considered which defaults to #' the version of styler used. -#' @param format Either "lucid" for a printed summary or "tabular" for a -#' tabular summary from [base::file.info()]. +#' @param format Either "lucid" for a summary emitted with [base::cat()] or +#' "tabular" for a tabular summary from [base::file.info()] or "both" for +#' both. #' @family cache managers #' @export -cache_info <- function(cache_name = NULL, format = "lucid") { +cache_info <- function(cache_name = NULL, format = "both") { assert_R.cache_installation(installation_only = TRUE) - rlang::arg_match(format, c("tabular", "lucid")) + rlang::arg_match(format, c("tabular", "lucid", "both")) path_cache <- cache_find_path(cache_name) files <- list.files(path_cache, full.names = TRUE) file_info <- file_info(files) @@ -286,19 +287,22 @@ cache_info <- function(cache_name = NULL, format = "lucid") { last_modified = suppressWarnings(max(file_info$mtime)), created = file.info(path_cache)$ctime, location = path_cache, - activated = cache_is_activated() + activated = cache_is_activated(cache_name) ) - if (format == "lucid") { + if (format %in% c("lucid", "both")) { cat( "Size:\t\t", tbl$size, " bytes (", tbl$n, " cached expressions)", "\nLast modified:\t", as.character(tbl$last_modified), "\nCreated:\t", as.character(tbl$created), "\nLocation:\t", path_cache, - "\nActivated:\t", cache_is_activated(), + "\nActivated:\t", tbl$activated, sep = "" ) - } else { + } + if (format == "tabular") { tbl + } else if (format == "both") { + invisible(tbl) } } @@ -322,7 +326,7 @@ cache_activate <- function(cache_name = NULL) { path, ".\n", sep = "" ) - path + invisible(path) } #' @rdname cache_activate diff --git a/man/cache_info.Rd b/man/cache_info.Rd index 80e53efa9..ab7ec4462 100644 --- a/man/cache_info.Rd +++ b/man/cache_info.Rd @@ -4,19 +4,19 @@ \alias{cache_info} \title{Show information about the styler cache} \usage{ -cache_info(cache_name = NULL, format = "lucid") +cache_info(cache_name = NULL, format = "both") } \arguments{ -\item{cache_name}{The name of the cache to use. If +\item{cache_name}{The name of the cache for which to show details. If \code{NULL}, the option "styler.cache_name" is considered which defaults to the version of styler used.} -\item{format}{Either "lucid" for a printed summary or "tabular" for a -tabular summary from \code{\link[base:file.info]{base::file.info()}}.} +\item{format}{Either "lucid" for a summary emitted with \code{\link[base:cat]{base::cat()}} or +"tabular" for a tabular summary from \code{\link[base:file.info]{base::file.info()}} or "both" for +both.} } \description{ -If the cache is used at all or not is determined via the R option -\code{styler.use_cache}. +Gives information about the cache } \seealso{ Other cache managers: \code{\link{cache_activate}}, From b2b421338edcd1c88a26d4094cf5cc33e5f0cb1a Mon Sep 17 00:00:00 2001 From: lorenzwalthert Date: Thu, 22 Aug 2019 09:11:12 +0200 Subject: [PATCH 24/67] outsource logic to get cache name from input, get it from the option or derive it fresh from the styler version. --- R/utils-cache.R | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/R/utils-cache.R b/R/utils-cache.R index 39e3633e5..f51d844d4 100644 --- a/R/utils-cache.R +++ b/R/utils-cache.R @@ -14,12 +14,7 @@ hash_standardize <- function(x) { #' @inheritParams cache_clear #' @keywords internal cache_find_path <- function(cache_name = NULL) { - if (is.null(cache_name)) { - cache_name <- cache_get_name() - if (is.null(cache_name)) { - cache_name <- cache_derive_name() - } - } + cache_name <- cache_get_or_derive_name(cache_name) R.cache::getCachePath(c("styler", cache_name)) } @@ -30,3 +25,13 @@ cache_derive_name <- function() { cache_get_name <- function() { getOption("styler.cache_name") } + +cache_get_or_derive_name <- function(cache_name) { + if (is.null(cache_name)) { + cache_name <- cache_get_name() + if (is.null(cache_name)) { + cache_name <- cache_derive_name() + } + } + cache_name +} From 865c6e8cd2766b7d4b7abf09acea5881bb54e10e Mon Sep 17 00:00:00 2001 From: lorenzwalthert Date: Thu, 22 Aug 2019 09:11:51 +0200 Subject: [PATCH 25/67] cache_is_activated() can now return information about a specific cache state. --- R/ui.R | 17 +++++++++++++++-- man/cache_is_activated.Rd | 17 +++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) create mode 100644 man/cache_is_activated.Rd diff --git a/R/ui.R b/R/ui.R index f8c2c4bcb..2fb59eef6 100644 --- a/R/ui.R +++ b/R/ui.R @@ -337,6 +337,19 @@ cache_deactivate <- function() { cat("Deactivated cache.\n") } -cache_is_activated <- function() { - !is.null(cache_get_name()) +#' Check if a cache is activated +#' +#' @param cache_name The name of the cache to check. If `NULL`, we check if +#' any cache is activated. If not `NULL`, we check if a specific cache is +#' activated. +#' @keywords internal +cache_is_activated <- function(cache_name = NULL) { + current_cache <- cache_get_name() + if (is.null(cache_name)) { + !is.null(current_cache) + } else if (!is.null(current_cache)) { + cache_name == current_cache + } else { + FALSE + } } diff --git a/man/cache_is_activated.Rd b/man/cache_is_activated.Rd new file mode 100644 index 000000000..b600a42a4 --- /dev/null +++ b/man/cache_is_activated.Rd @@ -0,0 +1,17 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/ui.R +\name{cache_is_activated} +\alias{cache_is_activated} +\title{Check if a cache is activated} +\usage{ +cache_is_activated(cache_name = NULL) +} +\arguments{ +\item{cache_name}{The name of the cache to check. If \code{NULL}, we check if +any cache is activated. If not \code{NULL}, we check if a specific cache is +activated.} +} +\description{ +Check if a cache is activated +} +\keyword{internal} From 73d022042d941feefa706e3708dcbe4b53de1562 Mon Sep 17 00:00:00 2001 From: lorenzwalthert Date: Thu, 22 Aug 2019 09:12:11 +0200 Subject: [PATCH 26/67] random doc update --- man/cache_clear.Rd | 2 -- 1 file changed, 2 deletions(-) diff --git a/man/cache_clear.Rd b/man/cache_clear.Rd index 73fa07c34..d2ed6fe24 100644 --- a/man/cache_clear.Rd +++ b/man/cache_clear.Rd @@ -18,8 +18,6 @@ Clears the cache that stores which files' styling is up-to-date. You won't be able to undo this. Note that the file corresponding to the cache (a folder on our file stystem) won't be deleted, but it will be empty after calling \code{cache_clear}. -If the cache is used at all or not is determined via the R option -\code{styler.use_cache}, defaulting to \code{TRUE}. } \details{ Each version of styler has it's own cache by default, because styling is From 6fad8c4b12a9010b6f32989ead46a92b7166b927 Mon Sep 17 00:00:00 2001 From: lorenzwalthert Date: Thu, 22 Aug 2019 10:16:28 +0200 Subject: [PATCH 27/67] improve tests --- tests/testthat/reference-objects/cache-info-1 | Bin 0 -> 191 bytes tests/testthat/reference-objects/cache-info-2 | Bin 0 -> 148 bytes tests/testthat/reference-objects/cache-info-3 | Bin 0 -> 99 bytes tests/testthat/test-cache-with-r-cache.R | 87 +++++++++++++----- tests/testthat/test-cache-without-r-cache.R | 30 +++--- 5 files changed, 81 insertions(+), 36 deletions(-) create mode 100644 tests/testthat/reference-objects/cache-info-1 create mode 100644 tests/testthat/reference-objects/cache-info-2 create mode 100644 tests/testthat/reference-objects/cache-info-3 diff --git a/tests/testthat/reference-objects/cache-info-1 b/tests/testthat/reference-objects/cache-info-1 new file mode 100644 index 0000000000000000000000000000000000000000..724c65e0ab9407fa7c121119e2f91c9ce520d54c GIT binary patch literal 191 zcmV;w06_mAiwFP!00000165DU3WG2Zj3%`xSPK0Q|HD%Yy?B$q?Cq9Ue|qXhIpCO`MBQ?I+)J1G`}o`IU|vu2!PC6(P^Y-4M~Vyr8&cTEtszVSFb tGVi^d8NN_v#5x$vyi@w`(yc8`!%TuPOpGR-H6y&1TfQrT?Zr$100860RQCV? literal 0 HcmV?d00001 diff --git a/tests/testthat/reference-objects/cache-info-2 b/tests/testthat/reference-objects/cache-info-2 new file mode 100644 index 0000000000000000000000000000000000000000..93a77023ef24d9a7a9640b815e609deac9d91a3e GIT binary patch literal 148 zcmb2|=3oE==I#ec2?+^F32Dre&N!%THt1kTic?5yNwO1o8&KH5$hAYDNXI8(R%@fM z$L43M&z6ZXOEM)mD>U^~2C#V-riE%vIXiPk%%sR^5tC*$rLm+3*6ox#BQZgGhE&6m zdIg8=+Lr1YCGP5No3v!kk0~Y^{gNk|jdlhJc?*5L)YZVoT=}0Njn8?-H=ttxEDtxB literal 0 HcmV?d00001 diff --git a/tests/testthat/reference-objects/cache-info-3 b/tests/testthat/reference-objects/cache-info-3 new file mode 100644 index 0000000000000000000000000000000000000000..742bca3f413a46838018fcb1b436497a1c5d07fe GIT binary patch literal 99 zcmb2|=3oE==I#ec2?+^F32Dre&N!$wGt4-D=m6)EtIfu%gVtJDp4IIYnWW_%)Gcb# zBQi;NvT;zbNRT#HS5SAi@#oK7rVCH~l%4EtbXv$eD0q#9bCXjn second["elapsed"]) -}) +})) - -test_that("unactivated cache does not bring speedup", { +capture.output(test_that("unactivated cache does not bring speedup", { skip_if_not_installed("R.cache") - cache_clear(ask = FALSE) - cache_deactivate() - first <- system.time(styler::style_file(test_path("reference-objects/caching.R"))) - second <- system.time(styler::style_file(test_path("reference-objects/caching.R"))) - expect_false(first["elapsed"] / 2 > second["elapsed"]) -}) + on.exit(clear_testthat_cache) + clear_testthat_cache() + cache_deactivate() + first <- system.time(styler::style_file(test_path("reference-objects/caching.R"))) + second <- system.time(styler::style_file(test_path("reference-objects/caching.R"))) + expect_false(first["elapsed"] / 2 > second["elapsed"]) +})) + +capture.output(test_that("cached expressions are displayed propperly", { + on.exit(clear_testthat_cache()) + clear_testthat_cache() + cache_info <- cache_info("testthat", format = "tabular") + expect_known_value( + cache_info[, c("n", "size", "last_modified", "activated")], + file = test_path("reference-objects/cache-info-1") + ) + cache_activate("testthat") + style_text("1+1") + cache_info <- cache_info(format = "tabular") + expect_known_value( + cache_info[, c("n", "size", "activated")], + file = test_path("reference-objects/cache-info-2") + ) + style_text("a <-function() NULL") + unstructured_cache_info <- capture.output(cache_info(format = "lucid")) + expect_known_value( + unstructured_cache_info[c(1, length(unstructured_cache_info))], + file = test_path("reference-objects/cache-info-3") + ) +})) diff --git a/tests/testthat/test-cache-without-r-cache.R b/tests/testthat/test-cache-without-r-cache.R index c9cbc107c..955ae84bb 100644 --- a/tests/testthat/test-cache-without-r-cache.R +++ b/tests/testthat/test-cache-without-r-cache.R @@ -13,23 +13,29 @@ test_that("Cache management fails mostly when R.cache is not installed", { test_that("styling works when R.cache is not installed", { + skip_if(rlang::is_installed("R.cache")) # warning for first time - expect_warning(capture.output( - withr::with_options( - # simulate .onLoad() in fresh R session - list(styler.cache_name = cache_derive_name()), - style_text("1+1") - )), + expect_warning( + capture.output( + withr::with_options( + # simulate .onLoad() in fresh R session + list(styler.cache_name = cache_derive_name()), + style_text("1+1") + ) + ), "Deactivating the caching feature for the current session" ) # No warnings subsequently - expect_warning(capture.output( - withr::with_options( - list(styler.cache_name = cache_derive_name()), { - suppressWarnings(style_text("1+1")) - style_text("1+1") - })), + expect_warning( + capture.output( + withr::with_options( + list(styler.cache_name = cache_derive_name()), { + suppressWarnings(style_text("1+1")) + style_text("1+1") + } + ) + ), NA ) }) From f51c707753ff3624898306f785b0321c0301a9f9 Mon Sep 17 00:00:00 2001 From: lorenzwalthert Date: Thu, 22 Aug 2019 10:16:49 +0200 Subject: [PATCH 28/67] add a verbose argument to cache_deactivate --- API | 2 +- R/communicate.R | 2 +- R/ui.R | 8 ++++++-- man/cache_activate.Rd | 2 +- 4 files changed, 9 insertions(+), 5 deletions(-) diff --git a/API b/API index e4c7a6b14..b45a5f159 100644 --- a/API +++ b/API @@ -4,7 +4,7 @@ cache_activate(cache_name = NULL) cache_clear(cache_name = NULL, ask = TRUE) -cache_deactivate() +cache_deactivate(verbose = TRUE) cache_info(cache_name = NULL, format = "both") create_style_guide(initialize = default_style_guide_attributes, line_break = NULL, space = NULL, token = NULL, indention = NULL, use_raw_indention = FALSE, reindention = tidyverse_reindention()) default_style_guide_attributes(pd_flat) diff --git a/R/communicate.R b/R/communicate.R index 198a34435..408c492b6 100644 --- a/R/communicate.R +++ b/R/communicate.R @@ -63,7 +63,7 @@ assert_R.cache_installation <- function(installation_only = FALSE, "Deactivating the caching feature for the current session.", "" )) - cache_deactivate() + cache_deactivate(verbose = FALSE) } } } diff --git a/R/ui.R b/R/ui.R index 2fb59eef6..548672939 100644 --- a/R/ui.R +++ b/R/ui.R @@ -262,6 +262,8 @@ cache_clear <- function(cache_name = NULL, ask = TRUE) { assert_R.cache_installation(installation_only = TRUE) path_cache <- cache_find_path(cache_name) R.cache::clearCache(path_cache, prompt = ask) + cache_deactivate(verbose = FALSE) + } #' Show information about the styler cache @@ -331,10 +333,12 @@ cache_activate <- function(cache_name = NULL) { #' @rdname cache_activate #' @export -cache_deactivate <- function() { +cache_deactivate <- function(verbose = TRUE) { options("styler.cache_name" = NULL) - cat("Deactivated cache.\n") + if (verbose) { + cat("Deactivated cache.\n") + } } #' Check if a cache is activated diff --git a/man/cache_activate.Rd b/man/cache_activate.Rd index 144af9804..cde7bdf64 100644 --- a/man/cache_activate.Rd +++ b/man/cache_activate.Rd @@ -7,7 +7,7 @@ \usage{ cache_activate(cache_name = NULL) -cache_deactivate() +cache_deactivate(verbose = TRUE) } \arguments{ \item{cache_name}{The name of the cache to use. If From 94d589121b4cacc91dda296be4dfb44e1c9b8480 Mon Sep 17 00:00:00 2001 From: lorenzwalthert Date: Thu, 22 Aug 2019 11:44:21 +0200 Subject: [PATCH 29/67] split R/ui.R into R/ui-caching.R and R/ui-styling.R --- DESCRIPTION | 3 +- R/ui-caching.R | 97 ++++++++++++++++++++++++++++++++ R/{ui.R => ui-styling.R} | 115 -------------------------------------- R/utils-cache.R | 18 ++++++ man/cache_activate.Rd | 4 +- man/cache_clear.Rd | 6 +- man/cache_find_path.Rd | 2 +- man/cache_info.Rd | 6 +- man/cache_is_activated.Rd | 2 +- man/prettify_any.Rd | 2 +- man/style_dir.Rd | 2 +- man/style_file.Rd | 2 +- man/style_pkg.Rd | 2 +- man/style_text.Rd | 2 +- 14 files changed, 132 insertions(+), 131 deletions(-) create mode 100644 R/ui-caching.R rename R/{ui.R => ui-styling.R} (71%) diff --git a/DESCRIPTION b/DESCRIPTION index 2d353dc6d..0f8a77570 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -80,7 +80,8 @@ Collate: 'token-create.R' 'transform-code.R' 'transform-files.R' - 'ui.R' + 'ui-caching.R' + 'ui-styling.R' 'unindent.R' 'utils-cache.R' 'utils-files.R' diff --git a/R/ui-caching.R b/R/ui-caching.R new file mode 100644 index 000000000..fa82cb492 --- /dev/null +++ b/R/ui-caching.R @@ -0,0 +1,97 @@ +#' Clear the cache +#' +#' Clears the cache that stores which files' styling is up-to-date. You won't be +#' able to undo this. Note that the file corresponding to the cache (a folder +#' on your file stystem) won't be deleted, but it will be empty after calling +#' `cache_clear`. +#' @param cache_name The name of the styler cache to use. If +#' `NULL`, the option "styler.cache_name" is considered which defaults to +#' the version of styler used. +#' @details +#' Each version of styler has it's own cache by default, because styling is +#' potentially different with different versions of styler. +#' @param ask Whether or not to interactively ask the user again. +#' @family cache managers +#' @export +cache_clear <- function(cache_name = NULL, ask = TRUE) { + assert_R.cache_installation(installation_only = TRUE) + path_cache <- cache_find_path(cache_name) + R.cache::clearCache(path_cache, prompt = ask) + cache_deactivate(verbose = FALSE) + +} + +#' Show information about the styler cache +#' +#' Gives information about the cache. +#' @param cache_name The name of the cache for which to show details. If +#' `NULL`, the option "styler.cache_name" is considered which defaults to +#' the version of styler used. +#' @param format Either "lucid" for a summary emitted with [base::cat()], +#' "tabular" for a tabular summary from [base::file.info()] or "both" for +#' both. +#' @family cache managers +#' @export +cache_info <- function(cache_name = NULL, format = "both") { + assert_R.cache_installation(installation_only = TRUE) + rlang::arg_match(format, c("tabular", "lucid", "both")) + path_cache <- cache_find_path(cache_name) + files <- list.files(path_cache, full.names = TRUE) + file_info <- file_info(files) + tbl <- tibble( + n = nrow(file_info), + size = sum(file_info$size), + last_modified = suppressWarnings(max(file_info$mtime)), + created = file.info(path_cache)$ctime, + location = path_cache, + activated = cache_is_activated(cache_name) + ) + if (format %in% c("lucid", "both")) { + cat( + "Size:\t\t", tbl$size, " bytes (", tbl$n, " cached expressions)", + "\nLast modified:\t", as.character(tbl$last_modified), + "\nCreated:\t", as.character(tbl$created), + "\nLocation:\t", path_cache, + "\nActivated:\t", tbl$activated, + sep = "" + ) + } + if (format == "tabular") { + tbl + } else if (format == "both") { + invisible(tbl) + } +} + +#' Activate or deactivate the styler cache +#' +#' Helper functions to control the behavior of caching. Simple wrappers around +#' [base::options()]. +#' @inheritParams cache_clear +#' @family cache managers +#' @export +cache_activate <- function(cache_name = NULL) { + assert_R.cache_installation(installation_only = TRUE) + if (!is.null(cache_name)) { + options("styler.cache_name" = cache_name) + } else { + options("styler.cache_name" = cache_derive_name()) + } + path <- cache_find_path(cache_name) + cat( + "Using cache ", cache_get_name(), " at ", + path, ".\n", + sep = "" + ) + invisible(path) +} + +#' @rdname cache_activate +#' @export +cache_deactivate <- function(verbose = TRUE) { + options("styler.cache_name" = NULL) + + if (verbose) { + cat("Deactivated cache.\n") + } +} diff --git a/R/ui.R b/R/ui-styling.R similarity index 71% rename from R/ui.R rename to R/ui-styling.R index 548672939..02d2a84c3 100644 --- a/R/ui.R +++ b/R/ui-styling.R @@ -242,118 +242,3 @@ style_file <- function(path, changed <- transform_files(path, transformers, include_roxygen_examples) invisible(changed) } - -#' Clear the cache -#' -#' Clears the cache that stores which files' styling is up-to-date. You won't be -#' able to undo this. Note that the file corresponding to the cache (a folder -#' on our file stystem) won't be deleted, but it will be empty after calling -#' `cache_clear`. -#' @param cache_name The name of the cache to use. If -#' `NULL`, the option "styler.cache_name" is considered which defaults to -#' the version of styler used. -#' @details -#' Each version of styler has it's own cache by default, because styling is -#' potentially different with different versions of styler. -#' @param ask Whether or not to interactively ask the user again. -#' @family cache managers -#' @export -cache_clear <- function(cache_name = NULL, ask = TRUE) { - assert_R.cache_installation(installation_only = TRUE) - path_cache <- cache_find_path(cache_name) - R.cache::clearCache(path_cache, prompt = ask) - cache_deactivate(verbose = FALSE) - -} - -#' Show information about the styler cache -#' -#' Gives information about the cache -#' @param cache_name The name of the cache for which to show details. If -#' `NULL`, the option "styler.cache_name" is considered which defaults to -#' the version of styler used. -#' @param format Either "lucid" for a summary emitted with [base::cat()] or -#' "tabular" for a tabular summary from [base::file.info()] or "both" for -#' both. -#' @family cache managers -#' @export -cache_info <- function(cache_name = NULL, format = "both") { - assert_R.cache_installation(installation_only = TRUE) - rlang::arg_match(format, c("tabular", "lucid", "both")) - path_cache <- cache_find_path(cache_name) - files <- list.files(path_cache, full.names = TRUE) - file_info <- file_info(files) - tbl <- tibble( - n = nrow(file_info), - size = sum(file_info$size), - last_modified = suppressWarnings(max(file_info$mtime)), - created = file.info(path_cache)$ctime, - location = path_cache, - activated = cache_is_activated(cache_name) - ) - if (format %in% c("lucid", "both")) { - cat( - "Size:\t\t", tbl$size, " bytes (", tbl$n, " cached expressions)", - "\nLast modified:\t", as.character(tbl$last_modified), - "\nCreated:\t", as.character(tbl$created), - "\nLocation:\t", path_cache, - "\nActivated:\t", tbl$activated, - sep = "" - ) - } - if (format == "tabular") { - tbl - } else if (format == "both") { - invisible(tbl) - } -} - -#' Activate or deactivate the styler cache -#' -#' Helper functions to control the behavior of caching. Simple wrappers around -#' [base::options()]. -#' @inheritParams cache_clear -#' @family cache managers -#' @export -cache_activate <- function(cache_name = NULL) { - assert_R.cache_installation(installation_only = TRUE) - if (!is.null(cache_name)) { - options("styler.cache_name" = cache_name) - } else { - options("styler.cache_name" = cache_derive_name()) - } - path <- cache_find_path(cache_name) - cat( - "Using cache ", cache_get_name(), " at ", - path, ".\n", - sep = "" - ) - invisible(path) -} - -#' @rdname cache_activate -#' @export -cache_deactivate <- function(verbose = TRUE) { - options("styler.cache_name" = NULL) - - if (verbose) { - cat("Deactivated cache.\n") - } -} - -#' Check if a cache is activated -#' -#' @param cache_name The name of the cache to check. If `NULL`, we check if -#' any cache is activated. If not `NULL`, we check if a specific cache is -#' activated. -#' @keywords internal -cache_is_activated <- function(cache_name = NULL) { - current_cache <- cache_get_name() - if (is.null(cache_name)) { - !is.null(current_cache) - } else if (!is.null(current_cache)) { - cache_name == current_cache - } else { - FALSE - } -} diff --git a/R/utils-cache.R b/R/utils-cache.R index f51d844d4..349e30992 100644 --- a/R/utils-cache.R +++ b/R/utils-cache.R @@ -18,6 +18,23 @@ cache_find_path <- function(cache_name = NULL) { R.cache::getCachePath(c("styler", cache_name)) } +#' Check if a cache is activated +#' +#' @param cache_name The name of the cache to check. If `NULL`, we check if +#' any cache is activated. If not `NULL`, we check if a specific cache is +#' activated. +#' @keywords internal +cache_is_activated <- function(cache_name = NULL) { + current_cache <- cache_get_name() + if (is.null(cache_name)) { + !is.null(current_cache) + } else if (!is.null(current_cache)) { + cache_name == current_cache + } else { + FALSE + } +} + cache_derive_name <- function() { utils::packageDescription("styler", fields = "Version") } @@ -35,3 +52,4 @@ cache_get_or_derive_name <- function(cache_name) { } cache_name } + diff --git a/man/cache_activate.Rd b/man/cache_activate.Rd index cde7bdf64..e0ed28ce3 100644 --- a/man/cache_activate.Rd +++ b/man/cache_activate.Rd @@ -1,5 +1,5 @@ % Generated by roxygen2: do not edit by hand -% Please edit documentation in R/ui.R +% Please edit documentation in R/ui-caching.R \name{cache_activate} \alias{cache_activate} \alias{cache_deactivate} @@ -10,7 +10,7 @@ cache_activate(cache_name = NULL) cache_deactivate(verbose = TRUE) } \arguments{ -\item{cache_name}{The name of the cache to use. If +\item{cache_name}{The name of the styler cache to use. If \code{NULL}, the option "styler.cache_name" is considered which defaults to the version of styler used.} } diff --git a/man/cache_clear.Rd b/man/cache_clear.Rd index d2ed6fe24..407bff6ee 100644 --- a/man/cache_clear.Rd +++ b/man/cache_clear.Rd @@ -1,5 +1,5 @@ % Generated by roxygen2: do not edit by hand -% Please edit documentation in R/ui.R +% Please edit documentation in R/ui-caching.R \name{cache_clear} \alias{cache_clear} \title{Clear the cache} @@ -7,7 +7,7 @@ cache_clear(cache_name = NULL, ask = TRUE) } \arguments{ -\item{cache_name}{The name of the cache to use. If +\item{cache_name}{The name of the styler cache to use. If \code{NULL}, the option "styler.cache_name" is considered which defaults to the version of styler used.} @@ -16,7 +16,7 @@ the version of styler used.} \description{ Clears the cache that stores which files' styling is up-to-date. You won't be able to undo this. Note that the file corresponding to the cache (a folder -on our file stystem) won't be deleted, but it will be empty after calling +on your file stystem) won't be deleted, but it will be empty after calling \code{cache_clear}. } \details{ diff --git a/man/cache_find_path.Rd b/man/cache_find_path.Rd index 3038c8acf..3d7d0c673 100644 --- a/man/cache_find_path.Rd +++ b/man/cache_find_path.Rd @@ -7,7 +7,7 @@ cache_find_path(cache_name = NULL) } \arguments{ -\item{cache_name}{The name of the cache to use. If +\item{cache_name}{The name of the styler cache to use. If \code{NULL}, the option "styler.cache_name" is considered which defaults to the version of styler used.} } diff --git a/man/cache_info.Rd b/man/cache_info.Rd index ab7ec4462..3a6c156aa 100644 --- a/man/cache_info.Rd +++ b/man/cache_info.Rd @@ -1,5 +1,5 @@ % Generated by roxygen2: do not edit by hand -% Please edit documentation in R/ui.R +% Please edit documentation in R/ui-caching.R \name{cache_info} \alias{cache_info} \title{Show information about the styler cache} @@ -11,12 +11,12 @@ cache_info(cache_name = NULL, format = "both") \code{NULL}, the option "styler.cache_name" is considered which defaults to the version of styler used.} -\item{format}{Either "lucid" for a summary emitted with \code{\link[base:cat]{base::cat()}} or +\item{format}{Either "lucid" for a summary emitted with \code{\link[base:cat]{base::cat()}}, "tabular" for a tabular summary from \code{\link[base:file.info]{base::file.info()}} or "both" for both.} } \description{ -Gives information about the cache +Gives information about the cache. } \seealso{ Other cache managers: \code{\link{cache_activate}}, diff --git a/man/cache_is_activated.Rd b/man/cache_is_activated.Rd index b600a42a4..ca3031b78 100644 --- a/man/cache_is_activated.Rd +++ b/man/cache_is_activated.Rd @@ -1,5 +1,5 @@ % Generated by roxygen2: do not edit by hand -% Please edit documentation in R/ui.R +% Please edit documentation in R/utils-cache.R \name{cache_is_activated} \alias{cache_is_activated} \title{Check if a cache is activated} diff --git a/man/prettify_any.Rd b/man/prettify_any.Rd index e6ffed554..17f9b47cd 100644 --- a/man/prettify_any.Rd +++ b/man/prettify_any.Rd @@ -1,5 +1,5 @@ % Generated by roxygen2: do not edit by hand -% Please edit documentation in R/ui.R +% Please edit documentation in R/ui-styling.R \name{prettify_any} \alias{prettify_any} \title{Prettify R code in current working directory} diff --git a/man/style_dir.Rd b/man/style_dir.Rd index 1affe1753..a23d2b1b7 100644 --- a/man/style_dir.Rd +++ b/man/style_dir.Rd @@ -1,5 +1,5 @@ % Generated by roxygen2: do not edit by hand -% Please edit documentation in R/ui.R +% Please edit documentation in R/ui-styling.R \name{style_dir} \alias{style_dir} \title{Prettify arbitrary R code} diff --git a/man/style_file.Rd b/man/style_file.Rd index a55c6fe9d..8d6d22088 100644 --- a/man/style_file.Rd +++ b/man/style_file.Rd @@ -1,5 +1,5 @@ % Generated by roxygen2: do not edit by hand -% Please edit documentation in R/ui.R +% Please edit documentation in R/ui-styling.R \name{style_file} \alias{style_file} \title{Style \code{.R}, \code{.Rmd} or \code{.Rnw} files} diff --git a/man/style_pkg.Rd b/man/style_pkg.Rd index 044fe21f2..236074b11 100644 --- a/man/style_pkg.Rd +++ b/man/style_pkg.Rd @@ -1,5 +1,5 @@ % Generated by roxygen2: do not edit by hand -% Please edit documentation in R/ui.R +% Please edit documentation in R/ui-styling.R \name{style_pkg} \alias{style_pkg} \title{Prettify R source code} diff --git a/man/style_text.Rd b/man/style_text.Rd index 3e9f9bc91..105722468 100644 --- a/man/style_text.Rd +++ b/man/style_text.Rd @@ -1,5 +1,5 @@ % Generated by roxygen2: do not edit by hand -% Please edit documentation in R/ui.R +% Please edit documentation in R/ui-styling.R \name{style_text} \alias{style_text} \title{Style a string} From 18807d14271aaa874edfa9315d85f34a7337ca10 Mon Sep 17 00:00:00 2001 From: lorenzwalthert Date: Thu, 22 Aug 2019 11:46:02 +0200 Subject: [PATCH 30/67] test that after clearing a cache, no cache is active. --- tests/testthat/test-cache-with-r-cache.R | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/testthat/test-cache-with-r-cache.R b/tests/testthat/test-cache-with-r-cache.R index 47ff440a2..92e24d1c6 100644 --- a/tests/testthat/test-cache-with-r-cache.R +++ b/tests/testthat/test-cache-with-r-cache.R @@ -17,6 +17,8 @@ capture.output(test_that("Cache management works when R.cache is installed", { skip_if_not_installed("R.cache") on.exit(clear_testthat_cache()) clear_testthat_cache() + # clearing a cache inactivates the caching functionality. + expect_false(cache_info(format = "tabular")$activated) cache_activate("testthat") # at fresh startup, with R.cache installed expect_s3_class(cache_info(format = "tabular"), "tbl_df") From 2f85b126432db3fc0772504b0b77b65cb6069f61 Mon Sep 17 00:00:00 2001 From: lorenzwalthert Date: Thu, 22 Aug 2019 18:45:58 +0200 Subject: [PATCH 31/67] describe caching. --- README.Rmd | 10 ++++++++++ README.md | 11 +++++++++++ 2 files changed, 21 insertions(+) diff --git a/README.Rmd b/README.Rmd index 1a882d7d2..5e12b95e9 100644 --- a/README.Rmd +++ b/README.Rmd @@ -119,6 +119,16 @@ not flexible enough for you, you can implement your own style guide, as explained in the corresponding [vignette](https://styler.r-lib.org/articles/customizing_styler.html). +**caching** + +In styler 1.1.1,9004, caching was introduced, which makes repeated styling +almost instantaneous. By default, it's enabled, but you need to have the +`R.cache` package installed. At first use, `R.cache` will ask you to let it +create a permanent cache on your file system that styler will use. This is needed +if you want to cache across R sessions and not just within. The cache is +specific to a version of styler by default, because different versions +potentially format code differently. See `?styler::cache_info()` for more +details on how to configure caching. ## Adaption of styler diff --git a/README.md b/README.md index fe21f1ab3..6bd9dc7c4 100644 --- a/README.md +++ b/README.md @@ -113,6 +113,17 @@ this is not flexible enough for you, you can implement your own style guide, as explained in the corresponding [vignette](https://styler.r-lib.org/articles/customizing_styler.html). +**caching** + +In styler 1.1.1,9004, caching was introduced, which makes repeated +styling almost instantaneous. By default, it’s enabled, but you need to +have the `R.cache` package installed. At first use, `R.cache` will ask +you to let it create a permanent cache on your file system that styler +will use. This is needed if you want to cache across R sessions and not +just within. The cache is specific to a version of styler by default, +because different versions potentially format code differently. See +`?styler::cache_info()` for more details on how to configure caching. + ## Adaption of styler styler functionality is made available through other packages, most From cea1922c76602b4b77c5ebe3b36261a5bd2df22b Mon Sep 17 00:00:00 2001 From: lorenzwalthert Date: Thu, 22 Aug 2019 19:17:05 +0200 Subject: [PATCH 32/67] cache_activate gains new argument verbose for consistency. --- API | 2 +- R/ui-caching.R | 17 ++++++++++------- man/cache_activate.Rd | 5 ++++- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/API b/API index b45a5f159..eb4538efb 100644 --- a/API +++ b/API @@ -2,7 +2,7 @@ ## Exported functions -cache_activate(cache_name = NULL) +cache_activate(cache_name = NULL, verbose = TRUE) cache_clear(cache_name = NULL, ask = TRUE) cache_deactivate(verbose = TRUE) cache_info(cache_name = NULL, format = "both") diff --git a/R/ui-caching.R b/R/ui-caching.R index fa82cb492..f1544c90b 100644 --- a/R/ui-caching.R +++ b/R/ui-caching.R @@ -18,7 +18,6 @@ cache_clear <- function(cache_name = NULL, ask = TRUE) { path_cache <- cache_find_path(cache_name) R.cache::clearCache(path_cache, prompt = ask) cache_deactivate(verbose = FALSE) - } #' Show information about the styler cache @@ -68,9 +67,11 @@ cache_info <- function(cache_name = NULL, format = "both") { #' Helper functions to control the behavior of caching. Simple wrappers around #' [base::options()]. #' @inheritParams cache_clear +#' @param verbose Whether or not to print an informative message about what the +#' function is doing. #' @family cache managers #' @export -cache_activate <- function(cache_name = NULL) { +cache_activate <- function(cache_name = NULL, verbose = TRUE) { assert_R.cache_installation(installation_only = TRUE) if (!is.null(cache_name)) { options("styler.cache_name" = cache_name) @@ -78,11 +79,13 @@ cache_activate <- function(cache_name = NULL) { options("styler.cache_name" = cache_derive_name()) } path <- cache_find_path(cache_name) - cat( - "Using cache ", cache_get_name(), " at ", - path, ".\n", - sep = "" - ) + if (verbose) { + cat( + "Using cache ", cache_get_name(), " at ", + path, ".\n", + sep = "" + ) + } invisible(path) } diff --git a/man/cache_activate.Rd b/man/cache_activate.Rd index e0ed28ce3..e7ba9ac7c 100644 --- a/man/cache_activate.Rd +++ b/man/cache_activate.Rd @@ -5,7 +5,7 @@ \alias{cache_deactivate} \title{Activate or deactivate the styler cache} \usage{ -cache_activate(cache_name = NULL) +cache_activate(cache_name = NULL, verbose = TRUE) cache_deactivate(verbose = TRUE) } @@ -13,6 +13,9 @@ cache_deactivate(verbose = TRUE) \item{cache_name}{The name of the styler cache to use. If \code{NULL}, the option "styler.cache_name" is considered which defaults to the version of styler used.} + +\item{verbose}{Whether or not to print an informative message about what the +function is doing.} } \description{ Helper functions to control the behavior of caching. Simple wrappers around From 66b276df8b9b5da1dace6192969d133c4bc46538 Mon Sep 17 00:00:00 2001 From: lorenzwalthert Date: Thu, 22 Aug 2019 19:27:30 +0200 Subject: [PATCH 33/67] tidy description. --- DESCRIPTION | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 0f8a77570..33386abbd 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,5 +1,5 @@ -Package: styler Type: Package +Package: styler Title: Non-Invasive Pretty Printing of R Code Version: 1.2.0.9000 Authors@R: @@ -34,11 +34,12 @@ Suggests: here, knitr, prettycode, + R.cache, rmarkdown, rstudioapi (>= 0.7), - testthat (>= 2.1.0), - R.cache -VignetteBuilder: knitr + testthat (>= 2.1.0) +VignetteBuilder: + knitr Encoding: UTF-8 LazyData: true Roxygen: list(markdown = TRUE, roclets = c("rd", "namespace", From 8c2c7edaeaf01e9568fc6fc970a586582b73fa1c Mon Sep 17 00:00:00 2001 From: lorenzwalthert Date: Thu, 22 Aug 2019 19:46:57 +0200 Subject: [PATCH 34/67] don't add another style_file that convolutes full text search and "go to file/function". --- tests/testthat/reference-objects/caching.R | 49 ++++++++-------------- 1 file changed, 18 insertions(+), 31 deletions(-) diff --git a/tests/testthat/reference-objects/caching.R b/tests/testthat/reference-objects/caching.R index 8e92f97cd..fbb8aa47e 100644 --- a/tests/testthat/reference-objects/caching.R +++ b/tests/testthat/reference-objects/caching.R @@ -1,26 +1,12 @@ -#' Style `.R` and/or `.Rmd` files +#' CHan deng #' -#' Performs various substitutions in the files specified. -#' Carefully examine the results after running this function! -#' @param path A character vector with paths to files to style. -#' @inheritParams style_pkg -#' @inheritSection transform_files Value -#' @inheritSection style_pkg Warning -#' @inheritSection style_pkg Roundtrip Validation +#' Performs various izil #' @examples -#' # the following is identical but the former is more convenient: -#' file <- tempfile("styler", -#' fileext = ".R" -#' ) +#' zz + 1 #' \dontrun{ -#' xfun::write_utf8("1++1", file) +#' xfun::xxio(fun(77), file) #' } -#' style_file( -#' file, -#' style = tidyverse_style, strict = TRUE -#' ) -#' style_file(file, transformers = tidyverse_style(strict = TRUE)) -#' xfun::read_utf8(file) +#' dplyr::filter(x == 3, zz = max(.data$`5`, na.rom = TRUE)) #' \dontrun{ #' unlink(file2) #' } @@ -30,16 +16,17 @@ #' } #' unlink(file2) #' } -#' @family stylers -#' @export -style_file <- function(path, - ..., - style = tidyverse_style, - transformers = style(...), - include_roxygen_examples = TRUE) { - changed <- withr::with_dir( - dirname(path), - transform_files(basename(path), transformers) - ) - invisible(changed) +xxtt <- function(bli, bla, blup = 3) { + changed <- withr::tzu( + zname(path), + condense_files(x_basename(path ), c_transformers) + ); visible(chan) +} + +g = 33 + +z <- fun(g, z = xxtt) + +if (not(x)== 9) { + cache_this_file() } From 2fc8808e13c2051e435634ebbd34261e6a7bf6c0 Mon Sep 17 00:00:00 2001 From: lorenzwalthert Date: Thu, 22 Aug 2019 19:48:20 +0200 Subject: [PATCH 35/67] minor fixes and simplification --- R/communicate.R | 5 +++-- R/transform-files.R | 4 ++-- R/ui-caching.R | 8 +++++--- R/utils-cache.R | 12 +++++++----- R/utils-files.R | 5 ----- man/assert_R.cache_installation.Rd | 2 +- man/cache_info.Rd | 4 ++-- man/hash_standardize.Rd | 7 +++---- 8 files changed, 23 insertions(+), 24 deletions(-) diff --git a/R/communicate.R b/R/communicate.R index 408c492b6..344673f99 100644 --- a/R/communicate.R +++ b/R/communicate.R @@ -35,6 +35,8 @@ assert_data.tree_installation <- function() { } } +#' Assert the R.cache installation in conjunction with the cache config +#' #' R.cache needs to be installed if caching functionality is enabled #' @param installation_only Whether or not to only check if R.cache is #' installed. @@ -60,8 +62,7 @@ assert_R.cache_installation <- function(installation_only = FALSE, } else { rlang::warn(paste0( msg_basic, " ", - "Deactivating the caching feature for the current session.", - "" + "Deactivating the caching feature for the current session." )) cache_deactivate(verbose = FALSE) } diff --git a/R/transform-files.R b/R/transform-files.R index 516d9e2db..a2a3e439b 100644 --- a/R/transform-files.R +++ b/R/transform-files.R @@ -83,8 +83,8 @@ make_transformer <- function(transformers, R.cache::findCache(key = hash_standardize(text), dir = cache_dir) ) should_use_cache <- cache_is_activated() - can_use_cache <- is_cached && should_use_cache - if (!can_use_cache) { + use_cache <- is_cached && should_use_cache + if (!use_cache) { transformed_code <- text %>% parse_transform_serialize_r(transformers, warn_empty = warn_empty) %>% when( diff --git a/R/ui-caching.R b/R/ui-caching.R index f1544c90b..e6b1c180f 100644 --- a/R/ui-caching.R +++ b/R/ui-caching.R @@ -24,8 +24,8 @@ cache_clear <- function(cache_name = NULL, ask = TRUE) { #' #' Gives information about the cache. #' @param cache_name The name of the cache for which to show details. If -#' `NULL`, the option "styler.cache_name" is considered which defaults to -#' the version of styler used. +#' `NULL`, the active cache is used. If none is active the cache corresponding +#' to the installed styler version is used. #' @param format Either "lucid" for a summary emitted with [base::cat()], #' "tabular" for a tabular summary from [base::file.info()] or "both" for #' both. @@ -36,7 +36,8 @@ cache_info <- function(cache_name = NULL, format = "both") { rlang::arg_match(format, c("tabular", "lucid", "both")) path_cache <- cache_find_path(cache_name) files <- list.files(path_cache, full.names = TRUE) - file_info <- file_info(files) + file_info <- file.info(files) %>% + as_tibble() tbl <- tibble( n = nrow(file_info), size = sum(file_info$size), @@ -52,6 +53,7 @@ cache_info <- function(cache_name = NULL, format = "both") { "\nCreated:\t", as.character(tbl$created), "\nLocation:\t", path_cache, "\nActivated:\t", tbl$activated, + "\n", sep = "" ) } diff --git a/R/utils-cache.R b/R/utils-cache.R index 349e30992..fa351f238 100644 --- a/R/utils-cache.R +++ b/R/utils-cache.R @@ -1,11 +1,13 @@ +#' Standardize text for hashing +#' #' Make sure text after styling results in the same hash as text before styling #' if it is indeed identical. -#' @param x A character vector. +#' @param text A character vector. #' @keywords internal -hash_standardize <- function(x) { - x <- ensure_last_is_empty(x) - Encoding(x) <- "UTF-8" - list(x) +hash_standardize <- function(text) { + text <- ensure_last_is_empty(text) + Encoding(text) <- "UTF-8" + list(text) } #' Where is the cache? diff --git a/R/utils-files.R b/R/utils-files.R index 7e326775d..d1c0c6db4 100644 --- a/R/utils-files.R +++ b/R/utils-files.R @@ -26,8 +26,3 @@ is_unsaved_file <- function(path) { map_filetype_to_pattern <- function(filetype) { paste0("(", paste(set_and_assert_arg_filetype(filetype), collapse = "|"), ")$") } - -file_info <- function(path) { - file.info(path) %>% - as_tibble() -} diff --git a/man/assert_R.cache_installation.Rd b/man/assert_R.cache_installation.Rd index c9d5d9166..a6c7a7b2b 100644 --- a/man/assert_R.cache_installation.Rd +++ b/man/assert_R.cache_installation.Rd @@ -2,7 +2,7 @@ % Please edit documentation in R/communicate.R \name{assert_R.cache_installation} \alias{assert_R.cache_installation} -\title{R.cache needs to be installed if caching functionality is enabled} +\title{Assert the R.cache installation in conjunction with the cache config} \usage{ assert_R.cache_installation(installation_only = FALSE, action = "abort") diff --git a/man/cache_info.Rd b/man/cache_info.Rd index 3a6c156aa..75261490e 100644 --- a/man/cache_info.Rd +++ b/man/cache_info.Rd @@ -8,8 +8,8 @@ cache_info(cache_name = NULL, format = "both") } \arguments{ \item{cache_name}{The name of the cache for which to show details. If -\code{NULL}, the option "styler.cache_name" is considered which defaults to -the version of styler used.} +\code{NULL}, the active cache is used. If none is active the cache corresponding +to the installed styler version is used.} \item{format}{Either "lucid" for a summary emitted with \code{\link[base:cat]{base::cat()}}, "tabular" for a tabular summary from \code{\link[base:file.info]{base::file.info()}} or "both" for diff --git a/man/hash_standardize.Rd b/man/hash_standardize.Rd index 546dc1300..8f210cc1e 100644 --- a/man/hash_standardize.Rd +++ b/man/hash_standardize.Rd @@ -2,13 +2,12 @@ % Please edit documentation in R/utils-cache.R \name{hash_standardize} \alias{hash_standardize} -\title{Make sure text after styling results in the same hash as text before styling -if it is indeed identical.} +\title{Standardize text for hashing} \usage{ -hash_standardize(x) +hash_standardize(text) } \arguments{ -\item{x}{A character vector.} +\item{text}{A character vector.} } \description{ Make sure text after styling results in the same hash as text before styling From dd19539aad8fabe87cb0a7311b9b6df30c1b1179 Mon Sep 17 00:00:00 2001 From: lorenzwalthert Date: Thu, 22 Aug 2019 19:51:49 +0200 Subject: [PATCH 36/67] assume R_REMOVE_RCACHE is false when not set to true. --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 4e7aff5fd..b807f9d6e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,7 +3,7 @@ # Usually you shouldn't need to change the first part of the file # DO NOT CHANGE THE CODE BELOW -before_install: R -q -e 'install.packages(c("remotes", "curl", "knitr", "rmarkdown")); remotes::install_github("ropenscilabs/tic"); tic::prepare_all_stages(); remotes::install_deps(dependencies = TRUE); if (as.logical(toupper(Sys.getenv("R_REMOVE_RCACHE")))) remove.packages("R.cache"); tic::before_install()' +before_install: R -q -e 'install.packages(c("remotes", "curl", "knitr", "rmarkdown")); remotes::install_github("ropenscilabs/tic"); tic::prepare_all_stages(); remotes::install_deps(dependencies = TRUE); if (isTRUE(as.logical(toupper(Sys.getenv("R_REMOVE_RCACHE"))))) remove.packages("R.cache"); tic::before_install()' install: R -q -e 'tic::install()' after_install: R -q -e 'tic::after_install()' before_script: R -q -e 'tic::before_script()' From 2848ac1cff691947ca5fc5c4cf15c67e07002d06 Mon Sep 17 00:00:00 2001 From: lorenzwalthert Date: Thu, 22 Aug 2019 19:55:28 +0200 Subject: [PATCH 37/67] make file less nicely styled --- tests/testthat/reference-objects/caching.R | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/testthat/reference-objects/caching.R b/tests/testthat/reference-objects/caching.R index fbb8aa47e..a121ebd67 100644 --- a/tests/testthat/reference-objects/caching.R +++ b/tests/testthat/reference-objects/caching.R @@ -19,14 +19,15 @@ xxtt <- function(bli, bla, blup = 3) { changed <- withr::tzu( zname(path), - condense_files(x_basename(path ), c_transformers) - ); visible(chan) + condense_files(x_basename(path), c_transformers) + ) + visible(chan) } -g = 33 +g <- 33 z <- fun(g, z = xxtt) -if (not(x)== 9) { +if (not(x) == 9) { cache_this_file() } From 6eba44905990a0cbb7506f46c9db734c88414505 Mon Sep 17 00:00:00 2001 From: lorenzwalthert Date: Thu, 22 Aug 2019 22:55:16 +0200 Subject: [PATCH 38/67] round values to make tests pass on all plattforms. --- tests/testthat/reference-objects/cache-info-2 | Bin 148 -> 147 bytes tests/testthat/reference-objects/cache-info-3 | Bin 99 -> 152 bytes tests/testthat/test-cache-with-r-cache.R | 7 +++++-- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/testthat/reference-objects/cache-info-2 b/tests/testthat/reference-objects/cache-info-2 index 93a77023ef24d9a7a9640b815e609deac9d91a3e..6a0a59fef3d9b47113d818d97e9200b9d9bb63db 100644 GIT binary patch delta 116 zcmV-)0E_>W0h0lcCT+<816&9>7A7#w!oUfnS@RNeQ;UHN0k9zuLB>2NgQYmLDizA+ zOiV7xEK4j&O+hn)vnan@4`vL=4NMI{;y)0e+m)P?Sd4BLTS-z*d`cRW#|-2_nOrG} WC5d`zML>gryng^&J#Rkt0002Cqbm#m delta 117 zcmV-*0E+*U0h9rdCT?+n0SdSfax6?>nuUQANVDc8=B5?{83JHKAcBl}PzFnJW>qSb z&6${7l3A8mlA3~M1ZPoxxgN|IkQG(u#lv19|@dW)4HB_5c6?Xx%O{ diff --git a/tests/testthat/reference-objects/cache-info-3 b/tests/testthat/reference-objects/cache-info-3 index 742bca3f413a46838018fcb1b436497a1c5d07fe..872cd474f9768721b9eb86ff641c28733d6f3e5b 100644 GIT binary patch delta 135 zcmV;20C@jnm;sO&f3q-%0AgMsW(3PIFz|uc4jmg9pnwZ12gEE)V48)26G*e>CFZ6U z0~rEfLm+~Tc~AyRab{I2l+Br#T#{LqSdyB8W&~$Zez_jZ7?2y78i2%qAV9Y(IVZ6g p-7dD0q@4JaG$@Z5$b&MuQW8rN_0o!f1_OEj0C&$nvi1M~0RXwzHB$fp delta 81 zcmV-X0IvU-0b`IDOM?KI&%(e7q}78nt5U5vIn6DN6p|`SQi~Ndj1-a+lQU9N6jCb+ nic*V-{cf Date: Thu, 22 Aug 2019 23:20:13 +0200 Subject: [PATCH 39/67] round two digits --- tests/testthat/reference-objects/cache-info-3 | Bin 152 -> 151 bytes tests/testthat/test-cache-with-r-cache.R | 4 ++-- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/testthat/reference-objects/cache-info-3 b/tests/testthat/reference-objects/cache-info-3 index 872cd474f9768721b9eb86ff641c28733d6f3e5b..0ab22efd3800afc60c1fb049372d8d64bd392535 100644 GIT binary patch delta 118 zcmV-+0Ez#Y0ha-gC~ut%Fu(k4M5^Q5TM(YoRe6LZWmigQciqI8kENjPh@e;`1&D>)~z7~L+mlBAsYlr$)h8OVb& Zxl$5K67|xGfCdA3{{VN-KeF}!005D3FHisg diff --git a/tests/testthat/test-cache-with-r-cache.R b/tests/testthat/test-cache-with-r-cache.R index 76b7be09d..62ffc84a9 100644 --- a/tests/testthat/test-cache-with-r-cache.R +++ b/tests/testthat/test-cache-with-r-cache.R @@ -78,14 +78,14 @@ capture.output(test_that("cached expressions are displayed propperly", { cache_activate("testthat") style_text("1+1") cache_info <- cache_info(format = "tabular") - cache_info$size <- round(cache_info$size, -1) + cache_info$size <- round(cache_info$size, -2) expect_known_value( cache_info[, c("n", "size", "activated")], file = test_path("reference-objects/cache-info-2") ) style_text("a <-function() NULL") cache_info <- cache_info(format = "tabular") - cache_info$size <- round(cache_info$size, -1) + cache_info$size <- round(cache_info$size, -2) expect_known_value( cache_info[, c("n", "size", "activated")], file = test_path("reference-objects/cache-info-3") From fac1558d72c5c0488bc8f5654b2bbe6bcba66030 Mon Sep 17 00:00:00 2001 From: lorenzwalthert Date: Fri, 23 Aug 2019 12:52:42 +0200 Subject: [PATCH 40/67] style with stylermd --- NEWS.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/NEWS.md b/NEWS.md index f65c1946d..8e8e167b8 100644 --- a/NEWS.md +++ b/NEWS.md @@ -12,9 +12,9 @@ ## New features -* Aligned function calls are detected and kept as is if they match the styler - [definition for aligned function calls](https://styler.r-lib.org/articles/detect-alignment.html) - (#537). +* Aligned function calls are detected and kept as is if they match the styler + [definition for aligned function + calls](https://styler.r-lib.org/articles/detect-alignment.html) (#537). * curly-curly (`{{`) syntactic sugar introduced with rlang 0.4.0 is now explicitly handled, where previously it was just treated as two consecutive From 346f76b75aa6ff484eb317f78106885fa88388f4 Mon Sep 17 00:00:00 2001 From: lorenzwalthert Date: Fri, 23 Aug 2019 14:07:12 +0200 Subject: [PATCH 41/67] cache NULL, not the sysdate, which can also be derived from the creation time of the hash on the file system. --- R/transform-files.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/transform-files.R b/R/transform-files.R index a2a3e439b..a0a5702fe 100644 --- a/R/transform-files.R +++ b/R/transform-files.R @@ -94,7 +94,7 @@ make_transformer <- function(transformers, ) if (should_use_cache) { R.cache::saveCache( - Sys.time(), + NULL, key = hash_standardize(transformed_code), dir = cache_dir ) From 813d4e8a9d4cfdb44fcb9d3210fa6199ebc5f7d8 Mon Sep 17 00:00:00 2001 From: lorenzwalthert Date: Fri, 23 Aug 2019 14:07:53 +0200 Subject: [PATCH 42/67] add news section on caching. --- NEWS.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/NEWS.md b/NEWS.md index 8e8e167b8..15ffe83b0 100644 --- a/NEWS.md +++ b/NEWS.md @@ -12,6 +12,13 @@ ## New features +* styler caches results of styling, so applying styler to code it has styled + before will be instantaneous. This brings large speed boosts in many + situations, e.g. when `style_pkg()` is run but only a few files have changed + since the last styling or when using the [styler pre-commit + hook](https://github.com/lorenzwalthert/pre-commit-hooks). See the README for + details (#538). + * Aligned function calls are detected and kept as is if they match the styler [definition for aligned function calls](https://styler.r-lib.org/articles/detect-alignment.html) (#537). From f170c27327a157a5fe6ea0c6b62c97338a83edf7 Mon Sep 17 00:00:00 2001 From: lorenzwalthert Date: Thu, 29 Aug 2019 22:54:43 +0200 Subject: [PATCH 43/67] adapt test for hashing NULL --- tests/testthat/reference-objects/cache-info-2 | Bin 147 -> 148 bytes tests/testthat/reference-objects/cache-info-3 | Bin 151 -> 152 bytes 2 files changed, 0 insertions(+), 0 deletions(-) diff --git a/tests/testthat/reference-objects/cache-info-2 b/tests/testthat/reference-objects/cache-info-2 index 6a0a59fef3d9b47113d818d97e9200b9d9bb63db..adf64e002494e0338c941b70912abef9d209e9ba 100644 GIT binary patch delta 117 zcmV-*0E+*U0h9rdCT?_q0SdSfax6?>nuUQANVDc8=B5?{83JHKAcBl}PzFnJW>qSb z&6${7l3A8mlA3~M1ZPoxxgN|IkQG(u#lv19|@dTXfRH_5c6?aC$E~ delta 116 zcmV-)0E_>W0h0lcCT+<816&9>7A7#w!oUfnS@RNeQ;UHN0k9zuLB>2NgQYmLDizA+ zOiV7xEK4j&O+hn)vnan@4`vL=4NMI{;y)0e+m)P?Sd4BLTS-z*d`cRW#|-2_nOrG} WC5d`zML>gryng^&J#Rkt0002Cqbm#m diff --git a/tests/testthat/reference-objects/cache-info-3 b/tests/testthat/reference-objects/cache-info-3 index 0ab22efd3800afc60c1fb049372d8d64bd392535..dc77f0e5eeb3ac643408579675457b06f6b15d79 100644 GIT binary patch delta 119 zcmV--0EqvW0hj@hC~!>&7@&X)DhI?YOkkRYffGox<|XE)76TaqU_&5+jCoK7OL1mZ zDwNHcm|T)smRORSf@TC~QGU4|%ovaxm>Ph@e;`1&D>)~z7~L+mlBAsYlr$)h8OVb& Zxl$5K67|xGfCdA3{{VA2ibeJS005aRF0cRq delta 118 zcmV-+0Ez#Y0ha-gC~ut%Fu(k4M5^Q5TM(YoRe6LZWmigQciqI8kENj Date: Tue, 10 Sep 2019 12:42:28 +0200 Subject: [PATCH 44/67] use the shallow cache only (needs my fork or R.cache). --- R/transform-files.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/transform-files.R b/R/transform-files.R index a0a5702fe..b09eb5a5d 100644 --- a/R/transform-files.R +++ b/R/transform-files.R @@ -96,7 +96,7 @@ make_transformer <- function(transformers, R.cache::saveCache( NULL, key = hash_standardize(transformed_code), - dir = cache_dir + dir = cache_dir, shallow = TRUE ) } transformed_code From 1b3814dd4082addab606232765d761f12f053e3c Mon Sep 17 00:00:00 2001 From: lorenzwalthert Date: Tue, 10 Sep 2019 13:40:26 +0200 Subject: [PATCH 45/67] use R.cache --- DESCRIPTION | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 33386abbd..7c9b057d9 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -27,14 +27,14 @@ Imports: tibble (>= 1.4.2), tools, withr (>= 1.0.0), - xfun (>= 0.1) + xfun (>= 0.1), + R.cache (>= 0.13.0.9000) Suggests: data.tree (>= 0.1.6), dplyr, here, knitr, prettycode, - R.cache, rmarkdown, rstudioapi (>= 0.7), testthat (>= 2.1.0) @@ -92,3 +92,5 @@ Collate: 'vertical.R' 'visit.R' 'zzz.R' +Remotes: + lorenzwalthert/R.cache@shallow From ab3ed59493c05b94f7b96102b11feb1cad17239a Mon Sep 17 00:00:00 2001 From: Lorenz Walthert Date: Mon, 23 Sep 2019 19:12:44 +0200 Subject: [PATCH 46/67] unrelated wording NEWS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Kirill Müller --- NEWS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index 15ffe83b0..624c1dfb2 100644 --- a/NEWS.md +++ b/NEWS.md @@ -19,7 +19,7 @@ hook](https://github.com/lorenzwalthert/pre-commit-hooks). See the README for details (#538). -* Aligned function calls are detected and kept as is if they match the styler +* Aligned function calls are detected and remain unchanged if they match the styler [definition for aligned function calls](https://styler.r-lib.org/articles/detect-alignment.html) (#537). From 9674200fdad827f7ace7ea762de8a4a58684b722 Mon Sep 17 00:00:00 2001 From: Lorenz Walthert Date: Mon, 23 Sep 2019 19:16:44 +0200 Subject: [PATCH 47/67] doc wording MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Kirill Müller --- R/ui-caching.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/ui-caching.R b/R/ui-caching.R index e6b1c180f..460052988 100644 --- a/R/ui-caching.R +++ b/R/ui-caching.R @@ -1,6 +1,6 @@ #' Clear the cache #' -#' Clears the cache that stores which files' styling is up-to-date. You won't be +#' Clears the cache that stores which files are already styled. You won't be #' able to undo this. Note that the file corresponding to the cache (a folder #' on your file stystem) won't be deleted, but it will be empty after calling #' `cache_clear`. From fe74f9ea66ddc8c44975a399bd85e57892e0b5b4 Mon Sep 17 00:00:00 2001 From: Lorenz Walthert Date: Mon, 23 Sep 2019 19:56:35 +0200 Subject: [PATCH 48/67] don't o$ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Kirill Müller --- R/utils-cache.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/utils-cache.R b/R/utils-cache.R index fa351f238..874303238 100644 --- a/R/utils-cache.R +++ b/R/utils-cache.R @@ -6,7 +6,7 @@ #' @keywords internal hash_standardize <- function(text) { text <- ensure_last_is_empty(text) - Encoding(text) <- "UTF-8" + text <- enc2utf8(text) list(text) } From b1cf70dda745da62006c3cd183b39fab46d966cd Mon Sep 17 00:00:00 2001 From: lorenzwalthert Date: Mon, 23 Sep 2019 22:09:34 +0200 Subject: [PATCH 49/67] derive cache more cheaply --- R/utils-cache.R | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/R/utils-cache.R b/R/utils-cache.R index 874303238..de8fc3da0 100644 --- a/R/utils-cache.R +++ b/R/utils-cache.R @@ -37,8 +37,10 @@ cache_is_activated <- function(cache_name = NULL) { } } +desc <- read.dcf("DESCRIPTION") + cache_derive_name <- function() { - utils::packageDescription("styler", fields = "Version") + unlist(unname(desc[, "Version"])) } cache_get_name <- function() { From f2b16a5999b84bf23f17222e79a2d552c7cb46a4 Mon Sep 17 00:00:00 2001 From: lorenzwalthert Date: Mon, 23 Sep 2019 22:10:23 +0200 Subject: [PATCH 50/67] chache size should be 0 with shallow option --- tests/testthat/reference-objects/cache-info-2 | Bin 148 -> 144 bytes tests/testthat/reference-objects/cache-info-3 | Bin 152 -> 148 bytes 2 files changed, 0 insertions(+), 0 deletions(-) diff --git a/tests/testthat/reference-objects/cache-info-2 b/tests/testthat/reference-objects/cache-info-2 index adf64e002494e0338c941b70912abef9d209e9ba..d4b0ce123ef4e5311dec10da8853bb948ca028c3 100644 GIT binary patch delta 116 zcmV-)0E_>W0gwTZBW)-R16&9>7A7#w!oUfnS@RNeQ;UHN0k9zuLB>2NgQYmLDizA+ zOiV7xEK4j&O+hn)vnan@4`vL=4NMI{;y)0e+m)P?Sd4BLTS-z*d`cRW#|-2_nOrG} WC5d`zML>gryng^If|h#r0001YXe#Oe delta 120 zcmV-;0EhpO0h9rdBXJPzP;`I+3b+t*EKFdUg@F@Dv*sn{rWOMk0$@WRf{b}k21{{f zRVtLtnV4LXS(aFmnu2BoXHkB+9?Te!8<-k^#D5?_w<|d(u^8Phwvwcr_>?p#j~U2= aGPzO`OA__cihu?KdH(=gbkf8200007crb(j diff --git a/tests/testthat/reference-objects/cache-info-3 b/tests/testthat/reference-objects/cache-info-3 index dc77f0e5eeb3ac643408579675457b06f6b15d79..ca56032e5fcc4cb3fef588aa53460736f2cdaa6e 100644 GIT binary patch delta 118 zcmV-+0Ez#Y0h9rdC2v95Fu(=n12GE|m}X(%1k$W|iMgr8K!yO=5QrdS9+bgSoLQ9$ zWpgGbmt>YDmZYYj8NpeUU#CFZ6U0~rEfLm+~Tc~AyR zab{I2l+Br#T#{LqSdyB8W&~$Zez_jZ7?2y78i2%qAV9Y(IVZ6g-7dD0q@4JaG$@Z5 c$b&MuQW8rN_0o!f1_OEj0CPEtMfLyy06!@(6aWAK From 427a1c09dcb324ccc1872dfde1699660a700b753 Mon Sep 17 00:00:00 2001 From: lorenzwalthert Date: Mon, 23 Sep 2019 22:11:29 +0200 Subject: [PATCH 51/67] move R.cache to suggest as initially thought --- DESCRIPTION | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 7c9b057d9..cbeb2f7bc 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -27,8 +27,7 @@ Imports: tibble (>= 1.4.2), tools, withr (>= 1.0.0), - xfun (>= 0.1), - R.cache (>= 0.13.0.9000) + xfun (>= 0.1) Suggests: data.tree (>= 0.1.6), dplyr, @@ -37,6 +36,7 @@ Suggests: prettycode, rmarkdown, rstudioapi (>= 0.7), + R.cache (>= 0.13.0.9000), testthat (>= 2.1.0) VignetteBuilder: knitr From 29963023f054f81ae54180c9c1fb7c7901ae5c09 Mon Sep 17 00:00:00 2001 From: lorenzwalthert Date: Mon, 23 Sep 2019 22:51:47 +0200 Subject: [PATCH 52/67] replace cache_derive_name() with constant --- R/ui-caching.R | 2 +- R/utils-cache.R | 8 ++------ R/zzz.R | 2 +- 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/R/ui-caching.R b/R/ui-caching.R index 460052988..f893dc73b 100644 --- a/R/ui-caching.R +++ b/R/ui-caching.R @@ -78,7 +78,7 @@ cache_activate <- function(cache_name = NULL, verbose = TRUE) { if (!is.null(cache_name)) { options("styler.cache_name" = cache_name) } else { - options("styler.cache_name" = cache_derive_name()) + options("styler.cache_name" = styler_version) } path <- cache_find_path(cache_name) if (verbose) { diff --git a/R/utils-cache.R b/R/utils-cache.R index de8fc3da0..d062cd355 100644 --- a/R/utils-cache.R +++ b/R/utils-cache.R @@ -37,11 +37,7 @@ cache_is_activated <- function(cache_name = NULL) { } } -desc <- read.dcf("DESCRIPTION") - -cache_derive_name <- function() { - unlist(unname(desc[, "Version"])) -} +styler_version <- unlist(unname(read.dcf("DESCRIPTION")[, "Version"])) cache_get_name <- function() { getOption("styler.cache_name") @@ -51,7 +47,7 @@ cache_get_or_derive_name <- function(cache_name) { if (is.null(cache_name)) { cache_name <- cache_get_name() if (is.null(cache_name)) { - cache_name <- cache_derive_name() + cache_name <- styler_version } } cache_name diff --git a/R/zzz.R b/R/zzz.R index bedd9ea5b..05d91dc5c 100644 --- a/R/zzz.R +++ b/R/zzz.R @@ -3,7 +3,7 @@ op <- options() op.styler <- list( styler.colored_print.vertical = TRUE, - styler.cache_name = cache_derive_name(), + styler.cache_name = styler_version, styler.addins_style_transformer = "styler::tidyverse_style()" ) toset <- !(names(op.styler) %in% names(op)) From 9326f44dc41b3ed9b3a2c770eb20048dc7144862 Mon Sep 17 00:00:00 2001 From: lorenzwalthert Date: Mon, 23 Sep 2019 22:51:54 +0200 Subject: [PATCH 53/67] random roxygenize --- man/cache_clear.Rd | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/man/cache_clear.Rd b/man/cache_clear.Rd index 407bff6ee..cf16dfa4c 100644 --- a/man/cache_clear.Rd +++ b/man/cache_clear.Rd @@ -14,7 +14,7 @@ the version of styler used.} \item{ask}{Whether or not to interactively ask the user again.} } \description{ -Clears the cache that stores which files' styling is up-to-date. You won't be +Clears the cache that stores which files are already styled. You won't be able to undo this. Note that the file corresponding to the cache (a folder on your file stystem) won't be deleted, but it will be empty after calling \code{cache_clear}. From 0a815666521686d9219bd455b4ae39106d32165c Mon Sep 17 00:00:00 2001 From: lorenzwalthert Date: Tue, 24 Sep 2019 00:46:58 +0200 Subject: [PATCH 54/67] capsule line break conversion in a function --- R/roxygen-examples-parse.R | 5 ++--- R/serialize.R | 2 +- R/utils.R | 11 +++++++++++ man/convert_newlines_to_linebreaks.Rd | 19 +++++++++++++++++++ 4 files changed, 33 insertions(+), 4 deletions(-) create mode 100644 man/convert_newlines_to_linebreaks.Rd diff --git a/R/roxygen-examples-parse.R b/R/roxygen-examples-parse.R index 3d3d6ff77..3d923d384 100644 --- a/R/roxygen-examples-parse.R +++ b/R/roxygen-examples-parse.R @@ -33,8 +33,7 @@ parse_roxygen <- function(roxygen) { #' @param raw Raw code to post-process. #' @keywords internal post_parse_roxygen <- function(raw) { - split <- raw %>% + raw %>% paste0(collapse = "") %>% - strsplit("\n", fixed = TRUE) - split[[1]] + convert_newlines_to_linebreaks() } diff --git a/R/serialize.R b/R/serialize.R index 22b3276f1..71e4cadeb 100644 --- a/R/serialize.R +++ b/R/serialize.R @@ -13,5 +13,5 @@ serialize_parse_data_flattened <- function(flattened_pd, start_line = 1) { map(lag_newlines, add_newlines), map(lag_spaces, add_spaces), text ) ) - strsplit(res, "\n")[[1L]] + convert_newlines_to_linebreaks(res) } diff --git a/R/utils.R b/R/utils.R index 60bef1be6..c396b6f14 100644 --- a/R/utils.R +++ b/R/utils.R @@ -13,6 +13,17 @@ ensure_last_is_empty <- function(x) { } } +#' Replace the newline character with a line break +#' +#' @param text A character vector +#' @examples +#' ensure_newline_is_linebreak("x\n2") +#' ensure_newline_is_linebreak(c("x", "2")) +#' @keywords internal +convert_newlines_to_linebreaks <- function(text) { + unlist(strsplit(text, "\n", fixed = TRUE)) +} + #' Check whether two columns match #' #' @param col1,col2 Column names as string. diff --git a/man/convert_newlines_to_linebreaks.Rd b/man/convert_newlines_to_linebreaks.Rd new file mode 100644 index 000000000..a615b0659 --- /dev/null +++ b/man/convert_newlines_to_linebreaks.Rd @@ -0,0 +1,19 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/utils.R +\name{convert_newlines_to_linebreaks} +\alias{convert_newlines_to_linebreaks} +\title{Replace the newline character with a line break} +\usage{ +convert_newlines_to_linebreaks(text) +} +\arguments{ +\item{text}{A character vector} +} +\description{ +Replace the newline character with a line break +} +\examples{ +ensure_newline_is_linebreak("x\\n2") +ensure_newline_is_linebreak(c("x", "2")) +} +\keyword{internal} From c0e12924307d0c2544d6c55202cf1b1c2e103f42 Mon Sep 17 00:00:00 2001 From: lorenzwalthert Date: Tue, 24 Sep 2019 00:48:36 +0200 Subject: [PATCH 55/67] make cache work when text is supplied with embedded line breaks still failing: mixed case --- R/utils-cache.R | 9 ++-- R/utils.R | 27 ++++++++--- man/convert_newlines_to_linebreaks.Rd | 3 ++ man/ensure_last_is_empty.Rd | 17 +++++++ tests/testthat/test-cache-with-r-cache.R | 58 +++++++++++++++++++++++- 5 files changed, 104 insertions(+), 10 deletions(-) create mode 100644 man/ensure_last_is_empty.Rd diff --git a/R/utils-cache.R b/R/utils-cache.R index d062cd355..804a65ee2 100644 --- a/R/utils-cache.R +++ b/R/utils-cache.R @@ -5,9 +5,12 @@ #' @param text A character vector. #' @keywords internal hash_standardize <- function(text) { - text <- ensure_last_is_empty(text) - text <- enc2utf8(text) - list(text) + text %>% + convert_newlines_to_linebreaks() %>% + ensure_last_is_empty() %>% + enc2utf8() %>% + paste0(collapse = "\n") %>% + list() } #' Where is the cache? diff --git a/R/utils.R b/R/utils.R index c396b6f14..650043e7f 100644 --- a/R/utils.R +++ b/R/utils.R @@ -4,13 +4,19 @@ line_col_names <- function() { c("line1", "line2", "col1", "col2") } +#' Ensure there is one (and only one) blank line at the end of a vector +#' @examples +#' ensure_last_is_empty("") +#' ensure_last_is_empty(letters) +#' ensure_last_is_empty(c(letters, "", "", "")) +#' @keywords internal ensure_last_is_empty <- function(x) { - has_line_break_at_eof <- x[length(x)] == "" - if (has_line_break_at_eof) { - return(x) - } else { - append(x, "") + if (all(x == "")) { + return("") } + x <- c(x, "", "") + x <- x[seq(1, length(x) - which(rev(x) != "")[1] + 2L)] + x } #' Replace the newline character with a line break @@ -18,10 +24,19 @@ ensure_last_is_empty <- function(x) { #' @param text A character vector #' @examples #' ensure_newline_is_linebreak("x\n2") +#' # a simple strsplit approach does not cover both cases +#' unlist(strsplit("x\n\n2", "\n", fixed = TRUE)) +#' unlist(strsplit(c("x", "", "2"), "\n", fixed = TRUE)) #' ensure_newline_is_linebreak(c("x", "2")) #' @keywords internal convert_newlines_to_linebreaks <- function(text) { - unlist(strsplit(text, "\n", fixed = TRUE)) + split <- strsplit(text, "\n", fixed = TRUE) + map(split, ~ if (identical(.x, character(0))) { + "" + } else { + .x + }) %>% + unlist() } #' Check whether two columns match diff --git a/man/convert_newlines_to_linebreaks.Rd b/man/convert_newlines_to_linebreaks.Rd index a615b0659..d48b77190 100644 --- a/man/convert_newlines_to_linebreaks.Rd +++ b/man/convert_newlines_to_linebreaks.Rd @@ -14,6 +14,9 @@ Replace the newline character with a line break } \examples{ ensure_newline_is_linebreak("x\\n2") +# a simple strsplit approach does not cover both cases +unlist(strsplit("x\\n\\n2", "\\n", fixed = TRUE)) +unlist(strsplit(c("x", "", "2"), "\\n", fixed = TRUE)) ensure_newline_is_linebreak(c("x", "2")) } \keyword{internal} diff --git a/man/ensure_last_is_empty.Rd b/man/ensure_last_is_empty.Rd new file mode 100644 index 000000000..0fe07b359 --- /dev/null +++ b/man/ensure_last_is_empty.Rd @@ -0,0 +1,17 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/utils.R +\name{ensure_last_is_empty} +\alias{ensure_last_is_empty} +\title{Ensure there is one (and only one) blank line at the end of a vector} +\usage{ +ensure_last_is_empty(x) +} +\description{ +Ensure there is one (and only one) blank line at the end of a vector +} +\examples{ +ensure_last_is_empty("") +ensure_last_is_empty(letters) +ensure_last_is_empty(c(letters, "", "", "")) +} +\keyword{internal} diff --git a/tests/testthat/test-cache-with-r-cache.R b/tests/testthat/test-cache-with-r-cache.R index 62ffc84a9..b69bf6f3e 100644 --- a/tests/testthat/test-cache-with-r-cache.R +++ b/tests/testthat/test-cache-with-r-cache.R @@ -45,7 +45,7 @@ capture.output(test_that("Cache management works when R.cache is installed", { -capture.output(test_that("activated cache brings speedup", { +capture.output(test_that("activated cache brings speedup on style_file() API", { skip_if_not_installed("R.cache") cache_activate("testthat") on.exit(clear_testthat_cache()) @@ -56,6 +56,62 @@ capture.output(test_that("activated cache brings speedup", { expect_true(first["elapsed"] / 2 > second["elapsed"]) })) +text <- c( + "#' Roxygen", + "#' Comment", + "#' @examples", + "#' 1 + 1", + "k <- function() {", + " 1 + 1", + " if (x) {", + " k()", + " }", + "}", + "" +) %>% + rep(10) + +capture.output(test_that("activated cache brings speedup on style_text() API on character vector", { + skip_if_not_installed("R.cache") + cache_activate("testthat") + on.exit(clear_testthat_cache()) + clear_testthat_cache() + cache_activate("testthat") + + first <- system.time(styler::style_text(text)) + second <- system.time(styler::style_text(text)) + expect_true(first["elapsed"] / 2 > second["elapsed"]) +})) + +capture.output(test_that("activated cache brings speedup on style_text() API on character scalar", { + skip_if_not_installed("R.cache") + cache_activate("testthat") + on.exit(clear_testthat_cache()) + clear_testthat_cache() + cache_activate("testthat") + + first <- system.time(styler::style_text(paste0(text, collapse = "\n"))) + second <- system.time(styler::style_text(paste0(text, collapse = "\n"))) + expect_true(first["elapsed"] / 2 > second["elapsed"]) +})) + +capture.output( + test_that(paste0( + "activated cache brings speedup on style_text() API on ", + "character scalar and character vector (mixed)" + ), { + skip_if_not_installed("R.cache") + cache_activate("testthat") + on.exit(clear_testthat_cache()) + clear_testthat_cache() + cache_activate("testthat") + + first <- system.time(styler::style_text(text)) + second <- system.time(styler::style_text(paste0(text, collapse = "\n"))) + expect_true(first["elapsed"] / 2 > second["elapsed"]) +})) + + capture.output(test_that("unactivated cache does not bring speedup", { skip_if_not_installed("R.cache") on.exit(clear_testthat_cache) From c2c8e6303a59eb8001d18f50c0e6d7ea555a8141 Mon Sep 17 00:00:00 2001 From: lorenzwalthert Date: Tue, 22 Oct 2019 21:06:16 +0200 Subject: [PATCH 56/67] need pkg qualifier for example with internal function --- R/utils.R | 10 +++++----- man/convert_newlines_to_linebreaks.Rd | 4 ++-- man/ensure_last_is_empty.Rd | 6 +++--- man/invalid_utf8.Rd | 7 ++----- 4 files changed, 12 insertions(+), 15 deletions(-) diff --git a/R/utils.R b/R/utils.R index 650043e7f..248048dd6 100644 --- a/R/utils.R +++ b/R/utils.R @@ -6,9 +6,9 @@ line_col_names <- function() { #' Ensure there is one (and only one) blank line at the end of a vector #' @examples -#' ensure_last_is_empty("") -#' ensure_last_is_empty(letters) -#' ensure_last_is_empty(c(letters, "", "", "")) +#' styler:::ensure_last_is_empty("") +#' styler:::ensure_last_is_empty(letters) +#' styler:::ensure_last_is_empty(c(letters, "", "", "")) #' @keywords internal ensure_last_is_empty <- function(x) { if (all(x == "")) { @@ -23,11 +23,11 @@ ensure_last_is_empty <- function(x) { #' #' @param text A character vector #' @examples -#' ensure_newline_is_linebreak("x\n2") +#' styler:::convert_newlines_to_linebreaks("x\n2") #' # a simple strsplit approach does not cover both cases #' unlist(strsplit("x\n\n2", "\n", fixed = TRUE)) #' unlist(strsplit(c("x", "", "2"), "\n", fixed = TRUE)) -#' ensure_newline_is_linebreak(c("x", "2")) +#' styler:::convert_newlines_to_linebreaks(c("x", "2")) #' @keywords internal convert_newlines_to_linebreaks <- function(text) { split <- strsplit(text, "\n", fixed = TRUE) diff --git a/man/convert_newlines_to_linebreaks.Rd b/man/convert_newlines_to_linebreaks.Rd index d48b77190..9764737e1 100644 --- a/man/convert_newlines_to_linebreaks.Rd +++ b/man/convert_newlines_to_linebreaks.Rd @@ -13,10 +13,10 @@ convert_newlines_to_linebreaks(text) Replace the newline character with a line break } \examples{ -ensure_newline_is_linebreak("x\\n2") +styler:::convert_newlines_to_linebreaks("x\\n2") # a simple strsplit approach does not cover both cases unlist(strsplit("x\\n\\n2", "\\n", fixed = TRUE)) unlist(strsplit(c("x", "", "2"), "\\n", fixed = TRUE)) -ensure_newline_is_linebreak(c("x", "2")) +styler:::convert_newlines_to_linebreaks(c("x", "2")) } \keyword{internal} diff --git a/man/ensure_last_is_empty.Rd b/man/ensure_last_is_empty.Rd index 0fe07b359..91cdd2aba 100644 --- a/man/ensure_last_is_empty.Rd +++ b/man/ensure_last_is_empty.Rd @@ -10,8 +10,8 @@ ensure_last_is_empty(x) Ensure there is one (and only one) blank line at the end of a vector } \examples{ -ensure_last_is_empty("") -ensure_last_is_empty(letters) -ensure_last_is_empty(c(letters, "", "", "")) +styler:::ensure_last_is_empty("") +styler:::ensure_last_is_empty(letters) +styler:::ensure_last_is_empty(c(letters, "", "", "")) } \keyword{internal} diff --git a/man/invalid_utf8.Rd b/man/invalid_utf8.Rd index 4f2c74224..9b1b64e75 100644 --- a/man/invalid_utf8.Rd +++ b/man/invalid_utf8.Rd @@ -2,14 +2,11 @@ % Please edit documentation in R/io.R \name{invalid_utf8} \alias{invalid_utf8} -\title{Drop-in replacement for \code{xfun:::invalid_utf8()}.} +\title{Drop-in replacement for \code{\link[xfun:::invalid_utf8]{xfun::::invalid_utf8()}}} \usage{ invalid_utf8(x) } -\arguments{ -\item{x}{A character vector.} -} \description{ -Drop-in replacement for \code{xfun:::invalid_utf8()}. +Drop-in replacement for \code{\link[xfun:::invalid_utf8]{xfun::::invalid_utf8()}} } \keyword{internal} From 6993203228838cf4bab361e9ca6e63d247a79041 Mon Sep 17 00:00:00 2001 From: lorenzwalthert Date: Fri, 1 Nov 2019 12:16:18 +0100 Subject: [PATCH 57/67] some version that seems to work hack: convert list with envs to text, otherwise it does not work --- R/transform-files.R | 7 +++++-- R/utils-cache.R | 10 ++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/R/transform-files.R b/R/transform-files.R index b09eb5a5d..b26fd6fb6 100644 --- a/R/transform-files.R +++ b/R/transform-files.R @@ -80,7 +80,9 @@ make_transformer <- function(transformers, assert_R.cache_installation(action = "warn") function(text) { is_cached <- rlang::is_installed("R.cache") && !is.null( - R.cache::findCache(key = hash_standardize(text), dir = cache_dir) + R.cache::findCache( + key = cache_make_key(text, transformers), + dir = cache_dir) ) should_use_cache <- cache_is_activated() use_cache <- is_cached && should_use_cache @@ -95,12 +97,13 @@ make_transformer <- function(transformers, if (should_use_cache) { R.cache::saveCache( NULL, - key = hash_standardize(transformed_code), + key = cache_make_key(transformed_code, transformers), dir = cache_dir, shallow = TRUE ) } transformed_code } else { + cat("cached value:\n") text } } diff --git a/R/utils-cache.R b/R/utils-cache.R index 804a65ee2..60efd3436 100644 --- a/R/utils-cache.R +++ b/R/utils-cache.R @@ -13,6 +13,16 @@ hash_standardize <- function(text) { list() } +cache_make_key <- function(text, transformers) { + print("---") + text <- hash_standardize(text) + print(digest::digest(text)) + print(digest::digest(transformers)) + out <- c(text = text, transfoermers = as.character(transformers)) + print(digest::digest(out)) + out +} + #' Where is the cache? #' #' Finds the path to the cache and creates it if it does not exist. From 4f3ab57a9ff486507d6d98d0e39a833cdc495215 Mon Sep 17 00:00:00 2001 From: lorenzwalthert Date: Fri, 1 Nov 2019 13:10:43 +0100 Subject: [PATCH 58/67] make commit to keep reproducible example where hashes of transformers return different things. for reference 43219ixmypi. --- R/utils-cache.R | 31 +++++++++++++++++++++++-- man/cache_make_key.Rd | 54 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+), 2 deletions(-) create mode 100644 man/cache_make_key.Rd diff --git a/R/utils-cache.R b/R/utils-cache.R index 60efd3436..bfd1b565c 100644 --- a/R/utils-cache.R +++ b/R/utils-cache.R @@ -13,12 +13,39 @@ hash_standardize <- function(text) { list() } +#' * Functions created with `purrr::partial()` are not identical when compared +#' with `identical()` +#' ([StackOverflow](https://stackoverflow.com/questions/58656033/when-are-purrrpartial-ized-functions-identical)) +#' * except when they have the exact same parent environment, which must be an +#' object created and then passed to `purrr::partial(.env = ...)`, not +#' created in-place. +#' * `purrr::partial()` seems to ignore `.env` after version 0.2.5, so until +#' this is fixed, we'd have to work with version 0.2.5. +#' * Our caching backend package, `R.cache`, uses +#' `R.cache:::getChecksum.default` (which uses `digest::digest()`) to hash the +#' input. The latter does not seem to care if the environments are exactly +#' equal (see 'Exampels'). +#' * However, when passing a list to `digest::digest()` that contains other +#' components, it seems to care. +#' @examples +#' add <- function(x, y) { +#' x + y +#' } +#' add1 <- purrr::partial(add, x = 1) +#' add2 <- purrr::partial(add, x = 1) +#' identical(add1, add2) +#' identical(digest::digest(add1), digest::digest(add2)) +#' identical(digest::digest(styler::tidyverse_style()), digest::digest(styler::tidyverse_style())) +#' Complicating elements: +#' + +#' * cache_make_key <- function(text, transformers) { - print("---") + cat("---") text <- hash_standardize(text) print(digest::digest(text)) print(digest::digest(transformers)) - out <- c(text = text, transfoermers = as.character(transformers)) + out <- c(text = text, transformers = transformers) print(digest::digest(out)) out } diff --git a/man/cache_make_key.Rd b/man/cache_make_key.Rd new file mode 100644 index 000000000..b7e11f767 --- /dev/null +++ b/man/cache_make_key.Rd @@ -0,0 +1,54 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/utils-cache.R +\name{cache_make_key} +\alias{cache_make_key} +\title{\itemize{ +\item Functions created with \code{purrr::partial()} are not identical when compared +with \code{identical()} +(\href{https://stackoverflow.com/questions/58656033/when-are-purrrpartial-ized-functions-identical}{StackOverflow}) +\item except when they have the exact same parent environment, which must be an +object created and then passed to \code{purrr::partial(.env = ...)}, not +created in-place. +\item \code{purrr::partial()} seems to ignore \code{.env} after version 0.2.5, so until +this is fixed, we'd have to work with version 0.2.5. +\item Our caching backend package, \code{R.cache}, uses +\code{R.cache:::getChecksum.default} (which uses \code{digest::digest()}) to hash the +input. The latter does not seem to care if the environments are exactly +equal (see 'Exampels'). +\item However, when passing a list to \code{digest::digest()} that contains other +components, it seems to care. +}} +\usage{ +cache_make_key(text, transformers) +} +\description{ +\itemize{ +\item Functions created with \code{purrr::partial()} are not identical when compared +with \code{identical()} +(\href{https://stackoverflow.com/questions/58656033/when-are-purrrpartial-ized-functions-identical}{StackOverflow}) +\item except when they have the exact same parent environment, which must be an +object created and then passed to \code{purrr::partial(.env = ...)}, not +created in-place. +\item \code{purrr::partial()} seems to ignore \code{.env} after version 0.2.5, so until +this is fixed, we'd have to work with version 0.2.5. +\item Our caching backend package, \code{R.cache}, uses +\code{R.cache:::getChecksum.default} (which uses \code{digest::digest()}) to hash the +input. The latter does not seem to care if the environments are exactly +equal (see 'Exampels'). +\item However, when passing a list to \code{digest::digest()} that contains other +components, it seems to care. +} +} +\examples{ +add <- function(x, y) { +x + y +} +add1 <- purrr::partial(add, x = 1) +add2 <- purrr::partial(add, x = 1) +identical(add1, add2) +identical(digest::digest(add1), digest::digest(add2)) +identical(digest::digest(styler::tidyverse_style()), digest::digest(styler::tidyverse_style())) +Complicating elements: + +* +} From 172ad9ae292c15e982a810c7732eedd4cce45827 Mon Sep 17 00:00:00 2001 From: lorenzwalthert Date: Fri, 1 Nov 2019 13:41:48 +0100 Subject: [PATCH 59/67] document edge cases --- R/utils-cache.R | 41 ++++++++++++++++++++++---------- man/cache_make_key.Rd | 54 ++++++++++++++++++++++++++----------------- 2 files changed, 62 insertions(+), 33 deletions(-) diff --git a/R/utils-cache.R b/R/utils-cache.R index bfd1b565c..f97486013 100644 --- a/R/utils-cache.R +++ b/R/utils-cache.R @@ -13,6 +13,29 @@ hash_standardize <- function(text) { list() } +#' Make a key for `R.cache` +#' +#' @details +#' +#' This function standardizes text and converts transformers to character (to +#' avoid issues described in details). +#' This means that the same code in `transformers`, +#' calling other code not in `transformers` that was modified, will lead +#' styler into thinking we can use the cache, although we should not. We believe +#' this is a highly unlikely event, in particular because we already invalidate +#' the cache when the styler version changes. Hence, our cache will cause +#' styler to return *not correctly styled* code iff one of these conditions +#' holds: +#' - An improperly versioned version of styler is used, e.g. the development +#' version on GitHub. +#' - A style guide from outside styler is used. +#' +#' Plus for both cases: the code in transformers does not change and changes in +#' code the transformers depend on result in different styling. +#' @section Experiments: +#' +#' There is unexamplainable behavior in conjunction with hashin and +#' environments: #' * Functions created with `purrr::partial()` are not identical when compared #' with `identical()` #' ([StackOverflow](https://stackoverflow.com/questions/58656033/when-are-purrrpartial-ized-functions-identical)) @@ -25,8 +48,10 @@ hash_standardize <- function(text) { #' `R.cache:::getChecksum.default` (which uses `digest::digest()`) to hash the #' input. The latter does not seem to care if the environments are exactly #' equal (see 'Exampels'). -#' * However, when passing a list to `digest::digest()` that contains other -#' components, it seems to care. +#' * However, under stome circumstances, it does: Commit 9c94c022 (if not +#' overwritten / rebased by now) contains a reprex. Otherwise, search for +#' 43219ixmypi in commit messages and restore this commit to reproduce the +#' behavior. #' @examples #' add <- function(x, y) { #' x + y @@ -36,18 +61,10 @@ hash_standardize <- function(text) { #' identical(add1, add2) #' identical(digest::digest(add1), digest::digest(add2)) #' identical(digest::digest(styler::tidyverse_style()), digest::digest(styler::tidyverse_style())) -#' Complicating elements: -#' - -#' * +#' @keywords internal cache_make_key <- function(text, transformers) { - cat("---") text <- hash_standardize(text) - print(digest::digest(text)) - print(digest::digest(transformers)) - out <- c(text = text, transformers = transformers) - print(digest::digest(out)) - out + c(text = text, transformers = as.character(transformers)) } #' Where is the cache? diff --git a/man/cache_make_key.Rd b/man/cache_make_key.Rd index b7e11f767..a3890ce02 100644 --- a/man/cache_make_key.Rd +++ b/man/cache_make_key.Rd @@ -2,26 +2,37 @@ % Please edit documentation in R/utils-cache.R \name{cache_make_key} \alias{cache_make_key} -\title{\itemize{ -\item Functions created with \code{purrr::partial()} are not identical when compared -with \code{identical()} -(\href{https://stackoverflow.com/questions/58656033/when-are-purrrpartial-ized-functions-identical}{StackOverflow}) -\item except when they have the exact same parent environment, which must be an -object created and then passed to \code{purrr::partial(.env = ...)}, not -created in-place. -\item \code{purrr::partial()} seems to ignore \code{.env} after version 0.2.5, so until -this is fixed, we'd have to work with version 0.2.5. -\item Our caching backend package, \code{R.cache}, uses -\code{R.cache:::getChecksum.default} (which uses \code{digest::digest()}) to hash the -input. The latter does not seem to care if the environments are exactly -equal (see 'Exampels'). -\item However, when passing a list to \code{digest::digest()} that contains other -components, it seems to care. -}} +\title{Make a key for \code{R.cache}} \usage{ cache_make_key(text, transformers) } \description{ +Make a key for \code{R.cache} +} +\details{ +This function standardizes text and converts transformers to character (to +avoid issues described in details). +This means that the same code in \code{transformers}, +calling other code not in \code{transformers} that was modified, will lead +styler into thinking we can use the cache, although we should not. We believe +this is a highly unlikely event, in particular because we already invalidate +the cache when the styler version changes. Hence, our cache will cause +styler to return \emph{not correctly styled} code iff one of these conditions +holds: +\itemize{ +\item An improperly versioned version of styler is used, e.g. the development +version on GitHub. +\item A style guide from outside styler is used. +} + +Plus for both cases: the code in transformers does not change and changes in +code the transformers depend on result in different styling. +} +\section{Experiments}{ + + +There is unexamplainable behavior in conjunction with hashin and +environments: \itemize{ \item Functions created with \code{purrr::partial()} are not identical when compared with \code{identical()} @@ -35,10 +46,13 @@ this is fixed, we'd have to work with version 0.2.5. \code{R.cache:::getChecksum.default} (which uses \code{digest::digest()}) to hash the input. The latter does not seem to care if the environments are exactly equal (see 'Exampels'). -\item However, when passing a list to \code{digest::digest()} that contains other -components, it seems to care. +\item However, under stome circumstances, it does: Commit 9c94c022 (if not +overwritten / rebased by now) contains a reprex. Otherwise, search for +43219ixmypi in commit messages and restore this commit to reproduce the +behavior. } } + \examples{ add <- function(x, y) { x + y @@ -48,7 +62,5 @@ add2 <- purrr::partial(add, x = 1) identical(add1, add2) identical(digest::digest(add1), digest::digest(add2)) identical(digest::digest(styler::tidyverse_style()), digest::digest(styler::tidyverse_style())) -Complicating elements: - -* } +\keyword{internal} From 5efc65676c95ba35985cfff0f3bd64356de44edb Mon Sep 17 00:00:00 2001 From: lorenzwalthert Date: Fri, 1 Nov 2019 13:43:10 +0100 Subject: [PATCH 60/67] add test --- tests/testthat/test-cache-with-r-cache.R | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/testthat/test-cache-with-r-cache.R b/tests/testthat/test-cache-with-r-cache.R index b69bf6f3e..f0a9ab347 100644 --- a/tests/testthat/test-cache-with-r-cache.R +++ b/tests/testthat/test-cache-with-r-cache.R @@ -95,6 +95,21 @@ capture.output(test_that("activated cache brings speedup on style_text() API on expect_true(first["elapsed"] / 2 > second["elapsed"]) })) + +capture.output(test_that("no speedup when tranformer changes", { + skip_if_not_installed("R.cache") + cache_activate("testthat") + on.exit(clear_testthat_cache()) + clear_testthat_cache() + cache_activate("testthat") + t1 <- tidyverse_style() + first <- system.time(style_text(text, transformers = t1)) + t1$use_raw_indention <- !t1$use_raw_indention + second <- system.time(style_text(text, transformers = t1)) + expect_false(first["elapsed"] / 2 > second["elapsed"]) +})) + + capture.output( test_that(paste0( "activated cache brings speedup on style_text() API on ", From d013ceadfd6b2d2bcc316ea9a8309be9c0ca44f7 Mon Sep 17 00:00:00 2001 From: lorenzwalthert Date: Fri, 1 Nov 2019 14:21:17 +0100 Subject: [PATCH 61/67] again remove xfun reference --- DESCRIPTION | 7 ++++--- R/io.R | 2 +- man/read_utf8_bare.Rd | 4 ++-- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index cbeb2f7bc..0574f7f27 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -30,16 +30,19 @@ Imports: xfun (>= 0.1) Suggests: data.tree (>= 0.1.6), + digest, dplyr, here, knitr, prettycode, + R.cache (>= 0.13.0.9000), rmarkdown, rstudioapi (>= 0.7), - R.cache (>= 0.13.0.9000), testthat (>= 2.1.0) VignetteBuilder: knitr +Remotes: + lorenzwalthert/R.cache@shallow Encoding: UTF-8 LazyData: true Roxygen: list(markdown = TRUE, roclets = c("rd", "namespace", @@ -92,5 +95,3 @@ Collate: 'vertical.R' 'visit.R' 'zzz.R' -Remotes: - lorenzwalthert/R.cache@shallow diff --git a/R/io.R b/R/io.R index 495650ca5..385b4ce91 100644 --- a/R/io.R +++ b/R/io.R @@ -61,7 +61,7 @@ read_utf8 <- function(path) { } } -#' Drop-in replacement for [xfun::read_utf8()], with an optional `warn` +#' Drop-in replacement for `xfun::read_utf8()`, with an optional `warn` #' argument. #' @keywords internal read_utf8_bare <- function(con, warn = TRUE) { diff --git a/man/read_utf8_bare.Rd b/man/read_utf8_bare.Rd index 725efac7f..52f397af4 100644 --- a/man/read_utf8_bare.Rd +++ b/man/read_utf8_bare.Rd @@ -2,13 +2,13 @@ % Please edit documentation in R/io.R \name{read_utf8_bare} \alias{read_utf8_bare} -\title{Drop-in replacement for \code{\link[xfun:read_utf8]{xfun::read_utf8()}}, with an optional \code{warn} +\title{Drop-in replacement for \code{xfun::read_utf8()}, with an optional \code{warn} argument.} \usage{ read_utf8_bare(con, warn = TRUE) } \description{ -Drop-in replacement for \code{\link[xfun:read_utf8]{xfun::read_utf8()}}, with an optional \code{warn} +Drop-in replacement for \code{xfun::read_utf8()}, with an optional \code{warn} argument. } \keyword{internal} From abb204e93732e1ca627a748ea4ad389ab21b561c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kirill=20M=C3=BCller?= Date: Sun, 1 Dec 2019 13:00:04 +0100 Subject: [PATCH 62/67] Tweaks --- R/transform-files.R | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/R/transform-files.R b/R/transform-files.R index 9c2d2ec3b..7b7f9c980 100644 --- a/R/transform-files.R +++ b/R/transform-files.R @@ -79,14 +79,21 @@ make_transformer <- function(transformers, force(transformers) cache_dir <- c("styler", cache_get_name()) assert_R.cache_installation(action = "warn") + + is_R.cache_installed <- rlang::is_installed("R.cache") + function(text) { - is_cached <- rlang::is_installed("R.cache") && !is.null( - R.cache::findCache( + should_use_cache <- is_R.cache_installed && cache_is_activated() + + if (should_use_cache) { + use_cache <- !is.null(R.cache::findCache( key = cache_make_key(text, transformers), - dir = cache_dir) - ) - should_use_cache <- cache_is_activated() - use_cache <- is_cached && should_use_cache + dir = cache_dir + )) + } else { + use_cache <- FALSE + } + if (!use_cache) { transformed_code <- text %>% parse_transform_serialize_r(transformers, warn_empty = warn_empty) %>% @@ -99,12 +106,12 @@ make_transformer <- function(transformers, R.cache::saveCache( NULL, key = cache_make_key(transformed_code, transformers), - dir = cache_dir, shallow = TRUE + dir = cache_dir ) } transformed_code } else { - cat("cached value:\n") + cat("(cached)") text } } From 36606a2a52828fcf959ee025e6e17e268b1253b9 Mon Sep 17 00:00:00 2001 From: Lorenz Walthert Date: Wed, 4 Dec 2019 22:07:59 +0100 Subject: [PATCH 63/67] use upstream R.cache for caching Creating file manually instead of mapping NULL to empty file --- .pre-commit-config.yaml | 4 ++-- DESCRIPTION | 2 +- R/transform-files.R | 15 ++++++++------- tests/testthat/reference-objects/cache-info-2 | Bin 144 -> 146 bytes tests/testthat/reference-objects/cache-info-3 | Bin 148 -> 148 bytes 5 files changed, 11 insertions(+), 10 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 38080dd90..d575d5dd7 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,9 +1,9 @@ # All available hooks: https://pre-commit.com/hooks.html repos: - repo: https://github.com/lorenzwalthert/precommit - rev: v0.0.0.9018 + rev: v0.0.0.9024 hooks: - - id: lintr + # - id: lintr - id: parsable-R - id: no-browser-statement - id: readme-rmd-rendered diff --git a/DESCRIPTION b/DESCRIPTION index 1fa32b6f4..7a88cf7e9 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -42,7 +42,7 @@ Suggests: VignetteBuilder: knitr Remotes: - lorenzwalthert/R.cache@shallow + HenrikBengtsson/R.cache@develop Encoding: UTF-8 LazyData: true Roxygen: list(markdown = TRUE, roclets = c("rd", "namespace", diff --git a/R/transform-files.R b/R/transform-files.R index 7b7f9c980..e406bbf60 100644 --- a/R/transform-files.R +++ b/R/transform-files.R @@ -86,10 +86,11 @@ make_transformer <- function(transformers, should_use_cache <- is_R.cache_installed && cache_is_activated() if (should_use_cache) { - use_cache <- !is.null(R.cache::findCache( + use_cache <- R.cache::generateCache( key = cache_make_key(text, transformers), - dir = cache_dir - )) + dirs = cache_dir + ) %>% + file.exists() } else { use_cache <- FALSE } @@ -103,11 +104,11 @@ make_transformer <- function(transformers, ~. ) if (should_use_cache) { - R.cache::saveCache( - NULL, + R.cache::generateCache( key = cache_make_key(transformed_code, transformers), - dir = cache_dir - ) + dirs = cache_dir + ) %>% + file.create() } transformed_code } else { diff --git a/tests/testthat/reference-objects/cache-info-2 b/tests/testthat/reference-objects/cache-info-2 index d4b0ce123ef4e5311dec10da8853bb948ca028c3..5bf3a665de5a6704679a7a801c7c96794cec53ab 100644 GIT binary patch literal 146 zcmb2|=3oE==I#ec2?+^F32DhG2}x{5Gg}>51U@qvDvNw}%#?WGbYi1HW2P9JpTgN$ z(`U?%;z{9X;BDkEdgdspb983t$^f(Uq$1C>$Ry7!u33U}Tz@{TGDvpFOUPmn`rpWK xciodYN0Lf*-|(7L^eFh!0-H1i{!>r9Jas&;cxy9B@jUs<%&`Anxav2cQvlkiH{}2T literal 144 zcmb2|=3oE==I#ec2?+^F32Dre&N!%THt1kTic?5yNwU-6Wsc#n6jL)ed*}e?lE}|O z#lhW;Jd7I@1ssprXf!Xi*sLXb>Y0y^-bo#A9Zz3JYT|DJ$d5Mo$+F6z9>d@S{r=CH(X8dU|?WkU}gi7%s?iyFo*zRULa-!%P}zULD?|C1?2-V z3lo@TVc-PPta*vKsl`Bs0N44 From c6c3affc99591ec7f57cb516bf132a34b0a732ce Mon Sep 17 00:00:00 2001 From: Lorenz Walthert Date: Wed, 4 Dec 2019 22:08:39 +0100 Subject: [PATCH 64/67] new documentation roxygen version --- man/assert_R.cache_installation.Rd | 3 +-- man/cache_activate.Rd | 5 +++-- man/cache_clear.Rd | 5 +++-- man/cache_info.Rd | 5 +++-- man/convert_newlines_to_linebreaks.Rd | 6 +++--- 5 files changed, 13 insertions(+), 11 deletions(-) diff --git a/man/assert_R.cache_installation.Rd b/man/assert_R.cache_installation.Rd index a6c7a7b2b..61f812c95 100644 --- a/man/assert_R.cache_installation.Rd +++ b/man/assert_R.cache_installation.Rd @@ -4,8 +4,7 @@ \alias{assert_R.cache_installation} \title{Assert the R.cache installation in conjunction with the cache config} \usage{ -assert_R.cache_installation(installation_only = FALSE, - action = "abort") +assert_R.cache_installation(installation_only = FALSE, action = "abort") } \arguments{ \item{installation_only}{Whether or not to only check if R.cache is diff --git a/man/cache_activate.Rd b/man/cache_activate.Rd index e7ba9ac7c..4bdb5d921 100644 --- a/man/cache_activate.Rd +++ b/man/cache_activate.Rd @@ -22,7 +22,8 @@ Helper functions to control the behavior of caching. Simple wrappers around \code{\link[base:options]{base::options()}}. } \seealso{ -Other cache managers: \code{\link{cache_clear}}, - \code{\link{cache_info}} +Other cache managers: +\code{\link{cache_clear}()}, +\code{\link{cache_info}()} } \concept{cache managers} diff --git a/man/cache_clear.Rd b/man/cache_clear.Rd index cf16dfa4c..c9adb9797 100644 --- a/man/cache_clear.Rd +++ b/man/cache_clear.Rd @@ -24,7 +24,8 @@ Each version of styler has it's own cache by default, because styling is potentially different with different versions of styler. } \seealso{ -Other cache managers: \code{\link{cache_activate}}, - \code{\link{cache_info}} +Other cache managers: +\code{\link{cache_activate}()}, +\code{\link{cache_info}()} } \concept{cache managers} diff --git a/man/cache_info.Rd b/man/cache_info.Rd index 75261490e..a7a315941 100644 --- a/man/cache_info.Rd +++ b/man/cache_info.Rd @@ -19,7 +19,8 @@ both.} Gives information about the cache. } \seealso{ -Other cache managers: \code{\link{cache_activate}}, - \code{\link{cache_clear}} +Other cache managers: +\code{\link{cache_activate}()}, +\code{\link{cache_clear}()} } \concept{cache managers} diff --git a/man/convert_newlines_to_linebreaks.Rd b/man/convert_newlines_to_linebreaks.Rd index 9764737e1..9ac928a95 100644 --- a/man/convert_newlines_to_linebreaks.Rd +++ b/man/convert_newlines_to_linebreaks.Rd @@ -13,10 +13,10 @@ convert_newlines_to_linebreaks(text) Replace the newline character with a line break } \examples{ -styler:::convert_newlines_to_linebreaks("x\\n2") +styler:::convert_newlines_to_linebreaks("x\n2") # a simple strsplit approach does not cover both cases -unlist(strsplit("x\\n\\n2", "\\n", fixed = TRUE)) -unlist(strsplit(c("x", "", "2"), "\\n", fixed = TRUE)) +unlist(strsplit("x\n\n2", "\n", fixed = TRUE)) +unlist(strsplit(c("x", "", "2"), "\n", fixed = TRUE)) styler:::convert_newlines_to_linebreaks(c("x", "2")) } \keyword{internal} From 6d3cd88631abe360a7d8bd68d1f2d01f7d02b7e5 Mon Sep 17 00:00:00 2001 From: Lorenz Walthert Date: Fri, 13 Dec 2019 22:52:50 +0100 Subject: [PATCH 65/67] don't cat info on cached --- R/transform-files.R | 1 - 1 file changed, 1 deletion(-) diff --git a/R/transform-files.R b/R/transform-files.R index e406bbf60..bda9803c6 100644 --- a/R/transform-files.R +++ b/R/transform-files.R @@ -112,7 +112,6 @@ make_transformer <- function(transformers, } transformed_code } else { - cat("(cached)") text } } From 6f15cc890ea8d6f2e5b31df805e1c8c928d481cc Mon Sep 17 00:00:00 2001 From: Lorenz Walthert Date: Fri, 13 Dec 2019 22:55:02 +0100 Subject: [PATCH 66/67] R.cache version we depend on now on CRAN --- DESCRIPTION | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 7a88cf7e9..e232a33a1 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -35,14 +35,12 @@ Suggests: here, knitr, prettycode, - R.cache (>= 0.13.0.9000), + R.cache (>= 0.14.0), rmarkdown, rstudioapi (>= 0.7), testthat (>= 2.1.0) VignetteBuilder: knitr -Remotes: - HenrikBengtsson/R.cache@develop Encoding: UTF-8 LazyData: true Roxygen: list(markdown = TRUE, roclets = c("rd", "namespace", From 0d41f9b19395c98895076df9eeeb57926a62e5dd Mon Sep 17 00:00:00 2001 From: Lorenz Walthert Date: Sun, 15 Dec 2019 12:28:48 +0100 Subject: [PATCH 67/67] more info --- R/ui-caching.R | 5 ++++- man/cache_info.Rd | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/R/ui-caching.R b/R/ui-caching.R index f893dc73b..4d3fd9ce5 100644 --- a/R/ui-caching.R +++ b/R/ui-caching.R @@ -22,7 +22,10 @@ cache_clear <- function(cache_name = NULL, ask = TRUE) { #' Show information about the styler cache #' -#' Gives information about the cache. +#' Gives information about the cache. Note that the size consumed by the cache +#' will always be displayed as zero because all the cache does is creating an +#' empty file of size 0 bytes for every cached expression. The innode is +#' excluded from this displayed size but negligible. #' @param cache_name The name of the cache for which to show details. If #' `NULL`, the active cache is used. If none is active the cache corresponding #' to the installed styler version is used. diff --git a/man/cache_info.Rd b/man/cache_info.Rd index a7a315941..13f702071 100644 --- a/man/cache_info.Rd +++ b/man/cache_info.Rd @@ -16,7 +16,10 @@ to the installed styler version is used.} both.} } \description{ -Gives information about the cache. +Gives information about the cache. Note that the size consumed by the cache +will always be displayed as zero because all the cache does is creating an +empty file of size 0 bytes for every cached expression. The innode is +excluded from this displayed size but negligible. } \seealso{ Other cache managers: