Skip to content
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

return() on newline #492

Merged
merged 7 commits into from Apr 17, 2019

Conversation

@lorenzwalthert
Copy link
Collaborator

commented Apr 14, 2019

This PR makes return() statements going to a new line and hence closes #473. They are already always on a new line with the current implementation of tidyverse_style(), the only exception occurs in conjunction with a conditional statement, as described in #473. For this reason, the rule to be implemented is not a line break rule. Because all line break rules do separate expressions that could also go on one line like {1} or x %>% y() but the requirements for #473 are different. Implementing as a line break rule leads to a lot of headake as described in #473 because it had to be as follows:

  • Implement line break rule for return().
  • Fix edge case in conjunction with if statement. When wrapping an expression in curly braces, check if it contains return() and then remove the line break right at the return token. However, finding return() might be non-trivial because you can also almost always have COMMENT tokens in the parse table.

This results in a lot of code and additional complexity (I tried it). Instead, we modify the behavior of wrap_if_else_multi_line_in_curly() (renamed) to always wrap the if statement in curly braces when it contains return(). In addition, this approach lets us easily wrap for and while loop statement into braces too, using a key_token that indicates for each of the cases (if, for, while loop) which tokens helps identifying the expression to wrap into curly braces (ignoring comments). Apart from this, this PR is unfortunately convoluted with some clean-up, documentation improvement and re-location of function declarations between files. Hence, this PR closes #286.

This PR also includes the renaming of wrap_if_else_while_for_multi_line_in_curly) in the tidyverse style guide, which some people might rely on because it is the output of an exported function.

@lorenzwalthert lorenzwalthert force-pushed the lorenzwalthert:return-on-newline branch from fc55c70 to 82b2c66 Apr 14, 2019

@lorenzwalthert lorenzwalthert force-pushed the lorenzwalthert:return-on-newline branch from 82b2c66 to 8581ca4 Apr 14, 2019

@lorenzwalthert lorenzwalthert merged commit 6f1ebbe into r-lib:master Apr 17, 2019

0 of 2 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build failed
Details

@lorenzwalthert lorenzwalthert deleted the lorenzwalthert:return-on-newline branch Apr 17, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.