Improve build for translations and a couple fixes for German translation #164
Conversation
This file contains 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
Instead of intltool, xmlstarlet and jq. Mirrors what was done for the desktop file in 186c983 and 0e29c8d. gettext supports AppData files since 0.19.6 from 2015 and we already require 0.19.7 anyway. Requires dropping the leading underscores previously processed by intltool and adds the developer name "Aegisub Group" as a new translateable string.
Base POSIX leaves the handling of backslahes in the operand implementation defined. The XSI extension specifies several escape sequences, like e.g. \n, which shall be transformed upon printing. Originally make_pot.sh expected XSI behaviour and indeed e.g. dash’s echo builtin implements this. echo builtins of other common shells such as bash however, do not (by default). Avoid this portability pitfall by just using printf at all relevant places. See: https://pubs.opengroup.org/onlinepubs/9699919799/utilities/echo.html https://pubs.opengroup.org/onlinepubs/9699919799/utilities/printf.html This also allows us to stop substituting each backslash with multiple backslashes before piping to maybe_append. This substituion existed to safely pass backslahes through maybe_append’s echo'ing. As a bonus also improve quoting. Prior commits, especially 0fbcaea 4aee271 940181c already tried to fix this and adjusted various parts to better work with the shell at hand. The second listed commit even proclaimed it would make the script POSIX compliant. This was not actually true, since I left several instances of echo in place and forgot to adjust the amount of backslash escaping. The latter was fixed (for XSI shells) in the third commit. While I’ll refrain from bold claims like "POSIX compliance" this time, I can attest the new version gives identical results between dash and bash at least. Ported from: arch1t3cht@bc3358f
To account for backslash-escape fixes in make_pot.sh from the preceding commit.
Implements suggestions by arch1t3cht. Ported from: arch1t3cht@5b66d47
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.
Ports over new changes from arch1t3cht#52
In summary this does:
aegisub.appdata.xmljq+xmlstarlet+intltoolto justgettextfor extracting and merging back translations from/intoaegisub.appdata.xmlmake_pot.sh(replacing my prior flawed attempt from 2020)The changed line count may look daunting, but the vast majority just comes from running
make_pot.shonce to fix up backslash escapes. Apart from the creation date string, this change can be easily reproduced by runningmake_pot.sh.Afaict CMake doesn't have logic for merging back translations into the desktop file and no logic at all for
appdata.xml? Therefor this only touches theconfigure+Makebuildsystem. Since I’m not sure how gettext is canonically integrated into CMake and this is an unrelated preëxisting issue, I refrained from integrating appdata and desktop file translations into the CMake build.Side note: In fact, I’m unable to successfully build with CMake. It (but not the configure+Make` build!) fails with:
EDIT: The Ubuntu-CMake build didn't suffer from this error and managed to get to building tests before being killed. MacOS build errors in CI (both Cmake and configure+Make) appear unrelated to this PR.