-
Notifications
You must be signed in to change notification settings - Fork 284
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
better i18n Accept-Language parsing to properly support examples (+ as standalone function) #1850
Merged
Merged
Changes from 2 commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,7 +10,7 @@ module vibe.web.i18n; | |
import vibe.http.server : HTTPServerRequest; | ||
|
||
import std.algorithm : canFind, min, startsWith; | ||
|
||
import std.range.primitives : isForwardRange; | ||
|
||
/** | ||
Annotates an interface method or class with translation information. | ||
|
@@ -62,6 +62,7 @@ unittest { | |
|
||
struct TranslationContext { | ||
import std.typetuple; | ||
// A language can be in the form en_US, en-US or en. Put the languages you want to prioritize first. | ||
alias languages = TypeTuple!("en_US", "de_DE", "fr_FR"); | ||
//mixin translationModule!"app"; | ||
//mixin translationModule!"somelib"; | ||
|
@@ -70,7 +71,7 @@ unittest { | |
// "Accept-Language" header | ||
static string determineLanguage(scope HTTPServerRequest req) | ||
{ | ||
if (!req.session) return null; // use default language | ||
if (!req.session) return req.determineLanguageByHeader(languages); // default behaviour using "Accept-Language" header | ||
return req.session.get("language", ""); | ||
} | ||
} | ||
|
@@ -221,11 +222,93 @@ template tr(CTX, string LANG) | |
} | ||
} | ||
|
||
package string determineLanguage(alias METHOD)(scope HTTPServerRequest req) | ||
/// Determines a language code from the value of a header string. | ||
/// Returns: The best match from the Accept-Language header for a language. `null` if there is no supported language. | ||
public string determineLanguageByHeader(T)(string accept_language, T allowed_languages) @safe pure @nogc | ||
if (isForwardRange!T) | ||
{ | ||
import std.algorithm : splitter, countUntil; | ||
import std.string : indexOf; | ||
import std.array; | ||
|
||
// TODO: verify that allowed_languages doesn't contain a mix of languages with and without extra specifier for the same lanaguage (but only if one without specifier comes before those with specifier) | ||
// Implementing that feature should try to give a compile time warning and not change the behaviour of this function. | ||
|
||
if (!accept_language.length) | ||
return null; | ||
|
||
string fallback = null; | ||
foreach (accept; accept_language.splitter(",")) { | ||
auto sidx = accept.indexOf(';'); | ||
if (sidx >= 0) | ||
accept = accept[0 .. sidx]; | ||
|
||
string alang, aextra; | ||
auto asep = accept.countUntil!(a => a == '_' || a == '-'); | ||
if (asep < 0) | ||
alang = accept; | ||
else { | ||
alang = accept[0 .. asep]; | ||
aextra = accept[asep + 1 .. $]; | ||
} | ||
|
||
foreach (lang; allowed_languages) { | ||
string lcode, lextra; | ||
sidx = lang.countUntil!(a => a == '_' || a == '-'); | ||
if (sidx < 0) | ||
lcode = lang; | ||
else { | ||
lcode = lang[0 .. sidx]; | ||
lextra = lang[sidx + 1 .. $]; | ||
} | ||
// request en_US == serve en_US | ||
if (lcode == alang && lextra == aextra) | ||
return lang; | ||
// request en_* == serve en | ||
if (lcode == alang && !lextra.length) | ||
return lang; | ||
// request en* == serve en_* && be first occurence | ||
if (lcode == alang && lextra.length && !fallback.length) | ||
fallback = lang; | ||
} | ||
} | ||
|
||
return fallback; | ||
} | ||
|
||
/// ditto | ||
public string determineLanguageByHeader(Tuple...)(string accept_language, Tuple allowed_languages) @safe pure | ||
{ | ||
return determineLanguageByHeader(accept_language, [allowed_languages]); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
} | ||
|
||
/// ditto | ||
public string determineLanguageByHeader(T)(HTTPServerRequest req, T allowed_languages) @safe pure | ||
if (isForwardRange!T) | ||
{ | ||
return determineLanguageByHeader(req.headers.get("Accept-Language", null), allowed_languages); | ||
} | ||
|
||
/// ditto | ||
public string determineLanguageByHeader(Tuple...)(HTTPServerRequest req, Tuple allowed_languages) @safe pure | ||
{ | ||
return determineLanguageByHeader(req.headers.get("Accept-Language", null), [allowed_languages]); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. also |
||
} | ||
|
||
@safe unittest { | ||
assert(determineLanguageByHeader("de,de-DE;q=0.8,en;q=0.6,en-US;q=0.4", ["en-US", "de_DE", "de_CH"]) == "de_DE"); | ||
assert(determineLanguageByHeader("de,de-CH;q=0.8,en;q=0.6,en-US;q=0.4", ["en_US", "de_DE", "de-CH"]) == "de-CH"); | ||
assert(determineLanguageByHeader("en_CA,en_US", ["ja_JP", "en"]) == "en"); | ||
assert(determineLanguageByHeader("en", ["ja_JP", "en"]) == "en"); | ||
assert(determineLanguageByHeader("en", ["ja_JP", "en_US"]) == "en_US"); | ||
assert(determineLanguageByHeader("en_US", ["ja-JP", "en"]) == "en"); | ||
assert(determineLanguageByHeader("de,de-DE;q=0.8,en;q=0.6,en-US;q=0.4", ["ja_JP"]) is null); | ||
assert(determineLanguageByHeader("de, de-DE ;q=0.8 , en ;q=0.6 , en-US;q=0.4", ["de-DE"]) == "de-DE"); | ||
assert(determineLanguageByHeader("en_GB", ["en_US"]) == "en_US"); | ||
assert(determineLanguageByHeader("de_DE", ["en_US"]) is null); | ||
} | ||
|
||
package string determineLanguage(alias METHOD)(scope HTTPServerRequest req) | ||
{ | ||
alias CTX = GetTranslationContext!METHOD; | ||
|
||
static if (!is(CTX == void)) { | ||
|
@@ -234,27 +317,7 @@ package string determineLanguage(alias METHOD)(scope HTTPServerRequest req) | |
"determineLanguage in a translation context must return a language string."); | ||
return CTX.determineLanguage(req); | ||
} else { | ||
auto accept_lang = req.headers.get("Accept-Language", null); | ||
|
||
size_t csidx = 0; | ||
while (accept_lang.length) { | ||
auto cidx = accept_lang[csidx .. $].indexOf(','); | ||
if (cidx < 0) cidx = accept_lang.length; | ||
auto entry = accept_lang[csidx .. csidx + cidx]; | ||
auto sidx = entry.indexOf(';'); | ||
if (sidx < 0) sidx = entry.length; | ||
auto entrylang = entry[0 .. sidx]; | ||
|
||
foreach (lang; CTX.languages) { | ||
if (entrylang == replace(lang, "_", "-")) return lang; | ||
if (entrylang == split(lang, "_")[0]) return lang; // FIXME: ensure that only one single-lang entry exists! | ||
} | ||
|
||
if (cidx >= accept_lang.length) break; | ||
accept_lang = accept_lang[cidx+1 .. $]; | ||
} | ||
|
||
return null; | ||
return determineLanguageByHeader(req, CTX.languages); | ||
} | ||
} else return null; | ||
} | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
There is one fallback case missing here:
determineLanguageByHeader("en_US,enCA", ["en_GB"]) == "en_GB")
. This should ideally still apply, though:determineLanguageByHeader("en_US,enCA", ["en_GB", "en"]) == "en")
.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.
those tests already work, assuming the allowed_languages argument is valid (most generic languages should be to the right of the specific ones)
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.
Nevermind, I misread this as
!lextra.length
and it's actually fine like it is.