Thanks for the reply John! My comments inlined. On Wed, Aug 26, 2009 at 7:57 PM, John Hjelmstad <[email protected]> wrote:
> Hi Shaopeng: > As I see it, there are two ways to implement this that fit with current > Shindig extension style. The main issue w/ the patch you've implemented is > that it embeds feature-sensitivity into a core piece of rendering logic. We > can use one of the existing mechanisms: > > 1) feature.xml and the JsLibrary/JsFeatureLoader/GadgetFeature system. > We've been considering the addition of language and country-sensitivity to > this mechanism for some time, and this might be the right place to do it. > The net result would be a feature.xml including blocks such as: > <gadget lang="en" country="us"> > <script src="data/DateTimeConstants_en_us.js"/> > <script src="..."/> > > This implementation approach is generic and reusable, both reasons I prefer > it, assuming this would work for the i18n library (perhaps you can say as > well as I; a perusal of the DateTimeConstants*.js files doesn't appear to > map cleanly to lang/country... is there other selection logic needed?). It > involves changes to GadgetFeature.java, JsFeatureLoader.java, > JsLibrary.java, and obviously i18n/feature.xml. > I agree this is a better approach. The i18n library almost map cleanly to lang/country. There are some cases where fallbacks are needed though. For example, there is no en_CN (English as used in China) data file, and this needs to fall back to the en data file. However, this sounds like a pretty significant change, and probably won't be in production until OpenSocial 1.0. That might be too late for developers (like Rajiv) who needs to use this feature now. Probably short term we should provide a quick fix, and go for this approach long term. > > 2) A GadgetRewriter. Essentially, write an OpensocialI18nGadgetRewriter > implementing GadgetRewriter, which does: > - Loads data/DateTimeConstants*.js files. > - In rewriter, injects the appropriate DTC.js when the gadget's features > include "opensocial-i18n". > This is quite similar to what I have been doing in my patch. The difference is instead of extending GadgetRewritter, I changed it directly. My only problem now is how to find the location of the data directory during rendering time. I saw the comments from Paul advising me loading the file directly from the classpath. I will give that a try tomorrow first thing when I get to the office. Hopefully this could work. > > Thoughts? Other Shindig folks? > > --John > > On Wed, Aug 26, 2009 at 12:41 AM, Shaopeng Jia (贾少鹏) < > [email protected]> wrote: > >> Hi shindig-dev, >> As per this bug: >> >> https://issues.apache.org/jira/browse/SHINDIG-1159 >> >> <https://issues.apache.org/jira/browse/SHINDIG-1159>it seems >> gadgets.i18n.DateTimeConstants.js are not outputted during rendering. I did >> some investigation and located the problem is at >> >> >> java/gadgets/src/main/java/org/apache/shindig/gadgets/render/RenderingContentRewriter.java >> >> I have attached a diff which illustrate how to fix the bug, but it is >> loading the DateTimeConstants from my local directory right now. Is there a >> way to get the location of the feature directory on the fly? Or there are >> better ways to load the js code? >> >> Thanks, >> >> Shaopeng >> >> >> -- >> Shaopeng Jia >> 贾少鹏 >> Software Engineer - Îñţérñåţîöñåļîžåţîöñ >> Google Switzerland GmbH |Identifikationsnummer: CH-020.4.028.116-1 >> > > -- Shaopeng Jia 贾少鹏 Software Engineer - Îñţérñåţîöñåļîžåţîöñ Google Switzerland GmbH |Identifikationsnummer: CH-020.4.028.116-1
