Problem/Motivation

A number times in /src we use the `StringTranslationTrait` but don't call `setStringTranslation()` or have a dependency on the translation service.

Proposed resolution

We should always have a dependency on the translation service and not rely on `\Drupal\Core\StringTranslation\StringTranslationTrait::getStringTranslation` to get the service unless we absolutely have to.

Issue fork automatic_updates-3243057

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tedbow created an issue. See original summary.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
CreditAttribution: phenaproxima at Acquia commented

Rather than a direct dependency on the string translation service, I propose we take advantage of the container's calls directive for services that use StringTranslationTrait. For example:

my_service:
  class: Foo\Bar
  calls:
    - ['setStringTranslation', ['@string_translation']]

This will instantiate the service, then immediately call setStringTranslation() on it, passing the string translation service. It reduces the amount of boilerplate we need to write.

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
CreditAttribution: tedbow at Acquia commented

hmmm. why do we have to do this different from core’s current practice? setStringTranslation is not found anywhere in core in *.services.yml files.
@string_translation is found many times in *.services.yml files.
maybe core should change its practices but for anybody new trying to understand the core codebase I think there is immense value in things being more uniform.

somebody who understands how this works for every other service in core that needs translation is going to have to think just a little bit harder to understand our module

kunal.sachdev made their first commit to this issue’s fork.

kunal.sachdev opened merge request !72

kunal.sachdev’s picture

kunal.sachdev CreditAttribution: kunal.sachdev commented
Status: Active » Needs review
tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
CreditAttribution: tedbow at Acquia commented
Status: Needs review » Reviewed & tested by the community

Looks good!

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
CreditAttribution: tedbow at Acquia commented
Status: Reviewed & tested by the community » Fixed
kunal.sachdev’s picture

kunal.sachdev CreditAttribution: kunal.sachdev at Acquia commented

Status: Fixed » Closed (fixed)

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