Skip to content
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

Add '+' as one of the valid characters in urls specified in the front matter #1966

Closed
wants to merge 1 commit into from

Conversation

egonSchiele
Copy link
Contributor

The UnicodeSanitize function contains a list of valid characters, + should be one of those chars.

@bep
Copy link
Member

bep commented Mar 13, 2016

Why should it be one of the valid characters?

@egonSchiele
Copy link
Contributor Author

It is one of the characters that is valid in urls http://stackoverflow.com/a/1547940

@bep
Copy link
Member

bep commented Mar 13, 2016

Hugo have its own restrictions. Thanks for your report.

@spf13
Copy link
Contributor

spf13 commented Mar 15, 2016

@egonSchiele Not all of the characters in that list are acceptable in commonly used filesystems. Hugo will only permit characters that are both acceptable in all filesystems and valid URL characters. Additionally we avoid characters that may be filesystem acceptable but will interfere with programs (finder, explorer, ls, dir, etc) which are more particular.

We did end up adding the "%" character even though it's an issue on windows due to its extensive use in URLs.

Is there a similarly valid use case for allowing the "+"?

@egonSchiele
Copy link
Contributor Author

I don't know, @spf13 . To be honest, this is not an issue for me...I was merely trying to help by fixing issue #1290. Since it was marked status/please contribute, I assumed y'all did want this change and were merely looking for a PR.

@spf13
Copy link
Contributor

spf13 commented Mar 15, 2016

Aah.. Thanks for providing the background. We appreciate the fix. I did some diligence and I can't find a reason to prohibit this character. It's fine on linux, mac and windows.. even back to Win95.

I'm fine with merging unless @bep has a reason against it.

@bep
Copy link
Member

bep commented Mar 15, 2016

No reason, please merge.

@spf13
Copy link
Contributor

spf13 commented Mar 21, 2016

merged as c42982f

Thanks @egonSchiele. Sorry this one took a while to merge in. Looking forward to further contributions.

@spf13 spf13 closed this Mar 21, 2016
@egonSchiele
Copy link
Contributor Author

Happy to help @spf13 !

@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants