jdillard / sphinx-sitemap

Sphinx extension to generate a multi-lingual, multi-version sitemap for HTML builds
https://sphinx-sitemap.readthedocs.io/en/latest/index.html
MIT License
55 stars 22 forks source link

[IMP] allow custom URL scheme for internationalized pages #23

Closed mart-e closed 4 years ago

mart-e commented 4 years ago

Allowing to use a custom i18n_url_scheme parameter. Before this commit, the URLs were forced to use the format <site>/<lang>/<version>/<link> which is the format used by RTD but some may use a different one.

Fixes jdillard/sphinx-sitemap#22

mart-e commented 4 years ago

@jdillard there you go. This needs to be triple checked for the leading/trailing /. For instance, I am not sure about

https://github.com/jdillard/sphinx-sitemap/blob/02bbe94613ca1164a0f73865659d240fadb3f808/sphinx_sitemap/__init__.py#L95-L98 meaning the version will always ends with a / ?

Should the site_url also always end with a slash?

edit: added some sanity check in the second commit

I would also like the opinion of my colleague @xmo-odoo before merging this to see if it integrates well with our deployment at odoo/documentation-user#499

xmo-odoo commented 4 years ago

Seems fine though the handling of trailing slashes seems like a bit of a mess. Maybe urllib.parse.urljoin could help there? It should handle trailing / fine, though it requires components to not have leading / as that's an "absolute" link:

>>> urljoin('http://foo.com/bar/baz/', 'qux/')
'http://foo.com/bar/baz/qux/'
>>> urljoin('http://foo.com/bar/baz/', '/qux/')
'http://foo.com/qux/'
jdillard commented 4 years ago

I don't know what was up with the handling of version you linked to. I changed how it was handled in https://github.com/jdillard/sphinx-sitemap/pull/23/commits/d61102781f6cc7be8e55405bacc7d2e9f376106c.

This defaults the scheme to "{lang}/{version}/{link}" where {lang} is defaulted to "en" (I didn't want to default to a specific language, but it was easiest) and {version} is defaulted to "latest". If the user doesn't want any of those defaults they can change them, such as setting the scheme to "{version}/{link}" or even just "{link}". Does that change make sense?