getgrav / grav-plugin-sitemap

Grav Sitemap Plugin
https://getgrav.org
MIT License
42 stars 42 forks source link

Added ability to add non-grav URL to sitemap #35

Closed Chouchen closed 7 years ago

Chouchen commented 7 years ago

I use Grav on a main domain which have 2 or 3 "non grav" URL which are important.

I needed the possibility to add those URL into the sitemap.

flaviocopes commented 7 years ago

👍

lulis commented 7 years ago

Nice improvement.

I think you should comment the lines on sitemap.yaml, otherwise this sample entry will be displayed on sites using old custom config files (my case).

Will be nice document this feature on README.md also ;)

Thanks!

Chouchen commented 7 years ago

Well, you should have your sitemap.yaml in your user/config/plugins folder so it shouldn't be added to your sitemap (if I understand Grav implementation the good way).

You're right about the README, I should make another PR for that :ambulance:

lulis commented 7 years ago

Yes, @Chouchen, I have my custom user/config/plugins/sitemap.yaml ;)

But what grav does is merge the custom settings with the default settings (maybe the plugin is implemented in that way). And if someone just update grav plugins, new parameters will silently "fallback" to this new default values.

See, it happened to me. Testing the latest development version, and inspecting my sitemap I've seen this strange entry. So I searched and found this new setting. I even can't disable it on my custom file (maybe not tried too much), so I removed the entry on original settings.yaml. That's why I'm reporting here ;)

As I see, its not a good practice mess with old (and working) installs silently. Anyway, I guess that plugins should work well "out of the box", not inserting sample entries on a fresh install.

Just my 2 cents ;)

Thanks!

flaviocopes commented 7 years ago

You're right @lulis, going to fix and also add a blueprint option to make it configurable via Admin

flaviocopes commented 7 years ago

Done in https://github.com/getgrav/grav-plugin-sitemap/commit/05b88f3701c40b062824ac2f326627267e697164

Chouchen commented 7 years ago

Nice :) And I didn't know it'd work that way, thanks @lulis ( and @flaviocopes for the update )