Skip to content

Commit 9a006b1

Browse files
committed
fix: optimize span_free calls in delete_rows and delete_columns
span_free() is now only called when the deletion actually breaks a vertical or horizontal span. Previously it was called systematically, resetting all spans to 1 even when the merge structure was intact. Add tests to verify span preservation and reset behavior.
1 parent 0bea306 commit 9a006b1

File tree

6 files changed

+111
-19
lines changed

6 files changed

+111
-19
lines changed

DESCRIPTION

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
Type: Package
22
Package: flextable
33
Title: Functions for Tabular Reporting
4-
Version: 0.9.12.002
4+
Version: 0.9.12.003
55
Authors@R: c(
66
person("David", "Gohel", , "david.gohel@ardata.fr", role = c("aut", "cre")),
77
person("ArData", role = "cph"),

NEWS.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,10 @@
33
## issues
44

55
- `padding.left` and `padding.right` are now supported in PDF/LaTeX output.
6+
- `delete_rows()` and `delete_columns()` no longer reset all spans
7+
unconditionally. `span_free()` is now only triggered when the
8+
deletion actually breaks a merged cell, preserving existing
9+
merge structures in all other cases.
610

711
## new features
812

R/augment_rows.R

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -131,10 +131,17 @@ delete_rows_from_part <- function(x, i) {
131131
x$rowheights <- x$rowheights[-i]
132132
# hrules
133133
x$hrule <- x$hrule[-i]
134-
# spans
134+
# spans — only reset if vertical spans are broken
135+
runs <- split(i, cumsum(c(1, diff(i) != 1)))
136+
has_broken_vspan <- any(vapply(runs, function(r) {
137+
any(colSums(x$spans$columns[r, , drop = FALSE]) != length(r))
138+
}, logical(1)))
139+
135140
x$spans$rows <- x$spans$rows[-i, , drop = FALSE]
136141
x$spans$columns <- x$spans$columns[-i, , drop = FALSE]
137-
x <- span_free(x)
142+
if (has_broken_vspan) {
143+
x <- span_free(x)
144+
}
138145

139146
# styles
140147
x$styles$cells <- delete_style_row(x$styles$cells, i)
@@ -191,12 +198,20 @@ delete_rows <- function(x, i = NULL, part = "body") {
191198

192199

193200
delete_colums_from_part <- function(x, j) {
201+
j_idx <- which(x$col_keys %in% j)
194202
# heights
195203
x$colwidths <- x$colwidths[!x$col_keys %in% j]
196-
# spans
197-
x$spans$rows <- x$spans$rows[, !x$col_keys %in% j, drop = FALSE]
198-
x$spans$columns <- x$spans$columns[, !x$col_keys %in% j, drop = FALSE]
199-
x <- span_free(x)
204+
# spans — only reset if horizontal spans are broken
205+
runs <- split(j_idx, cumsum(c(1, diff(j_idx) != 1)))
206+
has_broken_hspan <- any(vapply(runs, function(r) {
207+
any(rowSums(x$spans$rows[, r, drop = FALSE]) != length(r))
208+
}, logical(1)))
209+
210+
x$spans$rows <- x$spans$rows[, -j_idx, drop = FALSE]
211+
x$spans$columns <- x$spans$columns[, -j_idx, drop = FALSE]
212+
if (has_broken_hspan) {
213+
x <- span_free(x)
214+
}
200215

201216
# styles
202217
x$styles$cells <- delete_style_col(x$styles$cells, j)

R/merge_flextable.R

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -80,13 +80,20 @@ merge_v <- function(x, j = NULL, target = NULL, part = "body", combine = FALSE)
8080
#' @inheritParams args_x_i_part_no_all
8181
#' @family flextable merging function
8282
#' @examples
83-
#' dummy_df <- data.frame(
84-
#' col1 = letters,
85-
#' col2 = letters, stringsAsFactors = FALSE
83+
#' library(flextable)
84+
#'
85+
#' schedule <- data.frame(
86+
#' time = c("9h", "10h", "11h", "14h", "15h", "16h"),
87+
#' monday = c("Math", "Math", "French", "History", "Science", "French"),
88+
#' tuesday = c("English", "Math", "Art", "Math", "Math", "French"),
89+
#' wednesday = c("Science", "Math", "Science", "English", "English", "French"),
90+
#' stringsAsFactors = FALSE
8691
#' )
87-
#' ft_merge <- flextable(dummy_df)
88-
#' ft_merge <- merge_h(x = ft_merge)
89-
#' ft_merge
92+
#'
93+
#' ft <- flextable(schedule)
94+
#' ft <- theme_box(ft)
95+
#' ft <- merge_h(ft)
96+
#' ft
9097
#' @export
9198
merge_h <- function(x, i = NULL, part = "body") {
9299
if (!inherits(x, "flextable")) {

man/merge_h.Rd

Lines changed: 13 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

tests/testthat/test-merge.R

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,3 +51,62 @@ test_that("merged cells can be un-merged", {
5151
expect_true(all(ft$body$spans$columns == 1))
5252
})
5353

54+
55+
test_that("delete_rows preserves vertical spans when deletion does not break them", {
56+
ft <- flextable(data.frame(
57+
col1 = rep(letters[1:3], each = 2),
58+
col2 = 1:6,
59+
stringsAsFactors = FALSE
60+
))
61+
ft <- merge_v(ft, j = "col1")
62+
expect_equal(ft$body$spans$columns[, 1], c(2, 0, 2, 0, 2, 0), ignore_attr = TRUE)
63+
64+
# delete rows 5:6 (the "c" group) — no span is broken
65+
ft2 <- delete_rows(ft, i = 5:6)
66+
expect_equal(ft2$body$spans$columns[, 1], c(2, 0, 2, 0), ignore_attr = TRUE)
67+
})
68+
69+
test_that("delete_rows resets spans when deletion breaks a vertical span", {
70+
ft <- flextable(data.frame(
71+
col1 = rep(letters[1:3], each = 2),
72+
col2 = 1:6,
73+
stringsAsFactors = FALSE
74+
))
75+
ft <- merge_v(ft, j = "col1")
76+
77+
# delete row 3 — breaks the "b" group (rows 3:4)
78+
ft2 <- delete_rows(ft, i = 3)
79+
expect_true(all(ft2$body$spans$columns == 1))
80+
expect_true(all(ft2$body$spans$rows == 1))
81+
})
82+
83+
test_that("delete_columns preserves horizontal spans when deletion does not break them", {
84+
ft <- flextable(data.frame(
85+
col1 = c("x", "y"),
86+
col2 = c("x", "y"),
87+
col3 = c("a", "b"),
88+
stringsAsFactors = FALSE
89+
))
90+
ft <- merge_h(ft)
91+
expect_equal(ft$body$spans$rows[1, ], c(2, 0, 1), ignore_attr = TRUE)
92+
93+
# delete col3 — no horizontal span is broken
94+
ft2 <- delete_columns(ft, j = "col3")
95+
expect_equal(ft2$body$spans$rows[1, ], c(2, 0), ignore_attr = TRUE)
96+
})
97+
98+
test_that("delete_columns resets spans when deletion breaks a horizontal span", {
99+
ft <- flextable(data.frame(
100+
col1 = c("x", "y"),
101+
col2 = c("x", "y"),
102+
col3 = c("a", "b"),
103+
stringsAsFactors = FALSE
104+
))
105+
ft <- merge_h(ft)
106+
107+
# delete col1 — breaks the horizontal span col1+col2
108+
ft2 <- delete_columns(ft, j = "col1")
109+
expect_true(all(ft2$body$spans$columns == 1))
110+
expect_true(all(ft2$body$spans$rows == 1))
111+
})
112+

0 commit comments

Comments
 (0)