Multiple fixes - not ideal to be combined in a single PR, but here we are. #65
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rmpel
commented
Jul 30, 2025
rmpel
commented
- Update POT, add translations and update the compiled files.
- Add NPM commands for i18n management.
- Contact requests should not go through info@acato.nl. A better option should be created, but for now, use service with a tag.
- The fix that started this entire PR - Early Output breaking the website and more issues, see code comments for details.
… should be created, but for now, use service with a tag.
…te and more issues, see code comments
richardkorthuis
requested changes
Aug 18, 2025
| @@ -1 +1 @@ | |||
| {"translation-revision-date":"2024-08-13 16:54+0200","generator":"WP-CLI\/2.8.1","source":"src\/blocks\/owc-openkaarten\/streetmap\/assets\/scripts\/edit.js","domain":"messages","locale_data":{"messages":{"":{"domain":"messages","lang":"nl_NL","plural-forms":"nplurals=2; plural=(n != 1);"},"Select the datalayers you want to display on the map":["Selecteer de datalayers die getoond dienen te worden op de kaart"],"No datalayers selected":["Geen datalayers geselecteerd"],"Select datalayers:":["Selecteer datalayers"],"Selected datalayers":["Geselecteerde datalayers"]}}} No newline at end of file | |||
Member
There was a problem hiding this comment.
Klopt dit wel? Er worden nu Nederlandse vertalingen verwijderd.
Author
There was a problem hiding this comment.
ik zal kijken wat ik kan repareren, maar bijvoorbeeld de string "Select the datalayers you want to display on the map" zit niet in het PO bestand in main. gezien ik afgetakt ben van main ...
Hoe het in de JSON terecht gekomen is, is mij onbekend, maar er zijn 2 wijzigingen in de PO file op main
1 is de initiele opzet van jou; zit deze string niet in
2 is een eerdere merge van mij, die niet aan deze tekst iets wijzigt (want wat er niet is kun je niet wijzigen)
Wederom een vertaal-bestand-raadsel ...
Author
There was a problem hiding this comment.
npm run make-json verwijderd de missende regels uit de PO file ... - investigating ...
includes/class-i18n.php
Outdated
|
|
||
| return false; | ||
| /** | ||
| * To be discussed; Why did I remove this? (RP 2025-07-30) |
Member
There was a problem hiding this comment.
Ik zou dit soort comments niet in publieke code plaatsen. Verwijder gewoon de naar jouw mening foute code en plaats je redenering eventueel in de commit message.
To be discussed; Why did I remove this? (RP 2025-07-30) 1. $locale using get_locale() is the site locale, not the user locale. - The user locale could be different, so this is not correct. I noticed this during debugging, load_plugin_textdomain uses determine_locale(), not get_locale(). 2. The path displayed here is not correct; the correct path is $path/$textdomain-$locale.mo. So in my case, the error was `Could not find openkaarten-frontend-plugin/languages//nl_NL.mo` while it should have been `Could not find openkaarten-frontend-plugin/languages/openkaarten-frontend-plugin-en_US.mo` 3. We'll ignore the double slash for now 4. This error message is printed before any legitimate output, breaking every POST, potentially breaking every AJAX/REST call and at the minimum showing the error in raw HTML. At least this should be an error_log, but preferably a wrapper into Admin Notices. In my case it only broke wp-admin as I have set Admin to English whereas the site is in Dutch, but in case of a non-NL site, this would break the site. 5. Since all source texts are in English; at least, the error should not show on `en_*` locales. 6. Finally, the filter construction here is interesting; It is only used here, so it doesn't do anything, also, the expected intention is to allow the user to change the locale using a filter, but that is not how WordPress works (unfortunately).
… decided to remove the items from PO and MO. MO I can understand, because JS strings are not needed in PHP, but removing them from PO means removing them from JSON as well, next run. wp-cli/i18n-command#453
richardkorthuis
approved these changes
Aug 18, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.