Walkthrough

Adds a two-tier fallback resolution for theme-defined translations: look up theme-defined messages for the current language, then the default message set for current language, then theme-defined for a fallback language, then the default set for the fallback language; introduces helper getThemeDefinedMessage and a new messages_themeDefined_fallback parameter.

Changes

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant getI18n
  participant messages_fromKcServer
  participant messages_themeDefined_current
  participant messages_default_current
  participant messages_themeDefined_fallback
  participant messages_default_fallback

  Caller->>getI18n: request translation(key, themeName, currentLang, fallbackLang?)
  getI18n->>messages_fromKcServer: lookup(key)
  alt found in messages_fromKcServer
    messages_fromKcServer-->>getI18n: value
    getI18n-->>Caller: value
  else not found
    getI18n->>messages_themeDefined_current: getThemeDefinedMessage(key, currentLang, themeName)
    alt found in theme-defined current
      messages_themeDefined_current-->>getI18n: value (string or themed map -> selected by themeName)
      getI18n-->>Caller: value
    else
      getI18n->>messages_default_current: lookup(key, currentLang)
      alt found in default current
        messages_default_current-->>getI18n: value
        getI18n-->>Caller: value
      else
        getI18n->>messages_themeDefined_fallback: getThemeDefinedMessage(key, fallbackLang, themeName)
        alt found in theme-defined fallback
          messages_themeDefined_fallback-->>getI18n: value
          getI18n-->>Caller: value
        else
          messages_default_fallback-->>getI18n: lookup(key, fallbackLang)
          getI18n-->>Caller: value
        end
      end
    end
  end

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

I hop through keys and language trees,
Two-tiered paths now sway the breeze.
If current theme lacks the song,
A gentle fallback hums along.
— Rabbit 🐇

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Hello @BrDewit,

Thank you for the well detailed PR.

That being said I think this changes, while fixing the issue of overwriting the curstom translation of an existing key in only a specific language this change also cause important regression when defining custom message key.

.withCustomTranslations({
    en: {
        myCustomMessage: "Hello world"
    }
})

With this PR applied, if the current language is en, msgStr("myCustomMessage") gives "Hello World" but if the current language is, let's say, fr then it gives "myCustomMessage".

Defining custom, non standard message keys is the primary usecase of withCustomTranslations.

What would be a reasonable approach is, if the key is a known one then do what you suggest otherwise keep the current behavior....

But even then, you usually never need to overwrite one of the default message keys, in your case you do just because the translation file are outdated and I have to update them.
But in the general case, if you have overwriten a default message it's probably because you've altered the sementic and want this new semantic to be always visible, even in the wrong language.

This is debatable though but as it stands the rule of withCustomTranslations is. If you want to use it, you must provide a translation for every language you wish to support or we will fall back to english or the first one.

Hi @garronej

That's a very reasonable remark, if they did not specify the custom message, you would indeed want to fall back to an existing translation as that's better than just having the key inserted in that position. Especially as that's probably a more common case than the one where a custom translation is given for an existing key. When I have time, I'll revise the changes to fix this regression.

@garronej excuse me for the delay, but I've finally made the change.

The translation logic will now first check in the custom translation of the provided language, then it will check the default set of the provided language, then it will check the custom translation of the fallback language, after which it will check the default set of the fallback language.

This way, my issue, where a custom translation in 1 language would override the default translation of that key in every language, in is fixed.
While the regression is also fixed, which would cause the key itself to be shown instead of a translation, when no custom translation of a non-default key was provided in the language, but it was in another.

The only breaking change is of course if someone used the behaviour I changed, as a feature, where they use one custom translation to override default keys for every language, but that behaviour is unintuitive. It seems reasonable that with withCustomTranslations you must provide a translation for the key in every language you wish to override.

Hi @garronej I recently remembered that my pull request was still open. I've updated my fork with the latest changes, and the changes I made still seem relevant. If you agree with the changes, the pull request can be merged. If not, let me know what needs to be changed!

Thanks in advance and happy holidays :)