Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove excessive line breaks between top-level expressions #1239

Merged
merged 17 commits into from
Dec 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
# All available hooks: https://pre-commit.com/hooks.html
# R specific hooks: https://github.com/lorenzwalthert/precommit
default_stages: [commit]
default_stages: [pre-commit]
default_language_version:
python: python3

repos:
- repo: https://github.com/lorenzwalthert/precommit
rev: v0.4.2
rev: v0.4.3.9003
hooks:
- id: style-files
args:
Expand Down
6 changes: 6 additions & 0 deletions R/rules-line-breaks.R
Original file line number Diff line number Diff line change
Expand Up @@ -447,3 +447,9 @@ remove_empty_lines_after_opening_and_before_closing_braces <- function(pd) {

pd
}


set_line_breaks_between_top_level_exprs <- function(pd, allowed_blank_lines = 2L) {
pd$lag_newlines <- pmin(pd$lag_newlines, allowed_blank_lines + 1L)
pd
}
2 changes: 2 additions & 0 deletions R/style-guides.R
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,8 @@ tidyverse_style <- function(scope = "tokens",
if (strict) remove_line_break_before_round_closing_after_curly,
remove_line_breaks_in_fun_dec =
if (strict) remove_line_breaks_in_fun_dec,
set_line_breaks_between_top_level_exprs =
if (strict) set_line_breaks_between_top_level_exprs,
style_line_break_around_curly = partial(
style_line_break_around_curly,
strict
Expand Down
1 change: 0 additions & 1 deletion tests/testthat/alignment/named-out.R
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,6 @@ call(
)



# if all col1 arguments are named, col1 must also be aligned
# not aligned
fell(
Expand Down
1 change: 0 additions & 1 deletion tests/testthat/indention_curly_brackets/custom-out.R
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ if (value > 0) {
}



if (value > 0) {
print(value)
}
2 changes: 0 additions & 2 deletions tests/testthat/line_breaks_and_other/assignment-out.R
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ x <-
3



ImportantDataFrame$ImportantColumn1 <-
ImportantDataFrame$ImportantColumn2 <-
ComplicatedFunction(ImportantDataFrame$InputColumn)
Expand All @@ -32,6 +31,5 @@ ImportantDataFrame$ImportantColumn1 <-
ImportantDataFrame$ImportantColumn2 <- ComplicatedFunction(ImportantDataFrame$InputColumn)



ImportantDataFrame$ImportantColumn1 <-
ImportantDataFrame$ImportantColumn2 <- ComplicatedFunction(ImportantDataFrame$InputColumn)
1 change: 1 addition & 0 deletions tests/testthat/line_breaks_and_other/ggplot2-in.R
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ ggplot(data = mtcars, mapping = aes(x = mpg, y = vs)) +
ggplot(data = mtcars, mapping = aes(x = mpg, y = vs)) + ggplot2::geom_point() + g() # comment



# add when comment
ggplot(data = mtcars, mapping = aes(x = mpg, y = vs)) +
ggplot2::geom_point() + g() + geom_oint() # comment
Expand Down
1 change: 0 additions & 1 deletion tests/testthat/line_breaks_fun_call/blank-strict-out.R
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ call(
2,



# comment

1,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ switch( #
)



switch(x,
a = 2, #
y = 3
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ call(call(
))



# no more barcket on same line ->
call(call(
3, 4
Expand Down
16 changes: 16 additions & 0 deletions tests/testthat/line_breaks_top_level_exprs/braces-in.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
{
1 + 1
}





### some comment




{
NULL
}
11 changes: 11 additions & 0 deletions tests/testthat/line_breaks_top_level_exprs/braces-out.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
1 + 1
}


### some comment


{
NULL
}
20 changes: 20 additions & 0 deletions tests/testthat/line_breaks_top_level_exprs/conditionals-in.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
for (i in 1:10) {}






# some comment





while (TRUE) {}




# some comment
if (TRUE) NULL
11 changes: 11 additions & 0 deletions tests/testthat/line_breaks_top_level_exprs/conditionals-out.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
for (i in 1:10) {}


# some comment


while (TRUE) {}


# some comment
if (TRUE) NULL
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
f <- function() NULL








g <- function() NULL




f()

# comment





g()
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
f <- function() NULL


g <- function() NULL


f()

# comment


g()
23 changes: 23 additions & 0 deletions tests/testthat/line_breaks_top_level_exprs/non_strict-in.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
f <- function() NULL








g <- function() NULL




f()

# comment





g()
23 changes: 23 additions & 0 deletions tests/testthat/line_breaks_top_level_exprs/non_strict-out.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
f <- function() NULL








g <- function() NULL




f()

# comment





g()
21 changes: 21 additions & 0 deletions tests/testthat/line_breaks_top_level_exprs/piped_chains-in.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@


f %>% g()






h %>% i()





# some comment




j %>% k()
10 changes: 10 additions & 0 deletions tests/testthat/line_breaks_top_level_exprs/piped_chains-out.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
f %>% g()


h %>% i()


# some comment


j %>% k()
2 changes: 0 additions & 2 deletions tests/testthat/parse_comments/with_indention-out.R
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@ call(
# new comment




a()
# I think it gets boring
# new_line here
Expand Down
2 changes: 0 additions & 2 deletions tests/testthat/roxygen-examples-complete/13-empty-lines-out.R
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@
#' # two
#'
#'
#'
#'
#' (
#' # more
#' a <- 3
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ NULL
NULL



#' this
#'
#' empty line after example
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
#' Empty line in examples
#'
#' @examples
1






#' Empty line in examples
#'
#' @examples
#' \dontrun{
#' }
2
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
#' Empty line in examples
#'
#' @examples
1


#' Empty line in examples
#'
#' @examples
#' \dontrun{
#'
#' }
2
29 changes: 29 additions & 0 deletions tests/testthat/test-line_breaks_top_level_exprs.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
test_that("extra line breaks between conditional statements are removed", {
expect_no_warning(
test_collection("line_breaks_top_level_exprs", "conditionals", transformer = style_text)
)
})

test_that("extra line breaks between function and definitions and calls are removed", {
expect_no_warning(
test_collection("line_breaks_top_level_exprs", "function_defs_and_calls", transformer = style_text)
)
})

test_that("extra line breaks between piped chains are removed", {
expect_no_warning(
test_collection("line_breaks_top_level_exprs", "piped_chains", transformer = style_text)
)
})

test_that("extra line breaks between braced expressions are removed", {
expect_no_warning(
test_collection("line_breaks_top_level_exprs", "braces", transformer = style_text)
)
})

test_that("extra line breaks are not removed in non-strict mode", {
expect_no_warning(
test_collection("line_breaks_top_level_exprs", "non_strict", transformer = style_text, strict = FALSE)
)
})
5 changes: 5 additions & 0 deletions tests/testthat/test-roxygen-examples-complete-30.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# NB: DO NOT EDIT. Auto-generated by ./tests/dev/generate_roxygen_tests.R.

test_that("analogous to test-roxygen-examples-complete: 30", {
expect_warning(test_collection("roxygen-examples-complete", "^30-", transformer = style_text), NA)
})
1 change: 1 addition & 0 deletions tests/testthat/test-transformers-drop.R
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ test_that("tidyverse transformers are correctly dropped", {

names_line_break <- c(
"remove_empty_lines_after_opening_and_before_closing_braces",
"set_line_breaks_between_top_level_exprs",
"set_line_break_around_comma_and_or",
"set_line_break_after_assignment",
"set_line_break_after_opening_if_call_is_multi_line",
Expand Down
1 change: 0 additions & 1 deletion tests/testthat/token_adding_removing/mixed_token-out.R
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ d
'text with "quotes"'



# adding brackets in pipes
a %>%
b() %>%
Expand Down
Loading