Problem/Motivation
This issue has a non-test scope, which means don't convert test classes.
Classes which use t() should convert their usage to an injected translation service.
Proposed resolution
t() usages inside static class methods can't be injected, so leave those.
Other usages should ensure that the class uses Drupal\Core\StringTranslation\StringTranslationTrait, or otherwise has an implementation of $this->t().
Calls to t() in non-static methods should be changed to $this->t().
Remaining tasks
Eventually perform IoC for service traits such as StringTranslationTrait: #2733703: [plan] Service traits should require IoC injection
User interface changes
API changes
Data model changes
Comments
Comment #2
kunalkursija commentedAdding Patch.
Comment #3
minakshiPh commentedComment #5
aditya.n commentedComment #6
aditya.n commentedAdding the patch for the issue
Comment #8
kunalkursija commented@androiditya
Static functions can be used without instantiation, Hence you cannot use "$this" inside static functions.
Comment #9
naveenvalechaThis will fix the errors
Comment #10
naveenvalechaThis is a BC break, but as the module is in alpha then who cares ;)
Comment #11
amit.drupal commentedComment #12
mile23Unfortunately, hooks don't have a
$this, much less$this->t(). So either leave the globalt()or use$translation = \Drupal::translation()to get the service, and then call$translation->t().StringTranslationTraithas asetStringTranslation()method we should use during the constructor.Comment #13
amit.drupal commentedComment #14
amit.drupal commentedComment #15
amit.drupal commented@Mile23 Thanks for review code.
Comment #17
amit.drupal commented@naveenvalecha Please Suggest me issues in 2806983-11.patch
Comment #18
naveenvalecha#11, Please do test on local before posting, Did you even test the patch on d.o.
Unreleated changes.
#12.2
yup nice spot :)
Comment #19
amit.drupal commented@naveenvalecha Thanks
Next time improve my coding.
Comment #20
minakshiPh commentedComment #21
keshavv commentedHere is the patch please review
Comment #22
minakshiPh commentedcorrected the patch as mentioned in #12
Kindly review.
Thanks!
Comment #24
mile23Thanks. :-)
The trait adds
setStringTranslation()to$this, so it would be$this->setStringTranslation($string_translation);Comment #25
minakshiPh commentedComment #26
minakshiPh commentedHi @Mile23,
Thanks for reviewing my patch.
Have corrected the minor change as mentioned in #24.
Kindly review.
Thanks!
Comment #27
naveenvalechaThis will make the test fail b/c the $this->t() function will not be available here.
Comment #28
tedbow@minakshiPh Settings needs work because of #27
Comment #29
minakshiPh commentedComment #30
minakshiPh commentedHi @tedbow,
Change mentioned in #27 is done and attached the new patch.
Kindly review.
Thanks!
Comment #31
pmchristensen commentedLooking really good - reviewed and tested. It should perhaps be considered to do something about the skipping tests?
Comment #33
xjmThank you for your work on cleaning up Drupal core's use of deprecated APIs!
In order to deprecate APIs in a maintainable way, converting deprecated uses should be replaced across all of core for a given kind of usage, rather than in individual modules or files. Such issues should also always be part of an overall plan to ensure all usages are removed, rather than standalone patches. Such a plan may already exist for
t()so this issue should probably be closed in favor of that.For background information on why we usually will not commit cleanups that aren't scoped in that way, see the core issue scope guidelines. See #2575081: [policy, no patch] Use E_USER_DEPRECATED in Drupal 8 minor releases for more information on how we plan to manage deprecations in the future.
Contributing to the overall plan above will help ensure that your cleanups for core's deprecated code improve core in a maintainable and minimally disruptive way.
For this issue, we should start by replacing non-test
t()usages in all classes where the translation trait or service is already available. This can be done with a script. We would then have a followup to inject the service in classes where it is not already available and convert usages there. Finally, we should have a separate issue to discuss the use oft()in tests. We probably should close this issue as a duplicate, but setting to needs work for now so that contributors see this information. Thanks!You might want to help contribute to this issue instead: #2549805: [Meta] Remove all usage of FormattableMarkup in tests apart from explicit tests of that API
Comment #34
zeip commentedComment #35
tedbowClosing as per @xjm comments in #33
If anybody knows of such as issue please link to it from here.
Comment #36
mile23Can't be done with a script. Someone is welcome to prove me wrong, however.
I don't think the larger-scope issue exists since I couldn't find it. I'm re-opening and re-scoping here, since we already have a patch here.
Comment #45
manishsaharan commentedFixed this with 9.2.x
Comment #50
smustgrave commentedClosing as duplicate of #3122912: t() calls should be avoided , use $this->t() instead in Form Builders