The current entity language API works with $entity->getTranslation() to ask for a translated version of the entity and NOT with methods providing access to translated versions individually. Once a translation is retrieved, the methods just work with the translated values. The langcode argument on label() was added in #1616952: Add a langcode parameter to EntityInterface::label() in a previous life of the entity language API and is pointless now. It does not even work for the base field on the entity, it does not consider that langcode to retrieve the label (it only passes it on to a title callback if present). The title callback however could use getLanguage() on the entity, no need to pass it on explicitly. That is part of the appeal of the new API.
Remove the pointless argument. The title callback gets the whole entity, so it can do getLanguage() if it needs the active language on the entity copy.
Commit.
None.
Entities will not have a langcode method on label.
| Comment | File | Size | Author |
|---|---|---|---|
| #31 | label-langcode-pointless-31.patch | 9.31 KB | plach |
Comments
Comment #2
gábor hojtsyIt helps to update the interface and base Entity.php implementation :) The config entity base implementation does not even have the label method.
Comment #3
gábor hojtsyLooking at it more, the label() method on the content entity then is the same as the default Entity.php implementation, so pointless to have two copies of it.
Comment #4
plachMakes totally sense. RTBC if green.
Comment #5
gábor hojtsyI was a bit trigger happy there :D The content entity has a fielditem as the label, so it needs to use ->value. That is the only difference from the base entity implementation. Sad, but true. :/
We could also theoretically just invoke the parent and then work on top of that. BUT then the parent method would return a field item list instead of a string, so it would not be true to its interface docs, and that would be a fail. So we need to have this code in content entity.
Comment #6
gábor hojtsyComment #7
gábor hojtsyAlso updating docs on label_callback and the only implementations I found of that in entity_test and user. The user one did not have a langcode anyway, but the code comment on that was *misguided*. It was missing a word *no* :D Also not really at the right place. Since it relies on the username method which will rely on multilingual base fields if available, this comment should go to the base field definition (and corrected there).
In the meantime k4v found implementations of label() with langcode in config entities. Looking forward to more updates on that :)
Comment #10
k4v commentedSo here is a new patch with some more langcode parameters removed.
Comment #11
gábor hojtsyThe changes look good. Let's hope testbot is not going to act up.
Comment #13
k4v commented10: label-langcode-pointless-6.patch queued for re-testing.
Comment #15
k4v commentedlets see...
Comment #16
gábor hojtsyBack to RTBC as per #4, in person review with @fago and my review of the updates from k4v. Looks all good.
Comment #17
linl commented15: label-langcode-pointless-7.patch queued for re-testing.
Comment #18
linl commentedReroll, no longer applied.
Comment #21
gábor hojtsyLooks good :)
Comment #22
gábor hojtsyComment #23
webchickThis looks like good clean-up. Tagging.
Committed and pushed to 8.x. Thanks!
This may need a change notice.
Comment #24
webchickSigh. Spoke too soon. Patch no longer applies. >:\
I cannot over-emphasize how badly I want #2125269: [META] Regressions in issue queue workflow fixed.
Comment #25
gábor hojtsyFascinating how many conflicts there may be for a patch sitting even for 2 days.
Comment #26
gábor hojtsy25: label-langcode-pointless-25.patch queued for re-testing.
Comment #27
linl commentedAnother reroll following #2155889: Remove Editor::label()
Comment #28
gábor hojtsyGreat reroll, thanks.
Comment #29
xjm27: label-langcode-pointless-27.patch queued for re-testing.
Comment #30
webchickSorry, this went stale with all the recent major entity refactoring.
Comment #31
plachRerolled
Comment #32
webchickCommitted and pushed to 8.x. Thanks!
Comment #33
gábor hojtsyAdded change notice at https://drupal.org/node/2166535. Also added to the more general change notice (https://drupal.org/node/2040323), but did not modify that change notice since it already properly explains how to get a translation and how to work with it. The label($langcode) problem was an intra-d8 problem that we introduced earlier in 8.x and now removed.
Comment #34
gábor hojtsy