Exporting translations does not correctly generate empty strings

More specific title. Ajax recoverable errors can be many things.

Outi, I tried with basque and french with the official .po files for core and could not reproduce your issue (drupal-8.0-alpha2.eu.po & drupal-8.0-alpha2.fr.po).
Which po files did you import? could you upload them to the issue?

Just talked with Outi: she translated some strings in the interface, exported the PO file and then tried to import it again.
Doing the same I can reproduce it.

Uploading exported/imported po file for the reference.

For empty strings that are exported , there are not quotes

msgid "Include customized translations"
msgstr 
msgid "Include untranslated text"
msgstr 
msgid "The strings have been saved."
msgstr 

So when parsing the file we report the error: Recoverable fatal error: Argument 2 passed to Drupal\Component\Utility\String::format() must be of the type array, null given, called in /Users/outimunter/Sites/drupal/core/lib/Drupal/Component/Gettext/PoStreamReader.php on line 522 and defined in Drupal\Component\Utility\String::format() (line 89 of /Users/outimunter/Sites/drupal/core/lib/Drupal/Component/Utility/String.php).

If we run gettext-lint to the exported file, we see that is not valid:

apt-get install gettext-lint
POFileStatus ~/Downloads/fr.po 

returns:

$ POFileStatus ~/Downloads/fr.po 
<po-file-status>
  <file name="/Downloads/fr.po">
    <error>/Downloads/fr.po:23:6: syntax error
/Downloads/fr.po:25:6: syntax error
/Downloads/fr.po:27:6: syntax error
/Downloads/fr.po:29:6: syntax error
/Downloads/fr.po:31:6: syntax error
/Downloads/fr.po:33:6: syntax error
/Downloads/fr.po:35:1: syntax error
msgfmt: found 7 fatal errors
</error>
  </file>
</po-file-status>

As a confirmation, I checked an exported file with gtranslator (PO editor for gnome), and it crashed. Fixed the file manually and it worked.

Adding test coverage checking that the empty strings are properly quoted if non-translated strings are selected for exporting.
Quoting is done in PoItem itself.

+++ b/core/modules/locale/lib/Drupal/locale/Tests/LocaleExportTest.phpundefined
@@ -83,7 +83,7 @@ function testExportTranslation() {
-      ->setString('February')
+      ->setString('March')

Why are you changing from February to March? Is it because the language in the test used to not have a translation for February, and now does... but it still does not have translation for March?

Looks good.

tl;dr: February is not that bad...

Long version:

While testing this I wanted to test two different cases:
1. Importing a file with an unstranslated string (February)
2. Creating the string in locale storage (March)
but I noticed that in 1), nothing is imported and no string is saved into locale.storage, so it was not really needed.

This looks better. :)

I'll try to reproduce.

core gates

#11: po-export-2029505-10.only-tests.must-fail.patch queued for re-testing.

#11: po-export-2029505-10.patch queued for re-testing.

+++ b/core/modules/locale/lib/Drupal/locale/Tests/LocaleExportTest.phpundefined
@@ -168,4 +168,11 @@ function getCustomPoFile() {
 
+  function getEmptyPoTranslation() {
+    return <<< EOF
+msgid "February"
+msgstr ""
+EOF;
+  }

This lacks code comments as to what it is used for.

Renamed the function to getUntranslatedString for readability and added docblock.

Looks all good now then :)

+++ b/core/modules/locale/lib/Drupal/locale/Tests/LocaleExportTest.phpundefined
@@ -168,4 +168,14 @@ function getCustomPoFile() {
+   * Helper function that returns a .po fragment file with an unstranslated string.

unstranslated
should be untranslated.

Also put it below 80 chars.. and added the @return.

It was already RTBC by Gábor, and YesCT improved docs.

This issue was RTBC and passing tests on July 1, the beginning of API freeze.

-    $output .= 'msgstr '. (isset($this->_translation) ? $this->formatString($this->_translation) : '');
+    $output .= 'msgstr '. (isset($this->_translation) ? $this->formatString($this->_translation) : '""');

While we're here let's make the string concatenation follow coding standards.

Here is the reference to check:
https://drupal.org/coding-standards#concat

I'm not working on this at the moment, so any help is appreciated.

Included the whitespace fix that @catch requested. Also I checked the plural formatting as well, and it covers empty strings proper, so no such bug evident there.

    foreach ($this->_translation as $i => $trans) {
      if (isset($this->_translation[$i])) {
        $output .= 'msgstr[' . $i . '] ' . $this->formatString($trans);
      }
      else {
        $output .= 'msgstr[' . $i . '] ""' . "\n";
      }
    }

Back to RTBC as per above given the minor whitespace change only.

yep. looks good.

#27: po-export-2029505-27.patch queued for re-testing.

Committed b2b2a44 and pushed to 8.x. Thanks!

Remove sprint tag.

Summary update