-
Notifications
You must be signed in to change notification settings - Fork 38
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
Don't use . to separate function names #2
Comments
Hmm, that's what I really regret. Any suggestion to fix this but not to break the API? The only thing I can think of is to add underscore aliases and deprecate the dot ones. |
Yeah, I think that would be the best approach. |
I see you've implemented the best practice However, I think that you are also triggering your own error message by not adopting this practice in your code behind the scenes. I am only using showtext.auto = function(enable = TRUE)
{
enable = as.logical(enable)
has_hook = length(getHook("before.plot.new")) > 0
is_showtext_hook = sapply(getHook("before.plot.new"), identical,
y = showtext::showtext.begin)
has_hook_grid = length(getHook("grid.newpage")) > 0
is_showtext_hook_grid = sapply(getHook("grid.newpage"), identical,
y = showtext::showtext.begin)
already_hooked = has_hook && any(is_showtext_hook)
already_hooked_grid = has_hook_grid && any(is_showtext_hook_grid)
if(enable)
{
if(!already_hooked)
setHook("before.plot.new", showtext::showtext.begin)
if(!already_hooked_grid)
setHook("grid.newpage", showtext::showtext.begin)
} else {
if(already_hooked)
{
old_hooks = getHook("before.plot.new")
new_hooks = old_hooks[!is_showtext_hook]
setHook("before.plot.new", new_hooks, "replace")
}
if(already_hooked_grid)
{
old_hooks = getHook("grid.newpage")
new_hooks = old_hooks[!is_showtext_hook_grid]
setHook("grid.newpage", new_hooks, "replace")
}
}
} |
Good catch. In fact I have updated the code on CRAN, but I forgot to update github. The real reason is that in Also @yihui. |
I have changed it in knitr a while ago (the CRAN version of knitr should be using |
@yihui I see. I didn't check the code carefully. So there should be no code calling the old functions. Could you double check your issue? @DaveRGP |
It's probably too late to fix this, but in R, it's conventional to use
.
only for S3 method names.The text was updated successfully, but these errors were encountered: