search

wpilibsuite / sphinxext-opengraph

Sphinx extension to generate unique OpenGraph metadata
https://sphinxext-opengraph.readthedocs.io
Other
74 stars 27 forks source link

Use Canonical URL for ogp_site_url #55

Open ItayZiv opened 2 years ago

ItayZiv commented 2 years ago

Getting og:url automatically in RTD using html_baseurl doesn't work. We probably want to use html_baseurl when ogp_sire_url isn't set regardless of if we are in an RTD environment. It might be better to recommend users to set html_baseurl and only set ogp_site_url if they need it to be different than the normal canonical URL.

TheTripleV commented 2 years ago

Why doesn't it work on RTD?

ItayZiv commented 2 years ago

If you look at the og tags of the website of this extension you will see the website tag is just the page name. I'm quite sure this is because you can't modify the config object. My bad I missed something, still need to check why this happens tho. I want to use this opportunity to use html_baseurl even not in RTD.'

Edit: Basically I think that we should have it set up so that html_baseurl is used unless ogp_site_url is set, if it is set use ogp_site_url. If both aren't configured, raise an exception.

ItayZiv commented 2 years ago

It seems that when I check the PR docs build (and the PR build on other PRs) it actually seems to get the correct URL (https://sphinxext-opengraph.readthedocs.io/en/latest/index.html). Regardless I'd still like to do the aforementioned html_baseurl change, but not really sure why the latest docs don't work.

ItayZiv commented 2 years ago

Actually, it seems like these are two separate matters, the docs links to https://sphinxext-opengraph.readthedocs.io/en/latest/ as the default, yet the latest branch is not updated, while https://sphinxext-opengraph.readthedocs.io/en/stable is updated and also has og:url linked to https://sphinxext-opengraph.readthedocs.io/en/latest/index.html. I split this into #57, while this issue can remain for changing the extension to use html_baseurl outside of RTD (and refractoring the way this is currently done)