Skip to content

Redesign API #123

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

Merged
merged 8 commits into from
Aug 18, 2017
Merged

Redesign API #123

merged 8 commits into from
Aug 18, 2017

Conversation

krlmlr
Copy link
Member

@krlmlr krlmlr commented Aug 14, 2017

This is a major API change. It makes closes #121 and makes working with top-level stylers much easier, in particuar through enabling the use of ....
For example we change

style_text(text, flat = FALSE, transformers = get_transformers(flat = flat))

To

style_text(text, ..., style = tidyverse_style, transformers = style(...))

All other top-level stylers such as style_dir(), style_pkg() etc. are also adapted to be consistent with the example above. Second argument is always ....

In addition, the function create_style_guide() is introduced, which will help useres to create the input corresponding to the style argument. Since we use tibble::lst(), all transformer functions (and not just the lists containing them) are now named too.

@codecov
Copy link

codecov bot commented Aug 14, 2017

Codecov Report

Merging #123 into master will decrease coverage by 5.03%.
The diff coverage is 77.14%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #123      +/-   ##
==========================================
- Coverage   89.59%   84.56%   -5.04%     
==========================================
  Files          19       19              
  Lines         740      719      -21     
==========================================
- Hits          663      608      -55     
- Misses         77      111      +34
Impacted Files Coverage Δ
R/parsed.R 33.33% <ø> (-64.45%) ⬇️
R/transform.R 84.61% <0%> (-4.48%) ⬇️
R/style_guides.R 97.1% <100%> (ø)
R/ws.R 11.11% <12.5%> (+2.02%) ⬆️
R/utils.R 80% <0%> (-20%) ⬇️
R/serialize.R 23.91% <0%> (-17.4%) ⬇️

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 3bd976b...6bea186. Read the comment docs.

@lorenzwalthert
Copy link
Collaborator

Ok, do you have any specific use case in mind already or is it a matter of principle? I wonder which user-control should then be passed via the transformers argument and what other options should be passed through via .... Some needs to be passed via the transformers since it influences them (like strict).

@krlmlr
Copy link
Member Author

krlmlr commented Aug 14, 2017

I just wanted to write style_pkg(scope = "spaces") . There might be a better design, but this one looked simple enough.

@lorenzwalthert
Copy link
Collaborator

Ok, cool, I did not know that ... can also be used in that context.

@lorenzwalthert lorenzwalthert changed the title Add dots for convenience Redesign API Aug 17, 2017
@lorenzwalthert
Copy link
Collaborator

@krlmlr can you review quickly? Can't assign the PR for review to you since you created it.

- Remove relicts from flat approach
- Rename get_transformers to tidyverse_style
- Pass arguments to transformers via ... by default
Copy link
Member Author

@krlmlr krlmlr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

API Outdated
style_pkg(pkg = ".", flat = FALSE, transformers = get_transformers(flat = flat))
style_text(text, flat = FALSE, transformers = get_transformers(flat = flat))
create_style_guide(filler, line_break, space, token, indention, use_raw_indention = FALSE)
style_dir(path = ".", recursive = TRUE, ..., style = tidyverse_style, transformers = style(...))
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe also move recursive = FALSE past the dots?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds good.

@lorenzwalthert
Copy link
Collaborator

lorenzwalthert commented Aug 18, 2017

Also, I set defaults for all arguments of create_style_guide(), so create_style_guide() gives the empty style guide plus using lst() instead of c() to get named lists in tidyverse_style()

@lorenzwalthert lorenzwalthert merged commit 5a1cbbf into master Aug 18, 2017
krlmlr added a commit that referenced this pull request Aug 24, 2017
- Vignette on customizing styler (#145).
- No line break after `switch()` and friends (#152).
- Remove flat relicts completely (#151).
- Don't reindent function calls and break line correctly for multi-line calls (#149).
- Set space between "=" and "," (#150).
- Make R CMD Check perfect (#148).
- Adding tests for exception handling with invalid parse data (#139).
- Fix indention by checking for all potential triggers (#142).
- Fix un-indention (#135).
- Support wide characters (#130).
- No spaces around :, :: and :::.
- Redesigning the API (#123).
- Solve eq_sub indention in general (#125).
- Minor refactorings.
- Re-indent token-dependent (#119).
- Supporting more indention patterns.
- Allow raw indention.
- Definitively fixing eol issue with comments.
- Infrastructure.
- Flattening out the parse table.
- New rule: no space after ! -> !!! for tidyeval.
- Fix spacing around '{'.
- Don't drop tokens! Fixes #101.
- EOL spaces in empty comments (and in general) (#98).
- mal-indention in conditional statement due to wrong specification of indent_without_paren) (#95).
- Complicated indentions based on arithmetic and special operators (#96).
- indention interaction on with assignment operator and other operators (#97).
@lorenzwalthert lorenzwalthert deleted the f-convenience branch September 17, 2017 21:03
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.

remove relicts of flat styling completely
2 participants