Skip to content

Indent indices for token "EQ_SUB" are different. #126

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

Closed
lorenzwalthert opened this issue Aug 14, 2017 · 3 comments · Fixed by #127
Closed

Indent indices for token "EQ_SUB" are different. #126

lorenzwalthert opened this issue Aug 14, 2017 · 3 comments · Fixed by #127

Comments

@lorenzwalthert
Copy link
Collaborator

The problem with the token "EQ_SUB" mentioned here arises because of the fact that expression "a = 3" and "b = 44" are not nested one level deeper, i.e. they are all direct sub-expressions of the root token:

styler:::create_tree("data_frame(a = 3, b = 44)")
#>                                              levelName
#> 1  ROOT (token: short_text [lag_newlines/spaces] {id})
#> 2   °--expr:  [0/0] {18}                              
#> 3       ¦--expr:  [0/0] {3}                           
#> 4       ¦   °--SYMBOL_FUNCTION_CALL: data_ [0/0] {1}  
#> 5       ¦--'(': ( [0/0] {2}                           
#> 6       ¦--SYMBOL_SUB: a [0/1] {4}                    
#> 7       ¦--EQ_SUB: = [0/1] {5}                        
#> 8       ¦--expr:  [0/0] {7}                           
#> 9       ¦   °--NUM_CONST: 3 [0/0] {6}                 
#> 10      ¦--',': , [0/1] {8}                           
#> 11      ¦--SYMBOL_SUB: b [0/1] {11}                   
#> 12      ¦--EQ_SUB: = [0/1] {12}                       
#> 13      ¦--expr:  [0/0] {14}                          
#> 14      ¦   °--NUM_CONST: 44 [0/0] {13}               
#> 15      °--')': ) [0/0] {15}

We used compute_indent_indices() so far to compute the indices that need indention. However, with all other operators such as braces or + %>%, the indention is always from the token on until the last or the penultimate token in that parse table. Here, it is only the next token, so we could go about it as follows:

indent_eq_sub <- function(pd,
                          indent_by,
                          token = "EQ_SUB") {
  eq_sub <- which(pd$token == "EQ_SUB")
  if (length(eq_sub) == 0) return(pd)
  has_line_break <- which(pd$lag_newlines > 0)
  indent_indices <- intersect(eq_sub + 1, has_line_break)
  pd$indent[indent_indices] <- pd$indent[indent_indices] + indent_by
  pd
}

Alternatively, we had to modify compute_indent_indices(), which has already an argument indent_last. We could make that argument non-boolean and change it to indent_up_to and allow values "next", "penulatimate", "last".

@krlmlr
Copy link
Member

krlmlr commented Aug 14, 2017

Do we treat EQ_SUB as operator by any chance? In that case, one option would be simply not to treat it as operator but apply a different rule.

@lorenzwalthert
Copy link
Collaborator Author

No, it's not treated as an operator in the sense that it is not indented with the other operators such as + with indent_op(). We have the function indent_eq_sub() but it does not account for the structure described above properly, so I suggested to modify it.

@krlmlr
Copy link
Member

krlmlr commented Aug 14, 2017

Modifying indent_eq_sub() sounds good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants