search

code-lts / doctum

A php API documentation generator, fork of Sami
https://doctum.long-term.support/
MIT License
300 stars 32 forks source link

Fix - Url encoding of version numbers #31

Closed mad-briller closed 3 years ago

mad-briller commented 3 years ago

Semver supports adding build information to version numbers using +; 2.0.0+1.

Within urls, + must be encoded as %2B, as + is interpreted as a space character.

Doctum assumes that version numbers will be safe to use in urls without encoding, as such using build information in a version causes the version-switcher to route to an incorrect url.

This PR aims to fix that (forgive me for i am new to twig). It also fixes a few tests that were not passing when i cloned the repo and ran them.

Let me know how you feel about this change, thanks.

williamdes commented 3 years ago

By the way your GitHub seems not to know your email From: Brad Miller <brad@hallnet.co.uk> (https://github.com/code-lts/doctum/pull/31/commits/cde55bc86de21056b107ebdfa2bb4626123febcf.patch)

Maybe this is a git misconfiguration, just wanted to let you know ;)

mad-briller commented 3 years ago

By the way your GitHub seems not to know your email From: Brad Miller <brad@hallnet.co.uk> (https://github.com/code-lts/doctum/pull/31/commits/cde55bc86de21056b107ebdfa2bb4626123febcf.patch)

Maybe this is a git misconfiguration, just wanted to let you know ;)

woops, i made the change on my work pc and not my personal, good spot there!

Thanks for your input on the url_encode filter; i saw that in the docs but i wasn't sure it could be used when string concatenating like that.

I'll recreate a PR with the correct changeset, email and sign the commits and post it that way instead of modifying this one.

williamdes commented 3 years ago

Duplicate of #32

williamdes commented 3 years ago

Duplicate of #32

williamdes commented 3 years ago

I missed that you closed it, normally you do not need to do that, a simple force push will do the trick Marked it properly as duplicating the right one