[fyi] [icedtea-web] Czech locales and test for them

Jiri Vanek jvanek at redhat.com
Fri Nov 2 07:49:57 PDT 2012


On 11/02/2012 11:02 AM, Pavel Tisnovsky wrote:
> Hi Jiri,
>
> these sort of patches are really welcome and I'm looking forward for more localizations!
>
> Some comments:
>
> 1) The property file seems to use<CR><LF>  line ending which is not incorrect from the Java side,
> but it might confuse other developers&  patch&  diff tools (and you are going to add some new
> tests written in BASH, are not you?). AFAIK it's safer to use Unix-like line endings.

arg. Big failure. fixed

>
> 2) Some questionable translations:
> +Password=Heslo:<- should be ':' here?
> +Field=Pole -<- "Vstupní pole"?
> +LFatalVerificationInfo=<- missing value

All three are following original in best way.

>
> 3) The following snippet is not incorrect but it is quite unusual:
> +        int i = -1;
> +        for (Iterator<Entry<String, String>>  it = rr.iterator(); it.hasNext();) {
> +            i++;
> Why not to use standard idiom int i=0; before the loop and i++ inside to "for" statement itself:
> ...for (Iterator<Entry<String, String>>  it = rr.iterator(); it.hasNext(); i++)...

as you wish. done.

>
> 4) private void iteratePropertiesFor(ResourceBundle props, String s, boolean b, String id) {
> Well I know this is a private method, but it would be better to rename the parameters
> (what the "s" and especially "b" means?). And in general it would be great to add short
> JavaDoc to all methods :)

sure. Done !

>
> 5) I would prefer to use Locale parameter for the method getProperties(). You can then
> get instance of Locale class by passing country+language pair to the Locale constructor:
> language - lowercase two-letter ISO-639 code
> country - uppercase two-letter ISO-3166 code
>
> +    public ResourceBundle getPropertiesCz() throws IOException {
> +        return getProperties("_cs_CZ");
> +
> +    }

Ihave intentionaly used direct acces to resource. I do not wan to have middle man here between resources and test. I would really like to keep it as it is.
>
> 6) iteratePropertiesForAproxCz/iteratePropertiesFor:
> better name for an "x" variable should be keysFound or something like it

fixed.

>
> 7) method regexIt()&  String[] czEvil - well this is specific to systems *not* set to UTF-8 *and* using CZ
> at the same time (Windows and its stupid Windows-1250 code page?). AFAIK it would be better to use "\uXXXX" Unicode
> literals in the array and refactor regexIt() for other languages too (ie you should pass czEvil array
> to this method as an another parameter). And please add JavaDoc to regexId() - it took me 2-3 minutes
> to figure out this method behavior and it could be described in one sentence + one example :)

Yash. sorry for it. Javadocs added.
>
> 8) value = value.replace(string.toUpperCase(), "."); you need to use Locale here

You do not need:) This method is working quite well for all except  some really unusual fonts.

>
> Cheers,
> Pavel
>
>
>
> ----- Jiri Vanek<jvanek at redhat.com>  wrote:
>> Hi!
>> Here are the locales Alexander have sent me yesterday. I have added also small test to ensure they proceed correctly.
>> Should I add him to Authors? (As person who helped me with design of splash was forbiden to be add)
>> Should locales be mentioned in news? (I guess yes. So I will :
>>
>>    New in release 1.4 (2012-XX-XX):
>> +* Integrated cs_CZ internationalization
>>    * Security updates
>>
>> Cheers,
>>     J.
>>
>>
>> ps: sorry for not including news and authors directly in patch, but I have rest of this patch on offline machine:-/
>>
>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: czCsLocale2.patch
Type: text/x-patch
Size: 44321 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20121102/cd1398bd/czCsLocale2.patch 


More information about the distro-pkg-dev mailing list