[FEATURE] manifestCreator: enrich manifest with supportedLocales in i18n (for libraries) #547

tobiasso85 · 2020-11-20T16:40:10Z

With app descriptor v22 (1.21.0) the i18n sections in the manifest can be an object
with:

bundleUrl
supportedLocales

This change reads the provided properties files and automatically creates the supportedLocales
array based on this information.

e.g.

files:

i18n/i18n.properties
i18n/i18n_de.properties
i18n/i18n_en.properties

snippet in manifest.json

"i18n": {
      "bundleUrl": "i18n/i18n.properties",
      "supportedLocales": [
        "",
        "de",
        "en"
      ]
    },

coveralls · 2020-11-20T16:42:28Z

Coverage increased (+0.3%) to 93.546% when pulling d6e7c64 on manifest-creator-supported-locales into 1b7a277 on master.

matz3 · 2020-12-02T07:59:20Z

test/lib/processors/manifestCreator.js

-	t.is(verboseLogStub.getCall(1).args[1], "/resources/sap/apf");
-});
-
-test.serial("manifest creation for sap/ui/core", async (t) => {


Removed by accident?

restored with:
535668a

matz3 · 2020-12-02T08:07:04Z

lib/processors/manifestCreator.js

 					} else {
 						return false;
 					}
 				}
+				// i18n can be an empty string therefore check for not being null


But for an empty string createI18nSection returns false anyways. Is this correct?

tackled with: 8eaf458

matz3 · 2020-12-02T08:07:33Z

lib/processors/manifestCreator.js

+	 *   <code>null</code> if given i18n String is <code>null</code>
+	 */
+	function createI18nSection(i18n, i18nToSupportedLocales) {
+		if (!i18n) {


this handles the empty string

tackled with: 8eaf458

matz3 · 2020-12-02T08:09:33Z

lib/processors/manifestCreator.js

+				supportedLocales.add("");
+			}
+			const i18nPatternStr = i18n.substring(0, i18n.length - ".properties".length);
+			const i18nRegexp = new RegExp(i18nPatternStr + "_([^.]+)\\.properties$");


Does i18nPatternStr need to be encoded for RegExp or can we be sure that it doesn't contain special characters?

tackled with: f649422

matz3 · 2020-12-02T08:18:02Z

lib/processors/manifestCreator.js

+			i18nToSupportedLocales.set(i18n, supportedLocales);
+		}
+
+		const supportedLocalesArray = [...supportedLocales];


While this is fine to convert the Set to an Array I think Array.from(supportedLocales) would be easier to understand.

Interesting, I like the spread operator more, but that's a matter of taste.

With both ways, I anyhow struggle with the details of iteration (whether it iterates entries - like for Object.entries or Map's default iterator - or only values).

addressed with: c90b9a4

With app descriptor v22 (1.21.0) the i18n sections in the manifest are objects with: * bundleUrl * supportedLocales

adjusted line length

fixed tests after rebase

restored test

Improve i18n language extraction code by avoiding regexp

Add test and comments

Adjust test code

avoid duplicate check for null because createI18nSection() also performs an existence check

changed to Array.from instead of spread operator.

and is reflected as such in the manifest.json "i18n": ""

Add "return" after "reject() call", because a promise should either reject or resovle.

tobiasso85 requested review from codeworrior, matz3 and RandomByte November 20, 2020 16:40

tobiasso85 changed the title ~~[FEATURE] manifestCreator: i18n section v22~~ [FEATURE] manifestCreator: enrich manifest with supportedLocales in i18n Nov 20, 2020

tobiasso85 force-pushed the manifest-creator-supported-locales branch 2 times, most recently from ba9f72b to 745509b Compare December 1, 2020 09:21

ecker changed the title ~~[FEATURE] manifestCreator: enrich manifest with supportedLocales in i18n~~ [FEATURE] manifestCreator: enrich manifest with supportedLocales in i18n (for libraries) Dec 2, 2020

matz3 requested changes Dec 2, 2020

View reviewed changes

tobiasso85 force-pushed the manifest-creator-supported-locales branch from ff9521e to 53986f9 Compare December 23, 2020 09:20

tobiasso85 requested a review from matz3 December 23, 2020 09:58

tobiasso85 added 10 commits January 8, 2021 12:06

[FEATURE] manifestCreator: i18n section v22

a0d9a1a

With app descriptor v22 (1.21.0) the i18n sections in the manifest are objects with: * bundleUrl * supportedLocales

[FEATURE] manifestCreator: i18n section v22

546bc47

adjusted line length

[FEATURE] manifestCreator: i18n section v22

8432621

fixed tests after rebase

[FEATURE] manifestCreator: i18n section v22

c28962b

restored test

[FEATURE] manifestCreator: i18n section v22

ddb4802

Improve i18n language extraction code by avoiding regexp

[FEATURE] manifestCreator: i18n section v22

ba802be

Add test and comments

[FEATURE] manifestCreator: i18n section v22

b561ea7

Adjust test code

[FEATURE] manifestCreator: i18n section v22

1bda4b2

avoid duplicate check for null because createI18nSection() also performs an existence check

[FEATURE] manifestCreator: i18n section v22

d443546

changed to Array.from instead of spread operator.

Adjust logic such that empty node (i18n) corresponds to the empty string

fdec47a

and is reflected as such in the manifest.json "i18n": ""

tobiasso85 force-pushed the manifest-creator-supported-locales branch from b651ad1 to fdec47a Compare January 8, 2021 11:07

tobiasso85 added 3 commits January 8, 2021 12:32

Add test for invalid .library content

6d31f6a

Add "return" after "reject() call", because a promise should either reject or resovle.

remove unreachable code

7fcc042

add tests for manifestCreator to increase coverage

d6e7c64

matz3 approved these changes Jan 14, 2021

View reviewed changes

tobiasso85 merged commit 8102034 into master Jan 14, 2021

tobiasso85 deleted the manifest-creator-supported-locales branch January 14, 2021 13:19

kristian added a commit to kristian/ui5-builder that referenced this pull request Aug 21, 2021

Fix SAP#547, add first test for AbstractBuilder build

11908d0

kristian added a commit to kristian/ui5-builder that referenced this pull request Aug 21, 2021

[FIX] Fixes: SAP#547, add first test for AbstractBuilder build

46176a2

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FEATURE] manifestCreator: enrich manifest with supportedLocales in i18n (for libraries) #547

[FEATURE] manifestCreator: enrich manifest with supportedLocales in i18n (for libraries) #547

tobiasso85 commented Nov 20, 2020 •

edited

coveralls commented Nov 20, 2020 •

edited

matz3 Dec 2, 2020

tobiasso85 Dec 22, 2020

matz3 Dec 2, 2020

tobiasso85 Jan 4, 2021

matz3 Dec 2, 2020

tobiasso85 Dec 23, 2020

matz3 Dec 2, 2020

tobiasso85 Dec 23, 2020

matz3 Dec 2, 2020

codeworrior Dec 2, 2020

tobiasso85 Dec 23, 2020

[FEATURE] manifestCreator: enrich manifest with supportedLocales in i18n (for libraries) #547

[FEATURE] manifestCreator: enrich manifest with supportedLocales in i18n (for libraries) #547

Conversation

tobiasso85 commented Nov 20, 2020 • edited

e.g.

coveralls commented Nov 20, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tobiasso85 commented Nov 20, 2020 •

edited

coveralls commented Nov 20, 2020 •

edited