Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix separator div in case of wrapping #224

Merged
merged 9 commits into from
Nov 24, 2023
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
## formatters 0.5.4.9002
* Applied `styler` and resolved package lint. Changed default indentation from 4 spaces to 2.
* Fixed wrapping and section dividers error.
* Allowed sectiond divider between header and table body.
Melkiades marked this conversation as resolved.
Show resolved Hide resolved

## 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).
Expand Down
19 changes: 19 additions & 0 deletions R/generics.R
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
)
4 changes: 4 additions & 0 deletions R/matrix_form.R
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -252,6 +254,7 @@ MatrixPrintForm <- function(strings = NULL,
page_titles = character(),
main_footer = "",
prov_footer = character(),
header_section_div = NA_character_,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

class definition does not need this as it is not a full S4 (I think)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this default should not create problems in rlistings

col_gap = 3,
table_inset = 0L,
colwidths = NULL,
Expand All @@ -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,
Expand Down
20 changes: 13 additions & 7 deletions R/tostring.R
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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)
Expand Down Expand Up @@ -569,16 +575,16 @@ setMethod("toString", "MatrixPrintForm", function(x,
}

# retrieving titles and footers
allts <- all_titles(x)
allts <- all_titles(mat)

# Fix for ref_fnotes with \n characters XXX this does not count in the pagination
if (any(grepl("\n", ref_fnotes))) {
ref_fnotes <- unlist(strsplit(ref_fnotes, "\n", fixed = TRUE))
}

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)]
Expand All @@ -605,6 +611,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"),
Expand Down Expand Up @@ -977,7 +984,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)
}
Expand Down
5 changes: 5 additions & 0 deletions man/MatrixPrintForm.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 15 additions & 0 deletions tests/testthat/test-txt_wrap.R
Original file line number Diff line number Diff line change
Expand Up @@ -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)))
})