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

tools: update translation-related scripts #2998

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

illwieckz
Copy link
Member

@illwieckz illwieckz commented May 12, 2024

  • disable the generation of commands.pot,
    we dont translate this for now and having to delete the file by hand on every rule of the script is annoying.
  • add update_po.sh script,
    this automatically updates the existing .po files using the .pot files.

It looks like I never found the time to push update_po.sh before.

@illwieckz illwieckz force-pushed the illwieckz/translation-tools branch 2 times, most recently from 38cf696 to e203924 Compare May 12, 2024 13:39
@illwieckz illwieckz force-pushed the illwieckz/translation-tools branch from e203924 to 2183ef2 Compare May 12, 2024 14:24
@illwieckz illwieckz force-pushed the illwieckz/translation-tools branch from 2183ef2 to b63e1e1 Compare May 12, 2024 14:25
-k_ -kN_ -kP_:1,2 -k -f -
for name in 'game' 'commands'
do
if eval "\${${name}}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this do? Why do we need eval?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line turns the game string contained into the variable name into the variable ${game} then eval executes it to get true or false.


temp_pot_file="$(mktemp)"
. "${script_dir}/translation.conf"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this do?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sets basic config vars

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it the same as source? If so please use that instead of cryptically golfing

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it's the same as source. Technically speaking, source is a bash-ism and . foo is more portable, but we do enforce bash anyways so its probably fine.

Copy link
Member Author

@illwieckz illwieckz May 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is the standard syntax for including a file. I can use source indeed since there is no plan to make it compatible with another language than bash.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL, . is OK I guess

@@ -0,0 +1,51 @@
#! /usr/bin/env bash

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this program for?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/usr/bin/env is like which. /usr/bin/env is a "well known path" for env and will work on distros which do not have bash at /usr/bin like NixOS (ie, doing this is more portable than /usr/bin/bash

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol I meant what is update_po.sh for

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update the strings in .po files from the .pot file.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Weblate does that automatically in our current workflow right? I'm not saying we shouldn't have the script but I would like to see a comment about that if is the case.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure Weblate does that automatically. Some months ago I noticed some strings were not in Weblate but in source, after having run that script and pushed, Weblate went in sync. Maybe that was a bug in Weblate, but I now always run ./generate_pot.sh then ./update_po.sh after merging translation changes from Weblate. In case it's not needed anymore it can't do harm anyway, I always run it to make sure everything is fine.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to have a comment about your experience with that.


temp_pot_file="$(mktemp)"
. "${script_dir}/translation.conf"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL, . is OK I guess

then
temp_pot_file="$(mktemp)"

eval "generate_${name}_pot"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The eval seems to be superfluous. Like in external_deps/build.sh we do just "build_${pkg}"

@@ -0,0 +1,51 @@
#! /usr/bin/env bash

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to have a comment about your experience with that.

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.

None yet

3 participants