Merge pull request #595 from lorenzwalthert/issue-952-2

- Correctly identify block with caching and comments (#595).

Author: lorenzwalthert

R/nest.R

@@ -46,9 +46,10 @@ add_cache_block <- function(pd_nested) {
 
 #' Drop all children of a top level expression that are cached
 #'
-#' Note that we do cache top-level comments. Because package code has a lot of
-#' roxygen comments and each of them is a top level expresion, checking is
-#' very expensive.
+#' Note that we do not cache top-level comments. Because package code has a lot
+#' of roxygen comments and each of them is a top level expresion, checking is
+#' very expensive. More expensive than styling, because comments are always
+#' terminals.
 #' @param pd A top-level nest.
 #' @details
 #' Because we process in blocks of expressions for speed, a cached expression

R/transform-block.R

@@ -60,7 +60,7 @@ cache_find_block <- function(pd) {
 
   first_on_line_idx <- which(!not_first_on_line)
   valid_replacements <- map_int(invalid_turning_point_idx, function(x) {
-    last(which(x > first_on_line_idx))
+    first_on_line_idx[last(which(x > first_on_line_idx))]
   })
   sort(unique(c(
     setdiff(which(first_after_cache_state_switch), invalid_turning_point_idx),

man/drop_cached_children.Rd

@@ -16,6 +16,48 @@ test_that("caching utils make right blocks with semi-colon", {
   expect_equal(blocks_edge, c(1, 2, 2, 2))
 })
 
+test_that("caching utils make right blocks with comments", {
+  text <- '
+   ### comment
+   x = 1 ### comment
+   y = 2 # comment
+   x<-1 ###comment
+   y <- 2 # comment
+   "a string here"
+
+   # something something
+   tau1 = 1 # here?
+   '
+
+
+  blocks_simple_uncached <- compute_parse_data_nested(text) %>%
+    dplyr::mutate(is_cached = c(
+      FALSE, FALSE, FALSE, FALSE, FALSE, FALSE, FALSE, TRUE, FALSE,
+      TRUE, FALSE, FALSE, FALSE)
+    ) %>%
+    cache_find_block()
+  expect_equal(blocks_simple_uncached, c(1, 1, 1, 1, 1, 1, 1, 2, 2, 3, 4, 4, 4))
+
+  text <- '
+   ### comment
+   x = 1
+   y = 2 # comment
+   x<-1
+   y <- 2 # comment
+
+   # something something
+   tau1 = 1 # here?
+   '
+  blocks_simple_cached <- compute_parse_data_nested(text) %>%
+    dplyr::mutate(is_cached = c(
+      FALSE, FALSE, FALSE, FALSE, FALSE, TRUE, FALSE, FALSE, FALSE, FALSE)
+    ) %>%
+    cache_find_block()
+  expect_equal(blocks_simple_cached, c(1, 1, 1, 1, 1, 2, 2, 2, 2, 2))
+
+})
+
+
 test_that("blank lines are correctly identified", {
   on.exit(clear_testthat_cache())
   clear_testthat_cache()