[ALOY-1365] Improve how Alloy works with i18n and platform folders
GitHub Issue | n/a |
---|---|
Type | Improvement |
Priority | Critical |
Status | Closed |
Resolution | Fixed |
Resolution Date | 2016-03-16T21:15:28.000+0000 |
Affected Version/s | n/a |
Fix Version/s | Release 5.2.1, alloy 1.8.0 |
Components | I18N |
Labels | n/a |
Reporter | Fokke Zandbergen |
Assignee | Chris Barber |
Created | 2016-03-09T09:18:00.000+0000 |
Updated | 2017-03-15T21:43:02.000+0000 |
Description
The recent regression reported under TIMOB-20537 and the open request to theme
Defautlcon.png
filed under ALOY-1318, ask for a better way in which Alloy handles i18n
and platform
folders.
For X
read both i18n
and platform
:
* Like /Resources
, the /X
folders should be generated by Alloy.
* For Alloy apps, i18n and platform resources should be in the /app/X
folders.
* On compile, Alloy should replace the /X
folders with /app/X
, then override with app/theme/my-theme/X
(ALOY-858) and default with app/widgets/my-widget/X
(ALOY-967 + ALOY-1059).
* Alloy will no longer need TIMOB-17446 since Titanium can use the default paths.
We should require an app/X
folder to be present before we generate (and thus overwrite/replace}} /X
. This will make sure that an existing Alloy app that only has /X
and /app/themes/my-theme/X
or /app/widgets/my-widget/X
will not end up loosing the resources it had in /X
. Of course the change will still be breaking because the resources in themes and widgets will no longer make it to the app. But the migration path is simple because the developer simply has to move the /X
folders to /app/X
. We could even do that for the developer, which would make it not breaking at all, although we still need to notify the developer of the change.
[~fokkezb] [~cbarber] and I had a long offline conversation about this back in Dec 2015. This was all discussed. You reach out to him for details or I can let you have a transcript. tiapp.xml should ALSO be pushed into * *project/app/tiapp.xml* and then if required into * *project/app/themes/red/tiapp.xml* This fixes the last real hurdle of theming as long as all the above is also sorted. I do not care if the themed versions are overwritten or merged - the ability to theme is the key.
We should discuss that separately as this is currently urgent because of TIMOB-20537. We already have ALOY-1118 to discuss theming
tiapp.xml
, including the chicken-egg problem (since the CLI will have loadedtiapp.xml
by the time Alloy compiles.PR: https://github.com/appcelerator/alloy/pull/766 [~fokkezb] Care to give this a go? You can put
platform
andi18n
directories in yourapp
folder. When you build, theplatform/<platform-name>
andi18n
folders in the root of the project get nuked and the files copied from theapp/platform
andapp/i18n
directories. There's no way logic to preserve theplatform
andi18n
directories. Users will need to move those folders into theapp
directory. You can override the Alloy appplatform
andi18n
files by using a theme. Lastly, a custom widget may have ani18n
directory structure which gets merged with theapp/i18n
andapp/themes/<theme>/i18n
stings. This PR does not do any sort of differential build or smart file copying. It's not great, but it's also not terrible. At least it's only nuke theplatform/<platform-name>
instead of nuking the entireplatform
directory. There are no changes at the Titanium SDK level. This is purely solved in Alloy.Thanks [~cbarber]. I've tested all combinations I could think off and left a few comments on the PR. Two ideas for simple improvements not related to your changes. But more importantly two ideas to make sure this breaking change (which it still is after all) at least not deletes people's project
i18n
andplatform
folders without notice. We really can't just be deleting those. There's still people without versioning and backups there :D I also think we should bump Alloy to 1.8 to signal the impact of this change. Of course any breaking change should bump major and I'm fine with that as well, but since we've been adding features to patch versions since 1.7 I thought 1.8 would be more appropriate. In anyway, the error in the ideas I suggested needs to have the version we release this with. [~cng] we need to make sure CLI 5.2.1 has this Alloy version, of course. I'll do a blog post on Alloy theming to give this good exposure as well.[~cbarber] [~fokkezb] you can easily handle the concerns of deleting the root i18n and platform when putting this back in which will allow all existing projects to remain without change but be ready to update when the dev wants them to. * Check to see if there is a theme folder, if there is check to see if the current theme has an i18n or a platform folder individually. If i18n matches then delete the matching root folder and copy the theme folder. Then repeat with the platform folder. If one or both does not exist then NO root folders are touched. If only one folder exists inside theme then ONLY that matching one is deleted - not the other. This allows the dev to only change one of the choose to and makes sure if the do not use the facility then no folders are deleted. * If a matching folder inside theme was not found then check if i18n or platform folders are inside the app folder. If at least one is that was not already processed via the theme point above then repeat the same logic as the first point. * In essence you ONLY delete a root folder if a matching folder exists in theme and if not in theme in app. I18n and platform folder checking are treated independently of each other. You do not delete root platform just because i18n folder was found you MUST also have found platform. * New projects should have i18n and platform folders automatically created inside the app folder and documentation updated to match. * Apps that have i18n or platform folders in the root but not in either theme or app folders should see a warning during processing to show those folders should be moved and be given a URL to the docs to see why This way all existing projects can continue as they do right now as root folders will not be deleted as there are no theme or app folder versions. When devs start to take advantage of the ability to relocate the platform and i18n folders. A few if statements resolves all the problems and the warning console entry forwarns the need to update the app structure.
Thanks for the feedback. I updated the PR with a bunch of fixes. First and foremost, I have now made it so that Alloy will create a file called
alloy_generated
in the/i18n
and/platform
directories. If this file exists, then I can nuke it without worry. If thealloy_generated
file is missing, then the build fails with a message saying you must move those folders into yourapp
directory. It's simple and robust. I also: * Fixed the i18n XML parsing to be more robust * Added/i18n
and/platform
to the.gitignore
for new projects * Print a warning if there's a.gitignore
and it's missing/i18n
,/platform
, or/Resources
* Fixed a small bug with Alloy's hook when parsing the output from the subprocessed Alloy compile command * Bumped the version number to 1.8.0[~cbarber] looking good. One thing though, related to [~core13]'s remarks. If you
ti create
a Titanium project it includes/platform
. Then if you runalloy new
it will currently leave that folder there. After this PR the first time youalloy compile
it will fail saying you need to move/platform
. So, I think to finish this off we should modify [new](https://github.com/appcelerator/alloy/blob/master/Alloy/commands/new/index.js) to: * Move/platform
and/i18n
under/app
* [Ensure](https://github.com/appcelerator/alloy/blob/master/Alloy/commands/new/index.js#L23)/app/platform
,/app/i18n
(and I'd say also/app/lib
) like it does with/app/models
as a hint even if you don't have these folders.[~fokkezb] Done! Please review one last time.
Here's the docs: {quote} Alloy has improved i18n and platform-specific resources support. You can now add "i18n" and "platform" directories in your "/app" directory and in your active theme's directory. Additionally, widgets may also have an "i18n" directory. When you build your app, Alloy will delete the "/i18n" and "/platform/
One note in regards to: {quote}followed by the each widget's "/app/widgets/
[~fokkezb] do you mean that i18n files are simply copied and if one already exists then it is overwritten? Or would widget i18n entries be merged into theme i18n entries if they exist and then merged into app i18n entries? I think a few example files in all possible locations with suitable strings should be given complete with the expected fully merged i18n that would then be used by the app itself. Provided suitable string examples are given then this will make the trickle down rules very clear.
[~core13] what I mean is that the behaviour is unchanged: * i18n XML files are merged * Although i18n XML files from widgets come last, they don't override existing strings, allowing you to override strings from (third-party) widgets in your theme or main i18n files. I have a blog post ready
[~fokkezb] When updating features do you have a dependency checklist that you through to make sure that new rules work with dependant features as well? {noformat} alloy extract-i18n en —apply {noformat} This does not honour the correct locations of the app is an Alloy project. It always exports to the \{project/i18n/\} folder rather than the \{project/app/i18n/\} folder. If you use a dependency checklist (sometimes called a knock-ons list) a lot of these issues that then need immediate triage would simply never occur.
[~core13] No, apparently there was no unit test for
alloy extract-i18n
, which of course we should have. Luckily [~sfeather] found this bug already and we've got it fixed: ALOY-1375Verified fixed, using: MacOS 10.11.4 (15E65) Studio 4.5.0.201602170821 Ti SDK 5.2.1.v20160318225121 Appc NPM 4.2.4-2 Appc CLI 5.2.1-21 Alloy 1.8.2 Xcode 7.3 (7D175) The projects i18n and platform directories function as stated by Chris Barber, in the comments. They are copied from the /app directory, merged with theme and widget directories when appropriate, and sent back into the root folders with the addition of an alloy_generated file that indicates they have undergone this process. Builds fail with a message if the alloy_generated file is not present. Tested with multiple apps using different combinations of i18n and platform folder locations.