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

Load Rd macros if checking package #44

Merged
merged 4 commits into from
Oct 8, 2019
Merged

Load Rd macros if checking package #44

merged 4 commits into from
Oct 8, 2019

Conversation

gaborcsardi
Copy link
Contributor

Closes #42

@jeroen
Copy link
Member

jeroen commented Oct 8, 2019

I think we need a default value for the macros parameter in spell_check_file_rd because now it doesn't work at all if we don't specify macros :

> spell_check_files("man/spell_check_files.Rd")
# Error in tools::RdTextFilter(rdfile, macros = macros) : 
#  argument "macros" is missing, with no default 

By default it loads R's system macros.
@codecov-io
Copy link

codecov-io commented Oct 8, 2019

Codecov Report

Merging #44 into master will decrease coverage by 0.88%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #44      +/-   ##
==========================================
- Coverage   45.11%   44.22%   -0.89%     
==========================================
  Files           7        7              
  Lines         317      303      -14     
==========================================
- Hits          143      134       -9     
+ Misses        174      169       -5
Impacted Files Coverage Δ
R/spell-check.R 30.7% <0%> (-0.12%) ⬇️
R/check-files.R 63.51% <0%> (+0.35%) ⬆️
R/rmarkdown.R 33.33% <0%> (-10.67%) ⬇️
R/parse-markdown.R 83.33% <0%> (-0.88%) ⬇️
R/remove-chunks.R 85.71% <0%> (-0.65%) ⬇️

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 569a24f...befbd78. Read the comment docs.

@gaborcsardi
Copy link
Contributor Author

I changed it to use the RdTextFilter default if macros is not specified.

@jeroen
Copy link
Member

jeroen commented Oct 8, 2019

Thanks. Is the ... in spell_check_file_one() really needed you think? I guess most users will just use spell_check_package() for Rd files?

I think it's a bit confusing to have undocumented parameters in ... that only apply to a specific format (rd in this case). It it's not essential I'm going to take that out.

Looks good otherwise I think.

@gaborcsardi
Copy link
Contributor Author

I don't think it is essential, but I don't know when spell_check_file_one() is called.

@jeroen
Copy link
Member

jeroen commented Oct 8, 2019

OK I'll take it out then. It's only used for spell_check_files(), i.e. manually spelling checking a single file that is not part of a pkg.

@gaborcsardi
Copy link
Contributor Author

I took it out.

@jeroen
Copy link
Member

jeroen commented Oct 8, 2019

OK looks good now thanks!!

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.

Warnings when spell-checking a package with Rd macros
3 participants