-
Notifications
You must be signed in to change notification settings - Fork 30
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
Update ConfigFieldPlugin.php #23
Conversation
@@ -83,15 +83,15 @@ public function afterGetComment(Subject $subject, $result) | |||
$arguments = $phrase->getArguments(); | |||
} | |||
|
|||
if ($resultIsPhrase) { | |||
$result = new Phrase($result, $arguments); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't this exactly reverse what we did in the lines before?
We are transforming a phrase into text and arguments:
$phrase = $result;
$result = $phrase->getText();
$arguments = $phrase->getArguments();
And then we are transforming it back to a phrase with the same text and arguments as before:
$result = new Phrase($result, $arguments);
The original code transformed a phrase into a string, appended another string and then transformed it back to a phrase.
@sebastian-ehrling Can you explain how your change works?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Andreas,
thx for your comment. With your plugin for the method getComment the original comments will not be translated anymore. This is because the phrase to translate contains the information about the current config path.
With my modification, we translate the comment phrase before adding the path information.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The difference is that $phrase->getText()
returns the untranslated text and $phrase->__toString()
(implicitly called later) returns the translated string.
I assume that the original $result
can be a Phrase
if the comment is translatable or a string if not. So we can also return a string instead of a Phrase (and should! it's not a single translatable text anymore)
If my assuptions are correct, the fix in the PR will work but it can be done simpler:
public function afterGetComment(Subject $subject, $result)
if ($result instanceof Phrase) {
$result = (string) $result;
}
if (strlen(trim($result))) {
$result .= '<br />';
}
$result .= __('Path: <code>%1</code>', $this->getPath($subject));
return $result;
}
Closed because the fix has been implemented in #34. using @schmengler 's code and @sebastian-ehrling 's original approach. |
Improvement of comment translations in the store settings.