OK. I would also suggest these changes, but they have more impact: 1. Rename #translate() by #get() or #getMessage() 2. Introduce an interface InternationalizationManager with an implementation DefaultInternationalizationManagerImpl. 3. Separate messages management and I18N conversions. I would use a naming closer of 'message' that 'Internationlization' for InternationalizationManager, e.g.: MessageProvider. And I would move conversion methods, such as #translate(Double doubleValue, Boolean isCurrency) and #translate(Date dateValue, Boolean isTime), into another interface/class, and without using boolean to select format (define additional methods). 4. Allow BaseTelluriumJavaTestCase to change InternationalizationManager implementation 5. Do not store locale in InternationalizationManager, handle it as method parameter. I think it's more flexible 6. Related to #5: make sure Tellurium messages do not share same locale than test locale. If I swap between english and french messages for my tests, I don't want language for Tellurium messages change! Note also that Tellurium messages are easier to understdand in english than in french, even for a french user (some automatic translation ?) :-)
On 2 nov, 18:35, AJ R <[email protected]> wrote: > Hey Syllant > > Thanks for the patch. I will make changes and update the functionality, > Thanks for your input > > Regards > > Ajay Ravichandran > > On Mon, Nov 2, 2009 at 3:18 AM, syllant <[email protected]> wrote: > > > I can't submit a real patch since it would have major impacts on code > > base (InternationalizationManager is already used by several classes) > > and would require more discussion before. > > > Anyway, here is my current quick-and-dirty (cache or proper > > synchronizations are missing) version of InternationalizationManager > > keeping maximal compatibility with trunk > > > --- > > > package org.tellurium.i18n > > > import java.text.MessageFormat > > import org.tellurium.config.Configurable > > > /** > > * InternationalizationManager - provides internationalization support > > * > > * @author Ajay ([email protected]) > > * > > * Date: Sep 23, 2009 > > * > > */ > > > public class InternationalizationManager implements Configurable > > { > > private Locale locale; > > private Set<String> bundleBasenames; > > private Map<Locale, Set<ResourceBundle>> bundles; > > > public InternationalizationManager() > > { > > bundleBasenames = new HashSet<String>(); > > bundleBasenames.add("DefaultMessagesBundle"); > > > bundles = new HashMap<Locale, Set<ResourceBundle>>(); > > locale = Locale.getDefault(); > > } > > > public void setResourceBundles(String ... bundleNames) > > { > > for (String bundleName: bundleNames) > > { > > addResourceBundle(bundleName); > > } > > } > > > public void addResourceBundle(String bundleName) > > { > > bundleBasenames.add(bundleName); > > } > > > protected void checkResourceBundlesLoaded(Locale locale) > > { > > Set<ResourceBundle> bundleSet = bundles.get(locale); > > if (bundleSet == null) > > { > > bundleSet = new HashSet<ResourceBundle>(); > > bundles.put(locale, bundleSet); > > > for (String bundleBasename: bundleBasenames) > > { > > bundleSet.add(ResourceBundle.getBundle(bundleBasename, > > locale)); > > } > > } > > } > > > public String translate(String messageKey, Object ... arguments) > > { > > checkResourceBundlesLoaded(locale); > > > String result = null; > > Set<ResourceBundle> bundleSet = bundles.get(locale); > > for (ResourceBundle bundle: bundleSet) > > { > > try > > { > > result = bundle.getString(messageKey) > > } > > catch (MissingResourceException) > > { > > // ignored > > } > > > if (result != null) > > { > > break; > > } > > } > > > if (result == null) > > { > > throw new MissingResourceException("Can't find resource", > > getClass().getName(), messageKey); > > } > > > return MessageFormat.format(result, arguments); > > } > > > public void setLocale(Locale locale) > > { > > this.locale = locale > > } > > > public Locale getLocale() > > { > > return locale > > } > > > // Kept for compatibility > > public void createDefaultResourceBundle(Locale locale) > > { > > // Already done > > } > > } > > > On 30 oct, 19:56, John <[email protected]> wrote: > > > Thanks Syllant. > > > > Please feel free to post your patch here or send to us directly. We > > > like to see if we can > > > apply the patch to the trunk. > > > > Thanks, > > > > Jian > > > > On Oct 30, 11:13 am, syllant <[email protected]> wrote: > > > > > Thanks for response > > > > > 1. Concerning aggregation: this is what I've done, I have added some > > > > bundles for en/fr locales. But it can't work with current > > > > implementation of InternationalizationManager: when you retrieve > > > > messages, you call getString() on each bundle (seehttp:// > > code.google.com/p/aost/source/browse/trunk/core/src/main/groov...). > > > > But if iterated bundle does not contain key, a > > > > MissingResourceException is thrown and you do not continue with other > > > > bundles! I think you should catch MissingResourceException during > > > > iteration and only throw it if key is not found in all bundles. > > > > Otherwise I don't understand the point of aggregating bundles with > > > > same keys ? > > > > > 2. Concerning setLocale(). This does not work dynamically: if bundles > > > > where loaded with 'en' locale and you call setLocale(Locale.FRENCH) > > > > after, you will not load french bundles. InternationalizationManager > > > > has to store bundles by locales. Maybe you could take a look at Spring > > > > i18N implementations (e.g. : > >https://src.springframework.org/svn/spring-maintenance/trunk/src/org/...) > > > > > 3. Besides, 'translate' is used for retrieving labels AND for > > > > converting date/numbers, which is even more ambiguous > > > > > I've patched InternationalizationManager so it provides features I > > > > need, but I wish I switch back to trunk soon. > > > > > Feel free to contact me if you want to discuss or test some > > > > features :-) > > > > > Sylvain > > > > > On 30 oct, 15:34, AJ R <[email protected]> wrote: > > > > > > Hey Sylvain > > > > > > My reply is inline in blue font below > > > > > > On Fri, Oct 30, 2009 at 5:21 AM, syllant <[email protected]> wrote: > > > > > > > Hi all, > > > > > > > I had reported a request feature about implementing > > > > > > internationalization support ( > >http://code.google.com/p/aost/issues/ > > > > > > detail?id=221&q=i18n), I see it's committed and documented for > > 0.7.0 > > > > > > snaphost(http://code.google.com/p/aost/issues/detail?id=221&q=i18n > > ), > > > > > > great ;-) > > > > > > > However, I have some concerns about implementation: > > > > > > > 1. Management of multiple bundles is incorrect: > > > > > > InternationalizationManager#translate() methods do not handle case > > > > > > when bundle key is not found in some bundles, which makes this > > feature > > > > > > unusable according to me: you can't aggregate distinct messages > > from > > > > > > distinct bundles. This method should handle > > MissingResourceException. > > > > > > Actually you can aggregate distinct messages from different bundles. > > If you > > > > > go to this linkhttp:// > > code.google.com/p/aost/wiki/InternationalizationSupportTellurium > > > > > and scroll down to the section called "Internationalization Extension > > to > > > > > User defined tests" you will find how you can > > > > > add your own message bundle (use a function called > > addResourceBundle). > > > > > > The basic idea is that you can add as many distinct message bundles > > as long > > > > > as they are not called the same, and are not named > > "DefaultMessagesBundle" > > > > > since that is the default message bundle used in the core. > > > > > > Also if you dont find a key, it should throw a > > MissingResourceException. > > > > > > > 2. You don't allow to specify language when asking for a > > translation. > > > > > > It would be useful to be able to pass locale, either by adding > > > > > > optional locale parameter in translate method (translate('msgKey', > > > > > > Locale.FRENCH)), either by providing one > > InternationalizationManager > > > > > > instance by locale > > > > > > Yes this would definitely be a good additional feature to add, which > > I will > > > > > add as soon as I can. Although you can kinda do it right now, if you > > can > > > > > call setLocale() of the InternationalizationManager class. But i will > > > > > definitely add this functionality > > > > > > > 3. I find term 'translate' not totally relevant: it does not > > translate > > > > > > something, it just provides value for specified key. I think these > > > > > > methods should be named with someting like get() > > > > > > My intention of using translate was to signify that the English > > version of > > > > > the message was being translated to non english > > > > > But now that I think about it, your suggestion makes better sense. > > Let me > > > > > see what I can do about this. > > > > > ( translate just sounded cool :-) ) > > > > > > > 4. method BaseTelluriumJavaTestCase#geti18nManager() does not > > respect > > > > > > Java naming standard, it should be getI18nManager() > > > > > > > Yeah i agree, Will update the project soon. > > > > > > 5. It would be useful to allow to set a custom implementation of > > > > > > InternationalizationManager > > > > > > Yep definitely a good idea, will look into it. > > > > > > > Anyway, I go on with 0.7.0 snapshot, with patched implementation of > > > > > > InternationalizationManager for #1 issue. > > > > > > Thanks for trying this feature out. Please do let me know if you face > > any > > > > > problems or any of the functionalities are not working as desired. > > > > > > > Sylvain --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "tellurium-users" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, visit this group at http://groups.google.com/group/tellurium-users?hl=en -~----------~----~----~----~------~----~------~--~---
