Code in braces added to "if" not indented properly

[krlmlr]

styler::style_text("if (TRUE)\na")
#> if (TRUE) {
#>   a
#> }
styler::style_text("if (TRUE)\na <- b")
#> if (TRUE) {
#>   a <- b
#>   }

styler::style_text("if (TRUE)\ncall(\narg\n)")
#> if (TRUE) {
#>   call(
#>   arg
#>   )
#> }

Maybe we need to add braces before doing other transformations?

[lorenzwalthert]

I think the problem is that + (and other tokens) causes indention for all subsequent tokens in the nest. This was fine until now because braces were never on the same nest as those indention-causing operators.

reprex::reprex_info()
#> Created by the reprex package v0.1.1.9000 on 2017-10-25

styler:::create_tree("
if (x >= 52) {
  2 +1 \n } else {\n 5
}")
#>                                              levelName
#> 1  ROOT (token: short_text [lag_newlines/spaces] {id})
#> 2   °--expr:  [0/0] {41}                              
#> 3       ¦--IF: if [0/1] {3}                           
#> 4       ¦--'(': ( [0/0] {4}                           
#> 5       ¦--expr:  [0/0] {11}                          
#> 6       ¦   ¦--expr:  [0/1] {7}                       
#> 7       ¦   ¦   °--SYMBOL: x [0/0] {5}                
#> 8       ¦   ¦--GE: >= [0/1] {6}                       
#> 9       ¦   °--expr:  [0/0] {9}                       
#> 10      ¦       °--NUM_CONST: 52 [0/0] {8}            
#> 11      ¦--')': ) [0/1] {10}                          
#> 12      ¦--expr:  [0/1] {26}                          
#> 13      ¦   ¦--'{': { [0/2] {13}                      
#> 14      ¦   ¦--expr:  [1/1] {21}                      
#> 15      ¦   ¦   ¦--expr:  [0/1] {16}                  
#> 16      ¦   ¦   ¦   °--NUM_CONST: 2 [0/0] {15}        
#> 17      ¦   ¦   ¦--'+': + [0/0] {17}                  
#> 18      ¦   ¦   °--expr:  [0/0] {19}                  
#> 19      ¦   ¦       °--NUM_CONST: 1 [0/0] {18}        
#> 20      ¦   °--'}': } [1/0] {24}                      
#> 21      ¦--ELSE: else [0/1] {27}                      
#> 22      °--expr:  [0/0] {38}                          
#> 23          ¦--'{': { [0/1] {29}                      
#> 24          ¦--expr:  [1/0] {32}                      
#> 25          ¦   °--NUM_CONST: 5 [0/0] {31}            
#> 26          °--'}': } [1/0] {36}

However, when I added the brace insertion feature, I did not think of that and since it is also the easiest to implement, I just wrapped the children of expression id 20 below in braces, so the tree has no intermediate child.

reprex::reprex_info()
#> Created by the reprex package v0.1.1.9000 on 2017-10-25

styler:::create_tree("
if (x >= 52) 
  2 +1 else \n 5
")
#>                                              levelName
#> 1  ROOT (token: short_text [lag_newlines/spaces] {id})
#> 2   °--expr:  [0/0] {27}                              
#> 3       ¦--IF: if [0/1] {3}                           
#> 4       ¦--'(': ( [0/0] {4}                           
#> 5       ¦--expr:  [0/0] {11}                          
#> 6       ¦   ¦--expr:  [0/1] {7}                       
#> 7       ¦   ¦   °--SYMBOL: x [0/0] {5}                
#> 8       ¦   ¦--GE: >= [0/1] {6}                       
#> 9       ¦   °--expr:  [0/0] {9}                       
#> 10      ¦       °--NUM_CONST: 52 [0/0] {8}            
#> 11      ¦--')': ) [0/2] {10}                          
#> 12      ¦--expr:  [1/1] {20}                          
#> 13      ¦   ¦--expr:  [0/1] {15}                      
#> 14      ¦   ¦   °--NUM_CONST: 2 [0/0] {14}            
#> 15      ¦   ¦--'+': + [0/0] {16}                      
#> 16      ¦   °--expr:  [0/0] {18}                      
#> 17      ¦       °--NUM_CONST: 1 [0/0] {17}            
#> 18      ¦--ELSE: else [0/1] {19}                      
#> 19      °--expr:  [1/0] {24}                          
#> 20          °--NUM_CONST: 5 [0/0] {23}

The tree then looks like this

#>                                              levelName
#> 1  ROOT (token: short_text [lag_newlines/spaces] {id})
#> 2   °--expr:  [0/0] {27}                              
#> 3       ¦--IF: if [0/1] {3}                           
#> 4       ¦--'(': ( [0/0] {4}                           
#> 5       ¦--expr:  [0/0] {11}                          
#> 6       ¦   ¦--expr:  [0/1] {7}                       
#> 7       ¦   ¦   °--SYMBOL: x [0/0] {5}                
#> 8       ¦   ¦--GE: >= [0/1] {6}                       
#> 9       ¦   °--expr:  [0/0] {9}                       
#> 10      ¦       °--NUM_CONST: 52 [0/0] {8}            
#> 11      ¦--')': ) [0/2] {10}                          
#> 12      ¦--expr:  [1/1] {20}
#>         ¦   ¦--'{': ...                      
#> 13      ¦   ¦--expr:  [0/1] {15}                      
#> 14      ¦   ¦   °--NUM_CONST: 2 [0/0] {14}            
#> 15      ¦   ¦--'+': + [0/0] {16}                      
#> 16      ¦   °--expr:  [0/0] {18}                      
#> 17      ¦       °--NUM_CONST: 1 [0/0] {17}            
#>         ¦   ¦--'{': ...                      
#> 18      ¦--ELSE: else [0/1] {19}                      
#> 19      °--expr:  [1/0] {24}                          
#> 20          °--NUM_CONST: 5 [0/0] {23}

Bottom line: Probably need to adapt wrap_expr_in_curly() to not just wrap an nest, but essentially putting the nest into exp and wrapping expr in {, }. Maybe we can create these at the same level and then do something opposite of bind_with_child(). Involves assigning of another pos_id and needs to be done carefully.

[krlmlr]

I think I get the general idea. Do we still need the original pos_id with the nested parse data? In the flat data it defines the parent-child relationship, but in the nested parse data?

[lorenzwalthert]

Closed with #263.