Fix - Url encoding of version numbers#31
Fix - Url encoding of version numbers#31mad-briller wants to merge 1 commit intocode-lts:mainfrom mad-briller:fix.url-encoding
Conversation
| . 'with API information:' . "\n" | ||
| . "\n" | ||
| . ' <info>php ./vendor/bin/phpunit parse config/symfony.php</info>' . "\n" | ||
| . ' <info>php vendor/bin/phpunit parse config/symfony.php</info>' . "\n" |
There was a problem hiding this comment.
Not sure why you changed that :)
There was a problem hiding this comment.
there are the tests i mentioned were failing when i cloned the package, not sure whether you preferred to have the ./ or not so i just updated the tests to pass according to what was expected
There was a problem hiding this comment.
Well as you can see it fails our CI with the change you made
https://github.com/code-lts/doctum/pull/31/checks?check_run_id=3388964589#step:7:75
I am not really sure why this change is made, probably because you did not use ./vendor/bin/phpunit as a launch command or use the composer run phpunit command :)
There was a problem hiding this comment.
ahh yeah, i used php vendor/bin/phpunit out of habit
my bad on that one sorry
| <select class="form-control" id="version-switcher" name="version"> | ||
| {% for version in project.versions %} | ||
| <option value="{{ path('../' ~ version ~ '/index.html') }}" data-version="{{ version }}">{{ version.longname }}</option> | ||
| <option value="{{ path('../' ~ version.urlEncoded ~ '/index.html') }}" data-version="{{ version }}">{{ version.longname }}</option> |
There was a problem hiding this comment.
Thank you d'or the patch, I will try to find a way to escape this using twig
There was a problem hiding this comment.
thank you, i'm not precious about the patch being merged, only that it works for our use case
thanks for your amazing utility, really impressed with how quick and simple it was to use!
There was a problem hiding this comment.
Thank you !
This tool was previously named Sami but I really improved it since I took back maintenance in 2020.
So some credit for the amazingness goes to the original author ;)
williamdes
changed the title
|
By the way your GitHub seems not to know your email Maybe this is a git misconfiguration, just wanted to let you know ;) |
| <select class="form-control" id="version-switcher" name="version"> | ||
| {% for version in project.versions %} | ||
| <option value="{{ path('../' ~ version ~ '/index.html') }}" data-version="{{ version }}">{{ version.longname }}</option> | ||
| <option value="{{ path('../' ~ version.urlEncoded ~ '/index.html') }}" data-version="{{ version }}">{{ version.longname }}</option> |
There was a problem hiding this comment.
| <option value="{{ path('../' ~ version.urlEncoded ~ '/index.html') }}" data-version="{{ version }}">{{ version.longname }}</option> | |
| <option value="{{ path('../' ~ version|url_encode ~ '/index.html') }}" data-version="{{ version }}">{{ version.longname }}</option> |
https://twig.symfony.com/doc/3.x/filters/url_encode.html
Could you apply this and revert the other changes please ?
woops, i made the change on my work pc and not my personal, good spot there! Thanks for your input on the I'll recreate a PR with the correct changeset, email and sign the commits and post it that way instead of modifying this one. |
|
Duplicate of #32 |
|
Duplicate of #32 |
|
I missed that you closed it, normally you do not need to do that, a simple force push will do the trick |
2 participants
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.