Skip to content

[Translator] Adding support for intl message formatter and decoupling default formatter. #15068

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.

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

Closed
wants to merge 2 commits into from

Conversation

aitboudad
Copy link
Contributor

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? yes
Tests pass? yes
Fixed tickets #6009, #10152, one item in #11742, #11948
License MIT
Doc PR n/a

@aitboudad
Copy link
Contributor Author

ping @symfony/deciders

*/
public function format($locale, $id, $number = null, array $arguments = array())
{
$pattern = ($number !== null)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be on one line, without the parenthesis around the condition and null should be on the left.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fabpot can you explain why null should be on the left ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dupuchba in Symfony we use Yoda conditions (https://en.wikipedia.org/wiki/Yoda_conditions)

@stof
Copy link
Member

stof commented Jul 1, 2015

I see a big issue with this implementation: the symfony formatting and the Intl formatting are totally different, and your implementation expects to choose one of them only. But the whole bundle ecosystem relies on the Symfony formatting (including validation translations in Symfony itself). This means that the only way to use the intl formatter is to stop using any third-party translations (you have to provide them again in the intl format), which does not give us any chance to deprecate the old system in favor of intl formatting (which is indeed much more powerful than the simple transchoice format of Symfony).

What I would like to see (if it is possible) is a way to use both formatters in parallels (some messages using the intl format, and other messages using the symfony format), to allow progressive migration.

@aitboudad aitboudad force-pushed the translation_intl_formatter branch 2 times, most recently from 0b031d4 to d3e208c Compare July 1, 2015 12:29
@aitboudad
Copy link
Contributor Author

@stof well what do you think to add a converter that convet the lagacy message into intl format when the IntlMessageFormatter was chosen ?

@stof
Copy link
Member

stof commented Jul 1, 2015

Well, if such conversion is possible, it would be a great news for the migration to the intl format. but the implementation needs to be careful to be able to load files providing both.
but there is another case which needs to be converted if needed: currently, the translation uses the key passed as argument as translated string in case the catalogue does not have it. this should take care of the conversion when needed too.
and the conversion is not only about pluralization, but also about placeholders.

However, if we want to switch to the IntlMessageFormatter (which seems a good idea to me), we would need to provide a stub implementation of the IntlMessageFormatter to avoid the hard dependency on intl just for that (and this stub implementation should probably be able to support rules for any locale, like our existing MessageSelector, not only for en, otherwise we would still rely on intl)

@aitboudad aitboudad force-pushed the translation_intl_formatter branch from d3e208c to 973eac2 Compare August 27, 2015 10:39
@aitboudad aitboudad force-pushed the translation_intl_formatter branch 2 times, most recently from 599dfcf to d6335b4 Compare September 11, 2015 09:47
@aitboudad
Copy link
Contributor Author

@stof I added LegacyIntlMessageFormatter to fallback the legacy messages (seems it work as expected) + deprecated transChoice.
can you review it please ?

@aitboudad aitboudad force-pushed the translation_intl_formatter branch from d6335b4 to 42a69ec Compare September 11, 2015 10:10
@aitboudad aitboudad force-pushed the translation_intl_formatter branch 4 times, most recently from 210c7a9 to 4f20f58 Compare September 11, 2015 11:28
@aitboudad aitboudad force-pushed the translation_intl_formatter branch from 4f20f58 to 86189f6 Compare September 14, 2015 09:14
@aitboudad aitboudad force-pushed the translation_intl_formatter branch from 86189f6 to 49dbc6a Compare September 14, 2015 09:23
@aitboudad
Copy link
Contributor Author

@symfony/deciders is there anyone interested to provide a stub implementation of the IntlMessageFormatter ?

@aitboudad
Copy link
Contributor Author

closing in favor of #18314, the IntlFormatter will be add once #18314 merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants