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

New feature: style_book function #476

Closed
wants to merge 1 commit into from
Closed

New feature: style_book function #476

wants to merge 1 commit into from

Conversation

jcrodriguez1989
Copy link

While I was trying to style the The R in Spark bookdown project from within RStudio, I noticed there was not a straightforward way to do it.
As the styler package has the useful style_pkg function, I thought it would be equally useful to have a style_book function.
Note: If PR accepted, I will work on some tests.

Some output:
Trying to use style_pkg on the bookdown project:

style_pkg(filetype = c(".R", ".Rmd", ".Rnw"))
## ────────────────────────────────────────
## Status	Count	Legend 
## ✔ 	0	File unchanged.
## ℹ 	0	File changed.
## ✖ 	0	Styling threw an error.
## ────────────────────────────────────────

Using style_book:

style_book()
## Styling  19  files:
##  ./analysis.Rmd                               ℹ 
##  ./appendix.Rmd                               ℹ 
##  ./clusters.Rmd                               ℹ 
##  ./connections.Rmd                            ⚠ 
##  ./contributing.Rmd                           ℹ 
##  ./data.Rmd                                   ⚠ 
##  ./distributed-r.Rmd                          ℹ 
##  ./excersises/01-introduction-walkthrough.Rmd ℹ 
##  ./extensions.Rmd                             ℹ 
##  ./index.Rmd                                  ✔ 
##  ./intro.Rmd                                  ℹ 
##  ./modeling.Rmd                               ℹ 
##  ./preface.Rmd                                ℹ 
##  ./references.Rmd                             ℹ 
##  ./slides/slides.Rmd                          ℹ 
##  ./starting.Rmd                               ℹ 
##  ./streaming.Rmd                              ℹ 
##  ./styler_feature.Rmd                         ✔ 
##  ./tuning.Rmd                                 ℹ 
## ────────────────────────────────────────────
## Status	Count	Legend 
## ✔ 	2	File unchanged.
## ℹ 	15	File changed.
## ✖ 	2	Styling threw an error.
## ────────────────────────────────────────────
## Please review the changes carefully!
style_book(filetype = c(".R", ".Rmd", ".Rnw"))
## Styling  25  files:
##  ./analysis.Rmd                               ✔ 
##  ./appendix.Rmd                               ✔ 
##  ./build.R                                    ℹ 
##  ./clusters.Rmd                               ✔ 
##  ./connections.Rmd                            ⚠ 
##  ./contributing.Rmd                           ✔ 
##  ./data.Rmd                                   ⚠ 
##  ./distributed-r.Rmd                          ✔ 
##  ./excersises/01-introduction-walkthrough.Rmd ✔ 
##  ./extensions.Rmd                             ✔ 
##  ./index.Rmd                                  ✔ 
##  ./intro.Rmd                                  ✔ 
##  ./modeling.Rmd                               ✔ 
##  ./preface.Rmd                                ✔ 
##  ./r/ascii.R                                  ✔ 
##  ./r/plots.R                                  ℹ 
##  ./r/render.R                                 ℹ 
##  ./r/utils.R                                  ℹ 
##  ./r/webshots.R                               ℹ 
##  ./references.Rmd                             ✔ 
##  ./slides/slides.Rmd                          ✔ 
##  ./starting.Rmd                               ✔ 
##  ./streaming.Rmd                              ✔ 
##  ./styler_feature.Rmd                         ✔ 
##  ./tuning.Rmd                                 ✔ 
## ────────────────────────────────────────────
## Status	Count	Legend 
## ✔ 	18	File unchanged.
## ℹ 	5	File changed.
## ✖ 	2	Styling threw an error.
## ────────────────────────────────────────────
## Please review the changes carefully!

@codecov-io
Copy link

codecov-io commented Mar 8, 2019

Codecov Report

Merging #476 into master will decrease coverage by 0.59%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #476     +/-   ##
=========================================
- Coverage   89.77%   89.17%   -0.6%     
=========================================
  Files          36       36             
  Lines        1643     1654     +11     
=========================================
  Hits         1475     1475             
- Misses        168      179     +11
Impacted Files Coverage Δ
R/ui.R 81.66% <0%> (-18.34%) ⬇️

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 58bdace...42d5805. Read the comment docs.

@lorenzwalthert
Copy link
Collaborator

Hi @jcrodriguez1989. Why not simply use style_dir(file_type = "rmd")? I don't think we should complicate the API with another function that has basically 100% overlap with existing functionality. Also, I don't see how this is bookdown specific. Can you elaborate on that? Or what am I missing?

@jcrodriguez1989
Copy link
Author

Hi @lorenzwalthert. It is not bookdown specific, it is meant to be rmarkdown derivatives specific.
I found it useful to have this function available. It is as useful as using style_pkg, instead of simply using style_dir(file_type = "r") with some path exclusions.

It's just a suggestion 😄

@lorenzwalthert
Copy link
Collaborator

I understand that it would be nicer if you could call a function without specifying an argument for the file type. Unfortunately, the API decision was to not style .Rmd by default. This was probably a mistake. Maybe we will change this in a later release. Also, note that there are other issues such as #319 where we try to better understand configuring default values via yaml files. These would also apply to the Addins (not implemented in the solution mentioned in #319). You could install the experimental branch lorenzwalthert/styler@config (it is not guaranteed that we keep it up to date with r-lib/styler) and then change the default arguments in the config file. Feedback welcomed. But as I said, your proposed API change overlaps with existing functionality and is not bookdown specific so I hope you understand that I don't think we should merge this with master.

@jcrodriguez1989
Copy link
Author

@lorenzwalthert I absolutely understand your determination.
It is good to see how much you are working on the package.
Thank you for contributing to the open source and R communities 🙂.

@lorenzwalthert
Copy link
Collaborator

Thanks. Also, if you have questions related to #319, you can ask me here, I'll try to help.

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.

None yet

3 participants