styler icon indicating copy to clipboard operation
styler copied to clipboard

Allowing function-call-style function definitions (argument wrapping in signature)

Open MichaelChirico opened this issue 4 years ago • 1 comments

Consider the following:

poisson_gamma_mixture_extended_truncated_metric <- function(predictor_1,
                                                            predictor_2,
                                                            max_valid_value = 100) {
  # ...
}

styler is happy with this as is but the 3rd line is >80 characters.

My preferred approach to writing a function definition like this is:

poisson_gamma_mixture_extended_truncated_metric <- function(
  predictor_1,
  predictor_2,
  max_valid_value = 100
) {
  # ...
}

But styler::style_file() reverts this usage to the first one.

The compromise that makes styler happy and allows the function to fit within 80 chars is:

poisson_gamma_mixture_extended_truncated_metric <-
  function(predictor_1,
           predictor_2,
           max_valid_value = 100) {
    # ...
  }

But there are a few issues with this:

  1. Long lines in the function (79 or 80 characters) will be bumped to >80
  2. By bumping the indent level it makes it look like the function is defined on a different level of execution than it is
  3. For changing existing definitions, the diff on version control will look a lot bigger unless whitespace diffs are turned off (usually not by default)

Would it be OK for styler to leave definitions like the "preferred" one alone?

The style guide AFAICT is moot on this particular point. The closest we get is here: https://style.tidyverse.org/functions.html#long-lines-1

# Bad
long_function_name <- function(a = "a long argument",
  b = "another argument",
  c = "another long argument") {
  # Here it's hard to spot where the definition ends and the
  # code begins
}

My preferred usage I think makes clear the delineation between the signature and the function body by using ) { on its own separate line.

The style guide does recommend against such long function names as in my original example, but even with "more stylistic" function names this issue has arisen many times on our internal code.

Also filed https://github.com/tidyverse/style/issues/173 to the style guide upstream.

MichaelChirico avatar Aug 04 '21 20:08 MichaelChirico

I'll wait for https://github.com/tidyverse/style/issues/173 to decide on this. We are bound by that style guide. I can only say that personally I see the problems with the current rule, but I don't think they outweight the benefits of it, in particular regarding readability. It would be nice if R had more explicit namespacing like Python, to avoid very long names but that's out of scope for {styler}.

lorenzwalthert avatar Aug 04 '21 20:08 lorenzwalthert

Now I think we can tretat this as a duplicate of #845, which will be closed with #1083.

lorenzwalthert avatar Dec 27 '22 15:12 lorenzwalthert