Don't keep code as a string

[pawelru]

Please change the below so that we don't keep executed code as a string:

teal.reporter/R/Renderer.R

Lines 38 to 55 in b82c2ac

	format_code_block_function <- paste0(
	c(
	"code_block <- function (code_text) {",
	" df <- data.frame(code_text)",
	" ft <- flextable::flextable(df)",
	" ft <- flextable::delete_part(ft, part = 'header')",
	" ft <- flextable::autofit(ft, add_h = 0)",
	" ft <- flextable::fontsize(ft, size = 7, part = 'body')",
	" ft <- flextable::bg(x = ft, bg = 'lightgrey')",
	" ft <- flextable::border_outer(ft)",
	" if (flextable::flextable_dim(ft)$widths > 8) {",
	" ft <- flextable::width(ft, width = 8)",
	" }",
	" ft",
	"}"
	),
	collapse = "\n"
	)

If string is really needed then please convert code object into a string

[chlebowa]

Would you prefer a function that is never called but whose body is pasted into the Rmd? That was the case during review but I didn't like having a function that is never called.

[pawelru]

IMHO language (or expression if it's simple enough) is better choice to store a code rather than string. String should be just representation of it (e.g. used by print method). This obviously depends on the use case - I haven't analysed that one yet.
There is a big advantage of this being we don't hide code dependency. R CMD CHECK cannot interpret that code-string and detect package dependencies.

I didn't like having a function that is never called

I don't quite understand that. If it's not used why to keep it?
Are you sure we are not calling it?

teal.reporter/R/Renderer.R

Line 105 in b82c2ac

do.call(rmarkdown::render, args)

[chlebowa]

As you can see, the resulting string is a function definition statement. It is pasted into the resulting Rmd document and defined during rendering of that file. It is never called in the package itself.

I don't understand what you are linking to.

[pawelru]

I think it's called

teal.reporter/R/Renderer.R

Lines 31 to 82 in b82c2ac

	renderRmd = function(blocks, yaml_header, global_knitr = list()) {
	checkmate::assert_list(blocks, c("TextBlock", "PictureBlock", "NewpageBlock", "TableBlock", "RcodeBlock"))
	if (missing(yaml_header)) {
	yaml_header <- md_header(yaml::as.yaml(list(title = "Report")))
	}
	private$report_type <- get_yaml_field(yaml_header, "output")
	format_code_block_function <- paste0(
	c(
	"code_block <- function (code_text) {",
	" df <- data.frame(code_text)",
	" ft <- flextable::flextable(df)",
	" ft <- flextable::delete_part(ft, part = 'header')",
	" ft <- flextable::autofit(ft, add_h = 0)",
	" ft <- flextable::fontsize(ft, size = 7, part = 'body')",
	" ft <- flextable::bg(x = ft, bg = 'lightgrey')",
	" ft <- flextable::border_outer(ft)",
	" if (flextable::flextable_dim(ft)$widths > 8) {",
	" ft <- flextable::width(ft, width = 8)",
	" }",
	" ft",
	"}"
	),
	collapse = "\n"
	)
	parsed_global_knitr <- sprintf(
	"\n```{r setup, include=FALSE}\nknitr::opts_chunk$set(%s)\n%s\n```\n",
	capture.output(dput(global_knitr)),
	if (identical(private$report_type, "powerpoint_presentation")) {
	format_code_block_function
	} else {
	""
	}
	)
	parsed_blocks <- paste(
	unlist(
	lapply(blocks, function(b) private$block2md(b))
	),
	collapse = "\n\n"
	)
	rmd_text <- paste0(yaml_header, "\n", parsed_global_knitr, "\n", parsed_blocks, "\n")
	tmp <- tempfile(fileext = ".Rmd")
	input_path <- file.path(
	private$output_dir,
	sprintf("input_%s.Rmd", gsub("[.]", "", format(Sys.time(), "%Y%m%d%H%M%OS3")))
	)
	cat(rmd_text, file = input_path)
	input_path
	},

This method is creates a .Rmd file and returns path to it

teal.reporter/R/Renderer.R

Lines 95 to 105 in b82c2ac

	input_path <- self$renderRmd(blocks, yaml_header, global_knitr)
	args <- append(args, list(
	input = input_path,
	output_dir = private$output_dir,
	output_format = "all",
	quiet = TRUE
	))
	args_nams <- unique(names(args))
	args <- lapply(args_nams, function(x) args[[x]])
	names(args) <- args_nams
	do.call(rmarkdown::render, args)

Line 95: we call a method and store returned file path
Line 96-104: we add more args to the list
Line 105: we call render() on that newly created file which essentially means that we call a code inputted there.

[chlebowa]

My mistake, I missed the fact that the report is rendered when it is saved.

Would you rather have

format_code_block_function <- quote(
    code_block <- function(code_text) {
        df <- data.frame(code_text)
        ft <- flextable::flextable(df)
        ft <- flextable::delete_part(ft, part = "header")
        ft <- flextable::autofit(ft, add_h = 0)
        ft <- flextable::fontsize(ft, size = 7, part = "body")
        ft <- flextable::bg(x = ft, bg = "lightgrey")
        ft <- flextable::border_outer(ft)
        if (flextable::flextable_dim(ft)$widths > 8) {
            ft <- flextable::width(ft, width = 8)
        }
        ft
    }
)

and then

paste(deparse(format_code_block_function), collapse = "\n")

?

[pawelru]

Yes. Way way better than code code as a string.