Don't use . to separate function names

[hadley]

It's probably too late to fix this, but in R, it's conventional to use . only for S3 method names.

[yixuan]

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.

[hadley]

Yeah, I think that would be the best approach.

[DaveParr]

I see you've implemented the best practice my_fun(), and also have a useful message to that effect: 'showtext.begin()' is now renamed to 'showtext_begin()' The old version still works, but consider using the new function in future code.

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 fig.showtext = TRUE, and showtext_auto() in my script, and receiving that warning. Are lines such as showtext::showtext.begin triggering your own warnings?

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")
        }
    }
}

[yixuan]

Good catch. In fact I have updated the code on CRAN, but I forgot to update github. The real reason is that in knitr I still call showtext.begin() instead of showtext_begin(). I'll create a patch to knitr later this week.

Also @yihui.

[yihui]

I have changed it in knitr a while ago (the CRAN version of knitr should be using showtext_begin()):
https://github.com/yihui/knitr/blob/4d71624d8ab9cac51a2245dc97d7c54b5c29eb68/R/plot.R#L317

[yixuan]

@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