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

Parse locale from Accept-Language HTTP header #92

Merged
merged 2 commits into from
May 19, 2021
Merged

Parse locale from Accept-Language HTTP header #92

merged 2 commits into from
May 19, 2021

Conversation

schmijos
Copy link
Contributor

@schmijos schmijos commented May 17, 2021

Fixes #78

(no effect though, because we don't have any translated content yet)

@schmijos schmijos self-assigned this May 17, 2021
@schmijos schmijos requested a review from Makram95 May 17, 2021 21:19
Comment on lines +25 to +26
get test_path, params: {}, env: { 'HTTP_ACCEPT_LANGUAGE' => 'de-CH, en-US, fr' }
assert_equal 'fr', response.body
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Attention: no partial matches, only exact ones.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be OK for us, since we can control the request headers.

@@ -32,5 +32,9 @@ class Application < Rails::Application
config.api_only = true

config.time_zone = 'UTC'

config.i18n.available_locales = [:de, :en, :fr, :it, :rm]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand the code correctly, the option for swiss-german is missing. Could it be added?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How? What's the appropriate HTTP_ACCEPT_LANGUAGE header for schweizerdeutsch?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This site says it would be "de-ch". Using the ISO 639-1 up-to ISO 639-3 would be "gsw". Another possibility could be just "ch" to match the style of the others, but I'm really not familiar with how this is usually handled 😬

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

de-CH is german in Switzerland. That's for sure not what you intend. In BCP47's RFCs I didn't find any mapping to ISO, so I think gsw is the correct one according to the IANA tag list: http://www.iana.org/assignments/language-subtag-registry/language-subtag-registry

Copy link
Member

@Makram95 Makram95 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a small comment about the option for swiss-german, otherwise looking good. But I'm not very familiar with rails, maybe add @dbrgn also as a reviewer 😅

Copy link
Contributor

@dbrgn dbrgn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but it would be nice if :gsw could be added as well.

@schmijos schmijos merged commit fab058a into master May 19, 2021
@schmijos schmijos deleted the fix/78 branch May 19, 2021 20:54
@dbrgn dbrgn mentioned this pull request Aug 3, 2021
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 this pull request may close these issues.

Localiziable API requests
3 participants