Fix bug in printing table with only one row, topleft information and …

…column names with newlines (#326) Fixes insightsengineering/rtables#942 --------- Co-authored-by: shajoezhu <[email protected]> Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
insightsengineering · Nov 25, 2024 · 770ec03 · 770ec03
1 parent 84e8c3e
commit 770ec03
Show file tree

Hide file tree

Showing 4 changed files with 64 additions and 13 deletions.
diff --git a/DESCRIPTION b/DESCRIPTION
@@ -1,6 +1,6 @@
 Package: formatters
 Title: ASCII Formatting for Values and Tables
-Version: 0.5.9.9003
+Version: 0.5.9.9004
 Date: 2024-10-28
 Authors@R: c(
     person("Gabriel", "Becker", , "[email protected]", role = "aut",

diff --git a/NEWS.md b/NEWS.md
@@ -1,5 +1,7 @@
-## formatters 0.5.9.9003
+## formatters 0.5.9.9004
 * Fixed a bug in `mf_update_cinfo` causing an error when `export_as_txt` was applied to empty listings.
+* Fixed a bug in `mform_handle_newlines` that did not allow printing to console a table that has top left information, new lines in the column names and contained only one row.
+* Fixed a bug in `mform_handle_newlines` that did not allow for empty strings to be present in top left information.
 
 ## formatters 0.5.9
 * Fixed bug in `format_value` that caused multiple `NA` values to be associated with the wrong `na_str` values.

diff --git a/R/matrix_form.R b/R/matrix_form.R
@@ -17,18 +17,27 @@ mform_handle_newlines <- function(matform) {
   spamat <- mf_spans(matform)
   alimat <- mf_aligns(matform)
   nr_header <- mf_nrheader(matform)
-  nl_inds_header <- seq(1, mf_nlheader(matform))
-  hdr_inds <- 1:nr_header
+  nl_inds_header <- seq(mf_nlheader(matform))
+  hdr_inds <- seq(nr_header)
 
   # hack that is necessary only if top-left is bottom aligned (default)
   topleft_has_nl_char <- FALSE
   if (has_topleft) {
+    # extract topleft info
     tl <- strmat[nl_inds_header, 1, drop = TRUE]
+
+    # removes it from the header (temporary) - done so the header can be top aligned
     strmat[nl_inds_header, 1] <- ""
-    tl <- tl[nzchar(tl)] # we are not interested in initial "" but we cover initial \n
+
+    # remove top empty strings (because they are not topleft) and assign topleft to add back
+    if (any(nzchar(tl)) && length(tl) > 1) { # needed if ever has_top_left is true but only empties
+      # values that are "" before topleft information
+      to_remove <- seq(which(nzchar(tl))[1] - 1)
+      tl <- tl[-to_remove]
+    }
     topleft_has_nl_char <- any(grepl("\n", tl))
     tl_to_add_back <- strsplit(paste0(tl, collapse = "\n"), split = "\n", fixed = TRUE)[[1]]
-    how_many_nl <- length(tl_to_add_back)
+    tl_how_many_nl <- length(tl_to_add_back)
   }
 
   # pre-proc in case of wrapping and \n
@@ -44,7 +53,7 @@ mform_handle_newlines <- function(matform) {
   # because we don't care about wrapping here we're counting lines
   # TODO probably better if we had a nlines_nowrap fun to be more explicit
 
-  row_nlines <- apply(
+  row_nlines <- apply( # tells how many nlines for each row
     strmat,
     1,
     function(x) {
@@ -60,10 +69,9 @@ mform_handle_newlines <- function(matform) {
     }
   )
 
-
   # Correction for the case where there are more lines for topleft material than for cols
-  if (has_topleft && (sum(row_nlines[nl_inds_header]) < how_many_nl)) {
-    row_nlines[1] <- row_nlines[1] + how_many_nl - sum(row_nlines[nl_inds_header])
+  if (has_topleft && (sum(row_nlines[hdr_inds]) < tl_how_many_nl)) {
+    row_nlines[1] <- row_nlines[1] + tl_how_many_nl - sum(row_nlines[hdr_inds])
   }
 
   # There is something to change
@@ -95,8 +103,8 @@ mform_handle_newlines <- function(matform) {
     )
 
     if (has_topleft) {
-      starts_from_ind <- if (sum(row_nlines[hdr_inds]) - how_many_nl > 0) {
-        sum(row_nlines[hdr_inds]) - how_many_nl
+      starts_from_ind <- if (sum(row_nlines[hdr_inds]) - tl_how_many_nl > 0) {
+        sum(row_nlines[hdr_inds]) - tl_how_many_nl
       } else {
         0
       }
@@ -127,7 +135,6 @@ mform_handle_newlines <- function(matform) {
   prov_footer(matform) <- .quick_handle_nl(prov_footer(matform))
 
   # xxx \n in page titles are not working atm (I think)
-
   matform
 }
 

diff --git a/tests/testthat/test-txt_wrap.R b/tests/testthat/test-txt_wrap.R
@@ -461,3 +461,45 @@ test_that("misc font device related test coverage", {
   expect_true(is_monospace(font_spec()))
   expect_false(is_monospace(font_spec("Times")))
 })
+
+test_that("matrix_form is prepared correctly for printing when there is topleft information and new lines", {
+  # Regression test for #942 in {rtables}
+  fake_data <- data.frame("A", 0, 0)
+  colnames(fake_data) <- c("aaaaaaaaaa", "A: \n\nDrug X", "B: \nPlacebo")
+  bmf <- basic_matrix_form(fake_data, ignore_rownames = TRUE)
+  bmf$has_topleft <- TRUE
+
+  expect_silent(printed_out <- strsplit(toString(bmf, hsep = "-"), "\n")[[1]])
+  expect_equal(
+    printed_out,
+    c(
+      "               A:            ",
+      "                        B:   ",
+      "aaaaaaaaaa   Drug X   Placebo",
+      "-----------------------------",
+      "    A          0         0   "
+    )
+  )
+
+  # Another test for complex topleft information
+  colnames(fake_data) <- c("\n\naaaaaaaaaa\n\nb", "A: \n\nDrug X", "B: \nPlacebo")
+  bmf <- basic_matrix_form(fake_data, ignore_rownames = TRUE)
+  bmf$has_topleft <- TRUE
+  bmf$aligns[, 1] <- "left"
+
+  expect_equal(bmf$strings[, 1], c("", "", "aaaaaaaaaa", "", "b", "A"))
+
+  expect_silent(printed_out <- strsplit(toString(bmf, hsep = "-"), "\n")[[1]])
+  expect_equal(
+    printed_out,
+    c(
+      "                             ",
+      "                             ",
+      "aaaaaaaaaa     A:            ",
+      "                        B:   ",
+      "b            Drug X   Placebo",
+      "-----------------------------",
+      "A              0         0   "
+    )
+  )
+})