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

support for combining parameters and apostrophe #356

Merged

Conversation

chenejac
Copy link
Contributor

VIVO GitHub issue: 3393

Please, find more about the issue also at this discussion

What does this pull request do?

Add support for combining apostrophe and parameters:
Taille maximale de l'image: {0} mégaoctets

It might be resolved in i18n label files by replacing ' with '', and in some cases it has been done, but not in all cases. However, this might be also resolved in java files when we are invoking MessageFormat.format method. This PR should support both cases
Taille maximale de l'image: {0} mégaoctets
and
Taille maximale de l''image: {0} mégaoctets

What's new?

In the invocation of MessageFormat.format method, firstly all double apostrophe ('') are replaced with single, then single apostrophe is replaced with double. At the end, '' and ' are ''

How should this be tested?

  • Run VIVO application
  • Log in
  • Open a researcher profile page
  • Click on uploading the image
  • Change language to French
  • Check labels Taille maximale de l'image: 6 mégaoctets, Taille minimale de l'image: 200 par 200 pixels

Interested parties

Tag (@ mention) interested parties or, if unsure, @VIVO-project/vivo-committers

@@ -165,7 +165,7 @@ private static String formatString(String textString, Object... parameters) {
if (parameters.length == 0) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

While I was debugging I noticed that in some cases when {0} are used in the label translation, parameters.length is zero here, but later in I18nStringTemplateModel it is replaced as it should be (for instance for Taille minimale de l'image: {0} par {1} pixels). I hope @litvinovg is aware of this, and this is working in accordance with his expectations.

* @return
*/
private String getOnlineTranslationsFormattedMessage(String preProcessed, Object[] args) {
String[] parts = preProcessed.split(I18nBundle.INT_SEP);
final int messageIndex = parts.length -1;
String message = MessageFormat.format(parts[messageIndex], args);
String message = MessageFormat.format(parts[messageIndex].replaceAll("''", "'").replaceAll("'", "''"), args);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@litvinovg please check do we need this here

Copy link
Contributor

Choose a reason for hiding this comment

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

I think replace should be used instead of replaceAll as there is no regex used.
I suggest to extract method that does it instead of inline fixes.

… input strings for MethodFormat.format method
Copy link
Contributor

@litvinovg litvinovg left a comment

Choose a reason for hiding this comment

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

Fixes the issue.
Looks good.

Copy link

@mmannings mmannings left a comment

Choose a reason for hiding this comment

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

Fixed.

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