New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Cast TranslationWrapper to string #1619
Conversation
See http://php.net/manual/en/language.oop5.object-comparison.php#71623 on why the recursion happens. |
Fixed the issue for me! 👍 |
New patch in https://www.drupal.org/node/2571375 |
ping @greg-1-anderson @weitzman Can you take a look at it? Thanks! |
How is Drush dt() being called in the call chain above? It seems Drupal should be using t() directly here, so I don't see how the fix solves the problem (although empirically, it does). If Drush dt() is in fact being called from Drupal code, then it seems like it might be undesirable to typecast the value to a string, since Drupal 8's translate() method does not work this way. As far as Drush's internal use of dt() is concerned, though, the typecast to a string should be fine. |
Looks like this was fixed in core; seems better to leave the typecast off here. We can re-open if problems remain. |
Well having it be a TranslatableString object makes sense only for HTML output. I don't think we have to fear XSS injection in Drush ;-), so I think the explicit string makes things more consistent in Drush |
Yes, perhaps -- any place where the string is used by Drush, I agree. But I am worried about the call chain above; since I do not understand how the t() from Drupal passed through Drush, I don't know if there would be any bad effects (e.g. in tests) if we stripped away the object. Since the purpose of the object is to provide data for output, maybe it never matters if we lose information like this. |
The object is created by |
Yeah, I understand how dt() gets the TranslatableString from Drupal; I just don't understand what call path through Drupal calls dt() in such a way that the Form handler gets the result. |
https://www.drupal.org/node/2571375#comment-10354155 should have fixed this. |
Yes, it's fixed and closed; I'm just wondering how the output from dt() made it to the form handler. I think it's from the parameters such as the site name, but I haven't actually traced through it. While this question is closed for this issue, it is still relevant in #1637. |
Exactly, it's from |
OK, thanks. In that case, I think it is more appropriate to typecast here, or stop translating, as suggested in #1637. We can continue discussions there. |
@greg-1-anderson I rebased the change. Its ready to merge. |
Reversing my position; I think we might want to merge this after all. GitHub won't let me re-open after a force-push (which is odd, since it recovers from force-pushes when an issue is open), but we will still be able to merge this in via the command line. Discussion in #1637. |
It is not possible to install latest D8 with Drush because PHP can´t compare to TranslationWrapper objects. I always hit the internal recursion limit.