Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

[Translation][TwigBridge] Implementing variables extraction in "translation:update" #38884

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

Closed
wants to merge 23 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
06fba65
Implementing variables extraction in "translation:update"
liarco Oct 29, 2020
e2934b5
Applying patch from fabbot
liarco Oct 29, 2020
bedb0be
Fixing regression: Twig t() now works when domain is set with trans_d…
liarco Oct 30, 2020
87d542f
Fixing variables metadata overwriting previously set data (PHP extrac…
liarco Oct 30, 2020
8142f75
Updating existing tests to include variables
liarco Oct 30, 2020
a333f13
Applying patch from fabbot
liarco Oct 30, 2020
38d9e55
Supporting both long and short array syntax (PHP extractor)
liarco Oct 30, 2020
edb5aa9
Improving/fixing existing tests and adding variables tests (PHP extra…
liarco Oct 30, 2020
31f96a0
Adding variables tests (Twig extractor)
liarco Oct 31, 2020
5c2e782
Clearer naming
liarco Oct 31, 2020
c3e2e88
Avoid overwriting metadata completely when adding variables note (Twi…
liarco Oct 31, 2020
c50ce67
Supporting "t()" functions (Twig) without "|trans()" filter (e.g. as …
liarco Oct 31, 2020
6e4013c
Fixing duplicate notes. Extractors now keep the higher variables coun…
liarco Oct 31, 2020
3e0bb8e
Improving readability
liarco Oct 31, 2020
deb72d3
Always update variables with latest from code
liarco Oct 31, 2020
e8cee75
Fixing bug preventing update of variables from source in some cases
liarco Nov 13, 2020
63a52b9
Fixing unintended CS errors (thanks @wouterj)
liarco Nov 13, 2020
2a10f37
Applying patch from fabbot (reviewed manually)
liarco Dec 18, 2020
0a5366f
Minor fixes
liarco Mar 9, 2021
46d26ec
Using constants for metadata key and prefix
liarco Mar 9, 2021
1612c08
Fixing CI
liarco Mar 9, 2021
8ec6086
Improving code quality
liarco Mar 9, 2021
0025c52
Fixing static analysis (Psalm)
liarco Mar 15, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Using constants for metadata key and prefix
  • Loading branch information
liarco committed Mar 9, 2021
commit 46d26ec05131dd2d1ddaa76efb5f69be31786a1e
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,6 @@

class TwigExtractorTest extends TestCase
{
private const VARIABLES_NOTE_CATEGORY = 'symfony-extractor-variables';
private const VARIABLES_NOTE_PREFIX = 'Available variables: ';

/**
* @dataProvider getExtractData
*/
Expand Down Expand Up @@ -58,8 +55,8 @@ public function testExtract($template, $messages)
// Check variables
if ($notes = $catalogue->getMetadata($key, $data[0])['notes'] ?? null) {
foreach ($notes as $note) {
if (isset($note['category']) && self::VARIABLES_NOTE_CATEGORY === $note['category']) {
$this->assertEquals(self::VARIABLES_NOTE_PREFIX.$data[1], $note['content']);
if (isset($note['category']) && MessageCatalogue::METADATA_AVAILABLE_VARIABLES_KEY === $note['category']) {
$this->assertEquals(MessageCatalogue::METADATA_AVAILABLE_VARIABLES_PREFIX.$data[1], $note['content']);

break;
}
Expand All @@ -76,7 +73,7 @@ public function getExtractData()
* 1 => [
* 'message_key' => [
* 0 => 'domain',
* 1 => 'var1, var2'|null, // Complete message is 'Available variables: var1, var2'
* 1 => 'var1, var2'|null, // Complete message is 'Variables: var1, var2'
* ...
* ]
* ],
Expand Down
8 changes: 4 additions & 4 deletions 8 src/Symfony/Bridge/Twig/Translation/TwigExtractor.php
Original file line number Diff line number Diff line change
Expand Up @@ -87,15 +87,15 @@ protected function extractTemplate(string $template, MessageCatalogue $catalogue
if (\count($extractedMessage[2]) > 0) {
$metadata = $catalogue->getMetadata($message, $domain);
$variablesNote = [
'category' => 'symfony-extractor-variables',
'content' => 'Available variables: '.implode(', ', $extractedMessage[2]),
'category' => MessageCatalogue::METADATA_AVAILABLE_VARIABLES_KEY,
'content' => MessageCatalogue::METADATA_AVAILABLE_VARIABLES_PREFIX.implode(', ', $extractedMessage[2]),
];

// Update old variables note (if any)
if (isset($metadata['notes'])) {
foreach ($metadata['notes'] as $index => $note) {
if (isset($note['category']) && 'symfony-extractor-variables' === $note['category']) {
// Keep the higher variables count
if (isset($note['category']) && MessageCatalogue::METADATA_AVAILABLE_VARIABLES_KEY === $note['category']) {
// Keep the highest variables count
if (\count($extractedMessage[2]) > substr_count($note['content'], ',')) {
$metadata['notes'][$index] = $variablesNote;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ protected function processVariablesMetadata(string $domain)

// Get target variables note
foreach ($this->target->getMetadata($id, $targetDomain)['notes'] ?? [] as $note) {
if (isset($note['category']) && 'symfony-extractor-variables' === $note['category']) {
if (isset($note['category']) && MessageCatalogue::METADATA_AVAILABLE_VARIABLES_KEY === $note['category']) {
$variablesNote = $note;

break;
Expand All @@ -176,7 +176,7 @@ protected function processVariablesMetadata(string $domain)
// Update old variables note (if any)
if (isset($sourceMetadata['notes'])) {
foreach ($sourceMetadata['notes'] as $index => $note) {
if (isset($note['category']) && 'symfony-extractor-variables' === $note['category']) {
if (isset($note['category']) && MessageCatalogue::METADATA_AVAILABLE_VARIABLES_KEY === $note['category']) {
$sourceMetadata['notes'][$index] = $variablesNote;

break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -352,15 +352,15 @@ protected function parseTokens(array $tokens, MessageCatalogue $catalog, string

if (!empty($variables)) {
$variablesNote = [
'category' => 'symfony-extractor-variables',
'content' => 'Available variables: '.implode(', ', $variables),
'category' => MessageCatalogue::METADATA_AVAILABLE_VARIABLES_KEY,
'content' => MessageCatalogue::METADATA_AVAILABLE_VARIABLES_PREFIX.implode(', ', $variables),
];

// Update old variables note (if any)
if (isset($metadata['notes'])) {
foreach ($metadata['notes'] as $index => $note) {
if (isset($note['category']) && 'symfony-extractor-variables' === $note['category']) {
// Keep the higher variables count
if (isset($note['category']) && MessageCatalogue::METADATA_AVAILABLE_VARIABLES_KEY === $note['category']) {
// Keep the highest variables count
if (\count($variables) > substr_count($note['content'], ',')) {
$metadata['notes'][$index] = $variablesNote;
}
Expand Down
12 changes: 12 additions & 0 deletions 12 src/Symfony/Component/Translation/MessageCatalogue.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,18 @@
*/
class MessageCatalogue implements MessageCatalogueInterface, MetadataAwareInterface
{
/**
* This metadata key is used to store a note containing available variables for each message
*/
public const METADATA_AVAILABLE_VARIABLES_KEY = 'symfony-extractor-variables';

/**
* The comma-separated list of available variables is appended to this prefix.
*
* Example: "Variables: foo, bar"
*/
public const METADATA_AVAILABLE_VARIABLES_PREFIX = 'Variables: ';

private $messages = [];
private $metadata = [];
private $resources = [];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,14 +189,14 @@ public function testExtraction($resource)
'translatable concatenated message with heredoc and nowdoc' => null,
'translatable test-no-params-short-array' => null,
'translatable test-no-params-long-array' => null,
'translatable test-params-short-array' => 'Available variables: foo',
'translatable test-params-long-array' => 'Available variables: foo',
'translatable test-multiple-params-short-array' => 'Available variables: foo, foz',
'translatable test-multiple-params-long-array' => 'Available variables: foo, foz',
'translatable test-params-trailing-comma-short-array' => 'Available variables: foo',
'translatable test-params-trailing-comma-long-array' => 'Available variables: foo',
'translatable typecast-short-array' => 'Available variables: a',
'translatable typecast-long-array' => 'Available variables: a',
'translatable test-params-short-array' => MessageCatalogue::METADATA_AVAILABLE_VARIABLES_PREFIX.'foo',
'translatable test-params-long-array' => MessageCatalogue::METADATA_AVAILABLE_VARIABLES_PREFIX.'foo',
'translatable test-multiple-params-short-array' => MessageCatalogue::METADATA_AVAILABLE_VARIABLES_PREFIX.'foo, foz',
'translatable test-multiple-params-long-array' => MessageCatalogue::METADATA_AVAILABLE_VARIABLES_PREFIX.'foo, foz',
'translatable test-params-trailing-comma-short-array' => MessageCatalogue::METADATA_AVAILABLE_VARIABLES_PREFIX.'foo',
'translatable test-params-trailing-comma-long-array' => MessageCatalogue::METADATA_AVAILABLE_VARIABLES_PREFIX.'foo',
'translatable typecast-short-array' => MessageCatalogue::METADATA_AVAILABLE_VARIABLES_PREFIX.'a',
'translatable typecast-long-array' => MessageCatalogue::METADATA_AVAILABLE_VARIABLES_PREFIX.'a',
'translatable default-domain-short-array' => null,
'translatable default-domain-long-array' => null,

Expand All @@ -213,14 +213,14 @@ public function testExtraction($resource)
'translatable-fqn concatenated message with heredoc and nowdoc' => null,
'translatable-fqn test-no-params-short-array' => null,
'translatable-fqn test-no-params-long-array' => null,
'translatable-fqn test-params-short-array' => 'Available variables: foo',
'translatable-fqn test-params-long-array' => 'Available variables: foo',
'translatable-fqn test-multiple-params-short-array' => 'Available variables: foo, foz',
'translatable-fqn test-multiple-params-long-array' => 'Available variables: foo, foz',
'translatable-fqn test-params-trailing-comma-short-array' => 'Available variables: foo',
'translatable-fqn test-params-trailing-comma-long-array' => 'Available variables: foo',
'translatable-fqn typecast-short-array' => 'Available variables: a',
'translatable-fqn typecast-long-array' => 'Available variables: a',
'translatable-fqn test-params-short-array' => MessageCatalogue::METADATA_AVAILABLE_VARIABLES_PREFIX.'foo',
'translatable-fqn test-params-long-array' => MessageCatalogue::METADATA_AVAILABLE_VARIABLES_PREFIX.'foo',
'translatable-fqn test-multiple-params-short-array' => MessageCatalogue::METADATA_AVAILABLE_VARIABLES_PREFIX.'foo, foz',
'translatable-fqn test-multiple-params-long-array' => MessageCatalogue::METADATA_AVAILABLE_VARIABLES_PREFIX.'foo, foz',
'translatable-fqn test-params-trailing-comma-short-array' => MessageCatalogue::METADATA_AVAILABLE_VARIABLES_PREFIX.'foo',
'translatable-fqn test-params-trailing-comma-long-array' => MessageCatalogue::METADATA_AVAILABLE_VARIABLES_PREFIX.'foo',
'translatable-fqn typecast-short-array' => MessageCatalogue::METADATA_AVAILABLE_VARIABLES_PREFIX.'a',
'translatable-fqn typecast-long-array' => MessageCatalogue::METADATA_AVAILABLE_VARIABLES_PREFIX.'a',
'translatable-fqn default-domain-short-array' => null,
'translatable-fqn default-domain-long-array' => null,

Expand All @@ -237,14 +237,14 @@ public function testExtraction($resource)
'translatable-short concatenated message with heredoc and nowdoc' => null,
'translatable-short test-no-params-short-array' => null,
'translatable-short test-no-params-long-array' => null,
'translatable-short test-params-short-array' => 'Available variables: foo',
'translatable-short test-params-long-array' => 'Available variables: foo',
'translatable-short test-multiple-params-short-array' => 'Available variables: foo, foz',
'translatable-short test-multiple-params-long-array' => 'Available variables: foo, foz',
'translatable-short test-params-trailing-comma-short-array' => 'Available variables: foo',
'translatable-short test-params-trailing-comma-long-array' => 'Available variables: foo',
'translatable-short typecast-short-array' => 'Available variables: a',
'translatable-short typecast-long-array' => 'Available variables: a',
'translatable-short test-params-short-array' => MessageCatalogue::METADATA_AVAILABLE_VARIABLES_PREFIX.'foo',
'translatable-short test-params-long-array' => MessageCatalogue::METADATA_AVAILABLE_VARIABLES_PREFIX.'foo',
'translatable-short test-multiple-params-short-array' => MessageCatalogue::METADATA_AVAILABLE_VARIABLES_PREFIX.'foo, foz',
'translatable-short test-multiple-params-long-array' => MessageCatalogue::METADATA_AVAILABLE_VARIABLES_PREFIX.'foo, foz',
'translatable-short test-params-trailing-comma-short-array' => MessageCatalogue::METADATA_AVAILABLE_VARIABLES_PREFIX.'foo',
'translatable-short test-params-trailing-comma-long-array' => MessageCatalogue::METADATA_AVAILABLE_VARIABLES_PREFIX.'foo',
'translatable-short typecast-short-array' => MessageCatalogue::METADATA_AVAILABLE_VARIABLES_PREFIX.'a',
'translatable-short typecast-long-array' => MessageCatalogue::METADATA_AVAILABLE_VARIABLES_PREFIX.'a',
'translatable-short default-domain-short-array' => null,
'translatable-short default-domain-long-array' => null,

Expand All @@ -261,50 +261,50 @@ public function testExtraction($resource)
'concatenated message with heredoc and nowdoc' => null,
'test-no-params-short-array' => null,
'test-no-params-long-array' => null,
'test-params-short-array' => 'Available variables: foo',
'test-params-long-array' => 'Available variables: foo',
'test-multiple-params-short-array' => 'Available variables: foo, foz',
'test-multiple-params-long-array' => 'Available variables: foo, foz',
'test-params-trailing-comma-short-array' => 'Available variables: foo',
'test-params-trailing-comma-long-array' => 'Available variables: foo',
'typecast-short-array' => 'Available variables: a',
'typecast-long-array' => 'Available variables: a',
'test-params-short-array' => MessageCatalogue::METADATA_AVAILABLE_VARIABLES_PREFIX.'foo',
'test-params-long-array' => MessageCatalogue::METADATA_AVAILABLE_VARIABLES_PREFIX.'foo',
'test-multiple-params-short-array' => MessageCatalogue::METADATA_AVAILABLE_VARIABLES_PREFIX.'foo, foz',
'test-multiple-params-long-array' => MessageCatalogue::METADATA_AVAILABLE_VARIABLES_PREFIX.'foo, foz',
'test-params-trailing-comma-short-array' => MessageCatalogue::METADATA_AVAILABLE_VARIABLES_PREFIX.'foo',
'test-params-trailing-comma-long-array' => MessageCatalogue::METADATA_AVAILABLE_VARIABLES_PREFIX.'foo',
'typecast-short-array' => MessageCatalogue::METADATA_AVAILABLE_VARIABLES_PREFIX.'a',
'typecast-long-array' => MessageCatalogue::METADATA_AVAILABLE_VARIABLES_PREFIX.'a',
'default-domain-short-array' => null,
'default-domain-long-array' => null,
'message-used-multiple-times' => 'Available variables: var1, var2, var3',
'message-used-multiple-times' => MessageCatalogue::METADATA_AVAILABLE_VARIABLES_PREFIX.'var1, var2, var3',
],
'not_messages' => [
// translatable.html.php
'translatable other-domain-test-no-params-short-array' => null,
'translatable other-domain-test-no-params-long-array' => null,
'translatable other-domain-test-params-short-array' => 'Available variables: foo',
'translatable other-domain-test-params-long-array' => 'Available variables: foo',
'translatable other-domain-typecast-short-array' => 'Available variables: a',
'translatable other-domain-typecast-long-array' => 'Available variables: a',
'translatable other-domain-test-params-short-array' => MessageCatalogue::METADATA_AVAILABLE_VARIABLES_PREFIX.'foo',
'translatable other-domain-test-params-long-array' => MessageCatalogue::METADATA_AVAILABLE_VARIABLES_PREFIX.'foo',
'translatable other-domain-typecast-short-array' => MessageCatalogue::METADATA_AVAILABLE_VARIABLES_PREFIX.'a',
'translatable other-domain-typecast-long-array' => MessageCatalogue::METADATA_AVAILABLE_VARIABLES_PREFIX.'a',

// translatable-fqn.html.php
'translatable-fqn other-domain-test-no-params-short-array' => null,
'translatable-fqn other-domain-test-no-params-long-array' => null,
'translatable-fqn other-domain-test-params-short-array' => 'Available variables: foo',
'translatable-fqn other-domain-test-params-long-array' => 'Available variables: foo',
'translatable-fqn other-domain-typecast-short-array' => 'Available variables: a',
'translatable-fqn other-domain-typecast-long-array' => 'Available variables: a',
'translatable-fqn other-domain-test-params-short-array' => MessageCatalogue::METADATA_AVAILABLE_VARIABLES_PREFIX.'foo',
'translatable-fqn other-domain-test-params-long-array' => MessageCatalogue::METADATA_AVAILABLE_VARIABLES_PREFIX.'foo',
'translatable-fqn other-domain-typecast-short-array' => MessageCatalogue::METADATA_AVAILABLE_VARIABLES_PREFIX.'a',
'translatable-fqn other-domain-typecast-long-array' => MessageCatalogue::METADATA_AVAILABLE_VARIABLES_PREFIX.'a',

// translatable-short.html.php
'translatable-short other-domain-test-no-params-short-array' => null,
'translatable-short other-domain-test-no-params-long-array' => null,
'translatable-short other-domain-test-params-short-array' => 'Available variables: foo',
'translatable-short other-domain-test-params-long-array' => 'Available variables: foo',
'translatable-short other-domain-typecast-short-array' => 'Available variables: a',
'translatable-short other-domain-typecast-long-array' => 'Available variables: a',
'translatable-short other-domain-test-params-short-array' => MessageCatalogue::METADATA_AVAILABLE_VARIABLES_PREFIX.'foo',
'translatable-short other-domain-test-params-long-array' => MessageCatalogue::METADATA_AVAILABLE_VARIABLES_PREFIX.'foo',
'translatable-short other-domain-typecast-short-array' => MessageCatalogue::METADATA_AVAILABLE_VARIABLES_PREFIX.'a',
'translatable-short other-domain-typecast-long-array' => MessageCatalogue::METADATA_AVAILABLE_VARIABLES_PREFIX.'a',

// translation.html.php
'other-domain-test-no-params-short-array' => null,
'other-domain-test-no-params-long-array' => null,
'other-domain-test-params-short-array' => 'Available variables: foo',
'other-domain-test-params-long-array' => 'Available variables: foo',
'other-domain-typecast-short-array' => 'Available variables: a',
'other-domain-typecast-long-array' => 'Available variables: a',
'other-domain-test-params-short-array' => MessageCatalogue::METADATA_AVAILABLE_VARIABLES_PREFIX.'foo',
'other-domain-test-params-long-array' => MessageCatalogue::METADATA_AVAILABLE_VARIABLES_PREFIX.'foo',
'other-domain-typecast-short-array' => MessageCatalogue::METADATA_AVAILABLE_VARIABLES_PREFIX.'a',
'other-domain-typecast-long-array' => MessageCatalogue::METADATA_AVAILABLE_VARIABLES_PREFIX.'a',
],
];
$actualCatalogue = $catalogue->all();
Expand Down Expand Up @@ -390,13 +390,13 @@ private function getVariablesNoteContentFromMetadata(array $metadata)
* 'notes' => [
* 0 => [
* 'category' => 'symfony-extractor-variables',
* 'content' => 'Available variables: var1, var2',
* 'content' => 'Available: var1, var2',
* ],
* ...
* ],
* ...
* ]
*/
return array_filter($metadata['notes'] ?? [], function ($note) { return 'symfony-extractor-variables' === $note['category']; })[0]['content'] ?? null;
return array_filter($metadata['notes'] ?? [], function ($note) { return MessageCatalogue::METADATA_AVAILABLE_VARIABLES_KEY === $note['category']; })[0]['content'] ?? null;
}
}
Morty Proxy This is a proxified and sanitized view of the page, visit original site.