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

Adapt to single indent semantics in style guide #1235

Merged
merged 4 commits into from
Nov 25, 2024
Merged
Show file tree
Hide file tree
Changes from 3 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
16 changes: 9 additions & 7 deletions R/rules-indention.R
Original file line number Diff line number Diff line change
Expand Up @@ -15,36 +15,38 @@ indent_braces <- function(pd, indent_by) {
#'
#' Necessary for consistent indention of the function declaration header.
#' @param pd A parse table.
#' @inheritParams is_double_indent_function_declaration
#' @inheritParams is_single_indent_function_declaration
#' @seealso set_unindention_child update_indention_ref_fun_dec
#' @keywords internal
unindent_fun_dec <- function(pd, indent_by = 2L) {
if (is_function_declaration(pd)) {
idx_closing_brace <- which(pd$token == "')'")
fun_dec_head <- seq2(2L, idx_closing_brace)
if (is_double_indent_function_declaration(pd, indent_by = indent_by)) {
pd$indent[fun_dec_head] <- 2L * indent_by
if (is_single_indent_function_declaration(pd, indent_by = indent_by)) {
pd$indent[fun_dec_head] <- indent_by
pd$indent[idx_closing_brace] <- 0L
} else {
pd$indent[fun_dec_head] <- 0L
}
}
pd
}

#' Is the function declaration double indented?
#' Is the function declaration single indented?
#'
#' Assumes you already checked if it's a function with
#' `is_function_declaration`. It is double indented if the first token
#' `is_function_declaration`. It is single indented if the first token
#' after the first line break that is a `"SYMBOL_FORMALS"`.
#' @param pd A parse table.
#' @inheritParams tidyverse_style
#' @keywords internal
is_double_indent_function_declaration <- function(pd, indent_by = 2L) {
is_single_indent_function_declaration <- function(pd, indent_by = 2L) {
head_pd <- vec_slice(pd, -nrow(pd))
line_break_in_header <- which(head_pd$lag_newlines > 0L & head_pd$token == "SYMBOL_FORMALS")
if (length(line_break_in_header) > 0L) {
# indent results from applying the rules, spaces is the initial spaces
# (which is indention if a newline is ahead)
# The 2L factor is kept to convert double indent to single indent
pd$spaces[line_break_in_header[1L] - 1L] <= 2L * indent_by
} else {
FALSE
Expand Down Expand Up @@ -132,7 +134,7 @@ NULL
#'
#' @keywords internal
update_indention_ref_fun_dec <- function(pd_nested) {
if (is_function_declaration(pd_nested) && !is_double_indent_function_declaration(pd_nested)) {
if (is_function_declaration(pd_nested) && !is_single_indent_function_declaration(pd_nested)) {
seq <- seq2(3L, nrow(pd_nested) - 2L)
pd_nested$indention_ref_pos_id[seq] <- pd_nested$pos_id[2L]
}
Expand Down
6 changes: 4 additions & 2 deletions R/rules-line-breaks.R
Original file line number Diff line number Diff line change
Expand Up @@ -236,15 +236,17 @@ remove_line_break_before_round_closing_after_curly <- function(pd) {

remove_line_breaks_in_fun_dec <- function(pd) {
if (is_function_declaration(pd)) {
is_double_indention <- is_double_indent_function_declaration(pd)
is_double_indention <- is_single_indent_function_declaration(pd)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks odd. Need to rename is_double_indention?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@krlmlr can you fix this? You have

    is_double_indention <- is_single_indent_function_declaration(pd)

I think we need to rename the variable name and keep the function name. Then I'll approve the PR and we can merge.

round_after <- (
pd$token == "')'" | pd$token_before == "'('"
) &
pd$token_before != "COMMENT"
pd$lag_newlines[pd$lag_newlines > 1L] <- 1L
pd$lag_newlines[round_after] <- 0L
if (is_double_indention) {
pd$lag_newlines[lag(pd$token == "'('")] <- 1L
pd$lag_newlines[round_after] <- 1L
} else {
pd$lag_newlines[round_after] <- 0L
}
}
pd
Expand Down

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

7 changes: 4 additions & 3 deletions tests/testthat/fun_dec/line_break_fun_dec-out.R
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,9 @@ a <- function(x,
}

a <- function(
#
x,
y) {
#
x,
y
) {
x - 1
}
Original file line number Diff line number Diff line change
@@ -1,27 +1,27 @@
function(a =
33,
b
) {}
33,
b
) {}

function(a =
33,
b) {}
33,
b) {}

function(a,
b,
c
) {}
b,
c
) {}

function(a,
b,
c) {}
b,
c) {}

function(ss,
a =
3,
er =
4
) {}
a =
3,
er =
4
) {}

function(a =
b,
Expand Down
39 changes: 22 additions & 17 deletions tests/testthat/indention_operators/eq_formals_complex_tokens-out.R
Original file line number Diff line number Diff line change
@@ -1,29 +1,34 @@
function(
a =
33,
b) {}
a =
33,
b
) {}

function(
a =
33,
b) {}
a =
33,
b
) {}

function(
a,
b,
c) {}
a,
b,
c
) {}

function(
a,
b,
c) {}
a,
b,
c
) {}

function(
ss,
a =
3,
er =
4) {}
ss,
a =
3,
er =
4
) {}

function(a =
b,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ b
# multiple nested levels
{
v <- function(x =
122,
y) {
122,
y) {
}
}

Expand Down
32 changes: 18 additions & 14 deletions tests/testthat/unindention/mixed-double-out.R
Original file line number Diff line number Diff line change
Expand Up @@ -37,29 +37,33 @@ function(x,

# double
function(
x,
y) {
x,
y
) {
1
}


function(
x,
y,
k) {
x,
y,
k
) {
1
}


function(
x,
y) {
x,
y
) {
1
}


function(
x, y) {
x, y
) {
1
}

Expand All @@ -72,23 +76,23 @@ function(x,

# last brace
function(
x, y) {
x, y) {
NULL
}

function(
x, y) {
x, y) {
NULL
}

function(
x,
y) {
x,
y) {
NULL
}

function(
x,
y) {
x,
y) {
NULL
}
Loading