cleanup extract-db-locale-terms.php script

[DAcodedBEAT]

Description & Issue number it closes

@respencer and I chatted and found some areas for cleanup in this script

How to test the changes?

N/A

[DawoudIO]

Does the outcome change

[DAcodedBEAT]

Does the outcome change

No and that's the point - to ensure the outcome is consistently the same regardless of what platform ran it.

[DawoudIO]

I did a test on my local box, there are missing terms (countries) and the order has been changed...

[DAcodedBEAT]

I did a test on my local box, there are missing terms (countries) and the order has been changed...

I don't see how this change and the results you've seen are related

[respencer]

I did a test on my local box, there are missing terms (countries) and the order has been changed...

Is the order important?

I'm making some changes of my own that I hope to push soon, and it would be better if the part of the job that needs database access runs first.

Then if it fails nothing else runs.

Right now you can have an inconsistent end result due to partial failure.

If order is important I can change it so the database part runs first, but it assembles the files in the correct order.

[DawoudIO]

oder is not important as long it does not change from run to run... this way we can tell what changed...

[respencer]

I did a test on my local box, there are missing terms (countries) and the order has been changed...

I just did a build test going back to commit d88ef89 and there is still a change of order and missing countries. Very odd.

Checking the last committed version of locale/messages.po I see some change of order, but the countries are there.

I'll continue investigating.

[respencer]

After those changes all the missing data is back again, but the order is still different.

Unless sort has somehow changed its behaviors:

CRM/locale/update-locale.sh

Line 5 in da27702

find . -iname '*.php' | sort | grep -v ./vendor | xargs xgettext --no-location --no-wrap --from-code=UTF-8 -o ../locale/messages.po -L PHP

I have no idea how the previous order came about.

[respencer]

Best guess is something else has sorted the last committed messages.po file contents by the age of the original file.

A strictly alphabetical sort of the file names would have all the api message strings first, which is what I'm getting.

[DawoudIO]

does this address #1432

[DAcodedBEAT]

does this address #1432

@DawoudIO no, this is strictly for the machine running the script. \r existing in translations are due to inputs in the translations themselves

[respencer]

does this address #1432

@DawoudIO no, this is strictly for the machine running the script. \r existing in translations are due to inputs in the translations themselves

Why are we not replacing \r with \n, so those warnings stop?

[DAcodedBEAT]

does this address #1432

@DawoudIO no, this is strictly for the machine running the script. \r existing in translations are due to inputs in the translations themselves

Why are we not replacing \r with \n, so those warnings stop?

Alternatively, we fix the 1 bad locale which has \r - /~https://github.com/ChurchCRM/CRM/blob/master/src/locale/textdomain/es_ES/LC_MESSAGES/messages.po#L4656

[respencer]

does this address #1432

@DawoudIO no, this is strictly for the machine running the script. \r existing in translations are due to inputs in the translations themselves

Why are we not replacing \r with \n, so those warnings stop?

Alternatively, we fix the 1 bad locale which has \r - /~https://github.com/ChurchCRM/CRM/blob/master/src/locale/textdomain/es_ES/LC_MESSAGES/messages.po#L4656

Okay, that to needs to be fixed. My understanding is \r is not allowed in gettext -- \n is.

The translation system will translate \n to \r on a system that needs it, but \n is the only valid line ending that is allowed to be stored.

So we need to replace \r, and \r with \n to have those warnings go away as the gettext strings will then be completely valid.

[DAcodedBEAT]

does this address #1432

@DawoudIO no, this is strictly for the machine running the script. \r existing in translations are due to inputs in the translations themselves

Why are we not replacing \r with \n, so those warnings stop?

Alternatively, we fix the 1 bad locale which has \r - /~https://github.com/ChurchCRM/CRM/blob/master/src/locale/textdomain/es_ES/LC_MESSAGES/messages.po#L4656

Translated this string:

Session time limit in seconds\rSet to zero to disable session time limits.

🤦

[DawoudIO]

running the script on main

vs on the branch

Please test and diff the output of messages.po

[DAcodedBEAT]

@DawoudIO is this good to go on your end? I think @respencer fixed the issue you found