Skip to content

Force brace expression in function calls be on their own line #543

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

Merged
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
3 changes: 3 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@

## Minor improvements and fixes

* brace expressions in function calls are formatted in a less compact way. This
improves the formatting of `tryCatch()` in many cases (#543).

* escape characters in roxygen code examples are now correctly escaped (#512).

* style selection Addin now preserves line break when the last line selected is
Expand Down
2 changes: 1 addition & 1 deletion R/io.R
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ read_utf8_bare <- function(con, warn = TRUE) {
"The file ", con, " is not encoded in UTF-8. ",
"These lines contain invalid UTF-8 characters: "
),
paste(c(head(i), if (n > 6) "..."), collapse = ", ")
paste(c(utils::head(i), if (n > 6) "..."), collapse = ", ")
)
}
x
Expand Down
84 changes: 77 additions & 7 deletions R/rules-line-break.R
Original file line number Diff line number Diff line change
@@ -1,18 +1,88 @@
# A { should never go on its own line
remove_line_break_before_curly_opening <- function(pd) {
rm_break_idx <- which((pd$token_after == "'{'") & (pd$token != "COMMENT"))
rm_break_idx <- setdiff(rm_break_idx, nrow(pd))
if (length(rm_break_idx) > 0) {
#' Set line break before a curly brace
#'
#' Rule:
#' * Principle: Function arguments that consist of a braced expression always
#' need to start on a new line
#' * Exception: [...] unless it's the last argument and all other
#' arguments fit on the line of the function call
#' * Exception: [...] or they are named.
#' * Extension: Also, expressions following on braced expressions also cause a
#' line trigger.
#' @keywords internal
#' @examples
#' \dontrun{
#' tryCatch(
#' {
#' f(8)
#' },
#' error = function(e) NULL
#' )
#' # last-argument case
#' testthat("braces braces are cool", {
#' code(to = execute)
#' })
#' call2(
#' x = 2,
#' {
#' code(to = execute)
#' },
#' c = { # this is the named case
#' g(x = 7)
#' }
#' )
#' tryGugus(
#' {
#' g5(k = na)
#' },
#' a + b # line break also here because
#' # proceded by brace expression
#' )
#' }
set_line_break_before_curly_opening <- function(pd) {
line_break_to_set_idx <- which(
(pd$token_after == "'{'") & (pd$token != "COMMENT")
)

line_break_to_set_idx <- setdiff(line_break_to_set_idx, nrow(pd))
if (length(line_break_to_set_idx) > 0) {
is_not_curly_curly <- map_chr(
rm_break_idx + 1L,
line_break_to_set_idx + 1L,
~ next_terminal(pd[.x, ], vars = "token_after")$token_after
) != "'{'"
is_not_curly_curly_idx <- rm_break_idx[is_not_curly_curly]
last_expr_idx <- max(which(pd$token == "expr"))
is_last_expr <- ifelse(pd$token[1] == "IF",
# rule not applicable for IF
TRUE, (line_break_to_set_idx + 1L) == last_expr_idx
)
eq_sub_before <- pd$token[line_break_to_set_idx] == "EQ_SUB"
# no line break before last brace expression and named brace expression to
should_be_on_same_line <- is_not_curly_curly & (is_last_expr | eq_sub_before)
is_not_curly_curly_idx <- line_break_to_set_idx[should_be_on_same_line]
pd$lag_newlines[1 + is_not_curly_curly_idx] <- 0L

# other cases: line breaks
should_not_be_on_same_line <- is_not_curly_curly & (!is_last_expr & !eq_sub_before)
should_not_be_on_same_line_idx <- line_break_to_set_idx[should_not_be_on_same_line]

pd$lag_newlines[1 + should_not_be_on_same_line_idx] <- 1L

# non-curly expressions after curly expressions must have line breaks
if (length(should_not_be_on_same_line_idx) > 0) {
comma_exprs_idx <- which(pd$token == "','")
comma_exprs_idx <- setdiff(comma_exprs_idx, 1 + is_not_curly_curly_idx)
non_comment_after_comma <- map_int(comma_exprs_idx,
next_non_comment,
pd = pd
)
non_comment_after_expr <-
non_comment_after_comma[non_comment_after_comma > should_not_be_on_same_line_idx[1]]
pd$lag_newlines[non_comment_after_comma] <- 1L
}
}
pd
}


set_line_break_around_comma <- function(pd) {
comma_with_line_break_that_can_be_removed_before <-
(pd$token == "','") &
Expand Down
2 changes: 1 addition & 1 deletion R/style-guides.R
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ tidyverse_style <- function(scope = "tokens",
line_break_manipulators <- if (scope >= "line_breaks") {
lst(
set_line_break_around_comma,
remove_line_break_before_curly_opening,
set_line_break_before_curly_opening,
remove_line_break_before_round_closing_after_curly =
if (strict) remove_line_break_before_round_closing_after_curly,
remove_line_break_before_round_closing_fun_dec =
Expand Down
12 changes: 12 additions & 0 deletions man/invalid_utf8.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

37 changes: 37 additions & 0 deletions man/set_line_break_before_curly_opening.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

18 changes: 18 additions & 0 deletions tests/testthat/curly-curly/mixed-in.R
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,24 @@ call({

## ............................................................................
## multiple ####
call("test", {
1
})

call(
"test", {
1
})

call("test",
{
1
})

call("test", {
1 }
)

call({
1
}, a + b, { 33 / f(c)})
Expand Down
Loading