From 9458970a7de806825c5ea89b42122072a1bda8d0 Mon Sep 17 00:00:00 2001 From: Mark Weaver <mark@blushingpenguin.com> Date: Thu, 26 Jul 2018 14:47:09 +0000 Subject: [PATCH 1/4] make acceptedLanguages order the header by q values --- fluent-langneg/src/accepted_languages.js | 25 +++++++++++-- fluent-langneg/test/headers_test.js | 47 ++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 4 deletions(-) diff --git a/fluent-langneg/src/accepted_languages.js b/fluent-langneg/src/accepted_languages.js index 854482bc7..bda003937 100644 --- a/fluent-langneg/src/accepted_languages.js +++ b/fluent-langneg/src/accepted_languages.js @@ -1,7 +1,24 @@ -export default function acceptedLanguages(string = "") { - if (typeof string !== "string") { +export default function acceptedLanguages(acceptLanguageHeader = "") { + if (typeof acceptLanguageHeader !== "string") { throw new TypeError("Argument must be a string"); } - const tokens = string.split(",").map(t => t.trim()); - return tokens.filter(t => t !== "").map(t => t.split(";")[0]); + const tokens = acceptLanguageHeader.split(",").map(t => t.trim()); + const langsWithQ = []; + tokens.filter(t => t !== "").forEach((t, index) => { + const langWithQ = t.split(";").map(u => u.trim()); + if (langWithQ[0].length > 0) { + let q = 1.0; + if (langWithQ.length > 1) { + const qVal = langWithQ[1].split("=").map(u => u.trim()); + if (qVal.length === 2 && qVal[0].toLowerCase() === "q") { + const qn = Number(qVal[1]); + q = !isNaN(qn) ? qn : q; + } + } + langsWithQ.push({ index: index, lang: langWithQ[0], q }); + } + }); + // order by q descending, keeping the header order for equal weights + langsWithQ.sort((a, b) => a.q === b.q ? a.index - b.index : b.q - a.q); + return langsWithQ.map(t => t.lang); } diff --git a/fluent-langneg/test/headers_test.js b/fluent-langneg/test/headers_test.js index 4443f71a0..d3bbd8df5 100644 --- a/fluent-langneg/test/headers_test.js +++ b/fluent-langneg/test/headers_test.js @@ -29,6 +29,53 @@ suite('parse headers', () => { ); }); + test('with out of order quality values', () => { + assert.deepStrictEqual( + acceptedLanguages('en;q=0.8, fr;q=0.9, de;q=0.7, *;q=0.5, fr-CH'), [ + 'fr-CH', + 'fr', + 'en', + 'de', + '*' + ] + ); + }); + + test('with equal q values', () => { + assert.deepStrictEqual( + acceptedLanguages('en;q=0.1, fr;q=0.1, de;q=0.1, *;q=0.1'), [ + 'en', + 'fr', + 'de', + '*' + ] + ); + }); + + test('with duff q values', () => { + assert.deepStrictEqual( + acceptedLanguages('en;q=no, fr;z=0.9, de;q=0.7;q=9, *;q=0.5, fr-CH;q=a=0.1'), [ + 'en', + 'fr', + 'fr-CH', + 'de', + '*' + ] + ); + }); + + test('with empty entries', () => { + assert.deepStrictEqual( + acceptedLanguages('en;q=0.8,,, fr;q=0.9,, de;q=0.7, *;q=0.5, fr-CH'), [ + 'fr-CH', + 'fr', + 'en', + 'de', + '*' + ] + ); + }); + test('edge cases', () => { const args = [ null, From 9c700265e6935260f1c48d7d1cd9006ae91b3a85 Mon Sep 17 00:00:00 2001 From: Mark Weaver <mark@blushingpenguin.com> Date: Fri, 27 Jul 2018 05:28:03 +0000 Subject: [PATCH 2/4] addressing review comments --- fluent-langneg/src/accepted_languages.js | 32 +++++++++++++----------- fluent-langneg/test/headers_test.js | 4 +-- 2 files changed, 19 insertions(+), 17 deletions(-) diff --git a/fluent-langneg/src/accepted_languages.js b/fluent-langneg/src/accepted_languages.js index bda003937..32ef9d438 100644 --- a/fluent-langneg/src/accepted_languages.js +++ b/fluent-langneg/src/accepted_languages.js @@ -1,23 +1,25 @@ +function parseAcceptLanguageEntry(entry, index) { + const langWithQ = entry.split(";").map(u => u.trim()); + let q = 1.0; + if (langWithQ.length > 1) { + const qVal = langWithQ[1].split("=").map(u => u.trim()); + if (qVal.length === 2 && qVal[0].toLowerCase() === "q") { + const qn = Number(qVal[1]); + q = isNaN(qn) ? 0.0 : qn; + } + } + return { index: index, lang: langWithQ[0], q }; +} + export default function acceptedLanguages(acceptLanguageHeader = "") { if (typeof acceptLanguageHeader !== "string") { throw new TypeError("Argument must be a string"); } - const tokens = acceptLanguageHeader.split(",").map(t => t.trim()); + const tokens = acceptLanguageHeader.split(",").map(t => t.trim()). + filter(t => t !== ""); const langsWithQ = []; - tokens.filter(t => t !== "").forEach((t, index) => { - const langWithQ = t.split(";").map(u => u.trim()); - if (langWithQ[0].length > 0) { - let q = 1.0; - if (langWithQ.length > 1) { - const qVal = langWithQ[1].split("=").map(u => u.trim()); - if (qVal.length === 2 && qVal[0].toLowerCase() === "q") { - const qn = Number(qVal[1]); - q = !isNaN(qn) ? qn : q; - } - } - langsWithQ.push({ index: index, lang: langWithQ[0], q }); - } - }); + tokens.forEach((t, index) => + langsWithQ.push(parseAcceptLanguageEntry(t, index))); // order by q descending, keeping the header order for equal weights langsWithQ.sort((a, b) => a.q === b.q ? a.index - b.index : b.q - a.q); return langsWithQ.map(t => t.lang); diff --git a/fluent-langneg/test/headers_test.js b/fluent-langneg/test/headers_test.js index d3bbd8df5..9311af80b 100644 --- a/fluent-langneg/test/headers_test.js +++ b/fluent-langneg/test/headers_test.js @@ -55,11 +55,11 @@ suite('parse headers', () => { test('with duff q values', () => { assert.deepStrictEqual( acceptedLanguages('en;q=no, fr;z=0.9, de;q=0.7;q=9, *;q=0.5, fr-CH;q=a=0.1'), [ - 'en', 'fr', 'fr-CH', 'de', - '*' + '*', + 'en' ] ); }); From da9578979e6a88db603ac9e85595c4a61e885624 Mon Sep 17 00:00:00 2001 From: Mark Weaver <mark@blushingpenguin.com> Date: Fri, 27 Jul 2018 05:45:31 +0000 Subject: [PATCH 3/4] fix lint warning --- fluent-langneg/src/accepted_languages.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fluent-langneg/src/accepted_languages.js b/fluent-langneg/src/accepted_languages.js index 32ef9d438..1350b5c3d 100644 --- a/fluent-langneg/src/accepted_languages.js +++ b/fluent-langneg/src/accepted_languages.js @@ -15,8 +15,8 @@ export default function acceptedLanguages(acceptLanguageHeader = "") { if (typeof acceptLanguageHeader !== "string") { throw new TypeError("Argument must be a string"); } - const tokens = acceptLanguageHeader.split(",").map(t => t.trim()). - filter(t => t !== ""); + const tokens = acceptLanguageHeader.split(",").map(t => t.trim()) + .filter(t => t !== ""); const langsWithQ = []; tokens.forEach((t, index) => langsWithQ.push(parseAcceptLanguageEntry(t, index))); From fc62da6e3ef7a558db1b9c6d8c23ef24151e3e18 Mon Sep 17 00:00:00 2001 From: Mark Weaver <mark@blushingpenguin.com> Date: Fri, 6 Dec 2019 07:42:11 +0000 Subject: [PATCH 4/4] use built in methods that preserve array index rather than adding it to the language entry descriptor as suggested by zbraniecki's code review --- fluent-langneg/src/accepted_languages.js | 13 ++++++------- fluent-langneg/test/headers_test.js | 6 ++++++ 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/fluent-langneg/src/accepted_languages.js b/fluent-langneg/src/accepted_languages.js index 1350b5c3d..655124b7b 100644 --- a/fluent-langneg/src/accepted_languages.js +++ b/fluent-langneg/src/accepted_languages.js @@ -1,4 +1,4 @@ -function parseAcceptLanguageEntry(entry, index) { +function parseAcceptLanguageEntry(entry) { const langWithQ = entry.split(";").map(u => u.trim()); let q = 1.0; if (langWithQ.length > 1) { @@ -8,7 +8,7 @@ function parseAcceptLanguageEntry(entry, index) { q = isNaN(qn) ? 0.0 : qn; } } - return { index: index, lang: langWithQ[0], q }; + return { lang: langWithQ[0], q }; } export default function acceptedLanguages(acceptLanguageHeader = "") { @@ -17,10 +17,9 @@ export default function acceptedLanguages(acceptLanguageHeader = "") { } const tokens = acceptLanguageHeader.split(",").map(t => t.trim()) .filter(t => t !== ""); - const langsWithQ = []; - tokens.forEach((t, index) => - langsWithQ.push(parseAcceptLanguageEntry(t, index))); + const langsWithQ = Array.from(tokens.map(parseAcceptLanguageEntry).entries()); // order by q descending, keeping the header order for equal weights - langsWithQ.sort((a, b) => a.q === b.q ? a.index - b.index : b.q - a.q); - return langsWithQ.map(t => t.lang); + langsWithQ.sort(([aidx, aval], [bidx, bval]) => + aval.q === bval.q ? aidx - bidx : bval.q - aval.q); + return langsWithQ.map(([, val]) => val.lang); } diff --git a/fluent-langneg/test/headers_test.js b/fluent-langneg/test/headers_test.js index 9311af80b..c83b52733 100644 --- a/fluent-langneg/test/headers_test.js +++ b/fluent-langneg/test/headers_test.js @@ -2,6 +2,12 @@ import assert from 'assert'; import acceptedLanguages from '../src/accepted_languages'; suite('parse headers', () => { + test('without an argument', () => { + assert.deepStrictEqual( + acceptedLanguages(), [] + ); + }); + test('without quality values', () => { assert.deepStrictEqual( acceptedLanguages('en-US, fr, pl'), [