Skip to content

Commit

Permalink
- Merge branch indent-eq-formals into master (r-lib#353)
Browse files Browse the repository at this point in the history
  • Loading branch information
lorenzwalthert committed Mar 1, 2018
2 parents d771f9d + b591a12 commit 26413c6
Show file tree
Hide file tree
Showing 15 changed files with 190 additions and 53 deletions.
72 changes: 59 additions & 13 deletions R/indent.R
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ indent_braces <- function(pd, indent_by) {
set_unindention_child(pd, token = "')'", unindent_by = indent_by)
}

#' @describeIn update_indention Indents operators
#' @describeIn update_indention Indents *all* tokens after `token` - including
#' the last token.
indent_op <- function(pd,
indent_by,
token = c(
Expand All @@ -35,13 +36,29 @@ indent_op <- function(pd,
pd
}

#' Revert the indention of function declaration header
#'
#' Necessary for consistent indention of the function declaration header.
#' @param pd A parse table.
#' @seealso set_unindention_child update_indention_ref_fun_dec
unindent_fun_dec <- function(pd) {
if (is_function_dec(pd)) {
idx_closing_brace <- which(pd$token %in% "')'")
fun_dec_head <- seq2(2L, idx_closing_brace)
pd$indent[fun_dec_head] <- 0L
}
pd
}

#' @describeIn update_indention Updates indention for token EQ_SUB. Only differs
#' from indent_op in the sense that the last token on the table where EQ_SUB
#' from [indent_op()] in the sense that not all subsequent tokens in the parse
#' table are necessarily indented, as `EQ_SUB` and `EQ_FORMALS` can occur
#' multiple times in a parse table.
#' occurs is not indented (see[compute_indent_indices()])
indent_eq_sub <- function(pd,
indent_by,
token = "EQ_SUB") {
eq_sub <- which(pd$token == "EQ_SUB")
token = c("EQ_SUB", "EQ_FORMALS")) {
eq_sub <- which(pd$token %in% token)
if (length(eq_sub) == 0) return(pd)
has_line_break <- which(pd$lag_newlines > 0)
indent_indices <- intersect(eq_sub + 1, has_line_break)
Expand Down Expand Up @@ -126,7 +143,9 @@ compute_indent_indices <- function(pd,
token_closing = NULL) {
npd <- nrow(pd)
potential_triggers <- which(pd$token %in% token_opening)
needs_indention <- needs_indention(pd, potential_triggers)
needs_indention <- needs_indention(pd, potential_triggers,
other_trigger_tokens = c("EQ_SUB", "EQ_FORMALS")
)
trigger <- potential_triggers[needs_indention][1]
if (is.na(trigger)) return(numeric(0))
start <- trigger + 1
Expand All @@ -144,28 +163,55 @@ compute_indent_indices <- function(pd,
#'
#' Checks for each potential trigger token in `pd` whether it actually should
#' cause indention.
#' @param potential_triggers A vector with indices of the potential trigger
#' @param potential_triggers_pos A vector with indices of the potential trigger
#' tokens in `pd`.
#' @inheritParams needs_indention_one
needs_indention <- function(pd, potential_triggers) {
map_lgl(potential_triggers, needs_indention_one, pd = pd)
needs_indention <- function(pd,
potential_triggers_pos,
other_trigger_tokens = NULL) {
map_lgl(potential_triggers_pos, needs_indention_one,
pd = pd, other_trigger_tokens = other_trigger_tokens
)
}


#' Check whether indention is needed
#'
#' Indention is needed if and only if there is no multi-line token between the
#' trigger and the first line break.
#' Indention is needed if the two conditions apply:
#'
#' * there is no multi-line token between the trigger and the first line break.
#' * there is no other token between the potential trigger and the first line
#' break that is going to cause indention.
#'
#' @param pd A parse table.
#' @param potential_trigger the index of the token in the parse table
#' @param potential_trigger_pos the index of the token in the parse table
#' for which it should be checked whether it should trigger indention.
#' @return Returns `TRUE` if indention is needed, `FALSE` otherwise.
#' @param other_trigger_tokens Other tokens that are going to cause indention
#' if on the same line as the token corresponding to `potential_trigger`.
#' @return `TRUE` if indention is needed, `FALSE` otherwise.
#' @importFrom rlang seq2
needs_indention_one <- function(pd, potential_trigger) {
needs_indention_one <- function(pd,
potential_trigger_pos,
other_trigger_tokens) {
before_first_break <- which(pd$lag_newlines > 0)[1] - 1
if (is.na(before_first_break)) return(FALSE)
!any(pd$multi_line[seq2(potential_trigger, before_first_break)])
row_idx_between_trigger_and_line_break <- seq2(
potential_trigger_pos, before_first_break
)
multi_line_token <- pd_is_multi_line(
pd[row_idx_between_trigger_and_line_break, ]
)
remaining_row_idx_between_trigger_and_line_break <- setdiff(
row_idx_between_trigger_and_line_break,
potential_trigger_pos
)

other_trigger_on_same_line <-
pd$token[remaining_row_idx_between_trigger_and_line_break] %in%
other_trigger_tokens

!any(multi_line_token) & !any(other_trigger_on_same_line)
}


Expand Down
35 changes: 29 additions & 6 deletions R/reindent.R
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ update_indention_ref_fun_call <- function(pd_nested) {
#' @importFrom rlang seq2
update_indention_ref_fun_dec <- function(pd_nested) {
if (pd_nested$token[1] == "FUNCTION") {
seq <- seq2(3, nrow(pd_nested) - 1)
seq <- seq2(3, nrow(pd_nested) - 2)
pd_nested$indention_ref_pos_id[seq] <- pd_nested$pos_id[2]
}
pd_nested
Expand Down Expand Up @@ -91,15 +91,12 @@ apply_ref_indention <- function(flattened_pd) {
#' @param target_token The index of the token from which the indention level
#' should be applied to other tokens.
apply_ref_indention_one <- function(flattened_pd, target_token) {
token_points_to_ref <-
flattened_pd$indention_ref_pos_id == flattened_pd$pos_id[target_token]
first_token_on_line <- flattened_pd$lag_newlines > 0L
token_to_update <- which(token_points_to_ref & first_token_on_line)

token_to_update <- find_tokens_to_update(flattened_pd, target_token)
# udate spaces
copied_spaces <- flattened_pd$col2[target_token]
old_spaces <- flattened_pd$lag_spaces[token_to_update[1]]
shift <- copied_spaces - old_spaces
shift <- copied_spaces
flattened_pd$lag_spaces[token_to_update] <-
flattened_pd$lag_spaces[token_to_update] + shift

Expand All @@ -110,6 +107,32 @@ apply_ref_indention_one <- function(flattened_pd, target_token) {
flattened_pd
}

#' Find the tokens to update when applying a reference indention
#'
#' Given a target token and a flattened parse table, the token for which the
#' spacing information needs to be updated are computed. Since indention is
#' already embeded in the column `lag_spaces`, only tokens at the beginning of
#' a line are of concern.
#' @param flattened_pd A flattened parse table.
#' @inheritParams apply_ref_indention_one
#' @seealso apply_ref_indention_one()
#' @examples
#' style_text("function(a =
#' b,
#' dd
#' ) {}", scope = "indention")
#' style_text("function(a,
#' b,
#' dd
#' ) {}", scope = "indention")
find_tokens_to_update <- function(flattened_pd, target_token) {
token_points_to_ref <-
flattened_pd$indention_ref_pos_id == flattened_pd$pos_id[target_token]
first_token_on_line <- flattened_pd$lag_newlines > 0L
which(token_points_to_ref & first_token_on_line)
}


#' Set indention of tokens that match regex
#'
#' Force the level of indention of tokens whose text matches a regular
Expand Down
1 change: 1 addition & 0 deletions R/style_guides.R
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ tidyverse_style <- function(scope = "tokens",
space_manipulators <- if (scope >= "spaces") {
lst(
indent_braces = partial(indent_braces, indent_by = indent_by),
unindent_fun_dec,
indent_op = partial(indent_op, indent_by = indent_by),
indent_eq_sub = partial(indent_eq_sub, indent_by = indent_by),
indent_without_paren = partial(indent_without_paren,
Expand Down
1 change: 1 addition & 0 deletions R/transform-files.R
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ parse_transform_serialize <- function(text, transformers) {
#' hence line breaks must be modified first).
#' * spacing rules (must be after line-breaks and updating newlines and
#' multi-line).
#' * indention.
#' * token manipulation / replacement (is last since adding and removing tokens
#' will invalidate columns token_after and token_before).
#' * Update indention reference (must be after line breaks).
Expand Down
1 change: 1 addition & 0 deletions man/apply_transformers.Rd

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

33 changes: 33 additions & 0 deletions man/find_tokens_to_update.Rd

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

7 changes: 5 additions & 2 deletions man/needs_indention.Rd

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

17 changes: 13 additions & 4 deletions man/needs_indention_one.Rd

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

17 changes: 17 additions & 0 deletions man/unindent_fun_dec.Rd

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

9 changes: 6 additions & 3 deletions man/update_indention.Rd

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

2 changes: 1 addition & 1 deletion tests/testthat/fun_dec/line_break_fun_dec-out.R
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,6 @@ a <- function(x, #

a <- function(x, #
y #
) {
) {
y
}
2 changes: 1 addition & 1 deletion tests/testthat/indention_operators/eq_formal_simple-out.R
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
abbbb <- function(x =
22) {
22) {
data_frame(
x =
long_long_long * x
Expand Down
Original file line number Diff line number Diff line change
@@ -1,33 +1,33 @@
function(a =
33,
33,
b
) {}
) {}

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

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

function(a,
b,
c) {}

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

function(a =
b,
b,
f =
d, c =
3, d =
4) {
d, c =
3, d =
4) {

}
Loading

0 comments on commit 26413c6

Please sign in to comment.