Skip to content
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

Add support for the IntlMessageFormatter #1269

Merged
merged 3 commits into from
Jan 12, 2019

Conversation

digilist
Copy link
Contributor

@digilist digilist commented Jan 6, 2019

This adds support for the new ICU Message Format that was introduced with Symfony 4.2. I tested the plugin and I was getting auto completion, could insert new keys directly from template and got suggestions for the available placeholders.

The parsing of the available placeholders is rather simple and looks only for {var} and not for more complex ICU formats.

One issue I am aware of, is that Symfony registers registers the contents of a message+intl-icu file inside the message and message+intl-icu domains. So there are actually two domains, even though the intl-icu translations are append to the main domain. However, I do not see any reason why someone would access the intl-icu translation directly and loose the formatting, so I didn't consider this case.

Further information:
https://symfony.com/blog/new-in-symfony-4-2-intlmessageformatter
symfony/symfony#28952
#1251


// Remove +intl-icu suffix, as it is not part of the domain
// https://symfony.com/blog/new-in-symfony-4-2-intlmessageformatter
domain = domain.replace("+intl-icu", "");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually there are two domains now, one with the suffix and one without it. However, I do not think this is really necessary, when using the +intl-icu domain directly the IntlFormatter will not be used.

Copy link
Owner

Choose a reason for hiding this comment

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

maybe check for endWiths "+intl-icu" and then remove the ending chars, makes it more tolerate to naming

// Keep the whole placeholder for consistency with other formats, but also add just the placeholder name
// as this is allowed with the ICU format.
placeholder.add(matcher.group(1));
placeholder.add(matcher.group(1).replace("{", "").replace("}", "").trim());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this okay or should we only keep the placeholder without the braces?

Copy link
Owner

Choose a reason for hiding this comment

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

i am not sure what is a better approach for now. so just put in both i suggest, maybe we get some feedback after we released it

@Haehnchen
Copy link
Owner

good work. i think navigation is not working, but should be in a new pr. i will merge this during this week.

navigation scope:

fr.adrienbrault.idea.symfony2plugin.translation.dict.TranslationUtil#getTranslationPsiElements
fr.adrienbrault.idea.symfony2plugin.translation.dict.TranslationUtil#getTargetForXlfAsXmlFile

@digilist digilist force-pushed the icu_message_format branch from 53e82dc to 567f030 Compare January 8, 2019 08:36
@digilist
Copy link
Contributor Author

digilist commented Jan 8, 2019

Thx, I updated the code accordingly. I think, this can be merged then.

Regarding the navigation scope, I do not know yet what that is exactly, it's my first dive into IDEA Plugins. If you can give me some hints, I'd like to try to fix it :)

@Haehnchen Haehnchen merged commit aba9581 into Haehnchen:master Jan 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants