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: function crashes due to missing paginate argument #333

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Tazovsky
Copy link

Function crashes, because paginate is not argument of function.

Error:

Error in isFALSE(paginate) : object 'paginate' not found

Copy link
Contributor

github-actions bot commented Jan 14, 2025


🎉 Thank you for your contribution! Before this PR can be accepted, we require that you read and agree to our Contributor License Agreement.
You can digitally sign the CLA by posting a comment on this Pull Request in the format shown below. This agreement will apply to this PR as well as all future contributions on this repository.


I have read the CLA Document and I hereby sign the CLA


You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot.

@shajoezhu
Copy link
Contributor

hi @Tazovsky , can you share an example with us when you seeing the bug? Thanks!

@Tazovsky
Copy link
Author

hi @Tazovsky , can you share an example with us when you seeing the bug? Thanks!

@shajoezhu thanks for quick response!

I can prepare code to reproduce, but please confirm it is really needed... because:

paginate variable there https://github.com/insightsengineering/formatters/blob/main/R/mpf_exporters.R#L414C17-L414C25 is used before declaration when (.is_list_of_tables_or_listings(x)) is true:

export_as_rtf <- function(x,
                          file = NULL,
                          colwidths = NULL,
                          page_type = "letter",
                          pg_width = page_dim(page_type)[if (landscape) 2 else 1],
                          pg_height = page_dim(page_type)[if (landscape) 1 else 2],
                          landscape = FALSE,
                          margins = c(bottom = .5, left = .75, top = .5, right = .75),
                          font_family = "Courier",
                          font_size = 8,
                          lineheight = 1,
                          fontspec = font_spec(font_family, font_size, lineheight),
                          ...) {
  # Processing lists of tables or listings
  if (.is_list_of_tables_or_listings(x)) {
    if (isFALSE(paginate)) {
      warning(
        "paginate is FALSE, but x is a list of tables or listings, ",
        "so paginate will automatically be updated to TRUE"
      )
    }
    paginate <- TRUE
  }

Please let me know if you still need minimal code to reproduce error ;)

@shajoezhu
Copy link
Contributor

i see! thanks for bring this to our attention. export_as_rtf is not a function that we actively testing, improvement is definitely needed. please can you share an example, I dont use rtf output, so we dont have any good example to reproduce, and debug this. thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants