Only translations for enabled extensions should be loaded from language pack

[rob006]

fixes #1837
replaces #1954

This is continuation of #1954. I fixed condition for detecting core files (validation.yml and core.yml) and tested this locally - it is working as expected.

[rob006]

OK, I found one problem. Extensions that use translations but does not register Locales extender, does not trigger locale cache refresh after enabling/disabling the extension - user needs to refresh cache manually to load translations for these extensions. This affects only core extensions - they have their translations in https://github.com/flarum/lang-english/tree/master/locale. Every other extension has bundled English translation and register it using Locales extender.

I'm not sure how to proceed with this. Personally, I would just move these translation files to extensions, like in every other 3rd party extension - it seems to be more consistent and pragmatic (you don't need to open 2 PRs every time you add a new setting to extension, and you can release extensions and language pack independently).

[askvortsov1]

Is there a reason why core extensions don't use Locales?

[tankerkiller125]

I'm personally in favor of moving translations to their respective extensions.

[askvortsov1]

I'm personally in favor of moving translations to their respective extensions.

Should this be split into a separate issue?

[rob006]

I'm personally in favor of moving translations to their respective extensions.

Another reason to do this is that English fallback (#1961) will not work for these extensions, since there will be no English translations available (unless you have English language pack installed and enabled).

[askvortsov1]

I can confirm that this works, and I think it's a good idea!

[franzliedke]

I'm personally in favor of moving translations to their respective extensions.

There was lots of discussion about this when we made that decision. I don't remember the reasoning, but I do remember it was a very deliberate decision, weighing trade-offs and all that.

Could somebody do me the favor and try to dig up that old discussion? It must have been with participation by @dcsjapan and @tobyzerner (then @tobscure). This should help us reach a conclusion here.

[clarkwinkelmann]

Suggestion: call the Locale extender in each of the core extensions even if they have no locales files so that they clear the cache. That way we can ship this bug-free without having to make an immediate decision on whether we move language files out of the English pack.

Maybe we need an additional method on the Locale extender so that we can register an empty locale file list without having to create an empty locales folder.

[franzliedke]

We just talked about this in our dev meeting.

• We agree with the approach.
• We want to discuss translations for bundled extensions in a separate issue (Bundled extensions should use the locale extender instead of the lang-english language pack #2090) in time for the stable release.
• For bundled extensions, the problem currently isn't solved. IMO, that's not a huge problem. We can ignore that (as Bundled extensions should use the locale extender instead of the lang-english language pack #2090 may solve it, too). or go with Clark's approach.

---

Maybe we need an additional method on the Locale extender so that we can register an empty locale file list without having to create an empty locales folder.

@clarkwinkelmann I think that won't be necessary. For the current code in the extender, any directory without YAML files should do - and it might even work with non-existing directories. 😁

[franzliedke]

Possible follow-up tasks for interested people (hint hint @rob006 @clarkwinkelmann @askvortsov1):

Updates for bundled extensions
If we want to fix this for the bundled extensions (I don't care too much), feel free to send PRs to all bundled extensions (except for the English language pack) with the following:

1. A new file locale/.keep.
2. A new line new Extend\Locales(__DIR__ . '/locale'), in the extension's extend.php script.

Documentation update
The convention on how to name translation files is now a stricter requirement, if I get this right. So we should mention this in the documentation.