Avoid repetitive calls to print() [default] inside print.data.table()

[MichaelChirico]

          we really shouldn't need all this repetition. but that's not relevant to this PR.

Originally posted by @MichaelChirico in #6087 (comment)

As illustrated in the linked PR, it's a pain to maintain since we have to remember to update 4 separate call sites to print.default(), all with the same signature. This should be unified.

[joshhwuu]

What do we think of changing each call to print.default into a do.call with a list of printing arguments? This way, if we ever need to add additional arguments we can just update the argument list, instead of going to every call and adding the extra argument.

We could also make a function wrapper that calls print.default, but we would have to pass in the arguments to this wrapper anyways, which kind of defeats the purpose.

[MichaelChirico]

We could also make a function wrapper that calls print.default, but we would have to pass in the arguments to this wrapper anyways, which kind of defeats the purpose.

Not if the wrapper defined within the print.data.table() body, e.g.

print.data.table = function(...) {
  # ...
  print_default = function(x) print(x, quote=quote, na.print=na.print, right=right)
  # ...
  if (...) {
    print_default()
  } else {
    print_default()
  }
  # ...
}

The third option is if we can refactor the code to avoid needing 4 call sites to begin with, e.g.

data.table/R/print.data.table.R

Lines 122 to 124 in 7268eff

	cut_colnames(print(toprint, right=TRUE, quote=quote, na.print=na.print))
	} else {
	print(toprint, right=TRUE, quote=quote, na.print=na.print)

Should we do if (col.names != "none") cut_colnames=identity? Then we can run cut_colnames(print(...)) in both branches.

Can we combine the printdots and !printdots cases as well? Needs a bit more careful thought.

[joshhwuu]

Looking at it now, quite a bit of the code in printdots and !printdots branches can be factored out:

data.table/R/print.data.table.R

Lines 121 to 128 in 7268eff

	if (col.names == "none") {
	cut_colnames(print(toprint, right=TRUE, quote=quote, na.print=na.print))
	} else {
	print(toprint, right=TRUE, quote=quote, na.print=na.print)
	}
	if (trunc.cols && length(not_printed) > 0L)
	# prints names of variables not shown in the print
	trunc_cols_message(not_printed, abbs, class, col.names)

and

data.table/R/print.data.table.R

Lines 136 to 143 in 7268eff

	if (col.names == "none") {
	cut_colnames(print(toprint, right=TRUE, quote=quote, na.print=na.print))
	} else {
	print(toprint, right=TRUE, quote=quote, na.print=na.print)
	}
	if (trunc.cols && length(not_printed) > 0L)
	# prints names of variables not shown in the print
	trunc_cols_message(not_printed, abbs, class, col.names)

Would it be beneficial to factor out these two big chunks as a function in the print.data.table body, or even as a helper, and then handle:

Should we do if (col.names != "none") cut_colnames=identity? Then we can run cut_colnames(print(...)) in both branches.

in the factored code?

[MichaelChirico]

It seems reasonable to me to make the print.data.table() body simpler / relying on helpers. We have pretty good tests of output already, so I think you can feel comfortable refactoring as long as you pass tests.