-
Notifications
You must be signed in to change notification settings - Fork 84
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
Update po4a scripts for better markdown support #873
Update po4a scripts for better markdown support #873
Conversation
Note: the check fails because it looks for po4a version 0.68, which isn't yet in the assets repo. |
@@ -116,6 +119,8 @@ process_with_po4a () { | |||
--po "$PO_DIR/$lang/${filename}.po" \ | |||
--localized "$targ_doc" \ | |||
--localized-charset "UTF-8" \ | |||
--no-deprecation \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is to prevent a stream of warning messages - we use po4a in a way that is considered deprecated, but which actually works better for our workflow - we want separation between .po file updates and their processing into translated files. The 'updated' way of using po4a combines both steps, which we don't want.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm. But will we need to change the behaviour any time soon?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not that I know of. In reality the "updated" way to use po4a is just an automated combined use of the scripts that we use separately.
@@ -48,7 +48,7 @@ jobs: | |||
id: cache-po4a | |||
with: | |||
path: "~/po4a" | |||
key: ${{ runner.os }}-po4a | |||
key: ${{ runner.os }}-po4a-0.68 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could use a hash like in the main repo to make the key unique
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uh, I had a look but don't have a clue how that works...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The cache hasn't been used for a week, which means that any workflow run should now trigger the creation of a new one. So I could leave the key here as "po4a" and merge the new po4a version in the assets repo now. Does that sound ok @ann0see?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use ${{ hashFiles('file1, 'file2') }}... Will open an issue for that soon.
sudo apt install -yq gettext libsgmls-perl libyaml-tiny-perl opensp | ||
wget -O po4a.deb https://github.com/jamulussoftware/assets/raw/main/po4a/po4a_0.66.deb | ||
sudo apt install -yq gettext libsgmls-perl libyaml-tiny-perl opensp libsyntax-keyword-try-perl | ||
wget -O po4a.deb https://github.com/jamulussoftware/assets/raw/main/po4a/po4a_0.68.deb |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please upload the deb?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's in a PR in the assets repo. As I said in the description, I think #870 should be merged first, then the PR in assets, then this one. If I upload it now, the workflows won't find it.
@@ -116,6 +119,8 @@ process_with_po4a () { | |||
--po "$PO_DIR/$lang/${filename}.po" \ | |||
--localized "$targ_doc" \ | |||
--localized-charset "UTF-8" \ | |||
--no-deprecation \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm. But will we need to change the behaviour any time soon?
Could you please check the validity of this again? Locally I don't see a translation for any language. |
Ok. Got it. Maybe the threshold is too high? |
The new markdown processing in this PR causes a lot of strings to become fuzzies, which is why the threshold isn't reached for so many files. They're easy to update, as it's just a matter of removing a few characters, so I'd leave the threshold as it is. It will take some effort to do though, as there are quite a few languages now. |
Also, I think it would be better to get this in before all the changes to the EN files that are happening, so they get processed in the "new" way. |
Yes, but this will make the production site largely untranslated |
Ahh, true... So maybe we can make this PR against next-release then? |
We could. It's not perfect. Especially since this will make the build fail on release – unless we re-upload the old po4a deb and use different links. |
Let’s merge this to next release. |
As the release and next-release branch on jamuluswebsite need different po4a versions, we need to re-upload the old 0.66 deb. Related to: jamulussoftware/jamuluswebsite#873
Short description of changes
Update po4a and workflow scripts to better support markdown, and display errors (but not fail) when there are tag inconsistencies in html files.
The po4a cache has been renamed to force the creation of a new one with the updated version of po4a in the assets repo. The updated po4a scripts will not work with the old version as new options have been introduced, so this PR and this one should be merged at the same time. In any event, I think #870 should be merged before this PR to avoid possible conflicts (e.g. it would happen if this PR gets merged and afterwards the EN files get updated).
The stats script has been removed as with Weblate it's become redundant.
The next time the .po files are updated, a number of strings in each language will become fuzzies - it will mostly just be a matter of removing characters (`#, ```` and maybe others) in the fuzzy strings.
Context: Fixes an issue? Related issues
Fixes #817
Status of this Pull Request
Should be ready.
What is missing until this pull request can be merged?
Does this need translation?
No.
Checklist