[Widelands-dev] [Merge] lp:~widelands-dev/widelands/translation_stats into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/translation_stats into lp:widelands has been updated. Status: Needs review => Merged For more details, see: https://code.launchpad.net/~widelands-dev/widelands/translation_stats/+merge/332029 -- Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/translation_stats. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/translation_stats into lp:widelands
Continuous integration builds have changed state: Travis build 2754. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/297152789. Appveyor build 2566. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_translation_stats-2566. -- https://code.launchpad.net/~widelands-dev/widelands/translation_stats/+merge/332029 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/translation_stats. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/translation_stats into lp:widelands
I gave it another try, adding https:// results in a string truncation, so I'm leaving it out. I found another bug where I was parsing the wrong column, so I've added some error handling and am parsing the header info. Thanks for all your help in debugging this thing! @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/translation_stats/+merge/332029 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/translation_stats. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/translation_stats into lp:widelands
Continuous integration builds have changed state: Travis build 2746. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/296921592. Appveyor build 2558. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_translation_stats-2558. -- https://code.launchpad.net/~widelands-dev/widelands/translation_stats/+merge/332029 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/translation_stats. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/translation_stats into lp:widelands
I actually removed that https on purpose, it adds visual clutter and makes the string too long to fir. -- https://code.launchpad.net/~widelands-dev/widelands/translation_stats/+merge/332029 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/translation_stats. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/translation_stats into lp:widelands
Now its working :-) Could you please add 'https://' to the underlined link for "...please visit widelands.org/wiki/TranslatinWidelands" -- https://code.launchpad.net/~widelands-dev/widelands/translation_stats/+merge/332029 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/translation_stats. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/translation_stats into lp:widelands
Much better! No idea why I didn't use the csv format in the first place. -- https://code.launchpad.net/~widelands-dev/widelands/translation_stats/+merge/332029 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/translation_stats. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/translation_stats into lp:widelands
No, it doesn't work :( Whereas the version of pocount used by me has an option '--no-color', it is maybe better to use another pocount output format like --csv in combination with python csv module? https://docs.python.org/2/library/csv.html?highlight=csv#module-csv I guess the csv format provided by pocount will not change when switching the version of translate-toolkit. So the script might get more stable without using a fragile regexp. For comparison her is the output with option --csv on my machine: $:> pocount --csv po/map_the_green_plateau.wmf/la.po Filename, Translated Messages, Translated Source Words, Translated Target Words, Fuzzy Messages, Fuzzy Source Words, Untranslated Messages, Untranslated Source Words, Total Message, Total Source Words, Review Messages, Review Source Words po/map_the_green_plateau.wmf/la.po, 25, 438, 283, 0, 0, 0, 0, 25, 438 -- https://code.launchpad.net/~widelands-dev/widelands/translation_stats/+merge/332029 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/translation_stats. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/translation_stats into lp:widelands
Continuous integration builds have changed state: Travis build 2743. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/296669731. Appveyor build 2555. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_translation_stats-2555. -- https://code.launchpad.net/~widelands-dev/widelands/translation_stats/+merge/332029 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/translation_stats. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/translation_stats into lp:widelands
I am now testing for non-Word characters rather than tabs or spaces. Does that work for you? -- https://code.launchpad.net/~widelands-dev/widelands/translation_stats/+merge/332029 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/translation_stats. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/translation_stats into lp:widelands
My version of translate-toolkit is 2.2.5-1.
Pulled your changes but no change:
$:> python2 utils/update_translation_stats.py
Fetching translation stats .
ERROR: Invalid line in pocount output:
/home/kaputtnik/Quellcode/widelands-repo/translation_stats/po/map_the_green_plateau.wmf/la.po
source words: total: 438 | 438t 0f 0u | 100.0%t 0.0%f
0.0%u
I have tried to print the line showing escape sequences by adjusting line 106
like "...line.encode('string_escape'))" and the result is:
$:> python2 utils/update_translation_stats.py
Fetching translation stats .
ERROR: Invalid line in pocount output:
\x1b[95m/home/kaputtnik/Quellcode/widelands-repo/translation_stats/po/map_the_green_plateau.wmf/la.po\x1b[0m
source words: total: 438\t|
\x1b[92m438\x1b[0mt\t\x1b[93m0\x1b[0mf\t\x1b[91m0\x1b[0mu\t|
\x1b[92m100.0%\x1b[0mt\t\x1b[93m0.0%\x1b[0mf\t\x1b[91m0.0%\x1b[0mu
So tabs are there plus escape sequences showing colored output.
--
https://code.launchpad.net/~widelands-dev/widelands/translation_stats/+merge/332029
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/translation_stats.
___
Mailing list: https://launchpad.net/~widelands-dev
Post to : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/translation_stats into lp:widelands
Continuous integration builds have changed state: Travis build 2729. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/295908441. Appveyor build 2541. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_translation_stats-2541. -- https://code.launchpad.net/~widelands-dev/widelands/translation_stats/+merge/332029 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/translation_stats. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/translation_stats into lp:widelands
Just want to mention that i did run the file with python2 :-) This is what i get now: $:> python2 utils/update_translation_stats.py Fetching translation stats . ERROR: Invalid line in pocount output: /home/kaputtnik/Quellcode/widelands-repo/translation_stats/po/map_the_green_plateau.wmf/la.po source words: total: 438 | 438t 0f 0u | 100.0%t 0.0%f 0.0%u -- https://code.launchpad.net/~widelands-dev/widelands/translation_stats/+merge/332029 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/translation_stats. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/translation_stats into lp:widelands
It fails for me with Python3 too, but not with Python2. Good idea about the error handling though. I've added some :) -- https://code.launchpad.net/~widelands-dev/widelands/translation_stats/+merge/332029 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/translation_stats. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/translation_stats into lp:widelands
Review: Needs Fixing
The regex doesn't match here and the match is always 'None', resulting in the
List out of range error.
Don't know how to fix the regex... but maybe adding an else clause? See diff
comment.
Diff comments:
>
> === added file 'utils/update_translation_stats.py'
> --- utils/update_translation_stats.py 1970-01-01 00:00:00 +
> +++ utils/update_translation_stats.py 2017-11-01 15:02:19 +
> @@ -0,0 +1,127 @@
> +#!/usr/bin/python2
> +# encoding: utf-8
> +
> +
> +"""Uses pocount from the Translate Toolkit to write translation statistics to
> +data/i18n/translation_stats.conf.
> +
> +You will need to have the Translate Toolkit installed:
> +http://toolkit.translatehouse.org/
> +
> +For Debian-based Linux: sudo apt-get install translate-toolkit
> +
> +"""
> +
> +from collections import defaultdict
> +from subprocess import call, check_output, CalledProcessError
> +import os.path
> +import re
> +import subprocess
> +import sys
> +import traceback
> +
> +#
> +# Data Containers #
> +#
> +
> +
> +class TranslationStats:
> +"""Total source words and translated source words."""
> +
> +def __init__(self):
> +# We need the total only once, but since the entries come in per
> +# directory rather than per locale, we just store it here to keep the
> +# algorithm simpler
> +self.total = 0
> +self.translated = 0
> +
> +
> +#
> +# Main Loop #
> +#
> +
> +def generate_translation_stats(po_dir, output_file):
> +locale_stats = defaultdict(TranslationStats)
> +
> +sys.stdout.write('Fetching translation stats ')
> +
> +# We get errors for non-po files in the base po dir, so we have to walk
> +# the subdirs.
> +for subdir in sorted(os.listdir(po_dir), key=str.lower):
> +subdir = os.path.join(po_dir, subdir)
> +if not os.path.isdir(subdir):
> +continue
> +
> +sys.stdout.write('.')
> +sys.stdout.flush()
> +
> +try:
> +# We need shell=True, otherwise we get "No such file or
> directory".
> +stats_output = check_output(
> +['pocount ' + subdir + ' --short-words'],
> stderr=subprocess.STDOUT, shell=True)
> +if 'ERROR' in stats_output:
> +print('\nError running pocount:\n' +
> stats_output.split('\n', 0)
> + [0]) + '\nAborted creating translation statistics.'
> +return False
> +
> +except CalledProcessError:
> +print('Failed to run pocount:\n FILE: ' + po_dir +
> + '\n ' + stats_output.split('\n', 1)[1])
> +return False
> +
> +result = stats_output.split('\n')
> +
> +# Format provided by pocount:
> +# /home//po//.po source words: total:
> 1701| 500t 0f 1201u | 29%t 0%f 70%u
> +regex_translated = re.compile(
> +r"/\S+/(\w+)\.po\s+source words: total: (\d+)\t\|
> (\d+)t\t\d+f\t\d+u\t\| (\d+)%t\t\d+%f\t\d+%u")
> +
> +for line in result:
> +match = regex_translated.match(line)
> +if match:
> +entry = TranslationStats()
> +locale = match.group(1)
> +
> +if locale in locale_stats:
> +entry = locale_stats[locale]
> +
> +entry.total = entry.total + int(match.group(2))
> +entry.translated = entry.translated + int(match.group(3))
> +locale_stats[locale] = entry
> +
Add an else-clause?
else:
print('Regex didn't match...")
return False
> +print('\n\nLocale\tTotal\tTranslated')
> +print('--\t-\t--')
> +
> +# The total goes in a [global] section and is identical for all locales
> +result = '[global]\n'
> +result = result + 'total=' +
> str(locale_stats[locale_stats.keys()[0]].total) + '\n\n'
> +
> +# Write translation stats for all locales
> +for locale in sorted(locale_stats.keys(), key=str.lower):
> +entry = locale_stats[locale]
> +print(locale + '\t' + str(entry.total) + '\t' +
> str(entry.translated))
> +result = result + '[' + locale + ']\n'
> +result = result + 'translated=' + str(entry.translated) + '\n\n'
> +
> +with open(output_file, 'w+') as destination:
> +destination.write(result[:-1]) # Strip the final \n
> +print('\nResult written to ' + output_file)
> +return True
> +
> +
> +def main():
> +try:
> +po_dir = os.path.abspath(os.path.join(
> +os.path.dirname(__file__), '../po'))
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/translation_stats into lp:widelands
Thanks for pointing that out - there are multiple things in that script that won't work with Python 3 and a fix isn't trivial, so I've changed the Python header. -- https://code.launchpad.net/~widelands-dev/widelands/translation_stats/+merge/332029 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/translation_stats. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/translation_stats into lp:widelands
Continuous integration builds have changed state: Travis build 2722. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/295046876. Appveyor build 2534. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_translation_stats-2534. -- https://code.launchpad.net/~widelands-dev/widelands/translation_stats/+merge/332029 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/translation_stats. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/translation_stats into lp:widelands
I have run the update_translation_stats.py: Looks like this could be run only with python2. Maybe add a comment? I got a list out of range error: $ > python2 utils/update_translation_stats.py Fetching translation stats ... Locale Total Translated -- - -- Something went wrong: Traceback (most recent call last): File "utils/update_translation_stats.py", line 118, in main result = generate_translation_stats(po_dir, output_file) File "utils/update_translation_stats.py", line 97, in generate_translation_stats result = result + 'total=' + str(locale_stats[locale_stats.keys()[0]].total) + '\n\n' IndexError: list index out of range Do i miss something? -- https://code.launchpad.net/~widelands-dev/widelands/translation_stats/+merge/332029 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/translation_stats. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/translation_stats into lp:widelands
Review: Resubmit This should be ready now. I left the data container in Python as it is with the total for each locale, because the entires come in per directory rather than per locale. So, I sacrificed some memory in favour of code readability + performance. -- https://code.launchpad.net/~widelands-dev/widelands/translation_stats/+merge/332029 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/translation_stats. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/translation_stats into lp:widelands
Thanks for the comment, I mixed something up there. Will replace the vector
with a map and get rid of the std::sort call.
Diff comments:
>
> === modified file 'src/ui_fsmenu/options.cc'
> --- src/ui_fsmenu/options.cc 2017-09-11 16:59:41 +
> +++ src/ui_fsmenu/options.cc 2017-10-24 19:19:16 +
> @@ -432,7 +421,8 @@
> language_dropdown_.add(_("Try system language"), "", nullptr,
> current_locale == "");
> language_dropdown_.add("English", "en", nullptr, current_locale ==
> "en");
>
> - // Add translation directories to the list
> + // Add translation directories to the list. We are using a container
> that will support std::sort
> + // and that will avoid any eventual hash collisions on the sortname.
OK, I will go for a Map then and reimplement this. I've had my head stuck in
Java at my other job, so I'm used to maps being HashMaps now.
> std::vector entries;
> std::string selected_locale;
>
--
https://code.launchpad.net/~widelands-dev/widelands/translation_stats/+merge/332029
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/translation_stats.
___
Mailing list: https://launchpad.net/~widelands-dev
Post to : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/translation_stats into lp:widelands
Diff comments:
>
> === modified file 'src/ui_fsmenu/options.cc'
> --- src/ui_fsmenu/options.cc 2017-09-11 16:59:41 +
> +++ src/ui_fsmenu/options.cc 2017-10-24 19:19:16 +
> @@ -432,7 +421,8 @@
> language_dropdown_.add(_("Try system language"), "", nullptr,
> current_locale == "");
> language_dropdown_.add("English", "en", nullptr, current_locale ==
> "en");
>
> - // Add translation directories to the list
> + // Add translation directories to the list. We are using a container
> that will support std::sort
> + // and that will avoid any eventual hash collisions on the sortname.
std::map is not a hashmap, but a tree map. So the ordering is guaranteed to be
always exactly the same as using std::sort on a vector, given the same
comparison function.
> std::vector entries;
> std::string selected_locale;
>
--
https://code.launchpad.net/~widelands-dev/widelands/translation_stats/+merge/332029
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/translation_stats.
___
Mailing list: https://launchpad.net/~widelands-dev
Post to : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/translation_stats into lp:widelands
I just pushed another update, where I write the total only once as suggested by
Kaputtnik. I still need to clean up the Python script incl. the regex.
Diff comments:
>
> === modified file 'src/ui_fsmenu/options.cc'
> --- src/ui_fsmenu/options.cc 2017-09-11 16:59:41 +
> +++ src/ui_fsmenu/options.cc 2017-10-14 16:12:18 +
> @@ -432,7 +421,7 @@
> language_dropdown_.add(_("Try system language"), "", nullptr,
> current_locale == "");
> language_dropdown_.add("English", "en", nullptr, current_locale ==
> "en");
>
> - // Add translation directories to the list
> + // Add translation directories to the list. We use a vector so we can
> call std::sort on it.
The key to sort for is coming off Transifex, and I don't trust it not to
produce hash collisions, ever. For example, Japanese is "Nihongo" to make sure
that it will be sorted under n. If we went with the locale code "ja", we would
end up under j, which is awkward for Japanese speakers. I have added another
comment.
> std::vector entries;
> std::string selected_locale;
>
> @@ -484,6 +475,67 @@
> }
> }
>
> +/**
> + * Updates the language statistics message according to the currently
> selected locale.
> + * @param include_system_lang We only want to include the system lang if it
> matches the Widelands
> + * locale.
> + */
> +void FullscreenMenuOptions::update_language_stats(bool include_system_lang) {
> + int percent = 100;
> + std::string message = "";
> + if (language_dropdown_.has_selection()) {
> + std::string locale = language_dropdown_.get_selected();
> + // Empty locale means try system locale
> + if (locale.empty() && include_system_lang) {
> + std::vector parts;
> + boost::split(parts, i18n::get_locale(),
> boost::is_any_of("."));
> + if (language_entries_.count(parts[0]) == 1) {
> + locale = parts[0];
> + } else {
> + boost::split(parts, parts[0],
> boost::is_any_of("@"));
> + if (language_entries_.count(parts[0]) == 1) {
> + locale = parts[0];
> + } else {
> + boost::split(parts, parts[0],
> boost::is_any_of("_"));
> + if (language_entries_.count(parts[0])
> == 1) {
> + locale = parts[0];
> + }
> + }
> + }
> + }
> +
> + // If we have the locale, grab the stats and set the message
> + if (language_entries_.count(locale) == 1) {
> + try {
> + const LanguageEntry& entry =
> language_entries_[locale];
> + Profile prof("i18n/translation_stats.conf");
> + Section& s = prof.get_safe_section(locale);
> + percent = floor(100.f * s.get_int("translated")
> / s.get_int("total"));
> + if (percent == 100) {
> + message = (boost::format(_("The
> translation into %s is complete.")) %
> +
> entry.descname)
> + .str();
> + } else {
> + message = (boost::format(_("The
> translation into %s is %d%% complete.")) %
> +
> entry.descname % percent)
> + .str();
> + }
> + } catch (...) {
> + }
> + }
> + }
> +
> + // We will want some help with incomplete translations
> + if (percent <= 90) {
OK, done.
> + message = message + " " +
> + (boost::format(_("If you wish to help us translate,
> please visit %s")) %
> +" underline=1>widelands.org/wiki/TranslatingWidelands")
> + .str();
> + }
> + // Make font a bit smaller so the link will fit at 800x600 resolution.
> + translation_info_.set_text(as_uifont(message, 12));
> +}
> +
> void FullscreenMenuOptions::clicked_apply() {
>
> end_modal(FullscreenMenuBase::MenuTarget::kApplyOptions);
> }
--
https://code.launchpad.net/~widelands-dev/widelands/translation_stats/+merge/332029
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/translation_stats.
___
Mailing list: https://launchpad.net/~widelands-dev
Post to : widelands-dev@lists.launchpad.net
Unsubscribe :
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/translation_stats into lp:widelands
more comments inline.
Diff comments:
>
> === modified file 'src/ui_fsmenu/options.cc'
> --- src/ui_fsmenu/options.cc 2017-09-11 16:59:41 +
> +++ src/ui_fsmenu/options.cc 2017-10-14 16:12:18 +
> @@ -432,7 +421,7 @@
> language_dropdown_.add(_("Try system language"), "", nullptr,
> current_locale == "");
> language_dropdown_.add("English", "en", nullptr, current_locale ==
> "en");
>
> - // Add translation directories to the list
> + // Add translation directories to the list. We use a vector so we can
> call std::sort on it.
My question is why we are not using a std::map then - it is always sorted and
does not require std::sort.
> std::vector entries;
> std::string selected_locale;
>
> @@ -484,6 +475,67 @@
> }
> }
>
> +/**
> + * Updates the language statistics message according to the currently
> selected locale.
> + * @param include_system_lang We only want to include the system lang if it
> matches the Widelands
> + * locale.
> + */
> +void FullscreenMenuOptions::update_language_stats(bool include_system_lang) {
> + int percent = 100;
> + std::string message = "";
> + if (language_dropdown_.has_selection()) {
> + std::string locale = language_dropdown_.get_selected();
> + // Empty locale means try system locale
> + if (locale.empty() && include_system_lang) {
> + std::vector parts;
> + boost::split(parts, i18n::get_locale(),
> boost::is_any_of("."));
> + if (language_entries_.count(parts[0]) == 1) {
> + locale = parts[0];
> + } else {
> + boost::split(parts, parts[0],
> boost::is_any_of("@"));
> + if (language_entries_.count(parts[0]) == 1) {
> + locale = parts[0];
> + } else {
> + boost::split(parts, parts[0],
> boost::is_any_of("_"));
> + if (language_entries_.count(parts[0])
> == 1) {
> + locale = parts[0];
> + }
> + }
> + }
> + }
> +
> + // If we have the locale, grab the stats and set the message
> + if (language_entries_.count(locale) == 1) {
> + try {
> + const LanguageEntry& entry =
> language_entries_[locale];
> + Profile prof("i18n/translation_stats.conf");
> + Section& s = prof.get_safe_section(locale);
> + percent = floor(100.f * s.get_int("translated")
> / s.get_int("total"));
> + if (percent == 100) {
> + message = (boost::format(_("The
> translation into %s is complete.")) %
> +
> entry.descname)
> + .str();
> + } else {
> + message = (boost::format(_("The
> translation into %s is %d%% complete.")) %
> +
> entry.descname % percent)
> + .str();
> + }
> + } catch (...) {
> + }
> + }
> + }
> +
> + // We will want some help with incomplete translations
> + if (percent <= 90) {
I am fine with 90, just add a comment for the rationale here.
> + message = message + " " +
> + (boost::format(_("If you wish to help us translate,
> please visit %s")) %
> +" underline=1>widelands.org/wiki/TranslatingWidelands")
> + .str();
> + }
> + // Make font a bit smaller so the link will fit at 800x600 resolution.
> + translation_info_.set_text(as_uifont(message, 12));
> +}
> +
> void FullscreenMenuOptions::clicked_apply() {
>
> end_modal(FullscreenMenuBase::MenuTarget::kApplyOptions);
> }
--
https://code.launchpad.net/~widelands-dev/widelands/translation_stats/+merge/332029
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/translation_stats.
___
Mailing list: https://launchpad.net/~widelands-dev
Post to : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/translation_stats into lp:widelands
Answers to SirVer's comments in-line.
> Translation stats: I guess the count of 'total' is ever the same, because
> English is the base language. Is it needed to store this value for each
> language?
You are right, I should refactor that.
> The text for RTL languages are messed up somehow. I guess this will be fixed
> if the string is translated:
This is an issue with the BiDi support in the font renderer, so I'd rather not
change this just to hide the bug. And there are actually translators who don't
speak English that well - I was recently at a localizer's meeting and having a
language other than English in the translation tool's UI was a desired feature
for some of them.
Diff comments:
>
> === modified file 'src/ui_fsmenu/options.cc'
> --- src/ui_fsmenu/options.cc 2017-09-11 16:59:41 +
> +++ src/ui_fsmenu/options.cc 2017-10-14 16:12:18 +
> @@ -432,7 +421,7 @@
> language_dropdown_.add(_("Try system language"), "", nullptr,
> current_locale == "");
> language_dropdown_.add("English", "en", nullptr, current_locale ==
> "en");
>
> - // Add translation directories to the list
> + // Add translation directories to the list. We use a vector so we can
> call std::sort on it.
I ran into issues when I tried to refactor this I guess - I like to comment
decisions that cost me significant time. I have changed the comment to "We are
using a container that will support std::sort."
> std::vector entries;
> std::string selected_locale;
>
> @@ -484,6 +475,67 @@
> }
> }
>
> +/**
> + * Updates the language statistics message according to the currently
> selected locale.
> + * @param include_system_lang We only want to include the system lang if it
> matches the Widelands
> + * locale.
> + */
> +void FullscreenMenuOptions::update_language_stats(bool include_system_lang) {
> + int percent = 100;
> + std::string message = "";
> + if (language_dropdown_.has_selection()) {
> + std::string locale = language_dropdown_.get_selected();
> + // Empty locale means try system locale
> + if (locale.empty() && include_system_lang) {
No, we can't, because my system locale is "gd_GB.UTF-8", but the Gettext locale
used is "gd". The order for keywords (.@_) is clearly defined by the standard
though, so we shouldn't run into any issues here.
> + std::vector parts;
> + boost::split(parts, i18n::get_locale(),
> boost::is_any_of("."));
> + if (language_entries_.count(parts[0]) == 1) {
> + locale = parts[0];
> + } else {
> + boost::split(parts, parts[0],
> boost::is_any_of("@"));
> + if (language_entries_.count(parts[0]) == 1) {
> + locale = parts[0];
> + } else {
> + boost::split(parts, parts[0],
> boost::is_any_of("_"));
> + if (language_entries_.count(parts[0])
> == 1) {
> + locale = parts[0];
> + }
> + }
> + }
> + }
> +
> + // If we have the locale, grab the stats and set the message
> + if (language_entries_.count(locale) == 1) {
> + try {
> + const LanguageEntry& entry =
> language_entries_[locale];
> + Profile prof("i18n/translation_stats.conf");
> + Section& s = prof.get_safe_section(locale);
> + percent = floor(100.f * s.get_int("translated")
> / s.get_int("total"));
> + if (percent == 100) {
> + message = (boost::format(_("The
> translation into %s is complete.")) %
> +
> entry.descname)
> + .str();
> + } else {
> + message = (boost::format(_("The
> translation into %s is %d%% complete.")) %
> +
> entry.descname % percent)
> + .str();
> + }
> + } catch (...) {
> + }
> + }
> + }
> +
> + // We will want some help with incomplete translations
> + if (percent <= 90) {
Yes, it is arbitrary. A localizer can fall behind sometimes without us needing
any help though, e.g. I haven't translated the newest campaign scenario yet,
but I will before the next release. I sort of borrowed 90% from the 0AD
project, which is their cutoff to allow a
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/translation_stats into lp:widelands
Review: Approve
code lgtm, a few nits.
not tested.
Diff comments:
>
> === modified file 'src/ui_fsmenu/options.cc'
> --- src/ui_fsmenu/options.cc 2017-09-11 16:59:41 +
> +++ src/ui_fsmenu/options.cc 2017-10-14 16:12:18 +
> @@ -432,7 +421,7 @@
> language_dropdown_.add(_("Try system language"), "", nullptr,
> current_locale == "");
> language_dropdown_.add("English", "en", nullptr, current_locale ==
> "en");
>
> - // Add translation directories to the list
> + // Add translation directories to the list. We use a vector so we can
> call std::sort on it.
I do not get that comment... We could also use a std::map or std::set and it
would automatically be sorted? Looking through the code std::map seems most
appropriate.
> std::vector entries;
> std::string selected_locale;
>
> @@ -484,6 +475,67 @@
> }
> }
>
> +/**
> + * Updates the language statistics message according to the currently
> selected locale.
> + * @param include_system_lang We only want to include the system lang if it
> matches the Widelands
> + * locale.
> + */
> +void FullscreenMenuOptions::update_language_stats(bool include_system_lang) {
> + int percent = 100;
> + std::string message = "";
> + if (language_dropdown_.has_selection()) {
> + std::string locale = language_dropdown_.get_selected();
> + // Empty locale means try system locale
> + if (locale.empty() && include_system_lang) {
this parsing seems fragile... Can we not avoid this?
> + std::vector parts;
> + boost::split(parts, i18n::get_locale(),
> boost::is_any_of("."));
> + if (language_entries_.count(parts[0]) == 1) {
> + locale = parts[0];
> + } else {
> + boost::split(parts, parts[0],
> boost::is_any_of("@"));
> + if (language_entries_.count(parts[0]) == 1) {
> + locale = parts[0];
> + } else {
> + boost::split(parts, parts[0],
> boost::is_any_of("_"));
> + if (language_entries_.count(parts[0])
> == 1) {
> + locale = parts[0];
> + }
> + }
> + }
> + }
> +
> + // If we have the locale, grab the stats and set the message
> + if (language_entries_.count(locale) == 1) {
> + try {
> + const LanguageEntry& entry =
> language_entries_[locale];
> + Profile prof("i18n/translation_stats.conf");
> + Section& s = prof.get_safe_section(locale);
> + percent = floor(100.f * s.get_int("translated")
> / s.get_int("total"));
> + if (percent == 100) {
> + message = (boost::format(_("The
> translation into %s is complete.")) %
> +
> entry.descname)
> + .str();
> + } else {
> + message = (boost::format(_("The
> translation into %s is %d%% complete.")) %
> +
> entry.descname % percent)
> + .str();
> + }
> + } catch (...) {
> + }
> + }
> + }
> +
> + // We will want some help with incomplete translations
> + if (percent <= 90) {
90 feels arbitrary. Why not 100?
> + message = message + " " +
> + (boost::format(_("If you wish to help us translate,
> please visit %s")) %
> +" underline=1>widelands.org/wiki/TranslatingWidelands")
> + .str();
> + }
> + // Make font a bit smaller so the link will fit at 800x600 resolution.
> + translation_info_.set_text(as_uifont(message, 12));
> +}
> +
> void FullscreenMenuOptions::clicked_apply() {
>
> end_modal(FullscreenMenuBase::MenuTarget::kApplyOptions);
> }
>
> === added file 'utils/update_translation_stats.py'
> --- utils/update_translation_stats.py 1970-01-01 00:00:00 +
> +++ utils/update_translation_stats.py 2017-10-14 16:12:18 +
> @@ -0,0 +1,120 @@
> +#!/usr/bin/env python
> +# encoding: utf-8
> +
> +
> +"""Uses pocount from the Translate Toolkit to write translation statistics to
> +data/i18n/translation_stats.conf.
> +
> +You will need to have the Translate Toolkit installed:
> +http://toolkit.translatehouse.org/
> +
> +For Debian-based Linux: sudo apt-get install
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/translation_stats into lp:widelands
This is good approach to get more translators, i think. Translation stats: I guess the count of 'total' is ever the same, because English is the base language. Is it needed to store this value for each language? The text for RTL languages are messed up somehow. I guess this will be fixed if the string is translated: "is 0% complete. If you whish to help us translate, please visit [language name] The translation intowidelands.org/wiki/TranlatingWidelands" But if some one is willing to help translate, he should know English. So i would suggest to keep the English sentence here. Anyway adding "https://; to the underlined words would be helpful to show that this is an internet address. -- https://code.launchpad.net/~widelands-dev/widelands/translation_stats/+merge/332029 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/translation_stats into lp:widelands. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/translation_stats into lp:widelands
Continuous integration builds have changed state: Travis build 2702. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/285758712. Appveyor build 2517. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_translation_stats-2517. -- https://code.launchpad.net/~widelands-dev/widelands/translation_stats/+merge/332029 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/translation_stats into lp:widelands. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/translation_stats into lp:widelands
GunChleoc has proposed merging lp:~widelands-dev/widelands/translation_stats
into lp:widelands.
Commit message:
Show translation stats next to the language selection menu and invite
translators if a translation is incomplete, with the help of the Translate
Toolkit.
- Added a new utils script "update_translation_stats.py" that will write
translation statistics to data/i18n/translation_stats.conf.
This script is now called on every translation pull.
- Added translation stats + invitation to translators to Options.
Requested reviews:
Widelands Developers (widelands-dev)
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/translation_stats/+merge/332029
Some translations have very little work accomplished, so I thought we'd better
tell the user so. This also gives us an opportunity to invite translators.
--
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/translation_stats into lp:widelands.
=== added file 'data/i18n/translation_stats.conf'
--- data/i18n/translation_stats.conf 1970-01-01 00:00:00 +
+++ data/i18n/translation_stats.conf 2017-10-09 19:42:37 +
@@ -0,0 +1,239 @@
+[ar]
+translated=3400
+total=41289
+
+[ast]
+translated=471
+total=41289
+
+[bg]
+translated=41289
+total=41289
+
+[br]
+translated=422
+total=41289
+
+[ca]
+translated=41289
+total=41289
+
+[cs]
+translated=40858
+total=41289
+
+[da]
+translated=40774
+total=41289
+
+[de]
+translated=41009
+total=41289
+
+[el]
+translated=493
+total=41289
+
+[en_CA]
+translated=558
+total=41289
+
+[en_GB]
+translated=40774
+total=41289
+
+[en_US]
+translated=7
+total=41289
+
+[eo]
+translated=4455
+total=41289
+
+[es]
+translated=39621
+total=41289
+
+[et]
+translated=628
+total=41289
+
+[eu]
+translated=1359
+total=41289
+
+[fa]
+translated=123
+total=41289
+
+[fi]
+translated=41289
+total=41289
+
+[fr]
+translated=40891
+total=41289
+
+[ga]
+translated=0
+total=41289
+
+[gd]
+translated=41289
+total=41289
+
+[gl]
+translated=10511
+total=41289
+
+[he]
+translated=422
+total=41289
+
+[hi]
+translated=26
+total=41289
+
+[hr]
+translated=3239
+total=41289
+
+[hu]
+translated=33263
+total=41289
+
+[ia]
+translated=126
+total=41289
+
+[id]
+translated=34
+total=41289
+
+[it]
+translated=36675
+total=41289
+
+[ja]
+translated=7025
+total=41289
+
+[jv]
+translated=22
+total=41289
+
+[ka]
+translated=4
+total=41289
+
+[ko]
+translated=209
+total=41289
+
+[krl]
+translated=14
+total=41289
+
+[la]
+translated=4012
+total=41289
+
+[lt]
+translated=202
+total=41289
+
+[mr]
+translated=7
+total=41289
+
+[ms]
+translated=4099
+total=41289
+
+[my]
+translated=30
+total=41289
+
+[nb]
+translated=9192
+total=41289
+
+[nds]
+translated=876
+total=41289
+
+[nl]
+translated=34028
+total=41289
+
+[nn]
+translated=2946
+total=41289
+
+[oc]
+translated=195
+total=41289
+
+[pl]
+translated=41009
+total=41289
+
+[pt]
+translated=28352
+total=41289
+
+[pt_BR]
+translated=14495
+total=41289
+
+[ro]
+translated=1125
+total=41289
+
+[ru]
+translated=41289
+total=41289
+
+[rw]
+translated=49
+total=41289
+
+[si]
+translated=283
+total=41289
+
+[sk]
+translated=39563
+total=41289
+
+[sl]
+translated=1046
+total=41289
+
+[sr]
+translated=177
+total=41289
+
+[sv]
+translated=38623
+total=41289
+
+[tr]
+translated=1942
+total=41289
+
+[uk]
+translated=3621
+total=41289
+
+[vi]
+translated=1199
+total=41289
+
+[zh_CN]
+translated=2245
+total=41289
+
+[zh_TW]
+translated=655
+total=41289
=== modified file 'src/ui_fsmenu/options.cc'
--- src/ui_fsmenu/options.cc 2017-09-11 16:59:41 +
+++ src/ui_fsmenu/options.cc 2017-10-09 19:42:37 +
@@ -36,6 +36,7 @@
#include "graphic/text/bidi.h"
#include "graphic/text/font_set.h"
#include "graphic/text_constants.h"
+#include "graphic/text_layout.h"
#include "helper.h"
#include "io/filesystem/layered_filesystem.h"
#include "logic/constants.h"
@@ -47,23 +48,6 @@
namespace {
-// Data model for the entries in the language selection list.
-struct LanguageEntry {
- LanguageEntry(const std::string& init_localename,
- const std::string& init_descname,
- const std::string& init_sortname)
- : localename(init_localename), descname(init_descname), sortname(init_sortname) {
- }
-
- bool operator<(const LanguageEntry& other) const {
- return sortname < other.sortname;
- }
-
- std::string localename; // ISO code for the locale
- std::string descname;// Native language name
- std::string sortname;// ASCII Language name used for sorting
-};
-
// Locale identifiers can look like this: ca...@valencia.utf-8
// The contents of 'selected_locale' will be changed to match the 'current_locale'
void find_selected_locale(std::string* selected_locale, const std::string& current_locale) {
@@ -120,21 +104,23 @@
// Tabs
tabs_(this, g_gr->images().get("images/ui_basic/but1.png"), UI::TabPane
