Fix locale #1557
Fix locale #1557
Conversation
|
Could we wait with this PR until after #1494 has been merged? I guess a lot of it will be solved by then and merging this PR now only makes it harder to get #1494 out of the door. #1494 is only waiting for LycheeOrg/Lychee-front#322 to be merged. |
|
@nagmat84 this PR just makes a minor changes to some of the local and add tests to ensure consistency. I fail to see how this impacts much #1494 which is in draft mode. :) |
| } | ||
|
|
||
| /** | ||
| * Test Languages are strictly. |
There was a problem hiding this comment.
This comment doesn't seem to explain anything?
|
|
||
| foreach ($keys as $key) { | ||
| try { | ||
| static::assertArrayHasKey($key, $full, 'Language ' . $lang_test->code() . ' as too many keys.'); |
There was a problem hiding this comment.
| static::assertArrayHasKey($key, $full, 'Language ' . $lang_test->code() . ' as too many keys.'); | |
| static::assertArrayHasKey($key, $full, 'Language ' . $lang_test->code() . ' has too many keys.'); |
Would it make sense to also print the extra key? Or is that done already?
This PR is only in draft mode as a safety measure such you do not accidentally merge it. It already has been approved by you and @kamil4 and I am not doing anything on it but keeping it up-to-date with current changes. The obstacle is that #1494 uses unicode char points which our current frontend can not properly handle in all places. Hence, #1519 needs to be merged first, but then there is nothing else which prevents #1494 from being merged, too. |
There was a problem hiding this comment.
I am in strict favor to not change the current locale files. I just jumped into the middle of them and first six keys which have been removed are actually used currently. I stopped checking at this point. A simple grep did the job.
Moreover, this clean-up is already made in #1494 which has already been approved.
However, I am fine with improving the test for language consistency.
| @@ -303,7 +302,6 @@ public function get_locale(): array | |||
| 'SUCCESS' => 'OK', | |||
| 'RETRY' => 'Opakovať', | |||
|
|
|||
| 'SETTINGS_WARNING' => 'Zmena rozšírených nastavení môže mať negatívny vplyv na stabilitu, bezpečnosť a rýchlosť tejto aplikácie. Upravujte len to, o čom presne viete, čo robíte.', | |||
There was a problem hiding this comment.
This one is actually used in view.js by view.full_settings.content.init. We must not delete it.
| 'SETTINGS_SUCCESS_CSS' => 'CSS aktualizované', | ||
| 'SETTINGS_SUCCESS_UPDATE' => 'Nastavenia úspešne aktualizované', |
There was a problem hiding this comment.
Same thing here, see settings.js method settings.changeCSS. Don't delete it.
| 'CSS_TEXT' => 'Vlastné CSS:', | ||
| 'CSS_TITLE' => 'CSS zmeniť', |
There was a problem hiding this comment.
And again. See view.js method view.settings.content.setCSS.
| } | ||
| } | ||
|
|
||
| static::assertEquals('en', Lang::get_code()); | ||
| static::assertEquals('OK', Lang::get('SUCCESS')); | ||
| $this->assertFalse($failed); |
There was a problem hiding this comment.
Error: Static method 'assertFalse' should not be called dynamically
| $this->assertFalse($failed); | |
| static::assertFalse($failed); |
Which is also rather obvious compared with the two lines above.
| foreach ($keys as $key) { | ||
| try { | ||
| static::assertArrayHasKey($key, $full, 'Language ' . $lang_test->code() . ' as too many keys.'); | ||
| } catch (ExpectationFailedException $e) { | ||
| $msgSection->writeln('<comment>Error:</comment> ' . $e->getMessage()); | ||
| $failed = true; | ||
| } |
There was a problem hiding this comment.
Instead of looping across all the keys we could simply use the PHP native method array_diff_key. It should make the code a little bit more efficient (not that I care such much about it in the test), but it should also make the code much more clearer and easier to grasp.
Moreover, this test method and the test method above could be combined into one. Basically, they do the same thing just with swapped roles. So that is the pseudo code:
public function testLanguageConsistency(): void {
$englishDictionary = Lang::get_lang();
$availableDictionaries = Lang::get_lang_available();
$failed = false;
foreach ($availableDictionaries as $locale) {
$dictionary = Lang::factory()->make($locale);
$missingKeys = array_diff_key(englishDictionary, $dictionary);
$extraKeys = array_diff_key($dictionary, englishDictionary);
if (size($missingKeys) !== 0) {
$msgSection->writeln(sprintf('<comment>Error:</comment> Locale %s misses the following keys: %s', $locale, implode($missingKeys, ', ')));
$falied = true;
}
if (size($extraKeys) !== 0) {
$msgSection->writeln(sprintf('<comment>Error:</comment> Locale %s has the following extra keys: %s', $locale, implode($extraKeys, ', ')));
$falied = true;
}
}
static::assertFalse($failed);
}
Then I will close that one and directly open a PR on #1494 But then it will have to be merged before #1539 :) |
Add tests to remove overflowing-unused translations.