-
Notifications
You must be signed in to change notification settings - Fork 73
Supporting more indention patterns #120
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
Conversation
update the newline attribute according to lag_newlines after line breaks are definitively set.
indent on more operators such as logical, special and equal assignment.
This will create merge conflict with r-lib#119. Merge with strategy theirs.
69f7629
to
22a6619
Compare
Codecov Report
@@ Coverage Diff @@
## master #120 +/- ##
========================================
Coverage ? 88.6%
========================================
Files ? 18
Lines ? 676
Branches ? 0
========================================
Hits ? 599
Misses ? 77
Partials ? 0
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, looks good.
@@ -10,6 +10,7 @@ token <- tribble( | |||
">=" , "logical" , "GE", | |||
"!=" , "logical" , "NE", | |||
"==" , "logical" , "EQ", | |||
"=" , "assign_left" , "EQ_SUB", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the difference between "EQ_SUB"
and "EQ_ASSIGN"
? (Just curious.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
data_frame(a = 3))
# vs.
a = 3
tests/testthat/example/out.R
Outdated
|
||
nested( | ||
function_call(with), | ||
many | ||
many |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks odd.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's fine, because the rule add_space_after_comma()
was not checking for line break (which we need if we want to support raw indention) and this is the flat styling anyways. The token is now where it is in the in.R file.
tests/testthat/example/out.R
Outdated
@@ -130,7 +130,7 @@ test <- function() { | |||
difficult(nested( | |||
"function", call | |||
), | |||
with, more, args | |||
with, more, args |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And this looks like we should force the nested()
call on a new line (#118).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here, this is flat styling. Maybe we should remove tests for it anyways when implementing #121.
R/modify_pd.R
Outdated
#' (R/rules-line_break.R) but we need `newlines` to determine | ||
#' whether or not to set `spaces` (R/rules-spacing.R), we have to update the | ||
#' attribute. We cannot simply use `dplyr::lead(pd$lag_newlines)` since we would | ||
#' loose information for the last token. `spaces` is left as is in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
loose -> lose
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
uups
See tests for examples. This closes #108.