Fixes based on PR comments

willdurand · Feb 14, 2018 · 98494a1 · 98494a1
1 parent 2b2c5d4
commit 98494a1
Show file tree

Hide file tree

Showing 7 changed files with 38 additions and 99 deletions.
diff --git a/DependencyInjection/BazingaJsTranslationExtension.php b/DependencyInjection/BazingaJsTranslationExtension.php
@@ -44,9 +44,5 @@ public function load(array $configs, ContainerBuilder $container)
             ->replaceArgument(4, $config['default_domain'])
             ->replaceArgument(5, $config['active_locales'])
             ->replaceArgument(6, $config['active_domains']);
-
-        $container
-          ->getDefinition('bazinga.jstranslation.translation_frontend_extractor')
-          ->replaceArgument(1, $config['frontend']);
     }
 }
diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php
@@ -35,15 +35,12 @@ public function getConfigTreeBuilder()
                     ->end()
                 ->end()
                 ->arrayNode('frontend')
-                  ->prototype('array')
-                    ->children()
-                      ->arrayNode('extensions')
-                        ->prototype('scalar')
+                    ->prototype('array')
+                        ->children()
+                            ->scalarNode('extensions')->defaultValue(array('js', 'jsx', 'ts'))->end()
+                            ->scalarNode('sequence')->defaultValue('\.trans(?:Choice)?\(')->end()
                         ->end()
-                      ->end()
-                      ->scalarNode('sequence')->end()
                     ->end()
-                  ->end()
                 ->end()
             ->end();
 

diff --git a/Extractor/FrontendExtractor.php b/Extractor/FrontendExtractor.php
@@ -17,11 +17,14 @@ final class FrontendExtractor extends AbstractFileExtractor implements Extractor
 
     private $filesystem;
 
-    private $configuration;
+    private $extensions;
 
