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

Pavel Tisnovsky ptisnovs at redhat.com
Fri Nov 2 08:19:01 PDT 2012


----- Jiri Vanek <jvanek at redhat.com> wrote:
> 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
> 
TY

> >
> > 2) Some questionable translations:
> > +Password=Heslo:<- should be ':' here?
> > +Field=Pole -<- "Vstupní pole"?
> > +LFatalVerificationInfo=<- missing value
> 
> All three are following original in best way.
> 
Ok, I compared it with original eng. msgs and it seems ok

> >
> > 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.

:-) TY

> 
> >
> > 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.

Well it's not a big issue but then you need to make sure that every strings sent
to getProperties are really correct.

> >
> > 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.

great!

> >
> > 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.

well it's not related to fonts but to algorithm used to find uppercase<->lowercase pair. It's fairly
easy to to this in case of EN and CZ, but in case of more complicated languages it's quite strange
because it can change length of the string etc. etc. But I'm ok if you are aware of it.

Good from my side, thank you,
Pavel

> 
> >
> > 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:-/
> >>
> >
> 




More information about the distro-pkg-dev mailing list