-
Notifications
You must be signed in to change notification settings - Fork 22
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
Support for multiple JSON files #24
Comments
I didn't know Your code seems the right approach to it.
I'm wondering if this is really important. Since translations will be managed on Translation.io (only source JSON files will be touched by developers), the structure of target JSON files is not really relevant. Everything could be stored in The other more complex solution would be to store the path to the JSON file in the context of translations, like we do to differentiate JSON translations and GetText translations internally:
We could make the context more specific and transform it from:
to:
to be able to reuse this information when writing the target JSON. What do you think about this? |
My preference would definitely be to keep the translation files separate. Even though everything is managed via translation.io, it's really helpful to know which strings are associated with which major feature of the application. If we ever do a significant refactor or decide to sunset a feature, we need to be able to understand which translations will be affected. |
I understand that, and adding the path in the context will also allow you to filter the segments by feature on Translation.io interface. But we need to keep Feel free to create a PR for this feature, or else we will work on it in the following weeks. Could you please give me some of the |
I'm tinkering with a PR, but honestly—it almost feels like it'd be easier to write our own sync command that just calls the API. But it looks like the sync API isn't documented. Is this something that's stable and can be used by customers? |
I've opened #25 for the sake of discussion, while I explore the options. |
Also, if they sync API isn't supported, are there rate limit considerations around calling |
Thank you for this! I may help continue the PR. Does your project already sync correctly with these changes? It I understand correctly, all your source JSON files will be synced correctly but the translations will be saved in the standard
The API that is consumed by this package is not documented and uses GetText PO files to embed all translations (PHP, JSON and GetText) that your project may use. We created a more modern documented API that is optimized for And a classic API to simple manipulation of segments here: https://translation.io/docs/api
The only rate limite consideration is that only one sync can be executed simultaneously per project (actually, you can start many at the same time but they will be executed one after another using a mutex, with a timeout if it takes too long). But my guess is that it will be way more simple to finish this PR. I may help finish it. Could you please answer to this?
I'm not sure what directory structure you are using for your JSON files. |
As for our directory structure, we use this library: https://github.com/InterNACHI/modular This means our app can look like
And we may use source strings from both JSON files and key strings that look like I haven't tried using our branch yet, but I was able to get one side of the sync working with similar changes direct in our vendor folder. It's one 1/4 of the work, though. The JSON translations are discovered and send up, but are written back to the wrong file, and the namespaced PHP translations aren't picked up at all. |
Thank you for your directory structure. As much as I can see the point of supporting JSON files from different paths (the single Dealing with different modules in the hierarchy of PHP localization files already exists in Laravel with the Would you consider using this naming convention in the main
If so, your JSON files could be structured like this by completing your PR:
Does this make sense to you? If so, I could get the JSON files to be written back to the correct locations by extending your PR. |
Yeah, I get where you're coming from. I think I'll take a stab at a custom sync command that works with our project, since it's somewhat outside of the scope of a "typical" Laravel project. |
That works too, but be careful that PHP has We allow the source edition of I'll still try to finish this Pull Request about JSON files, it still makes sense for typical Laravel projects I guess. |
The See In an ideal world, this package would be compatible with both JSON and PHP files registered anywhere, as long as they're registered properly with the |
The good news I almost finished the JSON implementation and it works (read&write). I still need to:
I'll try to finish it tomorrow. The bad news I still feel that the But maybe this could be implemented the same way as the "subdirectories" feature, I'm not sure. This commit should give you an idea of what part it will need to touch: c14042e Quick question: Is |
They're mostly used in packages. See: https://laravel.com/docs/8.x/packages#translations |
Thank you for the link and the precision. I think that's a bit weird to have 2 completely separate processes for PHP and JSON paths, but it conforts me that supporting only multiple JSON paths (for now?) would already be an improvement. If this works well with JSON, I might try to implement the same logic for PHP files without bringing too much complexity. I'll keep you in touch. Thank you for your help on this issue! |
Here's a somewhat messy but working implementation of the two-way-sync that we're going to use. It might be useful for your implementation, too: https://gist.github.com/inxilpro/a989488c34975d57321bdea6c6ffb36f |
If new JSON paths have been added in the project using `Lang::addJsonPath()`
I just added a revision to my gist above. At this point, it's working very nicely for us. It handles:
It has partial support for source edits, but not 100%. The main thing this doesn't do is any gettext-related functionality, since that's not the native Laravel way to handle translations. |
Thanks @inxilpro, I'm glad you have something that works for your use case. We now also have a way to sync JSON files from different paths in the new 1.16 release thanks to you. Unfortunately, we don't intend to go further and implement the loading of PHP files from different modules. Maybe in the future but right now it will just make the code more complex, and I'm not sure that feature is needed by anyone else. Have a great week-end! |
We have a very large application that has multiple JSON translation files, registered with
Lang::addJsonPath()
. These files are split based on multiple modules within the app.It looks like it'd be relatively simple to update
GettextPOGenerator::jsonFiles()
to use all the registered JSON paths. Something like:The trickier issue would be resolving which JSON file to write back to on sync. Is this something you're interested in supporting? I've been digging around in
GettextTranslationSaver
and it seems like it'd take a bit of a refactor to support this use-case.The text was updated successfully, but these errors were encountered: