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

wmlxgettext does not handle objectives.lua's turn counter #4275

Closed
stevecotton opened this issue Aug 24, 2019 · 13 comments

Comments

@stevecotton
Copy link
Contributor

commented Aug 24, 2019

Running data/tools/wmlxgettext --domain wesnoth -o temp.pot data/lua/wml/objectives.lua will find most of the translatable strings in objectives.lua, but it misses both of turn_counter's strings:

turn_counter = _("(this turn left)", "($remaining_turns turns left)", turn_count)

@CelticMinstrel

This comment has been minimized.

Copy link
Member

commented Aug 24, 2019

Huh? I was pretty sure wmlxgettext was already extended for this case... I recall bringing it up quite some time ago when plural strings in Lua was first implemented.

@jostephd

This comment has been minimized.

Copy link
Member

commented Aug 31, 2019

@stevecotton I don't think the turn counter is the problem. Your command indexes _ "this turn left" but doesn't index _("this turn left"). It seems the parentheses are the problem.

@CelticMinstrel

This comment has been minimized.

Copy link
Member

commented Aug 31, 2019

Um, neither of those are actual strings in objectives.lua? Also I'm not even sure what you mean there.

@CelticMinstrel

This comment has been minimized.

Copy link
Member

commented Aug 31, 2019

Also, this thread requires input from @AncientLich, as the person who implemented this version of wmlxgettext and (I'm pretty sure) also implemented the exact feature in this issue report.

@stevecotton

This comment has been minimized.

Copy link
Contributor Author

commented Aug 31, 2019

@jostephd the examples you've given don't use the plural feature. We're looking at the feature added in 91c9677, and last bugfixed in 9095611 (#3481).

@jostephd

This comment has been minimized.

Copy link
Member

commented Aug 31, 2019

@stevecotton So the example I gave is an independent bug?

@CelticMinstrel I know those strings don't exist in that file. I was giving a minimal example. If I modify the lua file to have _ "foo" that does get picked up by wmlxgettext. If I modify it to have _("foo"), that doesn't get picked up.

@CelticMinstrel

This comment has been minimized.

Copy link
Member

commented Aug 31, 2019

Well, _("foo") should indeed be picked up, ideally, but that's just a syntactic variant of _ "foo", whereas the code in the opening post of this issue is a separate feature equivalent to _n("(this turn left)", "($remaining_turns turns left)", turn_count) in the C++.

@AncientLich

This comment has been minimized.

Copy link
Contributor

commented Sep 2, 2019

EDIT:
I confirm that something weird happened, due to a my fault probably. During my quick first investigation I discovered that probably the wmlxgettext version in the wesnoth repository is NOT the last one. I cannot confirm now (because I am having a big issue with my wesnoth fork so I need to delete it and fork it again), but it is a 90% chance. I testing with a broken wesnoth snapshot from my wesnoth fork and I reproduced all the issues reported (does not work _("string") and also plural string does not work).

I did the same test with wmlxgettext version on my dedicated repository and instead almost all worked (except perhaps the corner case reported). With the wmlxgettext version on my dedicated repository, _("string") variants and the "local turns_left = _("(this turn left)", "(%d turns left)", n)" plural string example worked.

While waiting for further news I would suggest to use wmlxgettext from the dedicated repository:

https://github.com/AncientLich/wmlxgettext-unoff

@CelticMinstrel

This comment has been minimized.

Copy link
Member

commented Sep 2, 2019

Could it be that the latest wmlxgettext was committed to master, but not to 1.14? Just a thought.

@AncientLich

This comment has been minimized.

Copy link
Contributor

commented Sep 2, 2019

I have no clues this moment. I will take a couple of days in order to understand exactly what happened (and the reason of the problem)... I will inform you here (and I will notify when pull request will be ready)

@stevecotton

This comment has been minimized.

Copy link
Contributor Author

commented Sep 2, 2019

Hmm - I don't know if it's the right fix, but the code prior to #3481 had a regular expression that would swallow .* before the underscore. Changing the code to c will add the translation to wesnoth.pot.

>>> input=r'turn_counter = _("(this turn left)", "($remaining_turns turns left)", turn_count)'
>>> a = re.compile(r'((?:_)|(?:.*?\s+_))\s*\(')     # a = old version, before #3481
>>> b = re.compile(r'\b_\b\s*\(')                   # b = #3481
>>> c = re.compile(r'.*\b_\b\s*\(')                 # c = re-adding the .* to b
>>> a.match(input)
<re.Match object; span=(0, 17), match='turn_counter = _('>
>>> b.match(input)
>>> c.match(input)
<re.Match object; span=(0, 17), match='turn_counter = _('>
@CelticMinstrel

This comment has been minimized.

Copy link
Member

commented Sep 2, 2019

Does it add it to the pot file as a plural string, like this?

msgid "(this turn left)"
msgid_plural "($remaining_turns turns left)"
msgstr[0] ""
msgstr[1] ""
@AncientLich

This comment has been minimized.

Copy link
Contributor

commented Sep 3, 2019

All investigation finished. I confirm that the last bugfix was indeed bugged. Can't understand (and remember) why I didn't notice during my tests. However, the pull request should solve all the issues (including the one reported here)..... I reverted wmlxgettext with the old regexp, adding an explicit case for '(_' in order to check also the corner case that the original bugfix wanted to solve. Please do also other test with actual wesnoth lua code, and let me know if I miss something.

AncientLich added a commit to AncientLich/wesnoth that referenced this issue Sep 3, 2019
stevecotton added a commit that referenced this issue Sep 11, 2019
wmlxgettext: fix #4275: not handle objectives.lua's turn counter + ty…
…pefix

(cherry picked from commit e1c7c84)
(cherry picked from commit 0cf8da9)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.