Allow setting global storage #80

didrocks · 2023-07-18T07:36:43Z

Is this a fix, improvement or something else?

This is an improvement as discussed on #52 about adding Getter and Setter for global config storage object.

What does this change implement/fix?

By setting globalConfig.storage object, it's up to the loading wrapper to set Locale.Domains map to desired manually loaded Translators like embedding po as []bytes or so, using dedicated files…
Another approach is to call Configure(), and by accessing the global storage object, call AddTranslator on it by setting a local Translator object you created.

Then, the project can use gotext.Get() or any root level methods transparently.

This is very useful if you want to test your code from your own repo (using embedded po files if you detect the root of your projects), while still being able to build your project and deploy to an installation, which will load from /usr/share/…

I have ...

answered the 2 questions above,
discussed this change in an issue,
included tests to cover this changes.

leonelquinteros · 2023-07-18T13:26:28Z

gotext.go

@@ -142,6 +142,30 @@ func SetLibrary(lib string) {
 	loadStorage(true)
 }

+// GetGlobalLocalStorage is the locale storage getter for the package configuration.
+func GetGlobalLocalStorage() *Locale {


Can we rename to GetStorage/SetStorage here? To maintain naming convention from other getter/setters.... unless you though of a reason to not do so.

Sure, I found "Storage" to be a little bit too technical as this is the internal code, where what we are doing is really setting a locale. I’m fine changing it either way and understand the symmetry argument. As this is an advanced setting, I think people using it can dive into the upstream code :)

gotext.go

leonelquinteros · 2023-07-18T13:29:01Z

Thank you @didrocks ! Thanks for adding the tests as well.
I left a couple of comments/questions to close the idea, but it's what was expected overall, let's talk about these and I'll merge the PR.

By setting globalConfig.storage object, it's up to the loading wrapper to set Locale.Domains map to desired manually loaded Translators like embedding po as []bytes or so, using dedicated files… Then, the project can use gotext.Get() or any root level methods transparently. This is very useful if you want to test your code from your own repo (using embedded po files if you detect the root of your projects), while still being able to build your project and deploy to an installation, which will load from /usr/share/… Fixes: leonelquinteros#52.

Ensure we override and set expected properties on the default config object.

didrocks · 2023-07-18T13:41:47Z

Thank you @didrocks ! Thanks for adding the tests as well. I left a couple of comments/questions to close the idea, but it's what was expected overall, let's talk about these and I'll merge the PR.

No worry! Thanks for creating and maintaining this project. I just rebased and pushed renaming the Setter and Getter as discussed on the previous comment. Let me know what you think about my answer on the second one.

leonelquinteros · 2023-07-18T14:01:00Z

@didrocks Thanks! Yes, about setting other props... either case present their inconveniences, I'll take your approach as the author of the change...

didrocks · 2023-07-18T14:02:05Z

@leonelquinteros: Thanks a lot for merging and the prompt feedback! :)

leonelquinteros reviewed Jul 18, 2023

View reviewed changes

didrocks added 2 commits July 18, 2023 15:39

Tests for setting and getting locale as global configuration.

6638bd2

Ensure we override and set expected properties on the default config object.

didrocks force-pushed the allow-setting-global-storage branch from 7f54498 to 6638bd2 Compare July 18, 2023 13:41

leonelquinteros merged commit 1f7d156 into leonelquinteros:master Jul 18, 2023

didrocks deleted the allow-setting-global-storage branch July 18, 2023 14:01

m-horky mentioned this pull request Aug 30, 2023

Add support for multiple languages #85

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow setting global storage #80

Allow setting global storage #80

didrocks commented Jul 18, 2023 •

edited

Loading

leonelquinteros Jul 18, 2023

didrocks Jul 18, 2023

leonelquinteros commented Jul 18, 2023

didrocks commented Jul 18, 2023

leonelquinteros commented Jul 18, 2023

didrocks commented Jul 18, 2023

Allow setting global storage #80

Allow setting global storage #80

Conversation

didrocks commented Jul 18, 2023 • edited Loading

Is this a fix, improvement or something else?

What does this change implement/fix?

I have ...

leonelquinteros Jul 18, 2023

Choose a reason for hiding this comment

didrocks Jul 18, 2023

Choose a reason for hiding this comment

leonelquinteros commented Jul 18, 2023

didrocks commented Jul 18, 2023

leonelquinteros commented Jul 18, 2023

didrocks commented Jul 18, 2023

didrocks commented Jul 18, 2023 •

edited

Loading