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

Granular scope selection #705

Merged
merged 13 commits into from Jan 2, 2021
Merged

Conversation

lorenzwalthert
Copy link
Collaborator

@lorenzwalthert lorenzwalthert commented Dec 20, 2020

Closes #703.

Context: When use_raw_indention is TRUE (which it is when scope < 'indention' prior to this PR), the number of spaces between tokens on different lines is applied after the line break to restore initial indention. For that reason, spaces between tokens on different lines must not be modified in any case, otherwise scope < 'indention' would not work properly anymore. You can see that pretty much all transformers under R/rules-spacing.R don't modify spaces without checking for newlines == 0L. Indention is tracked separately in the column indention and then applied at the end with choose_indention() if use_raw_indention is FALSE, otherwise, lag_spaces is used.


For some reason not obvious to me at this moment, all indention transformers were grouped under the space transformers in tidyverse_style at the beginning, except update_indention_ref_fun_dec() is listed under indention transformers (probably because it must come after all spacing rules). However, one could try to move all indention rules from the spaces transformers before update_indention_ref_fun_dec() and see if things pass.

Also, these rules live in R/indent.R, and not R/rules-indention.R (which does not exist), where they should be for consistency with other rules like spacing (but R/rules-replacment.R and R/rules-other.R is also not consistent...).

These concerns were addressed in #707.

@codecov-io
Copy link

codecov-io commented Dec 20, 2020

Codecov Report

Merging #705 (90feff2) into master (d5b8b0e) will increase coverage by 0.03%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #705      +/-   ##
==========================================
+ Coverage   90.43%   90.47%   +0.03%     
==========================================
  Files          47       47              
  Lines        2248     2257       +9     
==========================================
+ Hits         2033     2042       +9     
  Misses        215      215              
Impacted Files Coverage Δ
R/transform-files.R 100.00% <ø> (ø)
R/ui-styling.R 100.00% <ø> (ø)
R/style-guides.R 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d5b8b0e...90feff2. Read the comment docs.

Not currently trying to decouple further (moving indention rules before update_indention_ref_fun_dec() in the transformers$indention, where one might think they belong from a conceptupal point of view instead of transformers$spaces as there might be unanticipated dependencies)
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 this pull request may close these issues.

Allow specifying AsIs scope without including lower-level transformations
2 participants