Add translation support via XTB. Makes soy use goog.get...

Add translation support via XTB. Makes soy use goog.getMsg and converts the
extract command to output an XTB file based on extracted JsMessages. Allows
--language to be set at compile time, and also to be passed in during serve
time, including via referer.

BTW, I do plan on adding XLIFF support here, but those changes will be
mostly incremental on top of this base. Basically I need to either use
XliffGenerator or write my own in the extract thing, and create an
XliffMessageBundle that implements the right stuff. I think it's
achievable.

[alexv, 2012-09-20 09:31:07]

My 2 cents.

http://codereview.appspot.com/6194048/diff/1/src/org/plovr/cli/ExtractCommand...
File src/org/plovr/cli/ExtractCommand.java (right):

http://codereview.appspot.com/6194048/diff/1/src/org/plovr/cli/ExtractCommand...
src/org/plovr/cli/ExtractCommand.java:58: new GoogleJsMessageIdGenerator(null),
JsMessage.Style.CLOSURE);
Consider using a project ID for new GoogleJsMessageIdGenerator(). Maybe, take it
from plovr config id?

http://codereview.appspot.com/6194048/diff/1/src/org/plovr/cli/ExtractCommand...
src/org/plovr/cli/ExtractCommand.java:59: System.out.println("<translationbundle
lang=\"REPLACE_ME\">");
Why not get language from the config instead of REPLACE_ME?

http://codereview.appspot.com/6194048/diff/1/src/org/plovr/cli/ExtractCommand...
src/org/plovr/cli/ExtractCommand.java:81: //   out.append("<desc>" +
message.getDesc() + "</desc>\n");
should desc be an attribute? <translation id="..." desc="...">.
This might help:
http://cldr.unicode.org/development/development-process/design-proposals/xmb

http://codereview.appspot.com/6194048/diff/1/src/org/plovr/cli/ExtractCommand...
src/org/plovr/cli/ExtractCommand.java:89: out.append("<ph name=\"" +
((JsMessage.PlaceholderReference)part).getName() + "\"/>");
80 chars would be nice.

http://codereview.appspot.com/6194048/diff/1/src/org/plovr/cli/ExtractCommand...
src/org/plovr/cli/ExtractCommand.java:89: out.append("<ph name=\"" +
((JsMessage.PlaceholderReference)part).getName() + "\"/>");
If I'm not mistaken, Compiler expects <ph name="..."> to be upper case (e.g.
ph.getName().toUpperCase()). I might we wrong though, but I think it won't be
replaced properly when Soy templates are compiled with
--shouldGenerateGoogMsgDefs.
Also see this example:
http://code.google.com/p/grit-i18n/source/browse/trunk/grit/testdata/generate...

http://codereview.appspot.com/6194048/diff/1/testdata/translation/config.js
File testdata/translation/config.js (right):

http://codereview.appspot.com/6194048/diff/1/testdata/translation/config.js#n...
testdata/translation/config.js:9: "language": "en"
How about adding something like "languages": ["en", "de", "es"], so that when
you build, it'd do it for all needed locales. I'm not saying to eliminate
"language", as it's nice to be able to give as a parameter in test-simple.html

[alexv, 2012-09-20 15:12:51]

http://codereview.appspot.com/6194048/diff/1/src/org/plovr/cli/ExtractCommand...
File src/org/plovr/cli/ExtractCommand.java (right):

http://codereview.appspot.com/6194048/diff/1/src/org/plovr/cli/ExtractCommand...
src/org/plovr/cli/ExtractCommand.java:59: System.out.println("<translationbundle
lang=\"REPLACE_ME\">");
On 2012/09/20 09:31:07, crhyme wrote:
> Why not get language from the config instead of REPLACE_ME?

Sorry, what I really meant was "languages". See my comment in config.js.

[Ilia Mirkin, 2012-11-11 19:29:57]

https://codereview.appspot.com/6194048/diff/1/src/org/plovr/cli/ExtractComman...
File src/org/plovr/cli/ExtractCommand.java (right):

https://codereview.appspot.com/6194048/diff/1/src/org/plovr/cli/ExtractComman...
src/org/plovr/cli/ExtractCommand.java:58: new GoogleJsMessageIdGenerator(null),
JsMessage.Style.CLOSURE);
On 2012/09/20 09:31:07, alexv wrote:
> Consider using a project ID for new GoogleJsMessageIdGenerator(). Maybe, take
it
> from plovr config id?