-    public function __construct(Filesystem $filesystem, array $configuration = []) {
+    private $sequence;
+
+    public function __construct(Filesystem $filesystem, array $extensions, $sequence) {
         $this->filesystem = $filesystem;
-        $this->configuration = $configuration;
+        $this->extensions = $extensions;
+        $this->sequence = $sequence;
     }
 
     /**
@@ -32,14 +35,7 @@ public function extract($resource, MessageCatalogue $catalogue)
         $files = $this->extractFiles($resource);
 
         foreach ($files as $file) {
-            foreach ($this->configuration as $configuration)
-            {
-              $pathInfo = pathinfo($file);
-              if (in_array($pathInfo['extension'], $configuration['extensions'], true))
-              {
-                $this->parseMessagesFromContent(file_get_contents($file), $catalogue, $configuration['sequence']);
-              }
-            }
+            $this->parseMessagesFromContent(file_get_contents($file), $catalogue);
         }
     }
 
@@ -51,28 +47,20 @@ public function setPrefix($prefix)
         $this->prefix = $prefix;
     }
 
-    private function parseMessagesFromContent($fileContent, MessageCatalogue $catalogue, $sequence)
+    private function parseMessagesFromContent($fileContent, MessageCatalogue $catalogue)
     {
-        $messages = $this->getMessagesForSequence($fileContent, $sequence);
+        $messages = $this->getMessagesForSequence($fileContent);
 
         foreach ($messages as $message) {
             $catalogue->set($message, $this->prefix.$message);
         }
     }
 
-    /**
-     * @return array
-     */
-    protected function getSupportedFileExtensions()
-    {
-      return call_user_func_array('array_merge', array_column($this->configuration, 'extensions'));
-    }
-
     protected function canBeExtracted($file)
     {
         return $this->isFile($file) && in_array(
             pathinfo($file, PATHINFO_EXTENSION),
-            $this->getSupportedFileExtensions()
+            $this->extensions
         );
     }
 
@@ -87,17 +75,17 @@ protected function extractFromDirectory($directory)
 
         $finder->files();
 
-        foreach ($this->getSupportedFileExtensions() as $supportedExtension) {
+        foreach ($this->extensions as $supportedExtension) {
             $finder->name(sprintf('*.%s', $supportedExtension));
         }
 
         return $finder->in($directory);
     }
 
-    private function getMessagesForSequence($fileContent, $sequence)
+    private function getMessagesForSequence($fileContent)
     {
         $argumentsRegex = self::REGEX_DELIMITER
-            .$sequence
+            .$this->sequence
             .self::FUNCTION_STRING_ARGUMENT_REGEX
             .self::REGEX_DELIMITER;
 

diff --git a/Resources/config/services.xml b/Resources/config/services.xml
@@ -26,10 +26,11 @@
             <argument>%kernel.root_dir%</argument>
             <tag name="console.command" command="bazinga:js-translation:dump" />
         </service>
-        <service id="bazinga.jstranslation.translation_frontend_extractor" class="Bazinga\Bundle\JsTranslationBundle\Extractor\FrontendExtractor" public="true">
+        <service id="bazinga.jstranslation.translation_frontend_extractor" class="Bazinga\Bundle\JsTranslationBundle\Extractor\FrontendExtractor">
             <argument type="service" id="filesystem"></argument>
             <tag name="translation.extractor" alias="frontend" />
-            <argument></argument> <!-- frontend -->
+            <argument></argument> <!-- extensions -->
+            <argument></argument> <!-- sequence -->
         </service>
     </services>
 </container>
diff --git a/Resources/doc/index.md b/Resources/doc/index.md
@@ -324,14 +324,13 @@ Translator.locale;
 By default, the `locale` is set to the value defined in the `lang` attribute of
 the `html` tag.
 
-#### Extractors
+#### Extractor
 
-BazingaJsTranslationBundle includes two **extractors** for assets:
+BazingaJsTranslationBundle includes an **extractor** for assets:
 
-- JsExtractor
-- CoffeeExtractors
+- FrontendExtractor
 
-These two extractors allow Symfony to find translation keys used in ``.js|.jsx|.coffee`` files and thus, can be shown in ``translation:debug`` [command output](http://symfony.com/doc/current/book/translation.html#debugging-translations) or in the [translation profiler](http://symfony.com/blog/new-in-symfony-2-7-translation-profiler).
+This extractor allow Symfony to find translation keys used in ``.ts|.js|.jsx|.coffee`` files and thus, can be shown in ``translation:debug`` [command output](http://symfony.com/doc/current/book/translation.html#debugging-translations) or in the [translation profiler](http://symfony.com/blog/new-in-symfony-2-7-translation-profiler).
 
 Examples
 --------

diff --git a/Tests/Extractor/FrontendExtractorTest.php b/Tests/Extractor/FrontendExtractorTest.php
@@ -3,90 +3,51 @@
 namespace Bazinga\JsTranslationBundle\Tests\Extractor;
 
 use Bazinga\Bundle\JsTranslationBundle\Extractor\FrontendExtractor;
-use Bazinga\Bundle\JsTranslationBundle\Tests\WebTestCase;
+use PHPUnit\Framework\TestCase;
+use Symfony\Component\Filesystem\Filesystem;
 use Symfony\Component\Translation\MessageCatalogue;
 
-final class FrontendExtractorTest extends WebTestCase
+final class FrontendExtractorTest extends TestCase
 {
     const TEST_LOCALE = 'en';
     const TEST_KEY_1 = 'test-key-1';
 
     /**
      * @var FrontendExtractor
      */
-    protected $extractor;
+    private $extractor;
 
     public function setUp()
     {
-      $container = $this->getContainer();
-      $this->extractor = $container->get('bazinga.jstranslation.translation_frontend_extractor');
+        $filesystem = new Filesystem();
+        $this->extractor = new FrontendExtractor($filesystem, array('js', 'jsx', 'ts'), '\.trans(?:Choice)?\(');
     }
 
-    /**
-     * @dataProvider resourcesWithNotValidTransFunctionUsage
-     */
-    public function testExtractShouldNotRetrieveTransKey($resources)
+    public function testExtractShouldNotRetrieveTransKey()
     {
         $catalogue = new MessageCatalogue(self::TEST_LOCALE);
-        $this->extractor->extract($resources, $catalogue);
+        $this->extractor->extract(__DIR__.'/../Fixtures/Extractor/NotValidTransFunctionUsage', $catalogue);
         $this->assertEmpty($catalogue->all());
     }
 
-    /**
-     * @dataProvider resourcesWithATransFunctionUsage
-     */
-    public function testExtractShouldRetrieveTransKey($resources)
+    public function testExtractShouldRetrieveTransKey()
     {
         $catalogue = new MessageCatalogue(self::TEST_LOCALE);
-        $this->extractor->extract($resources, $catalogue);
+        $this->extractor->extract(__DIR__.'/../Fixtures/Extractor/ATransFunctionUsage', $catalogue);
         $this->assertTrue($catalogue->has(self::TEST_KEY_1));
     }
 
-    /**
-     * @dataProvider resourcesWithNotValidTransChoiceFunctionUsage
-     */
-    public function testExtractShouldNotRetrieveTransChoiceKey($resources)
+    public function testExtractShouldNotRetrieveTransChoiceKey()
     {
         $catalogue = new MessageCatalogue(self::TEST_LOCALE);
-        $this->extractor->extract($resources, $catalogue);
+        $this->extractor->extract(__DIR__.'/../Fixtures/Extractor/NotValidTransChoiceFunctionUsage', $catalogue);
         $this->assertEmpty($catalogue->all());
     }
 
-    /**
-     * @dataProvider resourcesWithATransChoiceFunctionUsage
-     */
-    public function testExtractShouldRetrieveTransChoiceKey($resources)
+    public function testExtractShouldRetrieveTransChoiceKey()
     {
         $catalogue = new MessageCatalogue(self::TEST_LOCALE);
-        $this->extractor->extract($resources, $catalogue);
+        $this->extractor->extract(__DIR__.'/../Fixtures/Extractor/ATransChoiceFunctionUsage', $catalogue);
         $this->assertTrue($catalogue->has(self::TEST_KEY_1));
     }
-
-    public function resourcesWithNotValidTransFunctionUsage()
-    {
-        return array(
-            array(__DIR__.'/../Fixtures/Extractor/NotValidTransFunctionUsage'),
-        );
-    }
-
-    public function resourcesWithATransFunctionUsage()
-    {
-        return array(
-            array(__DIR__.'/../Fixtures/Extractor/ATransFunctionUsage'),
-        );
-    }
-
-    public function resourcesWithNotValidTransChoiceFunctionUsage()
-    {
-        return array(
-            array(__DIR__.'/../Fixtures/Extractor/NotValidTransChoiceFunctionUsage'),
-        );
-    }
-
-    public function resourcesWithATransChoiceFunctionUsage()
-    {
-        return array(
-            array(__DIR__.'/../Fixtures/Extractor/ATransChoiceFunctionUsage'),
-        );
-    }
 }
diff --git a/Tests/Fixtures/app/config/base_config.yml b/Tests/Fixtures/app/config/base_config.yml
@@ -19,6 +19,3 @@ bazinga_js_translation:
         - messages
         - numerics
         - foo
-    frontend:
-        - { extensions: [ 'coffee' ], sequence: '\.trans(?:Choice)?\s' }
-        - { extensions: [ 'js', 'jsx', 'ts' ], sequence: '\.trans(?:Choice)?\(' }