Skip to content

Commit f3b445f

Browse files
authored
Say something when a formula string is written as text (#147)
A cell column has no column-wide "these are all formulas", so df[i, j] <- "=SUM(A1:A2)" writes those characters. That is the documented rule and it stays, but it is a quiet way to lose every formula on a sheet: run unpatched against this writexl, BioMonTools produces a workbook whose NOTES sheet has 0 formula cells and 20 inert HYPERLINK strings, and nothing -- not R, not R CMD check, not CRAN's reverse-dependency run -- says a word. The file opens, so the check passes. The assignment now warns and names the spelling that works. Nothing about what gets written changes. Scoped to assignment, where the ambiguity is. Measured: d[i, j] <- "=SUM(A1)" warns d[i, j] <- "plain text" silent d[i, j] <- 99 silent d[i, j] <- NA silent d[i, j] <- xl_formula("=SUM(A1)") silent xl_cell_general(value = "=SUM(A1)") silent The last is deliberate: constructing a text cell that looks like a formula is documented behaviour, and it is not the assignment path's business to second-guess a constructor. Unpatched BioMonTools now raises 50-odd warnings where it raised none. It is still worth patching -- the warning tells you the formulas are gone, it does not put them back.
1 parent a948bd7 commit f3b445f

2 files changed

Lines changed: 45 additions & 2 deletions

File tree

R/xl_cell_general.R

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -464,6 +464,7 @@ length.xl_cell_general <- function(x) length(unclass(x))
464464
#' @export
465465
`[<-.xl_cell_general` <- function(x, i, value) {
466466
recs <- .cell_records(x)
467+
.warn_formula_as_text(value)
467468
repl <- if (inherits(value, "xl_cell_general")) .cell_records(value)
468469
else .cell_records(xl_cell_general(value = value))
469470
# `i` may point past the end -- `d[3, 2] <-` on a one-row frame does exactly
@@ -474,6 +475,26 @@ length.xl_cell_general <- function(x) length(unclass(x))
474475
.new_cell_vector(.fill_empty_cells(recs))
475476
}
476477

478+
# A string beginning with "=" assigned into a cell column is written as those
479+
# characters, not as a formula: a cell column has no column-wide "these are all
480+
# formulas". That is the documented rule, but it is a quiet way to lose every
481+
# formula on a sheet -- a reverse dependency shipped a workbook with twenty
482+
# inert HYPERLINK strings and nothing said a word -- so say something.
483+
#
484+
# Only on assignment, where the ambiguity is. xl_cell_general(value = "=x")
485+
# writing text is deliberate and stays silent.
486+
.warn_formula_as_text <- function(value) {
487+
if (inherits(value, "xl_cell_general") || !is.character(value)) return(invisible(NULL))
488+
v <- value[!is.na(value)]
489+
if (!length(v) || !any(startsWith(v, "="))) return(invisible(NULL))
490+
warning("assigning a string beginning with \"=\" into a cell column writes ",
491+
"those characters, not a formula. Use xl_formula() for a formula, ",
492+
"either per cell or over the finished column:
493+
",
494+
" df[[j]] <- xl_formula(df[[j]])", call. = FALSE)
495+
invisible(NULL)
496+
}
497+
477498
# Extending a data frame pads its other columns, and `[<-.data.frame` does that
478499
# through `length<-`. Without this method the cell column stays short and the
479500
# frame is refused with "replacement has N rows, data has N+1".

tests/testthat/test-cell-vector.R

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,9 @@ test_that("the pattern that broke a reverse dependency works", {
5252
expect_s3_class(NOTES[[2]], "xl_cell_general")
5353

5454
NOTES[1, 1] <- "Title"
55-
NOTES[3, 2] <- "=LEFT(A1,1)"
55+
# writing a formula string as a value is what this pattern does; the warning
56+
# is the subject of its own test below
57+
expect_warning(NOTES[3, 2] <- "=LEFT(A1,1)", "not a formula")
5658
expect_s3_class(NOTES[[2]], "xl_cell_general")
5759
# the carrier and the records stay in step: a stale record list is how this
5860
# corrupts silently rather than loudly
@@ -123,7 +125,7 @@ test_that("assigning a bare string writes a string, not a formula", {
123125
# column-level kind here, and `d[i, j] <- "=A1"` writes the six characters.
124126
d <- data.frame(a = 1:2)
125127
d[, 2] <- xl_formula(c("=A1", "=A2"))
126-
d[2, 2] <- "=SUM(A1:A2)"
128+
expect_warning(d[2, 2] <- "=SUM(A1:A2)", "not a formula")
127129
expect_equal(.cell_records(d[[2]])[[2L]]$value, "=SUM(A1:A2)")
128130
expect_true(is.na(.cell_records(d[[2]])[[2L]]$formula))
129131
# the formula spelling is explicit, and still available
@@ -164,3 +166,23 @@ test_that("`length<-` grows and shrinks a cell vector", {
164166
expect_length(x, 2L)
165167
expect_equal(x[[2L]]$value, 2)
166168
})
169+
170+
test_that("writing a formula string as a value says so", {
171+
# a cell column has no column-wide "these are all formulas", so this writes
172+
# the characters. That is the rule, but it is a quiet way to lose every
173+
# formula on a sheet, so it warns -- and points at the spelling that works.
174+
d <- data.frame(a = 1:2)
175+
d[, 2] <- xl_cell_general(value = c(1, 2))
176+
expect_warning(d[2, 2] <- "=SUM(A1)", "Use xl_formula")
177+
expect_warning(d[2, 2] <- "=SUM(A1)", "df[[j]] <- xl_formula", fixed = TRUE)
178+
179+
# and stays quiet everywhere the intent is already clear
180+
expect_silent(d[2, 2] <- "plain text")
181+
expect_silent(d[2, 2] <- 99)
182+
expect_silent(d[2, 2] <- NA)
183+
expect_silent(d[2, 2] <- xl_formula("=SUM(A1)"))
184+
expect_silent(d[2, 2] <- xl_cell_general(formula = "=SUM(A1)"))
185+
# constructing a text cell that looks like a formula is deliberate and
186+
# documented, so it is not the assignment's business to second-guess it
187+
expect_silent(xl_cell_general(value = "=SUM(A1)"))
188+
})

0 commit comments

Comments
 (0)