I didn't quite understand what these project ids were good for. It seems like
they're included as part of the fp, which is both good and bad. If you ever
change your project name, it'll mess up all your translations, which is not
super-great.

How about I add a parameter in the config?

https://codereview.appspot.com/6194048/diff/1/src/org/plovr/cli/ExtractComman...
src/org/plovr/cli/ExtractCommand.java:59: System.out.println("<translationbundle
lang=\"REPLACE_ME\">");
On 2012/09/20 09:31:07, alexv wrote:
> Why not get language from the config instead of REPLACE_ME?

It's not language-specific in any way. If you wanted to generate translations
for 100 languages, your starting file, generated by this command, would still be
the same.

https://codereview.appspot.com/6194048/diff/1/src/org/plovr/cli/ExtractComman...
src/org/plovr/cli/ExtractCommand.java:81: //   out.append("<desc>" +
message.getDesc() + "</desc>\n");
On 2012/09/20 09:31:07, alexv wrote:
> should desc be an attribute? <translation id="..." desc="...">.
> This might help:
> http://cldr.unicode.org/development/development-process/design-proposals/xmb

I have no idea where I got these from. I'm pretty sure I didn't invent them, but
also XtbTranslationBundle doesn't use them. Perhaps I saw references to XTB in
other google projects.

I'm happy to drop these in whereever, as long as they don't upset the XTB parser
in the compiler.

https://codereview.appspot.com/6194048/diff/1/src/org/plovr/cli/ExtractComman...
src/org/plovr/cli/ExtractCommand.java:89: out.append("<ph name=\"" +
((JsMessage.PlaceholderReference)part).getName() + "\"/>");
On 2012/09/20 09:31:07, alexv wrote:
> 80 chars would be nice.

Yeah, occasionally I get lazy when I'm in mid-debug, and then I forget to
fix-up. Should be better now.

https://codereview.appspot.com/6194048/diff/1/src/org/plovr/cli/ExtractComman...
src/org/plovr/cli/ExtractCommand.java:89: out.append("<ph name=\"" +
((JsMessage.PlaceholderReference)part).getName() + "\"/>");
On 2012/09/20 09:31:07, alexv wrote:
> If I'm not mistaken, Compiler expects <ph name="..."> to be upper case (e.g.
> ph.getName().toUpperCase()). I might we wrong though, but I think it won't be
> replaced properly when Soy templates are compiled with
> --shouldGenerateGoogMsgDefs.
> Also see this example:
>
http://code.google.com/p/grit-i18n/source/browse/trunk/grit/testdata/generate...

Yep, something like that. Fixed to do the reverse of what the XTB parser does on
input.

https://codereview.appspot.com/6194048/diff/1/testdata/translation/config.js
File testdata/translation/config.js (right):

https://codereview.appspot.com/6194048/diff/1/testdata/translation/config.js#...
testdata/translation/config.js:9: "language": "en"
On 2012/09/20 09:31:07, alexv wrote:
> How about adding something like "languages": ["en", "de", "es"], so that when
> you build, it'd do it for all needed locales. I'm not saying to eliminate
> "language", as it's nice to be able to give as a parameter in test-simple.html

Then this runs into the same issue as modules -- how to output multiple files.
And if you are using modules, you get into a NxM situation.

This wants some sort of generic solution, which IMO is beyond the scope of this
change.

[alexv, 2012-11-11 23:55:42]

Hey, LGTM FWIW but I'm not that familiar with plovr internals, just wanted to
contribute to the review - it's been sitting there for 6 months.

https://codereview.appspot.com/6194048/diff/1/src/org/plovr/cli/ExtractComman...
File src/org/plovr/cli/ExtractCommand.java (right):

https://codereview.appspot.com/6194048/diff/1/src/org/plovr/cli/ExtractComman...
src/org/plovr/cli/ExtractCommand.java:58: new GoogleJsMessageIdGenerator(null),
JsMessage.Style.CLOSURE);
On 2012/11/11 19:29:57, Ilia Mirkin wrote:
> On 2012/09/20 09:31:07, alexv wrote:
> > Consider using a project ID for new GoogleJsMessageIdGenerator(). Maybe,
take
> it
> > from plovr config id?
> 
> I didn't quite understand what these project ids were good for. It seems like
> they're included as part of the fp, which is both good and bad. If you ever
> change your project name, it'll mess up all your translations, which is not
> super-great.
> 
> How about I add a parameter in the config?

