implementation of breaking long function calls

[lorenzwalthert]

The tidyverse style guide states that long function calls are not compliant with the style guide:

# Good
do_something_very_complicated(
  "that",
  requires = many,
  arguments = "some of which may be long"
)

# Bad
do_something_very_complicated("that", requires, many, arguments,
                              "some of which may be long"
                              )

It also states that multiple arguments can remain on the same line if they are related.

# Good
paste0(
  "Requirement: ", requires, "\n",
  "Result: ", result, "\n"
)

# Bad
paste0(
  "Requirement: ", requires,
  "\n", "Result: ",
  result, "\n")

I am not sure how to implement that and how strictly we should use it. A simple rule would be to add a line break after (, and leaving everything until ) untouched in terms of line breaks and putting the closing ) on a new line. Also, I think this should be a transformer that is only used if strict = TRUE since I think its one of the rules in the style guide that is not consistently obeyed by many people. @krlmlr maybe you can advise on that.

[krlmlr]

Your "simple rule" sounds good. It appears that we see the entire function call at the same level in the nested parse data -- we could look at col2 for the closing paren and decide, but this seems brittle if we insert line breaks or even space beforehand. We seem to have a similar problem for function declarations.

I suggest we leave this open for now.

[lorenzwalthert]

On a related note, I am also not sure about how closing brackets and pipes should interact, i.e.

call(b,
     c, d) %>%
  x()

# vs.
call(
  b, c, d
) %>%
  x

[lorenzwalthert]

I am not sure whether I understand you correctly, but I think we don't need to use col2. I think we can simply use lag_newlines to determine if there is a line break on the currently visited level. Since we use pre_visit(), we will deal with all multi-level expressions later during the visitting.
A simple implementation would be the following:

break_line_after_opening_if_call_is_multiline <- function(pd) {
  npd <- nrow(pd)
  if (npd < 2) return(pd)
  if (all(is.na(pd$token_before[2]))) return(pd)
  is_call <- pd$token_before[2] == "SYMBOL_FUNCTION_CALL"
  if (!is_call) return(pd)
  if (npd == 3) {
    pd$lag_newlines[3] <- 0L
    return(pd)
  }
  comments <- which(pd$token == "COMMENT")
  is_multi_line <- any(pd$lag_newlines[3:(npd - 1)] > 0)

  if (!is_multi_line) return(pd)

  pd$lag_newlines[setdiff(3, comments)] <- 1L
  pd
}

Note that it only adds line breaks after the opening parenthesis, not before the closing parenthesis. It could be easily extended though (or put in a seperate rule).

[krlmlr]

Closing parens and pipes: I like the second version better, but maybe it should be

b %>%
  call(
    c, d
  ) %>%
  x

(I'm not suggesting to do anything about it in styler.)

Looking at col2 would help detecting long lines, but then I may have misunderstood you entirely. We should consider always moving the closing ) to a separate line (because this seems to be easy to decide and implement), and keep the rest for later.

[krlmlr]

Closing in favor of #125.