Plurals are not exported correctly when exporting source translations

Hm, are translations properly exported when you choose that method (not the template)?

Not clear if this is broken only for saved translations or stuff that was never yet touched too? The code cited from the save seems like it would add the needed amount of delimiters(?)

I reported this to LingoTek. It was an export of untranslated strings I wanted to upload so we could get a translation of strings in our Twig templates.

Manually tested the patch and can confirm it fixes the issue.

Before patch

msgid "Host"
msgid_plural "Hosts"
msgstr[0] ""

After patch

msgid "Host"
msgid_plural "Hosts"
msgstr[0] ""
msgstr[1] ""

The following UI screen for export. And I was able to upload to LingoTek without errors in parsing the .po

Items which have been translated, as stated, do not have this problem.

Now there are tests, and this fixes the bug. +1 and RTBC from me.

1. Sorry for the following comments - it's one of the tricky things about the component space and the fact that they are supposed to be decoupled from \Drupal\Core\*.

2. +++ b/core/lib/Drupal/Component/Gettext/PoItem.php
   @@ -70,6 +77,13 @@ public function getLangcode() {
   +    if (\Drupal::moduleHandler()->moduleExists('locale')) {
   +      $this->_nPlurals = \Drupal::service('locale.plural.formula')->getNumberOfPlurals($langcode);
   +    }
   

   Hmmm... a component probably should not be reaching out into \Drupal::service land. I think we should add a param to this method to set the number of plurals. Or add a new method to set it. Tricky.

3. +++ b/core/lib/Drupal/Component/Gettext/PoItem.php
   @@ -187,7 +201,12 @@ public function setFromArray(array $values = []) {
   +        $this->setTranslation(explode(LOCALE_PLURAL_DELIMITER, $this->_translation));
   

   We need a followup to decouple this component from LOCALE_PLURAL_DELIMITER - or to move the definition of this to the component. The dependencies are wrong way around.

4. +++ b/core/lib/Drupal/Component/Gettext/PoStreamReader.php
   @@ -526,12 +526,18 @@ public function setItemFromArray($value) {
   +    if (isset($value['msgstr'])) {
   +      $item->setTranslation($value['msgstr']);
   +    }
   +    else {
   +      $nPlurals = \Drupal::service('locale.plural.formula')->getNumberOfPlurals($this->_langcode);
   +      $this->setTranslation(array_fill(0, $nPlurals, NULL));
   +    }
   

   As above - calling out to \Drupal::service is not great here.

It makes sense to check all string/array flip/flops and decide about home for delimiter definition
Faced in #1637662-14: Clean up the Gettext PO parser

Surely component's space should not use services and common.inc constant

PS: reroll

Fix for 15.3 in #2911606: Replace usage of the deprecated LOCALE_PLURAL_DELIMITER constant so this issue partially blocked on it

It looks totally wrong that PoItem should know language and number of plurals - it is used to store strings only and language mostly unused

I think it needs deeper refactor to store string & translation as arrays and implode/explode them where it actually expected.

Core using delimiter in following places
- .po files reader/writer - conditionally flipping source and translation depending on number of plurals - #2918489: Plurals are not exported correctly when exporting source translations
- storing plurals in config - also using flip/flop on export and to create translations- #2545730: Misuse of formatPlural() in Numeric field prefix/suffix
- UI elements and controls to edit translations

Testing the patch during DrupalCon Global.

I have tested #24 via a local ddev environment using the 9.1.x branch. I am also seeing the additional 'msgstr[1] ""' items for plurals. There were no other changes to the pot file - except the POT-Creation-Date and PO-Revision-Date lines, which I would expect.

Not sure if this should be moved to RTBC due to the additional discussion.

Fix interface docblock and added types to new interface

@andypost I tried to manually apply the patch on 9.1x. After site refresh I am getting website encountered error.

Dblog showed the php error:

It's strange, it works for me

Crediting @idebr as per #23.

In related it was fixed to handle this suffixes

1. +++ b/core/modules/locale/src/PoDatabaseReader.php
   @@ -34,6 +34,13 @@ class PoDatabaseReader implements PoReaderInterface {
   +  private $pluralCount = 2;
   

   Why private?

2. +++ b/core/modules/locale/src/PoDatabaseReader.php
   @@ -62,6 +69,20 @@ public function setLangcode($langcode) {
   +  /**
   +   * {@inheritdoc}
   +   */
   +  public function getPluralCount() {
   +    return $this->pluralCount;
   +  }
   +
   +  /**
   +   * {@inheritdoc}
   +   */
   +  public function setPluralCount($pluralCount) {
   +    $this->pluralCount = $pluralCount;
   +  }
   +
   

   This are not inheriting anything.

3. PoDatabaseWriter is not implementing these methods. Should the writer and reader interfaces extend the new PoPluralCountAwareInterface? That feels disruptive. The only thing in cotrib I found extending either interface is http://codcontrib.hank.vps-private.net/search?text=PoReaderInterface&fil... - I guess that should change to.

I am not surprised that there is not wider impact. IMHO we can coordinate with locale_translation_context to fix on their side as well.

Rerolled the patch 37.

While #2882667: [PP-1] Interface translations export contains incorrect plural formula was closed as a duplicate, this does not deal with the export of the plural formula itself in the .po file. Which is also still broken. I think that would be fine to be in scope for #2882617: String version of plural formula is not available, exported .po files contain an incorrect default though.

Recap from DrupalCon Lille 2023:

The functionallity of the latest patch #37 looks fine. If the reroll of the patch passes, we put this back to Needs Review.

Moved last patch to MR and fixed following to make the test case to pass

- https://www.drupal.org/node/3168858
- https://www.drupal.org/node/3129738

NW to add

1. +++ b/core/lib/Drupal/Component/Gettext/PoItem.php
   @@ -49,6 +49,13 @@ class PoItem {
   +   * @var int
   ...
   +  protected $pluralCount;
   
   +++ b/core/lib/Drupal/Component/Gettext/PoStreamReader.php
   @@ -62,6 +62,13 @@ class PoStreamReader implements PoStreamInterface, PoReaderInterface {
   +   * @var int
   ...
   +  protected $pluralCount = 2;
   
   +++ b/core/lib/Drupal/Component/Gettext/PoStreamWriter.php
   @@ -35,6 +35,13 @@ class PoStreamWriter implements PoWriterInterface, PoStreamInterface {
   +   * @var int
   ...
   +  protected $pluralCount = 2;
   
   +++ b/core/modules/locale/src/PoDatabaseReader.php
   @@ -34,6 +34,13 @@ class PoDatabaseReader implements PoReaderInterface {
   +   * @var int
   ...
   +  private $pluralCount = 2;
   

   should be typed to int, so default value will be 0

2. +++ b/core/lib/Drupal/Component/Gettext/PoPluralCountAwareInterface.php
   @@ -0,0 +1,28 @@
   +   * @return self
   ...
   +  public function setPluralCount(int $pluralCount);
   
   +++ b/core/lib/Drupal/Component/Gettext/PoStreamReader.php
   @@ -126,6 +133,21 @@ public function getHeader() {
   +  public function setPluralCount(int $pluralCount) {
   
   +++ b/core/lib/Drupal/Component/Gettext/PoStreamWriter.php
   @@ -75,6 +82,21 @@ public function setLangcode($langcode) {
   +  public function setPluralCount(int $pluralCount) {
   

   Probably should return :self as of PHP 8.0 https://php.watch/versions/8.0/static-return-type

Patch for Drupal 10.3