I think you're right. I'm using it so that my translation bundles don't get
messed up by (possibly) the same IDs generated from diff. projects. Plus, the
compiler needs --translations_project option. OTOH I've never tried to compile
without the project option, so it probably depends on how many folks would
actually use this feature.

[akito_netbrains.com, 2012-11-29 17:53:28]

I was going to stick post a patch here but was having the hardest time 
creating a patch so I'm copy and pasting.

I rewrote part of you're code to use the javax.xml. instead of hand writing 
the xml.

Added... desc and xml encoding comes for free using a tool.

 Element translationbundle = document.createElement("translationbundle");
      translationbundle.setAttribute("lang", "REPLACE_ME");
      document.appendChild(translationbundle);

        for (JsMessage message : 
extractor.extractMessages(Iterables.transform(inputs,
                new Function<JsInput, SourceFile>() {

                    @Override
                    public SourceFile apply(JsInput input) {
                        return SourceFile.fromGenerator(input.getName(), 
input);
                    }
                }))) {
            translationbundle.appendChild(document.createTextNode("\n\t"));

            Element translation = document.createElement("translation");
            translationbundle.appendChild(translation);
            translation.setAttribute("id", message.getId());
            translation.setAttribute("desc", message.getDesc());

            formatMessage(document, message, translation);
        }
        translationbundle.appendChild(document.createTextNode("\n"));

        TransformerFactory tf = TransformerFactory.newInstance();
        Transformer transformer = null;
        try {
            transformer = tf.newTransformer();
            transformer.setOutputProperty(OutputKeys.OMIT_XML_DECLARATION, 
"yes");
            StringWriter writer = new StringWriter();
            transformer.transform(new DOMSource(document), new 
StreamResult(writer));
            String output = writer.getBuffer().toString();
            System.out.println(output);
        } catch (TransformerConfigurationException e) {
            e.printStackTrace(); // To change body of catch statement use 
File | Settings | File Templates.
        } catch (TransformerException e) {
            e.printStackTrace(); // To change body of catch statement use 
File | Settings | File Templates.
        }
        return 0;
    }

    private void formatMessage(Document document, JsMessage message, 
Element translation) {
        for (CharSequence part : message.parts()) {
            if (part instanceof JsMessage.PlaceholderReference) {
                // Placeholder References need to be stored in
                // UPPER_UNDERSCORE format, with some exceptions. See
                // JsMessageVisitor.toLowerCamelCaseWithNumericSuffixes for
                // details.
                String phName = 
toUpperUnderscoreWithNumbericSuffixes(((JsMessage.PlaceholderReference) 
part).getName());
                Element ph = document.createElement("ph");
                ph.setAttribute("name", phName);

                translation.appendChild(ph);
            } else {
                
translation.appendChild(document.createTextNode(part.toString()));
            }
        }
    }


On Sunday, November 11, 2012 11:29:57 AM UTC-8, Ilia Mirkin wrote:
>
>
>
>
https://codereview.appspot.com/6194048/diff/1/src/org/plovr/cli/ExtractComman...

> File src/org/plovr/cli/ExtractCommand.java (right): 
>
>
https://codereview.appspot.com/6194048/diff/1/src/org/plovr/cli/ExtractComman...

>
>
src/org/plovr/cli/ExtractCommand.java:58<https://codereview.appspot.com/6194048/diff/1/src/org/plovr/cli/ExtractCommand.java#newcode58src/org/plovr/cli/ExtractCommand.java:58>:

> new 
> GoogleJsMessageIdGenerator(null), JsMessage.Style.CLOSURE); 
> On 2012/09/20 09:31:07, alexv wrote: 
> > Consider using a project ID for new GoogleJsMessageIdGenerator(). 
> Maybe, take it 
> > from plovr config id? 
>
> I didn't quite understand what these project ids were good for. It seems 
> like they're included as part of the fp, which is both good and bad. If 
> you ever change your project name, it'll mess up all your translations, 
> which is not super-great. 
>
> How about I add a parameter in the config? 
>
>
https://codereview.appspot.com/6194048/diff/1/src/org/plovr/cli/ExtractComman...

