From b2d4c08169f388b9d93fa5bf31731f61c3a6166c Mon Sep 17 00:00:00 2001
From: Jan Marvin Garbuszus <jan.garbuszus@ruhr-uni-bochum.de>
Date: Mon, 5 Aug 2024 19:22:54 +0200
Subject: [PATCH] [wb_dims] without `x` and non sequential rows, incorrect
 results were returned. fixes #1093 (#1094)

* [wb_dims] without `x` and non sequential rows, incorrect results were returned. fixes #1093

* [wb_dims] move code up so that frow creation is in a single condition

* [wb_dims] update NEWS
---
 NEWS.md                     |  4 ++++
 R/utils.R                   | 10 +++++++++-
 tests/testthat/test-utils.R | 32 ++++++++++++++++++++++++++++++++
 3 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/NEWS.md b/NEWS.md
index e8fc1a548..b24d0c739 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -6,6 +6,10 @@
 
 * Experimental support for reading shared formulas. If `show_formula` is used with `wb_to_df()`, we try to show the value that is shown in spreadsheet software. [1091](https://github.com/JanMarvin/openxlsx2/pull/1091)
 
+## Fixes
+
+* Previously, if only `cols` and `rows` were passed to `wb_dims()` and row `1` was selected, incorrect results were returned. This has been fixed. [1094](https://github.com/JanMarvin/openxlsx2/pull/1094)
+
 
 ***************************************************************************
 
diff --git a/R/utils.R b/R/utils.R
index 52011ddc8..84e11ce70 100644
--- a/R/utils.R
+++ b/R/utils.R
@@ -915,7 +915,7 @@ wb_dims <- function(..., select = NULL) {
         if (all(diff(rows_arg) == 1L))
           frow <- frow + min(rows_arg) - 1L
         else
-          frow         <- vapply(rows_arg, function(x) frow + min(x) - 1L, NA_real_)
+          frow <- vapply(rows_arg, function(x) frow + min(x) - 1L, NA_real_)
       }
     }
 
@@ -923,6 +923,14 @@ wb_dims <- function(..., select = NULL) {
       fcol <- fcol + row_names
       frow <- frow + col_names
     }
+
+    # for cases were cols and rows are vectors and row 1 is selected
+    # frow is a scalar and rows_arg is a vector. We need the vector.
+    # it's mystery, why crossing 1 is such an issue.
+    if (is.null(args$x) && !all(diff(rows_arg) == 1L)) {
+      diff <- min(frow) - min(rows_arg)
+      frow <- diff + rows_arg
+    }
   }
 
   # Ensure we are spanning at least 1 row and 1 col
diff --git a/tests/testthat/test-utils.R b/tests/testthat/test-utils.R
index acc99ae2e..683f71341 100644
--- a/tests/testthat/test-utils.R
+++ b/tests/testthat/test-utils.R
@@ -343,6 +343,38 @@ test_that("wb_dims() handles `from_dims`", {
   expect_error(wb_dims(from_dims = "65"))
 })
 
+test_that("wb_dims() corner cases work", {
+
+  exp <- "B2:B2,B30:B30"
+  got <- wb_dims(rows = c(2, 30), cols = 2)       # expect B2,B30
+  expect_equal(exp, got)
+
+  exp <- "B1:B1,B30:B30"
+  got <- wb_dims(rows = c(1, 30), cols = 2)       # expect B1,B30
+  expect_equal(exp, got)
+
+  exp <- "B11:B11,B40:B40"
+  got <- wb_dims(rows = c(1, 30), cols = 2, from_row = 11)
+  expect_equal(exp, got)
+
+  exp <- "B2:B2,D2:D2,B30:B30,D30:D30"
+  got <- wb_dims(rows = c(2, 30), cols = c(2, 4)) # expect B2,D2,B30,D30
+  expect_equal(exp, got)
+
+  exp <- "B1:B1,D1:D1,B30:B30,D30:D30"
+  got <- wb_dims(rows = c(1, 30), cols = c(2, 4)) # expect B1,D1,B30,D30
+  expect_equal(exp, got)
+
+  exp <- "B2:B2,D2:D2,B1:B1,D1:D1"
+  got <- wb_dims(rows = c(2, 1), cols = c(2, 4))  # expect B2,D2,B1,D1
+  expect_equal(exp, got)
+
+  exp <- "B5:B5,D5:D5,B1:B1,D1:D1"
+  got <- wb_dims(rows = c(5, 1), cols = c(2, 4))  # expect B5,D5,B1,D1
+  expect_equal(exp, got)
+
+})
+
 test_that("create_char_dataframe", {
 
   exp <- data.frame(x1 = rep("", 5), z1 = rep("", 5), stringsAsFactors = FALSE)