Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for Hebrew translation (he_IL) #54

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

Conversation

SamGoody
Copy link

Gibbon has been broken entirely when using the Hebrew translation, since upgrading to Php v8 (all pages show an error that the Gibbons have escaped).

The issue has to do with the way Php changed the handling of replacement strings.
This fork fixes the issues, and is tested with both Gibbon v24 and Gibbon v28.

The changes were all made in `22 (for Gibbon-24). Have now rebased the changes onto main, and tested with Gibbon-28, so that others can benefit as well.

Sam Goody added 10 commits November 26, 2024 19:31
sprintf expects placeholders to use the format `%1$s`.
Prior to v8, placeholders with a space would be interpreted incorrectly, but would not throw an error.
In v8 PHP throws a fatal error, causing all the Gibbons to escape.
Live example: https://onlinephp.io/c/523c3
This commit removes the extraneous spacing on 361 placeholders.
The 21 placeholders fixed were missing the "s" identifying the replacement type.
Less obvious issues, such as placeholders that span multiple lines or that were missing the `%`
@SKuipers
Copy link
Member

SKuipers commented Dec 2, 2024

Hi Sam, thank you for your work to fix these translation issues. As you've noted, there were errors in the string replacement formats, which previous to PHP 8 would have been ignored, but now generate a fatal error. We use the online tool PoEditor, so I've downloaded and merged your .po file into the online translations, then generated new .mo and .po files from PoEditor, which have now been pushed to this repository. Can you please download and test these new files and let me know if they have solved the issue: 4bba3d7

@SamGoody
Copy link
Author

SamGoody commented Dec 3, 2024 via email

@SKuipers
Copy link
Member

SKuipers commented Dec 4, 2024

Hi Sam, thanks for these updates and your continued translations! Yes, adding them to PoEditor would be best. Generally, I publish the translations from PoEditor every release, which pushes them to GitHub. However, feel free to let me know when you've completed some of your changes and I'm always happy to publish them mid-release. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants