Remove excessive line breaks between top-level expressions

[IndrajeetPatil]

Partly related to #1238, but kind of veered away from it during implementation. Opening for an early feedback.

library(styler)
styler::cache_deactivate()
#> Deactivated cache.

# Example-1 --------------------------------

"f <- function() NULL






g <- function() NULL" -> code

style_text(text = code)
#> f <- function() NULL
#> 
#> 
#> g <- function() NULL

# Example-2 --------------------------------

"while (TRUE) {
  print('hello')
}







for (i in 1:10) {
  print(i)
}" -> code

style_text(text = code)
#> while (TRUE) {
#>   print("hello")
#> }
#> 
#> 
#> for (i in 1:10) {
#>   print(i)
#> }

# Example-3 --------------------------------

"#' @param x A numeric vector
#' @return bla bla bla
f <- function(x) NULL







#' @param y A numeric vector
#' @return bla bla bla
g <- function(y) NULL" -> code

style_text(text = code)
#> #' @param x A numeric vector
#> #' @return bla bla bla
#> f <- function(x) NULL
#> 
#> 
#> #' @param y A numeric vector
#> #' @return bla bla bla
#> g <- function(y) NULL

^{Created on 2024-11-28 with reprex v2.1.1}

[IndrajeetPatil]

Style guide recommends one line breaks, but only for function calls. But that itself felt quite aggressive, given the huge numbers of changes in test outputs.

2L nicely aligns with Python and feels "natural" as well.

[lorenzwalthert]

Lets limit to two blank lines as towards the style guide and decide later if we'll go all the way to one line break. Also, can you please apply the 2L limit to strict = TRUE only?

[github-actions]

This is how benchmark results would change (along with a 95% confidence interval in relative change) if 1166e3b is merged into main:

• ✔️cache_applying: 184ms -> 183ms [-2.51%, +1.68%]
• ✔️cache_recording: 556ms -> 555ms [-0.85%, +0.53%]
• ✔️without_cache: 1.05s -> 1.05s [-1.56%, +2.38%]

Further explanation regarding interpretation and methodology can be found in the documentation.

[github-actions]

This is how benchmark results would change (along with a 95% confidence interval in relative change) if 5705339 is merged into main:

• ✔️cache_applying: 155ms -> 155ms [-2.77%, +2.92%]
• ✔️cache_recording: 518ms -> 523ms [-0.17%, +1.93%]
• ✔️without_cache: 993ms -> 1s [-0.22%, +1.79%]

Further explanation regarding interpretation and methodology can be found in the documentation.

[github-actions]

This is how benchmark results would change (along with a 95% confidence interval in relative change) if 62c29dd is merged into main:

• ✔️cache_applying: 142ms -> 141ms [-1.06%, +0.44%]
• ✔️cache_recording: 496ms -> 498ms [-0.01%, +0.89%]
• ❗🐌without_cache: 952ms -> 961ms [+0.33%, +1.68%]

Further explanation regarding interpretation and methodology can be found in the documentation.

[github-actions]

This is how benchmark results would change (along with a 95% confidence interval in relative change) if 62c29dd is merged into main:

• ✔️cache_applying: 160ms -> 158ms [-4.36%, +2.17%]
• ✔️cache_recording: 518ms -> 523ms [-0.33%, +2.29%]
• ✔️without_cache: 1000ms -> 1.01s [-0.07%, +1.92%]

Further explanation regarding interpretation and methodology can be found in the documentation.

[IndrajeetPatil]

@lorenzwalthert I am not fully sold on the testing strategy here. Instead of tweaking the existing tests, I think I should create a new folder for "line breaks and top-level expressions" and add all new test cases there.

WDYT?

[lorenzwalthert]

@lorenzwalthert I am not fully sold on the testing strategy here. Instead of tweaking the existing tests, I think I should create a new folder for "line breaks and top-level expressions" and add all new test cases there.

Right... Yes, you could crate a new folder if you want. Also, I renamed the transformer (again) to make its name more consistent with existing names of line break related transformes.

[github-actions]

This is how benchmark results would change (along with a 95% confidence interval in relative change) if 7dc83ba is merged into main:

• ✔️cache_applying: 152ms -> 151ms [-2.42%, +1.93%]
• ✔️cache_recording: 516ms -> 521ms [-0.22%, +1.84%]
• ✔️without_cache: 985ms -> 990ms [-0.18%, +1.23%]

Further explanation regarding interpretation and methodology can be found in the documentation.

[github-actions]

This is how benchmark results would change (along with a 95% confidence interval in relative change) if 6b5a4c5 is merged into main:

• ✔️cache_applying: 184ms -> 186ms [-1.91%, +4.4%]
• ❗🐌cache_recording: 534ms -> 542ms [+0.05%, +2.93%]
• ✔️without_cache: 1.05s -> 1.06s [-0.11%, +2.02%]

Further explanation regarding interpretation and methodology can be found in the documentation.

[lorenzwalthert]

Thanks a lot @IndrajeetPatil 🥳