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

Accept-Language de-DE does not end up using de.all.json #76

Closed
nuarhu opened this issue Jun 19, 2017 · 7 comments · Fixed by #92
Closed

Accept-Language de-DE does not end up using de.all.json #76

nuarhu opened this issue Jun 19, 2017 · 7 comments · Fixed by #92
Assignees
Milestone

Comments

@nuarhu
Copy link

nuarhu commented Jun 19, 2017

Hi!

This might be similar to #30 but that issue is quite old so I'm rather starting a new one here. I'm not sure it's completely related.

My setup

  • de.all.json
  • en.all.json

I am using simply de and en because there is just one translation for each language, no flavours and I want them to be used for any kind of locales of those languages that come along, be it en-GB, en-US, de-CH, de-DE, etc. Default language is en in any case, and I really want to get the English text if the German version should not be found for some reason (which best should never happen).

The issue

With Accept-Language header de-DE, the de translations are not found.

This is becauselang.MatchingTags() is only applied when adding translations (so in my case de, en, and of course the matching tags are only de and en).
When the translation function is requested for de-DE, only lang.Tag is used to lookup the fallback language and this is then only de-de (in bundle.translatedLanguage()).

So, my question is, would it not make more sense to use lang.MatchingTags() also in bundle.translatedLanguage() where the fallback is looked up?

@iainduncani
Copy link

I come from a Java world so I was surprised by this as well. In Java if you had the files:

en.all.json

Then en and en_US etc. would both match to that file whereas if you just had:

en_US.all.json

Then en would not match it but en_US would so the opposite of the matching rules here. I can see some merit of matching to more specific languages (i.e. en -> en_US, although I'm not convinced this will work for all languages) but think that the opposite should also be supported...

@iainduncani
Copy link

I just read through the very informative and interesting:

https://blog.golang.org/matchlang

Looking at what the golang.org/x/text/language package does it's matcher is much more comprehensive than either the one here or the one I knew from Java... For instance if en_US is the only supported English locale then en_US, en_GB and en will all match to it (as will Albanian!). I wonder if this project should switch to using that for it's language matching...

@nicksnyder
Copy link
Owner

I agree that it would be ideal for this project to depend on golang.org/x/text/language for matching.

So, my question is, would it not make more sense to use lang.MatchingTags() also in bundle.translatedLanguage() where the fallback is looked up?

There is probably some low hanging fruit as you describe (which was mentioned in #30) for making the current matching slightly better. I am open to PRs for this. Right now I don't have time to allocate to this.

@nicksnyder nicksnyder added the accepting PRs Pull requests from the community that solve this problem would be appreciated. label Sep 17, 2017
@nicksnyder
Copy link
Owner

Also related: #44

@nicksnyder nicksnyder removed the accepting PRs Pull requests from the community that solve this problem would be appreciated. label Feb 14, 2018
@nicksnyder nicksnyder added this to the v2 milestone Feb 14, 2018
@nicksnyder nicksnyder mentioned this issue Apr 10, 2018
Merged
4 tasks
@nicksnyder
Copy link
Owner

v2 contains a proposal to fix this. See #92

@nicksnyder
Copy link
Owner

More specifically, it relies go golang.org/x/text/language

@nicksnyder nicksnyder self-assigned this Apr 10, 2018
@nicksnyder
Copy link
Owner

I just tagged 2.0.0.beta.1. Please start using it and report any issues that you have.

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

Successfully merging a pull request may close this issue.

3 participants