ruby-i18n / i18n

Internationalization (i18n) library for Ruby
MIT License
977 stars 408 forks source link

Allow overriding of entry resolving entry resolving separate from defaults #622

Closed movermeyer closed 2 years ago

movermeyer commented 2 years ago

What are you trying to accomplish?

Fixes #617. Alternative to #620.

591 was incorrect, since I didn't check/realize that resolve is called in three different cases:

  1. When the value found in the translation file is a Symbol (Code)
    • This was the case I handled in #591. In this case, you need/want to restart your search from the original fallback locale.
  2. When a value is not found, then values from defaults are resolved. (Code)
    • When resolving defaults, you want to do it only at the level of the current locale, before falling back.
    • i.e., Go through all defaults before falling back through fallbacks locales, keeping the behaviour as it has always been.
  3. When the value found in the translation file is something else, especially a Proc (Code)
    • This is the same case as 1., where you are resolving a entry, but I mention it separately since it is being called from a different place in the code.

Prior to #591, there was no distinction between the cases. #591 should have introduced that distinction.

What approach did you choose and why?

I added a resolve_entry method which can be overridden to handle the case of resolving an entry separately from resolving defaults. By default resolve_entry is an alias of resolve.

I then changed the calls to resolve to use resolve_entry in cases where they are resolving entries.

What should reviewers focus on?

🤷

The impact of these changes

Fixes #617.

Testing

I added the regression test from #620 to show that it works with the existing Rails calls.

radar commented 2 years ago

LGTM, thank you @movermeyer for investigating this on a Sunday :)

radar commented 2 years ago

The tests confirm it, but just to double check as well, I've used the repro app from #617 along with this new branch -- I can (double extra) confirm this branch fixes the issue.