widelands-dev team mailing list archive

[SirVer <SirVer@xxxxxx>]

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 +0000
> +++ src/ui_fsmenu/options.cc	2017-10-14 16:12:18 +0000
> @@ -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<LanguageEntry> 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<std::string> 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")) %
> +		           "<font underline=1>widelands.org/wiki/TranslatingWidelands</font>")
> +		             .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>(FullscreenMenuBase::MenuTarget::kApplyOptions);
>  }
> 
> === added file 'utils/update_translation_stats.py'
> --- utils/update_translation_stats.py	1970-01-01 00:00:00 +0000
> +++ utils/update_translation_stats.py	2017-10-14 16:12:18 +0000
> @@ -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 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):
> +        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', 1)
> +                      [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/<snip>/po/<textdomain>/<locale>.po  source words: total: 1701	| 500t	0f	1201u	| 29%t	0%f	70%u
> +        regex_translated = re.compile(
> +            '/\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')

this regex is dangerous, instead use a raw python string: r''

> +
> +        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
> +
> +    print('\n\nLocale\tTotal\tTranslated')
> +    print('------\t-----\t----------')
> +    result = ''
> +    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'
> +        result = result + 'total=' + str(entry.total) + '\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'))
> +        output_file = os.path.abspath(os.path.join(
> +            os.path.dirname(__file__), '../data/i18n/translation_stats.conf'))
> +        result = generate_translation_stats(po_dir, output_file)
> +        return result
> +
> +    except Exception:
> +        print('Something went wrong:')
> +        traceback.print_exc()
> +        return 1
> +
> +if __name__ == '__main__':
> +    sys.exit(main())


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