Merge pull request #69 from lorenzwalthert/indent_with_renesting

- Fix indention of long operator chains.
- Properly format unary operators (#38).

Author: krlmlr

NAMESPACE

@@ -18,4 +18,5 @@ importFrom(purrr,partial)
 importFrom(purrr,pmap)
 importFrom(purrr,pwalk)
 importFrom(purrr,reduce)
+importFrom(purrr,when)
 importFrom(utils,write.table)

R/get_transformers.R

@@ -59,8 +59,10 @@ get_transformers_nested <- function(strict = TRUE,
   c(create_filler,
     partial(indent_round, indent_by = indent_by),
     partial(indent_curly, indent_by = indent_by),
+    partial(indent_op, indent_by = indent_by),
     strip_eol_spaces,
     get_transformers_flat(strict, start_comments_with_one_space),
+    remove_space_after_unary_pm_nested,
     set_space_before_comments,
     set_space_between_levels
     )

R/modify_pd.R

@@ -3,6 +3,7 @@
 #' @param pd A nested or flat parse table that is already enhanced with
 #'   line break and space information via [create_filler()].
 #' @param indent_by How many spaces should be added after the token of interest.
+#' @param token The token the indention should be based on.
 #' @name update_indention
 NULL
 
@@ -76,6 +77,12 @@ child_indents <- function(pd, opening, token) {
 
 #' Check whether a parse table contains a token
 #'
+#' Checks whether a token is in a parse table. `pd_has_token_not_one` is usesful
+#'   in the situation where one wants to check whether the *operator* + or - is
+#'   in the parse table, but not the *sign* + or -.
+#'   We use the fact that signs always appear as first tokens in the parse
+#'   table. There are two functions and not just one for performance reasons
+#'   only, as this function is rather low level and gets called very often.
 #' @param pd A parse table.
 #' @param token The token for which it should be checked whether it is in the
 #'   parse table.
@@ -85,6 +92,29 @@ pd_has_token <- function(pd, token) {
   any(has_indention_token)
 }
 
+#' @rdname pd_has_token
+pd_has_token_not_first <- function(pd, token) {
+  has_indention_token <- token %in% pd$token[-1]
+  any(has_indention_token)
+}
+
+
+#' @rdname update_indention
+indent_op <- function(pd, indent_by, token = c(math_token, "SPECIAL")) {
+  opening <- which(pd$token %in% token)
+  if (length(opening) > 0) {
+    start <- opening[1] + 1
+    stop <- nrow(pd)
+  } else {
+    start <- stop <- 0
+  }
+  pd <- pd %>%
+    mutate(indent = indent + ifelse(seq_len(nrow(pd)) %in% start:stop, indent_by, 0)) %>%
+    select_(~indent, ~newlines, ~everything())
+  pd
+}
+
+
 #' Strip EOL spaces
 #'
 #' Remove end-of-line spaces.

R/nested_to_tree.R

@@ -2,10 +2,14 @@
 #'
 #' Create a tree representation from a text.
 #' @param text A character vector.
+#' @param re_nest Whether or not the parse table should be renested with
+#'   [re_nest()].
 #' @return A data frame.
-create_tree <- function(text) {
+#' @importFrom purrr when
+create_tree <- function(text, re_nest = FALSE) {
   compute_parse_data_nested(text) %>%
-    visit(c(styler:::create_filler)) %>%
+    visit(c(create_filler)) %>%
+    when(re_nest ~ re_nest(.), ~.) %>%
     create_node_from_nested_root() %>%
     as.data.frame()
 }

R/relevel.R

@@ -0,0 +1,72 @@
+#' Reorganize a nested parse table
+#'
+#' Certain tokens are not placed optimally in the nested parse data with
+#'   [compute_parse_data_nested()]. For example, the token of arithmetic
+#'   operations 1 + 1 + 1 should all be on the same level of nesting since
+#'   the indention is the same for all but the first two terminals. Setting the
+#'   indention correcly is easier to achieve if they are put on the same level
+#'   of nesting.
+#' @examples
+#' code <- "1+1+1+1"
+#' # tree-structure with compute_parse_data_nested
+#' styler:::create_tree(code, re_nest = FALSE)
+#' # tree-structure with re_nest post-processing
+#' styler:::create_tree(code, re_nest = TRUE)
+#' @inheritParams move_children_up
+re_nest <- function(pd_nested) {
+  if (is.null(pd_nested)) return()
+  pd_nested <- pd_nested %>%
+    move_children_up(token = c("'+'", "'-'", "SPECIAL", "'/'", "'*'")) %>%
+    mutate(child = map(child, re_nest))
+  pd_nested
+}
+
+#' Move children up if appropriate
+#'
+#' Move children one level of nesting up if necessary. If any was moved up,
+#'   check again whether any child of the former child should be moved up in a
+#'   recursive way.
+#' @param pd_nested A nested parse table to reorganize.
+#' @param token Character vector with tokens that can cause a child to be moved
+#'   up.
+#' @details A child is moved up under the following conditions:
+#'
+#'   * The parent of the child to be considered for moving up (that is simply
+#'     the corresponding row in `pd_nested`) is not a top-level expression.
+#'     Otherwise, the second expression may be indented due to operator
+#'     indention(e.g. in "1+1; 1+1").
+#'   * The child has one of `token` in the parse table. The first token is not
+#'     considered to propper handle signs (see [pd_has_token_not_first()]).
+#'   * One of `token` also appears before or after the child in `pd_nested`.
+#'     For example, there is no need to move any child up in the expression
+#'     "{1+1}", since then, operator and bracket indention would conflict.
+#' @importFrom purrr map_lgl
+move_children_up <- function(pd_nested,
+                             token = c("'+'", "'-'", "SPECIAL", "'/'", "'*'")) {
+
+  move_up <- pd_nested$parent > 0 &
+    map_lgl(pd_nested$child, pd_has_token_not_first, token) &
+    token_is_adjacent(pd_nested, token)
+
+  out <- bind_rows(pd_nested, pd_nested$child[move_up]) %>%
+    mutate(old_parent = id %in% parent) %>%
+    filter(!old_parent) %>%
+    select(-old_parent) %>%
+    arrange(line1, col1)
+  if (any(move_up)) {
+    out <- re_nest(out)
+  } else {
+    out
+  }
+}
+
+#' Are they neighbours?
+#'
+#' Check whether a token is positioned next to another token in a parse table.
+#' @param pd A parse table.
+#' @param token A vector of tokens to check.
+token_is_adjacent <- function(pd, token) {
+  lead <- lead(pd$token %in% token, default = FALSE)
+  lag  <-  lag(pd$token %in% token, default = FALSE)
+  lead | lag
+}

R/rules.R

@@ -1,5 +1,6 @@
+math_token <- c("'+'", "'-'", "'*'", "'/'", "'^'")
 op_token <- c(
-  "'+'", "'-'", "'*'", "'/'", "'^'", "AND", "AND2", "EQ", "EQ_ASSIGN",
+  math_token, "AND", "AND2", "EQ", "EQ_ASSIGN",
   "GE", "GT", "LE", "LEFT_ASSIGN", "LT", "NE", "OR", "OR2", "RIGHT_ASSIGN",
   "SPECIAL", "EQ_SUB", "ELSE")
 
@@ -21,6 +22,7 @@ set_space_around_op <- function(pd_flat) {
   pd_flat
 }
 
+# depreciated!
 remove_space_after_unary_pm <- function(pd_flat) {
   op_pm <- c("'+'", "'-'")
   op_pm_unary_after <- c(op_pm, op_token, "'('", "','")
@@ -31,6 +33,16 @@ remove_space_after_unary_pm <- function(pd_flat) {
   pd_flat
 }
 
+
+remove_space_after_unary_pm_nested <- function(pd) {
+  if (any(c("'+'", "'-'") %in% pd$token[1])) {
+    pd$spaces[1] <- 0L
+  }
+
+  pd
+}
+
+
 fix_quotes <- function(pd_flat) {
   str_const <- pd_flat$token == "STR_CONST"
   str_const_change <- grepl("^'([^\"]*)'$", pd_flat$text[str_const])

R/serialized_tests.R

@@ -22,7 +22,7 @@
 #' @inheritParams transform_and_check
 #' @importFrom purrr flatten_chr pwalk map
 test_collection <- function(test, sub_test = NULL,
-                            write_back = FALSE,
+                            write_back = TRUE,
                             write_tree = TRUE,
                             transformer,
                             ...) {
@@ -180,3 +180,15 @@ style_indent_curly_round <- function(text) {
 
     serialize_parse_data_nested()
 }
+
+#' @describeIn test_transformer Transformations for indention based on operators
+style_op <- function(text) {
+  text %>%
+    compute_parse_data_nested() %>%
+    re_nest() %>%
+    visit(funs = c(create_filler,
+                   partial(indent_op, indent_by = 2),
+                   strip_eol_spaces)) %>%
+
+    serialize_parse_data_nested()
+}

R/transform.R

@@ -73,6 +73,7 @@ make_transformer_nested <- function(transformers) {
     text <- gsub("\t", "        ", text)
 
     pd_nested <- compute_parse_data_nested(text)
+    pd_nested <- re_nest(pd_nested)
     transformed_pd_nested <- visit(pd_nested, transformers)
     # TODO verify_roundtrip
     new_text <- serialize_parse_data_nested(transformed_pd_nested)