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

standalone files imported from other packages should be ignored in style_pkg() #1134

Closed
olivroy opened this issue Jun 28, 2023 · 2 comments · Fixed by #1138
Closed

standalone files imported from other packages should be ignored in style_pkg() #1134

olivroy opened this issue Jun 28, 2023 · 2 comments · Fixed by #1138

Comments

@olivroy
Copy link
Contributor

olivroy commented Jun 28, 2023

Hi,

I think that standalone files from core tidyverse packages should be ignored from styling.
At the moment, the default values for exclude_files is c("R/RcppExports.R", "R/cpp11.R"), but I think it could be c("R/RcppExports.R", "R/cpp11.R", "R/import-standalone-*")

or a new argument
exclude_import_standalone that would default to TRUE.

If you are not familiar with the new standalone files, here are some references

https://usethis.r-lib.org/reference/use_standalone.html

https://github.com/r-lib/rlang/blob/main/R/standalone-types-check.R

A PR in dplyr where they use it tidyverse/dplyr#6722. They also use it in ggplot2

In a package, if I use

usethis::use_standalone("r-lib/rlang", "types-check")
The file will be copied to `mypkg/R/important-standalone-type-check.R`
# it will be read-only.
# but when I run, `styler::style_pkg()`, it will restyle the file.

I think that because these files appear as read-only in RStudio. If they are to be styled, they should be styled in the original repos (in this case in rlang)

@lorenzwalthert
Copy link
Collaborator

Ok I see. I agree these should probably not be styled by default. A simple way to do that would be to make exclude_files take regex patterns instead of file names. We could adapt the default value such that the behavior is the same as now. Are you interested in making a PR? I can provide guidance.

@olivroy
Copy link
Contributor Author

olivroy commented Jun 28, 2023

Hi, I am not sure I have a lot of time.

I don't mind opening a preliminary version. but first thoughts

  • Allowing regex at large may be dangerous, if someone types excluded_files = ".", no file will be restyled.
  • In packages, files have similar patterns R/file.R, tests/testthat/test-file.R, so this could have interesting side effects to avoid restyling.

Here is my initial work

olivroy@e24dc68

I don't mind opening a PR, but I will not have time to review / readjust it in the coming days.

@olivroy olivroy changed the title standalone files from other packages should be ignored in style_pkg() standalone files imported from other packages should be ignored in style_pkg() Jun 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants