wernerjoss / grav-plugin-ipcount

The IPcount Plugin is for Grav CMS. It counts the visitors on your Website
MIT License
3 stars 1 forks source link

`counter` Twig function name is too global #5

Open hughbris opened 2 years ago

hughbris commented 2 years ago

Just a suggestion for improvement. The Twig function you have implemented uses the name counter. I think this could be a problem for two reasons:

I don't think there is a convention for the Twig prefix format used by Grav plugins, so you could probably simply change the name of your count Twig function to ipcounter() if you want to keep it simple.

Here's a more through but annoying technique I've used in other plugins which might work for you:

public function setUpTwigVariables() {
    $twig = $this->grav['twig'];
    if (!isset($twig->twig_vars['grav']['plugins'])) { // in my opinion, this should be the convention, but it may not exist
        $twig->twig_vars['grav']['plugins'] = [];
    }
    $twig->twig_vars['grav']['plugins'][$this->name] = new IPCounter();
}

By adding just a class instance, you allow yourself to expose any of its public methods in future without having to mess with Twig setup.

In your template, to get the stats counter, you can now use {{ grav.plugins.ipcount.getStats.count }}. You could also add an IP from Twig BTW, since that is a public method.

Whether you choose a simple or complex option for your Twig function name, you'll have to support your existing count() Twig function for a period at least, so that nothing breaks. Maybe consider adding a plugin config option so that developers can switch that Twig name off if it conflicts with something they are using.

And of course all of this is just feedback and suggestions and completely optional! :)

wernerjoss commented 2 years ago

thanks again, as for the other hints. I'll take a closer look and comment later !