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.

Sign up for GitHub

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

Jump to bottom

better i18n Accept-Language parsing to properly support examples (+ as standalone function) #1850

Merged
merged 3 commits into from
Jul 18, 2017
Merged

better i18n Accept-Language parsing to properly support examples (+ as standalone function) #1850

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2c667b4
More flexible standalone parsing of Accept-Language
WebFreak001 Jul 18, 2017
15b7c65
Adhere to codestyle
WebFreak001 Jul 18, 2017
f3be492
Use std.range.only & more unittests
WebFreak001 Jul 18, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
119 changes: 94 additions & 25 deletions web/vibe/web/i18n.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ module vibe.web.i18n;
import vibe.http.server : HTTPServerRequest;

import std.algorithm : canFind, min, startsWith;

import std.range.primitives : isForwardRange;
import std.range : only;

/**
Annotates an interface method or class with translation information.
Expand Down Expand Up @@ -62,6 +63,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";
Expand All @@ -70,7 +72,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", "");
}
}
Expand Down Expand Up @@ -221,11 +223,98 @@ 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)
Copy link
Member

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").

Copy link
Contributor Author

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)

Copy link
Member

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.

fallback = lang;
}
}

return fallback;
}

/// ditto
public string determineLanguageByHeader(Tuple...)(string accept_language, Tuple allowed_languages) @safe pure @nogc
{
return determineLanguageByHeader(accept_language, only(allowed_languages));
}

/// 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), only(allowed_languages));
}

@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);
assert(determineLanguageByHeader("en_US,enCA", ["en_GB"]) == "en_GB");
assert(determineLanguageByHeader("en_US,enCA", ["en_GB", "en"]) == "en");
assert(determineLanguageByHeader("en_US,enCA", ["en", "en_GB"]) == "en");
// TODO from above (should be invalid input having a more generic language first in the list!)
//assert(determineLanguageByHeader("en_US,enCA", ["en", "en_US"]) == "en_US");
}

package string determineLanguage(alias METHOD)(scope HTTPServerRequest req)
{
alias CTX = GetTranslationContext!METHOD;

static if (!is(CTX == void)) {
Expand All @@ -234,27 +323,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;
}
Expand Down