Don't call the t() function in OO code in the field_ui module

1: issue-2073075.patch queued for re-testing.

Unable to apply above patch. Please review rerolled patch.

Those forms were moved to field_ui. Try re-rolling with git rebase, by applying the patch on an old version first, that might just work.

I think some of these were fixed when the forms were moved, but here is a reroll of the other ones.

i will work on this issue.

my first re-roll i hope it's fine !

Sorry for the nitpick - your patch contains an extra empty line that got snuck into there.
Back to Needs Work for reroll.

--

Here's an unrelated remark.

Looking through the field-ui/src directory, I see there are more places that

• reference ' t('
• are using StringTranslationTrait (or more precise: a parent class is now using StringTranslationTrait)
• so the older t() can be replaced by $this->t().

This is optional. If you are looking for a new issue and want to do that too (or want to give this work to someone else), you can.

This does not have to be done in this same issue (because this is about forms, and the other classes are not forms). You can open a new issue for that, if there isn't one yet. If you want to discuss the title and/or scope of that new issue, talk to someone.

(I meant to do this: )

you don't have to be sorry. :D

thanks for your quickly review !

i'm ok to work on your remark. I will see that tomorow.

OK I think this is clear enough now - in scope as well as code fixes - for me to set RTBC.

sorry for #19 path i update directly the path file.

someone help ?

I don't understand why failed.

Failed message : Requesting https.php with a legitimate simpletest User-Agent returns OK.

It succeeded now. Probably a minor glitch from the busy test bot. (Random HTTP request that failed.)

So now I can actually set RTBC :)

- return t('Deleting a @entity-type will cause any output still requesting to use that @entity-type to use the default display settings.', array('@entity-type' => $entity_type->getLowercaseLabel()));
+ return $this->t('Deleting a @entity-type will cause any output still requesting to use that @entity-type to use the default display settings.', array('@entity-type' => $entity_type->getLowercaseLabel()));
}

+
}

there is an extra newline that is not needed

@#29 - no, that's patch #16; the added newline is not present anymore in #23.

The patch looks good to me so far!

Since there are so few things to do at this point, can we also incorporate EntityDisplayModeListBuilder and FieldStorageConfigListBuilder in this patch? There are 2 occurrences of t() in the first file and 5 in the second one.

Looks good, thanks!

@amateescu: agreed that this can be done here; great it's RTBC.

But do you know if/how we should attack the other *ListBuilder() calls (and/or other 'users' of EntityHandlerBase / StringTranslationTrait) in in other modules?

In other words: should #2471467: Phase out global t() in classes using StringTranslationTrait be re-scoped, or closed?

I tried to search for related issues, but didn't find one. I am not enough in issue queues to know if it's part of an existing clean-up.

It could be a back-ender novice issue for new sprinters. These are hard to find nowadays :)

I think it's safe to close #2471467: Phase out global t() in classes using StringTranslationTrait as a duplicate of this, and sure, we could open per-module issues if we find other usages of t() in OO code in core :)

retitled #2471467: Phase out global t() in classes using StringTranslationTrait and made it non-novice. we can use it to find out how big the task would be.

I thought it was a good first issue, but now i have a headache. Thanks roderik ! :D

I have a beginner question. To do this, it's not a conflict with parent issue in summary "#2072251: [meta] Modernize forms to use FormBase" ? Because we don't change only form.
We must change the summary of this issue ? Or we change parent summary # 2072251 ? Or it's OK ?

Thanks everybody !

@vanilla-bear It's a very good question and actually something we 'forgot'!

My interpretation of this is:

• the parent issue was big. So it only focused on forms.
• but this parent issue is mostly done, and (quote) "Since there are so few things to do at this point" for field_ui module, amateescu asked to include the 2 t() calls in field_ui module (in *ListBuilder) too.

It's indeed a good idea to update the issue summary. (This one, not the parent.) If you want to do this update, to reflect the above, great. (Don't change issue status please.)

Thanks everyone for your contributions to this issue!

The current scope of this issue isn't really an allowed change during the Drupal 8 beta (because t() is not deprecated for 8.0.0). During the beta, and especially for normal tasks, we should always do a beta evaluation before proceeding with an issue.

However, in the case of this issue, I'm going to grant an exception. It's a very small change with no disruption, it's an improvement, and we have some new contributors working on it, so I'd like to commit that work. :) In the future though, please note that converting code from procedural to OO is not in scope anymore for the beta. You can help in future issues by making sure they always have beta evaluations before working on them, especially for normal tasks since many of them are no longer in scope before 8.0.0.

Committed and pushed to 8.0.x. Congratulations @vanilla-bear on what looks like your first committed patch to Drupal 8!