PontusHorn / Pico-Tags

A plugin for Pico to add tagging functionality.
MIT License
25 stars 9 forks source link

Tags all #7

Closed tanghus closed 4 years ago

tanghus commented 4 years ago

Hejsa Pontus

I extended the plugin with a tags_all() function, which gives all tags used in the site. There's a small snippet in README.md showing how.

I hope you can use it :)

NOTE:

tanghus commented 4 years ago

I just noticed that this is probably what #4 is about?

PontusHorn commented 4 years ago

Hey Thomas! Nice addition, looks useful 🙂 I have some minor suggestions that I'll add in a bit.

PontusHorn commented 4 years ago

Looks great now 😄 I'll run a quick test with it tomorrow or sometime this week and merge if it it all works. Thanks for the additions!

tanghus commented 4 years ago

I had an itch to scratch :)

PontusHorn commented 4 years ago

Hey again, sorry for taking a while!

I ran into some problems when trying to use the updated plugin on a test site, which seems to have to do with the plugin API version. This plugin is a bit outdated in which methods it uses, but still works thanks to the magical PicoDeprecated plugin that comes with Pico :) But the onPageRendering probably wasn't available in the old plugin API, so doesn't get called.

One quick fix I found was to use the old onTwigRegistration method instead:

    /**
     * Register Twig's get_all_tags.
     */
    public function onTwigRegistration()
    {
        $this->getPico()->getTwig()->addFunction(new Twig_SimpleFunction('get_all_tags', array($this, 'getAllTags')));
    }

Maybe change to that for now? Another option would be to update the whole plugin to use the new API, but I think that's better handled separately as there are some difficulties with that.

tanghus commented 4 years ago

Hey again, sorry for taking a while!

I ran into some problems when trying to use the updated plugin on a test site, which seems to have to do with the plugin API version. This plugin is a bit outdated in which methods it uses, but still works thanks to the magical PicoDeprecated plugin that comes with Pico :) But the onPageRendering probably wasn't available in the old plugin API, so doesn't get called.

One quick fix I found was to use the old onTwigRegistration method instead:

    /**
     * Register Twig's get_all_tags.
     */
    public function onTwigRegistration()
    {
        $this->getPico()->getTwig()->addFunction(new Twig_SimpleFunction('get_all_tags', array($this, 'getAllTags')));
    }

Maybe change to that for now?

Sure. I didn't know where to place it, so I just copied what was used in the http-params plugin.

Another option would be to update the whole plugin to use the new API, but I think that's better handled separately as there are some difficulties with that.

Agreed.

Since you got it working, won't you just push the changes from your local repo? I'm in the middle of some other project atm, so :)

PontusHorn commented 4 years ago

Alright, I made some changes and will merge it now. Thanks a lot again for the additions!

tanghus commented 4 years ago

Thanks for accepting them. It's not entirely impossible, that I may have some more changes later. I guess that's not saying too much :grinning:

I was thinking maybe a method for getting all documents with a specific tag, to avoid having to traverse the pages in the templates. But I don't know when/if that will be, though.