-
Notifications
You must be signed in to change notification settings - Fork 51
Replace space with - in tags url #507
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Replace space with - in tags url #507
Conversation
I assume they used a 🦒 because of the inside joke "giraffe-ene OS" instead of "GrapheneOS" 😄 I noticed that in the markdown, the emoji
I don't know enough about Hugo to provide a solution, but maybe the function emojify could be helpful (turn an emoji into text, then turn it back to an emoji when it's displayed). |
@@ -1,6 +1,6 @@ | |||
{{ range $tag := . }} | |||
<span class="tag"> | |||
<!-- urlquery found here: https://discourse.gohugo.io/t/url-encoding-percent-encoding-with-hugo/16546/13 --> | |||
<a href="{{ `/tags/` }}{{ urlquery $tag }}/">{{$tag}}</a> | |||
<a href="{{ `/tags/` }}{{ replace $tag " " "-" | urlquery }}/">{{$tag}}</a> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason why you used urlquery
instead of urlize
? This function seems to replace hyphens with spaces internally, so the syntax is cleaner to read.
https://gohugo.io/functions/urlize/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
urlquery
won in favor of urlize
in #319
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @Balastrong,
First off, thank you for submitting a PR to fix this issue! We really appreciate it 🥳
Secondly, everything looks good to me (LGTM) and I will approve the PR but I wanted us to have a test in place so we can ensure this doesn't happen again. I created a test and submitted a PR against you PR's branch (Balastrong#1).
If you wouldn't mind accepting that I'll happily merge the changes in afterwards 😁
Thank you again for your contribution and I look forward to getting this fixed soon! 🥳 🎉
adding test for checking broken links
Hey @elreydetoda, thank you for the review and for adding the test! I merged your PR now :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, thanks @Balastrong !
LGTM
This PR fixes #497.
Hello everyone! I'm kind of new here but I'm looking for new challenges so here we are :)
I noticed
urlize
has been replaced withurlquery
in #456 (Issue #319).However, with this change the tag
c#
works, but tags with space in between don't (they did withurlize
though).I did some attemps and searches but couldn't find anything 100% working so the idea I had to keep the best of both worlds is to replace the empty space
with
-
first (so that tags with the space are ok) and then keep urlquery for weird chars like#
and+
.Tested with:
c# -> c%23
okc++ -> c%2B%2B
okcsam scanning -> csam-scanning
ok🦒 -> %F0%9F%A6%92
not ok :(The giraffe emoji is escaped just fine, the final url is
/tags/🦒/
which I would say is correct but I can't even manually find the correct tag page. Why would you use an emoji as tag anyway? 😂