laravel-translations-loader icon indicating copy to clipboard operation
laravel-translations-loader copied to clipboard

Fix #22 - Better support for vendor based .php files.

Open rabrowne85 opened this issue 4 years ago • 4 comments
trafficstars

Fixing issue #22:

  1. Determine if we're in the vendor folder.
  2. Use regex to search for the format /vendor/{package}/{locale}/filename.php
  3. Use the locale as the new "directory" name (N.B. renamed bundle[directory] to bundle[locale])
  4. Remove the locale from the filename.

This will convert the following: vendor/some-package/en-GB/filename.php to this:

{
    en-GB : {
        some-package/filename: {
            ...
       }
   }
}

I don't use the .json option, so not sure if this has the same problem with the vendor packages.

I haven't written a new test case for this, but can do if it's required.

I'm not sure if this is classified as a breaking change, as it fixes something that wouldn't have been possible to access previously.

Any questions, just let me know.

rabrowne85 avatar May 03 '21 13:05 rabrowne85

@rabrowne85 Sorry it took me this long to check your PR. If you could write a test case for this it would be good.

luisdalmolin avatar May 07 '21 12:05 luisdalmolin

This is a great addition, btw!

luisdalmolin avatar May 07 '21 12:05 luisdalmolin

@luisdalmolin Thanks 👍 I've added some dummy vendor package language files to the test suite and then extended the existing tests to ensure that they now pass.

The only one I've struggled with is the dependency test at the end of the suite. For this I've duplicated the namespace folder without the vendor files and left the test alone.

Let me know if there's anything else you want adding/changing.

rabrowne85 avatar May 07 '21 16:05 rabrowne85

@rabrowne85 The new assertions are looking good, but the builds are failing, though. From a quick look it seems like a few safety checks before running some stuff could be enough.

You can test it locally by running npm run test.

luisdalmolin avatar May 10 '21 10:05 luisdalmolin