Fix separator div in case of wrapping (#224)

Fixes #221 --------- Signed-off-by: Davide Garolini <[email protected]> Co-authored-by: 27856297+dependabot-preview[bot]@users.noreply.github.com <27856297+dependabot-preview[bot]@users.noreply.github.com> Co-authored-by: Emily de la Rua <[email protected]>
insightsengineering · Nov 24, 2023 · 514e4e5 · 514e4e5
1 parent cb5426f
commit 514e4e5
Show file tree

Hide file tree

Showing 6 changed files with 58 additions and 7 deletions.
diff --git a/NEWS.md b/NEWS.md
@@ -1,5 +1,7 @@
 ## formatters 0.5.4.9003
  * Applied `styler` and resolved package lint. Changed default indentation from 4 spaces to 2.
+ * Fixed wrapping and section dividers error.
+ * Allowed section divider between header and table body.
 
 ## formatters 0.5.4
  * Fixed a bug in `paginate_to_mpfs()` so that formatting in listings key columns is retained with pagination [`insightsengineering/rlistings#155`](https://github.com/insightsengineering/rlistings/issues/155).

diff --git a/R/generics.R b/R/generics.R
@@ -639,3 +639,22 @@ setGeneric("num_rep_cols", function(obj) standardGeneric("num_rep_cols"))
 #' @export
 #' @rdname num_rep_cols
 setMethod("num_rep_cols", "ANY", function(obj) 0L)
+
+# header_section_div -----------------------------------------------------------
+#' @keywords internal
+setGeneric("header_section_div", function(obj) standardGeneric("header_section_div"))
+#' @keywords internal
+setMethod(
+  "header_section_div", "MatrixPrintForm",
+  function(obj) obj$header_section_div
+)
+#' @keywords internal
+setGeneric("header_section_div<-", function(obj, value) standardGeneric("header_section_div<-"))
+#' @keywords internal
+setMethod(
+  "header_section_div<-", "MatrixPrintForm",
+  function(obj, value) {
+    obj$header_section_div <- value
+    obj
+  }
+)
diff --git a/R/matrix_form.R b/R/matrix_form.R
@@ -193,6 +193,8 @@ disp_from_spans <- function(spans) {
 #' @param main_footer character(1). Main footer as a string.
 #' @param  prov_footer character.  Provenance footer  information as a
 #'     character vector.
+#' @param header_section_div character(1). Divider to be used between header
+#'     and body sections.
 #' @param expand_newlines logical(1). Should the matrix form generated
 #'     expand  rows  whose  values   contain  newlines  into  multiple
 #'     'physical'  rows  (as  they  will  appear  when  rendered  into
@@ -252,6 +254,7 @@ MatrixPrintForm <- function(strings = NULL,
                             page_titles = character(),
                             main_footer = "",
                             prov_footer = character(),
+                            header_section_div = NA_character_,
                             col_gap = 3,
                             table_inset = 0L,
                             colwidths = NULL,
@@ -274,6 +277,7 @@ MatrixPrintForm <- function(strings = NULL,
       page_titles = page_titles,
       main_footer = main_footer,
       prov_footer = prov_footer,
+      header_section_div = header_section_div,
       col_gap = col_gap,
       table_inset = as.integer(table_inset),
       has_topleft = has_topleft,

diff --git a/R/tostring.R b/R/tostring.R
@@ -500,7 +500,7 @@ setMethod("toString", "MatrixPrintForm", function(x,
   aligns <- mf_aligns(mat)
   keep_mat <- mf_display(mat)
   ## spans <- mat$spans
-  ##    ri <- mat$row_info
+  mf_ri <- mf_rinfo(mat)
   ref_fnotes <- mf_rfnotes(mat)
   nl_header <- mf_nlheader(mat)
 
@@ -518,17 +518,23 @@ setMethod("toString", "MatrixPrintForm", function(x,
   # Define gap string and divisor string
   gap_str <- strrep(" ", col_gap)
   div <- substr(strrep(hsep, ncchar), 1, ncchar)
+  hsd <- header_section_div(mat)
+  if (!is.na(hsd)) {
+    hsd <- substr(strrep(hsd, ncchar), 1, ncchar)
+  } else {
+    hsd <- NULL # no divisor
+  }
 
   # text head (paste w/o NA content header and gap string)
   txt_head <- apply(head(content, nl_header), 1, .paste_no_na, collapse = gap_str)
 
   # txt body
-  sec_seps_df <- x$row_info[, c("abs_rownumber", "trailing_sep"), drop = FALSE]
+  sec_seps_df <- mf_ri[, c("abs_rownumber", "trailing_sep"), drop = FALSE]
   if (!is.null(sec_seps_df) && any(!is.na(sec_seps_df$trailing_sep))) {
     bdy_cont <- tail(content, -nl_header)
     ## unfortunately we count "header rows" wrt line grouping so it
     ## doesn't match the real (i.e. body) rows as is
-    row_grouping <- tail(x$line_grouping, -nl_header) - mf_nrheader(x)
+    row_grouping <- tail(mf_lgrouping(mat), - nl_header) - mf_nrheader(mat)
     nrbody <- NROW(bdy_cont)
     stopifnot(length(row_grouping) == nrbody)
     ## all rows with non-NA section divs and the final row (regardless of NA status)
@@ -569,7 +575,7 @@ setMethod("toString", "MatrixPrintForm", function(x,
   }
 
   # retrieving titles and footers
-  allts <- all_titles(x)
+  allts <- all_titles(mat)
 
   ref_fnotes <- reorder_ref_fnotes(ref_fnotes)
   # Fix for ref_fnotes with \n characters XXX this does not count in the pagination
@@ -578,8 +584,8 @@ setMethod("toString", "MatrixPrintForm", function(x,
   }
 
   allfoots <- list(
-    "main_footer" = main_footer(x),
-    "prov_footer" = prov_footer(x),
+    "main_footer" = main_footer(mat),
+    "prov_footer" = prov_footer(mat),
     "ref_footnotes" = ref_fnotes
   )
   allfoots <- allfoots[!sapply(allfoots, is.null)]
@@ -606,6 +612,7 @@ setMethod("toString", "MatrixPrintForm", function(x,
       titles_txt, # .do_inset(div, inset) happens if there are any titles
       .do_inset(txt_head, inset),
       .do_inset(div, inset),
+      .do_inset(hsd, inset), # header_section_div if present
       .do_inset(txt_body, inset),
       .footer_inset_helper(allfoots, div, inset)
     ), collapse = "\n"),
@@ -996,7 +1003,6 @@ spans_to_viscell <- function(spans) {
 ## ' mf <- matrix_form(tbl)
 ## ' propose_column_widths(mf)
 propose_column_widths <- function(x, indent_size = 2) {
-  ## stopifnot(is(x, "VTableTree"))
   if (!is(x, "MatrixPrintForm")) {
     x <- matrix_form(x, indent_rownames = TRUE, indent_size = indent_size)
   }

diff --git a/man/MatrixPrintForm.Rd b/man/MatrixPrintForm.Rd
diff --git a/tests/testthat/test-txt_wrap.R b/tests/testthat/test-txt_wrap.R
@@ -280,3 +280,18 @@ test_that("toString wrapping avoid trimming whitespaces", {
     res
   )
 })
+
+test_that("toString and wrapping cooperates well with separator divisors", {
+  # Fixes #221
+  testdf <- iris[seq_len(5), seq_len(2)]
+  rownames(testdf) <- paste("State ", LETTERS[seq_len(nrow(testdf))])
+  bmf <- basic_matrix_form(testdf)
+
+  # Adding topleft to wrap
+  bmf$has_topleft <- TRUE # no setter atm
+  mf_strings(bmf)[1, 1] <- "LETTERS"
+
+  sec_seps_df <- mf_rinfo(bmf)[, c("abs_rownumber", "trailing_sep"), drop = FALSE]
+  mf_rinfo(bmf)$trailing_sep[c(1, 3, 4)] <- " "
+  expect_silent(toString(bmf, widths = c(4, 4, 4)))
+})