Closed (fixed)
Project:
Drupal core
Version:
8.0.x-dev
Component:
base system
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
24 Jul 2013 at 21:14 UTC
Updated:
29 Jul 2014 at 22:42 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
plopescHello,
I glimpsed this issue and found that TranslatorInterface is implemented by 3 classes:
But only Drupal\Core\StringTranslation\TranslationManager class provides
translatemethod. Then, I don't know exactly how should we face with this issue, because if this method is added to the interface, all these classes should implement this method.Maybe we could extend
TranslatorInterfaceand implement this new interface only inTranslationManagerclass.What do you think?
Regards
Comment #2
tim.plunkett#2018411-8: Figure out a nice DX when working with injected translation proposes a new interface. We can borrow part of that patch here.
Comment #3
plopescIncluding patch based on #2049709-8: TranslationManager::translate() should be on an interface, just removing the
__invoke()call, which I think is not covered in this issue.However, this patch does not address the issue title given that this method is not included in TranslatorInterface.
What do you think?
Regards.
Comment #4
tim.plunkett:)
Comment #5
dawehnerI try to figure out why we both need Translator and TranslationInterface. Is it really impossible to combine this two methods into one?
Comment #6
ParisLiakos commentedwell
TranslationManageris actually a translator manager class that "holds" many translators and it can act as a translator by having its owngetStringTranslationmethod, in where it loops through its translators till it retrieves a translation.Almost same pattern with PathProcessorManager.
(Maybe the naming is wrong all along and should be named
TranslatorManager?)We could add this method to TranslatorInterface, but then we would need a base/abastract class to hold the logic instead of it being only in the manager class.
BUT I think having a
TranslationInterfaceis better, since it allows a totally different translation system (with or without translators). You can think of Translators being an implementation detail of core's string translation. A string translation itself only needs to have thetranslatemethod.If the fact that
TranslationManagerimplementing theTranslatorInterfaceis confusing, we can remove it and just make it implement TranslationInterface, not both, but we should keep translate() method out of translatorsComment #7
ParisLiakos commentedthis can be addressed in seperate issue so imo is rtbc. Tagging though for more opinions
Comment #8
ParisLiakos commentedwell, the sooner we get this in, the better
Comment #9
tim.plunkett+1. Misusage of TranslationManager and TranslatorInterface are going in all over, and this also blocks a proper resolution on #2059245: Add a FormBase class containing useful methods
Comment #10
tim.plunkett#3: translate_interface-2049709-1.patch queued for re-testing.
Comment #11
tim.plunkettOpened #2070367: Consider renaming \Drupal\Core\StringTranslation\Translator\TranslatorInterface as a follow-up to address the naming concerns.
Comment #12
alexpottCommitted 3e83934 and pushed to 8.x. Thanks!