From f0368f335e0c30959cde8d261f166931c8fefbde Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Fri, 22 Aug 2025 11:15:45 +0200 Subject: [PATCH 1/6] feat(korean): only use korean generator when requsted lang is "kor" --- builders/KOR.js | 7 ++++++- labelGenerator.js | 3 +-- test/labelGenerator_KOR.js | 20 ++++++++++---------- 3 files changed, 17 insertions(+), 13 deletions(-) diff --git a/builders/KOR.js b/builders/KOR.js index 56af3aa..9420d5d 100644 --- a/builders/KOR.js +++ b/builders/KOR.js @@ -58,7 +58,12 @@ function buildAdminLabelPart(schema, record) { // builds a complete label by combining several components // admin parts: administrative information like city // name or address parts: info on the actual record, like name or address -function koreaBuilder(schema, record) { +function koreaBuilder(schema, record, { language, defaultBuilder }) { + + // only run builder when requested language is set to Korean + if (language !== 'kor') { + return defaultBuilder.apply(null, arguments); + } const adminParts = buildAdminLabelPart(schema, record); const nameorAddressParts = nameOrAddressComponents(schema, record); diff --git a/labelGenerator.js b/labelGenerator.js index 8fd23ad..f589f6a 100644 --- a/labelGenerator.js +++ b/labelGenerator.js @@ -104,8 +104,7 @@ module.exports = function( record, language ){ const schema = getSchema(record, language); const separator = _.get(schema, ['meta','separator'], ', '); const builder = _.get(schema, ['meta', 'builder'], defaultBuilder); - - let labelParts = builder(schema, record); + const labelParts = builder(schema, record, { language, defaultBuilder }); return _.trim(labelParts.join(separator)); }; diff --git a/test/labelGenerator_KOR.js b/test/labelGenerator_KOR.js index 9475884..4a1bbc5 100644 --- a/test/labelGenerator_KOR.js +++ b/test/labelGenerator_KOR.js @@ -26,7 +26,7 @@ module.exports.tests.south_korea = function(test, common) { 'country_a': ['KOR'], 'country': ['South Korea'] }; - t.equal(generator(doc),'South Korea region name county name venue name'); + t.equal(generator(doc, 'kor'),'South Korea region name county name venue name'); t.end(); }); @@ -41,7 +41,7 @@ module.exports.tests.south_korea = function(test, common) { 'country_a': ['KOR'], 'country': ['South Korea'] }; - t.equal(generator(doc),'South Korea region name county name street name 123'); + t.equal(generator(doc, 'kor'),'South Korea region name county name street name 123'); t.end(); }); @@ -58,7 +58,7 @@ module.exports.tests.south_korea = function(test, common) { 'country_a': ['KOR'], 'country': ['South Korea'] }; - t.equal(generator(doc),'South Korea region name county name street name 123'); + t.equal(generator(doc, 'kor'),'South Korea region name county name street name 123'); t.end(); }); @@ -74,7 +74,7 @@ module.exports.tests.south_korea = function(test, common) { 'country_a': ['KOR'], 'country': ['South Korea'] }; - t.equal(generator(doc),'South Korea region name county name neighbourhood name'); + t.equal(generator(doc, 'kor'),'South Korea region name county name neighbourhood name'); t.end(); }); @@ -89,7 +89,7 @@ module.exports.tests.south_korea = function(test, common) { 'country_a': ['KOR'], 'country': ['South Korea'] }; - t.equal(generator(doc),'South Korea region name county name locality name'); + t.equal(generator(doc, 'kor'),'South Korea region name county name locality name'); t.end(); }); @@ -103,7 +103,7 @@ module.exports.tests.south_korea = function(test, common) { 'country_a': ['KOR'], 'country': ['South Korea'] }; - t.equal(generator(doc),'South Korea region name county name localadmin name'); + t.equal(generator(doc, 'kor'),'South Korea region name county name localadmin name'); t.end(); }); @@ -116,7 +116,7 @@ module.exports.tests.south_korea = function(test, common) { 'country_a': ['KOR'], 'country': ['South Korea'] }; - t.equal(generator(doc),'South Korea region name county name'); + t.equal(generator(doc, 'kor'),'South Korea region name county name'); t.end(); }); @@ -128,7 +128,7 @@ module.exports.tests.south_korea = function(test, common) { 'country_a': ['KOR'], 'country': ['South Korea'] }; - t.equal(generator(doc),'South Korea region name'); + t.equal(generator(doc, 'kor'),'South Korea region name'); t.end(); }); @@ -139,7 +139,7 @@ module.exports.tests.south_korea = function(test, common) { 'country_a': ['KOR'], 'country': ['South Korea'] }; - t.equal(generator(doc),'South Korea'); + t.equal(generator(doc, 'kor'),'South Korea'); t.end(); }); @@ -159,7 +159,7 @@ module.exports.tests.south_korea = function(test, common) { 'street': '모세로', 'country': ['한국'] }; - t.equal(generator(doc),'한국 서울 용산구 모세로 27'); + t.equal(generator(doc, 'kor'),'한국 서울 용산구 모세로 27'); t.end(); }); }; From 3fe45be0636efad00574deab47a7d82a63d9c113 Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Fri, 22 Aug 2025 12:14:48 +0200 Subject: [PATCH 2/6] feat(korean): refactor Korean configuration to return English labels --- builders/KOR.js | 13 ++++--------- labelGenerator.js | 17 ++++++++++++++--- labelSchema.js | 10 +++++++--- test/labelGenerator_KOR.js | 24 ++++++++++++++++++++++++ 4 files changed, 49 insertions(+), 15 deletions(-) diff --git a/builders/KOR.js b/builders/KOR.js index 9420d5d..75f734b 100644 --- a/builders/KOR.js +++ b/builders/KOR.js @@ -16,7 +16,7 @@ function dedupeNameAndLastLabelElement(labelParts) { return labelParts; } -function nameOrAddressComponents(schema, record) { +function nameOrAddressComponents(record) { const labelParts = []; // add the address/venue components @@ -47,7 +47,7 @@ function buildAdminLabelPart(schema, record) { // the order of the properties in the file determine // the order of elements in this arrayt // For Korea, we start with the country and work backwards - for (const field in schema.valueFunctions) { + for (const field of Object.keys(schema.valueFunctions).reverse()) { const valueFunction = schema.valueFunctions[field]; labelParts.push(valueFunction(record)); } @@ -58,15 +58,10 @@ function buildAdminLabelPart(schema, record) { // builds a complete label by combining several components // admin parts: administrative information like city // name or address parts: info on the actual record, like name or address -function koreaBuilder(schema, record, { language, defaultBuilder }) { - - // only run builder when requested language is set to Korean - if (language !== 'kor') { - return defaultBuilder.apply(null, arguments); - } +function koreaBuilder(schema, record) { const adminParts = buildAdminLabelPart(schema, record); - const nameorAddressParts = nameOrAddressComponents(schema, record); + const nameorAddressParts = nameOrAddressComponents(record); let labelParts = _.concat(adminParts, nameorAddressParts); diff --git a/labelGenerator.js b/labelGenerator.js index f589f6a..e44a874 100644 --- a/labelGenerator.js +++ b/labelGenerator.js @@ -102,9 +102,20 @@ function defaultBuilder(schema, record) { module.exports = function( record, language ){ const schema = getSchema(record, language); - const separator = _.get(schema, ['meta','separator'], ', '); - const builder = _.get(schema, ['meta', 'builder'], defaultBuilder); - const labelParts = builder(schema, record, { language, defaultBuilder }); + + let separator = ', '; + let builder = defaultBuilder; + + // country-specific builder + if (_.has(schema, 'meta.builder')) { + const predicate = _.get(schema, 'meta.predicate', () => true); + if (predicate(schema, record, language)) { + separator = _.get(schema, 'meta.separator', separator); + builder = _.get(schema, 'meta.builder'); + } + } + + const labelParts = builder(schema, record); return _.trim(labelParts.join(separator)); }; diff --git a/labelSchema.js b/labelSchema.js index 51e1af0..21763d8 100644 --- a/labelSchema.js +++ b/labelSchema.js @@ -219,13 +219,17 @@ module.exports = { }, 'KOR': { 'valueFunctions': { - 'country': getFirstProperty(['country']), + 'city': getFirstProperty(['county']), 'province': getFirstProperty(['region']), - 'city': getFirstProperty(['county']) + 'country': getFirstProperty(['country']) }, 'meta': { 'separator': ' ', - 'builder': require('./builders/KOR') + 'builder': require('./builders/KOR'), + 'predicate' (schema, record, language) { + // only run builder when requested language is set to Korean + return language === 'kor'; + } } }, 'FRA': { diff --git a/test/labelGenerator_KOR.js b/test/labelGenerator_KOR.js index 4a1bbc5..49c669a 100644 --- a/test/labelGenerator_KOR.js +++ b/test/labelGenerator_KOR.js @@ -164,6 +164,30 @@ module.exports.tests.south_korea = function(test, common) { }); }; +// when the requested language is not 'kor' fields are returned in the order +// specified by the default generator. +module.exports.tests.south_korea_lang_eng = function(test, common) { + test('venue', function(t) { + var doc = { + 'name': { 'default': 'venue name' }, + 'layer': 'venue', + 'housenumber': 'house number', + 'street': 'street name', + 'neighbourhood': ['neighbourhood name'], + 'localadmin': ['localadmin name'], + 'county': ['county name'], + 'macrocounty': ['macrocounty name'], + 'region_a': ['region abbr'], + 'region': ['region name'], + 'macroregion': ['macroregion name'], + 'country_a': ['KOR'], + 'country': ['South Korea'] + }; + t.equal(generator(doc, 'eng'),'venue name, county name, region name, South Korea'); + t.end(); + }); +}; + module.exports.all = function (tape, common) { function test(name, testFunction) { From 9dbcd3c48f9a6421c229eb0ef52118855ab600f4 Mon Sep 17 00:00:00 2001 From: Peter Johnson Date: Tue, 26 Aug 2025 13:57:23 +0200 Subject: [PATCH 3/6] feat(korean): use "languages" block --- builders/KOR.js | 6 +++--- labelGenerator.js | 22 +++------------------- labelSchema.js | 21 +++++++++++++-------- 3 files changed, 19 insertions(+), 30 deletions(-) diff --git a/builders/KOR.js b/builders/KOR.js index 75f734b..56af3aa 100644 --- a/builders/KOR.js +++ b/builders/KOR.js @@ -16,7 +16,7 @@ function dedupeNameAndLastLabelElement(labelParts) { return labelParts; } -function nameOrAddressComponents(record) { +function nameOrAddressComponents(schema, record) { const labelParts = []; // add the address/venue components @@ -47,7 +47,7 @@ function buildAdminLabelPart(schema, record) { // the order of the properties in the file determine // the order of elements in this arrayt // For Korea, we start with the country and work backwards - for (const field of Object.keys(schema.valueFunctions).reverse()) { + for (const field in schema.valueFunctions) { const valueFunction = schema.valueFunctions[field]; labelParts.push(valueFunction(record)); } @@ -61,7 +61,7 @@ function buildAdminLabelPart(schema, record) { function koreaBuilder(schema, record) { const adminParts = buildAdminLabelPart(schema, record); - const nameorAddressParts = nameOrAddressComponents(record); + const nameorAddressParts = nameOrAddressComponents(schema, record); let labelParts = _.concat(adminParts, nameorAddressParts); diff --git a/labelGenerator.js b/labelGenerator.js index e44a874..b1ed8ca 100644 --- a/labelGenerator.js +++ b/labelGenerator.js @@ -17,12 +17,6 @@ function dedupeNameAndFirstLabelElement(labelParts) { return labelParts; } -function getLanguage(language) { - if (!_.isString(language)) { return; } - - return language.toUpperCase(); -} - // this can go away once geonames is no longer supported // https://github.com/pelias/wof-admin-lookup/issues/49 function isCountry(layer) { @@ -102,20 +96,10 @@ function defaultBuilder(schema, record) { module.exports = function( record, language ){ const schema = getSchema(record, language); + const separator = _.get(schema, ['meta','separator'], ', '); + const builder = _.get(schema, ['meta', 'builder'], defaultBuilder); - let separator = ', '; - let builder = defaultBuilder; - - // country-specific builder - if (_.has(schema, 'meta.builder')) { - const predicate = _.get(schema, 'meta.predicate', () => true); - if (predicate(schema, record, language)) { - separator = _.get(schema, 'meta.separator', separator); - builder = _.get(schema, 'meta.builder'); - } - } - - const labelParts = builder(schema, record); + let labelParts = builder(schema, record); return _.trim(labelParts.join(separator)); }; diff --git a/labelSchema.js b/labelSchema.js index 21763d8..743731e 100644 --- a/labelSchema.js +++ b/labelSchema.js @@ -218,18 +218,23 @@ module.exports = { } }, 'KOR': { + 'languages': { + 'KOR': { + 'valueFunctions': { + 'county': getFirstProperty(['country']), + 'province': getFirstProperty(['region']), + 'city': getFirstProperty(['county']) + }, + 'meta': { + 'separator': ' ', + 'builder': require('./builders/KOR') + } + } + }, 'valueFunctions': { 'city': getFirstProperty(['county']), 'province': getFirstProperty(['region']), 'country': getFirstProperty(['country']) - }, - 'meta': { - 'separator': ' ', - 'builder': require('./builders/KOR'), - 'predicate' (schema, record, language) { - // only run builder when requested language is set to Korean - return language === 'kor'; - } } }, 'FRA': { From 1333b4331dfa8648f4d32a2e1a5d827a75e9db2f Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Tue, 26 Aug 2025 13:20:42 -0400 Subject: [PATCH 4/6] Fix typo county->country --- labelSchema.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/labelSchema.js b/labelSchema.js index 743731e..cf983f5 100644 --- a/labelSchema.js +++ b/labelSchema.js @@ -221,7 +221,7 @@ module.exports = { 'languages': { 'KOR': { 'valueFunctions': { - 'county': getFirstProperty(['country']), + 'country': getFirstProperty(['country']), 'province': getFirstProperty(['region']), 'city': getFirstProperty(['county']) }, From d87d2306060ad16d191fd2fd4c9ef91d5b1aa7f4 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Tue, 26 Aug 2025 13:32:56 -0400 Subject: [PATCH 5/6] feat(korea): Simplify South Korean labels In South Korea, we can probably simplify labels a bit, and only include province info if needed because there was no locality or localadmin info. --- labelSchema.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/labelSchema.js b/labelSchema.js index cf983f5..4ff9084 100644 --- a/labelSchema.js +++ b/labelSchema.js @@ -232,8 +232,7 @@ module.exports = { } }, 'valueFunctions': { - 'city': getFirstProperty(['county']), - 'province': getFirstProperty(['region']), + 'city': getFirstProperty(['locality', 'localadmin', 'region']), 'country': getFirstProperty(['country']) } }, From e7072731bd4fcc653a9e4a0e1d181f7fad29a601 Mon Sep 17 00:00:00 2001 From: Julian Simioni Date: Tue, 26 Aug 2025 13:33:40 -0400 Subject: [PATCH 6/6] Expand and clarify tests for South Korean venues/admins --- test/labelGenerator_KOR.js | 51 +++++++++++++++++++++++++++++++++++--- 1 file changed, 48 insertions(+), 3 deletions(-) diff --git a/test/labelGenerator_KOR.js b/test/labelGenerator_KOR.js index 49c669a..9c5ae56 100644 --- a/test/labelGenerator_KOR.js +++ b/test/labelGenerator_KOR.js @@ -164,9 +164,9 @@ module.exports.tests.south_korea = function(test, common) { }); }; -// when the requested language is not 'kor' fields are returned in the order +// when the requested language is english, fields are returned in the order // specified by the default generator. -module.exports.tests.south_korea_lang_eng = function(test, common) { +module.exports.tests.south_korea_lang_venue_eng = function(test, common) { test('venue', function(t) { var doc = { 'name': { 'default': 'venue name' }, @@ -183,7 +183,52 @@ module.exports.tests.south_korea_lang_eng = function(test, common) { 'country_a': ['KOR'], 'country': ['South Korea'] }; - t.equal(generator(doc, 'eng'),'venue name, county name, region name, South Korea'); + t.equal(generator(doc, 'eng'),'venue name, localadmin name, South Korea'); + t.end(); + }); +}; + +// when the requested language is unspecified, fields are returned in the order +// specified by the default generator. +module.exports.tests.south_korea_venue_lang_default = function(test, common) { + test('venue', function(t) { + var doc = { + 'name': { 'default': 'venue name' }, + 'layer': 'venue', + 'housenumber': 'house number', + 'street': 'street name', + 'neighbourhood': ['neighbourhood name'], + 'localadmin': ['localadmin name'], + 'county': ['county name'], + 'macrocounty': ['macrocounty name'], + 'region_a': ['region abbr'], + 'region': ['region name'], + 'macroregion': ['macroregion name'], + 'country_a': ['KOR'], + 'country': ['South Korea'] + }; + t.equal(generator(doc),'venue name, localadmin name, South Korea'); + t.end(); + }); +}; + +// when the requested language is unspecified, fields are returned in the order +// specified by the default generator. +module.exports.tests.south_korea_admin_lang_default = function(test, common) { + test('venue', function(t) { + var doc = { + 'name': { 'default': 'locality name' }, + 'layer': 'locality', + 'locality': ['locality name'], + 'county': ['county name'], + 'macrocounty': ['macrocounty name'], + 'region_a': ['region abbr'], + 'region': ['region name'], + 'macroregion': ['macroregion name'], + 'country_a': ['KOR'], + 'country': ['South Korea'] + }; + t.equal(generator(doc), 'locality name, South Korea'); t.end(); }); };