>
>
src/org/plovr/cli/ExtractCommand.java:59<https://codereview.appspot.com/6194048/diff/1/src/org/plovr/cli/ExtractCommand.java#newcode59src/org/plovr/cli/ExtractCommand.java:59>:

>
> System.out.println("<translationbundle lang=\"REPLACE_ME\">"); 
> On 2012/09/20 09:31:07, alexv wrote: 
> > Why not get language from the config instead of REPLACE_ME? 
>
> It's not language-specific in any way. If you wanted to generate 
> translations for 100 languages, your starting file, generated by this 
> command, would still be the same. 
>
>
https://codereview.appspot.com/6194048/diff/1/src/org/plovr/cli/ExtractComman...

>
>
src/org/plovr/cli/ExtractCommand.java:81<https://codereview.appspot.com/6194048/diff/1/src/org/plovr/cli/ExtractCommand.java#newcode81src/org/plovr/cli/ExtractCommand.java:81>:

> //   out.append("<desc>" + 
> message.getDesc() + "</desc>\n"); 
> On 2012/09/20 09:31:07, alexv wrote: 
> > should desc be an attribute? <translation id="..." desc="...">. 
> > This might help: 
>
>
> http://cldr.unicode.org/development/development-process/design-proposals/xmb 
>
> I have no idea where I got these from. I'm pretty sure I didn't invent 
> them, but also XtbTranslationBundle doesn't use them. Perhaps I saw 
> references to XTB in other google projects. 
>
> I'm happy to drop these in whereever, as long as they don't upset the 
> XTB parser in the compiler. 
>
>
https://codereview.appspot.com/6194048/diff/1/src/org/plovr/cli/ExtractComman...

>
>
src/org/plovr/cli/ExtractCommand.java:89<https://codereview.appspot.com/6194048/diff/1/src/org/plovr/cli/ExtractCommand.java#newcode89src/org/plovr/cli/ExtractCommand.java:89>:

> out.append("<ph name=\"" + 
> ((JsMessage.PlaceholderReference)part).getName() + "\"/>"); 
> On 2012/09/20 09:31:07, alexv wrote: 
> > 80 chars would be nice. 
>
> Yeah, occasionally I get lazy when I'm in mid-debug, and then I forget 
> to fix-up. Should be better now. 
>
>
https://codereview.appspot.com/6194048/diff/1/src/org/plovr/cli/ExtractComman...

>
>
src/org/plovr/cli/ExtractCommand.java:89<https://codereview.appspot.com/6194048/diff/1/src/org/plovr/cli/ExtractCommand.java#newcode89src/org/plovr/cli/ExtractCommand.java:89>:

> out.append("<ph name=\"" + 
> ((JsMessage.PlaceholderReference)part).getName() + "\"/>"); 
> On 2012/09/20 09:31:07, alexv wrote: 
> > If I'm not mistaken, Compiler expects <ph name="..."> to be upper case 
> (e.g. 
> > ph.getName().toUpperCase()). I might we wrong though, but I think it 
> won't be 
> > replaced properly when Soy templates are compiled with 
> > --shouldGenerateGoogMsgDefs. 
> > Also see this example: 
>
>
>
http://code.google.com/p/grit-i18n/source/browse/trunk/grit/testdata/generate...

>
> Yep, something like that. Fixed to do the reverse of what the XTB parser 
> does on input. 
>
>
> https://codereview.appspot.com/6194048/diff/1/testdata/translation/config.js 
> File testdata/translation/config.js (right): 
>
>
https://codereview.appspot.com/6194048/diff/1/testdata/translation/config.js#...

>
>
testdata/translation/config.js:9<https://codereview.appspot.com/6194048/diff/1/testdata/translation/config.js#newcode9testdata/translation/config.js:9>:

> "language": "en" 
> On 2012/09/20 09:31:07, alexv wrote: 
> > How about adding something like "languages": ["en", "de", "es"], so 
> that when 
> > you build, it'd do it for all needed locales. I'm not saying to 
> eliminate 
> > "language", as it's nice to be able to give as a parameter in 
> test-simple.html 
>
> Then this runs into the same issue as modules -- how to output multiple 
> files. And if you are using modules, you get into a NxM situation. 
>
> This wants some sort of generic solution, which IMO is beyond the scope 
> of this change. 
>
> https://codereview.appspot.com/6194048/ 
>