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

Case sensitivity #59

Closed
kavolorn opened this issue Feb 5, 2016 · 8 comments
Closed

Case sensitivity #59

kavolorn opened this issue Feb 5, 2016 · 8 comments

Comments

@kavolorn
Copy link

kavolorn commented Feb 5, 2016

Hi there,

Could you please write why you've decided to lowercase language codes? Is not it more consistent to keep ISO 639-1 codes where last part is in uppercase? Is it possible to make an option which will behave as it was before?

I've just noticed many broken links after update (because some parts of code cannot follow redirects).

Appreciate your work!

@mikehaertl
Copy link
Collaborator

Hmm, I think this was mainly my personal preference for all lowercase URLs. But I guess you have a valid point here. Can you check if my commit above helps you? It adds an option keepUpperCaseLanguageCode.

@kavolorn
Copy link
Author

kavolorn commented Feb 5, 2016

Thanks! This commit is working for me though consistency is not perfect yet :)

I have prepared table which explains how I see it.

Case sensitivity enabled Case sensitivity disabled
Config language URL language URL language
en-GB en-gb en-GB en-gb
en-GB VALID now VALID
should 404
or 302 en-GB
302 en-gb VALID
en-gb now VALID
should 404
or 302 en-gb
VALID now VALID
should 302 en-gb
VALID

Not sure how hard it is to implement such states (because of additional language detection and maybe other things).

Does it make sence?

@mikehaertl
Copy link
Collaborator

Config language should never use a lowercase country code, so your line with en-gb is not valid IMO. It must always be en-GB.

I'm also not sure, if we really should be so strict and issue 404 or 302 in the cases you've shown. Notice that you can also use wildcard patterns like en-*. The basic idea was, to make the extension case insensitive with URLs, so that both en-GB and en-gb work - but the former is redirected by default to make URLs unambiguous.

@kavolorn
Copy link
Author

kavolorn commented Feb 5, 2016

Ah, now I understand. I thought that you want to have lowercase codes in configuration also. It is fine how it works now. Thanks for new option!

@kavolorn kavolorn closed this as completed Feb 5, 2016
@mikehaertl
Copy link
Collaborator

@kavolorn Ok, thanks. I'll create a 1.4.0 release (new feature). Please note that I will rename keepUpperCaseLanguageCode to keepUppercaseLanguageCode (notice the lowercase c). This is more correct as "uppercase" is a single word in english.

@kavolorn
Copy link
Author

kavolorn commented Feb 5, 2016

Alright! Thanks :)

@kavolorn
Copy link
Author

Just noticed that default / url redirects only to lowercase version even keepUppercaseLanguageCode is turned on. Is it a big thing to redirect to uppercase version by default (if option specified)? Thx

@mikehaertl
Copy link
Collaborator

Confirmed and (hopefully) fixed in brandnew release 1.4.2 ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants