[dims] extend wb_dims() to accept multiple columns

[JanMarvin]

Fix #843

This PR extends wb_dims() to accept multiple columns:

library(openxlsx2)
wb <- wb_workbook() %>% 
  wb_add_worksheet() %>% 
  wb_add_data(x = mtcars) %>% 
  wb_add_fill(dims = wb_dims(x = mtcars, cols = c("cyl", "gear"), select = "col_names"), color = wb_color("blue")) %>% 
  wb_add_fill(dims = wb_dims(x = mtcars, cols = c("cyl", "gear")), color = wb_color("orange"))

if (interactive())  wb %>% wb_open()

# just to show what is going on
wb_dims(x = mtcars, cols = c("cyl"))
#> [1] "B2:B33"

wb_dims(x = mtcars, cols = c("cyl", "gear"))
#> [1] "B2:B33,J2:J33"

wb_dims(x = mtcars, select = "col_names")
#> [1] "A1:K1"

wb_dims(x = mtcars, cols = c("cyl", "gear"), select = "col_names")
#> [1] "B1,J1"

[JanMarvin]

Related issue: #796 (comment)

[JanMarvin]

Sure, take your time. I've adjusted the wb_dims() code, but let's put it this way, I'm not entirely satisfied myself. The wb_dims() code is a bit strange to me. I was just happy, when I finally had it working. Ideally, I would have wanted the rowcol_to_dim()/rowcol_to_dims() functions to take whatever we put into it and return the requested dimensions -cols is a start, but rows would be nice too- I just doubt I have the energy to rewrite the existing code.

[olivroy]

I may not have time to review before end of week..

[JanMarvin]

No worries, it’s not time critical.

[JanMarvin]

Achievement unlocked! Make it even more confusing 😄

library(openxlsx2)
d_full      <- wb_dims(x = mtcars)
d_col       <- wb_dims(x = mtcars, cols = c(2))
d_cols      <- wb_dims(x = mtcars, cols = c(2, 4))
d_row       <- wb_dims(x = mtcars, rows = c(4))
d_rows      <- wb_dims(x = mtcars, rows = c(2, 4:5))
d_cols_rows <- wb_dims(x = mtcars, cols = c(2, 4), rows = c(2, 4:5))

wb <- wb_workbook()$
  add_worksheet()$add_data(x = mtcars)$add_fill(dims = d_full, color = wb_color("green"))$
  add_worksheet()$add_data(x = mtcars)$add_fill(dims = d_col,  color = wb_color("green"))$
  add_worksheet()$add_data(x = mtcars)$add_fill(dims = d_cols, color = wb_color("green"))$
  add_worksheet()$add_data(x = mtcars)$add_fill(dims = d_row,  color = wb_color("green"))$
  add_worksheet()$add_data(x = mtcars)$add_fill(dims = d_rows, color = wb_color("green"))$
  add_worksheet()$add_data(x = mtcars)$add_fill(dims = d_cols_rows, color = wb_color("green"))

if (interactive()) wb$open()

[olivroy]

Sorry to have kept you waiting! I think this is a nice improvement. You were able to simplify the code from the previous time I looked at this?

I guess that a good way to test this would be to see if this works with various functions.

, will work with wb_add_conditional_formatting(), data?

I think it is worth publicizing this elsewhere in docs (in a follow-up perhaps?)

Thankfully, I didn't have to work with Excel a lot lately ;).

Maybe we can ask user testing too at this point? I remember this issue in particular that would be solved with this

#843

[olivroy]

Suggested change

	frow <- vapply(rows_arg, function(x) frow + min(x) - 1L, NA_real_)
	frow <- vapply(rows_arg, function(x) frow + min(x) - 1L, NA_real_)

[olivroy]

After merge, I will install this version so I can do a bit of integration testing before next release!

[JanMarvin]

Sorry to have kept you waiting! I think this is a nice improvement. You were able to simplify the code from the previous time I looked at this?

We have all the time in the world! But the code is not really simplified. I only fixed what I broke with non consecutive rows. And I'm not really hyped on the coding in this PR 😅

I guess that a good way to test this would be to see if this works with various functions.

, will work with wb_add_conditional_formatting(), data?

Haven't checked this. Chances are it wont work. Afaik conditional formatting requires a consecutive range, but haven't tried.

I think it is worth publicizing this elsewhere in docs (in a follow-up perhaps?)

Thankfully, I didn't have to work with Excel a lot lately ;).

Yeah, that's a luxury I don't have in my life ... at least with openxlsx2 I don't have to many chores in that nasty spreadsheet software.

Maybe we can ask user testing too at this point? I remember this issue in particular that would be solved with this

#843

With open source development everyone wants working releases and nobody wants to test. Same reason R-Core has to do many patch releases ...

[JanMarvin]

Alright, merging this one. After all we can always revert and at least our tests don't bark at us