I have added to the global CRM variable a container for translated strings, which seems like a good compromise of simplicity and convenience.
See templates/CRM/common/scripts.tpl for detailed documentation

For the final step (parsing js files to extract strings for people to translate), we need to decide where JS files should live. Some considerations:
1) The parser separates the .po files by component based on the file structure (CRM/Contribute/etc)
2) Our js folder does not currently contain this hierarchy, and replicating it there might be cumbersome (but maybe makes the most sense?)
3) If we choose no for #2, that leaves the CRM folder and the templates folder:
3a) templates is tempting because that's where most of our js lives at the moment (as inline scripts in templates) but javascript files really aren't templates
3b) the CRM folder is where we keep business logic (and our js contains more business logic now than ever, and this trend will likely continue) - it's called CRM not php, so maybe js files have a place there?

I'm now getting a bunch of errors from CRM_Core_ResourcesTest.

What I think we should do, though, is refactor r45074 a bit. There's a couple issues:

1. The revision creates a lot more coupling which makes it harder to test (and it invites other developers in the future to create more coupling). For example, the unit-test includes a small mock mapping function which takes about 4-5 lines of code and runs very quickly. After r45074, CRM_Core_Resources depends on CRM_Extension_Mapper – to test the class, one needs to create a mock mapper, mock cache, etc – and basically a full extension system. This is possible but makes the tests slower and ultimately more brittle. (Mocking the extension system is tricky – I spent a lot of time doing it with the CRM_Extension test-suite.)

2. As an alternative to r45074, we could do this:
2.a. Create a class "CRM_Core_JsTranslator" which includes the parseScript, translateScript, etc. Ideally, we would create JsTranslatorTest which passes in some sample JS code and verifies that it is correctly parsed.
2.b. In CRM_Core_Resources, add support for "filters". The JsTranslator is one filter. There could be others (such as minimizers, CDN-publishers, or pretty-printers). Specifically, addScriptFile would call any filters instead of directly calling JsTranslator.
2.c. In CRM_Core_Resources::singleton(), we create the "normal" or "default" instance of Resources. At this point, we can also instantiate JsTranslator and put them together. (e.g. $resources->addFilter(new JsTranslator()) ).

The failing unit-tests are blocking me from CRM-11760 (b/c I need to add new features to Resources to address the bug in 11760 – but I can't add new features if the tests are failing). So if the design makes sense to you, then I'm happy to help with implementing.

totten: yes that makes sense, should be a quick job to refactor it. I understand your proposal in theory but could use some help putting it into practice. Ping me on irc later today if you want to work on it together.

bgm: could you please review the changes I made at
https://github.com/civicrm/l10n/commit/1b2b808a1ec1302cfb871f275a95409432278b34

I think I understand the system, but don't want to break everything!
Also note the one line I changed in bin/smarty-extractor.php - was that a bug? It looked like it would cause hlp files to be skipped.

I've moved the parsing code form CRM_Core_Resources to CRM_Utils_JS (to make it more re-usable). The js-extractor.php in l10n should probably call CRM_Utils_JS::parseStrings to avoid code duplication.

Seems generally OK to me, but if I recall correctly, the script uses SVN branches to generate the .pot files. I'd rather wait to tests until the git migration is done, or until the 4.3 branch is available.

I think the following line may need adjusment:

$command = "find $dir/templates $dir/xml $jsModifier | grep -v '/\.svn/' | sort | xargs $jsExtractor $dir";
$jsPot = `$command`;

Should be: $command = "find $dir/js [...]" ?

(I'm not the original author of those scripts, but might use the git migration as a pretext to axe/refactor some stuff)

Mathieu Lutfy We decided to put our js files in the templates dir for now. Although some are also in js, so I guess we need to scan both. I'll make that change.

Tim Otten You know my feelings on code duplication but in this case we might have to live with it because l10n runs as standalone scripts and doesn't currently have any dependencies on the core civicrm code, so I'm not sure about the problems we might run into if we introduced some.

@ Coleman: I'm chasing a JS extraction bug, and was wondering if you could explain this code:

// "fix" string - strip slashes, escape and convert new lines to \n
function fs($text) {
$quote = $text[0];
// Remove newlines
$text = str_replace("\\\n", '', $text);
// Unescape escaped quotes
$text = str_replace('
' . $quote, $quote, $text);
// Remove end quotes
$text = substr(ltrim($text, $quote), 0, -1);
return $text;
}

(the smarty extractor does not do this)

• New lines should be converted to \n, not removed? (ortherwise, the translation string will not match in the .po dictionnary when a string lookup will be made).
• Why remove end quotes? Can you give an example of where this happens?
• We need to escape double quotes, otherwise the .po generates syntax errors.

For example:

#: js/view/crm.designer.js
msgid "A new window or tab will open. Use the new window to add your field, and then return to this window and click "Refresh.""
msgstr ""

The above needs to be \"Register\".

I'll fix the escaping of double quotes, but will wait for your feedback for the other items.

No answers to the questions, but an aside: when fixing the parser, one should also update the parser test-cases:

https://github.com/civicrm/civicrm-core/blob/master/tests/phpunit/CRM/Utils/JSTest.php

It should be pretty simple – just add another entry like:

$cases[] = array($javascriptFileContent, $extractedStrings);

Hi Mathieu, thanks for working on this. Here's what I know:
(I'm going to use # delimiters for enclosing strings instead of quotes to avoid confusion)
When we get to this point in the code the regex has given us the exact contents of the ts() call, so if the code says ts( "Hello World!" ) then our string looks like #"Hello World!"#
Note that the whitespace has been stripped from outside the string, but the quotes have not.
So the first line $quote = $text[0] is going to find that this string is enclosed in double quotes, not single quotes.
We then use that information to unescape that type of quotes, so in this case we'd convert #\"# to #"# (and if the string had been surrounded in single quotes we would have unescaped those instead)
It strikes me as odd that the po generator needs escaped double quotes, and I wonder how you accomplish that for smarty and php since you can't count on them always being escaped in the code, for example ts('This is "Bob\'s book"') contains unescaped double quotes but escaped single quotes. So how do you handle that? Do you somehow reverse it and unescape the singles and escape the doubles?
If that's the requirement then I would recommend you allow this code to do it's thing and unescape all quotes so we have a consistent baseline, then re-escape the doubles at the end.

Ok, moving on, line breaks:
Line breaks in javascript strings are weird. They're not allowed at all prior to es5, and so they are seldom used (though they are becoming more common). They also require escaping, so if our code were to use line breaks in a string it might look like this:
ts("Line one\
Line two\
Line three");
The code converts the backslash and the newline into an empty string, which is basically how javascript treats them too – they are invisible. So writing the above is, AFAIK, exactly the same as writing
ts("Line oneLine twoLine three")
In other words the line break doesn't appear in the markup.
On the other hand, if we were to write in javascript
ts("Line one\nLine two\nLine three") that actually would appear in the markup. But our find/replace is not going to touch it (our str_replace is searching for a backslash followed by a newline, not a backslash followed by the letter n), so I think we're ok on that front. What I haven't tested is whether our javascript ts() function correctly handles newline characters.

Hope that helps.

This should be fixed in the latest .pot files.

For example: in po/pot/common-base.pot

#: js/Common.js
msgid "Are you sure you want to continue?"
msgstr ""