Skip to content
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

Localization support for plugins #1357

Closed
wants to merge 3 commits into from
Closed

Localization support for plugins #1357

wants to merge 3 commits into from

Conversation

sla89
Copy link
Contributor

@sla89 sla89 commented Oct 7, 2017

Currently the plugins have to implement localization support by their own. The CoreCommandRouter called the method loadI18NStrings on each plugin if the method exists. But this was not documented at all so the method is not used so far.

I have replaced this method by a new method called getI18nFile. The idea is that each plugin returns the i18n filename that should be loaded into volumio on startup. Then volumio reads that file and merges the locales into the variable i18nStrings. Now the plugins can call self.commandRouter.getI18nString('MY_LOCALE_KEY') and they get the localized string for it.

With this feature we can now provide localized toast messages in the spotify, qobuz and youtube plugin.

When you accept the PR I will update the docs accordingly where we also have to setup some guidelines so that we have no merge issues. My code already takes aware of duplicated keys and only adds new keys to i18nStrings.

The implementation of getI18nFile could look like this and could be part of the example plugin too:

Youtube.prototype.getI18nFile = function (lang_code) {
  var supportedLanguages = ['en', 'it', 'de'];

  if (supportedLanguages.includes(lang_code)) {
    return path.join(__dirname, 'i18n', 'strings_' + lang_code + '.json');
  }

  // return default i18n file
  return path.join(__dirname, 'i18n', 'strings_en.json')
}

@volumio
Copy link
Owner

volumio commented Oct 7, 2017

@sla89 can you please solve the conflicts? Also, can you contact me directly at info at volumio dot org ?
THANKS!

@sla89
Copy link
Contributor Author

sla89 commented Oct 7, 2017

@volumio I have resolved them and I have sent you a mail.

@volumio
Copy link
Owner

volumio commented Jan 29, 2018

@sla89 sorry if I get back so late with this.
There is a problem with this PR: the diff is huge because of your different linting. If you could redo it with the same linting I would better appreciate its content and it could be safely merged. While you're there it would be great if you could solve the conflict... thanks

@volumio
Copy link
Owner

volumio commented Feb 5, 2018

@sla89 Just thought now that we have an issue with this:
this works when volumio starts, but what happens if we just install and then enable the plugin?

@volumio volumio reopened this Feb 5, 2018
@gvolt
Copy link
Contributor

gvolt commented Apr 12, 2018

I can confirm the problem volumio pointed out on 6 Feb. The localization files are scanned only when Volumio starts and the plugin is already enabled. Therefore after just installing and enabling the plugin or after booting Volumio with the plugin in disabled state and then enabling it had the effect that localized strings for toasts etc. were not available (toasts did not show up at all then).

Workaround is to call the method "self.commandRouter.loadI18nStrings()" on every start of a plugin (onStart method), see PR 187 (volumio/volumio-plugins#187) and 188 (volumio/volumio-plugins#188) in the volumio-plugins repo.

@sla89
Copy link
Contributor Author

sla89 commented Apr 12, 2018

Sorry for the late response. I don't think that this workaround is a good idea, because you will always load all localizations again and not just the one of the plugin.

Further I forgot to cleanup the store for all the terms.

I guess a better approach would be to extend my PR by moving the code for loading the terms of a plugin into a method and call this method in CoreCommandRouter.prototype.enablePlugin and cleanup things in CoreCommandRouter.prototype.disablePlugin.

What do you think about that? I guess that should fix all the problems. I would do the loading of the terms after the plugin manager successfully loaded/disabled the plugin.

@gvolt
Copy link
Contributor

gvolt commented Apr 12, 2018

Yes, I fully agree. It is not a nice solution at all, but I did not find another one ;-)

I am all for handling it in CoreCommandRouter.prototype.enablePlugin / ...disablePlugin.

While we are at it. I extended your suggested implementation of getI18nFile in order to automatically determine the supported languages and not have to adjust the plugin's code only because a new language has been added:

<plugin_name>.prototype.getI18nFile = function(lang_code) {
    var supportedLanguages = fs.readdirSync(__dirname + "/i18n");

    // determine supported languages
    for (var i = 0; i < supportedLanguages.length; ++i) {
        if (supportedLanguages[i].startsWith("strings_") && supportedLanguages[i].endsWith(".json")) {
            supportedLanguages[i] = supportedLanguages[i].replace("strings_", "").replace(".json", "");
        } else {
            supportedLanguages.splice(i, 1);
        }
    }
    if (supportedLanguages.includes(lang_code)) {
        return path.join(__dirname, "i18n", "strings_" + lang_code + ".json");
    }

    // return default i18n file
    return path.join(__dirname, "i18n", "strings_en.json");
};

Let me know what you think about it.

@gvolt
Copy link
Contributor

gvolt commented May 20, 2018

@sla89 Did you have the opportunity to have a look at this issue?

@volumio
Copy link
Owner

volumio commented Dec 26, 2018

Closing for inactivity...

@volumio volumio closed this Dec 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants