Add newline after multiline function declaration

[krlmlr]

trafficstars

# Good
f1 <- function(a, b) {
  code
}

f2 <- function(a, b,
               c) {

  code
}

# Bad
f3 <- function() {

  code
}

[lorenzwalthert]

Is there a corresponding rule in the tidyverse style or do you plan to add one?

[krlmlr]

I remember Hadley suggested that an empty line after a long function header helps readability. I wouldn't formalize this as a rule yet, but I've seen this pattern a few times in dplyr.

[lorenzwalthert]

Ok. I think this is easy to implement. I just think for the sake of consistency, we should in general aim to keep the style guide in sync with styler.

[lorenzwalthert]

For a propper implementation, we'd need #357 because we want to add a line break dependent on whether a token is multi-line, which we cannot do if the multi-line attribute is not up-to date. The multi-line attribute is not up-to-date because it is only updated after all line break rules were applied.

[lorenzwalthert]

Blocked by #357.

[lorenzwalthert]

Implemented in mlr branch: https://github.com/lorenzwalthert/styler/commit/2a3fbc6fe278fbf9a09d229a49379d778536b5ed

[lorenzwalthert]

The implementation in the mlr branch now depends on the number of lines in the body.

[lorenzwalthert]

For a propper implementation, we'd need #357 because we want to add a line break dependent on whether a token is multi-line, which we cannot do if the multi-line attribute is not up-to date. The multi-line attribute is not up-to-date because it is only updated after all line break rules were applied.

Wrong. We need the exact line count, ~which we can compute ad-hoc within the transformer. It's as simple as in https://github.com/lorenzwalthert/styler/commit/d9f1b22f52437368446daf2c4c654481bf914784. Performance implications probably small for non-package code because only taking effect for nests that are function declarations. Implementing it otherwise is most likely not much faster (i.e. doing it with a visitor for all top-level tokens) because most function declarations are not nested.~ (apparently there are more nested cases where this is insufficient, see #773). This rule needs to be at the end of the line-break transformers because it depends on them.

[maelle]

@krlmlr any specific examples of this pattern? :pray:

[maelle]

For context I was looking into never removing the newline at the beginning of a function body if there was one, following a comment by @mpadge. :grin:

[krlmlr]

I have tweaked the "bad" example in the original post.

Keeping an extra newline, at least in non-strict mode, doesn't sound too bad as a starter.

[maelle]

I have tweaked the "bad" example in the original post.

That's what I had guessed it should be. :grin: