Only fallback to an existing singular or nth plural form of a translation

Hiding patches for clarity.

Still seems feedback from #29 was added. Thanks for updating the issue summary.

Hi @sleitner,
Unable to reproduce the issue.

Steps followed:
1. Standard installation of using hi (i.e. non-English) language.
2. Created a few articles using Article content type.
3. Performed the bulk operation (/admin/content)

Each time operation ended successfully.

Hiding patches for clarity.

1) To be clearer. Not explicit here, but the error comes probably from the line,
$return = $translated_array[1];
(Line 136 in core/lib/Drupal/Core/StringTranslation/PluralTranslatableMarkup.php is actually something else)
2) What about rather (plus some comments)?,
$return = $translated_array[$index] ?? $translated_array[1] ?? $translated_array[0];
instead of,

    if ($index == 0) {
      // Singular form.
      $return = $translated_array[0];
    }
    else {
      if (isset($translated_array[$index])) {
        // N-th plural form.
        $return = $translated_array[$index];
      }
      else {
        // If the index cannot be computed or there's no translation, use the
        // second plural form as a fallback (which allows for most flexibility
        // with the replaceable @count value).
        $return = $translated_array[1];
      }
    }

@Chris64: 1) $return = $translated_array[1]; was in line 136 in 2017

2) I separated the singular form completely from the plural forms:

    $index = $this->getPluralIndex();
    // Singular form.
    if ($this->count == 1 || $index == 0 || count($translated_array) == 1) {
      return $this->placeholderFormat($translated_array[0], $arguments);
    }

    // N-th plural form, fallback to second plural form.
    $return = $translated_array[$index] ?? $translated_array[1];

I don't see any issue consolidating

I can confirm the diff resolves the issue. Drupal 10.1.7 & PHP 8.1

I'm triaging RTBC issues → . I read the IS.

An issue summary should provide contributors with a clear explanation of the problem and how to reproduce it. I don't see that in the issue summary here. Without that I don't have an understanding of what is being fixed and can't continue. Since having an up to date issue summary is a step in setting an issue to RTBC → I am tagging this for an issue summary update. It may help to look at the issue summary examples → .

I did skim the comments and yes, I see that alexpott, a person with many years experience, has reviewed. However, it is also good practice to have an accurate issue summary so that more people can contribute and learn.

I also noticed that @rksyravi was not able to reproduce the problem and there was no response to that. So, I must ask, is this reproducible on a supported version of Drupal?

For me it was not about the bulk operations. The exception was appearing when I go to the results page of the webforms. But the diff has resolved this.

it is also good practice to have an accurate issue summary

Absolutely.

@makby in #42 🐛 Notice: Undefined offset: 1 en Drupal\Core\StringTranslation\PluralTranslatableMarkup->render() Needs work and #44 🐛 Notice: Undefined offset: 1 en Drupal\Core\StringTranslation\PluralTranslatableMarkup->render() Needs work means the problem could be reproduced. Maybe @makby could be more explicit about how he meets the problem.

@quietone in #43 🐛 Notice: Undefined offset: 1 en Drupal\Core\StringTranslation\PluralTranslatableMarkup->render() Needs work seems face the same problem than me in #39 1) 🐛 Notice: Undefined offset: 1 en Drupal\Core\StringTranslation\PluralTranslatableMarkup->render() Needs work , and the answer #40 1) 🐛 Notice: Undefined offset: 1 en Drupal\Core\StringTranslation\PluralTranslatableMarkup->render() Needs work of @sleitner should be in the IS, otherwise the problem is ununderstendable from it.

#19 🐛 Notice: Undefined offset: 1 en Drupal\Core\StringTranslation\PluralTranslatableMarkup->render() Needs work from @codebymikey allows to understand the origin of the problem.

Some new changes append in the code. To have only one call to $this->placeholderFormat(*, $arguments)?
By now the ternary operator use is possible,

    // Singular form, or, N-th plural form, fallback to second plural form.
    $return = ($this->count == 1 || $index == 0 || count($translated_array) == 1)
                ? $translated_array[0]
                : $translated_array[$index] ?? $translated_array[1];

or,

    // Singular form, or, N-th plural form, fallback to second plural form.
    $return = ($this->count == 1 || $index == 0 || count($translated_array) == 1) ? $translated_array[0] : $translated_array[$index] ?? $translated_array[1];

instead of,

    if ($this->count == 1 || $index == 0 || count($translated_array) == 1) {
      // Singular form.
      $return = $translated_array[0];
    }
    else {
      // N-th plural form, fallback to second plural form.
      $return = $translated_array[$index] ?? $translated_array[1];
    }

MR appears to have failures.

Have not reviewed or tested

/scripts-127385-579080/step_script: line 272: /usr/bin/tr: Argument list too long
/scripts-127385-579080/step_script: line 272: /usr/bin/yarn: Argument list too long

No, that is a GitlabCI problem. When many files are changed (during reroll), the spell-checking job always fails.

Then MR needs to be rebased as that's been fixed.

Now it's all green

Ran the test-only feature

There was 1 error:
1) Drupal\Tests\Core\StringTranslation\PluralTranslatableMarkupTest::testMissingPluralTranslation
Undefined array key 1
/builds/issue/drupal-2863629/core/lib/Drupal/Core/StringTranslation/PluralTranslatableMarkup.php:128
/builds/issue/drupal-2863629/core/tests/Drupal/Tests/Core/StringTranslation/PluralTranslatableMarkupTest.php:51
/builds/issue/drupal-2863629/vendor/phpunit/phpunit/src/Framework/TestResult.php:728
/builds/issue/drupal-2863629/vendor/phpunit/phpunit/src/Framework/TestSuite.php:684
/builds/issue/drupal-2863629/vendor/phpunit/phpunit/src/TextUI/TestRunner.php:651
/builds/issue/drupal-2863629/vendor/phpunit/phpunit/src/TextUI/Command.php:144
/builds/issue/drupal-2863629/vendor/phpunit/phpunit/src/TextUI/Command.php:97
ERRORS!
Tests: 3, Assertions: 2, Errors: 1.

Issue summary appears complete and mirrors the proposed changed.

Tested with MR, installing non-english languages and didn't receive any errors. Believe this is good to go.

I'm triaging RTBC issues → . I read the title, the IS, comments and the MR.

The title should state what is being fixed. This title is simply an error message so I am tagging for an title update.

The steps to reproduce in the issue summary do not match the problem/motivation section. I found the complete steps to reproduce in #37 and I copied them to the issue summary.

I made two suggestions in the MR. Setting to NW for that and an improved title. Remember, the title is especially important when digging through git history. So, just a few things.

@rksyravi, remember to update the issue summary, as appropriate, for the work you have done. In this case, the steps to reproduce should have been updated.

Thanks!

Title appears to be updated.

This change looks good. Can someone open an issue to make a further optimisation and not call $this->getPluralIndex() when dealing with singular counts.

The code can be refactored to

    // Use the singular form when count is 1 or there is only a one translation.
    $index = $this->count == 1 || count($translated_array) === 1 ? 0 : $this->getPluralIndex();
    // Fallback to second plural form.
    return $this->placeholderFormat($translated_array[$index] ?? $translated_array[1], $arguments);
  }

Committed and pushed 47b08da6e0 to 11.x and 059e87def9 to 10.3.x and 1213c7c004 to 10.2.x. Thanks!

Automatically closed - issue fixed for 2 weeks with no activity.