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

buxfix(i18n): read utf8 resource if needed. #190 #212

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

khajavi
Copy link
Contributor

@khajavi khajavi commented Dec 30, 2015

No description provided.

JamesSullivan added a commit to JamesSullivan/mamute that referenced this pull request Jan 15, 2016
bugfix(i18n): read utf8 resource if needed. caelum#190
 caelum#212 opened 16 days ago by khajavi
}


private ResourceBundle extractUnsafeBundle(Object bundle, Locale locale) {
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 this code is quite big and complex, I think we should extract it to a separate class to be able to unit test it easier, what do you think @khajavi?

@csokol
Copy link
Contributor

csokol commented Feb 3, 2016

@khajavi thanks for the contribution!

I suppose you've tested this modifications in your machine, but is hard to accept such a complex PR without unit tests.

I've just finished reviewing it and I think you should extract some classes to be able to unit test it easier. What do you think? Maybe some simple tests with properties files and different encodings?

What do you think @xdarklight @JamesSullivan @leocwolter?

@xdarklight
Copy link
Contributor

The standard solution for this issue is to use unicode characters (\uXXXX).
You can use a JDK tool to convert your UTF-8 encoded Persian translation file into ISO-8859-1 with unicode characters with native2ascii (comes with a JDK of your choice): native2ascii -encoding UTF-8 text_utf8.properties text.properties

The downside of this "standard solution" is that the localization file is not editable with a standard text editor anymore (because all non-ASCII characters are encoded), but there are many tools out there - (Attesoro for example - which assist you with editing properties files: they present you the human readable (Persian) text while saving it as ISO-8859-1 with unicode characters.
This works not only for translations but also for the configuration/route properties (with your solution we would have to adjust route and configuration parsing code as well).

I'm not sure what we want to do in Mamute as both solutions have their pros and cons.
But in any case Mamute will get Persian translation thanks to @khajavi 👍

@JamesSullivan
Copy link
Contributor

I think @xdarklight has summed it up quite nicely. If forced to choose I would go with the standard solution, using Unicode characters (\uXXXX), to keep things as simple as possible.

@khajavi
Copy link
Contributor Author

khajavi commented Feb 4, 2016

Writing Unicode translations in ISO-8859-1 standard is cumbersome. I promote to use utf8 bundles for languages that need Unicode strings.

@xdarklight
could you please explain this issue?

This works not only for translations but also for the configuration/route properties (with your solution we would have to adjust route and configuration parsing code as well).

what is the problem with routing and configuration properties?

@xdarklight
Copy link
Contributor

@khajavi see Environment implementation - it's not using MamuteLocalization to load the configuration properties.
I haven't checked the routes implementation but I'm guessing that it's similar there.
You would have to patch everything that loads property files to allow UTF-8 encoded files everywhere.

An alternative to using Attesoro is editing the translations with UTF-8, then using JDK's native2ascii to convert it to ISO-8859-1.
Once you have to update the translations you can use native2ascii -reverse to convert it back to UTF-8, edit it and finally convert it to ascii again.

There's also plenty of web-based translation tools available which can be used instead of manually editing the .properties files. Just to name a few:

  • [http://zanata.org/]
  • [https://weblate.org/en/]
  • [https://www.transifex.com/]

Using these would be a decision which has to be made by the Mamute maintainers.
However, the result would be an ISO-8859-1 properties file - exactly what you get when using Attesoro (or any other tool).

@khajavi please don't get me wrong - your solution definitely has it's upsides!
But to make it work consistently in the whole application you have to customize many other places in the code.

@khajavi
Copy link
Contributor Author

khajavi commented Feb 5, 2016

Hmm, seems you are right. I need some time to fix this issue.

On Thu, Feb 4, 2016 at 10:56 PM, Martin Blumenstingl <
[email protected]> wrote:

@khajavi https://github.com/khajavi see Environment implementation
http:///caelum/vraptor-environment/blob/master/src/main/java/br/com/caelum/vraptor/environment/DefaultEnvironment.java

  • it's not using MamuteLocalization to load the configuration properties.
    I haven't checked the routes implementation but I'm guessing that it's
    similar there.
    You would have to patch everything that loads property files to allow
    UTF-8 encoded files everywhere.

An alternative to using Attesoro is editing the translations with UTF-8,
then using JDK's native2ascii to convert it to ISO-8859-1.
Once you have to update the translations you can use native2ascii -reverse
to convert it back to UTF-8, edit it and finally convert it to ascii again.

There's also plenty of web-based translation tools available which can be
used instead of manually editing the .properties files. Just to name a few:

  • [http://zanata.org/]
  • [https://weblate.org/en/]
  • [https://www.transifex.com/]

Using these would be a decision which has to be made by the Mamute
maintainers.
However, the result would be an ISO-8859-1 properties file - exactly what
you get when using Attesoro (or any other tool).

@khajavi https://github.com/khajavi please don't get me wrong - your
solution definitely has it's upsides!
But to make it work consistently in the whole application you have to
customize many other places in the code.


Reply to this email directly or view it on GitHub
#212 (comment).

Milād Khājavi
http://blog.khajavi.ir
Having the source means you can do it yourself.
I tried to change the world, but I couldn’t find the source code.

@xdarklight
Copy link
Contributor

@khajavi that's exactly what I would suggest you avoid: changing lots of code everywhere.

I just thought about it again and we might be able to mix both approaches:
You main point is that you want to be able to edit the .properties files in any text-editor you want.
On the other hand we should use the Java standard for reading the properties files in the application.
One solution to mix both could be the native2ascii-maven-plugin: this will automatically take care of the character conversion and it's transparent for the application. I'm not sure though how this would work in a development environment (where Mamute is started for example via eclipse) - but maybe you can figure this out :).

@felipeweb
Copy link
Contributor

@khajavi let us know when you finish to make the changes

@khajavi
Copy link
Contributor Author

khajavi commented Feb 13, 2016

Ok, sure.
I'm busy in these days. But I will work on this issue as soon as possible.

On Sat, Feb 13, 2016 at 6:40 PM, Felipe Oliveira [email protected]
wrote:

@khajavi https://github.com/khajavi let us know when you finish to make
the changes


Reply to this email directly or view it on GitHub
#212 (comment).

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