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

Update po4a scripts for better markdown support #873

Merged
merged 2 commits into from
Dec 22, 2022
Merged

Update po4a scripts for better markdown support #873

merged 2 commits into from
Dec 22, 2022

Conversation

ignotus666
Copy link
Contributor

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

  • I've verified that this Pull Request follows the general code principles
  • I waited some time after this Pull Request was opened and all GitHub checks completed without errors.
  • I'm sure that this Pull Request goes to the correct branch

@ignotus666
Copy link
Contributor Author

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 \
Copy link
Contributor Author

@ignotus666 ignotus666 Nov 16, 2022

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.

Copy link
Member

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?

Copy link
Contributor Author

@ignotus666 ignotus666 Nov 18, 2022

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.

@ignotus666 ignotus666 changed the title Upd po4a scr Update po4a scripts for better markdown support Nov 16, 2022
@@ -48,7 +48,7 @@ jobs:
id: cache-po4a
with:
path: "~/po4a"
key: ${{ runner.os }}-po4a
key: ${{ runner.os }}-po4a-0.68
Copy link
Member

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

Copy link
Contributor Author

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...

Copy link
Contributor Author

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?

Copy link
Member

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
Copy link
Member

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?

Copy link
Contributor Author

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 \
Copy link
Member

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?

@ann0see
Copy link
Member

ann0see commented Dec 7, 2022

Could you please check the validity of this again? Locally I don't see a translation for any language.

@ann0see
Copy link
Member

ann0see commented Dec 7, 2022

Ok. Got it. Maybe the threshold is too high?

@ignotus666
Copy link
Contributor Author

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.
But in the long run we'll have "cleaner" strings for translators.

@ignotus666
Copy link
Contributor Author

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.

@ann0see
Copy link
Member

ann0see commented Dec 10, 2022

Yes, but this will make the production site largely untranslated

@ignotus666
Copy link
Contributor Author

Ahh, true... So maybe we can make this PR against next-release then?

@ann0see
Copy link
Member

ann0see commented Dec 11, 2022

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.

@ann0see
Copy link
Member

ann0see commented Dec 22, 2022

Let’s merge this to next release.

@ann0see ann0see changed the base branch from release to next-release December 22, 2022 11:22
@ann0see ann0see merged commit bf5d6f3 into jamulussoftware:next-release Dec 22, 2022
ann0see added a commit to jamulussoftware/assets that referenced this pull request Dec 22, 2022
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
@ignotus666 ignotus666 deleted the upd-po4a-scr branch December 28, 2022 09:25
@pljones pljones added this to the Release 3.10.0 milestone Jul 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Update po4a for better markdown support
3 participants