AliSoftware / OHAutoNIBi18n

Automate the Localization/Translation of your XIB & interface without any additional code nor IBOutlet!
Other
120 stars 24 forks source link

More customization to autoloading #4

Open nobre84 opened 10 years ago

nobre84 commented 10 years ago

Hi! Very interesting project you published, thanks very much! I made some changes that were required for my use case, please check out wether any of it is of interest to the library:

Unfortunately the observer feature introduces a dependency on a platform 6.0 library, LRNotificationObserver which disposes of the observers in the right way easily. I don't know too much about CocoaPods, I think it can be made optional somehow as a sub-pod. It was important to me, I don't know if it is to you

AliSoftware commented 10 years ago

Hi @nobre84

I was not very reactive on OHAutoNIBi18n lately, sorry about that.

Why did you close this PR? Seems interesting to me, maybe I can reopen it if it can still be merged (even if I will surely refactor it to fix your LRNotificationObserver dependency issue)?

Thanks!

nobre84 commented 10 years ago

Hi! I had this PR come from master, and I made more severe change in another aspect of the library not related to the PR's title. The lib is originally meant to do a one-way translation, and this is the recommended approach to 99% of apps (i.e take a label's text, use it as the key in the localizable table, and put it back in the label's text). In a specific app scenario, I need dynamic language changes, and the Localizable Keys were lost in the first translation, so I now added an associated dictionary to each object, holding the original keys so one could change the localization several times, looking up different bundles each time.

I also use Cocoapods extensively, and the Macros defined in the podspec's pch isn't very convenient (each pod install will overwrite any changes), so I took it off and change it in a Podfile's post_install hook.

Let me know which of my changes are wanted to improve the library, and I can create a clean branch so you could review / merge in another PR.

AliSoftware commented 10 years ago

The work you did to support multiple translations seems interesting. A PR about this could be useful to others, especially since you seem not to be the only one needing this feature.

Anyway, while using associatedObjects is a good idea to keep the original translation key, I would only use it when we are in the dynamic-language mode. In other words, in the 99% case scenario, I would not add the associatedObjects (that would be useless), and only use your mechanism when the user opt-in for this feature.

I'm interesting in your feedback about this, especially provided that OHAutoNIBi18n loads itself automagically using +load, how would you suggest for the developer to configure which mode he wants to use? Maybe a subspec in the podspec to choose one mode or another (avoiding each pod install to override things)?

PS: Can you be more specific about the Macros defined in the podspec's pch? Didn't get the issue you got with that.

nobre84 commented 10 years ago

Hi! I agree with you, dynamic language support and the overhead from associatedObjects to support it should be an opt-in feature. My implementation on this still requires some refactoring, it was made on a hurry to release. Do you recall a more efficient way of storing the original keys or running the localization code in spite of associatedObjects and NSNotificationCenter? About Macros, I wasn't able to define the macros from an app's project. It will always pick up the definitions from the Pods project which are cleared on each pod install, thus prone to error specially on a multi-developer project. A subspec sounds like a good idea to enable this without having to mess with post install hooks which are very cumbersome. This would mean one subspec for each optional behavior such as debug mode, non-autoload mode, is that right ?

AliSoftware commented 10 years ago

I think using associatedObjects to keep the original keys is a good idea. Actually I can't seem to think about any other idea that would work for any UIView (except keeping a huge NSDictionary of UIView* pointers as keys and original key as value, but that would complexify the solution and the dictionary won't be emptied when views get deallocated, so that's not a good solution)

And yes we would need one subspec for each optional behavior.

Another solution (maybe better?) would be to allow the configuration of OHAutoNIBi18n using class methods. Like adding the ability to call [OHAutoNIBi18n setDebugMode:YES], [OHAutoNIBi18n setAutoLoadLocalizations:NO], or [OHAutoNIBi18n setLocale:…] etc in application:didFinishLaunchingWithOptions: or anywhere else. Those methods would then either set a property on an OHAutoNIBi18n singleton instance dedicated to keep track of those properties, or store those values in NSUserDefaults. This would then mean that the code of OHAutoNIBi18n itself would not rely on macros and #ifdef anymore, but would perform actions depending on these properties tested using some if condition at runtime.

nobre84 commented 10 years ago

I like the idea of class methods to configure these aspects of the library. However, I think the subspec approach would stil be required for autoloading, as swizzing can only be done safely in +(void)load . Then, the main podspec would be autoload ON, with an optional subspec to turn it OFF. And the regular features such as debugMode, locale, etc configured at runtime.

AliSoftware commented 10 years ago

Yep, fine by me!

Let me know if you need some help with the implied refactoring. Unfortunately I won't have much time lately (especially with all the news around iOS8/iPhone6 ^^) to spend on OHAutoNIBi18n otherwise I would have laid out the global new architecture with subspecs and class methods for configuration (and let you implement the manual loading with associatedObjects).

AliSoftware commented 9 years ago

I'm reopening this PR to avoid forgetting about implementing such a behavior some time (unfortunately not anytime soon due to lot of work)