WIP: Implemented new gettext-like functions. #321

phaux · 2020-01-29T10:12:42Z

Suggested merge commit message (convention)

Feature: Implemented new gettext-like functions.

coveralls · 2020-01-29T10:16:10Z

Coverage decreased (-2.04%) to 97.964% when pulling 3c8f5a0 on intl-refactor into b148e9c on master.

scofalik · 2020-01-29T11:39:55Z

src/locale.js

+	 * @private
+	 */
+	_ctn( msgctxt = '', msgid = '', msgidPlural = '', value = 0 ) {
+		return this._ct( msgctxt, value == 1 ? msgid : msgidPlural, [ value ] );


Will it work? What kind of JSON structure do you expect if _ct is used like that? How translate works? Assume that the language is Polish (translation keys are in English, of course).

Also, we are giving up a possibility to use multiple placeholders, which is not ideal. How about _ctn( msgctxt = '', msgid = '', msgidPlural = '', quantity = 0, values = null ) and if values is not set, then it becomes [ quantity ]?

Example usage: ctn( 'Sent x of y files', 'file', '%0 of %1 files', 4, [ 1, 4 ] );

I think that translate() function could take quantity as an additional parameter and return a proper string from the JSON language file.

The plural function usually takes only one parameter. I will implement proper plural selection after I change the webpack plugin

scofalik · 2020-01-29T11:41:39Z

src/locale.js

-			translatedString = translatedString.replace( /%(\d+)/g, ( match, index ) => {
-				return ( index < values.length ) ? values[ index ] : match;
-			} );
+		if ( values.some( value => typeof value == 'object' ) ) {


I could argue that Boolean type can be casted to a string, however maybe we could also check if a value is a function? Maybe there would be uses for this functionallity.

scofalik · 2020-01-29T11:43:02Z

src/locale.js

+		 *		editor.ct( 'Created file "x"', 'file "%0"', [ fileName ] );
+		 *
+		 * If at least one of the placeholder values is not a string or number,
+		 * then an array of all the literal and dynamic parts will be returned


. missing at the end of the sentence.

Please expand with an example. It was difficult for me to understand what this means and how I could use it.

scofalik · 2020-01-29T11:43:58Z

src/locale.js

+		 * The strings may contain a placeholder (`%0`) for a given dynamic value.
+		 * The value is also used for selecting the correct plural form.
+		 *
+		 *		editor.ctn( 'Created n files', '%0 file' '%0 files' fileCount );


When you fix docs for ct please check this function docs against ct docs. Some stuff is missing here, AFAICT.

scofalik · 2020-01-29T11:46:15Z

src/locale.js

 		 *
 		 * @method #t
-		 * @param {String} str The string to translate.
+		 * @param {String} msgid The string to translate.
 		 * @param {String[]} [values] Values that should be used to interpolate the string.


Array.<String>

scofalik · 2020-01-29T11:51:37Z

src/locale.js

-	_t( str, values ) {
-		let translatedString = translate( this.uiLanguage, str );
+	_t( msgid = '', values = [] ) {
+		return translate( this.uiLanguage, msgid )


Why not have the same functionallity here and in _ct and _ctn (splitting to array?).

We could move lines 183-200 to a seprate function and use them in _t, _ct and _ctn.

scofalik · 2020-01-29T11:54:43Z

Apart from what I commented:

Add additional blank line before ifs and after ifs.
Docs:
1. use capital letters for types (string => String)
2. use Array.<T> instead of T[]
Tests are missing. Please do a thorough tests.

scofalik · 2020-02-04T14:44:21Z

src/locale.js

+		 * Translates the given string to the {@link #uiLanguage}.
+		 *
+		 * This method allows passing the context as the first argument.
+		 * A context is used to provide a description or example of how the translated string will be used.


Suggested change

* A context is used to provide a description or example of how the translated string will be used.

* The context is used to provide a description or example of how the translated string will be used.

scofalik · 2020-02-04T14:44:30Z

src/locale.js

+		 *
+		 * This method allows passing the context as the first argument.
+		 * A context is used to provide a description or example of how the translated string will be used.
+		 * An unique context makes the whole translation unique.


Suggested change

* An unique context makes the whole translation unique.

* A unique context makes the whole translation unique.

scofalik · 2020-02-04T14:47:13Z

src/locale.js

 		 *
-		 *		editor.t( 'Created file "%0" in %1ms.', [ fileName, timeTaken ] );
+		 * This method allows passing the context as the first argument.
+		 * A context is used to provide a description or example of how the translated string will be used.


scofalik · 2020-02-04T15:07:25Z

src/locale.js

 		 */
-		this.t = ( ...args ) => this._t( ...args );
+		this.ctn = ( msgctxt, msgid, msgidPlural, quantity ) => this._ctn( msgctxt, msgid, msgidPlural, quantity );


How about we name this function ctq? Context-translation-quantity?

scofalik · 2020-02-04T15:08:12Z

src/locale.js

-		 * `<index>` is the index in the `values` array.
+		 * @method #t
+		 * @param {String} msgid The string to translate.
+		 * @returns {Message}


Does it return Message or String?

I think it can return Message, if it is stringify-able easily. But then we need to change t() function definition.

scofalik · 2020-02-04T15:15:22Z

src/locale.js

+	 * Get the interpolated message as string.
+	 * This method will replace the placeholders in the message with provided values.
+	 *
+	 * @example


I haven't seen us using those tags before, so make sure they are correctly parsed by Umberto into API docs.

scofalik · 2020-02-04T15:20:14Z

src/locale.js

+	 * and returns an array of the original string parts with provided values interspersed.
+	 * The values can be of any type and won't be stringified.
+	 *
+	 * @example


I'd like to see an example with non-strings here.

ma2ciek · 2020-02-14T09:09:06Z

src/locale.js

+ */
+export class Message {
+	/**
+	 * @hidden


What's the hidden tag? I'm afraid we don't use it anywhere. And I couldn't find it in https://jsdoc.app/ too.

ma2ciek · 2020-02-14T09:12:40Z

src/locale.js

+	 *
+	 * 		expect( msg ).to.equal( [ 'Replace ', 'foo', ' with ', 'bar' ] )
+	 *
+	 * @template T


I'm also unsure about the generic @template tag. Currently we don't use it anywhere. I know it's a very valuable thing, though it can be unsupported currently by Uberto and our JSDoc plugins 😞

ma2ciek · 2020-04-17T13:26:54Z

Closing in favor of #334.

scofalik reviewed Jan 29, 2020

View reviewed changes

scofalik suggested changes Feb 4, 2020

View reviewed changes

ma2ciek reviewed Feb 14, 2020

View reviewed changes

phaux added 6 commits February 24, 2020 15:17

Implemented new gettext-like functions.

243f071

Added separate message format functions.

e95f97d

Added formatToParts for translations.

3291070

Added new translation lookup functions.

46a3a56

Fixed NaN quantity.

c7fc680

Fixed legacy translations not working.

351a1a0

phaux force-pushed the intl-refactor branch from 09545d2 to 351a1a0 Compare February 25, 2020 12:36

phaux added 2 commits February 25, 2020 13:50

Tests.

b452b0c

Tests again.

3c8f5a0

ma2ciek closed this Apr 17, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: Implemented new gettext-like functions. #321

WIP: Implemented new gettext-like functions. #321

phaux commented Jan 29, 2020 •

edited by scofalik

Loading

coveralls commented Jan 29, 2020 •

edited

Loading

scofalik Jan 29, 2020

scofalik Jan 29, 2020 •

edited

Loading

phaux Jan 29, 2020

scofalik Jan 29, 2020

scofalik Jan 29, 2020

scofalik Jan 29, 2020

scofalik Jan 29, 2020 •

edited

Loading

scofalik Jan 29, 2020

scofalik commented Jan 29, 2020 •

edited

Loading

scofalik Feb 4, 2020

scofalik Feb 4, 2020

scofalik Feb 4, 2020

scofalik Feb 4, 2020

scofalik Feb 4, 2020

scofalik Feb 4, 2020

scofalik Feb 4, 2020

scofalik Feb 4, 2020

ma2ciek Feb 14, 2020

ma2ciek Feb 14, 2020 •

edited

Loading

ma2ciek commented Apr 17, 2020

	* A context is used to provide a description or example of how the translated string will be used.
	* The context is used to provide a description or example of how the translated string will be used.

	* An unique context makes the whole translation unique.
	* A unique context makes the whole translation unique.

WIP: Implemented new gettext-like functions. #321

WIP: Implemented new gettext-like functions. #321

Conversation

phaux commented Jan 29, 2020 • edited by scofalik Loading

Suggested merge commit message (convention)

coveralls commented Jan 29, 2020 • edited Loading

Choose a reason for hiding this comment

scofalik Jan 29, 2020 • edited Loading

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

scofalik Jan 29, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

scofalik commented Jan 29, 2020 • edited Loading

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

ma2ciek Feb 14, 2020 • edited Loading

Choose a reason for hiding this comment

ma2ciek commented Apr 17, 2020

phaux commented Jan 29, 2020 •

edited by scofalik

Loading

coveralls commented Jan 29, 2020 •

edited

Loading

scofalik Jan 29, 2020 •

edited

Loading

scofalik Jan 29, 2020 •

edited

Loading

scofalik commented Jan 29, 2020 •

edited

Loading

ma2ciek Feb 14, 2020 •

edited

Loading